Re: [PATCH net-next v2] net: dsa: Fix skb ownership in taggers

On Tue, Jun 16, 2026 at 11:37 PM Vladimir Oltean [off-list ref] wrote:

From my perspective, the tradeoff between pros and cons is not so well
explained. Consider the following not mentioned in the commit message:

- Changing the kfree_skb() convention, without any mechanical obstacle
  preventing the backporting of patches that are written assuming one
  convention down to trees expecting the other (obstacle like a failure
  to compile, for example, which would warn people of their otherwise
  silent incompatibility), is an avoidable experience (at best) from a
  maintainance perspective.

I added a terse blurb like this:

NOTICE: Backporting patches to taggers (e.g. for stable kernels) after
    this point cannot be mechanical or they will introduce double
    kfree_skb().

However, usually we do not consider obstacles for backporting things to
stable to be a major concern, c.f. Documentation/process/stable-api-nonsense.rst

- Has anyone proven that a real problem exists? Because dsa_user_xmit()
  -> skb_ensure_writable_head_tail() has run successfully at this stage,
  so we know that dev->needed_headroom bytes are available for writing.
  Because DSA uses VLAN as a tag, dsa_user_setup_tagger() will increase
  dev->needed_headroom by VLAN_HLEN for the tag_8021q protocols, so
  vlan_insert_tag() should not fail. I've looked at this function at it
  seems not to be coded up to fail for any other reason.

I guess what you're saying is that vlan_insert_tag() will never fail in
->xmit()?

If we decide to keep things as they are we can probably make a patch
to the Sashiko ruleset and tell the AI to explicitly shut up about any use
of risking double-free when calling vlan_insert_tag() in any DSA taggers
->xmit() callback, but it seems a bit hard to maintain this way.

Otherwise, sure, it seems cleaner this way, but the way I see it, it
risks introducing more issues than it fixes. If maintainers feel
different about this please go ahead, but given the fact that I don't
really have a lot of time to do proper review during this period, I'm
more on the pragmatic side on this one.

I am a bit in love with my own patch but I'm not married to it.

I'll let Jakub & al decide on its fate, if your stance is neutral.
If things explode we'll revert it...

Yours,
Linus Walleij