| Message ID | 1526059932-1470-1-git-send-email-yihung.wei@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] datapath: compat: Fix build on RHEL 7.5 | expand |
Great, thanks for the v2 On Fri, May 11, 2018 at 6:32 PM, Yi-Hung Wei <yihung.wei@gmail.com> 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> Acked-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
On Fri, May 11, 2018 at 10:32 AM, Yi-Hung Wei <yihung.wei@gmail.com> 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> Thanks. I pushed patch to master and 2.9 and 2.8 where the original MTU patch (commit 6c0bf091) is applied.
On Fri, 11 May 2018 10:32:12 -0700, Yi-Hung Wei wrote: > --- 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, Note that this will never be called by the RHEL kernel unless you also set .ndo_size to sizeof(struct net_device_ops). In other words, you've just effectively set .ndo_change_mtu to NULL. Jiri
On Thu, May 17, 2018 at 4:09 AM, Jiri Benc <jbenc@redhat.com> wrote: > On Fri, 11 May 2018 10:32:12 -0700, Yi-Hung Wei wrote: >> --- 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, > > Note that this will never be called by the RHEL kernel unless you > also set .ndo_size to sizeof(struct net_device_ops). > > In other words, you've just effectively set .ndo_change_mtu to NULL. > > Jiri Thanks for bringing up this issue. I submitted a fix here: https://patchwork.ozlabs.org/patch/915729/ 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..3cb8d06b2475 100644 --- a/datapath/vport-internal_dev.c +++ b/datapath/vport-internal_dev.c @@ -88,7 +88,7 @@ static const struct ethtool_ops internal_dev_ethtool_ops = { .get_link = ethtool_op_get_link, }; -#ifndef HAVE_NET_DEVICE_WITH_MAX_MTU +#if !defined(HAVE_NET_DEVICE_WITH_MAX_MTU) && !defined(HAVE_RHEL7_MAX_MTU) static int internal_dev_change_mtu(struct net_device *dev, int new_mtu) { if (new_mtu < ETH_MIN_MTU) { @@ -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 | 4 ++-- 7 files changed, 28 insertions(+), 2 deletions(-)