Skip to content

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions include/zephyr/net/net_pkt.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ struct net_pkt {
uint8_t chksum_done : 1; /* Checksum has already been computed for
* the packet.
*/
uint8_t loopback : 1; /* Packet is a loop back packet. */
#if defined(CONFIG_NET_IP_FRAGMENT)
uint8_t ip_reassembled : 1; /* Packet is a reassembled IP packet. */
#endif
Expand Down Expand Up @@ -1020,6 +1021,17 @@ static inline void net_pkt_set_ipv6_fragment_id(struct net_pkt *pkt,
}
#endif /* CONFIG_NET_IPV6_FRAGMENT */

static inline bool net_pkt_is_loopback(struct net_pkt *pkt)
{
return !!(pkt->loopback);
}

static inline void net_pkt_set_loopback(struct net_pkt *pkt,
bool loopback)
{
pkt->loopback = loopback;
}

#if defined(CONFIG_NET_IP_FRAGMENT)
static inline bool net_pkt_is_ip_reassembled(struct net_pkt *pkt)
{
Expand Down
9 changes: 5 additions & 4 deletions subsys/net/ip/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ static void conn_raw_socket_deliver(struct net_pkt *pkt, struct net_conn *conn,
#endif /* defined(CONFIG_NET_SOCKETS_PACKET) || defined(CONFIG_NET_SOCKETS_INET_RAW) */

#if defined(CONFIG_NET_SOCKETS_PACKET)
enum net_verdict net_conn_packet_input(struct net_pkt *pkt, uint16_t proto)
enum net_verdict net_conn_packet_input(struct net_pkt *pkt, uint16_t proto, enum net_sock_type type)
{
bool raw_sock_found = false;
bool raw_pkt_continue = false;
Expand Down Expand Up @@ -670,15 +670,15 @@ enum net_verdict net_conn_packet_input(struct net_pkt *pkt, uint16_t proto)
continue; /* wrong family */
}

if (conn->type == SOCK_DGRAM && !net_pkt_is_l2_processed(pkt)) {
if (conn->type == SOCK_DGRAM && type == SOCK_RAW) {
/* If DGRAM packet sockets are present, we shall continue
* with this packet regardless the result.
*/
raw_pkt_continue = true;
continue; /* L2 not processed yet */
}

if (conn->type == SOCK_RAW && net_pkt_is_l2_processed(pkt)) {
if (conn->type == SOCK_RAW && type != SOCK_RAW) {
continue; /* L2 already processed */
}

Expand Down Expand Up @@ -727,10 +727,11 @@ enum net_verdict net_conn_packet_input(struct net_pkt *pkt, uint16_t proto)
return NET_CONTINUE;
}
#else
enum net_verdict net_conn_packet_input(struct net_pkt *pkt, uint16_t proto)
enum net_verdict net_conn_packet_input(struct net_pkt *pkt, uint16_t proto, enum net_sock_type type)
{
ARG_UNUSED(pkt);
ARG_UNUSED(proto);
ARG_UNUSED(type);

return NET_CONTINUE;
}
Expand Down
5 changes: 4 additions & 1 deletion subsys/net/ip/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,14 @@ int net_conn_update(struct net_conn_handle *handle,
*
* @param pkt Network packet holding received data
* @param proto LL protocol for the connection
* @param type socket type
*
* @return NET_OK if the packet was consumed, NET_CONTINUE if the packet should
* be processed further in the stack.
*/
enum net_verdict net_conn_packet_input(struct net_pkt *pkt, uint16_t proto);
enum net_verdict net_conn_packet_input(struct net_pkt *pkt,
uint16_t proto,
enum net_sock_type type);

/**
* @brief Called by net_core.c when an IP packet is received
Expand Down
4 changes: 2 additions & 2 deletions subsys/net/ip/ipv4.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ int net_ipv4_parse_hdr_options(struct net_pkt *pkt,
}
#endif

enum net_verdict net_ipv4_input(struct net_pkt *pkt, bool is_loopback)
enum net_verdict net_ipv4_input(struct net_pkt *pkt)
{
NET_PKT_DATA_ACCESS_CONTIGUOUS_DEFINE(ipv4_access, struct net_ipv4_hdr);
NET_PKT_DATA_ACCESS_DEFINE(udp_access, struct net_udp_hdr);
Expand Down Expand Up @@ -301,7 +301,7 @@ enum net_verdict net_ipv4_input(struct net_pkt *pkt, bool is_loopback)
net_pkt_update_length(pkt, pkt_len);
}

if (!is_loopback) {
if (!net_pkt_is_loopback(pkt)) {
if (net_ipv4_is_addr_loopback_raw(hdr->dst) ||
net_ipv4_is_addr_loopback_raw(hdr->src)) {
NET_DBG("DROP: localhost packet");
Expand Down
4 changes: 2 additions & 2 deletions subsys/net/ip/ipv4_fragment.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ static void reassemble_packet(struct net_ipv4_reassembly *reass)
/* We need to use the queue when feeding the packet back into the
* IP stack as we might run out of stack if we call processing_data()
* directly. As the packet does not contain link layer header, we
* MUST NOT pass it to L2 so there will be a special check for that
* in process_data() when handling the packet.
* MUST NOT pass it to L2 so mark it as l2_processed.
*/
net_pkt_set_l2_processed(pkt, true);
if (net_recv_data(net_pkt_iface(pkt), pkt) >= 0) {
return;
}
Expand Down
6 changes: 3 additions & 3 deletions subsys/net/ip/ipv6.c
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ static inline bool is_src_non_tentative_itself(const uint8_t *src)
return false;
}

enum net_verdict net_ipv6_input(struct net_pkt *pkt, bool is_loopback)
enum net_verdict net_ipv6_input(struct net_pkt *pkt)
{
NET_PKT_DATA_ACCESS_CONTIGUOUS_DEFINE(ipv6_access, struct net_ipv6_hdr);
NET_PKT_DATA_ACCESS_DEFINE(udp_access, struct net_udp_hdr);
Expand Down Expand Up @@ -537,7 +537,7 @@ enum net_verdict net_ipv6_input(struct net_pkt *pkt, bool is_loopback)
goto drop;
}

if (!is_loopback) {
if (!net_pkt_is_loopback(pkt)) {
if (net_ipv6_is_addr_loopback_raw(hdr->dst) ||
net_ipv6_is_addr_loopback_raw(hdr->src)) {
NET_DBG("DROP: ::1 packet");
Expand Down Expand Up @@ -631,7 +631,7 @@ enum net_verdict net_ipv6_input(struct net_pkt *pkt, bool is_loopback)
}

if ((IS_ENABLED(CONFIG_NET_ROUTING) || IS_ENABLED(CONFIG_NET_ROUTE_MCAST)) &&
!is_loopback && is_src_non_tentative_itself(hdr->src)) {
!net_pkt_is_loopback(pkt) && is_src_non_tentative_itself(hdr->src)) {
NET_DBG("DROP: src addr is %s", "mine");
goto drop;
}
Expand Down
4 changes: 2 additions & 2 deletions subsys/net/ip/ipv6_fragment.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,9 +346,9 @@ static void reassemble_packet(struct net_ipv6_reassembly *reass)
/* We need to use the queue when feeding the packet back into the
* IP stack as we might run out of stack if we call processing_data()
* directly. As the packet does not contain link layer header, we
* MUST NOT pass it to L2 so there will be a special check for that
* in process_data() when handling the packet.
* MUST NOT pass it to L2 so mark it as l2_processed.
*/
net_pkt_set_l2_processed(pkt, true);
if (net_recv_data(net_pkt_iface(pkt), pkt) >= 0) {
return;
}
Expand Down
44 changes: 15 additions & 29 deletions subsys/net/ip/net_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)) {
Copy link
Contributor

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.:

if (!net_pkt_is_loopback(pkt) && !net_pkt_is_l2_processed(pkt)) {

instead of setting l2_processed flag for loopback packets?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

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);
Expand All @@ -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;
}
Expand All @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to break AF_PACKET/SOCK_RAW sockets processing on loopback, some tests use this combination and now fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to find a solution for AF_PACKET/SOCK_RAW. It can not stay unconditionally at the begining of process_data, as a packet will soon pass there multiple times potentially (unless we want the packet to be inputted many times into a AF_PACKET/SOCK_RAW, which I assume we do not want). When we have deffered packet processing a packet may go through process_data many times with different stages of its processing. Initially, maybe l2_processed is false, then on second pass, l2_processed is true, but l3_processed is false, .... That is at least my vision.

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 AF_PACKET/SOCK_RAW. Do locally routed packets have to be inputed in those sockets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: We also have tests, that check that reassembled packets reach the raw sockets...

Not sure, what to do with this either?

Copy link
Contributor Author

@ClaCodes ClaCodes Jul 16, 2025

Choose a reason for hiding this comment

The 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 raw socket only after it was processed by L2. See zpacket_set_source_addr where it currently has to do ethernet specific address parsing, because the packet was not yet processed by L2, when it was inputed into the raw socket.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note: We also have tests, that check that reassembled packets reach the raw sockets...

Which tests are you referring to, I don't see any packet sockets being used in IP fragmentation tests?

we may even want to input the packet into raw socket only after it was processed by L2. See zpacket_set_source_addr where it currently has to do ethernet specific address parsing, because the packet was not yet processed by L2, when it was inputed into the raw socket.

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.

We need to find a solution for AF_PACKET/SOCK_RAW. It can not stay unconditionally at the begining of process_data, as a packet will soon pass there multiple times potentially (unless we want the packet to be inputted many times into a AF_PACKET/SOCK_RAW, which I assume we do not want). When we have deffered packet processing a packet may go through process_data many times with different stages of its processing. Initially, maybe l2_processed is false, then on second pass, l2_processed is true, but l3_processed is false, .... That is at least my vision.

Then perhaps we need yet another flag on the packet similar to l2_processed, something like raw_processed to ensure packet is delivered only once. For sure we do not want the packet socket to receive multiple copies of the same packet.

Do locally routed packets have to be inputed in those sockets?

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 AF_PACKET/SOCK_RAW socket is able to receive loopback packets.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

My bad, I was confused.

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.

Ok, makes sense.

Then perhaps we need yet another flag on the packet similar to l2_processed, something like raw_processed to ensure packet is delivered only once. For sure we do not want the packet socket to receive multiple copies of the same packet.

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();
Expand Down
1 change: 1 addition & 0 deletions subsys/net/ip/net_pkt.c
Original file line number Diff line number Diff line change
Expand Up @@ -2047,6 +2047,7 @@ static void clone_pkt_attributes(struct net_pkt *pkt, struct net_pkt *clone_pkt)
net_pkt_set_rx_timestamping(clone_pkt, net_pkt_is_rx_timestamping(pkt));
net_pkt_set_forwarding(clone_pkt, net_pkt_forwarding(pkt));
net_pkt_set_chksum_done(clone_pkt, net_pkt_is_chksum_done(pkt));
net_pkt_set_loopback(pkt, net_pkt_is_loopback(pkt));
net_pkt_set_ip_reassembled(pkt, net_pkt_is_ip_reassembled(pkt));
net_pkt_set_cooked_mode(clone_pkt, net_pkt_is_cooked_mode(pkt));
net_pkt_set_ipv4_pmtu(clone_pkt, net_pkt_ipv4_pmtu(pkt));
Expand Down
12 changes: 4 additions & 8 deletions subsys/net/ip/net_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,25 +177,21 @@ extern void loopback_enable_address_swap(bool swap_addresses);
#endif /* CONFIG_NET_TEST */

#if defined(CONFIG_NET_NATIVE)
enum net_verdict net_ipv4_input(struct net_pkt *pkt, bool is_loopback);
enum net_verdict net_ipv6_input(struct net_pkt *pkt, bool is_loopback);
enum net_verdict net_ipv4_input(struct net_pkt *pkt);
enum net_verdict net_ipv6_input(struct net_pkt *pkt);
extern void net_tc_tx_init(void);
extern void net_tc_rx_init(void);
#else
static inline enum net_verdict net_ipv4_input(struct net_pkt *pkt,
bool is_loopback)
static inline enum net_verdict net_ipv4_input(struct net_pkt *pkt)
{
ARG_UNUSED(pkt);
ARG_UNUSED(is_loopback);

return NET_CONTINUE;
}

static inline enum net_verdict net_ipv6_input(struct net_pkt *pkt,
bool is_loopback)
static inline enum net_verdict net_ipv6_input(struct net_pkt *pkt)
{
ARG_UNUSED(pkt);
ARG_UNUSED(is_loopback);

return NET_CONTINUE;
}
Expand Down
6 changes: 4 additions & 2 deletions subsys/net/ip/packet_socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ LOG_MODULE_REGISTER(net_sockets_raw, CONFIG_NET_SOCKETS_LOG_LEVEL);
#include "connection.h"
#include "packet_socket.h"

enum net_verdict net_packet_socket_input(struct net_pkt *pkt, uint16_t proto)
enum net_verdict net_packet_socket_input(struct net_pkt *pkt,
uint16_t proto,
enum net_sock_type type)
{
sa_family_t orig_family;
enum net_verdict net_verdict;
Expand All @@ -41,7 +43,7 @@ enum net_verdict net_packet_socket_input(struct net_pkt *pkt, uint16_t proto)

net_pkt_set_family(pkt, AF_PACKET);

net_verdict = net_conn_packet_input(pkt, proto);
net_verdict = net_conn_packet_input(pkt, proto, type);

net_pkt_set_family(pkt, orig_family);

Expand Down
7 changes: 5 additions & 2 deletions subsys/net/ip/packet_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@
* disabled, the function will always return NET_DROP.
*/
#if defined(CONFIG_NET_SOCKETS_PACKET)
enum net_verdict net_packet_socket_input(struct net_pkt *pkt, uint16_t proto);
enum net_verdict net_packet_socket_input(struct net_pkt *pkt,
uint16_t proto,
enum net_sock_type type);
#else
static inline enum net_verdict net_packet_socket_input(struct net_pkt *pkt,
uint16_t proto)
uint16_t proto,
enum net_sock_type type)
{
return NET_CONTINUE;
}
Expand Down
4 changes: 2 additions & 2 deletions subsys/net/l2/virtual/ipip/ipip.c
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ static enum net_verdict interface_recv(struct net_if *iface,

net_pkt_cursor_restore(pkt, &hdr_start);

return net_ipv6_input(pkt, false);
return net_ipv6_input(pkt);
}

if (IS_ENABLED(CONFIG_NET_IPV4) && net_pkt_family(pkt) == AF_INET) {
Expand Down Expand Up @@ -421,7 +421,7 @@ static enum net_verdict interface_recv(struct net_if *iface,

net_pkt_cursor_restore(pkt, &hdr_start);

return net_ipv4_input(pkt, false);
return net_ipv4_input(pkt);
}

return NET_CONTINUE;
Expand Down
Loading