diff mbox series

[ovs-dev,v2] ofproto-dpif-xlate: do not unwildcard source address if not needed

Message ID CACmLciRYh2Y1coj1g0BUaxoWw41E33trzt9+H9xGEVhai3nBuQ@mail.gmail.com
State Superseded
Headers show
Series [ovs-dev,v2] ofproto-dpif-xlate: do not unwildcard source address if not needed | expand

Commit Message

Peng He March 22, 2020, 2:42 a.m. UTC
if the tunnel is specified as "remote_ip=flow", we can try to generate
a megaflow
which ignores all source address, such as source mac, source IP address
in the outter header of the packet. This can reduce the number of
megaflows and avoid expensive upcall and improve the performance.

Signed-off-by: hepeng <hepeng.0320@bytedance.com>
---
 include/openvswitch/flow.h    |  2 ++
 ofproto/ofproto-dpif-xlate.c  | 16 +++++++++++-
 ofproto/tunnel.c              | 17 +++++++++++++
 ofproto/tunnel.h              |  3 ++-
 tests/packet-type-aware.at    |  2 +-
 tests/tunnel-push-pop-ipv6.at |  2 +-
 tests/tunnel-push-pop.at      | 46 ++++++++++++++++++++++++++++++++++-
 7 files changed, 83 insertions(+), 5 deletions(-)

packets:0, bytes:0, used:never,
actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535))
 ])

 ovs-appctl time/warp 10000
@@ -623,3 +623,47 @@ NXST_FLOW reply:

 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([tunnel_push_pop - specify tun_src when remote_ip=flow])
