Re: [PATCH net-next v6 3/5] net: dsa: tag_ks8995: Add the KS8995 tag handling

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 13 Jun 2026 18:56:03 +0200 Linus Walleij wrote:
> Which is what I do.
> 
> So yeah. skb_free() will be free:ed twice. The code in tag_8021q.c will
> also do that. But what do you expect ->xmit() to return on error if
> not NULL?
> 
> When user.c does this:
> 
>     /* Transmit function may have to reallocate the original SKB,
>      * in which case it must have freed it. Only free it here on error.
>      */
>     nskb = p->xmit(skb, dev);
>     if (!nskb) {
>         kfree_skb(skb);
>         return NETDEV_TX_OK;
>     }
> 
>     return dsa_enqueue_skb(nskb, dev);
> 
> The only way to get clean out of this branch if you run
> into an error in ->xmit() is to return NULL!

Yes, maybe DSA experts remember the background here, and can guide us
But from a fresh look this and ->rcv have very odd semantics.

  nskb = func(skb);

should assume skb is either freed or returned. Freeing the input param
on failure of func() is a rather strange pattern.

I vote we drop these kfree_skb()s (both xmit and rcv) and fix up any
driver that depended on them (if any)?