Re: [PATCH net-next v6 1/8] hsr: Add header_ops::parse_protocol

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch has no Fixes: tag.

--

hsr: Add header_ops::parse_protocol

This patch adds eth_header_parse_protocol() as the parse_protocol callback
in hsr_header_ops. With it, af_packet's dev_parse_header_protocol() sets
skb->protocol correctly on hsr devices. Before this change the callback was
missing and the field was set to 0.

> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index fc512204600cd..59df647d2fd45 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -247,6 +247,7 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>  static const struct header_ops hsr_header_ops = {
>  	.create	 = eth_header,
>  	.parse	 = eth_header_parse,
> +	.parse_protocol = eth_header_parse_protocol,
>  };

[Severity: Low]
This isn't a bug, but should this patch carry a Fixes: tag?

hsr_dev_setup() calls ether_setup(), which installs eth_header_ops. It then
replaces them:

net/hsr/hsr_device.c:hsr_dev_setup() {
    ...
	dev->header_ops = &hsr_header_ops;
    ...
}

The eth_header_ops parse_protocol callback is lost at this point. So for
SOCK_RAW sends with protocol 0 or ETH_P_ALL on an hsr device,
packet_parse_headers() has been leaving skb->protocol at 0 on the master:

net/packet/af_packet.c:packet_parse_headers() {
    ...
		skb->protocol = dev_parse_header_protocol(skb);
    ...
}

The frames that reach the slave ports still look correct. In hsr_forward.c,
fill_frame_info() reads ethhdr->h_proto directly. hsr_fill_tag(),
prp_fill_rct() and create_stripped_skb_hsr() also re-derive skb->protocol
from the header, for example:

net/hsr/hsr_forward.c:hsr_fill_tag() {
    ...
	skb->protocol = hsr_ethhdr->ethhdr.h_proto;
    ...
}

The remaining effects look limited to things like protocol-based tc or tap
matching on hsr0 egress. The patch also reads as preparation for the PTP
series rather than a fix. If it isn't meant for stable, leaving out the
Fixes: tag seems fine. Was that the intent?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923-hsr_ptp-v6-0-6ea07b3fb8a8%40linutronix.de