> -----Original Message----- > From: Pablo Neira Ayuso [mailto:pablo@xxxxxxxxxxxxx] > Sent: Monday, March 27, 2017 8:13 PM > To: fgao@xxxxxxxxxx > Cc: netfilter-devel@xxxxxxxxxxxxxxx; gfree.wind@xxxxxxxxx > 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@xxxxxxxxxx wrote: > > From: Gao Feng <fgao@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
