Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Network raw socket support #11229

Closed
wants to merge 6 commits into from
Closed

Conversation

jukkar
Copy link
Member

@jukkar jukkar commented Nov 9, 2018

This is work-in-progress PR, it is by no means ready but is more in RFC state atm.

Things that are still missing:

  • adding BSD socket support
  • raw packet sending
  • adding code to network device drivers to support raw socket receiving
  • more testing
  • support disabling/removal L3 code if that is not needed

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 <jukka.rissanen@linux.intel.com>
Various defines and helpers for supporting raw sockets.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
These are used by the network device driver and net_core when
receiving the network packet.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
This commit adds basic raw socket support to net_context and
allows application to receive or send network packets in raw
format.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
We need to store the used upper layer protocol information into
the network packet so that the packet can be handled if raw
socket support is enabled.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
Print information about raw network packets received. Do not send
them back in this version.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar jukkar added In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on area: Networking DNM This PR should not be merged (Do Not Merge) labels Nov 9, 2018
@codecov-io
Copy link

Codecov Report

Merging #11229 into master will decrease coverage by 0.09%.
The diff coverage is 27.94%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #11229     +/-   ##
=========================================
- Coverage    53.1%   53.01%   -0.1%     
=========================================
  Files         218      218             
  Lines       26883    26926     +43     
  Branches     5952     5963     +11     
=========================================
- Hits        14277    14275      -2     
- Misses      10172    10209     +37     
- Partials     2434     2442      +8
Impacted Files Coverage Δ
subsys/net/ip/net_core.c 73.52% <ø> (ø) ⬆️
include/net/ethernet.h 56.25% <ø> (ø) ⬆️
subsys/net/ip/net_stats.h 39.16% <ø> (ø) ⬆️
include/net/net_ip.h 71.26% <ø> (ø) ⬆️
drivers/ethernet/eth_native_posix.c 24.8% <0%> (-0.4%) ⬇️
include/net/net_pkt.h 84.9% <0%> (-0.81%) ⬇️
subsys/net/ip/net_context.c 57.93% <3.12%> (-3.51%) ⬇️
include/net/net_context.h 66.66% <56.25%> (-11.91%) ⬇️
boards/posix/native_posix/timer_model.c 51.09% <0%> (-0.73%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bde26fb...ec10f87. Read the comment docs.

@jukkar jukkar added the RFC Request For Comments: want input from the community label Nov 9, 2018
@jukkar jukkar mentioned this pull request Nov 12, 2018
@pfalcon pfalcon added the area: Sockets Networking sockets label Nov 17, 2018
Copy link
Contributor

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR says "sockets", but at its current state, it doens't have changes for anything in net/lib/sockets/ (that's mentioned in the current description, yeah).

Raw sockets is very peculiar matter. Recv/send sides are quite inconsistent regarding for lower-level protocol header they return/accept or not.

I would like to see a sample application, in the vein of existing samples/net/sockets/, which can be compiled on both Linux and Zephyr, and behaves the same on both. (That's as a replacement of properly spec'ing what this is supposed to do - as we can't see to make people first explain what they want to do, and only then start dumping code.)

NET_CONTEXT_AF_INET = 0,
NET_CONTEXT_AF_INET6,
NET_CONTEXT_AF_PACKET,
} __packed;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why __packed?

NET_CONTEXT_SOCK_DGRAM = 0,
NET_CONTEXT_SOCK_STREAM,
NET_CONTEXT_SOCK_RAW,
} __packed;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

}

return AF_INET;
return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall it return AF_UNSPEC here instead?

@@ -0,0 +1,164 @@
/** @file
* @brief Generic raw socket connection related functions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gruik. I really hope you are going to merge all in connection.c instead of c/p 95% of its bare logic here just to add small raw dedicated stuff.

@@ -212,9 +237,11 @@ int net_context_get(sa_family_t family,
net_context_set_type(&contexts[i], type);
net_context_set_ip_proto(&contexts[i], ip_proto);

#if defined(CONFIG_NET_IPV4) || defined(CONFIG_NET_IPV6)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use IS_ENABLED()

@@ -997,6 +1097,28 @@ static int sendto(struct net_pkt *pkt,
}
} else
#endif /* CONFIG_NET_IPV4 */

#if defined(CONFIG_NET_SOCKET_RAW)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IS_ENABLED

@@ -149,6 +149,17 @@ struct net_stats_udp {
net_stats_t chkerr;
};

struct net_stats_raw {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the stats related part into its own patch

@@ -178,6 +178,12 @@ struct net_pkt {
u8_t ieee802154_rssi; /* Received Signal Strength Indication */
u8_t ieee802154_lqi; /* Link Quality Indicator */
#endif

#if defined(CONFIG_NET_SOCKET_RAW)
u16_t l2_proto; /* IEEE 802.3 protocol value of the L2 (in network
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put it in a union, I guess above 116 to 140 could be the right place: there are 2 u8_t, which would need to be combined in an anonymous struct, and that u16_t. something like that

Also, can you call it raw_proto instead?

@@ -321,6 +322,8 @@ static int read_data(struct eth_context *ctx, int fd)
count += frag->len;
} while (ret > 0);

type = NET_ETH_HDR(pkt)->type;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's something we'll need to think about. It piles up more stuff to add in every driver, while it could be centralized. I still would like to see this put into ethernet l2 somehow, here at this point you don't know if the packet is meant for a raw socket or not (and this approach would also make the union stuff as seen in previous patch impossible).

@@ -283,11 +298,14 @@ struct net_context {
#endif
} options;

/** Network interface assigned to this context */
u8_t iface;
/** Protocol (UDP, TCP or IEEE 802.3 protocol value */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing ')'

@rveerama1
Copy link
Collaborator

Closing this PR due to #12564. Future development will continue in #12564.

@rveerama1 rveerama1 closed this Jan 18, 2019
@ghost ghost removed the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Jan 18, 2019
@jukkar jukkar deleted the raw-socket branch February 29, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: Sockets Networking sockets DNM This PR should not be merged (Do Not Merge) RFC Request For Comments: want input from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants