On 2019-01-13 4:08 p.m., Cong Wang wrote:
On Sat, Jan 12, 2019 at 7:41 AM Jamal Hadi Salim <j...@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).
Martin example had a "reclassify" with the vlan pop.
Your example was showing a "continue"
Note: "Reclassify" is typical how we have explained
to users to deal with skb->protocol changes.
I wanted to demonstrate that if i have a rule that followed that
rule (as Martin did) with something other than vlan it would work:
match u32 protocol someethertype .... action change vl reclassify
match u32 protocol ip ... action continue
match u32 protocol ip ... action blah
infact that works before your patch. Basically it was an exercise
to reduce _the variables_ from knowing that reclassify/continue
do/nt work in presence of changing skb->protocol.
IMO that example demonstrated that the current skb->protocol
logic works and the vlan action maybe the guilty party.
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 <j...@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.
I agree with your rationale.
Lets have Lucas give this a run and we can move
forward.
cheers,
jamal
