Re: [ovs-dev] [PATCHv2] datapath: Add support for kernel 4.19.x and 4.20.x
Thanks Yi-Hung! I will look at it and come up with a new version.
Yifeng
On Mon, Apr 15, 2019 at 5:39 PM Yi-Hung Wei wrote:
>
> On Fri, Apr 12, 2019 at 10:23 AM Yifeng Sun wrote:
> >
> > This patch introduces changes needed by OVS to support latest
> > Linux kernels (4.19.x and 4.20.x). Recent kernels changed many
> > APIs that are being used by OVS. One major change is that
> > struct nf_conntrack_l3proto became invisible outside of kernel, so
> > get_l4proto function is added in file compact/nf_conntrack_core.c to
> > accommodate this issue.
> >
> > In addition, if kernel is not compiled with CONFIG_NF_NAT_IPV4
> > or CONFIG_NF_NAT_IPV6, flow action 'ct(nat)' can cause kernel
> > to crash. This patch handles this condition.
> >
> > This patch doesn't introduce new failed tests when running
> > 'make check-kmod' for kernels listed below:
> > 3.10.0-957.5.1.el7.x86_64
> > 4.4.0-142-generic
> > 4.17.14
> > 4.18.0-16-generic
> > 4.19.34
> > 4.20.17
> >
> > Travis passed at
> > https://travis-ci.org/yifsun/ovs-travis/builds/519011670
> >
> > Signed-off-by: Yifeng Sun
> > v1->v2: Fixed the CONFIG_NF_NAT_IPV4 bug by using Greg's config
> > file. Thanks Greg!
> > ---
> Hi Yifeng,
>
> Thanks for the patch.
>
> I think this patch mixes a couple upstream patches backport, 4.19,
> 4.20 compilation issues, and the nf_nat issue together so that it may
> be hard to keep track of the kernel backport. IMHO, it would be
> easier to break this patch down to a couple of them, so that it would
> be easier to maintain and review. My detailed comments are as below.
>
> > diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> > index 52208bad3029..ce36a8ddea50 100644
> > --- a/datapath/conntrack.c
> > +++ b/datapath/conntrack.c
> > @@ -38,6 +38,10 @@
> > #include
> > #endif
> >
> > +#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6) && defined(HAVE_IPV6_FRAG_H)
> > +#include
> > +#endif
> > +
> I think this is related to an upstream change 70b095c843266 ("ipv6:
> remove dependency of nf_defrag_ipv6 on ipv6 module"). Should we split
> this out in anther patch? We may be able to hide the following in the
> compat layer.
> +#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6) && defined(HAVE_IPV6_FRAG_H)
> +#endif
>
>
> > #include "datapath.h"
> > #include "conntrack.h"
> > #include "flow.h"
> > @@ -645,32 +649,62 @@ static struct nf_conn *
> > ovs_ct_find_existing(struct net *net, const struct nf_conntrack_zone *zone,
> > u8 l3num, struct sk_buff *skb, bool natted)
> > {
> > - const struct nf_conntrack_l3proto *l3proto;
> > const struct nf_conntrack_l4proto *l4proto;
> > struct nf_conntrack_tuple tuple;
> > struct nf_conntrack_tuple_hash *h;
> > struct nf_conn *ct;
> > - unsigned int dataoff;
> > u8 protonum;
> >
> > +#ifdef HAVE_NF_CT_INVERT_TUPLE_TAKES_L3PROTO
> > + const struct nf_conntrack_l3proto *l3proto;
> > + unsigned int dataoff;
> > +
> > l3proto = __nf_ct_l3proto_find(l3num);
> > if (l3proto->get_l4proto(skb, skb_network_offset(skb), &dataoff,
> > &protonum) <= 0) {
> > pr_debug("ovs_ct_find_existing: Can't get protonum\n");
> > return NULL;
> > }
> > +#else
> > + int protooff;
> > +
> > + protooff = get_l4proto(skb, skb_network_offset(skb),
> > + l3num, &protonum);
> > + if (protooff <= 0) {
> > + pr_warn("ovs_ct_find_existing: Can't get protonum\n");
> > + return NULL;
> > + }
> > +#endif
> > +
> > +#ifdef HAVE_NF_CT_L4PROTO_FIND_TAKES_L3PROTO
> > l4proto = __nf_ct_l4proto_find(l3num, protonum);
> > - if (!nf_ct_get_tuple(skb, skb_network_offset(skb), dataoff, l3num,
> > -protonum, net, &tuple, l3proto, l4proto)) {
> > +#else
> > + l4proto = __nf_ct_l4proto_find(protonum);
> > +#endif
> > +
> > +#ifdef HAVE_NF_CT_GET_TUPLE
> > + if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
> > + l3num, net, &tuple)) {
> > + pr_debug("ovs_ct_find_existing: Can't get tuple\n");
> > + return NULL;
> > + }
> > +#else
> > + if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
> > + l3num, net, &tuple)) {
> > pr_debug("ovs_ct_find_existing: Can't get tuple\n");
> > return NULL;
> > }
> > +#endif
> >
> > /* Must invert the tuple if skb has been transformed by NAT. */
> > if (natted) {
> > struct nf_conntrack_tuple inverse;
> >
> > +#ifdef HAVE_NF_CT_INVERT_TUPLE_TAKES_L3PROTO
> > if (!nf_ct_invert_tuple(&inverse, &tuple, l3proto,
> > l4proto)) {
> > +#else
> > + if (!nf_ct_invert_tuple(&inverse, &tuple, l4proto)) {
> > +#endif
> > pr_debug("ovs_ct_find_existing: Inversion
Re: [ovs-dev] [PATCHv2] datapath: Add support for kernel 4.19.x and 4.20.x
On Fri, Apr 12, 2019 at 10:23 AM Yifeng Sun wrote:
>
> This patch introduces changes needed by OVS to support latest
> Linux kernels (4.19.x and 4.20.x). Recent kernels changed many
> APIs that are being used by OVS. One major change is that
> struct nf_conntrack_l3proto became invisible outside of kernel, so
> get_l4proto function is added in file compact/nf_conntrack_core.c to
> accommodate this issue.
>
> In addition, if kernel is not compiled with CONFIG_NF_NAT_IPV4
> or CONFIG_NF_NAT_IPV6, flow action 'ct(nat)' can cause kernel
> to crash. This patch handles this condition.
>
> This patch doesn't introduce new failed tests when running
> 'make check-kmod' for kernels listed below:
> 3.10.0-957.5.1.el7.x86_64
> 4.4.0-142-generic
> 4.17.14
> 4.18.0-16-generic
> 4.19.34
> 4.20.17
>
> Travis passed at
> https://travis-ci.org/yifsun/ovs-travis/builds/519011670
>
> Signed-off-by: Yifeng Sun
> v1->v2: Fixed the CONFIG_NF_NAT_IPV4 bug by using Greg's config
> file. Thanks Greg!
> ---
Hi Yifeng,
Thanks for the patch.
I think this patch mixes a couple upstream patches backport, 4.19,
4.20 compilation issues, and the nf_nat issue together so that it may
be hard to keep track of the kernel backport. IMHO, it would be
easier to break this patch down to a couple of them, so that it would
be easier to maintain and review. My detailed comments are as below.
> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> index 52208bad3029..ce36a8ddea50 100644
> --- a/datapath/conntrack.c
> +++ b/datapath/conntrack.c
> @@ -38,6 +38,10 @@
> #include
> #endif
>
> +#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6) && defined(HAVE_IPV6_FRAG_H)
> +#include
> +#endif
> +
I think this is related to an upstream change 70b095c843266 ("ipv6:
remove dependency of nf_defrag_ipv6 on ipv6 module"). Should we split
this out in anther patch? We may be able to hide the following in the
compat layer.
+#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6) && defined(HAVE_IPV6_FRAG_H)
+#endif
> #include "datapath.h"
> #include "conntrack.h"
> #include "flow.h"
> @@ -645,32 +649,62 @@ static struct nf_conn *
> ovs_ct_find_existing(struct net *net, const struct nf_conntrack_zone *zone,
> u8 l3num, struct sk_buff *skb, bool natted)
> {
> - const struct nf_conntrack_l3proto *l3proto;
> const struct nf_conntrack_l4proto *l4proto;
> struct nf_conntrack_tuple tuple;
> struct nf_conntrack_tuple_hash *h;
> struct nf_conn *ct;
> - unsigned int dataoff;
> u8 protonum;
>
> +#ifdef HAVE_NF_CT_INVERT_TUPLE_TAKES_L3PROTO
> + const struct nf_conntrack_l3proto *l3proto;
> + unsigned int dataoff;
> +
> l3proto = __nf_ct_l3proto_find(l3num);
> if (l3proto->get_l4proto(skb, skb_network_offset(skb), &dataoff,
> &protonum) <= 0) {
> pr_debug("ovs_ct_find_existing: Can't get protonum\n");
> return NULL;
> }
> +#else
> + int protooff;
> +
> + protooff = get_l4proto(skb, skb_network_offset(skb),
> + l3num, &protonum);
> + if (protooff <= 0) {
> + pr_warn("ovs_ct_find_existing: Can't get protonum\n");
> + return NULL;
> + }
> +#endif
> +
> +#ifdef HAVE_NF_CT_L4PROTO_FIND_TAKES_L3PROTO
> l4proto = __nf_ct_l4proto_find(l3num, protonum);
> - if (!nf_ct_get_tuple(skb, skb_network_offset(skb), dataoff, l3num,
> -protonum, net, &tuple, l3proto, l4proto)) {
> +#else
> + l4proto = __nf_ct_l4proto_find(protonum);
> +#endif
> +
> +#ifdef HAVE_NF_CT_GET_TUPLE
> + if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
> + l3num, net, &tuple)) {
> + pr_debug("ovs_ct_find_existing: Can't get tuple\n");
> + return NULL;
> + }
> +#else
> + if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
> + l3num, net, &tuple)) {
> pr_debug("ovs_ct_find_existing: Can't get tuple\n");
> return NULL;
> }
> +#endif
>
> /* Must invert the tuple if skb has been transformed by NAT. */
> if (natted) {
> struct nf_conntrack_tuple inverse;
>
> +#ifdef HAVE_NF_CT_INVERT_TUPLE_TAKES_L3PROTO
> if (!nf_ct_invert_tuple(&inverse, &tuple, l3proto, l4proto)) {
> +#else
> + if (!nf_ct_invert_tuple(&inverse, &tuple, l4proto)) {
> +#endif
> pr_debug("ovs_ct_find_existing: Inversion failed!\n");
> return NULL;
> }
The changes in ovs_ct_find_existing() is due to upstream commit
60e3be94e6a ("openvswitch: use nf_ct_get_tuplepr, invert_tuplepr").
Can we split it out as a separate patch?
>From the upstream commit 60e3be94e6a, instead of using
nf_ct_get_tuple() it invokes nf_ct_get_tuplepr(), and it looks like
nf_ct_get_t
[ovs-dev] [PATCHv2] datapath: Add support for kernel 4.19.x and 4.20.x
This patch introduces changes needed by OVS to support latest Linux kernels (4.19.x and 4.20.x). Recent kernels changed many APIs that are being used by OVS. One major change is that struct nf_conntrack_l3proto became invisible outside of kernel, so get_l4proto function is added in file compact/nf_conntrack_core.c to accommodate this issue. In addition, if kernel is not compiled with CONFIG_NF_NAT_IPV4 or CONFIG_NF_NAT_IPV6, flow action 'ct(nat)' can cause kernel to crash. This patch handles this condition. This patch doesn't introduce new failed tests when running 'make check-kmod' for kernels listed below: 3.10.0-957.5.1.el7.x86_64 4.4.0-142-generic 4.17.14 4.18.0-16-generic 4.19.34 4.20.17 Travis passed at https://travis-ci.org/yifsun/ovs-travis/builds/519011670 Signed-off-by: Yifeng Sun v1->v2: Fixed the CONFIG_NF_NAT_IPV4 bug by using Greg's config file. Thanks Greg! --- .travis.yml| 20 ++--- NEWS | 2 + acinclude.m4 | 20 - datapath/conntrack.c | 86 +- .../include/net/netfilter/nf_conntrack_core.h | 6 ++ .../include/net/netfilter/nf_conntrack_count.h | 2 + datapath/linux/compat/nf_conncount.c | 6 +- datapath/linux/compat/nf_conntrack_core.c | 80 datapath/linux/compat/nf_conntrack_proto.c | 3 + 9 files changed, 209 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index 32d5f1918495..8578d1497f6c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,22 +28,24 @@ before_script: export PATH=$PATH:$HOME/bin env: - OPTS="--disable-ssl" - - TESTSUITE=1 KERNEL=3.16.54 + - TESTSUITE=1 KERNEL=3.16.65 - TESTSUITE=1 OPTS="--enable-shared" - BUILD_ENV="-m32" OPTS="--disable-ssl" - - KERNEL=3.16.54 DPDK=1 OPTS="--enable-shared" - - KERNEL=3.16.54 TESTSUITE=1 DPDK=1 - - KERNEL=3.16.54 DPDK_SHARED=1 - - KERNEL=3.16.54 DPDK_SHARED=1 OPTS="--enable-shared" + - KERNEL=3.16.65 DPDK=1 OPTS="--enable-shared" + - KERNEL=3.16.65 TESTSUITE=1 DPDK=1 + - KERNEL=3.16.65 DPDK_SHARED=1 + - KERNEL=3.16.65 DPDK_SHARED=1 OPTS="--enable-shared" + - KERNEL=4.20.17 + - KERNEL=4.19.34 - KERNEL=4.18.20 - KERNEL=4.17.19 - KERNEL=4.16.18 - KERNEL=4.15.18 - - KERNEL=4.14.63 - - KERNEL=4.9.149 - - KERNEL=4.4.148 + - KERNEL=4.14.111 + - KERNEL=4.9.168 + - KERNEL=4.4.178 - KERNEL=3.19.8 - - KERNEL=3.16.57 + - KERNEL=3.16.65 - TESTSUITE=1 LIBS=-ljemalloc matrix: diff --git a/NEWS b/NEWS index 1e4744dbd244..af5b5222f78f 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,8 @@ Post-v2.11.0 - OVN: * Select IPAM mac_prefix in a random manner if not provided by the user - New QoS type "linux-netem" on Linux. + - Linux datapath: + * Support for the kernel versions 4.19.x, 4.20.x. v2.11.0 - 19 Feb 2019 - diff --git a/acinclude.m4 b/acinclude.m4 index cfc8bcd06397..a2e1f9500955 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -151,10 +151,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [ AC_MSG_RESULT([$kversion]) if test "$version" -ge 4; then - if test "$version" = 4 && test "$patchlevel" -le 18; then + if test "$version" = 4 && test "$patchlevel" -le 20; then : # Linux 4.x else - AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 4.18.x is not supported (please refer to the FAQ for advice)]) + AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 4.20.x is not supported (please refer to the FAQ for advice)]) fi elif test "$version" = 3 && test "$patchlevel" -ge 10; then : # Linux 3.x @@ -590,6 +590,22 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ [OVS_DEFINE([HAVE_VOID_INET_FRAGS_INIT])]) OVS_GREP_IFELSE([$KSRC/include/net/inetpeer.h], [vif], [OVS_DEFINE([HAVE_INETPEER_VIF_SUPPORT])]) + OVS_FIND_PARAM_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_helper.h], +[nf_ct_helper_ext_add], [nf_conntrack_helper], +[OVS_DEFINE([HAVE_NF_CT_HELPER_EXT_ADD_TAKES_HELPER])]) + OVS_FIND_PARAM_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_core.h], +[nf_ct_invert_tuple], [l3proto], +[OVS_DEFINE([HAVE_NF_CT_INVERT_TUPLE_TAKES_L3PROTO])]) + OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_core.h], [nf_ct_get_tuple], + [OVS_DEFINE([HAVE_NF_CT_GET_TUPLE])]) + OVS_FIND_PARAM_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_core.h], +[nf_conntrack_in], [net], +[OVS_DEFINE([HAVE_NF_CONNTRACK_IN_TAKES_NET])]) + OVS_GREP_IFELSE([$KSRC/include/net/ipv6_frag.h], [IP6_DEFRAG_CONNTRACK_IN], + [OVS_DEFINE([HAVE_IPV6_FRAG_H])]) + OVS_FIND_PARAM_
