Message-ID: <55096C44.2020604@ericsson.com>
Date:	Wed, 18 Mar 2015 13:15:00 +0100
From:	Ulf Samuelsson <ulf.samuelsson@...csson.com>
To:	YOSHIFUJI Hideaki/吉藤英明 
	<hideaki.yoshifuji@...aclelinux.com>,
	yzhu1 <Yanjun.Zhu@...driver.com>, <brian.haley@...com>,
	<davem@...emloft.net>, <alexandre.dietsch@...driver.com>,
	<clinton.slabbert@...driver.com>, <kuznet@....inr.ac.ru>,
	<jmorris@...ei.org>, <kaber@...sh.net>, <netdev@...r.kernel.org>
CC:	"YOSHIFUJI Hideaki (USAGI Project)" <yoshfuji@...ux-ipv6.org>
Subject: Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE
 state

On 03/18/2015 11:34 AM, YOSHIFUJI Hideaki/吉藤英明 wrote:
> Hi,
>
> Ulf Samuelsson wrote:
>>
>> On 03/12/2015 09:42 AM, YOSHIFUJI Hideaki wrote:
>>> Hello.
>>>
>>> yzhu1 wrote:
>>>> The state machine is in the attachment.
>>>>
>>>> Best Regards!
>>>> Zhu Yanjun
>>>> On 03/12/2015 02:58 PM, Zhu Yanjun wrote:
>>>>> V2:
>>>>>    set ARP_PROBE_BCAST default N.
>>>>>
>>>>> V1:
>>>>>    Have a problem with an HP router at a certain location, which
>>>>>    is configured to only answer to broadcast ARP requests.
>>>>>    That cannot be changed.
>>>>>
>>>>>    The first ARP request the kernel sends out, is a broadcast 
>>>>> request,
>>>>>    which is fine, but after the reply, the kernel sends unicast 
>>>>> requests,
>>>>>    which will not get any replies.
>>>>>
>>>>>    The ARP entry will after some time enter STALE state,
>>>>>    and if nothing is done it will time out, and be removed.
>>>>>    This process takes to long, and I have been told that it is
>>>>>    difficult to makes changes that will eventually remove it.
>>>>>
>>>>>    Have tried to change the state from STALE to INCOMPLETE, which 
>>>>> failed,
>>>>>    and then tried to change the state to PROBE which also failed.
>>>>>
>>>>>    The stack is only sending out unicasts, and never broadcast.
>>>>>    Is there any way to get the stack to send out a broadcast ARP
>>>>>    without having to wait for the entry to be removed?
>>>
>>> Neighbour subsystem will send multicast probes after unicast
>>> probes in NUD_PROBE state if mcast_solicit is more than
>>> ucast_solicit.  Try setting net.ipv4.neigh.*.ucast_solicit to
>>> the value less than net.ipv4.neigh.*.mcast_solicit, please?
>>> e.g.
>>>
>>> net.ipv4.neigh.eth0.mcast_solicit = 3
>>> net.ipv4.neigh.eth0.ucast_solicit = 1
>>>
>>> --yoshfuji
>>>
>> I dont see how, and I would like to focus on code discussion.
>>
>> Below is simplified pseudo code of the timer handler
>> after you have reached REACHABLE the first time.
>>
>>      "mcast_solicit" is not used at all.
>>
>> It is only used when in INCOMPLETE state as far as I can tell.
>
> OK, I found I made this change in 2003:
>
> From d12fd76789e80ae337408834f45dae7cba23fc55 Mon Sep 17 00:00:00 2001
> From: Hideaki Yoshifuji <yoshfuji@...ux-ipv6.org>
> Date: Sun, 6 Jul 2003 23:32:45 +1000
> Subject: [PATCH] [NET] Send only unicast NSs in PROBE state.
>
> ---
>  net/core/neighbour.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index c640ad5..001fdb4 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -608,7 +608,9 @@ next_elt:
>  static __inline__ int neigh_max_probes(struct neighbour *n)
>  {
>      struct neigh_parms *p = n->parms;
> -    return p->ucast_probes + p->app_probes + p->mcast_probes;
> +    return (n->nud_state & NUD_PROBE ?
> +        p->ucast_probes :
> +        p->ucast_probes + p->app_probes + p->mcast_probes);
>  }
>
>
> As I recall, I was hesitating adding new sysctl knob, but now I am
> okay to have knob to enable mcast probes in PROBE state as well.
> (By default, it should NOT send multicast probe (expecially for IPv6)
> in PROBE state.)
>
> How about these?
> - introduce probe_mcast_probes knob, default to 0.
> - Change neigh_max_probes() to reflect that.
>
> Then, arp_colisit() and ndict_solicit() should send multicast probes
> in PROBE state as well, if probe_mcast_probes is set to positive
> value.
>
> Will this work for you?
>
> Regards,

"probe_mcast_probes" as a name sucks...

It is also confusing since it is doing something very similar to
ucast_solicit, app_solicit and mcast_solicit.

As I see it, it should be named "<XXX>_solicit" to show
how it is related to the rest of the sysctl entries.

If XXX is "bcast", as in my suggestion, is less important.

"mcast_probe_solicit" would work for me, but prefer "bcast_solicit".

What exactly is wrong with that name?
=================

Your suggestion was my initial suggestion for solution, and after 
consideration
by Wind River reviewers it was rejected, since it affected IPv6.
Did not check in what way.

The WR proposed solution, which is the one that was sent to the list,
was to keep neigh_max_probes as is, but add check for "bcast_solicit" inside
the timer handler, which they think makes sure that it affects IPv4 
processing only.

The solution should allow broadcast ARP requests in IPv4 after unicast ARP
requests without making IPv6 incompatible with RFCs.

Yanjun may be able to comment further.

Best Regards,
Ulf Samuelsson


>
> --yoshfuji
>
>>
>>
>> Best Regards,
>> Ulf Samuelsson
>>
>>
>>
>>>
>>>>>
>>>>>    I think the recommended behaviour in IPv6 is to send out 3 
>>>>> unicasts
>>>>>    and if all fails, to send out broadcasts.
>>>>>
>>>>> Zhu Yanjun (1):
>>>>>    neighbour: Support broadcast ARP in neighbor PROPE state
>>>>>
>>>>>   include/net/neighbour.h        |  7 ++++++
>>>>>   include/uapi/linux/neighbour.h |  6 +++++
>>>>>   include/uapi/linux/sysctl.h    |  3 +++
>>>>>   kernel/sysctl_binary.c         |  3 +++
>>>>>   net/core/neighbour.c           | 44 
>>>>> +++++++++++++++++++++++++++++---
>>>>>   net/ipv4/Kconfig               | 57 
>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>   net/ipv4/arp.c                 |  7 ++++--
>>>>>   7 files changed, 121 insertions(+), 6 deletions(-)
>>>>>
>>>>
>>>
>>
>


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists