Hi Ben.
Thanks for your report and the really useful script.
I spent a few hours debugging the issue and came up with the following patch:
https://patchwork.ozlabs.org/patch/981408/
Please, take a look.
Additionally we may add ufid lookup support in flow_put_on_pmd()
as a separate fix. This, probably, will speed up the operation.
Will wait for your opinion.
Best regards, Ilya Maximets.
> Hi Ian (or others), do you have any thoughts about this?
>
> On Fri, Sep 28, 2018 at 04:24:33PM -0700, Ben Pfaff wrote:
>> I have been playing with the OFTest framework off and on over the last
>> few months and I believe it's caught an actual bug with its
>> DirectVlanPackets test. I'm appending a script that demonstrates it.
>> To see it, run "make sandbox" then execute the script inside it.
>>
>> At some level at least, the problem is in dpif-netdev, which during the
>> test logs messages like this indicating that a datapath flow can't be
>> found and therefore can't be modified:
>>
>> WARN|dummy at ovs-dummy: failed to put[modify] (No such file or
>> directory) ufid:b2fe59af-26bd-4aa0-960e-fd559dc10c54
>>
>> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(1),packet_type(ns=0,id=0),eth(src=00:06:07:08:09:0a,dst=00:01:02:03:04:05),eth_type(0x8100),vlan(vid=1000/0x0,pcp=5/0x0),encap(eth_type(0x0800),ipv4(src=127.0.0.1/0.0.0.0,dst=127.0.0.1/0.0.0.0,proto=0/0,tos=0/0,ttl=64/0,frag=no)),
>>
>> actions:userspace(pid=0,controller(reason=7,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535))
>>
>> It's possible that the caller is screwing up by providing a flow match
>> different from one in the datapath flow table. I have not looked at
>> that question. But the really strange thing here is that the caller is
>> providing a ufid and dpif-netdev is ignoring it. When I make it honor
>> the ufid, like the following, then the test suddenly starts working.
>> This seems like a bug to me.
>>
>> (The following patch is not actually the real fix because the code
>> should still look at the key if no ufid was provided.)
>>
>> Any thoughts?
>>
>> Thanks,
>>
>> Ben.
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index e322f5553fa8..4855cc08f7b7 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -3273,7 +3273,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
>>
>> static int
>> flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
>> - struct netdev_flow_key *key,
>> + struct netdev_flow_key *key OVS_UNUSED,
>> struct match *match,
>> ovs_u128 *ufid,
>> const struct dpif_flow_put *put,
>> @@ -3287,7 +3287,7 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
>> }
>>
>> ovs_mutex_lock(&pmd->flow_mutex);
>> - netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
>> + netdev_flow = dp_netdev_pmd_find_flow(pmd, ufid, NULL, 0);
>> if (!netdev_flow) {
>> if (put->flags & DPIF_FP_CREATE) {
>> if (cmap_count(&pmd->flow_table) < MAX_FLOWS) {
>>
>>
>>
>> --8<--------------------------cut here-------------------------->8--
>>
>> #! /bin/sh -ex
>>
>> # Create a bridge and add port p1 on port 1, p2 on port 2
>> ovs-appctl vlog/set netdev_dummy dpif
>> ovs-vsctl --if-exists del-br br0 \
>> -- add-br br0 \
>> -- add-port br0 p1 -- set interface p1 ofport_request=1
>> options:tx_pcap=p1-tx.pcap options:rxq_pcap=p1-rx.pcap \
>> -- add-port br0 p2 -- set interface p2 ofport_request=2
>> options:tx_pcap=p2-tx.pcap options:rxq_pcap=p2-rx.pcap
>>
>> # Add a flow that directs some packets received on p1 to p2 and the
>> # rest back out p1.
>> #
>> # Then inject a packet of the form that should go to p2. Dump the
>> # datapath flow to see that it goes to p2 ("actions:2"). Trace the
>> # same packet for good measure, to also see that it should go to p2.
>> packet=00010203040500060708090a8100a3e80800450000140001000040007ce77f0000017f000001
>> ovs-ofctl add-flow br0
>> priority=1,ip,in_port=1,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,actions=output:2
>> ovs-ofctl add-flow br0 priority=0,in_port=1,actions=IN_PORT
>> ovs-appctl netdev-dummy/receive p1 $packet
>> ovs-appctl ofproto/trace br0 in_port=p1 $packet
>> ovs-appctl dpif/dump-flows br0
>>
>> # Delete the flows, then add new flows that would not match the same
>> # packet as before.
>> ovs-ofctl del-flows br0
>> ovs-ofctl add-flow br0
>> priority=1,in_port=1,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,dl_type=0x0801,actions=output:2
>> ovs-ofctl add-flow br0 priority=0,in_port=1,actions=IN_PORT
>>
>> # Give the datapath a few seconds to revalidate.
>> sleep 2
>>
>> # Now inject the same packet again. It should go to p1 because it
>> # only matches the priority-0 flow, as shown by the trace. However, instead,
>> # the original datapath flow is still there and its counters get incremented.
>> ovs-appctl netdev-dummy/receive p1 $packet
>> ovs-appctl ofproto/trace br0 in_port=p1 $packet
>> ovs-appctl dpif/dump-flows br0
>>
>> # Give the datapath some time to expire the flow.
>> sleep 12
>>
>> # Try again.
>> ovs-appctl netdev-dummy/receive p1 $packet
>> ovs-appctl ofproto/trace br0 in_port=p1 $packet
>> ovs-appctl dpif/dump-flows br0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
