* [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes
@ 2016-02-16 12:37 Jamal Hadi Salim
2016-02-16 15:20 ` Daniel Borkmann
0 siblings, 1 reply; 8+ messages in thread
From: Jamal Hadi Salim @ 2016-02-16 12:37 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, 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: 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 <jhs@mojatatu•com>
---
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;
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes
2016-02-16 12:37 [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes Jamal Hadi Salim
@ 2016-02-16 15:20 ` Daniel Borkmann
2016-02-17 11:19 ` Jamal Hadi Salim
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2016-02-16 15:20 UTC (permalink / raw)
To: Jamal Hadi Salim, davem; +Cc: netdev
On 02/16/2016 01:37 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: 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 <jhs@mojatatu•com>
> ---
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes
2016-02-16 15:20 ` Daniel Borkmann
@ 2016-02-17 11:19 ` Jamal Hadi Salim
2016-02-17 11:48 ` Jamal Hadi Salim
0 siblings, 1 reply; 8+ messages in thread
From: Jamal Hadi Salim @ 2016-02-17 11:19 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: netdev
On 16-02-16 10:20 AM, Daniel Borkmann wrote:
> On 02/16/2016 01:37 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: 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 <jhs@mojatatu•com>
>> ---
>> 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?
>
Thats how the original code was (and what i tested). I dont have time
to test right now but i will send an update with your suggestion.
cheers,
jamal
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes
2016-02-17 11:19 ` Jamal Hadi Salim
@ 2016-02-17 11:48 ` Jamal Hadi Salim
0 siblings, 0 replies; 8+ messages in thread
From: Jamal Hadi Salim @ 2016-02-17 11:48 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: netdev
On 16-02-17 06:19 AM, Jamal Hadi Salim wrote:
> Thats how the original code was (and what i tested). I dont have time
> to test right now but i will send an update with your suggestion.
>
My setup was still running so I managed to test it with IFE
cheers,
jamal
^ permalink raw reply [flat|nested] 8+ messages in thread
* [net-next PATCH v2 0/1] tc reclassification needs to consider ether protocol changes
@ 2016-02-17 11:37 Jamal Hadi Salim
2016-02-17 11:37 ` [net-next PATCH v2 1/1] net_sched fix: " Jamal Hadi Salim
0 siblings, 1 reply; 8+ messages in thread
From: Jamal Hadi Salim @ 2016-02-17 11:37 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu•com>
v2:
Use Daniel's suggestion on when to set the skb proto
v1:
fix compile error found by build bot when CONFIG_NET_CLS_ACT was off
Jamal Hadi Salim (1):
net_sched fix: reclassification needs to consider ether protocol
changes
net/sched/sch_api.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
1.9.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes
2016-02-17 11:37 [net-next PATCH v2 0/1] tc " Jamal Hadi Salim
@ 2016-02-17 11:37 ` Jamal Hadi Salim
2016-02-17 17:27 ` Cong Wang
0 siblings, 1 reply; 8+ messages in thread
From: Jamal Hadi Salim @ 2016-02-17 11:37 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, 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: 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 <jhs@mojatatu•com>
---
net/sched/sch_api.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b5c2cf2..af46fee 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1822,7 +1822,6 @@ int tc_classify(struct sk_buff *skb, const struct tcf_proto *tp,
#ifdef CONFIG_NET_CLS_ACT
const struct tcf_proto *old_tp = tp;
int limit = 0;
-
reclassify:
#endif
for (; tp; tp = rcu_dereference_bh(tp->next)) {
@@ -1852,6 +1851,7 @@ reset:
}
tp = old_tp;
+ protocol = tc_skb_protocol(skb);
goto reclassify;
#endif
}
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes
2016-02-17 11:37 ` [net-next PATCH v2 1/1] net_sched fix: " Jamal Hadi Salim
@ 2016-02-17 17:27 ` Cong Wang
2016-02-18 11:53 ` Daniel Borkmann
0 siblings, 1 reply; 8+ messages in thread
From: Cong Wang @ 2016-02-17 17:27 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, Daniel Borkmann, Linux Kernel Network Developers
On Wed, Feb 17, 2016 at 3:37 AM, Jamal Hadi Salim <jhs@mojatatu•com> 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: 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 <jhs@mojatatu•com>
> ---
> net/sched/sch_api.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index b5c2cf2..af46fee 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1822,7 +1822,6 @@ int tc_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> #ifdef CONFIG_NET_CLS_ACT
> const struct tcf_proto *old_tp = tp;
> int limit = 0;
> -
Why remove this empty line? I think it is still useful. :)
Also your patch should be targeted to -net instead of -net-next, since it
fixes a bug.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes
2016-02-17 17:27 ` Cong Wang
@ 2016-02-18 11:53 ` Daniel Borkmann
2016-02-18 12:26 ` Jamal Hadi Salim
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2016-02-18 11:53 UTC (permalink / raw)
To: Cong Wang, Jamal Hadi Salim; +Cc: David Miller, Linux Kernel Network Developers
On 02/17/2016 06:27 PM, Cong Wang wrote:
> On Wed, Feb 17, 2016 at 3:37 AM, Jamal Hadi Salim <jhs@mojatatu•com> 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: 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 <jhs@mojatatu•com>
>> ---
>> net/sched/sch_api.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
>> index b5c2cf2..af46fee 100644
>> --- a/net/sched/sch_api.c
>> +++ b/net/sched/sch_api.c
>> @@ -1822,7 +1822,6 @@ int tc_classify(struct sk_buff *skb, const struct tcf_proto *tp,
>> #ifdef CONFIG_NET_CLS_ACT
>> const struct tcf_proto *old_tp = tp;
>> int limit = 0;
>> -
>
> Why remove this empty line? I think it is still useful. :)
>
> Also your patch should be targeted to -net instead of -net-next, since it
> fixes a bug.
Yeah, good point. That should go to -net (minus the unrelated white space change).
Thanks,
Daniel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes
2016-02-18 11:53 ` Daniel Borkmann
@ 2016-02-18 12:26 ` Jamal Hadi Salim
0 siblings, 0 replies; 8+ messages in thread
From: Jamal Hadi Salim @ 2016-02-18 12:26 UTC (permalink / raw)
To: Daniel Borkmann, Cong Wang; +Cc: David Miller, Linux Kernel Network Developers
On 16-02-18 06:53 AM, Daniel Borkmann wrote:
> On 02/17/2016 06:27 PM, Cong Wang wrote:
>>> const struct tcf_proto *old_tp = tp;
>>> int limit = 0;
>>> -
>>
>> Why remove this empty line? I think it is still useful. :)
>>
>> Also your patch should be targeted to -net instead of -net-next, since it
>> fixes a bug.
>
> Yeah, good point. That should go to -net (minus the unrelated white
> space change).
>
One patch coming right up boys;->
cheers,
jamal
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-02-18 12:26 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-16 12:37 [net-next PATCH v2 1/1] net_sched fix: reclassification needs to consider ether protocol changes Jamal Hadi Salim
2016-02-16 15:20 ` Daniel Borkmann
2016-02-17 11:19 ` Jamal Hadi Salim
2016-02-17 11:48 ` Jamal Hadi Salim
-- strict thread matches above, loose matches on Subject: below --
2016-02-17 11:37 [net-next PATCH v2 0/1] tc " Jamal Hadi Salim
2016-02-17 11:37 ` [net-next PATCH v2 1/1] net_sched fix: " Jamal Hadi Salim
2016-02-17 17:27 ` Cong Wang
2016-02-18 11:53 ` Daniel Borkmann
2016-02-18 12:26 ` Jamal Hadi Salim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox