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

From: Daniel Borkmann <[email-masked]>
Date: Tue, 16 Feb 2016 16:20:56 +0100

On 02/16/2016 01:37 PM, Jamal Hadi Salim wrote:
> From: Jamal Hadi Salim <[email-masked]>
>
> 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: pref 1 protocol 802.1Q \
> u32 match u32 0 0 flowid 1:1 \
> action vlan pop reclassify
>
> Fixes: 3b3ae880266d ("net: sched: consolidate tc_classify{,_compat}")
> Signed-off-by: Jamal Hadi Salim <[email-masked]>
> ---
>   net/sched/sch_api.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index b5c2cf2..22ab634 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1818,13 +1818,14 @@ 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;
> +#endif
> +	const struct tcf_proto *old_tp = tp;
> +	__be16 protocol;
>
>   reclassify:
> -#endif
> +	protocol = tc_skb_protocol(skb);
>   	for (; tp; tp = rcu_dereference_bh(tp->next)) {
>   		int err;
>

But, how is that better than the one-liner I suggested to you in
my earlier mail?

With this patch, I now get :

   CC      net/sched/sch_api.o
net/sched/sch_api.c: In function ‘tc_classify’:
net/sched/sch_api.c:1827:1: warning: label ‘reclassify’ defined but not used [-Wunused-label]
  reclassify:
  ^
net/sched/sch_api.c:1824:26: warning: unused variable ‘old_tp’ [-Wunused-variable]
   const struct tcf_proto *old_tp = tp;
                           ^
Thanks,
Daniel