Fix the regression introduced in 9e30ecf23b1b whereby IPv4 broadcast packets were having their ethernet destination field mangled. This broke WOL magic packets and likely other IPv4 broadcast.
The regression can be observed by sending an IPv4 WOL packet using the wakeonlan program to any ethernet address:
wakeonlan 46:3b:ad:61:e0:5d
and capturing the packet with tcpdump:
tcpdump -i eth0 -w /tmp/bad.cap dst port 9
The ethernet destination MUST be ff:ff:ff:ff:ff:ff for broadcast, but is mangled with 9e30ecf23b1b applied.
Revert the change made in 9e30ecf23b1b and ensure the MTU value for broadcast routes is retained by calling ip_dst_init_metrics() directly, avoiding the need to enter the main code block in rt_set_nexthop().
Simplify the code path taken for broadcast packets back to the original before the regression, adding only the call to ip_dst_init_metrics().
The broadcast_pmtu.sh selftest provided with the original patch still passes with this patch applied.
Fixes: 9e30ecf23b1b ("net: ipv4: fix incorrect MTU in broadcast routes") Signed-off-by: Brett A C Sheffield bacs@librecast.net --- net/ipv4/route.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index f639a2ae881a..eaf78e128aca 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2588,6 +2588,7 @@ static struct rtable *__mkroute_output(const struct fib_result *res, do_cache = true; if (type == RTN_BROADCAST) { flags |= RTCF_BROADCAST | RTCF_LOCAL; + fi = NULL; } else if (type == RTN_MULTICAST) { flags |= RTCF_MULTICAST | RTCF_LOCAL; if (!ip_check_mc_rcu(in_dev, fl4->daddr, fl4->saddr, @@ -2657,8 +2658,12 @@ static struct rtable *__mkroute_output(const struct fib_result *res, rth->dst.output = ip_mc_output; RT_CACHE_STAT_INC(out_slow_mc); } + if (type == RTN_BROADCAST) { + /* ensure MTU value for broadcast routes is retained */ + ip_dst_init_metrics(&rth->dst, res->fi->fib_metrics); + } #ifdef CONFIG_IP_MROUTE - if (type == RTN_MULTICAST) { + else if (type == RTN_MULTICAST) { if (IN_DEV_MFORWARD(in_dev) && !ipv4_is_local_multicast(fl4->daddr)) { rth->dst.input = ip_mr_input;
base-commit: 01b9128c5db1b470575d07b05b67ffa3cb02ebf1
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes Link: https://lore.kernel.org/stable/20250822165231.4353-4-bacs%40librecast.net
Thanks for bisecting and fixing!
The broadcast_pmtu.sh selftest provided with the original patch still passes with this patch applied.
Hm, yes, AFACT we're losing PMTU discovery but perhaps original commit wasn't concerned with that. Hopefully Oscar can comment.
On Fri, 22 Aug 2025 16:50:51 +0000 Brett A C Sheffield wrote:
if (type == RTN_BROADCAST) {/* ensure MTU value for broadcast routes is retained */ip_dst_init_metrics(&rth->dst, res->fi->fib_metrics);
You need to check if res->fi is actually set before using it
Could you add a selftest / test case for the scenario we broke? selftests can be in C / bash / Python. If bash hopefully socat can be used to repro, cause it looks like wakeonlan is not very widely packaged.
Add test to check the broadcast ethernet destination field is set correctly.
This test uses the tcpdump and socat programs.
Send a UDP broadcast packet to UDP port 9 (DISCARD), capture this with tcpdump and ensure that all bits of the 6 octet ethernet destination are correctly set.
Cc: stable@vger.kernel.org Signed-off-by: Brett A C Sheffield bacs@librecast.net Link: https://lore.kernel.org/regressions/20250822165231.4353-4-bacs@librecast.net --- tools/testing/selftests/net/Makefile | 1 + .../selftests/net/broadcast_ether_dst.sh | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100755 tools/testing/selftests/net/broadcast_ether_dst.sh
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index b31a71f2b372..463642a78eea 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -116,6 +116,7 @@ TEST_GEN_FILES += skf_net_off TEST_GEN_FILES += tfo TEST_PROGS += tfo_passive.sh TEST_PROGS += broadcast_pmtu.sh +TEST_PROGS += broadcast_ether_dst.sh TEST_PROGS += ipv6_force_forwarding.sh
# YNL files, must be before "include ..lib.mk" diff --git a/tools/testing/selftests/net/broadcast_ether_dst.sh b/tools/testing/selftests/net/broadcast_ether_dst.sh new file mode 100755 index 000000000000..de6abe3513b6 --- /dev/null +++ b/tools/testing/selftests/net/broadcast_ether_dst.sh @@ -0,0 +1,38 @@ +#!/bin/bash -eu +# SPDX-License-Identifier: GPL-2.0 +# +# Author: Brett A C Sheffield bacs@librecast.net +# +# Ensure destination ethernet field is correctly set for +# broadcast packets + +if ! which tcpdump > /dev/null 2>&1; then + echo "No tcpdump found. Required for this test." + exit $ERR +fi + +CAPFILE=$(mktemp -u cap.XXXXXXXXXX) + +# start tcpdump listening on udp port 9 +# tcpdump will exit after receiving a single packet +# timeout will kill tcpdump if it is still running after 2s +timeout 2s tcpdump -c 1 -w ${CAPFILE} udp port 9 > /dev/null 2>&1 & +PID=$! +sleep 0.1 # let tcpdump wake up + +echo "Testing ethernet broadcast destination" + +# send broadcast UDP packet to port 9 (DISCARD) +echo "Alonso is a good boy" | socat - udp-datagram:255.255.255.255:9,broadcast + +# wait for tcpdump for exit after receiving packet +wait $PID + +# compare ethernet destination field to ff:ff:ff:ff:ff:ff +# pcap has a 24 octet header + 16 octet header for each packet +# ethernet destination is the first field in the packet +printf '\xff\xff\xff\xff\xff\xff'| cmp -i40:0 -n6 ${CAPFILE} > /dev/null 2>&1 +RESULT=$? + +rm -f "${CAPFILE}" +exit $RESULT
base-commit: 01b9128c5db1b470575d07b05b67ffa3cb02ebf1
Fix the regression introduced in 9e30ecf23b1b whereby IPv4 broadcast packets were having their ethernet destination field mangled. The problem was first observed with WOL magic packets but affects all UDP IPv4 broadcast packets.
The regression can be observed by sending an IPv4 WOL packet using the wakeonlan program to any ethernet address:
wakeonlan 46:3b:ad:61:e0:5d
and capturing the packet with tcpdump:
tcpdump -i eth0 -w /tmp/bad.cap dst port 9
The ethernet destination MUST be ff:ff:ff:ff:ff:ff for broadcast, but is mangled in affected kernels.
Revert the change made in 9e30ecf23b1b and ensure the MTU value for broadcast routes is retained by calling ip_dst_init_metrics() directly, avoiding the need to enter the main code block in rt_set_nexthop().
Simplify the code path taken for broadcast packets back to the original before the regression, adding only the call to ip_dst_init_metrics().
The broadcast_pmtu.sh selftest provided with the original patch still passes with this patch applied.
Cc: stable@vger.kernel.org Fixes: 9e30ecf23b1b ("net: ipv4: fix incorrect MTU in broadcast routes") Link: https://lore.kernel.org/regressions/20250822165231.4353-4-bacs@librecast.net Signed-off-by: Brett A C Sheffield bacs@librecast.net --- net/ipv4/route.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index f639a2ae881a..ab4d72a59c7b 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2588,6 +2588,7 @@ static struct rtable *__mkroute_output(const struct fib_result *res, do_cache = true; if (type == RTN_BROADCAST) { flags |= RTCF_BROADCAST | RTCF_LOCAL; + fi = NULL; } else if (type == RTN_MULTICAST) { flags |= RTCF_MULTICAST | RTCF_LOCAL; if (!ip_check_mc_rcu(in_dev, fl4->daddr, fl4->saddr, @@ -2657,8 +2658,12 @@ static struct rtable *__mkroute_output(const struct fib_result *res, rth->dst.output = ip_mc_output; RT_CACHE_STAT_INC(out_slow_mc); } + if (type == RTN_BROADCAST && res->fi) { + /* ensure MTU value for broadcast routes is retained */ + ip_dst_init_metrics(&rth->dst, res->fi->fib_metrics); + } #ifdef CONFIG_IP_MROUTE - if (type == RTN_MULTICAST) { + else if (type == RTN_MULTICAST) { if (IN_DEV_MFORWARD(in_dev) && !ipv4_is_local_multicast(fl4->daddr)) { rth->dst.input = ip_mr_input;
On 2025-08-22 18:32, Jakub Kicinski wrote:
Thanks for bisecting and fixing!
The broadcast_pmtu.sh selftest provided with the original patch still passes with this patch applied.
Hm, yes, AFACT we're losing PMTU discovery but perhaps original commit wasn't concerned with that. Hopefully Oscar can comment.
Indeed. This takes it back to the previous behaviour.
On Fri, 22 Aug 2025 16:50:51 +0000 Brett A C Sheffield wrote:
if (type == RTN_BROADCAST) {/* ensure MTU value for broadcast routes is retained */ip_dst_init_metrics(&rth->dst, res->fi->fib_metrics);You need to check if res->fi is actually set before using it
Ah, yes. Fixed.
Could you add a selftest / test case for the scenario we broke? selftests can be in C / bash / Python. If bash hopefully socat can be used to repro, cause it looks like wakeonlan is not very widely packaged.
Self-test added using socat as requested. If you want this wrapped in namespaces etc. let me know. I started doing that, but decided a simpler test without requiring root was better and cleaner.
Thanks for the review Jakub. v2 patches sent.
Cheers,
Brett
On Sat, Aug 23, 2025 at 02:24:39PM +0200, Brett Sheffield wrote:
On 2025-08-22 18:32, Jakub Kicinski wrote:
Thanks for bisecting and fixing!
The broadcast_pmtu.sh selftest provided with the original patch still passes with this patch applied.
Hm, yes, AFACT we're losing PMTU discovery but perhaps original commit wasn't concerned with that. Hopefully Oscar can comment.
Indeed. This takes it back to the previous behaviour.
On Fri, 22 Aug 2025 16:50:51 +0000 Brett A C Sheffield wrote:
if (type == RTN_BROADCAST) {/* ensure MTU value for broadcast routes is retained */ip_dst_init_metrics(&rth->dst, res->fi->fib_metrics);You need to check if res->fi is actually set before using it
Ah, yes. Fixed.
Could you add a selftest / test case for the scenario we broke? selftests can be in C / bash / Python. If bash hopefully socat can be used to repro, cause it looks like wakeonlan is not very widely packaged.
Self-test added using socat as requested. If you want this wrapped in namespaces etc. let me know. I started doing that, but decided a simpler test without requiring root was better and cleaner.
Thanks for the review Jakub. v2 patches sent.
Cheers,
Brett
Brett Sheffield (he/him) Librecast - Decentralising the Internet with Multicast https://librecast.net/ https://blog.brettsheffield.com/
Replying to you both.
I missed this regression when I tested the blamed commit. Glad this surfaced quickly.
Just to clarify, the issue isn't that the destination address is "mangled". The removal of fi = NULL inadvertently causes a gateway to be assigned to local broadcast packets (see the call to rt_set_nexthop), which results in the observed regression. __mkroute_output is still correct for directed broadcast packets due to them having their own routes in the local table. The regression can easily be fixed by setting the fib info to NULL for lbcast packets.
The submitted selftest only fails if a default gateway is configured, but does not set one up. I'll submit my own patches to fix the regression and improve the selftest shortly.
Oscar
Commit 9e30ecf23b1b ("net: ipv4: fix incorrect MTU in broadcast routes") introduced a regression where local-broadcast packets would have their gateway set in __mkroute_output, which was caused by fi = NULL being removed.
Fix this by resetting the fib_info for local-broadcast packets.
Fixes: 9e30ecf23b1b ("net: ipv4: fix incorrect MTU in broadcast routes") Reported-by: Brett A C Sheffield bacs@librecast.net Closes: https://lore.kernel.org/regressions/20250822165231.4353-4-bacs@librecast.net Signed-off-by: Oscar Maes oscmaes92@gmail.com --- net/ipv4/route.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index f639a2ae881a..98d237e3ec04 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2575,9 +2575,12 @@ static struct rtable *__mkroute_output(const struct fib_result *res, !netif_is_l3_master(dev_out)) return ERR_PTR(-EINVAL);
- if (ipv4_is_lbcast(fl4->daddr)) + if (ipv4_is_lbcast(fl4->daddr)) { type = RTN_BROADCAST; - else if (ipv4_is_multicast(fl4->daddr)) + + /* reset fi to prevent gateway resolution */ + fi = NULL; + } else if (ipv4_is_multicast(fl4->daddr)) type = RTN_MULTICAST; else if (ipv4_is_zeronet(fl4->daddr)) return ERR_PTR(-EINVAL);
Add test to check the broadcast ethernet destination field is set correctly.
This test sends a broadcast ping, captures it using tcpdump and ensures that all bits of the 6 octet ethernet destination address are correctly set by examining the output capture file.
Signed-off-by: Oscar Maes oscmaes92@gmail.com --- tools/testing/selftests/net/Makefile | 1 + .../selftests/net/broadcast_ether_dst.sh | 82 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100755 tools/testing/selftests/net/broadcast_ether_dst.sh
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index b31a71f2b372..463642a78eea 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -116,6 +116,7 @@ TEST_GEN_FILES += skf_net_off TEST_GEN_FILES += tfo TEST_PROGS += tfo_passive.sh TEST_PROGS += broadcast_pmtu.sh +TEST_PROGS += broadcast_ether_dst.sh TEST_PROGS += ipv6_force_forwarding.sh
# YNL files, must be before "include ..lib.mk" diff --git a/tools/testing/selftests/net/broadcast_ether_dst.sh b/tools/testing/selftests/net/broadcast_ether_dst.sh new file mode 100755 index 000000000000..865b5c7c8c8a --- /dev/null +++ b/tools/testing/selftests/net/broadcast_ether_dst.sh @@ -0,0 +1,82 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Author: Brett A C Sheffield bacs@librecast.net +# Author: Oscar Maes oscmaes92@gmail.com +# +# Ensure destination ethernet field is correctly set for +# broadcast packets + +source lib.sh + +CLIENT_IP4="192.168.0.1" +GW_IP4="192.168.0.2" + +setup() { + setup_ns CLIENT_NS SERVER_NS + + ip -net "${SERVER_NS}" link add link1 type veth \ + peer name link0 netns "${CLIENT_NS}" + + ip -net "${CLIENT_NS}" link set link0 up + ip -net "${CLIENT_NS}" addr add "${CLIENT_IP4}"/24 dev link0 + + ip -net "${SERVER_NS}" link set link1 up + + ip -net "${CLIENT_NS}" route add default via "${GW_IP4}" + ip netns exec "${CLIENT_NS}" arp -s "${GW_IP4}" 00:11:22:33:44:55 +} + +cleanup() { + rm -f "${CAPFILE}" + ip -net "${SERVER_NS}" link del link1 + cleanup_ns "${CLIENT_NS}" "${SERVER_NS}" +} + +test_broadcast_ether_dst() { + local rc=0 + CAPFILE=$(mktemp -u cap.XXXXXXXXXX) + + echo "Testing ethernet broadcast destination" + + # start tcpdump listening for icmp + # tcpdump will exit after receiving a single packet + # timeout will kill tcpdump if it is still running after 2s + timeout 2s ip netns exec "${CLIENT_NS}" \ + tcpdump -i link0 -c 1 -w "${CAPFILE}" icmp &> /dev/null & + pid=$! + sleep 0.1 # let tcpdump wake up + + # send broadcast ping + ip netns exec "${CLIENT_NS}" \ + ping -W0.01 -c1 -b 255.255.255.255 &> /dev/null + + # wait for tcpdump for exit after receiving packet + wait "${pid}" + + # compare ethernet destination field to ff:ff:ff:ff:ff:ff + ether_dst=$(tcpdump -r "${CAPFILE}" -tnne 2>/dev/null | \ + awk '{sub(/,/,"",$3); print $3}') + if [[ "${ether_dst}" == "ff:ff:ff:ff:ff:ff" ]]; then + echo "[ OK ]" + rc="${ksft_pass}" + else + echo "[FAIL] expected dst ether addr to be ff:ff:ff:ff:ff:ff," \ + "got ${ether_dst}" + rc="${ksft_fail}" + fi + + return "${rc}" +} + +if [ ! -x "$(command -v tcpdump)" ]; then + echo "SKIP: Could not run test without tcpdump tool" + exit "${ksft_skip}" +fi + +trap cleanup EXIT + +setup +test_broadcast_ether_dst + +exit $?
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH net 2/2] selftests: net: add test for destination in broadcast packets Link: https://lore.kernel.org/stable/20250825060918.4799-2-oscmaes92%40gmail.com
On 2025-08-25 08:09, Oscar Maes wrote:
Add test to check the broadcast ethernet destination field is set correctly.
This test sends a broadcast ping, captures it using tcpdump and ensures that all bits of the 6 octet ethernet destination address are correctly set by examining the output capture file.
Signed-off-by: Oscar Maes oscmaes92@gmail.com
Thanks Oscar.
Signed-off-by: Brett A C Sheffield bacs@librecast.net
Also, please add back the Cc: stable@vger.kernel.org line for the fix as this affects the stable kernels that went out last week.
Cheers,
Brett
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH net 1/2] net: ipv4: fix regression in local-broadcast routes Link: https://lore.kernel.org/stable/20250825060918.4799-1-oscmaes92%40gmail.com
On Mon, 25 Aug 2025 08:09:17 +0200 Oscar Maes wrote:
Commit 9e30ecf23b1b ("net: ipv4: fix incorrect MTU in broadcast routes") introduced a regression where local-broadcast packets would have their gateway set in __mkroute_output, which was caused by fi = NULL being removed.
Fix this by resetting the fib_info for local-broadcast packets.
Meaning that 9e30ecf23b1b would still change behavior for the subnet broadcast address?
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index f639a2ae881a..98d237e3ec04 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2575,9 +2575,12 @@ static struct rtable *__mkroute_output(const struct fib_result *res, !netif_is_l3_master(dev_out)) return ERR_PTR(-EINVAL);
- if (ipv4_is_lbcast(fl4->daddr))
- if (ipv4_is_lbcast(fl4->daddr)) { type = RTN_BROADCAST;
- else if (ipv4_is_multicast(fl4->daddr))
/* reset fi to prevent gateway resolution */fi = NULL;- } else if (ipv4_is_multicast(fl4->daddr)) type = RTN_MULTICAST; else if (ipv4_is_zeronet(fl4->daddr)) return ERR_PTR(-EINVAL);
nit: please add curly braces around all branches of this if / else if / else ladder, per kernel coding style guide.
On Mon, Aug 25, 2025 at 03:56:30PM -0700, Jakub Kicinski wrote:
On Mon, 25 Aug 2025 08:09:17 +0200 Oscar Maes wrote:
Commit 9e30ecf23b1b ("net: ipv4: fix incorrect MTU in broadcast routes") introduced a regression where local-broadcast packets would have their gateway set in __mkroute_output, which was caused by fi = NULL being removed.
Fix this by resetting the fib_info for local-broadcast packets.
Meaning that 9e30ecf23b1b would still change behavior for the subnet broadcast address?
Yes. This is OK because subnet broadcast addresses have separate entries in the local fib table.
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index f639a2ae881a..98d237e3ec04 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2575,9 +2575,12 @@ static struct rtable *__mkroute_output(const struct fib_result *res, !netif_is_l3_master(dev_out)) return ERR_PTR(-EINVAL);
- if (ipv4_is_lbcast(fl4->daddr))
- if (ipv4_is_lbcast(fl4->daddr)) { type = RTN_BROADCAST;
- else if (ipv4_is_multicast(fl4->daddr))
/* reset fi to prevent gateway resolution */fi = NULL;- } else if (ipv4_is_multicast(fl4->daddr)) type = RTN_MULTICAST; else if (ipv4_is_zeronet(fl4->daddr)) return ERR_PTR(-EINVAL);
nit: please add curly braces around all branches of this if / else if / else ladder, per kernel coding style guide. -- pw-bot: cr
Commit 9e30ecf23b1b ("net: ipv4: fix incorrect MTU in broadcast routes") introduced a regression where local-broadcast packets would have their gateway set in __mkroute_output, which was caused by fi = NULL being removed.
Fix this by resetting the fib_info for local-broadcast packets. This preserves the intended changes for directed-broadcast packets.
Cc: stable@vger.kernel.org Fixes: 9e30ecf23b1b ("net: ipv4: fix incorrect MTU in broadcast routes") Reported-by: Brett A C Sheffield bacs@librecast.net Closes: https://lore.kernel.org/regressions/20250822165231.4353-4-bacs@librecast.net Signed-off-by: Oscar Maes oscmaes92@gmail.com ---
Thanks to Brett Sheffield for finding the regression and writing the initial fix! --- net/ipv4/route.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index 1f212b2ce4c6..24c898b7654f 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2575,12 +2575,16 @@ static struct rtable *__mkroute_output(const struct fib_result *res, !netif_is_l3_master(dev_out)) return ERR_PTR(-EINVAL);
- if (ipv4_is_lbcast(fl4->daddr)) + if (ipv4_is_lbcast(fl4->daddr)) { type = RTN_BROADCAST; - else if (ipv4_is_multicast(fl4->daddr)) + + /* reset fi to prevent gateway resolution */ + fi = NULL; + } else if (ipv4_is_multicast(fl4->daddr)) { type = RTN_MULTICAST; - else if (ipv4_is_zeronet(fl4->daddr)) + } else if (ipv4_is_zeronet(fl4->daddr)) { return ERR_PTR(-EINVAL); + }
if (dev_out->flags & IFF_LOOPBACK) flags |= RTCF_LOCAL;
Add test to check the broadcast ethernet destination field is set correctly.
This test sends a broadcast ping, captures it using tcpdump and ensures that all bits of the 6 octet ethernet destination address are correctly set by examining the output capture file.
Signed-off-by: Oscar Maes oscmaes92@gmail.com ---
Thanks to Brett Sheffield for the initial selftest! --- tools/testing/selftests/net/Makefile | 1 + .../selftests/net/broadcast_ether_dst.sh | 82 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100755 tools/testing/selftests/net/broadcast_ether_dst.sh
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index eef0b8f8a7b0..9bbe1d010f5a 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -116,6 +116,7 @@ TEST_GEN_FILES += skf_net_off TEST_GEN_FILES += tfo TEST_PROGS += tfo_passive.sh TEST_PROGS += broadcast_pmtu.sh +TEST_PROGS += broadcast_ether_dst.sh TEST_PROGS += ipv6_force_forwarding.sh TEST_PROGS += route_hint.sh
diff --git a/tools/testing/selftests/net/broadcast_ether_dst.sh b/tools/testing/selftests/net/broadcast_ether_dst.sh new file mode 100755 index 000000000000..865b5c7c8c8a --- /dev/null +++ b/tools/testing/selftests/net/broadcast_ether_dst.sh @@ -0,0 +1,82 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Author: Brett A C Sheffield bacs@librecast.net +# Author: Oscar Maes oscmaes92@gmail.com +# +# Ensure destination ethernet field is correctly set for +# broadcast packets + +source lib.sh + +CLIENT_IP4="192.168.0.1" +GW_IP4="192.168.0.2" + +setup() { + setup_ns CLIENT_NS SERVER_NS + + ip -net "${SERVER_NS}" link add link1 type veth \ + peer name link0 netns "${CLIENT_NS}" + + ip -net "${CLIENT_NS}" link set link0 up + ip -net "${CLIENT_NS}" addr add "${CLIENT_IP4}"/24 dev link0 + + ip -net "${SERVER_NS}" link set link1 up + + ip -net "${CLIENT_NS}" route add default via "${GW_IP4}" + ip netns exec "${CLIENT_NS}" arp -s "${GW_IP4}" 00:11:22:33:44:55 +} + +cleanup() { + rm -f "${CAPFILE}" + ip -net "${SERVER_NS}" link del link1 + cleanup_ns "${CLIENT_NS}" "${SERVER_NS}" +} + +test_broadcast_ether_dst() { + local rc=0 + CAPFILE=$(mktemp -u cap.XXXXXXXXXX) + + echo "Testing ethernet broadcast destination" + + # start tcpdump listening for icmp + # tcpdump will exit after receiving a single packet + # timeout will kill tcpdump if it is still running after 2s + timeout 2s ip netns exec "${CLIENT_NS}" \ + tcpdump -i link0 -c 1 -w "${CAPFILE}" icmp &> /dev/null & + pid=$! + sleep 0.1 # let tcpdump wake up + + # send broadcast ping + ip netns exec "${CLIENT_NS}" \ + ping -W0.01 -c1 -b 255.255.255.255 &> /dev/null + + # wait for tcpdump for exit after receiving packet + wait "${pid}" + + # compare ethernet destination field to ff:ff:ff:ff:ff:ff + ether_dst=$(tcpdump -r "${CAPFILE}" -tnne 2>/dev/null | \ + awk '{sub(/,/,"",$3); print $3}') + if [[ "${ether_dst}" == "ff:ff:ff:ff:ff:ff" ]]; then + echo "[ OK ]" + rc="${ksft_pass}" + else + echo "[FAIL] expected dst ether addr to be ff:ff:ff:ff:ff:ff," \ + "got ${ether_dst}" + rc="${ksft_fail}" + fi + + return "${rc}" +} + +if [ ! -x "$(command -v tcpdump)" ]; then + echo "SKIP: Could not run test without tcpdump tool" + exit "${ksft_skip}" +fi + +trap cleanup EXIT + +setup +test_broadcast_ether_dst + +exit $?
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH net v2 2/2] selftests: net: add test for destination in broadcast packets Link: https://lore.kernel.org/stable/20250826121750.8451-2-oscmaes92%40gmail.com
On Tue, 26 Aug 2025 14:17:50 +0200 Oscar Maes wrote:
TEST_GEN_FILES += tfo TEST_PROGS += tfo_passive.sh TEST_PROGS += broadcast_pmtu.sh +TEST_PROGS += broadcast_ether_dst.sh TEST_PROGS += ipv6_force_forwarding.sh TEST_PROGS += route_hint.sh
This applies to net with a fuzz. Could you respin against net/main? Please repost as a new thread, you can add a link to the discussion under ---, use the lore archive.
Also nit: maybe lets try to sort somewhat alphabetically, e before p?
Hello,
kernel test robot noticed "Oops:general_protection_fault,probably_for_non-canonical_address#:#[##]SMP_KASAN_PTI" on:
commit: a1b445e1dcd6ee9682d77347faf3545b53354d71 ("[REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes") url: https://github.com/intel-lab-lkp/linux/commits/Brett-A-C-Sheffield/net-ipv4-... patch link: https://lore.kernel.org/all/20250822165231.4353-4-bacs@librecast.net/ patch subject: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes
in testcase: trinity version: trinity-x86_64-ba2360ed-1_20241228 with following parameters:
runtime: 300s group: group-04 nr_groups: 5
config: x86_64-randconfig-104-20250826 compiler: clang-20 test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot oliver.sang@intel.com | Closes: https://lore.kernel.org/oe-lkp/202508281637.f1c00f73-lkp@intel.com
[ 344.224405][ T239] [ 346.380232][ T239] [main] 270958 iterations. [F:200215 S:70364 HI:20538] [ 346.380362][ T239] [ 348.540466][ T239] [main] 282649 iterations. [F:208752 S:73502 HI:20538] [ 348.540488][ T239] [ 352.276620][ T4267] Oops: general protection fault, probably for non-canonical address 0xdffffc000000000b: 0000 [#1] SMP KASAN PTI [ 352.278585][ T4267] KASAN: null-ptr-deref in range [0x0000000000000058-0x000000000000005f] [ 352.279982][ T4267] CPU: 0 UID: 65534 PID: 4267 Comm: trinity-c0 Not tainted 6.17.0-rc2-00174-ga1b445e1dcd6 #1 PREEMPT(none) [ 352.281748][ T4267] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 [ 352.283361][ T4267] RIP: 0010:ip_route_output_key_hash_rcu (kbuild/src/consumer/net/ipv4/route.c:2663) [ 352.284480][ T4267] Code: 3c 10 00 48 8b 5c 24 60 74 12 48 89 df e8 d7 d5 f3 fc 48 ba 00 00 00 00 00 fc ff df 48 8b 1b 48 83 c3 58 48 89 d8 48 c1 e8 03 <80> 3c 10 00 74 12 48 89 df e8 b1 d5 f3 fc 48 ba 00 00 00 00 00 fc All code ======== 0: 3c 10 cmp $0x10,%al 2: 00 48 8b add %cl,-0x75(%rax) 5: 5c pop %rsp 6: 24 60 and $0x60,%al 8: 74 12 je 0x1c a: 48 89 df mov %rbx,%rdi d: e8 d7 d5 f3 fc call 0xfffffffffcf3d5e9 12: 48 ba 00 00 00 00 00 movabs $0xdffffc0000000000,%rdx 19: fc ff df 1c: 48 8b 1b mov (%rbx),%rbx 1f: 48 83 c3 58 add $0x58,%rbx 23: 48 89 d8 mov %rbx,%rax 26: 48 c1 e8 03 shr $0x3,%rax 2a:* 80 3c 10 00 cmpb $0x0,(%rax,%rdx,1) <-- trapping instruction 2e: 74 12 je 0x42 30: 48 89 df mov %rbx,%rdi 33: e8 b1 d5 f3 fc call 0xfffffffffcf3d5e9 38: 48 rex.W 39: ba 00 00 00 00 mov $0x0,%edx 3e: 00 fc add %bh,%ah
Code starting with the faulting instruction =========================================== 0: 80 3c 10 00 cmpb $0x0,(%rax,%rdx,1) 4: 74 12 je 0x18 6: 48 89 df mov %rbx,%rdi 9: e8 b1 d5 f3 fc call 0xfffffffffcf3d5bf e: 48 rex.W f: ba 00 00 00 00 mov $0x0,%edx 14: 00 fc add %bh,%ah [ 352.287420][ T4267] RSP: 0018:ffffc900037cf7e0 EFLAGS: 00010202 [ 352.288406][ T4267] RAX: 000000000000000b RBX: 0000000000000058 RCX: 0000000000000000 [ 352.289715][ T4267] RDX: dffffc0000000000 RSI: 0000000090000000 RDI: ffff888155e8a0a8 [ 352.291007][ T4267] RBP: ffff88815a690640 R08: ffff88815a6906d8 R09: 0000000000000002 [ 352.292287][ T4267] R10: ffff88815a6906d2 R11: ffffed102b4d20dc R12: ffff888118e51701 [ 352.293502][ T4267] R13: 1ffff1102b4d20ce R14: ffff88815a6906d4 R15: 0000000090000000 [ 352.294417][ T4267] FS: 00007fa824b10740(0000) GS:ffff8884259dd000(0000) knlGS:0000000000000000 [ 352.295629][ T4267] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 352.296648][ T4267] CR2: 0000000000000008 CR3: 00000001284fc000 CR4: 00000000000406f0 [ 352.297771][ T4267] Call Trace: [ 352.298404][ T4267] <TASK> [ 352.298958][ T4267] ? ip_route_output_key_hash (kbuild/src/consumer/include/linux/rcupdate.h:331 kbuild/src/consumer/include/linux/rcupdate.h:841 kbuild/src/consumer/net/ipv4/route.c:2700) [ 352.299842][ T4267] ip_route_output_key_hash (kbuild/src/consumer/net/ipv4/route.c:2701) [ 352.300711][ T4267] ip_route_output_flow (kbuild/src/consumer/include/linux/err.h:70 kbuild/src/consumer/net/ipv4/route.c:2930) [ 352.301444][ T4267] __ip4_datagram_connect (kbuild/src/consumer/include/net/route.h:355)
The kernel config and materials to reproduce are available at: https://download.01.org/0day-ci/archive/20250828/202508281637.f1c00f73-lkp@i...
On 8/28/25 10:17 AM, kernel test robot wrote:
commit: a1b445e1dcd6ee9682d77347faf3545b53354d71 ("[REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes") url: https://github.com/intel-lab-lkp/linux/commits/Brett-A-C-Sheffield/net-ipv4-... patch link: https://lore.kernel.org/all/20250822165231.4353-4-bacs@librecast.net/ patch subject: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes
in testcase: trinity version: trinity-x86_64-ba2360ed-1_20241228 with following parameters:
runtime: 300s group: group-04 nr_groups: 5
config: x86_64-randconfig-104-20250826 compiler: clang-20 test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
Since I just merged v3 of the mentioned patch and I'm wrapping the PR for Linus, the above scared me more than a bit.
AFAICS the issue reported here is the unconditional 'fi' dereference spotted and fixed during code review, so no real problem after all.
/P
On 2025-08-28 12:35, Paolo Abeni wrote:
On 8/28/25 10:17 AM, kernel test robot wrote:
commit: a1b445e1dcd6ee9682d77347faf3545b53354d71 ("[REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes") url: https://github.com/intel-lab-lkp/linux/commits/Brett-A-C-Sheffield/net-ipv4-... patch link: https://lore.kernel.org/all/20250822165231.4353-4-bacs@librecast.net/ patch subject: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes
in testcase: trinity version: trinity-x86_64-ba2360ed-1_20241228 with following parameters:
runtime: 300s group: group-04 nr_groups: 5
config: x86_64-randconfig-104-20250826 compiler: clang-20 test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
Since I just merged v3 of the mentioned patch and I'm wrapping the PR for Linus, the above scared me more than a bit.
AFAICS the issue reported here is the unconditional 'fi' dereference spotted and fixed during code review, so no real problem after all.
Correct. Jakub spotted the error, it was fixed in a v2 5 days ago, and has since been superceded by Oscar's patch, so nothing to worry about.
Is there a way to indicate to bots not to check superceded patches. In this case I'd have though my v2 would have been a signal? Is there something else I should have done?
Brett
hi, Brett, hi, Paolo Abeni,
On Thu, Aug 28, 2025 at 10:46:42AM +0000, Brett A C Sheffield wrote:
On 2025-08-28 12:35, Paolo Abeni wrote:
On 8/28/25 10:17 AM, kernel test robot wrote:
commit: a1b445e1dcd6ee9682d77347faf3545b53354d71 ("[REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes") url: https://github.com/intel-lab-lkp/linux/commits/Brett-A-C-Sheffield/net-ipv4-... patch link: https://lore.kernel.org/all/20250822165231.4353-4-bacs@librecast.net/ patch subject: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes
in testcase: trinity version: trinity-x86_64-ba2360ed-1_20241228 with following parameters:
runtime: 300s group: group-04 nr_groups: 5
config: x86_64-randconfig-104-20250826 compiler: clang-20 test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
Since I just merged v3 of the mentioned patch and I'm wrapping the PR for Linus, the above scared me more than a bit.
AFAICS the issue reported here is the unconditional 'fi' dereference spotted and fixed during code review, so no real problem after all.
Correct. Jakub spotted the error, it was fixed in a v2 5 days ago, and has since been superceded by Oscar's patch, so nothing to worry about.
Is there a way to indicate to bots not to check superceded patches. In this case I'd have though my v2 would have been a signal? Is there something else I should have done?
sorry for this. our bot failed to recognize the mail structure to spot out the v2 patch.
we'll consider how to improve it or be more careful while manual check. sorry for inconvience caused.
Brett
linux-stable-mirror@lists.linaro.org
-
Brett A C Sheffield -
Brett Sheffield -
Jakub Kicinski -
kernel test robot -
kernel test robot -
Oliver Sang
-
Oscar Maes -
Paolo Abeni