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

William Tu u9012063 at gmail.com
Thu Mar 26 15:05:19 UTC 2020


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


More information about the dev mailing list