Re: [PATCH net-next 0/5] tc: adjust network header after second vlan push

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 14 Aug 2024 16:07:14 +0300 Boris Sukholitko wrote:
> > The series is structured quite nicely for review, so kudos for that.
> > But I'm not seeing the motivation for changing how TC pushes VLANs
> > and not changing OvS (or BPF?), IOW the other callers of
> > skb_vlan_push().
> > 
> > Why would pushing a tag from TC actions behave differently?  
> 
> IMHO, the difference between TC and OvS and BPF is that in the TC case
> the dissector is invoked on the wrong position in the packet (IP vs L2
> header). We can regard reading garbage from there as a bug.
> 
> I am not sure that this is the case in OvS or BPF. E.g. in the BPF
> case there may some script expecting the skb to point to an IP header
> after second vlan push. My change will break it.

The packet either has correct format or it doesn't. You could easily
construct a TC ruleset which pushes the VLAN using act_bpf, instead of
act_vlan.

Let's not be too conservative, worrying about very unlikely
regressions, IMHO. Such divergence makes the code base much harder 
to maintainer.

> > Please also add your test case to
> > tools/testing/selftests/net/forwarding/tc_actions.sh
> > if you can.  
> 
> Done in v2.

Please do not respond to a discussion and immediate send the next
version.