Re: [PATCH net-next v11 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:
- [Medium] Insufficient `.needed_headroom` causes expensive SKB reallocation on the transmission fast-path for VLAN-tagged packets.
--

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

net: dsa: tag_ks8995: Add the KS8995 tag handling

This commit adds a new DSA tag driver for the Micrel KS8995 switch. It 
pushes a modified 802.1Q tag on transmission and untags it on reception, 
incorporating any hardware-accelerated VLAN tags into the packet payload.

> diff --git a/net/dsa/tag_ks8995.c b/net/dsa/tag_ks8995.c
> new file mode 100644
> index 0000000000000..f6ef1ef1891fd
> --- /dev/null
> +++ b/net/dsa/tag_ks8995.c
[ ... ]
> +static struct sk_buff *ks8995_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
[ ... ]
> +	/* The switch expects the special tag at offset 12. Move any hardware
> +	 * accelerated VLAN tag into the payload so the conduit cannot insert
> +	 * it outside the special tag.
> +	 */
> +	if (unlikely(skb_vlan_tag_present(skb))) {
> +		skb = __vlan_hwaccel_push_inside(skb);
> +		if (!skb)
> +			return NULL;
> +	}
> +
> +	/* Always add a distinct outer tag. The user port removes this field on
> +	 * egress, so reusing an existing 802.1Q tag would consume that VLAN tag.
> +	 */
> +	skb = vlan_insert_tag(skb, KS8995M_STPID(portmask), 0);
[ ... ]
> +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 = VLAN_HLEN,

[Severity: Medium]
Does this code reserve enough headroom for the worst-case transmission path?

When an SKB is transmitted via ks8995_xmit() with a hardware-accelerated
VLAN tag, the driver first pushes the hardware tag into the payload via
__vlan_hwaccel_push_inside() which consumes 4 bytes of headroom. It then
inserts the custom KS8995 switch tag via vlan_insert_tag(), consuming
another 4 bytes. This requires a total of 8 bytes of headroom.

Because needed_headroom only requests 4 bytes (VLAN_HLEN), if an SKB arrives
with exactly the requested headroom (such as when forwarded from another
interface), will the second tag insertion exhaust the headroom and trigger
an expensive SKB reallocation on the fast-path?

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-ks8995-to-ksz8-v11-0-0e04411a559d@kernel.org?part=2