Re: [PATCH net-next v13 2/4] net: dsa: tag_ks8995: Add the KS8995 tag handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The ks8995 tagger omits the `.flow_dissect` callback, causing the generic flow dissector to use an incorrect 8-byte offset for RX packets instead of 4 bytes, which breaks flow dissection on the conduit device.
--

commit beb5bab677cea0349bf40d65eeebd8d0ad76ec83
Author: Linus Walleij <linusw@kernel.org>

net: dsa: tag_ks8995: Add the KS8995 tag handling

This commit adds DSA tagging support for the Micrel KS8995 switch family.
It implements custom ingress and egress tagging using a modified 802.1Q
tag format and registers the protocol operations.

> diff --git a/net/dsa/tag_ks8995.c b/net/dsa/tag_ks8995.c
> new file mode 100644
> index 0000000000000..435665013a864
> --- /dev/null
> +++ b/net/dsa/tag_ks8995.c
[ ... ]
> +
> +#define KS8995_NAME "ks8995"
> +
> +#define KS8995_TAG_LEN		VLAN_HLEN
> +/* Reserve room for the switch tag and a hardware-accelerated VLAN tag. */
> +#define KS8995_NEEDED_HEADROOM	(KS8995_TAG_LEN + VLAN_HLEN)

[Severity: High]
Does setting this headroom to 8 bytes cause issues during RX flow dissection?
Because the needed_headroom is 8 bytes, dsa_tag_generic_flow_dissect() will
erroneously assume the RX tag is also 8 bytes long.

[ ... ]
> +static const struct dsa_device_ops ks8995_netdev_ops = {
> +	.name = KS8995_NAME,
> +	.proto	= DSA_TAG_PROTO_KS8995,
> +	.xmit = ks8995_xmit,
> +	.rcv = ks8995_rcv,
> +	.needed_headroom = KS8995_NEEDED_HEADROOM,
> +};

[Severity: High]
Because there is no custom .flow_dissect override here, the DSA core falls
back to the generic flow dissector, which will use the 8-byte needed_headroom
as the RX tag length instead of 4 bytes.

Will this corrupt protocol extraction by skipping 8 bytes into the packet
instead of 4? This could cause the network core to read an invalid
encapsulated EtherType and break Receive Packet Steering (RPS), Transmit
Packet Steering (XPS), and Traffic Control (TC) rules on the conduit
interface.

Should a custom .flow_dissect callback be provided to correctly return a
4-byte offset (or 0 if the tag was stripped into hardware metadata)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913-ks8995-to-ksz8-v13-0-327b16d309eb@kernel.org?part=2