Message ID | 1530126123-10520-1-git-send-email-pkusunyifeng@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev] gre: Fix GRE loading bug by separating ERSPAN from tunnel | expand |
On 6/27/2018 12:02 PM, Yifeng Sun wrote: > On kernel versions (for example, 3.10) where upstream provides GRE > functionality but no ERSPAN, GRE can fail to initialize. Because > there is no ERSPAN feature in kernel, openvswitch kernel module will > try to register its own IPPROTO_GRE, instead of using the upstream > GRE functionality. But GRE could already be added by kernel's gre > module. As a result, there will be GRE-related failures. > > This patch separates the detection of tunnel and ERSPAN so that the > result of ERSPAN detection in kernel doesn't affect tunnel functionality. > > This fix passed the travis for different kernel versions: > https://travis-ci.org/yifsun/ovs-travis/builds/397573497 > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > --- > acinclude.m4 | 6 ++++-- > datapath/linux/compat/include/net/dst_metadata.h | 6 ++++++ > datapath/linux/compat/include/net/erspan.h | 2 +- > 3 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index 991a6275b978..8e00ef664894 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -531,8 +531,10 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ > [OVS_GREP_IFELSE([$KSRC/include/net/ip_tunnels.h], > [iptunnel_pull_offloads], > [OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], [dst_cache], > - [OVS_GREP_IFELSE([$KSRC/include/net/erspan.h], [erspan_md2], > - [OVS_DEFINE([USE_UPSTREAM_TUNNEL])])])])]) > + [OVS_DEFINE([USE_UPSTREAM_TUNNEL])])])]) > + > + OVS_GREP_IFELSE([$KSRC/include/net/erspan.h], [erspan_md2], > + [OVS_DEFINE([USE_UPSTREAM_ERSPAN])]) > > OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], [dst_cache], > [OVS_DEFINE([USE_BUILTIN_DST_CACHE])]) > diff --git a/datapath/linux/compat/include/net/dst_metadata.h b/datapath/linux/compat/include/net/dst_metadata.h > index e53a29ed2222..47f0f54d0a47 100644 > --- a/datapath/linux/compat/include/net/dst_metadata.h > +++ b/datapath/linux/compat/include/net/dst_metadata.h > @@ -1,10 +1,12 @@ > #ifndef __NET_DST_METADATA_WRAPPER_H > #define __NET_DST_METADATA_WRAPPER_H 1 > > +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,13,0) > enum metadata_type { > METADATA_IP_TUNNEL, > METADATA_HW_PORT_MUX, > }; > +#endif Thanks for the patch Yifeng. I think you're on the right track here but I believe it would be better to add another #define in acinclude.m4 for HAVE_METADATA_IP_TUNNEL rather than checking the version code. It's safer that way because some distros backport. Thanks, - Greg > > #ifdef USE_UPSTREAM_TUNNEL > #include_next <net/dst_metadata.h> > @@ -119,7 +121,11 @@ void ovs_ip_tunnel_rcv(struct net_device *dev, struct sk_buff *skb, > static inline struct metadata_dst * > rpl_metadata_dst_alloc(u8 optslen, enum metadata_type type, gfp_t flags) > { > +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,13,0) > return metadata_dst_alloc(optslen, flags); > +#else > + return metadata_dst_alloc(optslen, type, flags); > +#endif > } > #define metadata_dst_alloc rpl_metadata_dst_alloc > > diff --git a/datapath/linux/compat/include/net/erspan.h b/datapath/linux/compat/include/net/erspan.h > index 9fdae97b5fc4..f3035b678e87 100644 > --- a/datapath/linux/compat/include/net/erspan.h > +++ b/datapath/linux/compat/include/net/erspan.h > @@ -1,4 +1,4 @@ > -#ifndef USE_UPSTREAM_TUNNEL > +#ifndef USE_UPSTREAM_ERSPAN > #ifndef __LINUX_ERSPAN_H > #define __LINUX_ERSPAN_H >
Thanks for the review. I sent another patch. Best, Yifeng On Thu, Jun 28, 2018 at 8:03 AM, Gregory Rose <gvrose8192@gmail.com> wrote: > On 6/27/2018 12:02 PM, Yifeng Sun wrote: > >> On kernel versions (for example, 3.10) where upstream provides GRE >> functionality but no ERSPAN, GRE can fail to initialize. Because >> there is no ERSPAN feature in kernel, openvswitch kernel module will >> try to register its own IPPROTO_GRE, instead of using the upstream >> GRE functionality. But GRE could already be added by kernel's gre >> module. As a result, there will be GRE-related failures. >> >> This patch separates the detection of tunnel and ERSPAN so that the >> result of ERSPAN detection in kernel doesn't affect tunnel functionality. >> >> This fix passed the travis for different kernel versions: >> https://travis-ci.org/yifsun/ovs-travis/builds/397573497 >> >> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> >> --- >> acinclude.m4 | 6 ++++-- >> datapath/linux/compat/include/net/dst_metadata.h | 6 ++++++ >> datapath/linux/compat/include/net/erspan.h | 2 +- >> 3 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/acinclude.m4 b/acinclude.m4 >> index 991a6275b978..8e00ef664894 100644 >> --- a/acinclude.m4 >> +++ b/acinclude.m4 >> @@ -531,8 +531,10 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ >> [OVS_GREP_IFELSE([$KSRC/includ >> e/net/ip_tunnels.h], >> [iptunnel_pull_offloads], >> [OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], >> [dst_cache], >> - [OVS_GREP_IFELSE([$KSRC/include/net/erspan.h], >> [erspan_md2], >> - [OVS_DEFINE([USE_UPSTREAM_TUN >> NEL])])])])]) >> + [OVS_DEFINE([USE_UPSTREAM_TUN >> NEL])])])]) >> + >> + OVS_GREP_IFELSE([$KSRC/include/net/erspan.h], [erspan_md2], >> + [OVS_DEFINE([USE_UPSTREAM_ERSPAN])]) >> OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], [dst_cache], >> [OVS_DEFINE([USE_BUILTIN_DST_CACHE])]) >> diff --git a/datapath/linux/compat/include/net/dst_metadata.h >> b/datapath/linux/compat/include/net/dst_metadata.h >> index e53a29ed2222..47f0f54d0a47 100644 >> --- a/datapath/linux/compat/include/net/dst_metadata.h >> +++ b/datapath/linux/compat/include/net/dst_metadata.h >> @@ -1,10 +1,12 @@ >> #ifndef __NET_DST_METADATA_WRAPPER_H >> #define __NET_DST_METADATA_WRAPPER_H 1 >> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,13,0) >> enum metadata_type { >> METADATA_IP_TUNNEL, >> METADATA_HW_PORT_MUX, >> }; >> +#endif >> > > Thanks for the patch Yifeng. I think you're on the right track here but I > believe it would be better to > add another #define in acinclude.m4 for HAVE_METADATA_IP_TUNNEL rather > than checking the > version code. It's safer that way because some distros backport. > > Thanks, > > - Greg > > > #ifdef USE_UPSTREAM_TUNNEL >> #include_next <net/dst_metadata.h> >> @@ -119,7 +121,11 @@ void ovs_ip_tunnel_rcv(struct net_device *dev, >> struct sk_buff *skb, >> static inline struct metadata_dst * >> rpl_metadata_dst_alloc(u8 optslen, enum metadata_type type, gfp_t flags) >> { >> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,13,0) >> return metadata_dst_alloc(optslen, flags); >> +#else >> + return metadata_dst_alloc(optslen, type, flags); >> +#endif >> } >> #define metadata_dst_alloc rpl_metadata_dst_alloc >> diff --git a/datapath/linux/compat/include/net/erspan.h >> b/datapath/linux/compat/include/net/erspan.h >> index 9fdae97b5fc4..f3035b678e87 100644 >> --- a/datapath/linux/compat/include/net/erspan.h >> +++ b/datapath/linux/compat/include/net/erspan.h >> @@ -1,4 +1,4 @@ >> -#ifndef USE_UPSTREAM_TUNNEL >> +#ifndef USE_UPSTREAM_ERSPAN >> #ifndef __LINUX_ERSPAN_H >> #define __LINUX_ERSPAN_H >> >> > >
diff --git a/acinclude.m4 b/acinclude.m4 index 991a6275b978..8e00ef664894 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -531,8 +531,10 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ [OVS_GREP_IFELSE([$KSRC/include/net/ip_tunnels.h], [iptunnel_pull_offloads], [OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], [dst_cache], - [OVS_GREP_IFELSE([$KSRC/include/net/erspan.h], [erspan_md2], - [OVS_DEFINE([USE_UPSTREAM_TUNNEL])])])])]) + [OVS_DEFINE([USE_UPSTREAM_TUNNEL])])])]) + + OVS_GREP_IFELSE([$KSRC/include/net/erspan.h], [erspan_md2], + [OVS_DEFINE([USE_UPSTREAM_ERSPAN])]) OVS_GREP_IFELSE([$KSRC/include/net/dst_cache.h], [dst_cache], [OVS_DEFINE([USE_BUILTIN_DST_CACHE])]) diff --git a/datapath/linux/compat/include/net/dst_metadata.h b/datapath/linux/compat/include/net/dst_metadata.h index e53a29ed2222..47f0f54d0a47 100644 --- a/datapath/linux/compat/include/net/dst_metadata.h +++ b/datapath/linux/compat/include/net/dst_metadata.h @@ -1,10 +1,12 @@ #ifndef __NET_DST_METADATA_WRAPPER_H #define __NET_DST_METADATA_WRAPPER_H 1 +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,13,0) enum metadata_type { METADATA_IP_TUNNEL, METADATA_HW_PORT_MUX, }; +#endif #ifdef USE_UPSTREAM_TUNNEL #include_next <net/dst_metadata.h> @@ -119,7 +121,11 @@ void ovs_ip_tunnel_rcv(struct net_device *dev, struct sk_buff *skb, static inline struct metadata_dst * rpl_metadata_dst_alloc(u8 optslen, enum metadata_type type, gfp_t flags) { +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,13,0) return metadata_dst_alloc(optslen, flags); +#else + return metadata_dst_alloc(optslen, type, flags); +#endif } #define metadata_dst_alloc rpl_metadata_dst_alloc diff --git a/datapath/linux/compat/include/net/erspan.h b/datapath/linux/compat/include/net/erspan.h index 9fdae97b5fc4..f3035b678e87 100644 --- a/datapath/linux/compat/include/net/erspan.h +++ b/datapath/linux/compat/include/net/erspan.h @@ -1,4 +1,4 @@ -#ifndef USE_UPSTREAM_TUNNEL +#ifndef USE_UPSTREAM_ERSPAN #ifndef __LINUX_ERSPAN_H #define __LINUX_ERSPAN_H
On kernel versions (for example, 3.10) where upstream provides GRE functionality but no ERSPAN, GRE can fail to initialize. Because there is no ERSPAN feature in kernel, openvswitch kernel module will try to register its own IPPROTO_GRE, instead of using the upstream GRE functionality. But GRE could already be added by kernel's gre module. As a result, there will be GRE-related failures. This patch separates the detection of tunnel and ERSPAN so that the result of ERSPAN detection in kernel doesn't affect tunnel functionality. This fix passed the travis for different kernel versions: https://travis-ci.org/yifsun/ovs-travis/builds/397573497 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> --- acinclude.m4 | 6 ++++-- datapath/linux/compat/include/net/dst_metadata.h | 6 ++++++ datapath/linux/compat/include/net/erspan.h | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-)