| Message ID | 1525915309-29789-1-git-send-email-yihung.wei@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] datapath: compat: Fix build on RHEL 7.5 | expand |
On 5/9/2018 6:21 PM, Yi-Hung Wei wrote: > 1) OVS datapath compat modules breaks on RHEL 7.5, because it moves > ndo_change_mtu function pointer from 'struct net_device_ops' to > 'struct net_device_ops_extended'. > > 2) RHEL 7.5 introduces the MTU range checking as mentioned in > 6c0bf091 ("datapath: use core MTU range checking in core net infra"). > However, the max_mtu field is defined in 'struct net_device_extended' > but not in 'struct net_device' as upstream kernel. > > This patch defines a new symbol HAVE_RHEL7_MAX_MTU that determines > the previous 2 conditions, and fixes the backport issue. > > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> > --- > acinclude.m4 | 2 ++ > datapath/linux/compat/geneve.c | 4 ++++ > datapath/linux/compat/ip_gre.c | 4 ++++ > datapath/linux/compat/lisp.c | 4 ++++ > datapath/linux/compat/stt.c | 4 ++++ > datapath/linux/compat/vxlan.c | 8 ++++++++ > datapath/vport-internal_dev.c | 2 +- > 7 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index 60186d33de88..a2444af33c22 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -576,6 +576,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ > > OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netdevice.h], [net_device], > [max_mtu]) > + OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netdevice.h], [net_device_ops_extended], > + [ndo_change_mtu], [OVS_DEFINE([HAVE_RHEL7_MAX_MTU])]) > > OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_hook_state]) > OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_register_net_hook]) > diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c > index c08dced3feaf..eacffb2c5b2a 100644 > --- a/datapath/linux/compat/geneve.c > +++ b/datapath/linux/compat/geneve.c > @@ -1271,7 +1271,11 @@ static const struct net_device_ops geneve_netdev_ops = { > .ndo_stop = geneve_stop, > .ndo_start_xmit = geneve_dev_xmit, > .ndo_get_stats64 = ip_tunnel_get_stats64, > +#ifdef HAVE_RHEL7_MAX_MTU > + .extended.ndo_change_mtu = geneve_change_mtu, > +#else > .ndo_change_mtu = geneve_change_mtu, > +#endif > .ndo_validate_addr = eth_validate_addr, > .ndo_set_mac_address = eth_mac_addr, > #ifdef HAVE_NDO_FILL_METADATA_DST > diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c > index 4e325914d8be..2f297a548e30 100644 > --- a/datapath/linux/compat/ip_gre.c > +++ b/datapath/linux/compat/ip_gre.c > @@ -491,7 +491,11 @@ static const struct net_device_ops gre_tap_netdev_ops = { > .ndo_start_xmit = gre_dev_xmit, > .ndo_set_mac_address = eth_mac_addr, > .ndo_validate_addr = eth_validate_addr, > +#ifdef HAVE_RHEL7_MAX_MTU > + .extended.ndo_change_mtu = ip_tunnel_change_mtu, > +#else > .ndo_change_mtu = ip_tunnel_change_mtu, > +#endif > #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39) > .ndo_get_stats64 = ip_tunnel_get_stats64, > #endif > diff --git a/datapath/linux/compat/lisp.c b/datapath/linux/compat/lisp.c > index 34f82324a7ff..4882a636a33e 100644 > --- a/datapath/linux/compat/lisp.c > +++ b/datapath/linux/compat/lisp.c > @@ -546,7 +546,11 @@ static const struct net_device_ops lisp_netdev_ops = { > .ndo_open = lisp_open, > .ndo_stop = lisp_stop, > .ndo_start_xmit = lisp_dev_xmit, > +#ifdef HAVE_RHEL7_MAX_MTU > + .extended.ndo_change_mtu = lisp_change_mtu, > +#else > .ndo_change_mtu = lisp_change_mtu, > +#endif > .ndo_validate_addr = eth_validate_addr, > .ndo_set_mac_address = eth_mac_addr, > #ifdef USE_UPSTREAM_TUNNEL > diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c > index 2189476e176e..ee1c7aa0a78d 100644 > --- a/datapath/linux/compat/stt.c > +++ b/datapath/linux/compat/stt.c > @@ -1857,7 +1857,11 @@ static const struct net_device_ops stt_netdev_ops = { > .ndo_stop = stt_stop, > .ndo_start_xmit = stt_dev_xmit, > .ndo_get_stats64 = ip_tunnel_get_stats64, > +#ifdef HAVE_RHEL7_MAX_MTU > + .extended.ndo_change_mtu = stt_change_mtu, > +#else > .ndo_change_mtu = stt_change_mtu, > +#endif > .ndo_validate_addr = eth_validate_addr, > .ndo_set_mac_address = eth_mac_addr, > #ifdef USE_UPSTREAM_TUNNEL > diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c > index 54b953454bf4..40cb12b1329d 100644 > --- a/datapath/linux/compat/vxlan.c > +++ b/datapath/linux/compat/vxlan.c > @@ -1481,7 +1481,11 @@ static const struct net_device_ops vxlan_netdev_ether_ops = { > .ndo_start_xmit = vxlan_dev_xmit, > .ndo_get_stats64 = ip_tunnel_get_stats64, > .ndo_set_rx_mode = vxlan_set_multicast_list, > +#ifdef HAVE_RHEL7_MAX_MTU > + .extended.ndo_change_mtu = vxlan_change_mtu, > +#else > .ndo_change_mtu = vxlan_change_mtu, > +#endif > .ndo_validate_addr = eth_validate_addr, > .ndo_set_mac_address = eth_mac_addr, > #ifdef HAVE_NDO_FILL_METADATA_DST > @@ -1496,7 +1500,11 @@ static const struct net_device_ops vxlan_netdev_raw_ops = { > .ndo_stop = vxlan_stop, > .ndo_start_xmit = vxlan_dev_xmit, > .ndo_get_stats64 = ip_tunnel_get_stats64, > +#ifdef HAVE_RHEL7_MAX_MTU > + .extended.ndo_change_mtu = vxlan_change_mtu, > +#else > .ndo_change_mtu = vxlan_change_mtu, > +#endif > #ifdef HAVE_NDO_FILL_METADATA_DST > .ndo_fill_metadata_dst = ovs_vxlan_fill_metadata_dst, > #endif > diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c > index f48684b2e70f..90e76bac2165 100644 > --- a/datapath/vport-internal_dev.c > +++ b/datapath/vport-internal_dev.c > @@ -153,7 +153,7 @@ static const struct net_device_ops internal_dev_netdev_ops = { > .ndo_stop = internal_dev_stop, > .ndo_start_xmit = internal_dev_xmit, > .ndo_set_mac_address = eth_mac_addr, > -#ifndef HAVE_NET_DEVICE_WITH_MAX_MTU > +#if !defined(HAVE_NET_DEVICE_WITH_MAX_MTU) && !defined(HAVE_RHEL7_MAX_MTU) > .ndo_change_mtu = internal_dev_change_mtu, > #endif > .ndo_get_stats64 = (void *)internal_get_stats, Very close to the same patch I was carrying locally for RHEL 7.5 but haven't had time to post. Thanks!!! LGTM. Reviewed-by: Greg Rose <gvrose8192@gmail.com>
> diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c > index f48684b2e70f..90e76bac2165 100644 > --- a/datapath/vport-internal_dev.c > +++ b/datapath/vport-internal_dev.c > @@ -153,7 +153,7 @@ static const struct net_device_ops internal_dev_netdev_ops = { > .ndo_stop = internal_dev_stop, > .ndo_start_xmit = internal_dev_xmit, > .ndo_set_mac_address = eth_mac_addr, > -#ifndef HAVE_NET_DEVICE_WITH_MAX_MTU > +#if !defined(HAVE_NET_DEVICE_WITH_MAX_MTU) && !defined(HAVE_RHEL7_MAX_MTU) > .ndo_change_mtu = internal_dev_change_mtu, > #endif > .ndo_get_stats64 = (void *)internal_get_stats, On RHEL 7.5, internal_dev_change_mtu will not be used and the compiler will throw a warning about it: CC [M] /root/ovs/datapath/linux/vport-internal_dev.o /root/ovs/datapath/linux/vport-internal_dev.c:92:12: warning: ‘internal_dev_change_mtu’ defined but not used [-Wunused-function] static int internal_dev_change_mtu(struct net_device *dev, int new_mtu) Can we have the same conditional [0] around that function definition to avoid such warning ? Other than that the patch looks good to me, thank you! [0] #if !defined(HAVE_NET_DEVICE_WITH_MAX_MTU) && !defined(HAVE_RHEL7_MAX_MTU)
> On RHEL 7.5, internal_dev_change_mtu will not be used and the compiler > will throw a warning about it: > > CC [M] /root/ovs/datapath/linux/vport-internal_dev.o > /root/ovs/datapath/linux/vport-internal_dev.c:92:12: warning: > ‘internal_dev_change_mtu’ defined but not used [-Wunused-function] > static int internal_dev_change_mtu(struct net_device *dev, int new_mtu) > > Can we have the same conditional [0] around that function definition > to avoid such warning ? > Sure, I add that condition to avoid the warning in v2. https://patchwork.ozlabs.org/patch/912095/ Thanks, -Yi-Hung
diff --git a/acinclude.m4 b/acinclude.m4 index 60186d33de88..a2444af33c22 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -576,6 +576,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netdevice.h], [net_device], [max_mtu]) + OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netdevice.h], [net_device_ops_extended], + [ndo_change_mtu], [OVS_DEFINE([HAVE_RHEL7_MAX_MTU])]) OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_hook_state]) OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_register_net_hook]) diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c index c08dced3feaf..eacffb2c5b2a 100644 --- a/datapath/linux/compat/geneve.c +++ b/datapath/linux/compat/geneve.c @@ -1271,7 +1271,11 @@ static const struct net_device_ops geneve_netdev_ops = { .ndo_stop = geneve_stop, .ndo_start_xmit = geneve_dev_xmit, .ndo_get_stats64 = ip_tunnel_get_stats64, +#ifdef HAVE_RHEL7_MAX_MTU + .extended.ndo_change_mtu = geneve_change_mtu, +#else .ndo_change_mtu = geneve_change_mtu, +#endif .ndo_validate_addr = eth_validate_addr, .ndo_set_mac_address = eth_mac_addr, #ifdef HAVE_NDO_FILL_METADATA_DST diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c index 4e325914d8be..2f297a548e30 100644 --- a/datapath/linux/compat/ip_gre.c +++ b/datapath/linux/compat/ip_gre.c @@ -491,7 +491,11 @@ static const struct net_device_ops gre_tap_netdev_ops = { .ndo_start_xmit = gre_dev_xmit, .ndo_set_mac_address = eth_mac_addr, .ndo_validate_addr = eth_validate_addr, +#ifdef HAVE_RHEL7_MAX_MTU + .extended.ndo_change_mtu = ip_tunnel_change_mtu, +#else .ndo_change_mtu = ip_tunnel_change_mtu, +#endif #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39) .ndo_get_stats64 = ip_tunnel_get_stats64, #endif diff --git a/datapath/linux/compat/lisp.c b/datapath/linux/compat/lisp.c index 34f82324a7ff..4882a636a33e 100644 --- a/datapath/linux/compat/lisp.c +++ b/datapath/linux/compat/lisp.c @@ -546,7 +546,11 @@ static const struct net_device_ops lisp_netdev_ops = { .ndo_open = lisp_open, .ndo_stop = lisp_stop, .ndo_start_xmit = lisp_dev_xmit, +#ifdef HAVE_RHEL7_MAX_MTU + .extended.ndo_change_mtu = lisp_change_mtu, +#else .ndo_change_mtu = lisp_change_mtu, +#endif .ndo_validate_addr = eth_validate_addr, .ndo_set_mac_address = eth_mac_addr, #ifdef USE_UPSTREAM_TUNNEL diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c index 2189476e176e..ee1c7aa0a78d 100644 --- a/datapath/linux/compat/stt.c +++ b/datapath/linux/compat/stt.c @@ -1857,7 +1857,11 @@ static const struct net_device_ops stt_netdev_ops = { .ndo_stop = stt_stop, .ndo_start_xmit = stt_dev_xmit, .ndo_get_stats64 = ip_tunnel_get_stats64, +#ifdef HAVE_RHEL7_MAX_MTU + .extended.ndo_change_mtu = stt_change_mtu, +#else .ndo_change_mtu = stt_change_mtu, +#endif .ndo_validate_addr = eth_validate_addr, .ndo_set_mac_address = eth_mac_addr, #ifdef USE_UPSTREAM_TUNNEL diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c index 54b953454bf4..40cb12b1329d 100644 --- a/datapath/linux/compat/vxlan.c +++ b/datapath/linux/compat/vxlan.c @@ -1481,7 +1481,11 @@ static const struct net_device_ops vxlan_netdev_ether_ops = { .ndo_start_xmit = vxlan_dev_xmit, .ndo_get_stats64 = ip_tunnel_get_stats64, .ndo_set_rx_mode = vxlan_set_multicast_list, +#ifdef HAVE_RHEL7_MAX_MTU + .extended.ndo_change_mtu = vxlan_change_mtu, +#else .ndo_change_mtu = vxlan_change_mtu, +#endif .ndo_validate_addr = eth_validate_addr, .ndo_set_mac_address = eth_mac_addr, #ifdef HAVE_NDO_FILL_METADATA_DST @@ -1496,7 +1500,11 @@ static const struct net_device_ops vxlan_netdev_raw_ops = { .ndo_stop = vxlan_stop, .ndo_start_xmit = vxlan_dev_xmit, .ndo_get_stats64 = ip_tunnel_get_stats64, +#ifdef HAVE_RHEL7_MAX_MTU + .extended.ndo_change_mtu = vxlan_change_mtu, +#else .ndo_change_mtu = vxlan_change_mtu, +#endif #ifdef HAVE_NDO_FILL_METADATA_DST .ndo_fill_metadata_dst = ovs_vxlan_fill_metadata_dst, #endif diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c index f48684b2e70f..90e76bac2165 100644 --- a/datapath/vport-internal_dev.c +++ b/datapath/vport-internal_dev.c @@ -153,7 +153,7 @@ static const struct net_device_ops internal_dev_netdev_ops = { .ndo_stop = internal_dev_stop, .ndo_start_xmit = internal_dev_xmit, .ndo_set_mac_address = eth_mac_addr, -#ifndef HAVE_NET_DEVICE_WITH_MAX_MTU +#if !defined(HAVE_NET_DEVICE_WITH_MAX_MTU) && !defined(HAVE_RHEL7_MAX_MTU) .ndo_change_mtu = internal_dev_change_mtu, #endif .ndo_get_stats64 = (void *)internal_get_stats,
1) OVS datapath compat modules breaks on RHEL 7.5, because it moves ndo_change_mtu function pointer from 'struct net_device_ops' to 'struct net_device_ops_extended'. 2) RHEL 7.5 introduces the MTU range checking as mentioned in 6c0bf091 ("datapath: use core MTU range checking in core net infra"). However, the max_mtu field is defined in 'struct net_device_extended' but not in 'struct net_device' as upstream kernel. This patch defines a new symbol HAVE_RHEL7_MAX_MTU that determines the previous 2 conditions, and fixes the backport issue. Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> --- acinclude.m4 | 2 ++ datapath/linux/compat/geneve.c | 4 ++++ datapath/linux/compat/ip_gre.c | 4 ++++ datapath/linux/compat/lisp.c | 4 ++++ datapath/linux/compat/stt.c | 4 ++++ datapath/linux/compat/vxlan.c | 8 ++++++++ datapath/vport-internal_dev.c | 2 +- 7 files changed, 27 insertions(+), 1 deletion(-)