+
+OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy
ofport_request=1 other-config:hwaddr=aa:55:aa:55:00:00])
+AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br
datapath_type=dummy], [0])
+AT_CHECK([ovs-vsctl add-port int-br t1 -- set Interface t1 type=gre \
+                       options:remote_ip=flow options:key=flow
options:seq=true ofport_request=3], [0])
+
+AT_CHECK([ovs-appctl dpif/show], [0], [dnl
+dummy@ovs-dummy: hit:0 missed:0
+  br0:
+    br0 65534/100: (dummy-internal)
+    p0 1/1: (dummy)
+  int-br:
+    int-br 65534/2: (dummy-internal)
+    t1 3/3: (gre: key=flow, remote_ip=flow, seq=true)
+])
+
+AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 1.1.2.88/24], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
+])
+AT_CHECK([ovs-ofctl add-flow br0 'arp,priority=1,action=normal'])
+
+dnl Use arp reply to achieve tunnel next hop mac binding
+AT_CHECK([ovs-appctl netdev-dummy/receive p0
'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
+
+AT_CHECK([ovs-appctl tnl/neigh/show | tail -n+3 | sort], [0], [dnl
+1.1.2.92                                      f8:bc:12:44:34:b6   br0
+])
+
+AT_CHECK([ovs-ofctl add-flow br0
'ip,ip_proto=47,nw_tos=0,eth_src=aa:55:aa:55:00:00,eth_dst=f8:bc:12:44:34:b6,ip_src=1.1.2.88,ip_dst=1.1.2.92,priority=99,action=normal'])
+
+dnl add a flow specifying tun_src
+AT_CHECK([ovs-ofctl add-flow int-br 'tun_src=1.1.2.92,in_port=3,action=drop'])
+
+AT_CHECK([ovs-appctl ofproto/trace int-br
'tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_id=456,in_port=3'], [0],
[stdout])
+AT_CHECK([tail -2 stdout], [0],
+  [Megaflow: recirc_id=0,eth,tun_id=0x1c8,tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_tos=0,tun_flags=-df-csum-key,in_port=3,dl_type=0x0000
+Datapath actions: drop
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
--
2.20.1

Comments

William Tu March 26, 2020, 3:05 p.m. UTC | #1
On Sun, Mar 22, 2020 at 10:42:16AM +0800, 贺鹏 wrote:
> if the tunnel is specified as "remote_ip=flow", we can try to generate
> a megaflow
> which ignores all source address, such as source mac, source IP address
> in the outter header of the packet. This can reduce the number of
> megaflows and avoid expensive upcall and improve the performance.
> 
> Signed-off-by: hepeng <hepeng.0320@bytedance.com>

Hi Hepeng,
I'm not able to apply your patch.
Can you check?

> ---
>  include/openvswitch/flow.h    |  2 ++
>  ofproto/ofproto-dpif-xlate.c  | 16 +++++++++++-
>  ofproto/tunnel.c              | 17 +++++++++++++
>  ofproto/tunnel.h              |  3 ++-
>  tests/packet-type-aware.at    |  2 +-
>  tests/tunnel-push-pop-ipv6.at |  2 +-
>  tests/tunnel-push-pop.at      | 46 ++++++++++++++++++++++++++++++++++-
>  7 files changed, 83 insertions(+), 5 deletions(-)
> 
> diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
> index 57b6c925c..f4b9a6d48 100644
> --- a/include/openvswitch/flow.h
> +++ b/include/openvswitch/flow.h
> @@ -204,6 +204,8 @@ struct flow_wildcards {
>      ((WC)->masks.FIELD |= (MASK))
>  #define WC_UNMASK_FIELD(WC, FIELD) \
>      memset(&(WC)->masks.FIELD, 0, sizeof (WC)->masks.FIELD)
> +#define WC_FIELD_MASKED(WC, FIELD) \
> +    ((WC)->masks.FIELD != 0)
> 
>  void flow_wildcards_init_catchall(struct flow_wildcards *);
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 28dcc67dd..8f438fad3 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4087,7 +4087,17 @@ terminate_native_tunnel(struct xlate_ctx *ctx,
> struct flow *flow,
>      /* XXX: Write better Filter for tunnel port. We can use in_port
>       * in tunnel-port flow to avoid these checks completely. */
>      if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
> -        *tnl_port = tnl_port_map_lookup(flow, wc);
> +        /* to check if the wc contains src info, if not, erase all
> +         * source info including smac, and sip
> +         */
> +        if (!WC_FIELD_MASKED(wc, nw_src)) {
> +            *tnl_port = tnl_port_map_lookup(flow, wc);
> +            if (*tnl_port != ODPP_NONE && !WC_FIELD_MASKED(wc, nw_src)) {
> +                memset(&wc->masks.dl_src, 0, sizeof wc->masks.dl_src);
> +            }
> +        } else {
> +            *tnl_port = tnl_port_map_lookup(flow, wc);
> +        }
this below can move out of if.. else ..
*tnl_port = tnl_port_map_lookup(flow, wc);

Thanks
William
> 
>          /* If no tunnel port was found and it's about an ARP or ICMPv6 packet,
>           * do tunnel neighbor snooping. */
> @@ -7616,6 +7626,10 @@ xlate_actions(struct xlate_in *xin, struct
> xlate_out *xout)
>          ctx.xin->xport_uuid = in_port->uuid;
>      }
> 
> +    if (in_port && in_port->is_tunnel) {
> +        tnl_wc_init_by_port(in_port->ofport, ctx.wc);
> +    }
> +
>      if (flow->packet_type != htonl(PT_ETH) && in_port &&
>          in_port->pt_mode == NETDEV_PT_LEGACY_L3 && ctx.table_id == 0) {
>          /* Add dummy Ethernet header to non-L2 packet if it's coming from a
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> index 03f0ab765..892f3aa5f 100644
> --- a/ofproto/tunnel.c
> +++ b/ofproto/tunnel.c
> @@ -300,6 +300,23 @@ tnl_port_del(const struct ofport_dpif *ofport,
> odp_port_t odp_port)
>      fat_rwlock_unlock(&rwlock);
>  }
> 
> +void
> +tnl_wc_init_by_port(const struct ofport_dpif *ofport,
> +                    struct flow_wildcards *wc)
> +{
> +    struct tnl_port *tnl_port;
> +
> +    fat_rwlock_rdlock(&rwlock);
> +    tnl_port = tnl_find_ofport(ofport);
> +    if (tnl_port) {
> +        if (tnl_port->match.ip_dst_flow) {
> +            WC_UNMASK_FIELD(wc, tunnel.ip_src);
> +            WC_UNMASK_FIELD(wc, tunnel.ipv6_src);
> +        }
> +    }
> +    fat_rwlock_unlock(&rwlock);
> +}
> +
>  /* Looks in the table of tunnels for a tunnel matching the metadata in 'flow'.
>   * Returns the 'ofport' corresponding to the new in_port, or a null pointer if
>   * none is found.
> diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
> index 323f3fa03..166190875 100644
> --- a/ofproto/tunnel.h
> +++ b/ofproto/tunnel.h
> @@ -45,7 +45,8 @@ bool tnl_process_ecn(struct flow *);
>  odp_port_t tnl_port_send(const struct ofport_dpif *, struct flow *,
>                           struct flow_wildcards *wc);
>  const char *tnl_port_get_type(const struct ofport_dpif *tnl_port);
> -
> +void tnl_wc_init_by_port(const struct ofport_dpif *ofport,
> +                         struct flow_wildcards *wc);
>  /* Returns true if 'flow' should be submitted to tnl_port_receive(). */
>  static inline bool
>  tnl_port_should_receive(const struct flow *flow)
> diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
> index 540cf98f3..9c16d1289 100644
> --- a/tests/packet-type-aware.at
> +++ b/tests/packet-type-aware.at
> @@ -1019,7 +1019,7 @@ ovs-appctl time/warp 1000
>  AT_CHECK([
>      ovs-appctl dpctl/dump-flows --names dummy@ovs-dummy | strip_used
> | grep -v ipv6 | sort
>  ], [0], [flow-dump from the main thread:
> -recirc_id(0),in_port(p0),packet_type(ns=0,id=0),eth(src=aa:bb:cc:00:00:02,dst=aa:bb:cc:00:00:01),eth_type(0x0800),ipv4(dst=20.0.0.1,proto=47,frag=no),
> packets:3, bytes:378, used:0.0s, actions:tnl_pop(gre_sys)
> +recirc_id(0),in_port(p0),packet_type(ns=0,id=0),eth(dst=aa:bb:cc:00:00:01),eth_type(0x0800),ipv4(dst=20.0.0.1,proto=47,frag=no),
> packets:3, bytes:378, used:0.0s, actions:tnl_pop(gre_sys)
>  tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0),in_port(gre_sys),packet_type(ns=1,id=0x8847),eth_type(0x8847),mpls(label=999/0x0,tc=0/0,ttl=64/0x0,bos=1/1),
> packets:3, bytes:264, used:0.0s,
> actions:push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),pop_mpls(eth_type=0x800),recirc(0x1)
>  tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0x1),in_port(gre_sys),packet_type(ns=0,id=0),eth(dst=00:00:00:00:00:00),eth_type(0x0800),ipv4(ttl=64,frag=no),
> packets:3, bytes:294, used:0.0s, actions:set(ipv4(ttl=63)),int-br
>  ])
> diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
> index 59723e63b..dbe72f7e7 100644
> --- a/tests/tunnel-push-pop-ipv6.at
> +++ b/tests/tunnel-push-pop-ipv6.at
> @@ -429,7 +429,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port
>  5'], [0], [dnl
>    port  5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=?
>  ])
>  AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl
> -tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:0, bytes:0, used:never,
> actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
> +tunnel(tun_id=0x7b,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:0, bytes:0, used:never,
> actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
>  ])
> 
>  ovs-appctl time/warp 10000
> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
> index b92c23fde..bee4be2fb 100644
> --- a/tests/tunnel-push-pop.at
> +++ b/tests/tunnel-push-pop.at
> @@ -499,7 +499,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port
>  5'], [0], [dnl
>    port  5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=?
>  ])
>  AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl
> -tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:0, bytes:0, used:never,
> actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535))
> +tunnel(tun_id=0x7b,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:0, bytes:0, used:never,
> actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535))
>  ])
> 
>  ovs-appctl time/warp 10000
> @@ -623,3 +623,47 @@ NXST_FLOW reply:
> 
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> +
> +AT_SETUP([tunnel_push_pop - specify tun_src when remote_ip=flow])
> +
> +OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy
> ofport_request=1 other-config:hwaddr=aa:55:aa:55:00:00])
> +AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br
> datapath_type=dummy], [0])
> +AT_CHECK([ovs-vsctl add-port int-br t1 -- set Interface t1 type=gre \
> +                       options:remote_ip=flow options:key=flow
> options:seq=true ofport_request=3], [0])
> +
> +AT_CHECK([ovs-appctl dpif/show], [0], [dnl
> +dummy@ovs-dummy: hit:0 missed:0
> +  br0:
> +    br0 65534/100: (dummy-internal)
> +    p0 1/1: (dummy)
> +  int-br:
> +    int-br 65534/2: (dummy-internal)
> +    t1 3/3: (gre: key=flow, remote_ip=flow, seq=true)
> +])
> +
> +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 1.1.2.88/24], [0], [OK
> +])
> +AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
> +])
> +AT_CHECK([ovs-ofctl add-flow br0 'arp,priority=1,action=normal'])
> +
> +dnl Use arp reply to achieve tunnel next hop mac binding
> +AT_CHECK([ovs-appctl netdev-dummy/receive p0
> 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
> +
> +AT_CHECK([ovs-appctl tnl/neigh/show | tail -n+3 | sort], [0], [dnl
> +1.1.2.92                                      f8:bc:12:44:34:b6   br0
> +])
> +
> +AT_CHECK([ovs-ofctl add-flow br0
> 'ip,ip_proto=47,nw_tos=0,eth_src=aa:55:aa:55:00:00,eth_dst=f8:bc:12:44:34:b6,ip_src=1.1.2.88,ip_dst=1.1.2.92,priority=99,action=normal'])
> +
> +dnl add a flow specifying tun_src
> +AT_CHECK([ovs-ofctl add-flow int-br 'tun_src=1.1.2.92,in_port=3,action=drop'])
> +
> +AT_CHECK([ovs-appctl ofproto/trace int-br
> 'tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_id=456,in_port=3'], [0],
> [stdout])
> +AT_CHECK([tail -2 stdout], [0],
> +  [Megaflow: recirc_id=0,eth,tun_id=0x1c8,tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_tos=0,tun_flags=-df-csum-key,in_port=3,dl_type=0x0000
> +Datapath actions: drop
> +])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> --
> 2.20.1
> 
> 
> -- 
> hepeng
> Bytedance
Peng He March 27, 2020, 2:21 a.m. UTC | #2
Hi, I rebase the patch on the latest master branch.


From 07f10707699a39e3a6de68012dd6c0453120c543 Mon Sep 17 00:00:00 2001
From: hepeng <hepeng.0320@bytedance.com>
Date: Wed, 4 Dec 2019 17:38:22 +0800
Subject: [PATCH] ofproto-dpif-xlate: do not unwildcard source address if not
 needed

if the tunnel is specified as "remote_ip=flow", we can try to generate
a megaflow
which ignores all source address, such as source mac, source IP address
in the outter header of the packet. This can reduce the number of
megaflows and avoid expensive upcall and improve the performance.

Signed-off-by: hepeng <hepeng.0320@bytedance.com>
---
 include/openvswitch/flow.h    |  2 ++
 ofproto/ofproto-dpif-xlate.c  | 16 +++++++++++-
 ofproto/tunnel.c              | 17 +++++++++++++
 ofproto/tunnel.h              |  3 ++-
 tests/packet-type-aware.at    |  2 +-
 tests/tunnel-push-pop-ipv6.at |  2 +-
 tests/tunnel-push-pop.at      | 46 ++++++++++++++++++++++++++++++++++-
 7 files changed, 83 insertions(+), 5 deletions(-)

diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index 3054015d9..a4c810e63 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -204,6 +204,8 @@ struct flow_wildcards {
     ((WC)->masks.FIELD |= (MASK))
 #define WC_UNMASK_FIELD(WC, FIELD) \
     memset(&(WC)->masks.FIELD, 0, sizeof (WC)->masks.FIELD)
+#define WC_FIELD_MASKED(WC, FIELD) \
+    ((WC)->masks.FIELD != 0)

 void flow_wildcards_init_catchall(struct flow_wildcards *);

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 042c50a63..4c0cae30d 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4088,7 +4088,17 @@ terminate_native_tunnel(struct xlate_ctx *ctx,
struct flow *flow,
     /* XXX: Write better Filter for tunnel port. We can use in_port
      * in tunnel-port flow to avoid these checks completely. */
     if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
-        *tnl_port = tnl_port_map_lookup(flow, wc);
+        /* Check if the wc contains source info, if not,
+         * erase all source info including smac and sip.
+         */
+        if (!WC_FIELD_MASKED(wc, nw_src)) {
+            *tnl_port = tnl_port_map_lookup(flow, wc);
+            if (*tnl_port != ODPP_NONE && !WC_FIELD_MASKED(wc, nw_src)) {
+                memset(&wc->masks.dl_src, 0, sizeof wc->masks.dl_src);
+            }
+        } else {
+            *tnl_port = tnl_port_map_lookup(flow, wc);
+        }

         /* If no tunnel port was found and it's about an ARP or ICMPv6 packet,
          * do tunnel neighbor snooping. */
@@ -7617,6 +7627,10 @@ xlate_actions(struct xlate_in *xin, struct
xlate_out *xout)
         ctx.xin->xport_uuid = in_port->uuid;
     }

+    if (in_port && in_port->is_tunnel) {
+        tnl_wc_init_by_port(in_port->ofport, ctx.wc);
+    }
+
     if (flow->packet_type != htonl(PT_ETH) && in_port &&
         in_port->pt_mode == NETDEV_PT_LEGACY_L3 && ctx.table_id == 0) {
         /* Add dummy Ethernet header to non-L2 packet if it's coming from a
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 03f0ab765..892f3aa5f 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -300,6 +300,23 @@ tnl_port_del(const struct ofport_dpif *ofport,
odp_port_t odp_port)
     fat_rwlock_unlock(&rwlock);
 }

+void
+tnl_wc_init_by_port(const struct ofport_dpif *ofport,
+                    struct flow_wildcards *wc)
+{
+    struct tnl_port *tnl_port;
+
+    fat_rwlock_rdlock(&rwlock);
+    tnl_port = tnl_find_ofport(ofport);
+    if (tnl_port) {
+        if (tnl_port->match.ip_dst_flow) {
+            WC_UNMASK_FIELD(wc, tunnel.ip_src);
+            WC_UNMASK_FIELD(wc, tunnel.ipv6_src);
+        }
+    }
+    fat_rwlock_unlock(&rwlock);
+}
+
 /* Looks in the table of tunnels for a tunnel matching the metadata in 'flow'.
  * Returns the 'ofport' corresponding to the new in_port, or a null pointer if
  * none is found.
diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
index 323f3fa03..166190875 100644
--- a/ofproto/tunnel.h
+++ b/ofproto/tunnel.h
@@ -45,7 +45,8 @@ bool tnl_process_ecn(struct flow *);
 odp_port_t tnl_port_send(const struct ofport_dpif *, struct flow *,
                          struct flow_wildcards *wc);
 const char *tnl_port_get_type(const struct ofport_dpif *tnl_port);
-
+void tnl_wc_init_by_port(const struct ofport_dpif *ofport,
+                         struct flow_wildcards *wc);
 /* Returns true if 'flow' should be submitted to tnl_port_receive(). */
 static inline bool
 tnl_port_should_receive(const struct flow *flow)
diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
index 540cf98f3..9c16d1289 100644
--- a/tests/packet-type-aware.at
+++ b/tests/packet-type-aware.at
@@ -1019,7 +1019,7 @@ ovs-appctl time/warp 1000
 AT_CHECK([
     ovs-appctl dpctl/dump-flows --names dummy@ovs-dummy | strip_used
| grep -v ipv6 | sort
 ], [0], [flow-dump from the main thread:
-recirc_id(0),in_port(p0),packet_type(ns=0,id=0),eth(src=aa:bb:cc:00:00:02,dst=aa:bb:cc:00:00:01),eth_type(0x0800),ipv4(dst=20.0.0.1,proto=47,frag=no),
packets:3, bytes:378, used:0.0s, actions:tnl_pop(gre_sys)
+recirc_id(0),in_port(p0),packet_type(ns=0,id=0),eth(dst=aa:bb:cc:00:00:01),eth_type(0x0800),ipv4(dst=20.0.0.1,proto=47,frag=no),
packets:3, bytes:378, used:0.0s, actions:tnl_pop(gre_sys)
 tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0),in_port(gre_sys),packet_type(ns=1,id=0x8847),eth_type(0x8847),mpls(label=999/0x0,tc=0/0,ttl=64/0x0,bos=1/1),
packets:3, bytes:264, used:0.0s,
actions:push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),pop_mpls(eth_type=0x800),recirc(0x1)
 tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0x1),in_port(gre_sys),packet_type(ns=0,id=0),eth(dst=00:00:00:00:00:00),eth_type(0x0800),ipv4(ttl=64,frag=no),
packets:3, bytes:294, used:0.0s, actions:set(ipv4(ttl=63)),int-br
 ])
diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
index 59723e63b..dbe72f7e7 100644
--- a/tests/tunnel-push-pop-ipv6.at
+++ b/tests/tunnel-push-pop-ipv6.at
@@ -429,7 +429,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port
 5'], [0], [dnl
   port  5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=?
 ])
 AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl
-tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:0, bytes:0, used:never,
actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
+tunnel(tun_id=0x7b,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:0, bytes:0, used:never,
actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
 ])

 ovs-appctl time/warp 10000
diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index 48c5de9d1..b35c2eab2 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -519,7 +519,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port
 5'], [0], [dnl
   port  5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=?
 ])
 AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl
-tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:0, bytes:0, used:never,
actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535))
+tunnel(tun_id=0x7b,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:0, bytes:0, used:never,
actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535))
 ])

 ovs-appctl time/warp 10000
@@ -645,3 +645,47 @@ NXST_FLOW reply:

 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([tunnel_push_pop - specify tun_src when remote_ip=flow])
+
+OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy
ofport_request=1 other-config:hwaddr=aa:55:aa:55:00:00])
+AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br
datapath_type=dummy], [0])
+AT_CHECK([ovs-vsctl add-port int-br t1 -- set Interface t1 type=gre \
+                       options:remote_ip=flow options:key=flow
options:seq=true ofport_request=3], [0])
+
+AT_CHECK([ovs-appctl dpif/show], [0], [dnl
+dummy@ovs-dummy: hit:0 missed:0
+  br0:
+    br0 65534/100: (dummy-internal)
+    p0 1/1: (dummy)
+  int-br:
+    int-br 65534/2: (dummy-internal)
+    t1 3/3: (gre: key=flow, remote_ip=flow, seq=true)
+])
+
+AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 1.1.2.88/24], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
+])
+AT_CHECK([ovs-ofctl add-flow br0 'arp,priority=1,action=normal'])
+
+dnl Use arp reply to achieve tunnel next hop mac binding
+AT_CHECK([ovs-appctl netdev-dummy/receive p0
'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
+
+AT_CHECK([ovs-appctl tnl/neigh/show | tail -n+3 | sort], [0], [dnl
+1.1.2.92                                      f8:bc:12:44:34:b6   br0
+])
+
+AT_CHECK([ovs-ofctl add-flow br0
'ip,ip_proto=47,nw_tos=0,eth_src=aa:55:aa:55:00:00,eth_dst=f8:bc:12:44:34:b6,ip_src=1.1.2.88,ip_dst=1.1.2.92,priority=99,action=normal'])
+
+dnl add a flow specifying tun_src
+AT_CHECK([ovs-ofctl add-flow int-br 'tun_src=1.1.2.92,in_port=3,action=drop'])
+
+AT_CHECK([ovs-appctl ofproto/trace int-br
'tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_id=456,in_port=3'], [0],
[stdout])
+AT_CHECK([tail -2 stdout], [0],
+  [Megaflow: recirc_id=0,eth,tun_id=0x1c8,tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_tos=0,tun_flags=-df-csum-key,in_port=3,dl_type=0x0000
+Datapath actions: drop
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
--
2.20.1

William Tu <u9012063@gmail.com> 于2020年3月26日周四 下午11:05写道:
>
> On Sun, Mar 22, 2020 at 10:42:16AM +0800, 贺鹏 wrote:
> > if the tunnel is specified as "remote_ip=flow", we can try to generate
> > a megaflow
> > which ignores all source address, such as source mac, source IP address
> > in the outter header of the packet. This can reduce the number of
> > megaflows and avoid expensive upcall and improve the performance.
> >
> > Signed-off-by: hepeng <hepeng.0320@bytedance.com>
>
> Hi Hepeng,
> I'm not able to apply your patch.
> Can you check?
>
> > ---
> >  include/openvswitch/flow.h    |  2 ++
> >  ofproto/ofproto-dpif-xlate.c  | 16 +++++++++++-
> >  ofproto/tunnel.c              | 17 +++++++++++++
> >  ofproto/tunnel.h              |  3 ++-
> >  tests/packet-type-aware.at    |  2 +-
> >  tests/tunnel-push-pop-ipv6.at |  2 +-
> >  tests/tunnel-push-pop.at      | 46 ++++++++++++++++++++++++++++++++++-
> >  7 files changed, 83 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
> > index 57b6c925c..f4b9a6d48 100644
> > --- a/include/openvswitch/flow.h
> > +++ b/include/openvswitch/flow.h
> > @@ -204,6 +204,8 @@ struct flow_wildcards {
> >      ((WC)->masks.FIELD |= (MASK))
> >  #define WC_UNMASK_FIELD(WC, FIELD) \
> >      memset(&(WC)->masks.FIELD, 0, sizeof (WC)->masks.FIELD)
> > +#define WC_FIELD_MASKED(WC, FIELD) \
> > +    ((WC)->masks.FIELD != 0)
> >
> >  void flow_wildcards_init_catchall(struct flow_wildcards *);
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 28dcc67dd..8f438fad3 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -4087,7 +4087,17 @@ terminate_native_tunnel(struct xlate_ctx *ctx,
> > struct flow *flow,
> >      /* XXX: Write better Filter for tunnel port. We can use in_port
> >       * in tunnel-port flow to avoid these checks completely. */
> >      if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
> > -        *tnl_port = tnl_port_map_lookup(flow, wc);
> > +        /* to check if the wc contains src info, if not, erase all
> > +         * source info including smac, and sip
> > +         */
> > +        if (!WC_FIELD_MASKED(wc, nw_src)) {
> > +            *tnl_port = tnl_port_map_lookup(flow, wc);
> > +            if (*tnl_port != ODPP_NONE && !WC_FIELD_MASKED(wc, nw_src)) {
> > +                memset(&wc->masks.dl_src, 0, sizeof wc->masks.dl_src);
> > +            }
> > +        } else {
> > +            *tnl_port = tnl_port_map_lookup(flow, wc);
> > +        }
> this below can move out of if.. else ..
> *tnl_port = tnl_port_map_lookup(flow, wc);
>
> Thanks
> William
> >
> >          /* If no tunnel port was found and it's about an ARP or ICMPv6 packet,
> >           * do tunnel neighbor snooping. */
> > @@ -7616,6 +7626,10 @@ xlate_actions(struct xlate_in *xin, struct
> > xlate_out *xout)
> >          ctx.xin->xport_uuid = in_port->uuid;
> >      }
> >
> > +    if (in_port && in_port->is_tunnel) {
> > +        tnl_wc_init_by_port(in_port->ofport, ctx.wc);
> > +    }
> > +
> >      if (flow->packet_type != htonl(PT_ETH) && in_port &&
> >          in_port->pt_mode == NETDEV_PT_LEGACY_L3 && ctx.table_id == 0) {
> >          /* Add dummy Ethernet header to non-L2 packet if it's coming from a
> > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> > index 03f0ab765..892f3aa5f 100644
> > --- a/ofproto/tunnel.c
> > +++ b/ofproto/tunnel.c
> > @@ -300,6 +300,23 @@ tnl_port_del(const struct ofport_dpif *ofport,
> > odp_port_t odp_port)
> >      fat_rwlock_unlock(&rwlock);
> >  }
> >
> > +void
> > +tnl_wc_init_by_port(const struct ofport_dpif *ofport,
> > +                    struct flow_wildcards *wc)
> > +{
> > +    struct tnl_port *tnl_port;
> > +
> > +    fat_rwlock_rdlock(&rwlock);
> > +    tnl_port = tnl_find_ofport(ofport);
> > +    if (tnl_port) {
> > +        if (tnl_port->match.ip_dst_flow) {
> > +            WC_UNMASK_FIELD(wc, tunnel.ip_src);
> > +            WC_UNMASK_FIELD(wc, tunnel.ipv6_src);
> > +        }
> > +    }
> > +    fat_rwlock_unlock(&rwlock);
> > +}
> > +
> >  /* Looks in the table of tunnels for a tunnel matching the metadata in 'flow'.
> >   * Returns the 'ofport' corresponding to the new in_port, or a null pointer if
> >   * none is found.
> > diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
> > index 323f3fa03..166190875 100644
> > --- a/ofproto/tunnel.h
> > +++ b/ofproto/tunnel.h
> > @@ -45,7 +45,8 @@ bool tnl_process_ecn(struct flow *);
> >  odp_port_t tnl_port_send(const struct ofport_dpif *, struct flow *,
> >                           struct flow_wildcards *wc);
> >  const char *tnl_port_get_type(const struct ofport_dpif *tnl_port);
> > -
> > +void tnl_wc_init_by_port(const struct ofport_dpif *ofport,
> > +                         struct flow_wildcards *wc);
> >  /* Returns true if 'flow' should be submitted to tnl_port_receive(). */
> >  static inline bool
> >  tnl_port_should_receive(const struct flow *flow)
> > diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
> > index 540cf98f3..9c16d1289 100644
> > --- a/tests/packet-type-aware.at
> > +++ b/tests/packet-type-aware.at
> > @@ -1019,7 +1019,7 @@ ovs-appctl time/warp 1000
> >  AT_CHECK([
> >      ovs-appctl dpctl/dump-flows --names dummy@ovs-dummy | strip_used
> > | grep -v ipv6 | sort
> >  ], [0], [flow-dump from the main thread:
> > -recirc_id(0),in_port(p0),packet_type(ns=0,id=0),eth(src=aa:bb:cc:00:00:02,dst=aa:bb:cc:00:00:01),eth_type(0x0800),ipv4(dst=20.0.0.1,proto=47,frag=no),
> > packets:3, bytes:378, used:0.0s, actions:tnl_pop(gre_sys)
> > +recirc_id(0),in_port(p0),packet_type(ns=0,id=0),eth(dst=aa:bb:cc:00:00:01),eth_type(0x0800),ipv4(dst=20.0.0.1,proto=47,frag=no),
> > packets:3, bytes:378, used:0.0s, actions:tnl_pop(gre_sys)
> >  tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0),in_port(gre_sys),packet_type(ns=1,id=0x8847),eth_type(0x8847),mpls(label=999/0x0,tc=0/0,ttl=64/0x0,bos=1/1),
> > packets:3, bytes:264, used:0.0s,
> > actions:push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),pop_mpls(eth_type=0x800),recirc(0x1)
> >  tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0x1),in_port(gre_sys),packet_type(ns=0,id=0),eth(dst=00:00:00:00:00:00),eth_type(0x0800),ipv4(ttl=64,frag=no),
> > packets:3, bytes:294, used:0.0s, actions:set(ipv4(ttl=63)),int-br
> >  ])
> > diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
> > index 59723e63b..dbe72f7e7 100644
> > --- a/tests/tunnel-push-pop-ipv6.at
> > +++ b/tests/tunnel-push-pop-ipv6.at
> > @@ -429,7 +429,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port
> >  5'], [0], [dnl
> >    port  5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=?
> >  ])
> >  AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl
> > -tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> > packets:0, bytes:0, used:never,
> > actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
> > +tunnel(tun_id=0x7b,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> > packets:0, bytes:0, used:never,
> > actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
> >  ])
> >
> >  ovs-appctl time/warp 10000
> > diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
> > index b92c23fde..bee4be2fb 100644
> > --- a/tests/tunnel-push-pop.at
> > +++ b/tests/tunnel-push-pop.at
> > @@ -499,7 +499,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port
> >  5'], [0], [dnl
> >    port  5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=?
> >  ])
> >  AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl
> > -tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> > packets:0, bytes:0, used:never,
> > actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535))
> > +tunnel(tun_id=0x7b,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> > packets:0, bytes:0, used:never,
> > actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535))
> >  ])
> >
> >  ovs-appctl time/warp 10000
> > @@ -623,3 +623,47 @@ NXST_FLOW reply:
> >
> >  OVS_VSWITCHD_STOP
> >  AT_CLEANUP
> > +
> > +AT_SETUP([tunnel_push_pop - specify tun_src when remote_ip=flow])
> > +
> > +OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy
> > ofport_request=1 other-config:hwaddr=aa:55:aa:55:00:00])
> > +AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br
> > datapath_type=dummy], [0])
> > +AT_CHECK([ovs-vsctl add-port int-br t1 -- set Interface t1 type=gre \
> > +                       options:remote_ip=flow options:key=flow
> > options:seq=true ofport_request=3], [0])
> > +
> > +AT_CHECK([ovs-appctl dpif/show], [0], [dnl
> > +dummy@ovs-dummy: hit:0 missed:0
> > +  br0:
> > +    br0 65534/100: (dummy-internal)
> > +    p0 1/1: (dummy)
> > +  int-br:
> > +    int-br 65534/2: (dummy-internal)
> > +    t1 3/3: (gre: key=flow, remote_ip=flow, seq=true)
> > +])
> > +
> > +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 1.1.2.88/24], [0], [OK
> > +])
> > +AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
> > +])
> > +AT_CHECK([ovs-ofctl add-flow br0 'arp,priority=1,action=normal'])
> > +
> > +dnl Use arp reply to achieve tunnel next hop mac binding
> > +AT_CHECK([ovs-appctl netdev-dummy/receive p0
> > 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
> > +
> > +AT_CHECK([ovs-appctl tnl/neigh/show | tail -n+3 | sort], [0], [dnl
> > +1.1.2.92                                      f8:bc:12:44:34:b6   br0
> > +])
> > +
> > +AT_CHECK([ovs-ofctl add-flow br0
> > 'ip,ip_proto=47,nw_tos=0,eth_src=aa:55:aa:55:00:00,eth_dst=f8:bc:12:44:34:b6,ip_src=1.1.2.88,ip_dst=1.1.2.92,priority=99,action=normal'])
> > +
> > +dnl add a flow specifying tun_src
> > +AT_CHECK([ovs-ofctl add-flow int-br 'tun_src=1.1.2.92,in_port=3,action=drop'])
> > +
> > +AT_CHECK([ovs-appctl ofproto/trace int-br
> > 'tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_id=456,in_port=3'], [0],
> > [stdout])
> > +AT_CHECK([tail -2 stdout], [0],
> > +  [Megaflow: recirc_id=0,eth,tun_id=0x1c8,tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_tos=0,tun_flags=-df-csum-key,in_port=3,dl_type=0x0000
> > +Datapath actions: drop
> > +])
> > +
> > +OVS_VSWITCHD_STOP
> > +AT_CLEANUP
> > --
> > 2.20.1
> >
> >
> > --
> > hepeng
> > Bytedance
William Tu April 6, 2020, 3:11 p.m. UTC | #3
Hi Hepeng,

Thanks for your reply. Can you re-send a separate v3 patch email?
I can't apply your patch to master.
Also some comment below


On Thu, Mar 26, 2020 at 7:21 PM 贺鹏 <xnhp0320@gmail.com> wrote:
>
> Hi, I rebase the patch on the latest master branch.
>
>
> From 07f10707699a39e3a6de68012dd6c0453120c543 Mon Sep 17 00:00:00 2001
> From: hepeng <hepeng.0320@bytedance.com>
> Date: Wed, 4 Dec 2019 17:38:22 +0800
> Subject: [PATCH] ofproto-dpif-xlate: do not unwildcard source address if not
>  needed
>
> if the tunnel is specified as "remote_ip=flow", we can try to generate
> a megaflow
> which ignores all source address, such as source mac, source IP address
> in the outter header of the packet. This can reduce the number of
> megaflows and avoid expensive upcall and improve the performance.
>
> Signed-off-by: hepeng <hepeng.0320@bytedance.com>
> ---
>  include/openvswitch/flow.h    |  2 ++
>  ofproto/ofproto-dpif-xlate.c  | 16 +++++++++++-
>  ofproto/tunnel.c              | 17 +++++++++++++
>  ofproto/tunnel.h              |  3 ++-
>  tests/packet-type-aware.at    |  2 +-
>  tests/tunnel-push-pop-ipv6.at |  2 +-
>  tests/tunnel-push-pop.at      | 46 ++++++++++++++++++++++++++++++++++-
>  7 files changed, 83 insertions(+), 5 deletions(-)
>
> diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
> index 3054015d9..a4c810e63 100644
> --- a/include/openvswitch/flow.h
> +++ b/include/openvswitch/flow.h
> @@ -204,6 +204,8 @@ struct flow_wildcards {
>      ((WC)->masks.FIELD |= (MASK))
>  #define WC_UNMASK_FIELD(WC, FIELD) \
>      memset(&(WC)->masks.FIELD, 0, sizeof (WC)->masks.FIELD)
> +#define WC_FIELD_MASKED(WC, FIELD) \
> +    ((WC)->masks.FIELD != 0)
>
>  void flow_wildcards_init_catchall(struct flow_wildcards *);
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 042c50a63..4c0cae30d 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4088,7 +4088,17 @@ terminate_native_tunnel(struct xlate_ctx *ctx,
> struct flow *flow,
>      /* XXX: Write better Filter for tunnel port. We can use in_port
>       * in tunnel-port flow to avoid these checks completely. */
>      if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
> -        *tnl_port = tnl_port_map_lookup(flow, wc);
> +        /* Check if the wc contains source info, if not,
> +         * erase all source info including smac and sip.
> +         */
> +        if (!WC_FIELD_MASKED(wc, nw_src)) {
> +            *tnl_port = tnl_port_map_lookup(flow, wc);

Move out the if else statement since both has it?

> +            if (*tnl_port != ODPP_NONE && !WC_FIELD_MASKED(wc, nw_src)) {

why check "!WC_FIELD_MASKED(wc, nw_src)" again?  the outer "if"
already check it.

> +                memset(&wc->masks.dl_src, 0, sizeof wc->masks.dl_src);

use WC_UNMASK_FIELD?

Thank you
William
> +            }
> +        } else {
> +            *tnl_port = tnl_port_map_lookup(flow, wc);
> +        }
>
>          /* If no tunnel port was found and it's about an ARP or ICMPv6 packet,
>           * do tunnel neighbor snooping. */
> @@ -7617,6 +7627,10 @@ xlate_actions(struct xlate_in *xin, struct
> xlate_out *xout)
>          ctx.xin->xport_uuid = in_port->uuid;
>      }
>
> +    if (in_port && in_port->is_tunnel) {
> +        tnl_wc_init_by_port(in_port->ofport, ctx.wc);
> +    }
> +
>      if (flow->packet_type != htonl(PT_ETH) && in_port &&
>          in_port->pt_mode == NETDEV_PT_LEGACY_L3 && ctx.table_id == 0) {
>          /* Add dummy Ethernet header to non-L2 packet if it's coming from a
diff mbox series

Patch

diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index 57b6c925c..f4b9a6d48 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -204,6 +204,8 @@  struct flow_wildcards {
     ((WC)->masks.FIELD |= (MASK))
 #define WC_UNMASK_FIELD(WC, FIELD) \
     memset(&(WC)->masks.FIELD, 0, sizeof (WC)->masks.FIELD)
+#define WC_FIELD_MASKED(WC, FIELD) \
+    ((WC)->masks.FIELD != 0)

 void flow_wildcards_init_catchall(struct flow_wildcards *);

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 28dcc67dd..8f438fad3 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4087,7 +4087,17 @@  terminate_native_tunnel(struct xlate_ctx *ctx,
struct flow *flow,
     /* XXX: Write better Filter for tunnel port. We can use in_port
      * in tunnel-port flow to avoid these checks completely. */
     if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
-        *tnl_port = tnl_port_map_lookup(flow, wc);
+        /* to check if the wc contains src info, if not, erase all
+         * source info including smac, and sip
+         */
+        if (!WC_FIELD_MASKED(wc, nw_src)) {
+            *tnl_port = tnl_port_map_lookup(flow, wc);
+            if (*tnl_port != ODPP_NONE && !WC_FIELD_MASKED(wc, nw_src)) {
+                memset(&wc->masks.dl_src, 0, sizeof wc->masks.dl_src);
+            }
+        } else {
+            *tnl_port = tnl_port_map_lookup(flow, wc);
+        }

         /* If no tunnel port was found and it's about an ARP or ICMPv6 packet,
          * do tunnel neighbor snooping. */
@@ -7616,6 +7626,10 @@  xlate_actions(struct xlate_in *xin, struct
xlate_out *xout)
         ctx.xin->xport_uuid = in_port->uuid;
     }

+    if (in_port && in_port->is_tunnel) {
+        tnl_wc_init_by_port(in_port->ofport, ctx.wc);
+    }
+
     if (flow->packet_type != htonl(PT_ETH) && in_port &&
         in_port->pt_mode == NETDEV_PT_LEGACY_L3 && ctx.table_id == 0) {
         /* Add dummy Ethernet header to non-L2 packet if it's coming from a
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 03f0ab765..892f3aa5f 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -300,6 +300,23 @@  tnl_port_del(const struct ofport_dpif *ofport,
odp_port_t odp_port)
     fat_rwlock_unlock(&rwlock);
 }

+void
+tnl_wc_init_by_port(const struct ofport_dpif *ofport,
+                    struct flow_wildcards *wc)
+{
+    struct tnl_port *tnl_port;
+
+    fat_rwlock_rdlock(&rwlock);
+    tnl_port = tnl_find_ofport(ofport);
+    if (tnl_port) {
+        if (tnl_port->match.ip_dst_flow) {
+            WC_UNMASK_FIELD(wc, tunnel.ip_src);
+            WC_UNMASK_FIELD(wc, tunnel.ipv6_src);
+        }
+    }
+    fat_rwlock_unlock(&rwlock);
+}
+
 /* Looks in the table of tunnels for a tunnel matching the metadata in 'flow'.
  * Returns the 'ofport' corresponding to the new in_port, or a null pointer if
  * none is found.
diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
index 323f3fa03..166190875 100644
--- a/ofproto/tunnel.h
+++ b/ofproto/tunnel.h
@@ -45,7 +45,8 @@  bool tnl_process_ecn(struct flow *);
 odp_port_t tnl_port_send(const struct ofport_dpif *, struct flow *,
                          struct flow_wildcards *wc);
 const char *tnl_port_get_type(const struct ofport_dpif *tnl_port);
-
+void tnl_wc_init_by_port(const struct ofport_dpif *ofport,
+                         struct flow_wildcards *wc);
 /* Returns true if 'flow' should be submitted to tnl_port_receive(). */
 static inline bool
 tnl_port_should_receive(const struct flow *flow)
diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
index 540cf98f3..9c16d1289 100644
--- a/tests/packet-type-aware.at
+++ b/tests/packet-type-aware.at
@@ -1019,7 +1019,7 @@  ovs-appctl time/warp 1000
 AT_CHECK([
     ovs-appctl dpctl/dump-flows --names dummy@ovs-dummy | strip_used
| grep -v ipv6 | sort
 ], [0], [flow-dump from the main thread:
-recirc_id(0),in_port(p0),packet_type(ns=0,id=0),eth(src=aa:bb:cc:00:00:02,dst=aa:bb:cc:00:00:01),eth_type(0x0800),ipv4(dst=20.0.0.1,proto=47,frag=no),
packets:3, bytes:378, used:0.0s, actions:tnl_pop(gre_sys)
+recirc_id(0),in_port(p0),packet_type(ns=0,id=0),eth(dst=aa:bb:cc:00:00:01),eth_type(0x0800),ipv4(dst=20.0.0.1,proto=47,frag=no),
packets:3, bytes:378, used:0.0s, actions:tnl_pop(gre_sys)
 tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0),in_port(gre_sys),packet_type(ns=1,id=0x8847),eth_type(0x8847),mpls(label=999/0x0,tc=0/0,ttl=64/0x0,bos=1/1),
packets:3, bytes:264, used:0.0s,
actions:push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),pop_mpls(eth_type=0x800),recirc(0x1)
 tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0x1),in_port(gre_sys),packet_type(ns=0,id=0),eth(dst=00:00:00:00:00:00),eth_type(0x0800),ipv4(ttl=64,frag=no),
packets:3, bytes:294, used:0.0s, actions:set(ipv4(ttl=63)),int-br
 ])
diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
index 59723e63b..dbe72f7e7 100644
--- a/tests/tunnel-push-pop-ipv6.at
+++ b/tests/tunnel-push-pop-ipv6.at
@@ -429,7 +429,7 @@  AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port
 5'], [0], [dnl
   port  5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=?
 ])
 AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl
-tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:0, bytes:0, used:never,
actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
+tunnel(tun_id=0x7b,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:0, bytes:0, used:never,
actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
 ])

 ovs-appctl time/warp 10000
diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index b92c23fde..bee4be2fb 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -499,7 +499,7 @@  AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port
 5'], [0], [dnl
   port  5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=?
 ])
 AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl
-tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:0, bytes:0, used:never,
actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535))
+tunnel(tun_id=0x7b,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),