Re: [PATCH net-next v2 2/8] netfilter: add API to manage NAT helpers.

[Flavio Leitner <fbl@redhat.com>]

On Mon, Apr 15, 2019 at 07:50:34AM +0200, Pablo Neira Ayuso wrote:
> On Sat, Apr 13, 2019 at 08:17:10PM -0300, Flavio Leitner wrote:
> [...]
> > +void nf_nat_helper_put(struct nf_conntrack_helper *helper)
> > +{
> > +	struct nf_conntrack_nat_helper *nat;
> > +
> > +	nat = nf_conntrack_nat_helper_find(helper->nat_mod_name);
> > +	BUG_ON(nat == NULL);
> 
> We've been trying to avoid BUG_ON() in many spots recently. Could you
> turn this into... ?
> 
>         if (WARN_ON(!nat))
>                 return;

OK.


> 
> > +	module_put(nat->module);
> > +}
> > +EXPORT_SYMBOL_GPL(nf_nat_helper_put);
> > +
> >  struct nf_conn_help *
> >  nf_ct_helper_ext_add(struct nf_conn *ct, gfp_t gfp)
> >  {
> > @@ -430,6 +502,10 @@ void nf_ct_helper_init(struct nf_conntrack_helper *helper,
> >  	helper->help = help;
> >  	helper->from_nlattr = from_nlattr;
> >  	helper->me = module;
> > +	helper->nat_mod_name[0] = '\0';
> > +	if (name)
> > +		snprintf(helper->nat_mod_name, sizeof(helper->nat_mod_name),
> > +			 NF_NAT_HELPER_PREFIX"%s", name);
> >  
> >  	if (spec_port == default_port)
> >  		snprintf(helper->name, sizeof(helper->name), "%s", name);
> > @@ -466,6 +542,26 @@ void nf_conntrack_helpers_unregister(struct nf_conntrack_helper *helper,
> >  }
> >  EXPORT_SYMBOL_GPL(nf_conntrack_helpers_unregister);
> >  
> > +void nf_nat_helper_register(struct nf_conntrack_nat_helper *nat)
> > +{
> > +	BUG_ON(nat->module == NULL);
> 
> Same here.


OK, no problem.

> 
> > +
> > +	mutex_lock(&nf_ct_nat_helpers_mutex);
> > +	list_add_rcu(&nat->list, &nf_ct_nat_helpers);
> > +	mutex_unlock(&nf_ct_nat_helpers_mutex);
> > +}
> > +EXPORT_SYMBOL_GPL(nf_nat_helper_register);
> > +
> > +void nf_nat_helper_unregister(struct nf_conntrack_nat_helper *nat)
> > +{
> > +	BUG_ON(nat->module == NULL);
> 
> And here.

Sure.
Thanks!
fbl

> 
> Thanks.