From a072cf1a2d7e6514f03a005c640f9cfdf828b50f Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Fri, 17 Jan 2025 16:38:07 +0200 Subject: [PATCH 1/7] net: ethernet: Only try ARP for IP packets The work in Ethernet send in commit 2f10d7d81624 ("net: ethernet: Set the ptype by the caller in send") introduced a bug because we could try to do ARP resolving for ARP packets too. This is clearly a wrong thing to do. So before trying to do ARP resolving, make sure the the packet type is IP packet. Signed-off-by: Jukka Rissanen --- subsys/net/l2/ethernet/ethernet.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/subsys/net/l2/ethernet/ethernet.c b/subsys/net/l2/ethernet/ethernet.c index 71eb9fcda248..ab7add695b13 100644 --- a/subsys/net/l2/ethernet/ethernet.c +++ b/subsys/net/l2/ethernet/ethernet.c @@ -711,7 +711,8 @@ static int ethernet_send(struct net_if *iface, struct net_pkt *pkt) goto send; } - if (IS_ENABLED(CONFIG_NET_IPV4) && net_pkt_family(pkt) == AF_INET) { + if (IS_ENABLED(CONFIG_NET_IPV4) && net_pkt_family(pkt) == AF_INET && + net_pkt_ll_proto_type(pkt) == NET_ETH_PTYPE_IP) { if (!net_pkt_ipv4_acd(pkt)) { struct net_pkt *tmp; From 0e1f0854c95b03a28587c0463b5d59545e6d33f9 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Fri, 17 Jan 2025 16:43:39 +0200 Subject: [PATCH 2/7] net: arp: Enhance debug messages Print more data / debug information for ARP messages. Also remove unnecessary "&" when printing IPv4 address. Signed-off-by: Jukka Rissanen --- subsys/net/l2/ethernet/arp.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/subsys/net/l2/ethernet/arp.c b/subsys/net/l2/ethernet/arp.c index df9a9286b1a4..eda9086f42bb 100644 --- a/subsys/net/l2/ethernet/arp.c +++ b/subsys/net/l2/ethernet/arp.c @@ -78,6 +78,9 @@ static struct arp_entry *arp_entry_find(sys_slist_t *list, if (entry->iface == iface && net_ipv4_addr_cmp(&entry->ip, dst)) { + NET_DBG("found dst %s", + net_sprint_ipv4_addr(dst)); + return entry; } @@ -463,7 +466,7 @@ struct net_pkt *net_arp_prepare(struct net_pkt *pkt, NET_DBG("ARP using ll %s for IP %s", net_sprint_ll_addr(net_pkt_lladdr_dst(pkt)->addr, sizeof(struct net_eth_addr)), - net_sprint_ipv4_addr(&NET_IPV4_HDR(pkt)->dst)); + net_sprint_ipv4_addr(NET_IPV4_HDR(pkt)->dst)); return pkt; } @@ -691,10 +694,10 @@ void net_arp_update(struct net_if *iface, net_pkt_lladdr_dst(pkt)->addr = (uint8_t *) &NET_ETH_HDR(pkt)->dst.addr; - NET_DBG("iface %d (%p) dst %s pending %p frag %p", + NET_DBG("iface %d (%p) dst %s pending %p frag %p ptype 0x%04x", net_if_get_by_iface(iface), iface, net_sprint_ipv4_addr(&entry->ip), - pkt, pkt->frags); + pkt, pkt->frags, net_pkt_ll_proto_type(pkt)); /* We directly send the packet without first queueing it. * The pkt has already been queued for sending, once by From 7d807d5149872b96ac689fdcf97529261919283b Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Fri, 17 Jan 2025 16:48:38 +0200 Subject: [PATCH 3/7] tests: net: arp: Remove the ARP cache clear from the tests It was incorrectly introduced earlier and not really needed because the test will pass without it. Signed-off-by: Jukka Rissanen --- tests/net/arp/src/main.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/net/arp/src/main.c b/tests/net/arp/src/main.c index 71524261e5f7..eaeb3abd72c3 100644 --- a/tests/net/arp/src/main.c +++ b/tests/net/arp/src/main.c @@ -565,9 +565,6 @@ ZTEST(arp_fn_tests, test_arp) net_pkt_unref(pkt); - /* Clear the ARP cache so that old entries do not confuse the tests */ - net_arp_clear_cache(iface); - /* Then feed in ARP request */ pkt = net_pkt_alloc_with_buffer(iface, sizeof(struct net_eth_hdr) + sizeof(struct net_arp_hdr), From 86048ad73d1777667994c2b37e587c13f7ca4bdd Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Mon, 20 Jan 2025 17:30:13 +0200 Subject: [PATCH 4/7] net: ipv4: Do not change the protocol type when sending The ARP code has set the protocol type of the packet to 0x806, so do not change it when preparing to send to 0x800 which is the IP protocol type. Lets trust the previously called functions to set the ptype correctly and do not set it here. Signed-off-by: Jukka Rissanen --- subsys/net/ip/ipv4.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/subsys/net/ip/ipv4.c b/subsys/net/ip/ipv4.c index 4f06a74207bc..b96cb10b8f08 100644 --- a/subsys/net/ip/ipv4.c +++ b/subsys/net/ip/ipv4.c @@ -454,8 +454,6 @@ enum net_verdict net_ipv4_input(struct net_pkt *pkt, bool is_loopback) enum net_verdict net_ipv4_prepare_for_send(struct net_pkt *pkt) { - net_pkt_set_ll_proto_type(pkt, NET_ETH_PTYPE_IP); - if (IS_ENABLED(CONFIG_NET_IPV4_PMTU)) { struct net_pmtu_entry *entry; struct sockaddr_in dst = { From 636ec623e140530f2babce28acb2944ac2e61eb5 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Mon, 20 Jan 2025 18:44:23 +0200 Subject: [PATCH 5/7] net: ipv6: Do not set ptype when preparing for sending Trust that the protocol type is set correctly by functions called before this one. We should not set the protocol type blindly in this generic function. Signed-off-by: Jukka Rissanen --- subsys/net/ip/ipv6_nbr.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/subsys/net/ip/ipv6_nbr.c b/subsys/net/ip/ipv6_nbr.c index b141f4bdfb0c..36e8b219691d 100644 --- a/subsys/net/ip/ipv6_nbr.c +++ b/subsys/net/ip/ipv6_nbr.c @@ -806,8 +806,6 @@ enum net_verdict net_ipv6_prepare_for_send(struct net_pkt *pkt) return NET_DROP; } - net_pkt_set_ll_proto_type(pkt, NET_ETH_PTYPE_IPV6); - #if defined(CONFIG_NET_IPV6_FRAGMENT) /* If we have already fragmented the packet, the fragment id will * contain a proper value and we can skip other checks. From 521879b985c6269fe837eeac175b7f1677dbe953 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Tue, 21 Jan 2025 10:50:40 +0200 Subject: [PATCH 6/7] net: Set the protocol type of fragmented packet The fragmented packet should inherit the protocol type of the original packet. Signed-off-by: Jukka Rissanen --- subsys/net/ip/ipv4_fragment.c | 2 ++ subsys/net/ip/ipv6_fragment.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/subsys/net/ip/ipv4_fragment.c b/subsys/net/ip/ipv4_fragment.c index 69fe95221598..720430753a9b 100644 --- a/subsys/net/ip/ipv4_fragment.c +++ b/subsys/net/ip/ipv4_fragment.c @@ -441,6 +441,8 @@ static int send_ipv4_fragment(struct net_pkt *pkt, uint16_t rand_id, uint16_t fi net_pkt_cursor_backup(pkt, &cur_pkt); net_pkt_cursor_backup(frag_pkt, &cur); + net_pkt_set_ll_proto_type(frag_pkt, net_pkt_ll_proto_type(pkt)); + /* Copy the original IPv4 headers back to the fragment packet */ if (net_pkt_copy(frag_pkt, pkt, net_pkt_ip_hdr_len(pkt))) { goto fail; diff --git a/subsys/net/ip/ipv6_fragment.c b/subsys/net/ip/ipv6_fragment.c index f50e5f9e4e21..a32aae979b14 100644 --- a/subsys/net/ip/ipv6_fragment.c +++ b/subsys/net/ip/ipv6_fragment.c @@ -609,6 +609,8 @@ static int send_ipv6_fragment(struct net_pkt *pkt, net_pkt_cursor_init(pkt); + net_pkt_set_ll_proto_type(frag_pkt, net_pkt_ll_proto_type(pkt)); + /* We copy original headers back to the fragment packet * Note that we insert the right next header to point to fragment header */ From 9980af294adc6e5e871b7a6373a520a6f70a9e63 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Tue, 21 Jan 2025 10:55:46 +0200 Subject: [PATCH 7/7] net: Build assert issue with llvm Remove the build assert from NET_L3_REGISTER() macro as that is causing an issue with llvm. Add runtime check of the handler pointer value. subsys/net/l2/ethernet/arp.c:1044:1: error: static_assert expression is not an integral constant expression ETH_NET_L3_REGISTER(ARP, NET_ETH_PTYPE_ARP, arp_recv); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/zephyr/net/ethernet.h:1272:2: note: expanded from macro 'ETH_NET_L3_REGISTER' NET_L3_REGISTER(&NET_L2_GET_NAME(ETHERNET), name, ptype, handler) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/zephyr/net/net_core.h:190:2: note: expanded from macro 'NET_L3_REGISTER' BUILD_ASSERT((_handler) != NULL, "Handler is not defined") ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/zephyr/toolchain/gcc.h:87:51: note: expanded from macro 'BUILD_ASSERT' define BUILD_ASSERT(EXPR, MSG...) _Static_assert((EXPR), "" MSG) ^~~~~~ subsys/net/l2/ethernet/arp.c:1044:1: note: cast from 'void *' is not allowed in a constant expression include/zephyr/net/ethernet.h:1272:2: note: expanded from macro 'ETH_NET_L3_REGISTER' NET_L3_REGISTER(&NET_L2_GET_NAME(ETHERNET), name, ptype, handler) ^ include/zephyr/net/net_core.h:190:29: note: expanded from macro 'NET_L3_REGISTER' BUILD_ASSERT((_handler) != NULL, "Handler is not defined") ^ /usr/lib/llvm-14/lib/clang/14.0.0/include/stddef.h:89:16: note: expanded from macro 'NULL' define NULL ((void*)0) Signed-off-by: Jukka Rissanen --- include/zephyr/net/net_core.h | 3 +-- subsys/net/l2/ethernet/ethernet.c | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/zephyr/net/net_core.h b/include/zephyr/net/net_core.h index 1b4bf0479bda..5c017a9780e2 100644 --- a/include/zephyr/net/net_core.h +++ b/include/zephyr/net/net_core.h @@ -186,8 +186,7 @@ struct net_l3_register { .handler = _handler, \ .name = STRINGIFY(_name), \ .l2 = _l2_type, \ - }; \ - BUILD_ASSERT((_handler) != NULL, "Handler is not defined") + }; /* @endcond */ diff --git a/subsys/net/l2/ethernet/ethernet.c b/subsys/net/l2/ethernet/ethernet.c index ab7add695b13..c93705bc11a9 100644 --- a/subsys/net/l2/ethernet/ethernet.c +++ b/subsys/net/l2/ethernet/ethernet.c @@ -353,7 +353,8 @@ static enum net_verdict ethernet_recv(struct net_if *iface, net_buf_pull(pkt->frags, hdr_len); STRUCT_SECTION_FOREACH(net_l3_register, l3) { - if (l3->ptype != type || l3->l2 != &NET_L2_GET_NAME(ETHERNET)) { + if (l3->ptype != type || l3->l2 != &NET_L2_GET_NAME(ETHERNET) || + l3->handler == NULL) { continue; }