[net-next PATCH 1/1] net_sched fix: reclassification needs to consider ether protocol changes

[net-next PATCH 1/1] net_sched fix: reclassification needs to consider ether protocol changes

[Jamal Hadi Salim]

From: Jamal Hadi Salim <jhs@mojatatu.com>

actions could change the etherproto in particular with ethernet
tunnelled data. Typically such actions, after peeling the outer header,
will ask for the packet to be  reclassified. We then need to restart
the classification with the new proto header.

Example setup used to catch this:
sudo tc qdisc add dev $ETH ingress
sudo tc filter add dev $ETH parent ffff: prio 2 protocol 0xbeef \
u32 match u32 0 0 flowid 1:1 \
action ife decode reclassify

Fixes: 3b3ae880266d ("net: sched: consolidate tc_classify{,_compat}")
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_api.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b5c2cf2..07cfb54 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1818,12 +1818,13 @@ done:
 int tc_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 		struct tcf_result *res, bool compat_mode)
 {
-	__be16 protocol = tc_skb_protocol(skb);
 #ifdef CONFIG_NET_CLS_ACT
 	const struct tcf_proto *old_tp = tp;
 	int limit = 0;
+	__be16 protocol;
 
 reclassify:
+	protocol = tc_skb_protocol(skb);
 #endif
 	for (; tp; tp = rcu_dereference_bh(tp->next)) {
 		int err;
-- 
1.9.1

Re: [net-next PATCH 1/1] net_sched fix: reclassification needs to consider ether protocol changes

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1798 bytes --]

Hi Jamal,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jamal-Hadi-Salim/net_sched-fix-reclassification-needs-to-consider-ether-protocol-changes/20160216-035147
config: i386-randconfig-x008-201607 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   net/sched/sch_api.c: In function 'tc_classify':
>> net/sched/sch_api.c:1832:23: error: 'protocol' undeclared (first use in this function)
      if (tp->protocol != protocol &&
                          ^
   net/sched/sch_api.c:1832:23: note: each undeclared identifier is reported only once for each function it appears in

vim +/protocol +1832 net/sched/sch_api.c

3b3ae880 Daniel Borkmann  2015-08-26  1826  reclassify:
bd4d9963 Jamal Hadi Salim 2016-02-15  1827  	protocol = tc_skb_protocol(skb);
3b3ae880 Daniel Borkmann  2015-08-26  1828  #endif
25d8c0d5 John Fastabend   2014-09-12  1829  	for (; tp; tp = rcu_dereference_bh(tp->next)) {
3b3ae880 Daniel Borkmann  2015-08-26  1830  		int err;
3b3ae880 Daniel Borkmann  2015-08-26  1831  
cc7ec456 Eric Dumazet     2011-01-19 @1832  		if (tp->protocol != protocol &&
cc7ec456 Eric Dumazet     2011-01-19  1833  		    tp->protocol != htons(ETH_P_ALL))
cc7ec456 Eric Dumazet     2011-01-19  1834  			continue;
cc7ec456 Eric Dumazet     2011-01-19  1835  

:::::: The code at line 1832 was first introduced by commit
:::::: cc7ec456f82da7f89a5b376e613b3ac4311b3e9a net_sched: cleanups

:::::: TO: Eric Dumazet <eric.dumazet@gmail.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22583 bytes --]

Re: [net-next PATCH 1/1] net_sched fix: reclassification needs to consider ether protocol changes

[Daniel Borkmann]

On 02/15/2016 08:49 PM, Jamal Hadi Salim wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> actions could change the etherproto in particular with ethernet
> tunnelled data. Typically such actions, after peeling the outer header,
> will ask for the packet to be  reclassified. We then need to restart
> the classification with the new proto header.
>
> Example setup used to catch this:
> sudo tc qdisc add dev $ETH ingress
> sudo tc filter add dev $ETH parent ffff: prio 2 protocol 0xbeef \
> u32 match u32 0 0 flowid 1:1 \
> action ife decode reclassify

ife action is out of tree, but I believe this should be possible with
vlan action and using reclassify as tc opcode.

> Fixes: 3b3ae880266d ("net: sched: consolidate tc_classify{,_compat}")
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Has kbuild bot issues, I'd probably just move this under 'reset' label,
like:

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b5c2cf2..af1acf0 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1852,6 +1852,7 @@ reset:
  	}

  	tp = old_tp;
+	protocol = tc_skb_protocol(skb);
  	goto reclassify;
  #endif
  }

Thanks,
Daniel

Re: [net-next PATCH 1/1] net_sched fix: reclassification needs to consider ether protocol changes

[Jamal Hadi Salim]

On 16-02-15 03:59 PM, Daniel Borkmann wrote:
> On 02/15/2016 08:49 PM, Jamal Hadi Salim wrote:
>> From: Jamal Hadi Salim <jhs@mojatatu.com>
>>
>> actions could change the etherproto in particular with ethernet
>> tunnelled data. Typically such actions, after peeling the outer header,
>> will ask for the packet to be  reclassified. We then need to restart
>> the classification with the new proto header.
>>
>> Example setup used to catch this:
>> sudo tc qdisc add dev $ETH ingress
>> sudo tc filter add dev $ETH parent ffff: prio 2 protocol 0xbeef \
>> u32 match u32 0 0 flowid 1:1 \
>> action ife decode reclassify
>
> ife action is out of tree, but I believe this should be possible with
> vlan action and using reclassify as tc opcode.
>

Ok, let me do some test with vlan example.(Note: Caught this when i was
trying to test the IFE code for submission on the flight home).

>> Fixes: 3b3ae880266d ("net: sched: consolidate tc_classify{,_compat}")
>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Has kbuild bot issues, I'd probably just move this under 'reset' label,
> like:
>
 >
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index b5c2cf2..af1acf0 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1852,6 +1852,7 @@ reset:
>       }
>
>       tp = old_tp;
> +    protocol = tc_skb_protocol(skb);
>       goto reclassify;
>   #endif
>   }


The polite bot is confused in its report. I had to look at .config to
figure it out. I guess this is the point where human intervention is
needed. Basically the problem is it turned off CONFIG_NET_CLS_ACT
and recompiled; for a second i thought it was i386 specific.
I will juggle things around to make it happy.

cheers,
jamal


> Thanks,
> Daniel