Re: [PATCH net-next v2 1/4] net: hsr: add PRP interlink (RedBox) datapath and duplicate discard

[Fernando Fernandez Mancera <fmancera@suse.de>]

On Mon, 06 Jul 2026 13:41:28 +0000, Xin Xie <xiexinet@gmail.com> wrote:
> A PRP RedBox proxies SANs that sit behind an interlink port: their frames
> must reach the PRP network with the SAN source MAC preserved, and PRP
> unicast must be steered between the LAN and the SAN segment correctly.
> 
> Add the PRP interlink forwarding rules to prp_drop_frame() and give RedBox
> nodes a second duplicate-discard slot so the two LAN copies of a frame
> destined to a SAN collapse to a single delivery out the interlink.
> 
> The destination classification (is the unicast DA a PRP-network node or a
> proxied SAN) is resolved once per frame in fill_frame_info(), gated to PRP
> RedBox devices, and cached in struct hsr_frame_info, so prp_drop_frame()
> stays O(1) and does not walk the node tables for every candidate egress
> port in the softIRQ path. HSR RedBox frame classification is untouched.
> 
> Factor the LAN A/B duplicate test into prp_is_lan_dup() so the new PRP
> interlink rules do not change hsr_drop_frame() behaviour, including the
> NETIF_F_HW_HSR_FWD path which keeps using the LAN-duplicate test only.
> 
> Publish the RedBox state before the first hsr_add_port(): the slave and
> interlink rx handlers are live from hsr_add_port() on and rtnl does not
> stop softirq processing, so a frame could otherwise be handled while
> hsr->redbox is still false. hsr_add_node() sizes each node's per-port
> sequence state from hsr->redbox; a node learned in that window would get
> a single-port sequence block, breaking the interlink duplicate discard
> (WARN_ON_ONCE plus duplicate delivery to the SAN) and letting the
> supervision sequence-block merge read beyond the source node's allocated
> sequence bitmap. Publishing the flag before any port exists makes the
> per-node sizing uniform by construction. This is safe: the proxy
> announce timer is only armed from hsr_check_announce() once the master
> is running, the packet-path readers of hsr->redbox tolerate an empty
> proxy node database and an absent interlink port, and the
> prune_proxy_timer is still armed only after the interlink port has been
> attached successfully.
> 
> Additionally bound the supervision sequence-block merge by the smaller
> of the two nodes' seq_port_cnt as defense in depth against mismatched
> node sizes.
> 
> Signed-off-by: Xin Xie <xiexinet@gmail.com>
>
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index 5555b71ab19b..5af491ed2b72 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -768,6 +768,15 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
>  	/* Make sure the 1st call to netif_carrier_on() gets through */
>  	netif_carrier_off(hsr_dev);
>  
> +	/* Publish the RedBox state before any port is attached: the rx
> +	 * handlers are live from hsr_add_port() on, and hsr_add_node()
> +	 * sizes each node's per-port sequence state from hsr->redbox.
> +	 */
> +	if (interlink) {
> +		hsr->redbox = true;
> +		ether_addr_copy(hsr->macaddress_redbox, interlink->dev_addr);
> +	}
> +
>  	res = hsr_add_port(hsr, hsr_dev, HSR_PT_MASTER, extack);
>  	if (res)
>  		goto err_add_master;
> @@ -805,8 +814,6 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
>  		if (res)
>  			goto err_unregister;
>  
> -		hsr->redbox = true;
> -		ether_addr_copy(hsr->macaddress_redbox, interlink->dev_addr);
>  		mod_timer(&hsr->prune_proxy_timer,
>  			  jiffies + msecs_to_jiffies(PRUNE_PROXY_PERIOD));
>  	}
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index 0774981a65c1..efcd0acef38c 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -440,12 +440,37 @@ static int hsr_xmit(struct sk_buff *skb, struct hsr_port *port,
>  	return dev_queue_xmit(skb);
>  }
>  
> +static bool prp_is_lan_dup(struct hsr_frame_info *frame,
> +			   struct hsr_port *port)
> +{
> +	enum hsr_port_type rx = frame->port_rcv->type;
> +
> +	return (rx == HSR_PT_SLAVE_A && port->type == HSR_PT_SLAVE_B) ||
> +	       (rx == HSR_PT_SLAVE_B && port->type == HSR_PT_SLAVE_A);
> +}
> +
>  bool prp_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port)
>  {
> -	return ((frame->port_rcv->type == HSR_PT_SLAVE_A &&
> -		 port->type == HSR_PT_SLAVE_B) ||
> -		(frame->port_rcv->type == HSR_PT_SLAVE_B &&
> -		 port->type == HSR_PT_SLAVE_A));
> +	enum hsr_port_type rx = frame->port_rcv->type;
> +
> +	/* Supervision frames are not delivered to a SAN on the interlink. */
> +	if (frame->is_supervision && port->type == HSR_PT_INTERLINK)
> +		return true;
> +
> +	if (prp_is_lan_dup(frame, port))
> +		return true;

Since we already have rx (enum hsr_port_type) here, can we pass this
directly to prp_is_lan_dup() function instead of passing frame?

> +
> +	/* LAN to interlink: keep PRP-network unicast off the SAN segment. */
> +	if ((rx == HSR_PT_SLAVE_A || rx == HSR_PT_SLAVE_B) &&
> +	    port->type == HSR_PT_INTERLINK)
> +		return frame->dst_in_node_db;
> +
> +	/* Interlink to LAN: keep SAN-to-SAN unicast local. */
> +	if ((port->type == HSR_PT_SLAVE_A || port->type == HSR_PT_SLAVE_B) &&
> +	    rx == HSR_PT_INTERLINK)
> +		return frame->dst_in_proxy_node_db;
> +
> +	return false;
>  }
>  
>  bool hsr_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port)
> @@ -453,7 +478,7 @@ bool hsr_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port)
>  	struct sk_buff *skb;
>  
>  	if (port->dev->features & NETIF_F_HW_HSR_FWD)
> -		return prp_drop_frame(frame, port);
> +		return prp_is_lan_dup(frame, port);
>  

Of course these calls will need to pass rx too.