[PATCH net-next] openvswitch: add seqadj extension when NAT is used.

[PATCH net-next] openvswitch: add seqadj extension when NAT is used.

[Flavio Leitner]

When the conntrack is initialized, there is no helper attached
yet so the nat info initialization (nf_nat_setup_info) skips
adding the seqadj ext.

A helper is attached later when the conntrack is not confirmed
but is going to be committed. In this case, if NAT is needed then
adds the seqadj ext as well.

Fixes: 16ec3d4fbb96 ("openvswitch: Fix cached ct with helper.")
Signed-off-by: Flavio Leitner <fbl@sysclose.org>
---
 net/openvswitch/conntrack.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 1b6896896fff..a7664515c943 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -990,6 +990,11 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
 							    GFP_ATOMIC);
 			if (err)
 				return err;
+
+			if (info->nat && nfct_help(ct) && !nfct_seqadj(ct)) {
+				if (!nfct_seqadj_ext_add(ct))
+					return -EINVAL;
+			}
 		}
 
 		/* Call the helper only if:
-- 
2.20.1

Re: [PATCH net-next] openvswitch: add seqadj extension when NAT is used.

[Pravin Shelar]

On Thu, Mar 21, 2019 at 9:52 AM Flavio Leitner <fbl@sysclose.org> wrote:
>
> When the conntrack is initialized, there is no helper attached
> yet so the nat info initialization (nf_nat_setup_info) skips
> adding the seqadj ext.
>
> A helper is attached later when the conntrack is not confirmed
> but is going to be committed. In this case, if NAT is needed then
> adds the seqadj ext as well.
>
> Fixes: 16ec3d4fbb96 ("openvswitch: Fix cached ct with helper.")
> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> ---
>  net/openvswitch/conntrack.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
I am not able to apply this patch.

> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 1b6896896fff..a7664515c943 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -990,6 +990,11 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
>                                                             GFP_ATOMIC);
>                         if (err)
>                                 return err;
> +
> +                       if (info->nat && nfct_help(ct) && !nfct_seqadj(ct)) {
Given helper is just assigned, is nfct_help() check required here?

> +                               if (!nfct_seqadj_ext_add(ct))
> +                                       return -EINVAL;
> +                       }
>                 }
>
>                 /* Call the helper only if:
> --
> 2.20.1
>
>
>

Re: [PATCH net-next] openvswitch: add seqadj extension when NAT is used.

[Flavio Leitner]

On Sat, Mar 23, 2019 at 12:32:37PM -0700, Pravin Shelar wrote:
> On Thu, Mar 21, 2019 at 9:52 AM Flavio Leitner <fbl@sysclose.org> wrote:
> >
> > When the conntrack is initialized, there is no helper attached
> > yet so the nat info initialization (nf_nat_setup_info) skips
> > adding the seqadj ext.
> >
> > A helper is attached later when the conntrack is not confirmed
> > but is going to be committed. In this case, if NAT is needed then
> > adds the seqadj ext as well.
> >
> > Fixes: 16ec3d4fbb96 ("openvswitch: Fix cached ct with helper.")
> > Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> > ---
> >  net/openvswitch/conntrack.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> I am not able to apply this patch.

This is for Davem net-next. I got the patch from ML and it worked for
me.


> > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> > index 1b6896896fff..a7664515c943 100644
> > --- a/net/openvswitch/conntrack.c
> > +++ b/net/openvswitch/conntrack.c
> > @@ -990,6 +990,11 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
> >                                                             GFP_ATOMIC);
> >                         if (err)
> >                                 return err;
> > +
> > +                       if (info->nat && nfct_help(ct) && !nfct_seqadj(ct)) {
> Given helper is just assigned, is nfct_help() check required here?

I tried to be very clear, but you're right that it's not required.
I will post a v2 removing that.
Thanks
fbl

> 
> > +                               if (!nfct_seqadj_ext_add(ct))
> > +                                       return -EINVAL;
> > +                       }
> >                 }
> >
> >                 /* Call the helper only if:
> > --
> > 2.20.1
> >
> >
> >