On Sat, 27 Apr 2019 at 04:02, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
> On Sat, Apr 27, 2019 at 02:48:15AM +0900, Taehee Yoo wrote:
> > On Sat, 27 Apr 2019 at 02:03, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > >
> >
> > Hi Pablo!
> > Thank you for the review!
> >
> > > On Sat, Apr 27, 2019 at 01:53:27AM +0900, Taehee Yoo wrote:
> > > > nft_flow_offload_eval() makes flow_offload session but it doesn't check
> > > > tcp state.
> > > > So, it can make un-ESTABLISHED tcp flow offload session such as SYN-RECV.
> > > > But, this is not a normal case.
> > > >
> > > > Fixes: a3c90f7a2323 ("netfilter: nf_tables: flow offload expression")
> > > > Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx>
> > > > ---
> > > > net/netfilter/nft_flow_offload.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> > > > index ff50bc1b144f..8538ddf9c6bf 100644
> > > > --- a/net/netfilter/nft_flow_offload.c
> > > > +++ b/net/netfilter/nft_flow_offload.c
> > > > @@ -84,6 +84,9 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
> > > >
> > > > switch (ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum) {
> > > > case IPPROTO_TCP:
> > > > + if (ct->proto.tcp.state != TCP_CONNTRACK_ESTABLISHED)
> > > > + goto out;
> > >
> > > You can restrict this via policy:
> > >
> > > ct status assured flow add @x
> > >
> >
> > I think If we allow to make non-ESTABLISHED flow offload,
> > teardown code should be changed because teardown changes TCP status to
> > ESTABLISHED. How do you think about it?
>
> Hm, right, the teardown fixup routine cannot then bogusly assumes the
> TCP established state if we create a TCP entry from the first packet,
> ie. the SYN_SENT state...
>
Thank you for sharing opinion.
> [...]
> > > And this would fix the flowtable infrastructure UDP traffic that only
> > > goes in one direction.
> > >
> > > We can document in the manpage a few examples for this.
> >
> > I agree that flowtable infrastructure provides UDP one direction flow
> > offloading.
>
> OK, probably I can collapse your patch to mine. Let me think over the
> weekend.
Okay, I will wait for your response!
