Re: [PATCH net-next v9 3/8] hsr: Add a magic header for sending PTP packets

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

On 2026-10-16 11:15:12 [+0200], Daniel Machon wrote:
> > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> > index 6d96bc4a12446..c5ea7ef0209db 100644
> > --- a/net/hsr/hsr_device.c
> > +++ b/net/hsr/hsr_device.c
> > @@ -223,24 +223,66 @@ static netdev_features_t hsr_fix_features(struct net_device *dev,
> >  
> >  static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> >  {
> > +	enum hsr_port_type tx_port = HSR_PT_NONE;
> >  	struct hsr_priv *hsr = netdev_priv(dev);
> >  	struct hsr_port *master;
> > +	bool has_header = false;
> >  
> >  	rcu_read_lock();
> >  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
> > -	if (master) {
> > -		skb->dev = master->dev;
> > -		skb_reset_mac_header(skb);
> > -		skb_reset_mac_len(skb);
> > -		spin_lock_bh(&hsr->seqnr_lock);
> > -		hsr_forward_skb(skb, master);
> > -		spin_unlock_bh(&hsr->seqnr_lock);
> > -	} else {
> > -		dev_core_stats_tx_dropped_inc(dev);
> > -		dev_kfree_skb_any(skb);
> > +	if (!master)
> > +		goto drop;
> > +
> > +	skb->dev = master->dev;
> > +	if (skb->protocol == htons(ETH_P_1588)) {
> 
> Seems like this is also accepted for PRP - should it?

Nothing bad happens if one uses this. The timestamps will be missing so
worst thing would be a complaint. If the packet is dropped the same
complain might follow. It doesn't make sense to use.

> > +		struct hsr_inline_header *hsr_opt;
> > +		struct ethhdr *eth_hdr;
> > +		unsigned int hdr_len;
> > +
> > +		BUILD_BUG_ON(sizeof(struct hsr_inline_header) != sizeof(struct ethhdr));
> > +
> > +		/* need to access the magic header */
> > +		if (!pskb_may_pull(skb, sizeof(struct hsr_inline_header)))
> > +			goto drop;
> > +
> > +		hsr_opt = (struct hsr_inline_header *)skb_mac_header(skb);
> > +		if (hsr_opt->magic != htonl(HSR_INLINE_HDR))
> > +			goto drop;
> > +
> > +		has_header = hsr_opt->hsr_hdr;
> > +		tx_port = hsr_opt->tx_port;
> > +		if (tx_port != HSR_PT_SLAVE_A && tx_port != HSR_PT_SLAVE_B)
> > +			goto drop;
> > +
> > +		if (has_header)
> > +			hdr_len = ETH_HLEN + HSR_HLEN;
> > +		else
> > +			hdr_len = ETH_HLEN;
> 
> Nit: Maybe hdr_len = ETH_HLEN during declaration, hdr_len += HSR_HLEN reads a
> bit simpler.

Okay.
 
> /Daniel

Sebastian