[ovs-dev] [RFC] ipv6 tunneling: Fix for performance drop introduced by ipv6 tunnel support.

[Thadeu Lima de Souza Cascardo]

On Mon, Dec 14, 2015 at 11:06:40PM +0000, Sugesh Chandran wrote:
> Adding a new flag to validate if the tunnel metadata is valid/not. This flag avoids
> the need of resetting and validating the entire ipv4/ipv6 tunnel destination
> address which caused a serious performance drop.
> 
> Signed-off-by: Sugesh Chandran <sugesh.chandran at intel.com>

Hi, Sugesh.

This looks fine to me. Have you tested it? What is the drop in throughput when
you use this patch, compared to before the IPv6 tunneling support was added?

Thanks.
Cascardo.

> ---
>  lib/match.c        | 2 ++
>  lib/netdev-vport.c | 3 ++-
>  lib/packets.h      | 8 ++++----
>  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/match.c b/lib/match.c
> index 95d34bc..79a561d 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -188,6 +188,7 @@ match_set_tun_dst_masked(struct match *match, ovs_be32 dst, ovs_be32 mask)
>  {
>      match->wc.masks.tunnel.ip_dst = mask;
>      match->flow.tunnel.ip_dst = dst & mask;
> +    match->flow.tunnel.is_tnl_valid = 1;
>  }
>  
>  void
> @@ -210,6 +211,7 @@ match_set_tun_ipv6_dst(struct match *match, const struct in6_addr *dst)
>  {
>      match->flow.tunnel.ipv6_dst = *dst;
>      match->wc.masks.tunnel.ipv6_dst = in6addr_exact;
> +    match->flow.tunnel.is_tnl_valid = 1;
>  }
>  
>  void
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 88f5022..1cf37d7 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -920,6 +920,7 @@ ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl,
>  
>          tnl->ip_src = ip_src;
>          tnl->ip_dst = ip_dst;
> +        tnl->is_tnl_valid = 1;
>          tnl->ip_tos = ip->ip_tos;
>          tnl->ip_ttl = ip->ip_ttl;
>  
> @@ -931,7 +932,7 @@ ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl,
>          memcpy(tnl->ipv6_dst.s6_addr, ip6->ip6_dst.be16, sizeof ip6->ip6_dst);
>          tnl->ip_tos = 0;
>          tnl->ip_ttl = ip6->ip6_hlim;
> -
> +        tnl->is_tnl_valid = 1;
>          *hlen += IPV6_HEADER_LEN;
>  
>      } else {
> diff --git a/lib/packets.h b/lib/packets.h
> index edf140b..06c1e19 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -37,6 +37,7 @@ struct ds;
>  
>  /* Tunnel information used in flow key and metadata. */
>  struct flow_tnl {
> +    bool is_tnl_valid;
>      ovs_be32 ip_dst;
>      struct in6_addr ipv6_dst;
>      ovs_be32 ip_src;
> @@ -49,7 +50,7 @@ struct flow_tnl {
>      ovs_be16 tp_dst;
>      ovs_be16 gbp_id;
>      uint8_t  gbp_flags;
> -    uint8_t  pad1[5];        /* Pad to 64 bits. */
> +    uint8_t  pad1[1];        /* Pad to 64 bits. */
>      struct tun_metadata metadata;
>  };
>  
> @@ -79,7 +80,7 @@ static inline bool ipv6_addr_is_set(const struct in6_addr *addr);
>  static inline bool
>  flow_tnl_dst_is_set(const struct flow_tnl *tnl)
>  {
> -    return tnl->ip_dst || ipv6_addr_is_set(&tnl->ipv6_dst);
> +    return tnl->is_tnl_valid;
>  }
>  
>  struct in6_addr flow_tnl_dst(const struct flow_tnl *tnl);
> @@ -157,8 +158,7 @@ pkt_metadata_init(struct pkt_metadata *md, odp_port_t port)
>       * we can just zero out ip_dst and the rest of the data will never be
>       * looked at. */
>      memset(md, 0, offsetof(struct pkt_metadata, in_port));
> -    md->tunnel.ip_dst = 0;
> -    md->tunnel.ipv6_dst = in6addr_any;
> +    md->tunnel.is_tnl_valid = 0;
>  
>      md->in_port.odp_port = port;
>  }
> -- 
> 1.9.1
>