Re: [PATCH RFC net-next v3] hsr: Allow to send a specific port and with HSR header

Willem de Bruijn
2026-05-04T17:08:06Z

> > > +#define HSR_INLINE_HDR 0xaf485352
> > > +struct hsr_inline_header {
> > > + uint8_t tx_port;
> > > + uint8_t hsr_hdr;
> > > + uint8_t __pad0[4];
> > > + uint32_t magic;
> > > + uint8_t __pad1[2];
> > > + uint16_t eth_type;
> > > +} __packed;
> > > +
> >
> > No specific need to make this header ethhdr like?
>
> What do you mean? eth_type is at the same spot or do you mean it should
> be named h_proto?

I mean there is no reason your fake header needs to be 14B or
or otherwise resemble an Ethernet header.

That ties into my last comment to use sizeof(struct hsr_inline_header)
where working with that header, rather than ETH_HLEN. It is an
independent header.

This padding and packing is unnecessary.

> > > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> > > index 5555b71ab19b5..ac39b2347aa0f 100644
> > > --- a/net/hsr/hsr_device.c
> > > +++ b/net/hsr/hsr_device.c
> > > @@ -228,20 +228,51 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> > >
> > > 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->len > ETH_HLEN * 2) {
> > > + struct hsr_inline_header *hsr_opt;
> > > +
> > > + BUILD_BUG_ON(sizeof(struct hsr_inline_header) != sizeof(struct ethhdr));
> > > + hsr_opt = (struct hsr_inline_header *)skb_mac_header(skb);
> > > + if (hsr_opt->eth_type == htons(ETH_P_1588) &&
> > > + hsr_opt->magic == htonl(HSR_INLINE_HDR)) {
> > > + enum hsr_port_type tx_port;
> > > + bool has_header;
> > > +
> > > + 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 (!hsr_skb_add_header_port(skb, has_header, tx_port))
> > > + goto drop;
> >
> > All use of this information happens in the context of this ndo_start_xmit?
>
> I receive it from af_packet in HSR's ndo_start_xmit, yes. Then
> hsr_xmit() is forwarding it to the slave device via dev_queue_xmit().
> Here the skb->cb information gets overwritten.
>
> I need this hint in the slave eth driver in case there is hsr-offloading
> available.

This assumes that the slave device is somehow HSR aware. But standard
net_devices are not?

Isn't it HSR's ndo_start_xmit that selects which slave to forward to,
based on the information in this header?