Network packet socket support. See "man 7 packet" on Linux. #12564
Conversation
rveerama1
requested review from
jukkar,
pfalcon and
tbursztyka
as code owners
rveerama1
added
RFC
|
All checks are passing now. Review history of this comment for details about previous failed status. |
There was a problem hiding this comment.
-1 for patching any existing samples. Samples are samples - easy pieces of code for people to quickly learn from, not scare them away with walls of code.
Instead, please act on the comment made in #11229 - provide sample(s) which show that the same code works both in Linux and in Zephyr.
@pfalcon Jukka submitted this PR 2 months ago and he was on vacation on two different time intervals. I am trying to address comments from old PR too. |
ATM really not paying attention to -1 or +1. Look at my label DNM. I am not in hurry to merge this. I am just concentrating on architecture and design changes. How it would fit with current ongoing major changes from @tbursztyka |
Well, I can see at least the following: this actually implements RAW sockets (i.e. exposes via BSD Sockets API), and does so uses recently introduced struct socket_op_vtable. That's definitely the right way, thanks for using it. And the integration looks pretty clean, as expected. (Will need to look in more detail yet.) |
Good, thanks.
Sounds good, just trying to note high-level things which may need improvement. |
Codecov Report
@@ Coverage Diff @@
## master #12564 +/- ##
==========================================
- Coverage 48.78% 48.73% -0.05%
==========================================
Files 313 314 +1
Lines 46488 46523 +35
Branches 10729 10735 +6
==========================================
- Hits 22678 22675 -3
- Misses 19313 19340 +27
- Partials 4497 4508 +11
Continue to review full report at Codecov.
|
|
Nothing major changes. Dropped patches related to echo_server and echo_client. Now provided a separate raw sample which receives all the packets on the wire. |
|
Sample app updated to send some dummy data. |
|
Dropped unnecessary patches from original PR. So far no use at the moment. Just in case fyi dropped patches are (drivers: eth: native_posix: Add raw socket support , net: pkt: Add helpers to set/get the L2 protocol). |
|
Ok, this subthread is no longer visible by default, so let me bring it to the top level, as it's pretty important: (The talk is about net_context_find() function which was previously in this PR)
Right, so I wondered if this PR actually does that.
We definitely need to do something about this case. But cloning a packet should be the last, "nothing else worked" solution. Because we have a packet reference count exactly for such cases! If we don't make use of it, why do we need it at all? So, the first choice would be to just increment refcount for each queue the packet gets added to (packet socket vs normal socket). Problem: in many places in the stack a code deals in such a way as if it owns a packet completely - without actually checking refcount. @tbursztyka currently fixes a case with ethernet header, in #12709 (comment) I admit that socket TCP code is written with similar assumptions. We to go thru the stack and fix such cases.
Thanks, the smaller the initial code, the better. |
Packet socket expects packet with L2 header, normal socket expects only data (we have to remove L2 and L3 header). How could refcount mechanism works here with same net_pkt? |
That's more a question to @tbursztyka. What you write a requirement, he can tell us how to implement it. An obvious way is to provide "current position" pointer. Normal sockets move that pointer past the headers, packet sockets have it at the headers. The main point that moving pointer should be non-destructive in case of refcount > 1, and can be destructive if recount == 1 (these rules are ok for recv packets only of course, and with some additional rules of how refcounts are initially set up). |
There was a problem hiding this comment.
Well, I agree that @rveerama1 did a great job on refactoring this and addressing comments. My initial intention was to make sure there's no obviously wrong stuff, and just be +0, on it, because I still think that we're hasting with pushing this stuff into 1.14. But well, again, in circumstances we have, it's really good job, so let me be +1.
Major issue remains the sample - right from the start it was requested to follow a simple rule "If it's socket API, please provide a sample which works both on Linux and Zephyr". And yet the sample remains Zephyr-specific. @rveerama1, please rewrite it completely as the next step, remove threads, etc. No, you can use pthreads of course, but I don't see why they're needed for just a sample. How true example of POSIX/BSD portability is truly a must.
Ok, I will modify in another patch. Thanks. |
Add ETH_P_xxx protocol types if they are missing. After this we can use the protocol types when working with BSD sockets. Signed-off-by: Jukka Rissanen <[email protected]>
When creating net_pkt, the default value of net_pkt data_len was set to zero. In case of RAW sockets, when family is AF_PACKET, the data_len will be zero as there is no higher level protocol information. So in this case, the default value of data_len must be set to interface MTU Signed-off-by: Ravi kumar Veeramally <[email protected]>
Various defines and helpers for supporting packet sockets. Signed-off-by: Jukka Rissanen <[email protected]> Signed-off-by: Ravi kumar Veeramally <[email protected]>
As we are adding more protocol families and protocol types to connection handlers, some values might be same across different types. Current connection handler only stores proto type to match the handler, which is not enough if we add more types. Also combination of family and types may vary too. So adding family to connection handler to figure out best match. Also changing proto variable in net_conn from u8_t to u16_t. net_context has 16 bit proto. Signed-off-by: Ravi kumar Veeramally <[email protected]>
This commit adds basic packet socket support to net_context and allows application to receive or send network packets in raw format. Signed-off-by: Jukka Rissanen <[email protected]> Signed-off-by: Ravi kumar Veeramally <[email protected]>
If CONFIG_NET_SOCKETS_PACKET is enabled, then feed the packet to net_packet_socket_input() for processing. It will search for the net_contexts and if proper handler is found, pass the packet to connection handler. Signed-off-by: Ravi kumar Veeramally <[email protected]>
If packet family is AF_PACKET and CONFIG_NET_SOCKETS_PACKET is enabled, just handover the packet to driver for sending. L2 layer will not touch AF_PACKETs at the moment. Signed-off-by: Ravi kumar Veeramally <[email protected]>
rveerama1
force-pushed
the
sock_raw
branch
2 times, most recently
from
6ff39d6 to
5a10a8a
Compare
This commit adds packet socket support to socket api. This version supports basic packet socket features. Protocol family is AF_PACKET, type of socket is SOCK_RAW and proto type is ETH_P_ALL. The user will receive every packet (with L2 header) on the wire. For TX, the subsystem expects that the user has set all the protocol headers (L2 and L3) properly. Networking subsystem doesn't verify or alter the headers while sending or receiving the packets. This version supports packet socket over Etherent only. Also combination of other family and protocols doesn't work (i.e. Application can not open packet-socket and non packet-socket together). Signed-off-by: Ravi kumar Veeramally <[email protected]>
Sample application which opens a packet socket and receives every packet on the wire and send some dummy packet over socket. Simple demo of how to use packet sockets. Signed-off-by: Ravi kumar Veeramally <[email protected]>
Initial version of packet socket support to Networking subsystem. This PR is targeted for basic feature, like recv and send every packet on the wire (i.e AF_PACKET, SOCK_RAW and ETH_P_ALL). It means user's of socket API has to include all the headers (L2, L3 and so on) while sending, also the will receive all the packets including L2 header. This version supports raw socket over Ethernet only. Also combination of other family and protocols not supported in this version.