* [PATCH net-next] net: sched: flower: use correct ht function to prevent duplicates
@ 2019-04-11 16:12 Vlad Buslov
2019-04-11 17:14 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Vlad Buslov @ 2019-04-11 16:12 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, lucasb, john.hurley, Vlad Buslov
Implementation of function rhashtable_insert_fast() check if its internal
helper function __rhashtable_insert_fast() returns non-NULL pointer and
seemingly return -EEXIST in such case. However, since
__rhashtable_insert_fast() is called with NULL key pointer, it never
actually checks for duplicates, which means that -EEXIST is never returned
to the user. Use rhashtable_lookup_insert_fast() hash table API instead. In
order to verify that it works as expected and prevent the problem from
happening in future, extend tc-tests with new test that verifies that no
new filters with existing key can be inserted to flower classifier.
Fixes: 1f17f7742eeb ("net: sched: flower: insert filter to ht before offloading it to hw")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/cls_flower.c | 6 +++---
.../tc-testing/tc-tests/filters/tests.json | 20 +++++++++++++++++++
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 2763176e369c..9cd8122a5c38 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1466,9 +1466,9 @@ static int fl_ht_insert_unique(struct cls_fl_filter *fnew,
struct fl_flow_mask *mask = fnew->mask;
int err;
- err = rhashtable_insert_fast(&mask->ht,
- &fnew->ht_node,
- mask->filter_ht_params);
+ err = rhashtable_lookup_insert_fast(&mask->ht,
+ &fnew->ht_node,
+ mask->filter_ht_params);
if (err) {
*in_ht = false;
/* It is okay if filter with same key exists when
diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
index 99a5ffca1088..1b6501983018 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
@@ -38,5 +38,25 @@
"$TC qdisc del dev $DEV2 ingress",
"/bin/rm $BATCH_FILE"
]
+ },
+ {
+ "id": "4cbd",
+ "name": "Try to add filter with duplicate key",
+ "category": [
+ "filter",
+ "flower"
+ ],
+ "setup": [
+ "$TC qdisc add dev $DEV2 ingress",
+ "$TC filter add dev $DEV2 protocol ip prio 1 parent ffff: flower dst_mac e4:11:22:11:4a:51 src_mac e4:11:22:11:4a:50 ip_proto tcp src_ip 1.1.1.1 dst_ip 2.2.2.2 action drop"
+ ],
+ "cmdUnderTest": "$TC filter add dev $DEV2 protocol ip prio 1 parent ffff: flower dst_mac e4:11:22:11:4a:51 src_mac e4:11:22:11:4a:50 ip_proto tcp src_ip 1.1.1.1 dst_ip 2.2.2.2 action drop",
+ "expExitCode": "2",
+ "verifyCmd": "$TC -s filter show dev $DEV2 ingress",
+ "matchPattern": "filter protocol ip pref 1 flower chain 0 handle",
+ "matchCount": "1",
+ "teardown": [
+ "$TC qdisc del dev $DEV2 ingress"
+ ]
}
]
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: sched: flower: use correct ht function to prevent duplicates
2019-04-11 16:12 [PATCH net-next] net: sched: flower: use correct ht function to prevent duplicates Vlad Buslov
@ 2019-04-11 17:14 ` Jakub Kicinski
2019-04-11 18:33 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2019-04-11 17:14 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem, lucasb, john.hurley
On Thu, 11 Apr 2019 19:12:20 +0300, Vlad Buslov wrote:
> Implementation of function rhashtable_insert_fast() check if its internal
> helper function __rhashtable_insert_fast() returns non-NULL pointer and
> seemingly return -EEXIST in such case. However, since
> __rhashtable_insert_fast() is called with NULL key pointer, it never
> actually checks for duplicates, which means that -EEXIST is never returned
> to the user. Use rhashtable_lookup_insert_fast() hash table API instead. In
> order to verify that it works as expected and prevent the problem from
> happening in future, extend tc-tests with new test that verifies that no
> new filters with existing key can be inserted to flower classifier.
>
> Fixes: 1f17f7742eeb ("net: sched: flower: insert filter to ht before offloading it to hw")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Ah, John just posted the same patch internally this morning..
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: sched: flower: use correct ht function to prevent duplicates
2019-04-11 17:14 ` Jakub Kicinski
@ 2019-04-11 18:33 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2019-04-11 18:33 UTC (permalink / raw)
To: jakub.kicinski
Cc: vladbu, netdev, jhs, xiyou.wangcong, jiri, lucasb, john.hurley
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Thu, 11 Apr 2019 10:14:15 -0700
> On Thu, 11 Apr 2019 19:12:20 +0300, Vlad Buslov wrote:
>> Implementation of function rhashtable_insert_fast() check if its internal
>> helper function __rhashtable_insert_fast() returns non-NULL pointer and
>> seemingly return -EEXIST in such case. However, since
>> __rhashtable_insert_fast() is called with NULL key pointer, it never
>> actually checks for duplicates, which means that -EEXIST is never returned
>> to the user. Use rhashtable_lookup_insert_fast() hash table API instead. In
>> order to verify that it works as expected and prevent the problem from
>> happening in future, extend tc-tests with new test that verifies that no
>> new filters with existing key can be inserted to flower classifier.
>>
>> Fixes: 1f17f7742eeb ("net: sched: flower: insert filter to ht before offloading it to hw")
>> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
>
> Ah, John just posted the same patch internally this morning..
>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Applied.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-11 18:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-11 16:12 [PATCH net-next] net: sched: flower: use correct ht function to prevent duplicates Vlad Buslov
2019-04-11 17:14 ` Jakub Kicinski
2019-04-11 18:33 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.