LGTM, thanks Greg. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com>
On Tue, Apr 14, 2020 at 11:42 AM Greg Rose <gvrose8...@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 <gvrose8...@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 > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev