[PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[Zhu Yanjun]

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?

  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(-)

-- 
1.9.1

[PATCH V2 1/1] neighbour: Support broadcast ARP in neighbor PROPE state

[Zhu Yanjun]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

When the neighbor state machine is in PROBE state, it will normally send
a number of unicast ARP requests (number defined in "ucast_probes" entry
in the proc file system, default=3) and if no reply is received, it will
change state to FAILED.

Enabling CONFIG_ARP_PROBE_BCAST, will make the state machine try to send
broadcast ARP requests if the unicast ARP requests failed. The state machine
will only enter FAILED state if the broadcast ARP requests did not receive
a reply.

Enabling CONFIG_ARP_PROBE_BCAST, makes the IPv4 ARP behaviour more
similar to the IPv6 Neighbor Discovery protocol, and is necessary,
if the other end only responds to broadcast ARPs.

CC: WANG Cong <xiyou.wangcong@gmail.com> 
Reviewed-by: Yang Shi <yang.shi@windriver.com>
Signed-off-by: eulfsam <ulf.samuelsson@windriver.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
---
 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(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 76f7084..85070b2 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -40,6 +40,9 @@ struct neighbour;
 
 enum {
 	NEIGH_VAR_MCAST_PROBES,
+#ifdef CONFIG_IP_ARP_BROADCAST
+	NEIGH_VAR_BCAST_PROBES,
+#endif
 	NEIGH_VAR_UCAST_PROBES,
 	NEIGH_VAR_APP_PROBES,
 	NEIGH_VAR_RETRANS_TIME,
@@ -122,6 +125,10 @@ struct neigh_statistics {
 	unsigned long rcv_probes_mcast;	/* number of received mcast ipv6 */
 	unsigned long rcv_probes_ucast; /* number of received ucast ipv6 */
 
+#ifdef CONFIG_IP_ARP_BROADCAST
+	unsigned long rcv_probes_bcast; /* number of received ucast ipv6 */
+#endif
+
 	unsigned long periodic_gc_runs;	/* number of periodic GC runs */
 	unsigned long forced_gc_runs;	/* number of forced GC runs */
 
diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index 3873a35..79cef9e 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -103,6 +103,9 @@ struct ndt_stats {
 	__u64		ndts_lookups;
 	__u64		ndts_hits;
 	__u64		ndts_rcv_probes_mcast;
+#ifdef CONFIG_IP_ARP_BROADCAST
+	__u64		ndts_rcv_probes_bcast;
+#endif
 	__u64		ndts_rcv_probes_ucast;
 	__u64		ndts_periodic_gc_runs;
 	__u64		ndts_forced_gc_runs;
@@ -121,6 +124,9 @@ enum {
 	NDTPA_APP_PROBES,		/* u32 */
 	NDTPA_UCAST_PROBES,		/* u32 */
 	NDTPA_MCAST_PROBES,		/* u32 */
+#ifdef CONFIG_IP_ARP_BROADCAST
+	NDTPA_BCAST_PROBES,		/* u32 */
+#endif
 	NDTPA_ANYCAST_DELAY,		/* u64, msecs */
 	NDTPA_PROXY_DELAY,		/* u64, msecs */
 	NDTPA_PROXY_QLEN,		/* u32 */
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index 0956373..0c54d16 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -598,6 +598,9 @@ enum {
 	NET_NEIGH_GC_THRESH3=16,
 	NET_NEIGH_RETRANS_TIME_MS=17,
 	NET_NEIGH_REACHABLE_TIME_MS=18,
+#ifdef CONFIG_IP_ARP_BROADCAST
+	NET_NEIGH_BCAST_SOLICIT=19,
+#endif
 };
 
 /* /proc/sys/net/dccp */
diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 7e7746a..47482a6 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -267,6 +267,9 @@ static const struct bin_table bin_net_neigh_vars_table[] = {
 	{ CTL_INT,	NET_NEIGH_MCAST_SOLICIT,	"mcast_solicit" },
 	{ CTL_INT,	NET_NEIGH_UCAST_SOLICIT,	"ucast_solicit" },
 	{ CTL_INT,	NET_NEIGH_APP_SOLICIT,		"app_solicit" },
+#ifdef CONFIG_IP_ARP_BROADCAST
+	{ CTL_INT,	NET_NEIGH_BCAST_SOLICIT,	"bcast_solicit" },
+#endif
 	/* NET_NEIGH_RETRANS_TIME "retrans_time" no longer used */
 	{ CTL_INT,	NET_NEIGH_REACHABLE_TIME,	"base_reachable_time" },
 	{ CTL_INT,	NET_NEIGH_DELAY_PROBE_TIME,	"delay_first_probe_time" },
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 70fe9e1..b372af4 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -936,10 +936,16 @@ static void neigh_timer_handler(unsigned long arg)
 
 	if ((neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) &&
 	    atomic_read(&neigh->probes) >= neigh_max_probes(neigh)) {
-		neigh->nud_state = NUD_FAILED;
-		notify = 1;
-		neigh_invalidate(neigh);
-		goto out;
+#ifdef CONFIG_IP_ARP_BROADCAST
+		int delta_probes = atomic_read(&neigh->probes) - neigh_max_probes(neigh);
+		if (delta_probes >= NEIGH_VAR(neigh->parms, BCAST_PROBES))
+#endif
+		{
+			neigh->nud_state = NUD_FAILED;
+			notify = 1;
+			neigh_invalidate(neigh);
+			goto out;
+		}
 	}
 
 	if (neigh->nud_state & NUD_IN_TIMER) {
@@ -973,6 +979,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
 		goto out_unlock_bh;
 
 	if (!(neigh->nud_state & (NUD_STALE | NUD_INCOMPLETE))) {
+		/* We are in (NUD_NONE | NUD_FAILED) */
 		if (NEIGH_VAR(neigh->parms, MCAST_PROBES) +
 		    NEIGH_VAR(neigh->parms, APP_PROBES)) {
 			unsigned long next, now = jiffies;
@@ -1783,6 +1790,10 @@ static int neightbl_fill_parms(struct sk_buff *skb, struct neigh_parms *parms)
 			NEIGH_VAR(parms, UCAST_PROBES)) ||
 	    nla_put_u32(skb, NDTPA_MCAST_PROBES,
 			NEIGH_VAR(parms, MCAST_PROBES)) ||
+#ifdef CONFIG_IP_ARP_BROADCAST
+	    nla_put_u32(skb, NDTPA_BCAST_PROBES,
+			NEIGH_VAR(parms, BCAST_PROBES)) ||
+#endif
 	    nla_put_msecs(skb, NDTPA_REACHABLE_TIME, parms->reachable_time) ||
 	    nla_put_msecs(skb, NDTPA_BASE_REACHABLE_TIME,
 			  NEIGH_VAR(parms, BASE_REACHABLE_TIME)) ||
@@ -1870,6 +1881,9 @@ static int neightbl_fill_info(struct sk_buff *skb, struct neigh_table *tbl,
 			ndst.ndts_lookups		+= st->lookups;
 			ndst.ndts_hits			+= st->hits;
 			ndst.ndts_rcv_probes_mcast	+= st->rcv_probes_mcast;
+#ifdef CONFIG_IP_ARP_BROADCAST
+			ndst.ndts_rcv_probes_bcast	+= st->rcv_probes_bcast;
+#endif
 			ndst.ndts_rcv_probes_ucast	+= st->rcv_probes_ucast;
 			ndst.ndts_periodic_gc_runs	+= st->periodic_gc_runs;
 			ndst.ndts_forced_gc_runs	+= st->forced_gc_runs;
@@ -1942,6 +1956,9 @@ static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
 	[NDTPA_APP_PROBES]		= { .type = NLA_U32 },
 	[NDTPA_UCAST_PROBES]		= { .type = NLA_U32 },
 	[NDTPA_MCAST_PROBES]		= { .type = NLA_U32 },
+#ifdef CONFIG_IP_ARP_BROADCAST
+	[NDTPA_BCAST_PROBES]		= { .type = NLA_U32 },
+#endif
 	[NDTPA_BASE_REACHABLE_TIME]	= { .type = NLA_U64 },
 	[NDTPA_GC_STALETIME]		= { .type = NLA_U64 },
 	[NDTPA_DELAY_PROBE_TIME]	= { .type = NLA_U64 },
@@ -2042,6 +2059,12 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh)
 				NEIGH_VAR_SET(p, MCAST_PROBES,
 					      nla_get_u32(tbp[i]));
 				break;
+#ifdef CONFIG_IP_ARP_BROADCAST
+			case NDTPA_BCAST_PROBES:
+				NEIGH_VAR_SET(p, BCAST_PROBES,
+					      nla_get_u32(tbp[i]));
+				break;
+#endif
 			case NDTPA_BASE_REACHABLE_TIME:
 				NEIGH_VAR_SET(p, BASE_REACHABLE_TIME,
 					      nla_get_msecs(tbp[i]));
@@ -2702,11 +2725,18 @@ static int neigh_stat_seq_show(struct seq_file *seq, void *v)
 	struct neigh_statistics *st = v;
 
 	if (v == SEQ_START_TOKEN) {
+#ifdef CONFIG_IP_ARP_BROADCAST
+		seq_printf(seq, "entries  allocs destroys hash_grows  lookups hits  res_failed  rcv_probes_bcast rcv_probes_mcast rcv_probes_ucast  periodic_gc_runs forced_gc_runs unresolved_discards\n");
+#else
 		seq_printf(seq, "entries  allocs destroys hash_grows  lookups hits  res_failed  rcv_probes_mcast rcv_probes_ucast  periodic_gc_runs forced_gc_runs unresolved_discards\n");
+#endif
 		return 0;
 	}
 
 	seq_printf(seq, "%08x  %08lx %08lx %08lx  %08lx %08lx  %08lx  "
+#ifdef CONFIG_IP_ARP_BROADCAST
+			"%08lx "
+#endif
 			"%08lx %08lx  %08lx %08lx %08lx\n",
 		   atomic_read(&tbl->entries),
 
@@ -2719,6 +2749,9 @@ static int neigh_stat_seq_show(struct seq_file *seq, void *v)
 
 		   st->res_failed,
 
+#ifdef CONFIG_IP_ARP_BROADCAST
+		   st->rcv_probes_bcast,
+#endif
 		   st->rcv_probes_mcast,
 		   st->rcv_probes_ucast,
 
@@ -2992,6 +3025,9 @@ static struct neigh_sysctl_table {
 } neigh_sysctl_template __read_mostly = {
 	.neigh_vars = {
 		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(MCAST_PROBES, "mcast_solicit"),
+#ifdef CONFIG_IP_ARP_BROADCAST
+		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(BCAST_PROBES, "bcast_solicit"),
+#endif
 		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(UCAST_PROBES, "ucast_solicit"),
 		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(APP_PROBES, "app_solicit"),
 		NEIGH_SYSCTL_USERHZ_JIFFIES_ENTRY(RETRANS_TIME, "retrans_time"),
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index bd29016..2f43dc9 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -1,6 +1,63 @@
 #
 # IP configuration
 #
+config IP_ARP_BROADCAST
+	bool "IP: allow broadcast ARP"
+	default N
+	---help---
+	  The stack will periodically refresh ARP cache by sending unicast ARP requests.
+	  if a number of unicast ARP request fails, the stack will normally enter
+	  FAILED state, and the ARP entry will be removed by the garbage collector.
+	  Enabling this option will make the stack try to send broadcast ARP requests
+	  if the unicast ARP requests fails, before entering FAILED state.
+	  The number of broadcast packets is controlled by
+	  '/proc/sys/net/ipv4/neigh/<dev>/bcast_solicit' which defaults to "0".
+	  The parameter must be assigned a value to enable sending broadcast ARPs.
+
+	  If unsure, say N here.
+
+
+config IP_ARP_UCAST_PROBES
+	int "ucast_solicit initial value"
+	default "3"
+
+	---help---
+	  This value defines the initial value for how many unicast ARP requests
+	  are sent by the ARP state machine in PROBE state.
+	  It can be controlled runtime by '/proc/sys/net/ipv4/neigh/<dev>/ucast_solicit'
+
+	  If unsure, say "3" here.
+
+
+config IP_ARP_MCAST_PROBES
+	int "mcast_solicit initial value"
+	default "3"
+
+	---help---
+	  This value defines the initial value for how many broadcast ARP requests
+	  are sent by the ARP state machine in INCOMPLETE state.
+	  It can be controlled runtime by '/proc/sys/net/ipv4/neigh/<dev>/mcast_solicit'
+
+	  If unsure, say "3" here.
+
+
+config IP_ARP_BCAST_PROBES
+	int "bcast_solicit initial value"
+	depends on IP_ARP_BROADCAST
+	default "0"
+
+	---help---
+	  This value defines the initial value for how many broadcast ARP requests
+	  are sent by the ARP state machine in PROBE state.
+	  It can be controlled runtime by '/proc/sys/net/ipv4/neigh/<dev>/bcast_solicit'
+	  The default value of 'bcast_solicit' is zero, mimicking the behaviour of
+	  older kernels which does not send out broadcast ARPs except for new entries.
+	  If you want broadcast ARP requests to happen in PROBE state, set the number here.
+	  To be compatible with IPv6, the number should be "3"
+
+	  If unsure, say "0" here.
+
+
 config IP_MULTICAST
 	bool "IP: multicasting"
 	help
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 205e147..4795ab6 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -168,8 +168,11 @@ struct neigh_table arp_tbl = {
 		.tbl			= &arp_tbl,
 		.reachable_time		= 30 * HZ,
 		.data	= {
-			[NEIGH_VAR_MCAST_PROBES] = 3,
-			[NEIGH_VAR_UCAST_PROBES] = 3,
+			[NEIGH_VAR_MCAST_PROBES] = CONFIG_IP_ARP_MCAST_PROBES,
+			[NEIGH_VAR_UCAST_PROBES] = CONFIG_IP_ARP_UCAST_PROBES,
+#ifdef CONFIG_IP_ARP_BROADCAST
+			[NEIGH_VAR_BCAST_PROBES] = CONFIG_IP_ARP_BCAST_PROBES,
+#endif
 			[NEIGH_VAR_RETRANS_TIME] = 1 * HZ,
 			[NEIGH_VAR_BASE_REACHABLE_TIME] = 30 * HZ,
 			[NEIGH_VAR_DELAY_PROBE_TIME] = 5 * HZ,
-- 
1.9.1

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[yzhu1]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

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?
>
>    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(-)
>


[-- Attachment #2: 1.jpg --]
[-- Type: image/jpeg, Size: 99365 bytes --]

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[YOSHIFUJI Hideaki]

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 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(-)
>>
>

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[Ulf samuelsson]

That also means that you increase the number of broadcasts in incomplete state.
Is that really desirable?

Best Regards
Ulf Samuelsson
ulf@emagii.com
+46  (722) 427 437


> 12 mar 2015 kl. 09:42 skrev YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>:
> 
> 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 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(-)
> 
> -- 
> Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
> Technical Division, MIRACLE LINUX CORPORATION
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[YOSHIFUJI Hideaki/吉藤英明]

Hi,

Ulf samuelsson wrote:
> That also means that you increase the number of broadcasts in incomplete state.
> Is that really desirable?

In NUD_INCOMPLETE state, neighbour subsystem *skips* sending unicast probes
and send multicast probes mcast_solicit times. If you do not increase
the number of mcast_solicit, the number of multicast probes in
NUD_INCOMPLETE will not increased.

--yoshfuji

>
> Best Regards
> Ulf Samuelsson
> ulf@emagii.com
> +46  (722) 427 437
>
>
>> 12 mar 2015 kl. 09:42 skrev YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>:
>>
>> 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 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(-)
>>
>> --
>> Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
>> Technical Division, MIRACLE LINUX CORPORATION
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
ミラクル・リナックス株式会社 技術本部 サポート部

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[Ulf samuelsson]

If you want to send 3 unicasts followed by 3 broadcast,
you have to set mcast_solicit to 6.
This means you will send 6 broadcasts in incomplete state.

I think the following configuration is wanted:

Incomplete state: send 3 broadcast
Probe state: send 3 unicast, followed by 3 broadcast then goto STALE OR
                     Send 3 unicast, then go to STALE

Best Regards
Ulf Samuelsson
ulf@emagii.com
+46  (722) 427 437


> 12 mar 2015 kl. 10:28 skrev YOSHIFUJI Hideaki/吉藤英明  <hideaki.yoshifuji@miraclelinux.com>:
> 
> Hi,
> 
> Ulf samuelsson wrote:
>> That also means that you increase the number of broadcasts in incomplete state.
>> Is that really desirable?
> 
> In NUD_INCOMPLETE state, neighbour subsystem *skips* sending unicast probes
> and send multicast probes mcast_solicit times. If you do not increase
> the number of mcast_solicit, the number of multicast probes in
> NUD_INCOMPLETE will not increased.
> 
> --yoshfuji
> 
>> 
>> Best Regards
>> Ulf Samuelsson
>> ulf@emagii.com
>> +46  (722) 427 437
>> 
>> 
>>> 12 mar 2015 kl. 09:42 skrev YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>:
>>> 
>>> 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 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(-)
>>> 
>>> --
>>> Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
>>> Technical Division, MIRACLE LINUX CORPORATION
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> 吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
> ミラクル・リナックス株式会社 技術本部 サポート部
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/1] neighbour: Support broadcast ARP in neighbor PROPE state

[YOSHIFUJI Hideaki/吉藤英明]

Hi,

Zhu Yanjun wrote:
> From: Zhu Yanjun <yanjun.zhu@windriver.com>
> 
> When the neighbor state machine is in PROBE state, it will normally send
> a number of unicast ARP requests (number defined in "ucast_probes" entry
> in the proc file system, default=3) and if no reply is received, it will
> change state to FAILED.
> 
> Enabling CONFIG_ARP_PROBE_BCAST, will make the state machine try to send
> broadcast ARP requests if the unicast ARP requests failed. The state machine
> will only enter FAILED state if the broadcast ARP requests did not receive
> a reply.
> 
> Enabling CONFIG_ARP_PROBE_BCAST, makes the IPv4 ARP behaviour more
> similar to the IPv6 Neighbor Discovery protocol, and is necessary,
> if the other end only responds to broadcast ARPs.

I don't think There are any difference between the state machine of ARP and
the state machine of NDP here.

--yoshfuji
> 
> CC: WANG Cong <xiyou.wangcong@gmail.com>
> Reviewed-by: Yang Shi <yang.shi@windriver.com>
> Signed-off-by: eulfsam <ulf.samuelsson@windriver.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
> ---
>   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(-)
> 
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index 76f7084..85070b2 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -40,6 +40,9 @@ struct neighbour;
>   
>   enum {
>   	NEIGH_VAR_MCAST_PROBES,
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +	NEIGH_VAR_BCAST_PROBES,
> +#endif
>   	NEIGH_VAR_UCAST_PROBES,
>   	NEIGH_VAR_APP_PROBES,
>   	NEIGH_VAR_RETRANS_TIME,
> @@ -122,6 +125,10 @@ struct neigh_statistics {
>   	unsigned long rcv_probes_mcast;	/* number of received mcast ipv6 */
>   	unsigned long rcv_probes_ucast; /* number of received ucast ipv6 */
>   
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +	unsigned long rcv_probes_bcast; /* number of received ucast ipv6 */
> +#endif
> +
>   	unsigned long periodic_gc_runs;	/* number of periodic GC runs */
>   	unsigned long forced_gc_runs;	/* number of forced GC runs */
>   
> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
> index 3873a35..79cef9e 100644
> --- a/include/uapi/linux/neighbour.h
> +++ b/include/uapi/linux/neighbour.h
> @@ -103,6 +103,9 @@ struct ndt_stats {
>   	__u64		ndts_lookups;
>   	__u64		ndts_hits;
>   	__u64		ndts_rcv_probes_mcast;
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +	__u64		ndts_rcv_probes_bcast;
> +#endif
>   	__u64		ndts_rcv_probes_ucast;
>   	__u64		ndts_periodic_gc_runs;
>   	__u64		ndts_forced_gc_runs;
> @@ -121,6 +124,9 @@ enum {
>   	NDTPA_APP_PROBES,		/* u32 */
>   	NDTPA_UCAST_PROBES,		/* u32 */
>   	NDTPA_MCAST_PROBES,		/* u32 */
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +	NDTPA_BCAST_PROBES,		/* u32 */
> +#endif
>   	NDTPA_ANYCAST_DELAY,		/* u64, msecs */
>   	NDTPA_PROXY_DELAY,		/* u64, msecs */
>   	NDTPA_PROXY_QLEN,		/* u32 */
> diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
> index 0956373..0c54d16 100644
> --- a/include/uapi/linux/sysctl.h
> +++ b/include/uapi/linux/sysctl.h
> @@ -598,6 +598,9 @@ enum {
>   	NET_NEIGH_GC_THRESH3=16,
>   	NET_NEIGH_RETRANS_TIME_MS=17,
>   	NET_NEIGH_REACHABLE_TIME_MS=18,
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +	NET_NEIGH_BCAST_SOLICIT=19,
> +#endif
>   };
>   
>   /* /proc/sys/net/dccp */
> diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
> index 7e7746a..47482a6 100644
> --- a/kernel/sysctl_binary.c
> +++ b/kernel/sysctl_binary.c
> @@ -267,6 +267,9 @@ static const struct bin_table bin_net_neigh_vars_table[] = {
>   	{ CTL_INT,	NET_NEIGH_MCAST_SOLICIT,	"mcast_solicit" },
>   	{ CTL_INT,	NET_NEIGH_UCAST_SOLICIT,	"ucast_solicit" },
>   	{ CTL_INT,	NET_NEIGH_APP_SOLICIT,		"app_solicit" },
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +	{ CTL_INT,	NET_NEIGH_BCAST_SOLICIT,	"bcast_solicit" },
> +#endif
>   	/* NET_NEIGH_RETRANS_TIME "retrans_time" no longer used */
>   	{ CTL_INT,	NET_NEIGH_REACHABLE_TIME,	"base_reachable_time" },
>   	{ CTL_INT,	NET_NEIGH_DELAY_PROBE_TIME,	"delay_first_probe_time" },
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 70fe9e1..b372af4 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -936,10 +936,16 @@ static void neigh_timer_handler(unsigned long arg)
>   
>   	if ((neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) &&
>   	    atomic_read(&neigh->probes) >= neigh_max_probes(neigh)) {
> -		neigh->nud_state = NUD_FAILED;
> -		notify = 1;
> -		neigh_invalidate(neigh);
> -		goto out;
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +		int delta_probes = atomic_read(&neigh->probes) - neigh_max_probes(neigh);
> +		if (delta_probes >= NEIGH_VAR(neigh->parms, BCAST_PROBES))
> +#endif
> +		{
> +			neigh->nud_state = NUD_FAILED;
> +			notify = 1;
> +			neigh_invalidate(neigh);
> +			goto out;
> +		}
>   	}
>   
>   	if (neigh->nud_state & NUD_IN_TIMER) {
> @@ -973,6 +979,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
>   		goto out_unlock_bh;
>   
>   	if (!(neigh->nud_state & (NUD_STALE | NUD_INCOMPLETE))) {
> +		/* We are in (NUD_NONE | NUD_FAILED) */
>   		if (NEIGH_VAR(neigh->parms, MCAST_PROBES) +
>   		    NEIGH_VAR(neigh->parms, APP_PROBES)) {
>   			unsigned long next, now = jiffies;
> @@ -1783,6 +1790,10 @@ static int neightbl_fill_parms(struct sk_buff *skb, struct neigh_parms *parms)
>   			NEIGH_VAR(parms, UCAST_PROBES)) ||
>   	    nla_put_u32(skb, NDTPA_MCAST_PROBES,
>   			NEIGH_VAR(parms, MCAST_PROBES)) ||
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +	    nla_put_u32(skb, NDTPA_BCAST_PROBES,
> +			NEIGH_VAR(parms, BCAST_PROBES)) ||
> +#endif
>   	    nla_put_msecs(skb, NDTPA_REACHABLE_TIME, parms->reachable_time) ||
>   	    nla_put_msecs(skb, NDTPA_BASE_REACHABLE_TIME,
>   			  NEIGH_VAR(parms, BASE_REACHABLE_TIME)) ||
> @@ -1870,6 +1881,9 @@ static int neightbl_fill_info(struct sk_buff *skb, struct neigh_table *tbl,
>   			ndst.ndts_lookups		+= st->lookups;
>   			ndst.ndts_hits			+= st->hits;
>   			ndst.ndts_rcv_probes_mcast	+= st->rcv_probes_mcast;
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +			ndst.ndts_rcv_probes_bcast	+= st->rcv_probes_bcast;
> +#endif
>   			ndst.ndts_rcv_probes_ucast	+= st->rcv_probes_ucast;
>   			ndst.ndts_periodic_gc_runs	+= st->periodic_gc_runs;
>   			ndst.ndts_forced_gc_runs	+= st->forced_gc_runs;
> @@ -1942,6 +1956,9 @@ static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
>   	[NDTPA_APP_PROBES]		= { .type = NLA_U32 },
>   	[NDTPA_UCAST_PROBES]		= { .type = NLA_U32 },
>   	[NDTPA_MCAST_PROBES]		= { .type = NLA_U32 },
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +	[NDTPA_BCAST_PROBES]		= { .type = NLA_U32 },
> +#endif
>   	[NDTPA_BASE_REACHABLE_TIME]	= { .type = NLA_U64 },
>   	[NDTPA_GC_STALETIME]		= { .type = NLA_U64 },
>   	[NDTPA_DELAY_PROBE_TIME]	= { .type = NLA_U64 },
> @@ -2042,6 +2059,12 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh)
>   				NEIGH_VAR_SET(p, MCAST_PROBES,
>   					      nla_get_u32(tbp[i]));
>   				break;
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +			case NDTPA_BCAST_PROBES:
> +				NEIGH_VAR_SET(p, BCAST_PROBES,
> +					      nla_get_u32(tbp[i]));
> +				break;
> +#endif
>   			case NDTPA_BASE_REACHABLE_TIME:
>   				NEIGH_VAR_SET(p, BASE_REACHABLE_TIME,
>   					      nla_get_msecs(tbp[i]));
> @@ -2702,11 +2725,18 @@ static int neigh_stat_seq_show(struct seq_file *seq, void *v)
>   	struct neigh_statistics *st = v;
>   
>   	if (v == SEQ_START_TOKEN) {
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +		seq_printf(seq, "entries  allocs destroys hash_grows  lookups hits  res_failed  rcv_probes_bcast rcv_probes_mcast rcv_probes_ucast  periodic_gc_runs forced_gc_runs unresolved_discards\n");
> +#else
>   		seq_printf(seq, "entries  allocs destroys hash_grows  lookups hits  res_failed  rcv_probes_mcast rcv_probes_ucast  periodic_gc_runs forced_gc_runs unresolved_discards\n");
> +#endif
>   		return 0;
>   	}
>   
>   	seq_printf(seq, "%08x  %08lx %08lx %08lx  %08lx %08lx  %08lx  "
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +			"%08lx "
> +#endif
>   			"%08lx %08lx  %08lx %08lx %08lx\n",
>   		   atomic_read(&tbl->entries),
>   
> @@ -2719,6 +2749,9 @@ static int neigh_stat_seq_show(struct seq_file *seq, void *v)
>   
>   		   st->res_failed,
>   
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +		   st->rcv_probes_bcast,
> +#endif
>   		   st->rcv_probes_mcast,
>   		   st->rcv_probes_ucast,
>   
> @@ -2992,6 +3025,9 @@ static struct neigh_sysctl_table {
>   } neigh_sysctl_template __read_mostly = {
>   	.neigh_vars = {
>   		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(MCAST_PROBES, "mcast_solicit"),
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(BCAST_PROBES, "bcast_solicit"),
> +#endif
>   		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(UCAST_PROBES, "ucast_solicit"),
>   		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(APP_PROBES, "app_solicit"),
>   		NEIGH_SYSCTL_USERHZ_JIFFIES_ENTRY(RETRANS_TIME, "retrans_time"),
> diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
> index bd29016..2f43dc9 100644
> --- a/net/ipv4/Kconfig
> +++ b/net/ipv4/Kconfig
> @@ -1,6 +1,63 @@
>   #
>   # IP configuration
>   #
> +config IP_ARP_BROADCAST
> +	bool "IP: allow broadcast ARP"
> +	default N
> +	---help---
> +	  The stack will periodically refresh ARP cache by sending unicast ARP requests.
> +	  if a number of unicast ARP request fails, the stack will normally enter
> +	  FAILED state, and the ARP entry will be removed by the garbage collector.
> +	  Enabling this option will make the stack try to send broadcast ARP requests
> +	  if the unicast ARP requests fails, before entering FAILED state.
> +	  The number of broadcast packets is controlled by
> +	  '/proc/sys/net/ipv4/neigh/<dev>/bcast_solicit' which defaults to "0".
> +	  The parameter must be assigned a value to enable sending broadcast ARPs.
> +
> +	  If unsure, say N here.
> +
> +
> +config IP_ARP_UCAST_PROBES
> +	int "ucast_solicit initial value"
> +	default "3"
> +
> +	---help---
> +	  This value defines the initial value for how many unicast ARP requests
> +	  are sent by the ARP state machine in PROBE state.
> +	  It can be controlled runtime by '/proc/sys/net/ipv4/neigh/<dev>/ucast_solicit'
> +
> +	  If unsure, say "3" here.
> +
> +
> +config IP_ARP_MCAST_PROBES
> +	int "mcast_solicit initial value"
> +	default "3"
> +
> +	---help---
> +	  This value defines the initial value for how many broadcast ARP requests
> +	  are sent by the ARP state machine in INCOMPLETE state.
> +	  It can be controlled runtime by '/proc/sys/net/ipv4/neigh/<dev>/mcast_solicit'
> +
> +	  If unsure, say "3" here.
> +
> +
> +config IP_ARP_BCAST_PROBES
> +	int "bcast_solicit initial value"
> +	depends on IP_ARP_BROADCAST
> +	default "0"
> +
> +	---help---
> +	  This value defines the initial value for how many broadcast ARP requests
> +	  are sent by the ARP state machine in PROBE state.
> +	  It can be controlled runtime by '/proc/sys/net/ipv4/neigh/<dev>/bcast_solicit'
> +	  The default value of 'bcast_solicit' is zero, mimicking the behaviour of
> +	  older kernels which does not send out broadcast ARPs except for new entries.
> +	  If you want broadcast ARP requests to happen in PROBE state, set the number here.
> +	  To be compatible with IPv6, the number should be "3"
> +
> +	  If unsure, say "0" here.
> +
> +
>   config IP_MULTICAST
>   	bool "IP: multicasting"
>   	help
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 205e147..4795ab6 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -168,8 +168,11 @@ struct neigh_table arp_tbl = {
>   		.tbl			= &arp_tbl,
>   		.reachable_time		= 30 * HZ,
>   		.data	= {
> -			[NEIGH_VAR_MCAST_PROBES] = 3,
> -			[NEIGH_VAR_UCAST_PROBES] = 3,
> +			[NEIGH_VAR_MCAST_PROBES] = CONFIG_IP_ARP_MCAST_PROBES,
> +			[NEIGH_VAR_UCAST_PROBES] = CONFIG_IP_ARP_UCAST_PROBES,
> +#ifdef CONFIG_IP_ARP_BROADCAST
> +			[NEIGH_VAR_BCAST_PROBES] = CONFIG_IP_ARP_BCAST_PROBES,
> +#endif
>   			[NEIGH_VAR_RETRANS_TIME] = 1 * HZ,
>   			[NEIGH_VAR_BASE_REACHABLE_TIME] = 30 * HZ,
>   			[NEIGH_VAR_DELAY_PROBE_TIME] = 5 * HZ,
> 

-- 
吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
ミラクル・リナックス株式会社 技術本部 サポート部

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[YOSHIFUJI Hideaki]

Hi,

Ulf samuelsson wrote:
> If you want to send 3 unicasts followed by 3 broadcast,
> you have to set mcast_solicit to 6.
> This means you will send 6 broadcasts in incomplete state.
>
> I think the following configuration is wanted:
>
> Incomplete state: send 3 broadcast
> Probe state: send 3 unicast, followed by 3 broadcast then goto STALE OR
>                       Send 3 unicast, then go to STALE

I pointed out that we deal with nodes that won't reply against
unicast ARP/NDP via existing knobs.

If you really want such configuration, it is better to split
mcast_solicit.  "bcast_solicit" does not sound good to me.
You could say "probe_mcast_solicit".

--yoshfuji

>
> Best Regards
> Ulf Samuelsson
> ulf@emagii.com
> +46  (722) 427 437
>
>
>> 12 mar 2015 kl. 10:28 skrev YOSHIFUJI Hideaki/吉藤英明  <hideaki.yoshifuji@miraclelinux.com>:
>>
>> Hi,
>>
>> Ulf samuelsson wrote:
>>> That also means that you increase the number of broadcasts in incomplete state.
>>> Is that really desirable?
>>
>> In NUD_INCOMPLETE state, neighbour subsystem *skips* sending unicast probes
>> and send multicast probes mcast_solicit times. If you do not increase
>> the number of mcast_solicit, the number of multicast probes in
>> NUD_INCOMPLETE will not increased.
>>
>> --yoshfuji
>>
>>>
>>> Best Regards
>>> Ulf Samuelsson
>>> ulf@emagii.com
>>> +46  (722) 427 437
>>>
>>>
>>>> 12 mar 2015 kl. 09:42 skrev YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>:
>>>>
>>>> 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 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(-)
>>>>
>>>> --
>>>> Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
>>>> Technical Division, MIRACLE LINUX CORPORATION
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>> 吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
>> ミラクル・リナックス株式会社 技術本部 サポート部
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[David Miller]

Like Yoshifuji and others I agree that:

1) You can get working behavior by adjusting existing sysctls.

2) IPV4 ARP and IPV6 NDISC are not different in this regard.

I'm really tired of all of these ARP hacks being submitted recently,
and I want people to think more deeply about what they are proposing
first.

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[Ulf Samuelsson]

On 03/12/2015 08:22 PM, David Miller wrote:
> Like Yoshifuji and others I agree that:
>
> 1) You can get working behavior by adjusting existing sysctls.
>
> 2) IPV4 ARP and IPV6 NDISC are not different in this regard.
>
> I'm really tired of all of these ARP hacks being submitted recently,
> and I want people to think more deeply about what they are proposing
> first.
Which sysctls are you referring to?

As I can see it, there are three sysctls that control this.

ucast_solicit.
mcast_solicit
app_solicit.

If you think any other sysctl is useful here, which one?

 From Documentation/networking/ip-sysctl.txt.
+++++++++++++
app_solicit - INTEGER
     The maximum number of probes to send to the user space ARP daemon
     via netlink before dropping back to multicast probes (see
     mcast_solicit).  Defaults to 0.
----------------------------
so that ain't it.
If you disagree, what should the Documentation look like?

=============================================
 From net/core/neighbour.c(timer_handler):

     if ((neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) &&
         atomic_read(&neigh->probes) >= neigh_max_probes(neigh)) {
         neigh->nud_state = NUD_FAILED;
         notify = 1;
         neigh_invalidate(neigh);
         goto out;
     }

 From net/core/neighbour.c(neigh_max_probes):
static __inline__ int neigh_max_probes(struct neighbour *n)
{
     struct neigh_parms *p = n->parms;
     int max_probes = NEIGH_VAR(p, UCAST_PROBES) + NEIGH_VAR(p, APP_PROBES);
     if (!(n->nud_state & NUD_PROBE))
         max_probes += NEIGH_VAR(p, MCAST_PROBES);
     return max_probes;
}
=============================================


If we simplify this when we are in NUD_PROBE state we see:

=============================================
     if (atomic_read(&neigh->probes) >= NEIGH_VAR(p, UCAST_PROBES) + 
NEIGH_VAR(p, APP_PROBES)) {
         neigh->nud_state = NUD_FAILED;
         notify = 1;
         neigh_invalidate(neigh);
         goto out;
     }
=============================================

Since APP_PROBES is 0, and according to documentation has nothing to do 
with Broadcast:

=============================================
     if (atomic_read(&neigh->probes) >= NEIGH_VAR(p, UCAST_PROBES)) {
         neigh->nud_state = NUD_FAILED;
         notify = 1;
         neigh_invalidate(neigh);
         goto out;
     }
=============================================

so we will send out "ucast_solicit" unikcast probes, and then we will 
enter FAILED state.
mcast_solicit is ignored when in NUD_PROBE state.

"mcast_solicit" is only used when in NUD_INCOMPLETE state, and 
broadcasts will never
be sent out by the stack for an entry, once it is in NUD_REACHABLE for 
the first time.

I would be happy, if you can show me that I have misunderstood something.

Best Regards,
Ulf Samuelsson

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[Ulf Samuelsson]

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.

I do not see that the stack is returning to INCOMPLETE
after it has reached REACHABLE once.

Can it do so? In that case, in what part of the code.

=============================
int    confirmed;     /* time of last ARP reply */
int    ucast_solicit;    /* sysctl */
int    app_solicit;    /* sysctl */
neigh_timer_handler()
begin
     if (in REACHABLE) then
         if ("current time" <=( confirmed + reachable)) then
             we are OK, test later
         else if ("current time" <= used + DELAY_PROBE_TIME) then
             set state to DELAY
         else
             set state to STALE
         end if
     else if (in DELAY) then
         if ("current time" <= confirmed + DELAY_PROBE_TIME) then
             change state to REACHABLE
             send notification
         else
             change state to PROBE
             probes = 0;
             do not send notification
         end if

     if (in PROBE state) then
         if (probes >= (ucast_solicit + app_solicit)) then
             change state to FAILED
             send notification
         end if
     end if

     if (in PROBE state) then
         send ARP request;
     end if

     if (notify) then
         send notification
     end if
end

====================
Anyway, the behaviour I would like to see is:

INCOMPLETE:
     Send 3 broadcast ARP
PROBE
     Send 3 unicast ARP
     Send 3 broadcast ARP

which not possible with your suggestion
I do not want to send 6 broadcast ARP in INCOMPLETE state.


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(-)
>>>
>>
>

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[YOSHIFUJI Hideaki/吉藤英明]

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@linux-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,

--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(-)
>>>>
>>>
>>
>

-- 
吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
ミラクル・リナックス株式会社 技術本部 サポート部

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[Ulf Samuelsson]

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@linux-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(-)
>>>>>
>>>>
>>>
>>
>

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[YOSHIFUJI Hideaki/吉藤英明]

Hi,

Ulf Samuelsson wrote:
> 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.
>>>>>My proposal is rather fix my ancient mistake.

>>>>> 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@linux-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".

Sorry, I meant "probe_mcast_solicit", as you see, which denotes
"mcast_solicit" in PROBE state.  I do not prefer "bcast" because
1) it is not a broadcast for IPv6 and 2) it is not descriptive
about the affected state.

> 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 "probe_mcast_solicit" variable can be (and MUST be) set per
interface, per protocol basis, so I do not think it will affect
IPv6 if the variable is set properly, and it should be done by
default.

>
> 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.

Please do not do this; it becomes more complex.

Thanks.

--yoshfuji

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[Ulf Samuelsson]

On 03/18/2015 02:22 PM, YOSHIFUJI Hideaki/吉藤英明 wrote:
> Hi,
>
> Ulf Samuelsson wrote:
>> 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.
>>>>>> My proposal is rather fix my ancient mistake.
>
>>>>>> 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@linux-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".
>
> Sorry, I meant "probe_mcast_solicit", as you see, which denotes
> "mcast_solicit" in PROBE state.  I do not prefer "bcast" because
> 1) it is not a broadcast for IPv6 and 2) it is not descriptive
> about the affected state.
>

"mcast_probe_solicit" is better than "probe_mcast_solicit" since they 
will  sort together.

Another alternative would be to change "mcast_solicit" from an integer 
to a record

echo    "1"     > mcast_solicit            # set # broadcasts in 
incomplete state
echo    "1,3"  > mcast_solicit            # set # broadcasts in 
incomplete and probe state.

could be "1:3", "{1,3}" or whatever syntax is most suitable for existing 
kernel parsers.

Think I like this more

The two values have to be assigned to two different parameters.

>> 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 "probe_mcast_solicit" variable can be (and MUST be) set per
> interface, per protocol basis, so I do not think it will affect
> IPv6 if the variable is set properly, and it should be done by
> default.
>
You are right, so then it would work to set neigh_max_probes
     to use the second parameter of mcast_solicit.

One question is what the sane default is.

For large datacenters I could see that you want the new sysctl to be set 
to 0.

For home users, then it should be set to a low number like 3.

I have problems at home where machines lose contact with each other at 
startup.
Windows/Linux machines are not found
After 5-10 minutes, they can suddenly appear.
I suspect it is related to this issue.

Have no problems with a few broadcasts at home.
I have problem if machines cannot be found.


>>
>> 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.
>
> Please do not do this; it becomes more complex.
>
> Thanks.
>
> --yoshfuji

Best Regards,
Ulf Samuelsson
KI/EAB/ILM/GF
ulf.samuelsson@ericsson.com
+46 722 427 437

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[yzhu1]

On 03/18/2015 08:15 PM, Ulf Samuelsson wrote:
> 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@linux-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.
Yes. I prefer to this solution since this only affects IPv4.

Best Regards!
Zhu Yanjun
>
> 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(-)
>>>>>>
>>>>>
>>>>
>>>
>>
>
>
>
>

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[yzhu1]

On 03/18/2015 04:51 PM, 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.
>
> I do not see that the stack is returning to INCOMPLETE
> after it has reached REACHABLE once.
>
> Can it do so? In that case, in what part of the code.
>
> =============================
> int    confirmed;     /* time of last ARP reply */
> int    ucast_solicit;    /* sysctl */
> int    app_solicit;    /* sysctl */
> neigh_timer_handler()
> begin
>     if (in REACHABLE) then
>         if ("current time" <=( confirmed + reachable)) then
>             we are OK, test later
>         else if ("current time" <= used + DELAY_PROBE_TIME) then
>             set state to DELAY
>         else
>             set state to STALE
>         end if
>     else if (in DELAY) then
>         if ("current time" <= confirmed + DELAY_PROBE_TIME) then
>             change state to REACHABLE
>             send notification
>         else
>             change state to PROBE
>             probes = 0;
>             do not send notification
>         end if
>
>     if (in PROBE state) then
>         if (probes >= (ucast_solicit + app_solicit)) then
>             change state to FAILED
>             send notification
>         end if
>     end if
>
>     if (in PROBE state) then
>         send ARP request;
>     end if
>
>     if (notify) then
>         send notification
>     end if
> end
>
> ====================
> Anyway, the behaviour I would like to see is:
>
> INCOMPLETE:
>     Send 3 broadcast ARP
> PROBE
>     Send 3 unicast ARP
>     Send 3 broadcast ARP
>
> which not possible with your suggestion
> I do not want to send 6 broadcast ARP in INCOMPLETE state.
Yes. I agree with you. I made tests for several days. I can not 
reproduce this with the method from YOSHIFUJI.

Best Regards!
Zhu Yanjun

>
>
> 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(-)
>>>>
>>>
>>
>
>
>

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[yzhu1]

On 03/18/2015 09:22 PM, YOSHIFUJI Hideaki/吉藤英明 wrote:
> Hi,
>
> Ulf Samuelsson wrote:
>> 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.
>>>>>> My proposal is rather fix my ancient mistake.
>
>>>>>> 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@linux-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".
>
> Sorry, I meant "probe_mcast_solicit", as you see, which denotes
> "mcast_solicit" in PROBE state.  I do not prefer "bcast" because
> 1) it is not a broadcast for IPv6 and 2) it is not descriptive
> about the affected state.
>
>> 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 "probe_mcast_solicit" variable can be (and MUST be) set per
> interface, per protocol basis, so I do not think it will affect
> IPv6 if the variable is set properly, and it should be done by
> default.
>
>>
>> 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.
>
> Please do not do this; it becomes more complex.
This can avoid risk to IPv6. I think it should put in the timer handler.

Best Regards!
Zhu Yanjun

>
> Thanks.
>
> --yoshfuji
>
>

Re: [PATCH V2 0/1] neighbour: Support broadcast ARP in neighbor PROPE state

[yzhu1]

On 03/16/2015 08:39 PM, Ulf Samuelsson wrote:
> On 03/12/2015 08:22 PM, David Miller wrote:
>> Like Yoshifuji and others I agree that:
>>
>> 1) You can get working behavior by adjusting existing sysctls.
>>
>> 2) IPV4 ARP and IPV6 NDISC are not different in this regard.
>>
>> I'm really tired of all of these ARP hacks being submitted recently,
>> and I want people to think more deeply about what they are proposing
>> first.
> Which sysctls are you referring to?
>
> As I can see it, there are three sysctls that control this.
>
> ucast_solicit.
> mcast_solicit
> app_solicit.
>
> If you think any other sysctl is useful here, which one?
>
> From Documentation/networking/ip-sysctl.txt.
> +++++++++++++
> app_solicit - INTEGER
>     The maximum number of probes to send to the user space ARP daemon
>     via netlink before dropping back to multicast probes (see
>     mcast_solicit).  Defaults to 0.
> ----------------------------
> so that ain't it.
> If you disagree, what should the Documentation look like?
>
> =============================================
> From net/core/neighbour.c(timer_handler):
>
>     if ((neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) &&
>         atomic_read(&neigh->probes) >= neigh_max_probes(neigh)) {
>         neigh->nud_state = NUD_FAILED;
>         notify = 1;
>         neigh_invalidate(neigh);
>         goto out;
>     }
>
> From net/core/neighbour.c(neigh_max_probes):
> static __inline__ int neigh_max_probes(struct neighbour *n)
> {
>     struct neigh_parms *p = n->parms;
>     int max_probes = NEIGH_VAR(p, UCAST_PROBES) + NEIGH_VAR(p, 
> APP_PROBES);
>     if (!(n->nud_state & NUD_PROBE))
>         max_probes += NEIGH_VAR(p, MCAST_PROBES);
>     return max_probes;
> }
> =============================================
>
>
> If we simplify this when we are in NUD_PROBE state we see:
>
> =============================================
>     if (atomic_read(&neigh->probes) >= NEIGH_VAR(p, UCAST_PROBES) + 
> NEIGH_VAR(p, APP_PROBES)) {
>         neigh->nud_state = NUD_FAILED;
>         notify = 1;
>         neigh_invalidate(neigh);
>         goto out;
>     }
> =============================================
>
> Since APP_PROBES is 0, and according to documentation has nothing to 
> do with Broadcast:
>
> =============================================
>     if (atomic_read(&neigh->probes) >= NEIGH_VAR(p, UCAST_PROBES)) {
>         neigh->nud_state = NUD_FAILED;
>         notify = 1;
>         neigh_invalidate(neigh);
>         goto out;
>     }
> =============================================
>
> so we will send out "ucast_solicit" unikcast probes, and then we will 
> enter FAILED state.
> mcast_solicit is ignored when in NUD_PROBE state.
Yes. I agree with you. That is why I add the bcast_probe here.

>
> "mcast_solicit" is only used when in NUD_INCOMPLETE state, and 
> broadcasts will never
> be sent out by the stack for an entry, once it is in NUD_REACHABLE for 
> the first time.
We can see this in "Figure 26-13. Transitions among NUD states" of 
Understanding Linux Network Internals.

Zhu Yanjun
>
> I would be happy, if you can show me that I have misunderstood something.
>
> Best Regards,
> Ulf Samuelsson
>
>
>