[patch net] sched: cls_flower: remove from hashtable only in case skip sw flag is not set

[patch net] sched: cls_flower: remove from hashtable only in case skip sw flag is not set

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Be symmetric to hashtable insert and remove filter from hashtable only
in case skip sw flag is not set.

Fixes: e69985c67c33 ("net/sched: cls_flower: Introduce support in SKIP SW flag")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_flower.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index f6f40fb..641c44c 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -711,8 +711,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 		goto errout;
 
 	if (fold) {
-		rhashtable_remove_fast(&head->ht, &fold->ht_node,
-				       head->ht_params);
+		if (!tc_skip_sw(fold->flags))
+			rhashtable_remove_fast(&head->ht, &fold->ht_node,
+					       head->ht_params);
 		fl_hw_destroy_filter(tp, (unsigned long)fold);
 	}
 
@@ -739,8 +740,9 @@ static int fl_delete(struct tcf_proto *tp, unsigned long arg)
 	struct cls_fl_head *head = rtnl_dereference(tp->root);
 	struct cls_fl_filter *f = (struct cls_fl_filter *) arg;
 
-	rhashtable_remove_fast(&head->ht, &f->ht_node,
-			       head->ht_params);
+	if (!tc_skip_sw(f->flags))
+		rhashtable_remove_fast(&head->ht, &f->ht_node,
+				       head->ht_params);
 	list_del_rcu(&f->list);
 	fl_hw_destroy_filter(tp, (unsigned long)f);
 	tcf_unbind_filter(tp, &f->res);
-- 
2.7.4

Re: [patch net] sched: cls_flower: remove from hashtable only in case skip sw flag is not set

[Amir Vadai"]

On Mon, Nov 28, 2016 at 03:40:13PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Be symmetric to hashtable insert and remove filter from hashtable only
> in case skip sw flag is not set.
> 
> Fixes: e69985c67c33 ("net/sched: cls_flower: Introduce support in SKIP SW flag")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
Reviewed-by: Amir Vadai <amir@vadai.me>

Re: [patch net] sched: cls_flower: remove from hashtable only in case skip sw flag is not set

[Or Gerlitz]

On Mon, Nov 28, 2016 at 4:40 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Be symmetric to hashtable insert and remove filter from hashtable only
> in case skip sw flag is not set.
>
> Fixes: e69985c67c33 ("net/sched: cls_flower: Introduce support in SKIP SW flag")

Amir, Jiri - what was the impact of running without this fix for the
last 3-4 kernels? I haven't seen any crashes, is that leaking took
place? or this is just a cleanup to make things more clear and
maintainable?

Re: [patch net] sched: cls_flower: remove from hashtable only in case skip sw flag is not set

[Jiri Pirko]

Mon, Nov 28, 2016 at 10:04:56PM CET, gerlitz.or@gmail.com wrote:
>On Mon, Nov 28, 2016 at 4:40 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Be symmetric to hashtable insert and remove filter from hashtable only
>> in case skip sw flag is not set.
>>
>> Fixes: e69985c67c33 ("net/sched: cls_flower: Introduce support in SKIP SW flag")
>
>Amir, Jiri - what was the impact of running without this fix for the
>last 3-4 kernels? I haven't seen any crashes, is that leaking took
>place? or this is just a cleanup to make things more clear and
>maintainable?

It's a fix for real bug. If you add rule with skip_sw flag, it is not
inserted into hashtable. But once you remove it, the current code
removes it from hashtable (did not inspect how rhashtable implementation
handles this).

Re: [patch net] sched: cls_flower: remove from hashtable only in case skip sw flag is not set

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 28 Nov 2016 15:40:13 +0100

> From: Jiri Pirko <jiri@mellanox.com>
> 
> Be symmetric to hashtable insert and remove filter from hashtable only
> in case skip sw flag is not set.
> 
> Fixes: e69985c67c33 ("net/sched: cls_flower: Introduce support in SKIP SW flag")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Applied, thanks Jiri.