Re: [PATCH] netfilter: nf_flow_table: count offloaded flows

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 26, 2023 at 01:28:17PM +0100, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Sun, 26 Feb 2023, Sven Auhagen wrote:
> 
> > On Sun, Feb 26, 2023 at 10:53:58AM +0200, Julian Anastasov wrote:
> > > 
> > > > diff --git a/net/netfilter/nf_flow_table_procfs.c b/net/netfilter/nf_flow_table_procfs.c
> > > > index 159b033a43e6..cff9ad58c7c9 100644
> > > > --- a/net/netfilter/nf_flow_table_procfs.c
> > > > +++ b/net/netfilter/nf_flow_table_procfs.c
> > > > @@ -64,6 +64,16 @@ static const struct seq_operations nf_flow_table_cpu_seq_ops = {
> > > >  	.show	= nf_flow_table_cpu_seq_show,
> > > >  };
> > > >  
> > > > +static int nf_flow_table_counter_show(struct seq_file *seq, void *v)
> > > > +{
> > > > +	struct net *net = seq_file_net(seq);
> > > > +
> > > > +	seq_printf(seq, "%lld\n",
> > > > +		   atomic64_read(&net->ft.count_flowoffload)
> > > > +		);
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  int nf_flow_table_init_proc(struct net *net)
> > > >  {
> > > >  	struct proc_dir_entry *pde;
> > > > @@ -71,6 +81,9 @@ int nf_flow_table_init_proc(struct net *net)
> > > >  	pde = proc_create_net("nf_flowtable", 0444, net->proc_net_stat,
> > > >  			      &nf_flow_table_cpu_seq_ops,
> > > >  			      sizeof(struct seq_net_private));
> > > 
> > > 	Result should be checked, pde is not needed:
> > > 
> > 
> > pde is needed to free the per cpu stat structure in nf_flow_table_fini_net
> 
> 	Nothing is needed to add in nf_flow_table_fini_net(),
> only in nf_flow_table_fini_proc() which is called by
> nf_flow_table_pernet_exit().
> 
> > in the error case. This was implemented before this patch though.
> > I understand that some kind of error check would be good.
> > Since there is no consequence when this does not work I did not implement one
> > and let the old error checking go along.
> 
> 	The checking tools will warn for such leaks, so
> every module should cleanup everything on init error.
> 
> > Do you have any ideas how to solve the problem more?
> 
> 	Nothing more is needed except the missing
> remove_proc_entry("nf_flowtable_counter", net->proc_net_stat);
> in nf_flow_table_fini_proc(). The var 'pde' is not needed.
> On error in nf_flow_table_init_proc() it should free everything
> allocated there and nf_flow_table_fini_proc() should free
> everything that is allocated in nf_flow_table_init_proc().

Got it and you are correct about nf_flow_table_fini_proc but it is also
called from nf_flow_table_pernet_init in case of a nf_flow_table_init_proc
error which is checking the return of the pde variable.
That what I was referring to.

I will send a v2 with your proposed changes.

> 
> > Best
> > Sven
> > 
> > > 	if (!proc_create_net())
> > > 		goto err;
> > > 
> > > > +	proc_create_net_single("nf_flowtable_counter", 0444,
> > > > +			net->proc_net, nf_flow_table_counter_show, NULL);
> > > 
> > > 	if (!proc_create_net_single())
> > > 		goto err_net;
> > > 	return 0;
> > > 
> > > err_net:
> > > 	remove_proc_entry("nf_flowtable", net->proc_net_stat);
> > > 
> > > err:
> > > 	return -ENOMEM;
> > > 
> > > > +
> > > >  	return pde ? 0 : -ENOMEM;
> > > >  }
> 
> Regards
> 
> --
> Julian Anastasov <ja@xxxxxx>
> 



[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux