| Message ID | 1450134400-54127-1-git-send-email-sugesh.chandran@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
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@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 >
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 -----Original Message----- From: Thadeu Lima de Souza Cascardo [mailto:cascardo@redhat.com] Sent: Tuesday, December 15, 2015 12:20 PM To: Chandran, Sugesh <sugesh.chandran@intel.com> Cc: dev@openvswitch.org; jbenc@redhat.com 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 <sugesh.chandran@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 >
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:cascardo@redhat.com] > Sent: Tuesday, December 15, 2015 12:20 PM > To: Chandran, Sugesh <sugesh.chandran@intel.com> > Cc: dev@openvswitch.org; jbenc@redhat.com > 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 <sugesh.chandran@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 > >
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@intel.com> Hi, Sugesh. You missed some spots. Tunnel destination is also set at ofproto/tunnel.c:tnl_port_send. At lib/packets.h:flow_tnl_size, I suggest you optimize it to not copy any of the destinations if !flow_tnl_dst_is_set. That is, change the first offset to ip_dst instead of ip_src. Then, you need to fix other cases where dst might be used without checking for flow_tnl_dst_is_set. By the way, it might be a good thing to just rename it to flow_tnl_is_valid. These other cases include ofproto/ofproto-dpif-ipfix.c and ofproto/ofproto-dpif-sflow.c. Notice these only support IPv4 as of now. Also, if we are not initializing the addresses, you might as well check for is_tnl_valid at lib/packets.c:flow_tnl_dst. I am not sure, but I suspect we might screw up at lib/meta-flow.c:mf_get_value. Same for lib/flow.c:flow_get_metadata. match_set_tun_ipv6_dst_masked was not updated. By the way, how do you tell if the tunnel is IPv4 or IPv6? Since we are wasting a lot of bits with this single boolean, why not do what the original patch did, and use it the bits for protocol as well? It could get really messy with a lot of identation around the code. But I guess that we almost everything using IPv6, you could just hide it behind flow_tnl_dst, maybe use a wrapper for setting up IPv4 and IPv6. 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 >
Hi Cascardo, Thank you for reviewing the patch. I had sent out the new patch after incorporating all your comments, please have a look on it and let me know your inputs/suggestions/comments. I verified that the performance drop is fixed by the patch and also I noticed there is a 10% improvement on ipv4 tunneling performance. I haven't tested any other functionalities such as sflow and areas where the tunnel metadata validation has been changed. Please note that the patch uses the newly introduced flag for not only tunnel metadata validation, but also to store the tunnel type(ipv4/ipv6) now. The wrapper functions are introduced to set the destination address and flag. All the ipv6/ipv4 tunnel destination address assignments are now uses these wrappers to assign addresses. Regards _Sugesh -----Original Message----- From: Thadeu Lima de Souza Cascardo [mailto:cascardo@redhat.com] Sent: Friday, December 18, 2015 8:12 PM To: Chandran, Sugesh <sugesh.chandran@intel.com> Cc: dev@openvswitch.org; jbenc@redhat.com 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 <sugesh.chandran@intel.com> Hi, Sugesh. You missed some spots. Tunnel destination is also set at ofproto/tunnel.c:tnl_port_send. At lib/packets.h:flow_tnl_size, I suggest you optimize it to not copy any of the destinations if !flow_tnl_dst_is_set. That is, change the first offset to ip_dst instead of ip_src. Then, you need to fix other cases where dst might be used without checking for flow_tnl_dst_is_set. By the way, it might be a good thing to just rename it to flow_tnl_is_valid. These other cases include ofproto/ofproto-dpif-ipfix.c and ofproto/ofproto-dpif-sflow.c. Notice these only support IPv4 as of now. Also, if we are not initializing the addresses, you might as well check for is_tnl_valid at lib/packets.c:flow_tnl_dst. I am not sure, but I suspect we might screw up at lib/meta-flow.c:mf_get_value. Same for lib/flow.c:flow_get_metadata. match_set_tun_ipv6_dst_masked was not updated. By the way, how do you tell if the tunnel is IPv4 or IPv6? Since we are wasting a lot of bits with this single boolean, why not do what the original patch did, and use it the bits for protocol as well? It could get really messy with a lot of identation around the code. But I guess that we almost everything using IPv6, you could just hide it behind flow_tnl_dst, maybe use a wrapper for setting up IPv4 and IPv6. 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 >
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; }
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@intel.com> --- lib/match.c | 2 ++ lib/netdev-vport.c | 3 ++- lib/packets.h | 8 ++++---- 3 files changed, 8 insertions(+), 5 deletions(-)