Subject: Re: [PATCH net-next 1/2] net: neighbour: Add mcast_resolicit to configure the number of multicast resolicitations in PROBE state.

From: Ulf Samuelsson <[email-masked]>
Date: Fri, 27 Mar 2015 10:42:10 +0100

Should not this patch add modifications to "kernel/sysctl_binary.c" and
"include/linux/sysctl.h" as well?

mcast_probes and ucast_probes are initialized in arp.c and ndisc.c.
Should we not initialize also mcast_reprobes, even if it is zero?

There are some statistics printout in net/core/neighbour.c.
They handle mcast_solicit and ucast_solicit.

Are they unneccessary for mcast_resolicit?

Best Regards,
Ulf Samuelsson

On 03/19/2015 02:41 PM, YOSHIFUJI Hideaki wrote:

> We send unicast neighbor (ARP or NDP) solicitations ucast_probes
> times in PROBE state.  Zhu Yanjun reported that some implementation
> does not reply against them and the entry will become FAILED, which
> is undesirable.
>
> We had been dealt with such nodes by sending multicast probes mcast_
> solicit times after unicast probes in PROBE state.  In 2003, I made
> a change not to send them to improve compatibility with IPv6 NDP.
>
> Let's introduce per-protocol per-interface sysctl knob "mcast_
> reprobe" to configure the number of multicast (re)solicitation for
> reconfirmation in PROBE state.  The default is 0, since we have
> been doing so for 10+ years.
>
> Reported-by: Zhu Yanjun <[email-masked]>
> CC: Ulf Samuelsson <[email-masked]>
> Signed-off-by: YOSHIFUJI Hideaki <[email-masked]>
> Signed-off-by: YOSHIFUJI Hideaki <[email-masked]>
> ---
>   include/net/neighbour.h        |  1 +
>   include/uapi/linux/neighbour.h |  1 +
>   net/core/neighbour.c           | 15 +++++++++++----
>   3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index e7bdf51..bd33e66 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -42,6 +42,7 @@ enum {
>   	NEIGH_VAR_MCAST_PROBES,
>   	NEIGH_VAR_UCAST_PROBES,
>   	NEIGH_VAR_APP_PROBES,
> +	NEIGH_VAR_MCAST_REPROBES,
>   	NEIGH_VAR_RETRANS_TIME,
>   	NEIGH_VAR_BASE_REACHABLE_TIME,
>   	NEIGH_VAR_DELAY_PROBE_TIME,
> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
> index 3873a35..2e35c61 100644
> --- a/include/uapi/linux/neighbour.h
> +++ b/include/uapi/linux/neighbour.h
> @@ -126,6 +126,7 @@ enum {
>   	NDTPA_PROXY_QLEN,		/* u32 */
>   	NDTPA_LOCKTIME,			/* u64, msecs */
>   	NDTPA_QUEUE_LENBYTES,		/* u32 */
> +	NDTPA_MCAST_REPROBES,		/* u32 */
>   	__NDTPA_MAX
>   };
>   #define NDTPA_MAX (__NDTPA_MAX - 1)
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 0e8b32e..3de6542 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -817,10 +817,9 @@ out:
>   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;
> +	return NEIGH_VAR(p, UCAST_PROBES) + NEIGH_VAR(p, APP_PROBES) +
> +	       (n->nud_state & NUD_PROBE ? NEIGH_VAR(p, MCAST_REPROBES) :
> +	        NEIGH_VAR(p, MCAST_PROBES));
>   }
>   
>   static void neigh_invalidate(struct neighbour *neigh)
> @@ -1742,6 +1741,8 @@ 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)) ||
> +	    nla_put_u32(skb, NDTPA_MCAST_REPROBES,
> +			NEIGH_VAR(parms, MCAST_REPROBES)) ||
>   	    nla_put_msecs(skb, NDTPA_REACHABLE_TIME, parms->reachable_time) ||
>   	    nla_put_msecs(skb, NDTPA_BASE_REACHABLE_TIME,
>   			  NEIGH_VAR(parms, BASE_REACHABLE_TIME)) ||
> @@ -1901,6 +1902,7 @@ 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 },
> +	[NDTPA_MCAST_REPROBES]		= { .type = NLA_U32 },
>   	[NDTPA_BASE_REACHABLE_TIME]	= { .type = NLA_U64 },
>   	[NDTPA_GC_STALETIME]		= { .type = NLA_U64 },
>   	[NDTPA_DELAY_PROBE_TIME]	= { .type = NLA_U64 },
> @@ -2001,6 +2003,10 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh)
>   				NEIGH_VAR_SET(p, MCAST_PROBES,
>   					      nla_get_u32(tbp[i]));
>   				break;
> +			case NDTPA_MCAST_REPROBES:
> +				NEIGH_VAR_SET(p, MCAST_REPROBES,
> +					      nla_get_u32(tbp[i]));
> +				break;
>   			case NDTPA_BASE_REACHABLE_TIME:
>   				NEIGH_VAR_SET(p, BASE_REACHABLE_TIME,
>   					      nla_get_msecs(tbp[i]));
> @@ -2987,6 +2993,7 @@ static struct neigh_sysctl_table {
>   		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(MCAST_PROBES, "mcast_solicit"),
>   		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(UCAST_PROBES, "ucast_solicit"),
>   		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(APP_PROBES, "app_solicit"),
> +		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(MCAST_REPROBES, "mcast_resolicit"),
>   		NEIGH_SYSCTL_USERHZ_JIFFIES_ENTRY(RETRANS_TIME, "retrans_time"),
>   		NEIGH_SYSCTL_JIFFIES_ENTRY(BASE_REACHABLE_TIME, "base_reachable_time"),
>   		NEIGH_SYSCTL_JIFFIES_ENTRY(DELAY_PROBE_TIME, "delay_first_probe_time"),