On 2 Mar 2023, at 8:57, 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]>
> ---
> v2:
> - Add tc offload test case
> v3:
> - No change
> v4:
> - No change
> v5:
> - No change
> v6:
> - No change
> v7:
> - Minor fix for test case and rebased
> v8:
> - Shorten the time of the test case
> ---
> lib/netdev-offload-tc.c | 3 +-
> tests/system-offloads-traffic.at | 54 ++++++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+), 1 deletion(-)
>
>
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 4fb9d9f21..dd2020cad 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -276,8 +276,9 @@ del_filter_and_ufid_mapping(struct tcf_id *id, const
> ovs_u128 *ufid,
> }
>
> err = tc_del_flower_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 7558812eb..ba18711cb 100644
> --- a/tests/system-offloads-traffic.at
> +++ b/tests/system-offloads-traffic.at
> @@ -690,3 +690,57 @@
> 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)
> +
> +dnl Disable IPv6 to skip unexpected 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
> +
> +dnl 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 1
> +AT_CHECK([ovs-appctl revalidator/purge])
> +
> +dnl Generate 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 1
> +AT_CHECK([ovs-appctl revalidator/purge])
> +
> +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
> +/on nonexistent port/d
> +/failed to flow_get/d
> +/Failed to acquire udpif_key/d
> +"])
> +AT_CLEANUP
The changes look fine, however, you still have a 3-second delay in your tests.
Any specific reason for this?
I tried removing them and I found that if I did I needed to add another log
messages exclusion. With this I was able to still make it fail without your
change, and get it to pass for 400 runs.
Thought?
//Eelco
diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index ba18711cb..b2a402dec 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -710,7 +710,7 @@ 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
+#sleep 1
dnl Delete and add interface ovs-p0/p0
AT_CHECK([ip link del dev ovs-p0])
@@ -721,7 +721,7 @@ 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 1
+#sleep 1
AT_CHECK([ovs-appctl revalidator/purge])
dnl Generate flows to trigger the hmap expand once
@@ -733,7 +733,7 @@ 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 1
+#sleep 1
AT_CHECK([ovs-appctl revalidator/purge])
AT_CHECK([test $(ovs-appctl dpctl/dump-flows | grep -c "eth_type(0x0800)") -eq
0], [0], [ignore])
@@ -742,5 +742,6 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network device
ovs-p0/d
/on nonexistent port/d
/failed to flow_get/d
/Failed to acquire udpif_key/d
+/failed to get ifindex for ovs-p0: No such device/d
"])
AT_CLEANUP
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
