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

[Nikolay Aleksandrov <razor@blackwall.org>]

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>