Add proto to sockets #8752

carson-coder · 2023-12-23T23:37:21Z

carson-coder · 2023-12-23T23:51:38Z

espressif port is tested but I couldn't test the raspberry pi because I don't have a device to test it

carson-coder · 2023-12-23T23:54:49Z

looks like raspberrypi_pico and some other builds failed so I will try to fix that

jepler

This commit adds some files that should probably not have been added (os, os.path, etc) with many thousands of lines of content. Please rebase this branch so that these files are not added to our history. If that's beyond your git skills, I can help you figure out how.

The rest looks plausible, based on my small understanding of networking.

jepler · 2023-12-25T01:58:21Z

shared-bindings/socketpool/SocketPool.c

 //|         """Create a new socket
 //|
 //|         :param ~int family: AF_INET or AF_INET6
 //|         :param ~int type: SOCK_STREAM, SOCK_DGRAM or SOCK_RAW
+//|         :param ~int proto: Int


This change should probably also add some "proto" named constants to the socketpool object, such as IPPROTO_IP and IPPROTO_ICMP. It might also make sense to state that this value is only used for SOCK_RAW.

Ok. I can work on this tomorrow.

…r/circuitpython into add-proto-to-sockets Rebase

carson-coder · 2023-12-27T17:15:17Z

Could you help me with git rebase @jepler

dhalbert · 2023-12-27T17:17:39Z

