RE: [PATCH nf-next 1/4] netfilter: amanda: Correct the return value comparison of the func nf_nat_mangle_udp_packet

["Gao Feng" <gfree.wind@foxmail.com>]

> -----Original Message-----
> From: Pablo Neira Ayuso [mailto:pablo@netfilter.org]
> Sent: Monday, March 27, 2017 8:13 PM
> To: fgao@ikuai8.com
> Cc: netfilter-devel@vger.kernel.org; gfree.wind@gmail.com
> Subject: Re: [PATCH nf-next 1/4] netfilter: amanda: Correct the return
value
> comparison of the func nf_nat_mangle_udp_packet
> 
> On Fri, Mar 17, 2017 at 02:47:19PM +0800, fgao@ikuai8.com wrote:
> > From: Gao Feng <fgao@ikuai8.com>
> >
> > The return value of nf_nat_mangle_udp_packet actually is 1 and 0 as
> > bool type. But the amanda codes compare it with NF_ACCEPT.
> >
> > Signed-off-by: Gao Feng <fgao@ikuai8.com>
> > ---
> >  net/netfilter/nf_nat_amanda.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/netfilter/nf_nat_amanda.c
> > b/net/netfilter/nf_nat_amanda.c index eb77238..e4d61a7 100644
> > --- a/net/netfilter/nf_nat_amanda.c
> > +++ b/net/netfilter/nf_nat_amanda.c
> > @@ -33,7 +33,6 @@ static unsigned int help(struct sk_buff *skb,  {
> >  	char buffer[sizeof("65535")];
> >  	u_int16_t port;
> > -	unsigned int ret;
> >
> >  	/* Connection comes from client. */
> >  	exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port; @@ -63,14
> > +62,14 @@ static unsigned int help(struct sk_buff *skb,
> >  	}
> >
> >  	sprintf(buffer, "%u", port);
> > -	ret = nf_nat_mangle_udp_packet(skb, exp->master, ctinfo,
> > -				       protoff, matchoff, matchlen,
> > -				       buffer, strlen(buffer));
> > -	if (ret != NF_ACCEPT) {
> > +	if (!nf_nat_mangle_udp_packet(skb, exp->master, ctinfo,
> > +				      protoff, matchoff, matchlen,
> > +				      buffer, strlen(buffer))) {
> >  		nf_ct_helper_log(skb, exp->master, "cannot mangle packet");
> >  		nf_ct_unexpect_related(exp);
> > +		return NF_DROP;
> >  	}
> > -	return ret;
> > +	return NF_ACCEPT;
> 
> This cleanup patches are a bit oversplit.
> 
> Better, send one patch where you update nf_nat_mangle_udp_packet() and
> nf_nat_mangle_tcp_packet() to return boolean and update *all of the
netfilter
> spots* where we use them accordingly.
> 
> Please be careful on this...
Ok, I would merge them into one patch.
I already checked them before, but I would check it again after merge.

Regards
Feng