[ovs-dev] [PATCH 5/7] openvswitch: use nf_ct_get_tuplepr, invert_tuplepr
Yifeng Sun
pkusunyifeng at gmail.com
Wed May 8 23:27:45 UTC 2019
Thanks for reviewing, please see my acks in below.
Yifeng
On Wed, May 8, 2019 at 2:19 PM Yi-Hung Wei <yihung.wei at gmail.com> wrote:
>
> On Mon, May 6, 2019 at 2:59 PM Yifeng Sun <pkusunyifeng at gmail.com> wrote:
> >
> > From: Florian Westphal <fw at strlen.de>
> >
> > Upstream commit:
> > commit 60e3be94e6a1c5162a0763c9aafb5190b2b1fdce
> > Author: Florian Westphal <fw at strlen.de>
> > Date: Mon Jun 25 17:55:32 2018 +0200
> >
> > openvswitch: use nf_ct_get_tuplepr, invert_tuplepr
> >
> > These versions deal with the l3proto/l4proto details internally.
> > It removes only caller of nf_ct_get_tuple, so make it static.
> >
> > After this, l3proto->get_l4proto() can be removed in a followup patch.
> >
> > Signed-off-by: Florian Westphal <fw at strlen.de>
> > Acked-by: Pravin B Shelar <pshelar at ovn.org>
> > Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
> >
> > This patch backports the above upstream kernel patch to OVS.
> >
> > Cc: Florian Westphal <fw at strlen.de>
> > Signed-off-by: Yifeng Sun <pkusunyifeng at gmail.com>
> > ---
> > acinclude.m4 | 2 ++
> > datapath/conntrack.c | 17 +++--------------
> > .../include/net/netfilter/nf_conntrack_core.h | 19 +++++++++++++++++++
> > datapath/linux/compat/nf_conntrack_core.c | 21 +++++++++++++++++++++
> > 4 files changed, 45 insertions(+), 14 deletions(-)
> >
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 4f9aebc325ba..4c533bb98949 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -936,6 +936,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
> > [OVS_DEFINE([HAVE_NF_CONNTRACK_IN_TAKES_NF_HOOK_STATE])])
> > OVS_GREP_IFELSE([$KSRC/include/net/ipv6_frag.h], [IP6_DEFRAG_CONNTRACK_IN],
> > [OVS_DEFINE([HAVE_IPV6_FRAG_H])])
> > + OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack.h],
> > + [nf_ct_invert_tuplepr])
> >
> > if cmp -s datapath/linux/kcompat.h.new \
> > datapath/linux/kcompat.h >/dev/null 2>&1; then
> > diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> > index 52825a6b20fb..391755c25cb0 100644
> > --- a/datapath/conntrack.c
> > +++ b/datapath/conntrack.c
> > @@ -646,23 +646,12 @@ 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;
> >
> > - 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;
> > - }
> > - l4proto = __nf_ct_l4proto_find(l3num, protonum);
> > - if (!nf_ct_get_tuple(skb, skb_network_offset(skb), dataoff, l3num,
> > - protonum, net, &tuple, l3proto, l4proto)) {
> > + 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;
> > }
> > @@ -671,7 +660,7 @@ ovs_ct_find_existing(struct net *net, const struct nf_conntrack_zone *zone,
> > if (natted) {
> > struct nf_conntrack_tuple inverse;
> >
> > - if (!nf_ct_invert_tuple(&inverse, &tuple, l3proto, l4proto)) {
> > + if (!nf_ct_invert_tuplepr(&inverse, &tuple, l3num, skb)) {
> The upstream patch 60e3be94e6a ("openvswitch: use nf_ct_get_tuplepr,
> invert_tuplepr") only gives two parameters for
> nf_ct_invert_tuplepr(). Any reason why we provide 4 parameters for it?
The issue here is that for kernels without nf_ct_invert_tuplepr(), two
parameters
of nf_ct_invert_tuplepr() are not enough to implement this invert
functionality, as
see in rpl_nf_ct_invert_tuplepr() below. It is considered a compromise. Do you
have any good idea?
>
> @@ -632,7 +621,7 @@ ovs_ct_find_existing(struct net *net, const struct
> nf_conntrack_zone *zone,
> if (natted) {
> struct nf_conntrack_tuple inverse;
>
> - if (!nf_ct_invert_tuple(&inverse, &tuple, l3proto, l4proto)) {
> + if (!nf_ct_invert_tuplepr(&inverse, &tuple)) {
> pr_debug("ovs_ct_find_existing: Inversion failed!\n");
> return NULL;
> }
>
>
>
> > pr_debug("ovs_ct_find_existing: Inversion failed!\n");
> > return NULL;
> > }
> > diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
> > index d0e9aadcba76..8eba1242042b 100644
> > --- a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
> > +++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
> > @@ -3,6 +3,25 @@
> >
> > #include_next <net/netfilter/nf_conntrack_core.h>
> >
> > +#ifdef HAVE_NF_CT_INVERT_TUPLEPR
> > +
> > +#include <net/netfilter/nf_conntrack.h>
> > +
> > +static inline bool
> > +rpl_nf_ct_invert_tuplepr(struct nf_conntrack_tuple *inverse,
> > + const struct nf_conntrack_tuple *orig,
> > + u8 l3num, struct sk_buff *skb)
> > +{
> > + return nf_ct_invert_tuplepr(inverse, orig);
> > +}
> > +#else
> > +bool
> > +rpl_nf_ct_invert_tuplepr(struct nf_conntrack_tuple *inverse,
> > + const struct nf_conntrack_tuple *orig,
> > + u8 l3num, struct sk_buff *skb);
> > +#endif /* HAVE_NF_CT_INVERT_TUPLEPR */
> > +#define nf_ct_invert_tuplepr rpl_nf_ct_invert_tuplepr
> > +
> AFAIK, nf_ct_invert_tuplepr() was introduced in quite old kernel, and
> OVS kernel datapath only supports back to 3.10 kernel. I am not sure
> if these backports is required given that we provide the correct 2
> parameters? Can you check on that?
After checking, nf_ct_invert_tuplepr() is being used here by kernel 4.19, 4.20
and 5.0.
>
> Thanks,
>
> -Yi-Hung
>
>
> > #ifndef HAVE_NF_CT_TMPL_ALLOC_TAKES_STRUCT_ZONE
> >
> > #include <net/netfilter/nf_conntrack_zones.h>
> > diff --git a/datapath/linux/compat/nf_conntrack_core.c b/datapath/linux/compat/nf_conntrack_core.c
> > index a7d3d4331e4a..397a5f69ffe9 100644
> > --- a/datapath/linux/compat/nf_conntrack_core.c
> > +++ b/datapath/linux/compat/nf_conntrack_core.c
> > @@ -11,3 +11,24 @@ const struct nf_conntrack_zone nf_ct_zone_dflt = {
> > };
> >
> > #endif /* HAVE_NF_CT_ZONE_INIT */
> > +
> > +#ifndef HAVE_NF_CT_INVERT_TUPLEPR
> > +bool
> > +rpl_nf_ct_invert_tuplepr(struct nf_conntrack_tuple *inverse,
> > + const struct nf_conntrack_tuple *orig,
> > + u8 l3num, struct sk_buff *skb)
> > +{
> > + const struct nf_conntrack_l3proto *l3proto;
> > + const struct nf_conntrack_l4proto *l4proto;
> > + u8 protonum;
> > +
> > + 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 false;
> > + }
> > + l4proto = __nf_ct_l4proto_find(l3num, protonum);
> > + return nf_ct_invert_tuple(&inverse, &tuple, l3proto, l4proto);
> > +}
> > +#endif /* HAVE_NF_CT_INVERT_TUPLEPR */
> > --
> > 2.7.4
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list