[PATCH net-next 0/2] bridge: Prevent unicast ARP/NS packets from being suppressed by bridge

[PATCH net-next 0/2] bridge: Prevent unicast ARP/NS packets from being suppressed by bridge

[Petr Machata]

From: Amit Cohen <amcohen@nvidia.com>

Currently, unicast ARP requests/NS packets are replied by bridge when
suppression is enabled, then they are also forwarded, which results two
replicas of ARP reply/NA - one from the bridge and second from the target.

The purpose of ARP/ND suppression is to reduce flooding in the broadcast
domain, which is not relevant for unicast packets. In addition, the use
case of unicast ARP/NS is to poll a specific host, so it does not make
sense to have the switch answer on behalf of the host.

Forward ARP requests/NS packets and prevent the bridge from replying to
them.

Patch set overview:
Patch #1 prevents unicast ARP/NS packets from being suppressed by bridge
Patch #2 adds test cases for unicast ARP/NS with suppression enabled

Amit Cohen (2):
  net: bridge: Prevent unicast ARP/NS packets from being suppressed by
    bridge
  selftests: test_bridge_neigh_suppress: Test unicast ARP/NS with
    suppression

 net/bridge/br_arp_nd_proxy.c                  |   7 +
 .../net/test_bridge_neigh_suppress.sh         | 125 ++++++++++++++++++
 2 files changed, 132 insertions(+)

-- 
2.47.0

[PATCH net-next 1/2] net: bridge: Prevent unicast ARP/NS packets from being suppressed by bridge

[Petr Machata]

From: Amit Cohen <amcohen@nvidia.com>

When Proxy ARP or ARP/ND suppression are enabled, ARP/NS packets can be
handled by bridge in br_do_proxy_suppress_arp()/br_do_suppress_nd().
For broadcast packets, they are replied by bridge, but later they are not
flooded. Currently, unicast packets are replied by bridge when suppression
is enabled, and they are also forwarded, which results two replicas of
ARP reply/NA - one from the bridge and second from the target.

RFC 1122 describes use case for unicat ARP packets - "unicast poll" -
actively poll the remote host by periodically sending a point-to-point ARP
request to it, and delete the entry if no ARP reply is received from N
successive polls.

The purpose of ARP/ND suppression is to reduce flooding in the broadcast
domain. If a host is sending a unicast ARP/NS, then it means it already
knows the address and the switches probably know it as well and there
will not be any flooding.

In addition, the use case of unicast ARP/NS is to poll a specific host,
so it does not make sense to have the switch answer on behalf of the host.

According to RFC 9161:
"A PE SHOULD reply to broadcast/multicast address resolution messages,
i.e., ARP Requests, ARP probes, NS messages, as well as DAD NS messages.
An ARP probe is an ARP Request constructed with an all-zero sender IP
address that may be used by hosts for IPv4 Address Conflict Detection as
specified in [RFC5227]. A PE SHOULD NOT reply to unicast address resolution
requests (for instance, NUD NS messages)."

Forward such requests and prevent the bridge from replying to them.

