[ovs-dev] [PATCH v6 1/5] userspace: Switching of L3 packets in L2 pipeline

Ben Pfaff blp at ovn.org
Fri May 19 04:08:46 UTC 2017


On Fri, May 12, 2017 at 11:07:38AM +0000, Zoltán Balogh wrote:
> From: Jan Scheurich <jan.scheurich at ericsson.com>
> 
> Ports have a new layer3 attribute if they send/receive L3 packets.
> 
> The packet_type included in structs dp_packet and flow is considered in
> ofproto-dpif. The classical L2 match fields (dl_src, dl_dst, dl_type, and
> vlan_tci, vlan_vid, vlan_pcp) now have Ethernet as pre-requisite.
> 
> A dummy ethernet header is pushed to L3 packets received from L3 ports
> before the the pipeline processing starts. The ethernet header is popped
> before sending a packet to a L3 port.
> 
> For datapath ports that can receive L2 or L3 packets, the packet_type
> becomes part of the flow key for datapath flows and is handled
> appropriately in dpif-netdev.
> 
> In the 'else' branch in flow_put_on_pmd() function, the additional check
> flow_equal(&match.flow, &netdev_flow->flow) was removed, as a) the dpcls
> lookup is sufficient to uniquely identify a flow and b) it caused false
> negatives because the flow in netdev->flow may not properly masked.
> 
> In dpif_netdev_flow_put() we now use the same method for constructing the
> netdev_flow_key as the one used when adding the flow to the dplcs to make sure
> these always match. The function netdev_flow_key_from_flow() used so far was
> not only inefficient but sometimes caused mismatches and subsequent flow
> update failures.
> 
> Signed-off-by: Lorand Jakab <lojakab at cisco.com>
> Signed-off-by: Simon Horman <simon.horman at netronome.com>
> Signed-off-by: Jiri Benc <jbenc at redhat.com>
> Signed-off-by: Yi Yang <yi.y.yang at intel.com>
> Signed-off-by: Jan Scheurich <jan.scheurich at ericsson.com>
> Co-authored-by: Zoltan Balogh <zoltan.balogh at ericsson.com>

Hi Zoltan, thanks for sending v6.

I suggest folding in the incremental appended to this message.  It
changes "packet_type=(0,0x0)" into just "packet_type=(0,0)", which looks
more natural to me, and corrects a few minor style points.

More importantly, I'm getting a test failure.  The relevant bits are:

    785. tunnel-push-pop.at:229: testing tunnel_push_pop - underlay bridge match ...
...
    ../../tests/tunnel-push-pop.at:264: ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=17,tos=0,ttl=64,frag=no),udp(src=51283,dst=4789)'
    stdout:
    Flow: udp,in_port=LOCAL,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,nw_src=1.1.3.88,nw_dst=1.1.3.112,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=51283,tp_dst=4789

    bridge("int-br")
    ----------------
     0. priority 32768
        output:3
         -> output to native tunnel
         -> tunneling to 1.1.2.92 via br0
         -> tunneling from aa:55:aa:55:00:00 1.1.2.88 to f8:bc:12:44:34:b6 1.1.2.92

    Final flow: udp,in_port=LOCAL,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,nw_src=1.1.3.88,nw_dst=1.1.3.112,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=51283,tp_dst=4789
    Megaflow: recirc_id=0,ip,in_port=LOCAL,nw_ecn=0,nw_frag=no
    Datapath actions: push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100))
    ../../tests/tunnel-push-pop.at:265: tail -1 stdout
    --- -   2017-05-18 20:54:02.253568043 -0700
    +++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/785/stdout        2017-05-18 20:54:02.249243540 -0700
    @@ -1,2 +1,2 @@
    -Datapath actions: tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100))
    +Datapath actions: push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100))


--8<--------------------------cut here-------------------------->8--

diff --git a/lib/match.c b/lib/match.c
index 4855c74a4401..6b7bb9bd476a 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1258,11 +1258,11 @@ match_format(const struct match *match, struct ds *s, int priority)
             ds_put_format(s, "packet_type=(%u,*),",
                           pt_ns(f->packet_type));
         } else if (pt_ns_type_be(wc->masks.packet_type) == OVS_BE16_MAX) {
-            ds_put_format(s, "packet_type=(%u,0x%"PRIx16"),",
+            ds_put_format(s, "packet_type=(%u,%#"PRIx16"),",
                           pt_ns(f->packet_type),
                           pt_ns_type(f->packet_type));
-        } else{
-            ds_put_format(s, "packet_type=(%u,0x%"PRIx16"/0x%"PRIx16"),",
+        } else {
+            ds_put_format(s, "packet_type=(%u,%#"PRIx16"/%#"PRIx16"),",
                           pt_ns(f->packet_type),
                           pt_ns_type(f->packet_type),
                           pt_ns_type(wc->masks.packet_type));
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 4d07d1c2a03b..4780d285397e 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -2965,11 +2965,11 @@ format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma,
             if (mask == 0) {
                 ds_put_format(ds, "ns=%u,id=*", ns);
             } else {
-                ds_put_format(ds, "ns=%u,id=0x%"PRIx16"/0x%"PRIx16,
+                ds_put_format(ds, "ns=%u,id=%#"PRIx16"/%#"PRIx16,
                               ns, ns_type, mask_ns_type);
             }
         } else {
-            ds_put_format(ds, "ns=%u,id=0x%"PRIx16, ns, ns_type);
+            ds_put_format(ds, "ns=%u,id=%#"PRIx16, ns, ns_type);
         }
         break;
     }
@@ -4855,13 +4855,13 @@ odp_key_to_dp_packet(const struct nlattr *key, size_t key_len,
         }
     }
 
-    if (packet_type == htonl(PT_ETH)){
+    if (packet_type == htonl(PT_ETH)) {
         packet->packet_type = htonl(PT_ETH);
     } else if (packet_type == htonl(PT_UNKNOWN) && ethertype != 0) {
         packet->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,
                                              ntohs(ethertype));
     } else {
-        VLOG_ERR_RL(&rl, "Packet without ETHERTYPE. Unknown packet_type.\n");
+        VLOG_ERR_RL(&rl, "Packet without ETHERTYPE. Unknown packet_type.");
     }
 }
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7a9c58cf8922..d3e8687e1aa4 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3320,12 +3320,11 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         }
     }
 
