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

William Tu u9012063 at gmail.com
Sat Mar 21 15:28:10 UTC 2020


Thanks for the patch, I'm not able to apply, can you rebase to master?

On Sat, Mar 14, 2020 at 07:47:08PM +0800, 贺鹏 wrote:
> if the tunnel is specified as "ip_remote=flow", we can try to generate

s/ip_remote/remote_ip/

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

what if users specified tun_src=xx.xx.xx.xx in flow, then is it ignored?
> 
> Signed-off-by: hepeng <hepeng.0320 at 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      |  2 +-
>  7 files changed, 39 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 adf57a5e8..9b5c3ddad 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4070,7 +4070,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
> +         */
How about rephrase as:
"
Check if the wc contains source info, if not, erase all
source info including smac and sip.
"

William
> +        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. */
> @@ -7599,6 +7609,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..306348377 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
> --
> 2.20.1
> 
> 
> -- 
> hepeng
> Bytedance
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list