netdev - Re: [PATCH net V3] Exempt multicast addresses from five-second neighbor lifetime

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7f3d2fa6-b17b-91b9-d47c-c5e14edcced5@akamai.com>
Date:   Thu, 12 Nov 2020 20:59:03 -0500
From:   Jeff Dike <jdike@...mai.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, David Ahern <dsahern@...il.com>
Subject: Re: [PATCH net V3] Exempt multicast addresses from five-second
 neighbor lifetime

Hi Jakub,

Yes to all your suggestions.

Thanks for the review.

Jeff

On 11/12/20 12:32 PM, Jakub Kicinski wrote:
> On Tue, 10 Nov 2020 12:23:05 -0500 Jeff Dike wrote:
>> Commit 58956317c8de ("neighbor: Improve garbage collection")
>> guarantees neighbour table entries a five-second lifetime.  Processes
>> which make heavy use of multicast can fill the neighour table with
>> multicast addresses in five seconds.  At that point, neighbour entries
>> can't be GC-ed because they aren't five seconds old yet, the kernel
>> log starts to fill up with "neighbor table overflow!" messages, and
>> sends start to fail.
>>
>> This patch allows multicast addresses to be thrown out before they've
>> lived out their five seconds.  This makes room for non-multicast
>> addresses and makes messages to all addresses more reliable in these
>> circumstances.
> 
> We should add 
> 
> Fixes: 58956317c8de ("neighbor: Improve garbage collection")
> 
> right?
> 
>> Signed-off-by: Jeff Dike <jdike@...mai.com>
> 
>> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
>> index 687971d83b4e..097aa8bf07ee 100644
>> --- a/net/ipv4/arp.c
>> +++ b/net/ipv4/arp.c
>> @@ -125,6 +125,7 @@ static int arp_constructor(struct neighbour *neigh);
>>  static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb);
>>  static void arp_error_report(struct neighbour *neigh, struct sk_buff *skb);
>>  static void parp_redo(struct sk_buff *skb);
>> +static int arp_is_multicast(const void *pkey);
>>  
>>  static const struct neigh_ops arp_generic_ops = {
>>  	.family =		AF_INET,
>> @@ -156,6 +157,7 @@ struct neigh_table arp_tbl = {
>>  	.key_eq		= arp_key_eq,
>>  	.constructor	= arp_constructor,
>>  	.proxy_redo	= parp_redo,
>> +	.is_multicast   = arp_is_multicast,
> 
> extreme nit pick - please align the = sign using tabs like the
> surrounding code does.
> 
>>  	.id		= "arp_cache",
>>  	.parms		= {
>>  		.tbl			= &arp_tbl,
>> @@ -928,6 +930,10 @@ static void parp_redo(struct sk_buff *skb)
>>  	arp_process(dev_net(skb->dev), NULL, skb);
>>  }
>>  
>> +static int arp_is_multicast(const void *pkey)
>> +{
>> +	return ipv4_is_multicast(*((__be32 *)pkey));
>> +}
>>  
>>  /*
>>   *	Receive an arp request from the device layer.
>> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
>> index 27f29b957ee7..67457cfadcd2 100644
>> --- a/net/ipv6/ndisc.c
>> +++ b/net/ipv6/ndisc.c
>> @@ -81,6 +81,7 @@ static void ndisc_error_report(struct neighbour *neigh, struct sk_buff *skb);
>>  static int pndisc_constructor(struct pneigh_entry *n);
>>  static void pndisc_destructor(struct pneigh_entry *n);
>>  static void pndisc_redo(struct sk_buff *skb);
>> +static int ndisc_is_multicast(const void *pkey);
>>  
>>  static const struct neigh_ops ndisc_generic_ops = {
>>  	.family =		AF_INET6,
>> @@ -115,6 +116,7 @@ struct neigh_table nd_tbl = {
>>  	.pconstructor =	pndisc_constructor,
>>  	.pdestructor =	pndisc_destructor,
>>  	.proxy_redo =	pndisc_redo,
>> +	.is_multicast = ndisc_is_multicast,
> 
> looks like the character after = is expected to be a tab, for better or
> worse
> 
>>  	.allow_add  =   ndisc_allow_add,
>>  	.id =		"ndisc_cache",
>>  	.parms = {
>> @@ -1706,6 +1708,11 @@ static void pndisc_redo(struct sk_buff *skb)
>>  	kfree_skb(skb);
>>  }
>>  
>> +static int ndisc_is_multicast(const void *pkey)
>> +{
>> +	return ipv6_addr_is_multicast((struct in6_addr *)pkey);
>> +}
>> +
>>  static bool ndisc_suppress_frag_ndisc(struct sk_buff *skb)
>>  {
>>  	struct inet6_dev *idev = __in6_dev_get(skb->dev);
>