Re: [Patch net] net_sched: refetch skb protocol for each filter

[Cong Wang <xiyou.wangcong@gmail.com>]

On Sat, Jan 12, 2019 at 7:41 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2019-01-12 7:23 a.m., Jamal Hadi Salim wrote:
>
> > Do we have a test case for a setup like this in tdc?
> > i.e incoming tagged and then vlan popped by action?
> > Otherwise a test with IFE which resets the ethertype
> > would be sufficient i.e just something that will messup
> > with skb->protocol.
>
> And here is a slightly complex test with IFE.
> Wanted to show both reclassify and continue in play
> as well as something that change skb->protocol.
>

I don't know why you need a complex one, Martin's
test case is pretty simple (as I already sent to you).

Also, you can add two printk()'s around the skb_vlan_pop()
in tcf_vlan_act() to see the difference of tc_skb_protocol()
return values before and after. I tried, it clearly shows
ETH_P_8021Q and ETH_P_IP.

Of course, it could be tc_skb_protocol() which is wrong,
as skb->protocol stays same.

This patch is always correct despite of tc_skb_protocol():

1. If tc_skb_protocol() is wrong, this patch fixes nothing,
and harms nothing.

2. If tc_skb_protocol() is correct, this patch fixes a bug.

Changing tc_skb_protocol() is much more risky than this
patch. You fixed a very similar bug before:

commit 619fe32640b4b01f370574d50344ae0f62689816
Author: Jamal Hadi Salim <jhs@mojatatu.com>
Date:   Thu Feb 18 07:38:04 2016 -0500

    net_sched fix: reclassification needs to consider ether protocol changes

which also implies tc_skb_protocol() is correct.

Thanks.