On Tue, Dec 15, 2015 at 01:43:08PM +0000, Chandran, Sugesh wrote: > Hi Cascardo, > Thank you for the quick review. > I verified the throughput is improved by 25% by the given patch. There is a > slight tunneling performance improvement also noticed after applying it. > This patch reverts the throughput drop introduced by ipv6 tunneling support. > However I haven't tested ipv6 tunneling with the patch. > > I will work on the patch to submit on the mailing-list if this approach fine > for everyone. > > > > Regards > _Sugesh
I will do a proper review and send my Ack, unless I find any issues. Cascardo. > > > -----Original Message----- > From: Thadeu Lima de Souza Cascardo [mailto:[email protected]] > Sent: Tuesday, December 15, 2015 12:20 PM > To: Chandran, Sugesh <[email protected]> > Cc: [email protected]; [email protected] > Subject: Re: [RFC] ipv6 tunneling: Fix for performance drop introduced by > ipv6 tunnel support. > > 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 <[email protected]> > > 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 > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
