-
Notifications
You must be signed in to change notification settings - Fork 7.7k
net_core: Decide about l2-processing based on l2_processed-flag #93050
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,27 +64,15 @@ LOG_MODULE_REGISTER(net_core, CONFIG_NET_CORE_LOG_LEVEL); | |
#include "net_stats.h" | ||
|
||
#if defined(CONFIG_NET_NATIVE) | ||
static inline enum net_verdict process_data(struct net_pkt *pkt, | ||
bool is_loopback) | ||
static inline enum net_verdict process_data(struct net_pkt *pkt) | ||
{ | ||
int ret; | ||
bool locally_routed = false; | ||
|
||
net_pkt_set_l2_processed(pkt, false); | ||
|
||
/* Initial call will forward packets to SOCK_RAW packet sockets. */ | ||
ret = net_packet_socket_input(pkt, ETH_P_ALL); | ||
ret = net_packet_socket_input(pkt, ETH_P_ALL, SOCK_RAW); | ||
if (ret != NET_CONTINUE) { | ||
return ret; | ||
} | ||
|
||
/* If the packet is routed back to us when we have reassembled an IPv4 or IPv6 packet, | ||
* then do not pass it to L2 as the packet does not have link layer headers in it. | ||
*/ | ||
if (net_pkt_is_ip_reassembled(pkt)) { | ||
locally_routed = true; | ||
} | ||
|
||
/* If there is no data, then drop the packet. */ | ||
if (!pkt->frags) { | ||
NET_DBG("Corrupted packet (frags %p)", pkt->frags); | ||
|
@@ -93,8 +81,9 @@ static inline enum net_verdict process_data(struct net_pkt *pkt, | |
return NET_DROP; | ||
} | ||
|
||
if (!is_loopback && !locally_routed) { | ||
if (!net_pkt_is_l2_processed(pkt)) { | ||
ret = net_if_recv_data(net_pkt_iface(pkt), pkt); | ||
net_pkt_set_l2_processed(pkt, true); | ||
if (ret != NET_CONTINUE) { | ||
if (ret == NET_DROP) { | ||
NET_DBG("Packet %p discarded by L2", pkt); | ||
|
@@ -106,18 +95,13 @@ static inline enum net_verdict process_data(struct net_pkt *pkt, | |
} | ||
} | ||
|
||
net_pkt_set_l2_processed(pkt, true); | ||
|
||
/* L2 has modified the buffer starting point, it is easier | ||
* to re-initialize the cursor rather than updating it. | ||
*/ | ||
net_pkt_cursor_init(pkt); | ||
|
||
if (IS_ENABLED(CONFIG_NET_SOCKETS_PACKET_DGRAM)) { | ||
/* Consecutive call will forward packets to SOCK_DGRAM packet sockets | ||
* (after L2 removed header). | ||
*/ | ||
ret = net_packet_socket_input(pkt, net_pkt_ll_proto_type(pkt)); | ||
ret = net_packet_socket_input(pkt, net_pkt_ll_proto_type(pkt), SOCK_DGRAM); | ||
if (ret != NET_CONTINUE) { | ||
return ret; | ||
} | ||
|
@@ -131,9 +115,9 @@ static inline enum net_verdict process_data(struct net_pkt *pkt, | |
uint8_t vtc_vhl = NET_IPV6_HDR(pkt)->vtc & 0xf0; | ||
|
||
if (IS_ENABLED(CONFIG_NET_IPV6) && vtc_vhl == 0x60) { | ||
return net_ipv6_input(pkt, is_loopback); | ||
return net_ipv6_input(pkt); | ||
} else if (IS_ENABLED(CONFIG_NET_IPV4) && vtc_vhl == 0x40) { | ||
return net_ipv4_input(pkt, is_loopback); | ||
return net_ipv4_input(pkt); | ||
} | ||
|
||
NET_DBG("Unknown IP family packet (0x%x)", NET_IPV6_HDR(pkt)->vtc & 0xf0); | ||
|
@@ -148,10 +132,10 @@ static inline enum net_verdict process_data(struct net_pkt *pkt, | |
return NET_DROP; | ||
} | ||
|
||
static void processing_data(struct net_pkt *pkt, bool is_loopback) | ||
static void processing_data(struct net_pkt *pkt) | ||
{ | ||
again: | ||
switch (process_data(pkt, is_loopback)) { | ||
switch (process_data(pkt)) { | ||
case NET_CONTINUE: | ||
if (IS_ENABLED(CONFIG_NET_L2_VIRTUAL)) { | ||
/* If we have a tunneling packet, feed it back | ||
|
@@ -421,7 +405,9 @@ int net_try_send_data(struct net_pkt *pkt, k_timeout_t timeout) | |
* to RX processing. | ||
*/ | ||
NET_DBG("Loopback pkt %p back to us", pkt); | ||
processing_data(pkt, true); | ||
net_pkt_set_loopback(pkt, true); | ||
net_pkt_set_l2_processed(pkt, true); | ||
processing_data(pkt); | ||
ret = 0; | ||
goto err; | ||
} | ||
|
@@ -481,7 +467,6 @@ int net_try_send_data(struct net_pkt *pkt, k_timeout_t timeout) | |
|
||
static void net_rx(struct net_if *iface, struct net_pkt *pkt) | ||
{ | ||
bool is_loopback = false; | ||
size_t pkt_len; | ||
|
||
pkt_len = net_pkt_get_len(pkt); | ||
|
@@ -493,12 +478,13 @@ static void net_rx(struct net_if *iface, struct net_pkt *pkt) | |
if (IS_ENABLED(CONFIG_NET_LOOPBACK)) { | ||
#ifdef CONFIG_NET_L2_DUMMY | ||
if (net_if_l2(iface) == &NET_L2_GET_NAME(DUMMY)) { | ||
is_loopback = true; | ||
net_pkt_set_loopback(pkt, true); | ||
net_pkt_set_l2_processed(pkt, true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to break There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to find a solution for Then, maybe we could also move this into the layer 2 if: if (!net_pkt_is_l2_processed(pkt)) {
/* forward packets to SOCK_RAW packet sockets. */
ret = net_packet_socket_input(pkt, ETH_P_ALL);
if (ret != NET_CONTINUE) {
return ret;
}
ret = net_if_recv_data(net_pkt_iface(pkt), pkt);
net_pkt_set_l2_processed(pkt, true);
// ... But this will of course equally break the tests, that want to send a loopback packet and expect it to be passed to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: We also have tests, that check that Not sure, what to do with this either? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One more note (sorry for spam): we may even want to input the packet into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Which tests are you referring to, I don't see any packet sockets being used in IP fragmentation tests?
I don't think that's a viable approach (at least not without some even further refactoring) as L2 strips the L2 header, which is expected to be present in packet with SOCK_RAW socket.
Then perhaps we need yet another flag on the packet similar to
To be fair, I'm not sure if they have to (we currently only seem to use this "feature" in tests) but at the same time I'm not convinced on limiting functionality because of implementation details. Probably would be good to check how it works in other systems (with Linux in mind), i.e. whether There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My bad, I was confused.
Ok, makes sense.
Ok, will go this direction in future PRs. We can discuss then in detail. |
||
} | ||
#endif | ||
} | ||
|
||
processing_data(pkt, is_loopback); | ||
processing_data(pkt); | ||
|
||
net_print_statistics(); | ||
net_pkt_print(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referring to the previous commit, perhaps this should check for both flags, i.e.:
instead of setting
l2_processed
flag for loopback packets?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, will do but see comment below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2bd0fc4 reverts and tries a different approach.