Re: [PATCH nf] netfilter: nat: limit port clash resolution attempts
On Sat, Dec 08, 2018 at 11:07:44AM +0100, Florian Westphal wrote: > Pablo, > > this will unfortunately result in a nf-next merge conflict > due to *rover removal in nf-next. > I can send a patch vs. nf-next instead if you prefer. > > net/netfilter/nf_nat_proto_common.c | 26 ++ > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/net/netfilter/nf_nat_proto_common.c > b/net/netfilter/nf_nat_proto_common.c > index 5d849d835561..0e3321660624 100644 > --- a/net/netfilter/nf_nat_proto_common.c > +++ b/net/netfilter/nf_nat_proto_common.c > @@ -41,9 +41,10 @@ void nf_nat_l4proto_unique_tuple(const struct > nf_nat_l3proto *l3proto, >const struct nf_conn *ct, >u16 *rover) > { > - unsigned int range_size, min, max, i; > + unsigned int range_size, min, max, i, attempts; > __be16 *portptr; > - u_int16_t off; > + u16 off; > + static const unsigned int max_attempts = 128; > > if (maniptype == NF_NAT_MANIP_SRC) > portptr = >src.u.all; > @@ -89,15 +90,32 @@ void nf_nat_l4proto_unique_tuple(const struct > nf_nat_l3proto *l3proto, > off = *rover; > } > > - for (i = 0; ; ++off) { > + attempts = range_size; > + if (attempts > max_attempts) > + attempts = max_attempts; > + > + /* We are in softirq; doing a search of the entire range risks > + * soft lockup when all tuples are already used. > + * > + * If we can't find any free port from first offset, pick a new > + * one and try again, with ever smaller search window. > + */ > +another_round: > + for (i = 0; i < attempts; ++off) { > *portptr = htons(min + off % range_size); > - if (++i != range_size && nf_nat_used_tuple(tuple, ct)) > + if (nf_nat_used_tuple(tuple, ct)) > continue; > if (!(range->flags & (NF_NAT_RANGE_PROTO_RANDOM_ALL| > NF_NAT_RANGE_PROTO_OFFSET))) > *rover = off; > return; > } i never gets increased here so will it loop forever in the worst? Thanks, Xiaozhou
[PATCH nf-next] netfilter: nat: remove unnecessary 'else if' branch
Since a pseudo-random starting point is used in finding a port in the default case, that 'else if' branch above is no longer a necessity. So remove it to simplify code. Signed-off-by: Xiaozhou Liu --- net/netfilter/nf_nat_proto_common.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/net/netfilter/nf_nat_proto_common.c b/net/netfilter/nf_nat_proto_common.c index a7de939fa5a9..136ab65c4082 100644 --- a/net/netfilter/nf_nat_proto_common.c +++ b/net/netfilter/nf_nat_proto_common.c @@ -80,8 +80,6 @@ void nf_nat_l4proto_unique_tuple(const struct nf_nat_l3proto *l3proto, off = l3proto->secure_port(tuple, maniptype == NF_NAT_MANIP_SRC ? tuple->dst.u.all : tuple->src.u.all); - } else if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) { - off = prandom_u32(); } else if (range->flags & NF_NAT_RANGE_PROTO_OFFSET) { off = (ntohs(*portptr) - ntohs(range->base_proto.all)); } else { -- 2.11.0
[PATCH] netfilter: update comment about get_unique_tuple()
`__ip_conntrack_confirm' in the comments is confusing to newcomers since it has long been replaced with __nf_conntrack_confirm. Signed-off-by: Xiaozhou Liu --- net/netfilter/nf_nat_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c index e2b196054dfc..527d125964d1 100644 --- a/net/netfilter/nf_nat_core.c +++ b/net/netfilter/nf_nat_core.c @@ -315,7 +315,8 @@ find_best_ips_proto(const struct nf_conntrack_zone *zone, * and NF_INET_LOCAL_OUT, we change the destination to map into the * range. It might not be possible to get a unique tuple, but we try. * At worst (or if we race), we will end up with a final duplicate in - * __ip_conntrack_confirm and drop the packet. */ + * __nf_conntrack_confirm and drop the packet. + */ static void get_unique_tuple(struct nf_conntrack_tuple *tuple, const struct nf_conntrack_tuple *orig_tuple, -- 2.11.0