[ovs-dev] [PATCH] compat: Fix broken partial backport of extack op parameter

Gregory Rose gvrose8192 at gmail.com
Tue Apr 14 18:32:25 UTC 2020



On 4/14/2020 9:52 AM, Greg Rose wrote:
> A series of commits added support for the extended ack
> parameter to the newlink, changelink and validate ops in
> the rtnl_link_ops structure:
> a8b8a889e369d ("net: add netlink_ext_ack argument to rtnl_link_ops.validate")
> 7a3f4a185169b ("net: add netlink_ext_ack argument to rtnl_link_ops.newlink")
> ad744b223c521 ("net: add netlink_ext_ack argument to rtnl_link_ops.changelink")
> 
> These commits were all added at the same time and present since the
> Linux kernel 4.13 release. In our compatiblity layer we have a
> define HAVE_EXT_ACK_IN_RTNL_LINKOPS that indicates the presence of
> the extended ack parameter for these three link operations.
> 
> At least one distro has only backported two of the three patches,
> for newlink and changelink, while not backporting patch a8b8a889e369d
> for the validate op.  Our compatibility layer code in acinclude.m4
> is able to find the presence of the extack within the rtnl_link_ops
> structure so it defines HAVE_EXT_ACK_IN_RTNL_LINKOPS but since the
> validate link op does not have the extack parameter the compilation
> fails on recent kernels for that particular distro. Other kernel
> distributions based upon this distro will presumably also encounter
> the compile errors.
> 
> Introduce a new function in acinclude.m4 that will find function
> op definitions and then search for the required parameter.  Then
> use this function to define HAVE_RTNLOP_VALIDATE_WITH_EXTACK so
> that we can detect and enable correct compilation on kernels
> which have not backported the entire set of patches.  This function
> is generic to any function op - it need not be in a structure.
> 
> In places where HAVE_EXT_ACK_IN_RTNL_LINKOPS wraps validate functions
> replace it with the new HAVE_RTNLOP_VALIDATE_WITH_EXTACK define.
> 
> Passes Travis here:
> https://travis-ci.org/github/gvrose8192/ovs-experimental/builds/674599698
> 
> Passes a kernel check-kmod test on several systems, including
> sles12 sp4 4.12.14-95.48-default kernel, without any regressions.
> 
> Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
> ---
>   acinclude.m4                       | 34 ++++++++++++++++++++++++++++++++++
>   datapath/linux/compat/geneve.c     |  2 +-
>   datapath/linux/compat/ip6_gre.c    | 10 +++++-----
>   datapath/linux/compat/ip6_tunnel.c |  2 +-
>   datapath/linux/compat/ip_gre.c     | 10 +++++-----
>   datapath/linux/compat/lisp.c       |  2 +-
>   datapath/linux/compat/stt.c        |  2 +-
>   datapath/linux/compat/vxlan.c      |  2 +-
>   8 files changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 02efea6..1089f41 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -520,6 +520,37 @@ AC_DEFUN([OVS_FIND_PARAM_IFELSE], [
>     fi
>   ])
>   
> +dnl OVS_FIND_OP_PARAM_IFELSE(FILE, OP, REGEX, [IF-MATCH], [IF-NO-MATCH])
> +dnl
> +dnl Looks for OP in FILE. If it is found, greps for REGEX within the
> +dnl OP definition. If this is successful, runs IF-MATCH, otherwise
> +dnl IF_NO_MATCH. If IF-MATCH is empty then it defines to
> +dnl OVS_DEFINE(HAVE_<OP>_WITH_<REGEX>), with <STRUCTURE> and <REGEX>

I was just looking at this again because I forgot to include the VMware
BZ # and then I noticed that <STRUCTURE> doesn't belong here either.

V2 forthcoming, please ignore this one.

- Greg