-    if (flow->packet_type == htonl(PT_ETH) && xport->is_layer3 ) {
+    if (flow->packet_type == htonl(PT_ETH) && xport->is_layer3) {
         /* Ethernet packet to L3 outport -> pop ethernet header. */
         flow->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,
                                            ntohs(flow->dl_type));
-    }
-    else if (flow->packet_type != htonl(PT_ETH) && !xport->is_layer3) {
+    } else if (flow->packet_type != htonl(PT_ETH) && !xport->is_layer3) {
         /* L2 outport and non-ethernet packet_type -> add dummy eth header. */
         flow->packet_type = htonl(PT_ETH);
         flow->dl_dst = eth_addr_zero;
@@ -6371,7 +6370,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
                                          ctx.base_flow.in_port.ofp_port);
 
     if (flow->packet_type != htonl(PT_ETH) && in_port && in_port->is_layer3 &&
-            ctx.table_id == 0) {
+        ctx.table_id == 0) {
         /* Add dummy Ethernet header to non-L2 packet if it's coming from a
          * L3 port. So all packets will be L2 packets for lookup.
          * The dl_type has already been set from the packet_type. */
diff --git a/tests/tunnel.at b/tests/tunnel.at
index a887d309b96b..222fb8d1a603 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -82,28 +82,28 @@ AT_CHECK([ovs-appctl dpif/show | tail -n +3], [0], [dnl
 dnl Tunnel CE and encapsulated packet CE
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(src=1.1.1.1,dst=2.2.2.2,tos=0x3,ttl=64,flags()),in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=3,ttl=64,frag=no),tcp(src=8,dst=9)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=3,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=3,nw_frag=no
 Datapath actions: 2
 ])
 
 dnl Tunnel CE and encapsulated packet ECT(1)
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(src=1.1.1.1,dst=2.2.2.2,tos=0x3,ttl=64,flags()),in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=1,ttl=64,frag=no),tcp(src=8,dst=9)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=1,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=1,nw_frag=no
 Datapath actions: set(ipv4(tos=0x3/0x3)),2
 ])
 
 dnl Tunnel CE and encapsulated packet ECT(2)
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(src=1.1.1.1,dst=2.2.2.2,tos=0x3,ttl=64,flags()),in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=2,ttl=64,frag=no),tcp(src=8,dst=9)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=2,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=2,nw_frag=no
 Datapath actions: set(ipv4(tos=0x3/0x3)),2
 ])
 
 dnl Tunnel CE and encapsulated packet Non-ECT
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(src=1.1.1.1,dst=2.2.2.2,tos=0x3,ttl=64,flags()),in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=9)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=0,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=0,nw_frag=no
 Datapath actions: drop
 ])
 OVS_VSWITCHD_STOP(["/dropping tunnel packet marked ECN CE but is not ECN capable/d"])
@@ -490,14 +490,14 @@ AT_CHECK([tail -1 stdout], [0],
 dnl Option match
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=0,len=4,0xb}),flags(df|key)),in_port(6081),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata0=0xb/0xf,in_port=1,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata0=0xb/0xf,in_port=1,nw_frag=no
 Datapath actions: 2
 ])
 
 dnl Skip unknown option
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=0,len=4,0xb}{class=0xffff,type=2,len=4,0xc}),flags(df|key)),in_port(6081),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata0=0xb/0xf,in_port=1,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata0=0xb/0xf,in_port=1,nw_frag=no
 Datapath actions: 2
 ])
 
@@ -531,7 +531,7 @@ AT_CHECK([ovs-ofctl add-tlv-map br0 "{class=0xffff,type=3,len=8}->tun_metadata3"
 AT_CHECK([ovs-ofctl add-flow br0 tun_metadata3=0x1234567890abcdef,actions=2])
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=3,len=8,0x1234567890abcdef}),flags(df|key)),in_port(6081),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata3=0x1234567890abcdef,in_port=1,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata3=0x1234567890abcdef,in_port=1,nw_frag=no
 Datapath actions: 2
 ])
 
@@ -640,13 +640,13 @@ NXST_FLOW reply:
 
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=0,len=4,0x12345678}),flags(df|key)),in_port(6081),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata0,tun_metadata1=NP,tun_metadata2=NP,in_port=1,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata0,tun_metadata1=NP,tun_metadata2=NP,in_port=1,nw_frag=no
 Datapath actions: 2
 ])
 
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=1,len=0}),flags(df|key)),in_port(6081),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: recirc_id=0,packet_type=(0,0x0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata1,tun_metadata2=NP,in_port=1,nw_ecn=0,nw_frag=no
+  [Megaflow: recirc_id=0,packet_type=(0,0),ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata1,tun_metadata2=NP,in_port=1,nw_ecn=0,nw_frag=no
 Datapath actions: set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,tp_dst=6081,geneve({class=0xffff,type=0x1,len=0}),flags(df|key))),6081
 ])
 


More information about the dev mailing list