Currently, using ofproto/trace to trace a datapath flow with eth_type()
but without eth() may hit unexpected behavior because OVS sets
the packet_type to be (1, eth_type) when decoding the odp flow key.
This patch updates the logic of odp flow key decoding so that the
packet_type defaults to (0,0) unless user explicitly specifies the
packet_type.  An unit test is added to prevent future regression.

Travis build: https://travis-ci.org/YiHungWei/ovs/builds/351565383

Reported-by: Amar Padmanabhan <amarpadmanab...@fb.com>
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2017-December/045817.html
Reported-by: Su Wang <suw...@vmware.com>
VMWare-BZ: #2070488
Signed-off-by: Yi-Hung Wei <yihung....@gmail.com>
---
 lib/odp-util.c        |  8 --------
 tests/ofproto-dpif.at | 24 ++++++++++++++++++++++++
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 5da83b4c64c4..682f1d3bd4b5 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -6440,14 +6440,6 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t 
key_len,
         }
         expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ETHERNET;
     }
-    else if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE)) {
-        ovs_be16 ethertype = nl_attr_get_be16(attrs[OVS_KEY_ATTR_ETHERTYPE]);
-        if (!is_mask) {
-            flow->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,
-                                               ntohs(ethertype));
-        }
-        expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE;
-    }
 
     /* Get Ethertype or 802.1Q TPID or FLOW_DL_TYPE_NONE. */
     if (!parse_ethertype(attrs, present_attrs, &expected_attrs, flow,
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index d2058eddd3eb..13e57b90edd1 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -10162,3 +10162,27 @@ AT_CHECK([grep flow_del ovs-vswitchd.log], [1])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - debug ofproto/trace])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2 3 4
+AT_DATA([flows.txt], [dnl
+table=0 priority=100 in_port=1,udp actions=output:2
+table=0 priority=99 in_port=1 actions=output:3
+])
+AT_CHECK([ovs-ofctl -O OpenFlow15 add-flows br0 flows.txt])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(1),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=17,tos=0,ttl=128,frag=no),udp(src=8,dst=9)'],
 [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 2
+])
+
+AT_CHECK([ovs-ofctl -O OpenFlow15 add-flow br0 "table=0 priority=100 
in_port=1,packet_type=(1,0x800) actions=output:4"])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(1),packet_type(ns=1,id=0x800),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=17,tos=0,ttl=128,frag=no),udp(src=8,dst=9)'],
 [0], [stdout])
+AT_CHECK([cat stdout | grep output], [0],
+  [    output:4
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
-- 
2.7.4

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to