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

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] When the KS8995 special tag has been stripped into VLAN metadata
  by the conduit MAC, flow dissection of the frame fails:…

--

net: dsa: tag_ks8995: Add the KS8995 tag handling

This adds a DSA tagger for the Micrel KS8995 / Microchip KSZ8995XA switch,
where the switch tag is a modified 0x8100 ethertype with one bit per port
set in the low nibble. The tagger always inserts a distinct outer tag on
transmit and moves an in-band tag into VLAN metadata on receive.

> diff --git a/net/dsa/tag_ks8995.c b/net/dsa/tag_ks8995.c
> new file mode 100644
> index 0000000000000..f011bf01845ef
> --- /dev/null
> +++ b/net/dsa/tag_ks8995.c
> @@ -0,0 +1,180 @@

[ ... ]

> +static struct sk_buff *ks8995_rcv(struct sk_buff *skb, struct net_device *dev)
> +{
> +	int portmask;
> +	int port;
> +	u16 etype;
> +
> +	/* The special tag may be in the packet or VLAN metadata. In either
> +	 * case, its TPID must retain the source-port bits.
> +	 *
> +	 * Read the in-band TPID directly because skb->protocol contains
> +	 * ETH_P_XDSA.
> +	 */
> +	if (skb_vlan_tag_present(skb))
> +		etype = ntohs(skb->vlan_proto);
> +	else
> +		etype = ntohs(*(__be16 *)dsa_etype_header_pos_rx(skb));

[Severity: Low]

This branch accepts a special tag that the conduit MAC has already
stripped into VLAN metadata, so skb->vlan_proto then holds 0x8101,
0x8102, 0x8104 or 0x8108. For those frames, can flow dissection on the
conduit still work?

In __skb_flow_dissect(), the protocol used to reach the DSA tagger hook is
taken from the VLAN metadata when it is present:

net/core/flow_dissector.c:__skb_flow_dissect() {
	...
		proto = skb_vlan_tag_present(skb) ?
			 skb->vlan_proto : skb->protocol;
	...
		if (unlikely(skb->dev && netdev_uses_dsa(skb->dev) &&
			     proto == htons(ETH_P_XDSA))) {
			...
			ops->flow_dissect(skb, &proto, &offset);
	...
}

With proto equal to 0x810X rather than ETH_P_XDSA, ks8995_flow_dissect()
is never called, and 0x810X matches no case in the protocol switch, so
dissection ends in:

net/core/flow_dissector.c:__skb_flow_dissect() {
	...
	default:
		fdret = FLOW_DISSECT_RET_OUT_BAD;
		break;
	...
}

The result is that skb_get_hash() derives a hash from zeroed keys, so RPS,
RFS and tc-flower matching on the conduit see no L3/L4 keys.

Two details worth noting: this only happens with a conduit MAC that strips
the non-standard 0x810X TPID into metadata while preserving the port bits,
which was not demonstrated here, and the same OUT_BAD outcome already
applies to other VLAN-metadata DSA taggers such as the tag_8021q family,
where proto becomes ETH_P_XDSA and also has no case in the switch. Should
the receive path here rely only on the in-band tag, or is degraded hashing
on the conduit acceptable for this configuration?

[ ... ]

> +static void ks8995_flow_dissect(const struct sk_buff *skb, __be16 *proto,
> +				int *offset)
> +{
> +	const __be16 *encap_proto;
> +	__be16 buffer;
> +
> +	*proto = 0;
> +	*offset = 0;
> +
> +	/* Extra TX headroom does not increase the in-band RX tag length. */
> +	encap_proto = skb_header_pointer(skb, KS8995_TAG_LEN - sizeof(buffer),
> +					 sizeof(buffer), &buffer);
> +	if (!encap_proto)
> +		return;
> +
> +	*proto = *encap_proto;
> +	*offset = KS8995_TAG_LEN;
> +}
> +
> +static const struct dsa_device_ops ks8995_netdev_ops = {
> +	.name = KS8995_NAME,
> +	.proto	= DSA_TAG_PROTO_KS8995,
> +	.xmit = ks8995_xmit,
> +	.rcv = ks8995_rcv,
> +	.flow_dissect = ks8995_flow_dissect,
> +	.needed_headroom = KS8995_NEEDED_HEADROOM,
> +};

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916-ks8995-to-ksz8-v15-0-8825ff5fd4be%40kernel.org