Do you have `adafruit/circuitpython as a remote, e.g.:

$ git remote -v
adafruit	https://github.com/adafruit/circuitpython.git (fetch)
adafruit	https://github.com/adafruit/circuitpython.git (push)
origin	https://github.com/dhalbert/circuitpython.git (fetch)
origin	https://github.com/dhalbert/circuitpython.git (push)

It might be called upstream instead. If not add it as a remote.
So, to rebase:

$ git fetch adafruit
$ git rebase adafruit/main

There may well be conflicts that you have to resolve.

carson-coder · 2023-12-27T17:53:13Z

$ git fetch adafruit
$ git rebase adafruit/main

Could I do adafruit/8.2.x because I am adding this on the 8.2.x branch

carson-coder · 2023-12-27T17:58:11Z

I think I did it

dhalbert · 2023-12-27T18:03:50Z

ould I do adafruit/8.2.x because I am adding this on the 8.2.x branch

fer sure, and I guess you did

carson-coder · 2023-12-27T18:08:31Z

Yep

jepler · 2023-12-27T18:59:35Z

Looks like pre-commit wants a few more changes. Other than that, this looks good to me (but i didn't test)

dhalbert · 2023-12-27T19:22:33Z

What kind of network testing do you want to do with this? Can you write a ping?

300bps · 2023-12-29T00:28:06Z

shared-bindings/socketpool/SocketPool.c

 //|         """Create a new socket
 //|
 //|         :param ~int family: AF_INET or AF_INET6
 //|         :param ~int type: SOCK_STREAM, SOCK_DGRAM or SOCK_RAW
+//|         :param ~int proto: IPPROTO_TCP or IPPROTO_IP. Only works with SOCK_RAW 


I manually passed the value of proto = 1 for ICMP and it works to allow me to create a manual ICMP packet with a raw socket (I've tested it). Unfortunately, it doesn't look like IPPROTO_ICMP = 1 is defined anywhere, in the existing code base.

300bps · 2023-12-29T01:30:37Z

I've been testing the Raspberry Pi Pico version of this build (some comments were in the associated issue). The results are:

These code changes in this build SUCCESSFULLY allow me to create a raw ICMP (ping) socket by passing proto=1. Previously, this was not supported.

    import socketpool
    import wifi

    IP_HEADER_PROTOCOL_ICMP = 1
    sp = socketpool.SocketPool(wifi.radio)
    sp.socket(family=socketpool.AF_INET, type=socketpool.SOCK_RAW, proto=IP_HEADER_PROTOCOL_ICMP)

I've manually created ICMP ping (echo) raw packets and SUCCESSFULLY sent them and received ping responses from a number of sites: (1.1.1.1, 8.8.8.8, 1.0.0.1, and 8.8.4.4, along with my local router). I've tested it by continuously pinging once per second for several hours at a time. I've also run faster rapid-fire pings for shorter intervals successfully.
I checked the circuitpython codebase and didn't find an IPPROTO_ICMP = 1 or SOCKETPOOL_IPPROTO_ICMP = 1 that could be passed as a symbolic constant for the 'proto' parameter in the 'socket' call. This isn't necessary for the changes to be usable and functional. If this were desired to be added, the locations could be found by searching 'IPPROTO_IP' or 'IPPROTO_TCP'.
The only unexpected behavior that I've observed is: if I attempt to open 2 or more ICMP raw sockets, the last one opened functions correctly and the earlier ones repeatedly time out. I don't see a direct cause of that in the changes in this pull request, and think it seems most likely to be a consequence or limitation of the lower-level 'lwip' networking code.
All-in-all the changes in this PR do enable the ability to use raw sockets to build ICMP and other raw messages and it brings CP more in-line with regular python socket functionality. I recommend adopting it into the main for new CP9 builds.

Kudos @carson-coder for enabling this functionality.

300bps · 2023-12-29T01:32:51Z

What kind of network testing do you want to do with this? Can you write a ping?

@dhalbert I did successfully do this with the .uf2 from this PR for the RPi Pico and it works well. See my previous comment above for more detailed results. Thanks!

anecdata · 2023-12-29T02:16:53Z

I think socketpool.SocketPool.IPPROTO_ICMP could be added:

>>> help(socketpool.SocketPool)
object <class 'SocketPool'> is of type type
  socket -- <function>
  getaddrinfo -- <function>
  gaierror -- <class 'gaierror'>
  AF_INET -- 2
  AF_INET6 -- 10
  SOCK_STREAM -- 1
  SOCK_DGRAM -- 2
  SOCK_RAW -- 3
  TCP_NODELAY -- 1
  IPPROTO_TCP -- 6
  IPPROTO_IP -- 0
  IP_MULTICAST_TTL -- 5
  EAI_NONAME -- -2

circuitpython/shared-bindings/socketpool/SocketPool.h

Line 47 in 6627bd7

typedef enum {

carson-coder · 2023-12-29T02:45:36Z

For constantant for the proto parm I just use the Two that already exist

carson-coder · 2023-12-29T15:33:04Z

Should I add every IPPROTO_* constant. And what would I do for the socketpool.socket() docstring because listing every constant isn't viable because there are a lot

dhalbert · 2023-12-29T15:43:07Z

You could add only the ones that are actually implemented by Espressif or RP2040 wifi. And yes, add each one, with a docstring underneath. as class constants.

carson-coder · 2023-12-29T15:45:51Z

I just added each one and in the docstring for socket I said that proto can be IPPROTO_TCP, IPPROTO_IP or any other IPPROTO_ constant. and I added each constant in the class doc string. you can look at the next commit.

dhalbert · 2023-12-29T15:54:05Z

shared-bindings/socketpool/SocketPool.c

+//|     IPPROTO_AH: int
+//|     IPPROTO_DSTOPTS: int
+//|     IPPROTO_EGP: int
+//|     IPPROTO_ESP: int
+//|     IPPROTO_FRAGMENT: int
+//|     IPPROTO_GRE: int
+//|     IPPROTO_HOPOPTS: int
+//|     IPPROTO_ICMP: int
+//|     IPPROTO_ICMPV6: int
+//|     IPPROTO_IDP: int
+//|     IPPROTO_IGMP: int
+//|     IPPROTO_IPIP: int
+//|     IPPROTO_IPV6: int
+//|     IPPROTO_MPTCP: int
+//|     IPPROTO_NONE: int
+//|     IPPROTO_PIM: int
+//|     IPPROTO_PUP: int
+//|     IPPROTO_RAW: int
+//|     IPPROTO_ROUTING: int
+//|     IPPROTO_RSVP: int
+//|     IPPROTO_SCTP: int
+//|     IPPROTO_TP: int
+//|     IPPROTO_UDP: int
+//|     IPPROTO_UDPLITE: int


Adding them here does not make them available in Python You need to add them around line 221 in this file. But there are too many to add, and most will not be used. Just add a few of the common ones, including ICMP.

For instance, MicroPython defines these common ones:
https://github.com/micropython/micropython/blob/9feb0689eeaca5ce88aedcc680f997a3b4d0221c/extmod/modsocket.c#L640

IPPROTO_ICMP=1 and IPPROTO_RAW=255 are the 2 that'd I'd suggest explicitly adding to those that are already implemented in CP.

carson-coder · 2023-12-29T16:49:30Z

CPython's socket doesn't define IPPROTO_IPV4 but the micropython file that was linked did so I am not going to define it because it is not in python

jepler

I think it's good to merge if the CI comes up green. Thanks for taking care of all the details here!

carson-coder · 2023-12-29T18:54:27Z

Np

this was completed

300bps · 2023-12-29T21:48:09Z

Nice work!

@dhalbert GitHub newbie question here, but it looks like this was merged into a branch (8.2.x). How does it get into main so that it will be available in the CP9.0 codebase? Are there periodic merges from older branches or does it have to be manually done?

dhalbert · 2023-12-29T22:35:57Z

That was done here: #8769 by @jepler.

300bps · 2023-12-30T00:28:13Z

That was done here: #8769 by @jepler.

Awesome! Thanks for the 411.

carson-coder and others added 7 commits December 23, 2023 17:59

At proto support for espressif port + global

211c307

Add proto support for raspberrypi port

5d78150

fix bug

55aa000

Fix wrong arg pos

668fe0e

Fix socket.c

621df56

Fix proto arg pos

602c358

Make Precommit happy

7ca699a

Fix raspberrypi build

0e5df64

jepler suggested changes Dec 25, 2023

View reviewed changes

carson-coder added 4 commits December 27, 2023 11:52

Add changes

9611a7a

Remove some files

04c6ed3

Rebase

fb03b85

Merge branch 'add-proto-to-sockets' of https://github.com/carson-code…

01c4ee4

…r/circuitpython into add-proto-to-sockets Rebase

carson-coder added 4 commits December 27, 2023 12:56

Add proto support for raspberrypi port

7c72040

fix bug

a21f4c2

Make Precommit happy

79df32d

Fix raspberrypi build

9ebeac0

300bps reviewed Dec 29, 2023

View reviewed changes

300bps mentioned this pull request Dec 29, 2023

socketpool.SocketPool().socket: Add ability to specify protocol for raw sockets to more closely align with CPython. #8750

Closed

fix precommit

8c567cc

carson-coder added 2 commits December 29, 2023 10:46

Add socketpool.SocketPool.IPPROTO_ constants

2ae3347

Clean up some code

ba7918a

dhalbert previously requested changes Dec 29, 2023

View reviewed changes

carson-coder added 2 commits December 29, 2023 11:52

Remove some ipproto constants

da92e5d

Make precommit happy

ed60528

carson-coder requested a review from jepler December 29, 2023 16:55

carson-coder added 3 commits December 29, 2023 12:36

Fix compilation errors

a9585a0

Fix compilation errors

9feadb0

Make precommit happy

ee8b6f4

jepler approved these changes Dec 29, 2023

View reviewed changes

jepler merged commit 5c3361b into adafruit:8.2.x Dec 29, 2023

carson-coder deleted the add-proto-to-sockets branch December 29, 2023 21:03

jepler mentioned this pull request Dec 29, 2023

Merge 8.2.x #8769

Merged

carson-coder mentioned this pull request Dec 30, 2023

OSError Whenever running code #8775

Closed

dhalbert mentioned this pull request Jan 19, 2024

CPY core module socketpool request add attributes to be able to receive udp multicast packets #7556

Closed

Add proto to sockets #8752

Add proto to sockets #8752

Uh oh!

Conversation

carson-coder commented Dec 23, 2023

Uh oh!

carson-coder commented Dec 23, 2023

Uh oh!

carson-coder commented Dec 23, 2023

Uh oh!

jepler left a comment

Choose a reason for hiding this comment

Uh oh!

jepler Dec 25, 2023

Choose a reason for hiding this comment

Uh oh!

carson-coder Dec 25, 2023

Choose a reason for hiding this comment

Uh oh!

carson-coder commented Dec 27, 2023

Uh oh!

dhalbert commented Dec 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carson-coder commented Dec 27, 2023

Uh oh!

carson-coder commented Dec 27, 2023

Uh oh!

dhalbert commented Dec 27, 2023

Uh oh!

carson-coder commented Dec 27, 2023

Uh oh!

jepler commented Dec 27, 2023

Uh oh!

dhalbert commented Dec 27, 2023

Uh oh!

300bps Dec 29, 2023

Choose a reason for hiding this comment

Uh oh!

300bps commented Dec 29, 2023

Uh oh!

300bps commented Dec 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anecdata commented Dec 29, 2023

Uh oh!

carson-coder commented Dec 29, 2023

Uh oh!

carson-coder commented Dec 29, 2023

Uh oh!

dhalbert commented Dec 29, 2023

Uh oh!

carson-coder commented Dec 29, 2023

Uh oh!

dhalbert Dec 29, 2023

Choose a reason for hiding this comment

Uh oh!

300bps Dec 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carson-coder commented Dec 29, 2023

Uh oh!

jepler left a comment

Choose a reason for hiding this comment

Uh oh!

carson-coder commented Dec 29, 2023

Uh oh!

300bps commented Dec 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhalbert commented Dec 29, 2023

Uh oh!

300bps commented Dec 30, 2023

Uh oh!

Uh oh!

dhalbert commented Dec 27, 2023 •

edited

Loading

300bps commented Dec 29, 2023 •

edited

Loading

300bps Dec 29, 2023 •

edited

Loading

300bps commented Dec 29, 2023 •

edited

Loading