>From b8ec4ac672e7ef3370d52bae66db7eef7b8d9da6 Mon Sep 17 00:00:00 2001From: >Faicker Mo <[email protected]> Date: Thu, 12 Jan 2023 11:55:37 +0800 Subject: [PATCH] netdev-offload-tc: del ufid mapping if device not exist.
The device may be deleted and added with ifindex changed. The tc rules on the device will be deleted if the device is deleted. The func tc_del_filter will fail when flow del. The mapping of ufid to tc will not be deleted. The traffic will trigger the same flow(with same ufid) to put to tc on the new device. Duplicated ufid mapping will be added. If the hashmap is expanded, the old mapping entry will be the first entry, and now the dp flow can't be deleted. Signed-off-by: Faicker Mo <[email protected]> --- lib/netdev-offload-tc.c | 3 ++- tests/system-offloads-traffic.at | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index ce7f8ad97..e731badc0 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -240,8 +240,9 @@ del_filter_and_ufid_mapping(struct tcf_id *id, const ovs_u128 *ufid) int err; err = tc_del_filter(id); - if (!err) { + if (!err || err == ENODEV) { del_ufid_tc_mapping(ufid); + return 0; } return err; } diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index 1a6057080..0e9d6ad15 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -678,3 +678,48 @@ OVS_CHECK_ACTIONS([check_pkt_len(size=200,gt(5),le(check_pkt_len(size=100,gt(5), OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([offloads - delete ufid mapping if device not exist - offloads enabled]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +ADD_NAMESPACES(at_ns0, at_ns1, at_ns2) +# avoid unexpected ipv6 flow +AT_CHECK([sysctl -w net.ipv6.conf.br0.disable_ipv6=1], [0], [ignore]) +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], [ignore]) +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], [ignore]) +NS_CHECK_EXEC([at_ns2], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], [ignore]) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "aa:1a:54:e9:c5:56") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +NS_CHECK_EXEC([at_ns0], [ping -q -c 2 -i 0.2 10.1.1.2 | FORMAT_PING], [0], [dnl +2 packets transmitted, 2 received, 0% packet loss, time 0ms +]) +sleep 1 + +#delete and add interface ovs-p0/p0 +AT_CHECK([ip link del dev ovs-p0]) +AT_CHECK([ip link add p0 type veth peer name ovs-p0 || return 77]) +AT_CHECK([ip link set p0 netns at_ns0]) +AT_CHECK([ip link set dev ovs-p0 up]) +NS_CHECK_EXEC([at_ns0], [ip addr add dev p0 "10.1.1.1/24"]) +NS_CHECK_EXEC([at_ns0], [ip link set dev p0 up]) +NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address "aa:1a:54:e9:c5:56"]) + +sleep 12 +#flows to trigger the hmap expand once +ADD_VETH(p2, at_ns2, br0, "10.1.1.3/24") +NS_CHECK_EXEC([at_ns0], [ping -q -c 2 -i 0.2 10.1.1.2 | FORMAT_PING], [0], [dnl +2 packets transmitted, 2 received, 0% packet loss, time 0ms +]) +NS_CHECK_EXEC([at_ns0], [ping -q -c 2 -i 0.2 10.1.1.3 | FORMAT_PING], [0], [dnl +2 packets transmitted, 2 received, 0% packet loss, time 0ms +]) + +sleep 12 +AT_CHECK([test $(ovs-appctl dpctl/dump-flows | grep -c "eth_type(0x0800)") -eq 0], [0], [ignore]) + +OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network device ovs-p0/d"]) +AT_CLEANUP -- 2.31.1 From: Eelco Chaudron <[email protected]> Date: 2023-01-17 20:09:07 To: Faicker Mo <[email protected]> Cc: [email protected] Subject: Re: [ovs-dev] [PATCH] netdev-offload-tc: del ufid mapping if device not exist> > >On 12 Jan 2023, at 7:55, Faicker Mo wrote: > >> The device may be deleted and added with ifindex changed. >> The tc rules on the device will be deleted if the device is deleted. >> The func tc_del_filter will fail when flow del. The mapping of >> ufid to tc will not be deleted. >> The traffic will trigger the same flow(with same ufid) to put to tc >> on the new device. Duplicated ufid mapping will be added. >> If the hashmap is expanded, the old mapping entry will be the first entry, >> and now the dp flow can't be deleted. >> >> Signed-off-by: Faicker Mo <[email protected]> >> --- >> lib/netdev-offload-tc.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c >> index ce7f8ad97..e731badc0 100644 >> --- a/lib/netdev-offload-tc.c >> +++ b/lib/netdev-offload-tc.c >> @@ -240,8 +240,9 @@ del_filter_and_ufid_mapping(struct tcf_id *id, const >> ovs_u128 *ufid) >> int err; >> >> err = tc_del_filter(id); >> - if (!err) { >> + if (!err || err == ENODEV) { >> del_ufid_tc_mapping(ufid); >> + return 0; >> } >> return err; >> } >> -- > >The change by itself looks ok, but can you also add a test case for this in >system-offloads-traffic.at? > >//Eelco > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
