On Thu, 11 Oct 2018 at 03:09, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
Hi Pablo,
Thank you for review!
> On Tue, Oct 09, 2018 at 02:59:48AM +0900, Taehee Yoo wrote:
> > When interface is down, offload cleanup function(nf_flow_table_do_cleanup)
> > is called and that checks whether interface index of offload and
> > index of link down interface is same. but only interface index checking
> > is not enough because flowtable is not pernet list.
> > So that, if other netns's interface that has index is same with offload
> > is down, that offload will be removed.
> > This patch adds netns checking code to the offload cleanup routine.
> > And it also removes unnecessary parameter of nf_flow_table_cleanup().
> >
> > Fixes: 59c466dd68e7 ("netfilter: nf_flow_table: add a new flow state for tearing down offloading")
> > Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx>
> > ---
> > include/net/netfilter/nf_flow_table.h | 2 +-
> > net/netfilter/nf_flow_table_core.c | 10 +++++++---
> > net/netfilter/nft_flow_offload.c | 2 +-
> > 3 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
> > index 0e355f4a3d76..77e2761d4f2f 100644
> > --- a/include/net/netfilter/nf_flow_table.h
> > +++ b/include/net/netfilter/nf_flow_table.h
> > @@ -99,7 +99,7 @@ int nf_flow_table_iterate(struct nf_flowtable *flow_table,
> > void (*iter)(struct flow_offload *flow, void *data),
> > void *data);
> >
> > -void nf_flow_table_cleanup(struct net *net, struct net_device *dev);
> > +void nf_flow_table_cleanup(struct net_device *dev);
> >
> > int nf_flow_table_init(struct nf_flowtable *flow_table);
> > void nf_flow_table_free(struct nf_flowtable *flow_table);
> > diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> > index d8125616edc7..88aae0ae499c 100644
> > --- a/net/netfilter/nf_flow_table_core.c
> > +++ b/net/netfilter/nf_flow_table_core.c
> > @@ -478,14 +478,18 @@ EXPORT_SYMBOL_GPL(nf_flow_table_init);
> > static void nf_flow_table_do_cleanup(struct flow_offload *flow, void *data)
> > {
> > struct net_device *dev = data;
> > + struct flow_offload_entry *e;
> > +
> > + e = container_of(flow, struct flow_offload_entry, flow);
> >
> > if (!dev) {
> > flow_offload_teardown(flow);
> > return;
> > }
> >
> > - if (flow->tuplehash[0].tuple.iifidx == dev->ifindex ||
> > - flow->tuplehash[1].tuple.iifidx == dev->ifindex)
> > + if (net_eq(nf_ct_net(e->ct), dev_net(dev)) &&
> > + (flow->tuplehash[0].tuple.iifidx == dev->ifindex ||
> > + flow->tuplehash[1].tuple.iifidx == dev->ifindex))
> > flow_offload_dead(flow);
> > }
> >
>
> These two chunks below doesn't belong here. I'd prefer this goes
> in a separated patch for nf-next.
>
I agree with that
I will send separate two patches for nf and nf-next.
Thanks!
> Thanks.
>
> > @@ -496,7 +500,7 @@ static void nf_flow_table_iterate_cleanup(struct nf_flowtable *flowtable,
> > flush_delayed_work(&flowtable->gc_work);
> > }
> >
> > -void nf_flow_table_cleanup(struct net *net, struct net_device *dev)
> > +void nf_flow_table_cleanup(struct net_device *dev)
> > {
> > struct nf_flowtable *flowtable;
> >
> > diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> > index d6bab8c3cbb0..e82d9a966c45 100644
> > --- a/net/netfilter/nft_flow_offload.c
> > +++ b/net/netfilter/nft_flow_offload.c
> > @@ -201,7 +201,7 @@ static int flow_offload_netdev_event(struct notifier_block *this,
> > if (event != NETDEV_DOWN)
> > return NOTIFY_DONE;
> >
> > - nf_flow_table_cleanup(dev_net(dev), dev);
> > + nf_flow_table_cleanup(dev);
> >
> > return NOTIFY_DONE;
> > }
> > --
> > 2.17.1
> >
