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@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

Reply via email to