Reported-by: Denis Yulevych <denisyu@nvidia.com>
Signed-off-by: Amit Cohen <amcohen@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 net/bridge/br_arp_nd_proxy.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index 115a23054a58..1e2b51769eec 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -160,6 +160,9 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
 	if (br_opt_get(br, BROPT_NEIGH_SUPPRESS_ENABLED)) {
 		if (br_is_neigh_suppress_enabled(p, vid))
 			return;
+		if (is_unicast_ether_addr(eth_hdr(skb)->h_dest) &&
+		    parp->ar_op == htons(ARPOP_REQUEST))
+			return;
 		if (parp->ar_op != htons(ARPOP_RREQUEST) &&
 		    parp->ar_op != htons(ARPOP_RREPLY) &&
 		    (ipv4_is_zeronet(sip) || sip == tip)) {
@@ -410,6 +413,10 @@ void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
 	if (br_is_neigh_suppress_enabled(p, vid))
 		return;
 
+	if (is_unicast_ether_addr(eth_hdr(skb)->h_dest) &&
+	    msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION)
+		return;
+
 	if (msg->icmph.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT &&
 	    !msg->icmph.icmp6_solicited) {
 		/* prevent flooding to neigh suppress ports */
-- 
2.47.0

[PATCH net-next 2/2] selftests: test_bridge_neigh_suppress: Test unicast ARP/NS with suppression

[Petr Machata]

From: Amit Cohen <amcohen@nvidia.com>

Add test cases to check that unicast ARP/NS packets are replied once, even
if ARP/ND suppression is enabled.

Without the previous patch:
$ ./test_bridge_neigh_suppress.sh
...
Unicast ARP, per-port ARP suppression - VLAN 10
-----------------------------------------------
TEST: "neigh_suppress" is on                                        [ OK ]
TEST: Unicast ARP, suppression on, h1 filter                        [FAIL]
TEST: Unicast ARP, suppression on, h2 filter                        [ OK ]

Unicast ARP, per-port ARP suppression - VLAN 20
-----------------------------------------------
TEST: "neigh_suppress" is on                                        [ OK ]
TEST: Unicast ARP, suppression on, h1 filter                        [FAIL]
TEST: Unicast ARP, suppression on, h2 filter                        [ OK ]
...
Unicast NS, per-port NS suppression - VLAN 10
---------------------------------------------
TEST: "neigh_suppress" is on                                        [ OK ]
TEST: Unicast NS, suppression on, h1 filter                         [FAIL]
TEST: Unicast NS, suppression on, h2 filter                         [ OK ]

Unicast NS, per-port NS suppression - VLAN 20
---------------------------------------------
TEST: "neigh_suppress" is on                                        [ OK ]
TEST: Unicast NS, suppression on, h1 filter                         [FAIL]
TEST: Unicast NS, suppression on, h2 filter                         [ OK ]
...
Tests passed: 156
Tests failed:   4

With the previous patch:
$ ./test_bridge_neigh_suppress.sh
...
Unicast ARP, per-port ARP suppression - VLAN 10
-----------------------------------------------
TEST: "neigh_suppress" is on                                        [ OK ]
TEST: Unicast ARP, suppression on, h1 filter                        [ OK ]
TEST: Unicast ARP, suppression on, h2 filter                        [ OK ]

Unicast ARP, per-port ARP suppression - VLAN 20
-----------------------------------------------
TEST: "neigh_suppress" is on                                        [ OK ]
TEST: Unicast ARP, suppression on, h1 filter                        [ OK ]
TEST: Unicast ARP, suppression on, h2 filter                        [ OK ]
...
Unicast NS, per-port NS suppression - VLAN 10
---------------------------------------------
TEST: "neigh_suppress" is on                                        [ OK ]
TEST: Unicast NS, suppression on, h1 filter                         [ OK ]
TEST: Unicast NS, suppression on, h2 filter                         [ OK ]

Unicast NS, per-port NS suppression - VLAN 20
---------------------------------------------
TEST: "neigh_suppress" is on                                        [ OK ]
TEST: Unicast NS, suppression on, h1 filter                         [ OK ]
TEST: Unicast NS, suppression on, h2 filter                         [ OK ]
...
Tests passed: 160
Tests failed:   0

Signed-off-by: Amit Cohen <amcohen@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 .../net/test_bridge_neigh_suppress.sh         | 125 ++++++++++++++++++
 1 file changed, 125 insertions(+)

diff --git a/tools/testing/selftests/net/test_bridge_neigh_suppress.sh b/tools/testing/selftests/net/test_bridge_neigh_suppress.sh
index 02b986c9c247..9067197c9055 100755
--- a/tools/testing/selftests/net/test_bridge_neigh_suppress.sh
+++ b/tools/testing/selftests/net/test_bridge_neigh_suppress.sh
@@ -51,7 +51,9 @@ ret=0
 # All tests in this script. Can be overridden with -t option.
 TESTS="
 	neigh_suppress_arp
+	neigh_suppress_uc_arp
 	neigh_suppress_ns
+	neigh_suppress_uc_ns
 	neigh_vlan_suppress_arp
 	neigh_vlan_suppress_ns
 "
@@ -388,6 +390,52 @@ neigh_suppress_arp()
 	neigh_suppress_arp_common $vid $sip $tip
 }
 
+neigh_suppress_uc_arp_common()
+{
+	local vid=$1; shift
+	local sip=$1; shift
+	local tip=$1; shift
+	local tmac
+
+	echo
+	echo "Unicast ARP, per-port ARP suppression - VLAN $vid"
+	echo "-----------------------------------------------"
+
+	run_cmd "bridge -n $sw1 link set dev vx0 neigh_suppress on"
+	run_cmd "bridge -n $sw1 -d link show dev vx0 | grep \"neigh_suppress on\""
+	log_test $? 0 "\"neigh_suppress\" is on"
+
+	tmac=$(ip -n $h2 -j -p link show eth0.$vid | jq -r '.[]["address"]')
+	run_cmd "bridge -n $sw1 fdb replace $tmac dev vx0 master static vlan $vid"
+	run_cmd "ip -n $sw1 neigh replace $tip lladdr $tmac nud permanent dev br0.$vid"
+
+	run_cmd "tc -n $h1 qdisc replace dev eth0.$vid clsact"
+	run_cmd "tc -n $h1 filter replace dev eth0.$vid ingress pref 1 handle 101 proto arp flower arp_sip $tip arp_op reply action pass"
+
+	run_cmd "tc -n $h2 qdisc replace dev eth0.$vid clsact"
+	run_cmd "tc -n $h2 filter replace dev eth0.$vid egress pref 1 handle 101 proto arp flower arp_tip $sip arp_op reply action pass"
+
+	run_cmd "ip netns exec $h1 mausezahn eth0.$vid -c 1 -a own -b $tmac -t arp 'request sip=$sip, tip=$tip, tmac=$tmac' -q"
+	tc_check_packets $h1 "dev eth0.$vid ingress" 101 1
+	log_test $? 0 "Unicast ARP, suppression on, h1 filter"
+	tc_check_packets $h2 "dev eth0.$vid egress" 101 1
+	log_test $? 0 "Unicast ARP, suppression on, h2 filter"
+}
+
+neigh_suppress_uc_arp()
+{
+	local vid=10
+	local sip=192.0.2.1
+	local tip=192.0.2.2
+
+	neigh_suppress_uc_arp_common $vid $sip $tip
+
+	vid=20
+	sip=192.0.2.17
+	tip=192.0.2.18
+	neigh_suppress_uc_arp_common $vid $sip $tip
+}
+
 neigh_suppress_ns_common()
 {
 	local vid=$1; shift
@@ -494,6 +542,78 @@ neigh_suppress_ns()
 	neigh_suppress_ns_common $vid $saddr $daddr $maddr
 }
 
+icmpv6_header_get()
+{
+	local csum=$1; shift
+	local tip=$1; shift
+	local type
+	local p
+
+	# Type 135 (Neighbor Solicitation), hex format
+	type="87"
+	p=$(:
+		)"$type:"$(                     : ICMPv6.type
+		)"00:"$(                        : ICMPv6.code
+		)"$csum:"$(                     : ICMPv6.checksum
+		)"00:00:00:00:"$(               : Reserved
+	        )"$tip:"$(	                : Target Address
+		)
+	echo $p
+}
+
+neigh_suppress_uc_ns_common()
+{
+	local vid=$1; shift
+	local sip=$1; shift
+	local dip=$1; shift
+	local full_dip=$1; shift
+	local csum=$1; shift
+	local tmac
+
+	echo
+	echo "Unicast NS, per-port NS suppression - VLAN $vid"
+	echo "---------------------------------------------"
+
+	run_cmd "bridge -n $sw1 link set dev vx0 neigh_suppress on"
+	run_cmd "bridge -n $sw1 -d link show dev vx0 | grep \"neigh_suppress on\""
+	log_test $? 0 "\"neigh_suppress\" is on"
+
+	tmac=$(ip -n $h2 -j -p link show eth0.$vid | jq -r '.[]["address"]')
+	run_cmd "bridge -n $sw1 fdb replace $tmac dev vx0 master static vlan $vid"
+	run_cmd "ip -n $sw1 -6 neigh replace $dip lladdr $tmac nud permanent dev br0.$vid"
+
+	run_cmd "tc -n $h1 qdisc replace dev eth0.$vid clsact"
+	run_cmd "tc -n $h1 filter replace dev eth0.$vid ingress pref 1 handle 101 proto ipv6 flower ip_proto icmpv6 src_ip $dip type 136 code 0 action pass"
+
+	run_cmd "tc -n $h2 qdisc replace dev eth0.$vid clsact"
+	run_cmd "tc -n $h2 filter replace dev eth0.$vid egress pref 1 handle 101 proto ipv6 flower ip_proto icmpv6 dst_ip $sip type 136 code 0 action pass"
+
+	run_cmd "ip netns exec $h1 mausezahn -6 eth0.$vid -c 1 -a own -b $tmac -A $sip -B $dip -t ip hop=255,next=58,payload=$(icmpv6_header_get $csum $full_dip) -q"
+	tc_check_packets $h1 "dev eth0.$vid ingress" 101 1
+	log_test $? 0 "Unicast NS, suppression on, h1 filter"
+	tc_check_packets $h2 "dev eth0.$vid egress" 101 1
+	log_test $? 0 "Unicast NS, suppression on, h2 filter"
+}
+
+neigh_suppress_uc_ns()
+{
+	local vid=10
+	local saddr=2001:db8:1::1
+	local daddr=2001:db8:1::2
+	local full_daddr=20:01:0d:b8:00:01:00:00:00:00:00:00:00:00:00:02
+	local csum="ef:79"
+
+	neigh_suppress_uc_ns_common $vid $saddr $daddr $full_daddr $csum
+
+	vid=20
+	saddr=2001:db8:2::1
+	daddr=2001:db8:2::2
+	full_daddr=20:01:0d:b8:00:02:00:00:00:00:00:00:00:00:00:02
+	csum="ef:76"
+
+	neigh_suppress_uc_ns_common $vid $saddr $daddr $full_daddr $csum
+}
+
 neigh_vlan_suppress_arp()
 {
 	local vid1=10
@@ -825,6 +945,11 @@ if [ ! -x "$(command -v jq)" ]; then
 	exit $ksft_skip
 fi
 
+if [ ! -x "$(command -v mausezahn)" ]; then
+	echo "SKIP: Could not run test without mausezahn tool"
+	exit $ksft_skip
+fi
+
 bridge link help 2>&1 | grep -q "neigh_vlan_suppress"
 if [ $? -ne 0 ]; then
    echo "SKIP: iproute2 bridge too old, missing per-VLAN neighbor suppression support"
-- 
2.47.0

Re: [PATCH net-next 1/2] net: bridge: Prevent unicast ARP/NS packets from being suppressed by bridge

[Nikolay Aleksandrov]

On 4/8/25 18:40, Petr Machata wrote:
> From: Amit Cohen <amcohen@nvidia.com>
> 
> When Proxy ARP or ARP/ND suppression are enabled, ARP/NS packets can be
> handled by bridge in br_do_proxy_suppress_arp()/br_do_suppress_nd().
> For broadcast packets, they are replied by bridge, but later they are not
> flooded. Currently, unicast packets are replied by bridge when suppression
> is enabled, and they are also forwarded, which results two replicas of
> ARP reply/NA - one from the bridge and second from the target.
> 
> RFC 1122 describes use case for unicat ARP packets - "unicast poll" -
> actively poll the remote host by periodically sending a point-to-point ARP
> request to it, and delete the entry if no ARP reply is received from N
> successive polls.
> 
> The purpose of ARP/ND suppression is to reduce flooding in the broadcast
> domain. If a host is sending a unicast ARP/NS, then it means it already
> knows the address and the switches probably know it as well and there
> will not be any flooding.
> 
> In addition, the use case of unicast ARP/NS is to poll a specific host,
> so it does not make sense to have the switch answer on behalf of the host.
> 
> According to RFC 9161:
> "A PE SHOULD reply to broadcast/multicast address resolution messages,
> i.e., ARP Requests, ARP probes, NS messages, as well as DAD NS messages.
> An ARP probe is an ARP Request constructed with an all-zero sender IP
> address that may be used by hosts for IPv4 Address Conflict Detection as
> specified in [RFC5227]. A PE SHOULD NOT reply to unicast address resolution
> requests (for instance, NUD NS messages)."
> 
> Forward such requests and prevent the bridge from replying to them.
> 
> Reported-by: Denis Yulevych <denisyu@nvidia.com>
> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> ---
>  net/bridge/br_arp_nd_proxy.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
> index 115a23054a58..1e2b51769eec 100644
> --- a/net/bridge/br_arp_nd_proxy.c
> +++ b/net/bridge/br_arp_nd_proxy.c
> @@ -160,6 +160,9 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
>  	if (br_opt_get(br, BROPT_NEIGH_SUPPRESS_ENABLED)) {
>  		if (br_is_neigh_suppress_enabled(p, vid))
>  			return;
> +		if (is_unicast_ether_addr(eth_hdr(skb)->h_dest) &&
> +		    parp->ar_op == htons(ARPOP_REQUEST))
> +			return;
>  		if (parp->ar_op != htons(ARPOP_RREQUEST) &&
>  		    parp->ar_op != htons(ARPOP_RREPLY) &&
>  		    (ipv4_is_zeronet(sip) || sip == tip)) {
> @@ -410,6 +413,10 @@ void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
>  	if (br_is_neigh_suppress_enabled(p, vid))
>  		return;
>  
> +	if (is_unicast_ether_addr(eth_hdr(skb)->h_dest) &&
> +	    msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION)
> +		return;
> +
>  	if (msg->icmph.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT &&
>  	    !msg->icmph.icmp6_solicited) {
>  		/* prevent flooding to neigh suppress ports */

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 2/2] selftests: test_bridge_neigh_suppress: Test unicast ARP/NS with suppression

[Nikolay Aleksandrov]

On 4/8/25 18:40, Petr Machata wrote:
> From: Amit Cohen <amcohen@nvidia.com>
> 
> Add test cases to check that unicast ARP/NS packets are replied once, even
> if ARP/ND suppression is enabled.
> 
> Without the previous patch:
> $ ./test_bridge_neigh_suppress.sh
> ...
> Unicast ARP, per-port ARP suppression - VLAN 10
> -----------------------------------------------
> TEST: "neigh_suppress" is on                                        [ OK ]
> TEST: Unicast ARP, suppression on, h1 filter                        [FAIL]
> TEST: Unicast ARP, suppression on, h2 filter                        [ OK ]
> 
> Unicast ARP, per-port ARP suppression - VLAN 20
> -----------------------------------------------
> TEST: "neigh_suppress" is on                                        [ OK ]
> TEST: Unicast ARP, suppression on, h1 filter                        [FAIL]
> TEST: Unicast ARP, suppression on, h2 filter                        [ OK ]
> ...
> Unicast NS, per-port NS suppression - VLAN 10
> ---------------------------------------------
> TEST: "neigh_suppress" is on                                        [ OK ]
> TEST: Unicast NS, suppression on, h1 filter                         [FAIL]
> TEST: Unicast NS, suppression on, h2 filter                         [ OK ]
> 
> Unicast NS, per-port NS suppression - VLAN 20
> ---------------------------------------------
> TEST: "neigh_suppress" is on                                        [ OK ]
> TEST: Unicast NS, suppression on, h1 filter                         [FAIL]
> TEST: Unicast NS, suppression on, h2 filter                         [ OK ]
> ...
> Tests passed: 156
> Tests failed:   4
> 
> With the previous patch:
> $ ./test_bridge_neigh_suppress.sh
> ...
> Unicast ARP, per-port ARP suppression - VLAN 10
> -----------------------------------------------
> TEST: "neigh_suppress" is on                                        [ OK ]
> TEST: Unicast ARP, suppression on, h1 filter                        [ OK ]
> TEST: Unicast ARP, suppression on, h2 filter                        [ OK ]
> 
> Unicast ARP, per-port ARP suppression - VLAN 20
> -----------------------------------------------
> TEST: "neigh_suppress" is on                                        [ OK ]
> TEST: Unicast ARP, suppression on, h1 filter                        [ OK ]
> TEST: Unicast ARP, suppression on, h2 filter                        [ OK ]
> ...
> Unicast NS, per-port NS suppression - VLAN 10
> ---------------------------------------------
> TEST: "neigh_suppress" is on                                        [ OK ]
> TEST: Unicast NS, suppression on, h1 filter                         [ OK ]
> TEST: Unicast NS, suppression on, h2 filter                         [ OK ]
> 
> Unicast NS, per-port NS suppression - VLAN 20
> ---------------------------------------------
> TEST: "neigh_suppress" is on                                        [ OK ]
> TEST: Unicast NS, suppression on, h1 filter                         [ OK ]
> TEST: Unicast NS, suppression on, h2 filter                         [ OK ]
> ...
> Tests passed: 160
> Tests failed:   0
> 
> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> ---
>  .../net/test_bridge_neigh_suppress.sh         | 125 ++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 

Thanks,
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 0/2] bridge: Prevent unicast ARP/NS packets from being suppressed by bridge

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 8 Apr 2025 17:40:22 +0200 you wrote:
> From: Amit Cohen <amcohen@nvidia.com>
> 
> Currently, unicast ARP requests/NS packets are replied by bridge when
> suppression is enabled, then they are also forwarded, which results two
> replicas of ARP reply/NA - one from the bridge and second from the target.
> 
> The purpose of ARP/ND suppression is to reduce flooding in the broadcast
> domain, which is not relevant for unicast packets. In addition, the use
> case of unicast ARP/NS is to poll a specific host, so it does not make
> sense to have the switch answer on behalf of the host.
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net: bridge: Prevent unicast ARP/NS packets from being suppressed by bridge
    https://git.kernel.org/netdev/net-next/c/827b2ac8e796
  - [net-next,2/2] selftests: test_bridge_neigh_suppress: Test unicast ARP/NS with suppression
    https://git.kernel.org/netdev/net-next/c/0ffb594212a0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html