Re: [PATCH 1/3] netfilter: Expose connection tracking accounting toggles

[Patrick McHardy <kaber@trash.net>]

Tim Gardner wrote:
> nf_ct_acct_enabled() - Get CT accounting state.
> nf_ct_set_acct() - Enable/disable CT accountuing.
>   

Thanks for taking care of this. Just one final comment:

> diff --git a/include/net/netfilter/nf_conntrack_acct.h b/include/net/netfilter/nf_conntrack_acct.h
> index 03e218f..31f5cd3 100644
> --- a/include/net/netfilter/nf_conntrack_acct.h
> +++ b/include/net/netfilter/nf_conntrack_acct.h
> @@ -45,6 +45,18 @@ struct nf_conn_counter *nf_ct_acct_ext_add(struct nf_conn *ct, gfp_t gfp)
>  extern unsigned int
>  seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir);
>  
> +/* Check if connection tracking accounting is enabled */
> +static inline bool nf_ct_acct_enabled(const struct xt_mtchk_param *par)
>   

 From an API point of view its cleaner to have the caller just pass
in the net pointer. Accounting has no direct relationship to xtables.

It would also make sense to fold this patch into 2/3 since this is
where these functions are actually getting used.

> +{
> +	return par->net->ct.sysctl_acct == 0 ? false : true;
> +}
> +
> +/* Enable/disable connection tracking accounting */
> +static inline void nf_ct_set_acct(const struct xt_mtchk_param *par, bool enable)
>   

Same here.

> +{
> +	par->net->ct.sysctl_acct = enable == true ? 1 : 0;
> +}
> +
>  extern int nf_conntrack_acct_init(struct net *net);
>  extern void nf_conntrack_acct_fini(struct net *net);
>  
>