Messages in this thread | | | Subject | Re: [PATCH] net: neigh: decrement the family specific qlen | From | Paolo Abeni <> | Date | Tue, 15 Nov 2022 10:56:45 +0100 |
| |
Hello,
On Sat, 2022-11-12 at 11:48 +0100, Thomas Zeitlhofer wrote: > Commit 0ff4eb3d5ebb ("neighbour: make proxy_queue.qlen limit > per-device") introduced the length counter qlen in struct neigh_parms. > There are separate neigh_parms instances for IPv4/ARP and IPv6/ND, and > while the family specific qlen is incremented in pneigh_enqueue(), the > mentioned commit decrements always the IPv4/ARP specific qlen, > regardless of the currently processed family, in pneigh_queue_purge() > and neigh_proxy_process(). > > As a result, with IPv6/ND, the family specific qlen is only incremented > (and never decremented) until it exceeds PROXY_QLEN, and then, according > to the check in pneigh_enqueue(), neighbor solicitations are not > answered anymore. As an example, this is noted when using the > subnet-router anycast address to access a Linux router. After a certain > amount of time (in the observed case, qlen exceeded PROXY_QLEN after two > days), the Linux router stops answering neighbor solicitations for its > subnet-router anycast address and effectively becomes unreachable. > > Another result with IPv6/ND is that the IPv4/ARP specific qlen is > decremented more often than incremented. This leads to negative qlen > values, as a signed integer has been used for the length counter qlen, > and potentially to an integer overflow. > > Fix this by introducing the helper function neigh_parms_qlen_dec(), > which decrements the family specific qlen. Thereby, make use of the > existing helper function neigh_get_dev_parms_rcu(), whose definition > therefore needs to be placed earlier in neighbour.c. Take the family > member from struct neigh_table to determine the currently processed > family and appropriately call neigh_parms_qlen_dec() from > pneigh_queue_purge() and neigh_proxy_process(). > > Additionally, use an unsigned integer for the length counter qlen. > > Fixes: 0ff4eb3d5ebb ("neighbour: make proxy_queue.qlen limit per-device") > Signed-off-by: Thomas Zeitlhofer <thomas.zeitlhofer+lkml@ze-it.at> > --- > include/net/neighbour.h | 2 +- > net/core/neighbour.c | 58 +++++++++++++++++++++-------------------- > 2 files changed, 31 insertions(+), 29 deletions(-) > > diff --git a/include/net/neighbour.h b/include/net/neighbour.h > index 20745cf7ae1a..cc0b65b7c829 100644 > --- a/include/net/neighbour.h > +++ b/include/net/neighbour.h > @@ -83,7 +83,7 @@ struct neigh_parms { > struct rcu_head rcu_head; > > int reachable_time; > - int qlen; > + __u32 qlen; > int data[NEIGH_VAR_DATA_MAX]; > DECLARE_BITMAP(data_state, NEIGH_VAR_DATA_MAX); > };
The patch LGTM, but why did you use __u32 above? this is not part of uAPI, plain u32 should be fine.
Thanks!
Paolo
| |