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

[300bps]

First, CircuitPython has great networking support! It is amazing how much you can do with it. While I'm aware of wifi.radio.ping, I've recently been attempting to manually create an ICMP ping as a RAW packet and have run into a bit of a speed bump. I think I see a fairly simple possible solution that I'll describe below. Note that I'm using CircuitPython 9 on a RPi Pico.

The issue: To create an ICMP packet, in addition to specifying a socket type of SOCK_RAW, you also need to be able to specify the protocol value (1 for ICMP). CPython includes the 'proto' parameter to pass this information. CircuitPython's implementation doesn't allow this to be passed, and in the source hard codes the value to 0. The signatures of both versions are: CircuitPython: socketpool.SocketPool().socket(family: int = AF_INET, type: int = SOCK_STREAM) CPython: socket.socket(family=AF_INET, type=SOCK_STREAM, proto=0, fileno=None)

Using wireshark, I've verified that a manually created ICMP packet contains a value of 0 in the internet header in the protocol field (https://datatracker.ietf.org/doc/html/rfc791). This protocol field is the one that would typically be filled with the value passed to socket in the 'proto' parameter WHEN type=SOCK_RAW.

The enhancement: After examining the code, I think that adding support for specifying the protocol for a raw packet would require the following:

1. Modify 'socketpool.SocketPool().socket' to match CPython by adding a 'proto=0' param after the 'type'.
2. Modify the block of code in circuitpython/ports/raspberrypi/common-hal/socketpool/Socket.C in the 'socketpool_socket' function to change:

       case SOCKETPOOL_SOCK_RAW: {
           socket->pcb.raw = raw_new(0);
           break;
       }

   to:

       case SOCKETPOOL_SOCK_RAW: {
           socket->pcb.raw = raw_new(proto);
           break;
       }

3. In the espressif port, the 'proto' parameter would have to be added as above, and it looks like the _socketpool_socket function should have the following change:

   } else { // SOCKETPOOL_SOCK_RAW
       socket_type = SOCK_RAW;
   }

   to:

   } else { // SOCKETPOOL_SOCK_RAW
       socket_type = SOCK_RAW;
       ipproto = proto;         // Assuming that 'proto' is the name of the new parameter added
   }

Comments: I think that those changes along with the the associated ones required by the CircuitPython/MicroPython architecture to modify the parameters to the 'socket' function would extend the capability of CP RAW sockets and bring it a little closer to CPython. I'll be the first to admit I'm not familiar enough with the CP C-code architecture to know if there are other cans of worms this opens. From the pure Python perspective, the addition of the 'proto' parameter to the 'socket' method shouldn't break existing code since it has a default value of 0 and is at the end of the parameter list.

Regardless of whether this gets adopted or not, kudos to the CircuitPython team for all of the hard work on this code base. It is a joy to use!

[carson-coder]

I think circuitpython wouldn't require more than what you said but I could try to fix / add this.

[dhalbert]

A PR would be welcome.

[carson-coder]

Could you provide some code or test the pull request for me. I tried but networking is not for me

[300bps]

Do you have a .uf2 file for the test build?  If so, I could test it out for you.Sent from my iPhoneOn Dec 24, 2023, at 4:09 PM, Carson @.***> wrote: Could you provide some code or test the pull request for me. I tried but networking is not for me

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

[carson-coder]

I can link you one but it's one of the build artifact from one of the builds

[carson-coder]

What device do you have

[300bps]

A Raspberry Pi Pico (RP2040). Sent from my iPhoneOn Dec 24, 2023, at 7:03 PM, Carson @.***> wrote: What device do you have

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

[300bps]

@carson-coder Any chance to link me the .uf2 for the RPi Pico that you’d like tested?

[carson-coder]

I can In a second

[carson-coder]

https://github.com/adafruit/circuitpython/actions/runs/7315146763/artifacts/1133347374 Here @300bps

[justmobilize]

Should this also be added to the ESPI32SPI Library to keep them similar?

[300bps]

@carson-coder Thank you. Will attempt to test in 1 to 2 hours and let you know.  Sent from my iPhoneOn Dec 27, 2023, at 11:41 AM, Carson @.***> wrote: https://github.com/adafruit/circuitpython/actions/runs/7315146763/artifacts/1133347374 Here @300bps

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[300bps]

@carson-coder: Kudos! My first tests of the changes that you implemented appear to work! Note that this is the "happy path" -- I haven't proceeded to more exhaustive testing including potential edge cases, but I was pleasantly surprised that the first test works (that almost never happens ;)! I'll do some more testing and update you with more results.

Can you link me to your fork that includes the source for these changes? I can examine that as well.

[300bps]

Should this also be added to the ESPI32SPI Library to keep them similar?

@justmobilize I took a quick look at the ESPI32SPI file you mentioned and it looks like that one already accepts and attempts to use the 'proto' parameter to the 'socket' call -- at least at that level. I didn't dig deeper to see what happens further down the stack of turtles, though ;)

[justmobilize]

@300bps it's passed into the init, but not used. Probably added to make signatures match CPython.

[300bps]

@carson-coder Posted my test results in the associated PR at: https://github.com/adafruit/circuitpython/pull/8752