[ovs-dev] [PATCH V2] compat: Fix broken partial backport of extack op parameter
Yifeng Sun
pkusunyifeng at gmail.com
Wed Apr 15 00:45:14 UTC 2020
LGTM, thanks Greg.
Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
On Tue, Apr 14, 2020 at 11:42 AM Greg Rose <gvrose8192 at gmail.com> 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.
>
> VMWare-BZ: #2544032
>
> Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
>
> ---
> V2 - Fix comment for OVS_FIND_OP_PARAM_IFELSE function and don't
> forget the VMWare BZ #
> ---
> 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..0901f28 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 <OP> and <REGEX>
> +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
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list