Sorry.
The commit message and code are not changed.
Resended when I met a bug of intel-ovs-compilation test fail and add version
descriptions.
From: Simon Horman <simon.hor...@corigine.com>
Date: 2023-02-21 23:09:05
To: Faicker Mo <faicker...@ucloud.cn>
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v6] netdev-offload-tc: del ufid mapping if device
not exist>On Wed, Feb 01, 2023 at 10:49:22AM +0800, 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 <faicker...@ucloud.cn>
>> ---
>> v2:
>> - Add tc offload test case
>> v3:
>> - No change
>> v4:
>> - No change
>> v5:
>> - No change
>> v6:
>> - No change
>
>I am confused.
>Why are there 4 versions (v3 - v6) with no change?
>What does that mean?
>
>> ---
>> 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 6e1bbaa28..a4e8818ab 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_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 16a4c1a00..15ea549a6 100644
>> --- a/tests/system-offloads-traffic.at
>> +++ b/tests/system-offloads-traffic.at
>
>The test seems to pass both with and without the change to
>del_filter_and_ufid_mapping(). I think it would be better to construct a
>test that fails without the code change and succeeds with it.
>
>I ran:
>
>TESTSUITEFLAGS="-k ufid" make check-offloads
>
>> @@ -680,3 +680,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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
