Re: [PATCH net-next v9 5/8] hsr: Use the port and header information in hsr_forward_skb()

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-10-16 11:24:16 [+0200], Daniel Machon wrote:
> > The upper layer can submit a port on which the skb should be sent and
> > whether or not a HSR header should be added. Use this information while
> > forwarding the packet:
> > - If the port set, deliver it only on the requested port.
> > - If a header is already present then skip parts of the stack:
> >   - The packet originates from the master port and we don't want to
> >     assign a sequence number to it, it needs to preserve the original
> >     one.
> >   - The existing h_source must be used (and not replaced).
> >   - The node should already exist in the node_db if "other" packets are
> >     exchanged. If the node information is not yet existing then
> >     creating it makes no sense because it is used for deduplication of
> >     incoming packets and this is not used in the PTP case. If no other
> >     packets are exchanged then the node_db will age and then removed.
> > 
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> >  net/hsr/hsr_forward.c  | 45 ++++++++++++++++++++++++++++++++++++---------
> >  net/hsr/hsr_framereg.h |  2 ++
> >  2 files changed, 38 insertions(+), 9 deletions(-)
> > 
> > diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> > index 2b8ae9723bd7f..534798da48d53 100644
> > --- a/net/hsr/hsr_forward.c
> > +++ b/net/hsr/hsr_forward.c
> > @@ -426,7 +426,7 @@ static void hsr_deliver_master(struct sk_buff *skb, struct net_device *dev,
> >  static int hsr_xmit(struct sk_buff *skb, struct hsr_port *port,
> >  		    struct hsr_frame_info *frame)
> >  {
> > -	if (frame->port_rcv->type == HSR_PT_MASTER) {
> > +	if (frame->port_rcv->type == HSR_PT_MASTER && !frame->has_foreign_header) {
> >  		hsr_addr_subst_dest(frame->node_src, skb, port);
> >  
> >  		/* Address substitution (IEC62439-3 pp 26, 50): replace mac
> > @@ -547,11 +547,12 @@ bool hsr_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port)
> >  static void hsr_forward_do(struct hsr_frame_info *frame)
> >  {
> >  	struct hsr_port *port;
> > -	struct sk_buff *skb;
> 
> Is this needed, or just cleanup?

Sort of cleanup. It is still used in the loop where it is used. The NULL
init is not needed so It could stay as-is where it is.

> >  	bool sent = false;
> >  
> >  	hsr_for_each_port(frame->port_rcv->hsr, port) {
> >  		struct hsr_priv *hsr = port->hsr;
> > +		struct sk_buff *skb = NULL;
> > +
> >  		/* Don't send frame back the way it came */
> >  		if (port == frame->port_rcv)
> >  			continue;
> > @@ -570,6 +571,17 @@ static void hsr_forward_do(struct hsr_frame_info *frame)
> >  		if ((port->dev->features & NETIF_F_HW_HSR_DUP) && sent)
> >  			continue;
> >  
> > +		/* PTP TX packets have an outgoing port specified */
> > +		if (frame->req_tx_port != HSR_PT_NONE && frame->req_tx_port != port->type)
> > +			continue;
> 
> This checks that if the frame is directed and that the frame is only send to the
> directed port. Maybe the comments could be more clear about that.

Oki.

…
> > diff --git a/net/hsr/hsr_framereg.h b/net/hsr/hsr_framereg.h
> > index 127a3fb64d5f0..e5e6363c0466f 100644
> > --- a/net/hsr/hsr_framereg.h
> > +++ b/net/hsr/hsr_framereg.h
> > @@ -29,6 +29,8 @@ struct hsr_frame_info {
> >  	bool is_from_san;
> >  	bool dst_in_node_db;
> >  	bool dst_in_proxy_node_db;
> > +	bool has_foreign_header;
> > +	enum hsr_port_type req_tx_port;
> 
> The same flag is hsr_hdr in the uAPI, has_header in hsr_dev_xmit(),
> has_hsr_header in hsr_forward_skb() and has_foreign_header here. The
> port is tx_port in some places and req_tx_port in others. Could the naming be
> aligned?

I could align it with has_hsr_header probably.

> >  };
> >  
> >  void hsr_del_self_node(struct hsr_priv *hsr);
> 
> /Daniel

Sebastian