[ovs-dev] [PATCH] dpctl: Fix broken flow deletion via ovs-dpctl due to missing ufid.

Ilya Maximets i.maximets at ovn.org
Fri Oct 9 15:29:15 UTC 2020


On 10/5/20 2:04 PM, Eelco Chaudron wrote:
> 
> 
> On 5 Oct 2020, at 12:09, Ilya Maximets wrote:
> 
>> Current code generates UFID for flows installed by ovs-dpctl.  This
>> leads to inability to remove such flows by the same command.  Ex:
>>
>>   ovs-dpctl add-dp test
>>   ovs-dpctl add-if test vport0
>>   ovs-dpctl add-flow test "in_port(0),eth(),eth_type(0x800),ipv4(src=100.1.0.1)" 0
>>   ovs-dpctl del-flow test "in_port(0),eth(),eth_type(0x800),ipv4(src=100.1.0.1)"
>>
>>   dpif|WARN|system at test: failed to flow_del (No such file or directory)
>>       ufid:e4457189-3990-4a01-bdcf-1e5f8b208711 in_port(0),
>>       eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),eth_type(0x0800),
>>       ipv4(src=100.1.0.1,dst=0.0.0.0,proto=0,tos=0,ttl=0,frag=no)
>>
>>   ovs-dpctl: deleting flow (No such file or directory)
>>   Perhaps you need to specify a UFID?
>>
>> During del-flow operation UFID is generated too, however resulted
>> value is different from one generated during add-flow.  This happens
>> because odp_flow_key_hash() function uses random base value for flow
>> hashes which is different on every invocation.  That is not an issue
>> while running 'ovs-appctl dpctl/{add,del}-flow' because execution
>> of these requests happens in context of the OVS main process, i.e.
>> there will be same random seed.
>>
>> Commit e61984e781e6 was intended to allow offloading for flows
>> added by dpctl/add-flow unixctl command, so it's better to generate
>> UFIDs conditionally inside dpctl command handler only for appctl
>> invocations.  Offloading is not possible from ovs-dpctl utility anyway.
>>
>> There are still couple of corner case:  It will not be possible to
>> remove flow by 'ovs-appctl dpctl/del-flow' without specifying UFID if
>> main OVS process was restarted since flow addition and it will not
>> be possible to remove flow by ovs-dpctl without specifying UUID if
>> it was added by 'ovs-appctl dpctl/add-flow'.  But these scenarios
>> seems minor since these commands intended for testing only.
>>
>> Reported-by: Eelco Chaudron <echaudro at redhat.com>
>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-September/374863.html
>> Fixes: e61984e781e6 ("dpif-netlink: Generate ufids for installing TC flowers")
>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>> ---
> 
> This change looks good to me, with one small comment below...
> Also, verified this in my scenario, and all works as expected.
> 
> Acked-by: Eelco Chaudron <echaudro at redhat.com>
> Tested-by: Eelco Chaudron <echaudro at redhat.com>


Thanks, Eelco!

I went ahead and applied this patch to master and backported to 2.14.

Best regards, Ilya Maximets.


More information about the dev mailing list