[REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[Brett A C Sheffield]

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
-- 
2.49.1

Re: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[Jakub Kicinski]

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.

[PATCH v2 1/2] selftests: net: add test for broadcast destination

[Brett A C Sheffield]

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
-- 
2.49.1

[PATCH v2 2/2] net: ipv4: fix regression in broadcast routes

[Brett A C Sheffield]

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;
-- 
2.49.1

Re: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[Brett Sheffield]

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/

Re: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[Oscar Maes]

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

[PATCH net 1/2] net: ipv4: fix regression in local-broadcast routes

[Oscar Maes]

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);
-- 
2.39.5

[PATCH net 2/2] selftests: net: add test for destination in broadcast packets

[Oscar Maes]

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 $?
-- 
2.39.5

Re: [PATCH net 2/2] selftests: net: add test for destination in broadcast packets

[Brett A C Sheffield]

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

Re: [PATCH net 1/2] net: ipv4: fix regression in local-broadcast routes

[Jakub Kicinski]

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.
-- 
pw-bot: cr

Re: [PATCH net 1/2] net: ipv4: fix regression in local-broadcast routes

[Oscar Maes]

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

[PATCH net v2 1/2] net: ipv4: fix regression in local-broadcast routes

[Oscar Maes]

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;
-- 
2.39.5

[PATCH net v2 2/2] selftests: net: add test for destination in broadcast packets

[Oscar Maes]

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 $?
-- 
2.39.5

Re: [PATCH net v2 2/2] selftests: net: add test for destination in broadcast packets

[Jakub Kicinski]

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?

Re: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[kernel test robot]

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-fix-regression-in-broadcast-routes/20250825-181407
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@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[Paolo Abeni]

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-fix-regression-in-broadcast-routes/20250825-181407
> 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

Re: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[Brett A C Sheffield]

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-fix-regression-in-broadcast-routes/20250825-181407
> > 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
-- 

Re: [REGRESSION][BISECTED][PATCH] net: ipv4: fix regression in broadcast routes

[Oliver Sang]

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-fix-regression-in-broadcast-routes/20250825-181407
> > > 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
> -- 
>