> +dnl translated to uppercase.
> +AC_DEFUN([OVS_FIND_OP_PARAM_IFELSE], [
> +  AC_MSG_CHECKING([whether $2 has member $3 in $1])
> +  if test -f $1; then
> +    awk '/$2[[ \t\n]]*\)\(/,/;/' $1 2>/dev/null | grep '$3' >/dev/null
> +    status=$?
> +    case $status in
> +      0)
> +        AC_MSG_RESULT([yes])
> +        m4_if([$4], [], [OVS_DEFINE([HAVE_]m4_toupper([$2])[_WITH_]m4_toupper([$3]))], [$4])
> +        ;;
> +      1)
> +        AC_MSG_RESULT([no])
> +        $5
> +        ;;
> +      *)
> +        AC_MSG_ERROR([grep exited with status $status])
> +        ;;
> +    esac
> +  else
> +    AC_MSG_RESULT([file not found])
> +    $5
> +  fi
> +])
> +
>   dnl OVS_DEFINE(NAME)
>   dnl
>   dnl Defines NAME to 1 in kcompat.h.
> @@ -1056,6 +1087,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>     OVS_GREP_IFELSE([$KSRC/include/net/netlink.h],
>                     [nla_parse_deprecated_strict],
>                     [OVS_DEFINE([HAVE_NLA_PARSE_DEPRECATED_STRICT])])
> +  OVS_FIND_OP_PARAM_IFELSE([$KSRC/include/net/rtnetlink.h],
> +                           [validate], [extack],
> +                           [OVS_DEFINE([HAVE_RTNLOP_VALIDATE_WITH_EXTACK])])
>   
>     if cmp -s datapath/linux/kcompat.h.new \
>               datapath/linux/kcompat.h >/dev/null 2>&1; then
> diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
> index 5b18396..1551a37 100644
> --- a/datapath/linux/compat/geneve.c
> +++ b/datapath/linux/compat/geneve.c
> @@ -1419,7 +1419,7 @@ static const struct nla_policy geneve_policy[IFLA_GENEVE_MAX + 1] = {
>   	[IFLA_GENEVE_UDP_ZERO_CSUM6_RX]	= { .type = NLA_U8 },
>   };
>   
> -#ifdef  HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef  HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int geneve_validate(struct nlattr *tb[], struct nlattr *data[],
>   			   struct netlink_ext_ack *extack)
>   #else
> diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
> index da0fa43..3aa9844 100644
> --- a/datapath/linux/compat/ip6_gre.c
> +++ b/datapath/linux/compat/ip6_gre.c
> @@ -1687,7 +1687,7 @@ static struct pernet_operations ip6gre_net_ops = {
>   	.id   = &ip6gre_net_id,
>   	.size = sizeof(struct ip6gre_net),
>   };
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int rpl_ip6gre_tunnel_validate(struct nlattr *tb[],
>   				      struct nlattr *data[],
>   				      struct netlink_ext_ack *extack)
> @@ -1713,7 +1713,7 @@ static int rpl_ip6gre_tunnel_validate(struct nlattr *tb[],
>   }
>   #define ip6gre_tunnel_validate rpl_ip6gre_tunnel_validate
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int rpl_ip6gre_tap_validate(struct nlattr *tb[], struct nlattr *data[],
>   				   struct netlink_ext_ack *extack)
>   #else
> @@ -1739,7 +1739,7 @@ static int rpl_ip6gre_tap_validate(struct nlattr *tb[], struct nlattr *data[])
>   	}
>   
>   out:
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   	return ip6gre_tunnel_validate(tb, data, extack);
>   #else
>   	return ip6gre_tunnel_validate(tb, data);
> @@ -1747,7 +1747,7 @@ out:
>   }
>   #define ip6gre_tap_validate rpl_ip6gre_tap_validate
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int rpl_ip6erspan_tap_validate(struct nlattr *tb[],
>   				      struct nlattr *data[],
>   				      struct netlink_ext_ack *extack)
> @@ -1762,7 +1762,7 @@ static int rpl_ip6erspan_tap_validate(struct nlattr *tb[],
>   	if (!data)
>   		return 0;
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   	ret = ip6gre_tap_validate(tb, data, extack);
>   #else
>   	ret = ip6gre_tap_validate(tb, data);
> diff --git a/datapath/linux/compat/ip6_tunnel.c b/datapath/linux/compat/ip6_tunnel.c
> index 9f4bae7..984a51b 100644
> --- a/datapath/linux/compat/ip6_tunnel.c
> +++ b/datapath/linux/compat/ip6_tunnel.c
> @@ -1754,7 +1754,7 @@ static int __net_init ip6_fb_tnl_dev_init(struct net_device *dev)
>   	return 0;
>   }
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int rpl_ip6_tnl_validate(struct nlattr *tb[], struct nlattr *data[],
>   				struct netlink_ext_ack *extack)
>   #else
> diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c
> index 41379b1..c194ffe 100644
> --- a/datapath/linux/compat/ip_gre.c
> +++ b/datapath/linux/compat/ip_gre.c
> @@ -623,7 +623,7 @@ static const struct gre_protocol ipgre_protocol = {
>   	.err_handler = __gre_err,
>   };
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int ipgre_tunnel_validate(struct nlattr *tb[], struct nlattr *data[],
>   				 struct netlink_ext_ack *extack)
>   #else
> @@ -646,7 +646,7 @@ static int ipgre_tunnel_validate(struct nlattr *tb[], struct nlattr *data[])
>   	return 0;
>   }
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int ipgre_tap_validate(struct nlattr *tb[], struct nlattr *data[],
>   			      struct netlink_ext_ack *extack)
>   #else
> @@ -672,7 +672,7 @@ static int ipgre_tap_validate(struct nlattr *tb[], struct nlattr *data[])
>   	}
>   
>   out:
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   	return ipgre_tunnel_validate(tb, data, NULL);
>   #else
>   	return ipgre_tunnel_validate(tb, data);
> @@ -707,7 +707,7 @@ enum {
>   
>   #define RPL_IFLA_GRE_MAX (IFLA_GRE_ERSPAN_HWID + 1)
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int erspan_validate(struct nlattr *tb[], struct nlattr *data[],
>   			   struct netlink_ext_ack *extack)
>   #else
> @@ -720,7 +720,7 @@ static int erspan_validate(struct nlattr *tb[], struct nlattr *data[])
>   	if (!data)
>   		return 0;
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   	ret = ipgre_tap_validate(tb, data, NULL);
>   #else
>   	ret = ipgre_tap_validate(tb, data);
> diff --git a/datapath/linux/compat/lisp.c b/datapath/linux/compat/lisp.c
> index 58144ad..6dc066d 100644
> --- a/datapath/linux/compat/lisp.c
> +++ b/datapath/linux/compat/lisp.c
> @@ -612,7 +612,7 @@ static const struct nla_policy lisp_policy[IFLA_LISP_MAX + 1] = {
>   	[IFLA_LISP_PORT]              = { .type = NLA_U16 },
>   };
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int lisp_validate(struct nlattr *tb[], struct nlattr *data[],
>   			 struct netlink_ext_ack __always_unused *extack)
>   #else
> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
> index 8a5853f..39a2947 100644
> --- a/datapath/linux/compat/stt.c
> +++ b/datapath/linux/compat/stt.c
> @@ -1904,7 +1904,7 @@ static const struct nla_policy stt_policy[IFLA_STT_MAX + 1] = {
>   	[IFLA_STT_PORT]              = { .type = NLA_U16 },
>   };
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int stt_validate(struct nlattr *tb[], struct nlattr *data[],
>   			struct netlink_ext_ack __always_unused *extack)
>   #else
> diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
> index 6090f42..f8f667e 100644
> --- a/datapath/linux/compat/vxlan.c
> +++ b/datapath/linux/compat/vxlan.c
> @@ -1708,7 +1708,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
>   	[IFLA_VXLAN_REMCSUM_NOPARTIAL]	= { .type = NLA_FLAG },
>   };
>   
> -#ifdef HAVE_EXT_ACK_IN_RTNL_LINKOPS
> +#ifdef HAVE_RTNLOP_VALIDATE_WITH_EXTACK
>   static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
>   			  struct netlink_ext_ack *extack)
>   #else
> 


More information about the dev mailing list