Re: [PATCH nf-next 1/3] netfilter: conntrack: export nf_ct_acct_update()

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Mar 25, 2020 at 10:59:14AM +0800, wenxu wrote:
> 
> On 3/25/2020 1:50 AM, Pablo Neira Ayuso wrote:
> > This function allows you to update the conntrack counters.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  include/net/netfilter/nf_conntrack_acct.h |  2 ++
> >  net/netfilter/nf_conntrack_core.c         | 15 +++++++--------
> >  2 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/net/netfilter/nf_conntrack_acct.h b/include/net/netfilter/nf_conntrack_acct.h
> > index f7a060c6eb28..df198c51244a 100644
> > --- a/include/net/netfilter/nf_conntrack_acct.h
> > +++ b/include/net/netfilter/nf_conntrack_acct.h
> > @@ -65,6 +65,8 @@ static inline void nf_ct_set_acct(struct net *net, bool enable)
> >  #endif
> >  }
> >  
> > +void nf_ct_acct_update(struct nf_conn *ct, u32 dir, unsigned int bytes);
> > +
> >  void nf_conntrack_acct_pernet_init(struct net *net);
> >  
> >  int nf_conntrack_acct_init(void);
> > diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> > index a18f8fe728e3..a55c1d6f8191 100644
> > --- a/net/netfilter/nf_conntrack_core.c
> > +++ b/net/netfilter/nf_conntrack_core.c
> > @@ -863,9 +863,7 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)
> >  }
> >  EXPORT_SYMBOL_GPL(nf_conntrack_hash_check_insert);
> >  
> > -static inline void nf_ct_acct_update(struct nf_conn *ct,
> > -				     enum ip_conntrack_info ctinfo,
> > -				     unsigned int len)
> > +void nf_ct_acct_update(struct nf_conn *ct, u32 dir, unsigned int bytes)
> >  {
> >  	struct nf_conn_acct *acct;
> >  
> > @@ -873,10 +871,11 @@ static inline void nf_ct_acct_update(struct nf_conn *ct,
> >  	if (acct) {
> >  		struct nf_conn_counter *counter = acct->counter;
> >  
> > -		atomic64_inc(&counter[CTINFO2DIR(ctinfo)].packets);
> > -		atomic64_add(len, &counter[CTINFO2DIR(ctinfo)].bytes);
> > +		atomic64_inc(&counter[dir].packets);
> > +		atomic64_add(bytes, &counter[dir].bytes);
> >  	}
> >  }
> > +EXPORT_SYMBOL_GPL(nf_ct_acct_update);
> 
> This function only add one packet once. Maybe is not so suit for all the scenario
> 
> such as the HW flowtable offload get the counter from HW periodicly.

This patchset is not addressing the flowtable HW offload.

That will need something like nf_ct_acct_add() to accumulate packets
and bytes. Probably nf_ct_acct_update() should be a static inline that
calls nf_ct_acct_add().