Commit Message

Since commit 7e98056964("ipv6: router reachability probing"), a router falls
into NUD_FAILED will be probed.
Now if function rt6_select() selects a router which neighbour state is NUD_FAILED,
and at the same time function rt6_probe() changes the neighbour state to NUD_PROBE,
then function dst_neigh_output() can directly send packets, but actually the
neighbour still is unreachable. If we set nud_state to NUD_INCOMPLETE instead
NUD_PROBE, packets will not be sent out until the neihbour is reachable.
In addition, because the route should be probes with a single NS, so we must
set neigh->probes to neigh_max_probes(), then the neigh timer timeout and function
neigh_timer_handler() will not send other NS Messages.
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
Changes from v1:
*modify changelog to explain in detail why use neigh_max_probes().
net/core/neighbour.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Comments

On Thu, May 8, 2014, at 22:16, Duan Jiong wrote:
> > Since commit 7e98056964("ipv6: router reachability probing"), a router falls> into NUD_FAILED will be probed.> > Now if function rt6_select() selects a router which neighbour state is NUD_FAILED,> and at the same time function rt6_probe() changes the neighbour state to NUD_PROBE,> then function dst_neigh_output() can directly send packets, but actually the> neighbour still is unreachable. If we set nud_state to NUD_INCOMPLETE instead> NUD_PROBE, packets will not be sent out until the neihbour is reachable.> > In addition, because the route should be probes with a single NS, so we must> set neigh->probes to neigh_max_probes(), then the neigh timer timeout and function> neigh_timer_handler() will not send other NS Messages.> > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>> ---> Changes from v1:> *modify changelog to explain in detail why use neigh_max_probes().> > net/core/neighbour.c | 4 ++--> 1 file changed, 2 insertions(+), 2 deletions(-)> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c> index 8f8a96e..32d872e 100644> --- a/net/core/neighbour.c> +++ b/net/core/neighbour.c> @@ -1248,8 +1248,8 @@ void __neigh_set_probe_once(struct neighbour *neigh)> neigh->updated = jiffies;> if (!(neigh->nud_state & NUD_FAILED))> return;> - neigh->nud_state = NUD_PROBE;> - atomic_set(&neigh->probes, NEIGH_VAR(neigh->parms, UCAST_PROBES));> + neigh->nud_state = NUD_INCOMPLETE;> + atomic_set(&neigh->probes, neigh_max_probes(neigh));
Wouldn't it be better if we neigh_suspect the neighbour and leaving the state in NUD_PROBE? We call down to ->output in case neighbour is in NUD_PROBE state, so we must just disable connected 'fast-path'.
Greetings,
Hannes
--
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

于 2014年05月12日 06:04, Hannes Frederic Sowa 写道:
> On Thu, May 8, 2014, at 22:16, Duan Jiong wrote:>>>> Since commit 7e98056964("ipv6: router reachability probing"), a router falls>> into NUD_FAILED will be probed.>>>> Now if function rt6_select() selects a router which neighbour state is NUD_FAILED,>> and at the same time function rt6_probe() changes the neighbour state to NUD_PROBE,>> then function dst_neigh_output() can directly send packets, but actually the>> neighbour still is unreachable. If we set nud_state to NUD_INCOMPLETE instead>> NUD_PROBE, packets will not be sent out until the neihbour is reachable.>>>> In addition, because the route should be probes with a single NS, so we must>> set neigh->probes to neigh_max_probes(), then the neigh timer timeout and function>> neigh_timer_handler() will not send other NS Messages.>>>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>>> --->> Changes from v1:>> *modify changelog to explain in detail why use neigh_max_probes().>>>> net/core/neighbour.c | 4 ++-->> 1 file changed, 2 insertions(+), 2 deletions(-)>>>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c>> index 8f8a96e..32d872e 100644>> --- a/net/core/neighbour.c>> +++ b/net/core/neighbour.c>> @@ -1248,8 +1248,8 @@ void __neigh_set_probe_once(struct neighbour *neigh)>> neigh->updated = jiffies;>> if (!(neigh->nud_state & NUD_FAILED))>> return;>> - neigh->nud_state = NUD_PROBE;>> - atomic_set(&neigh->probes, NEIGH_VAR(neigh->parms, UCAST_PROBES));>> + neigh->nud_state = NUD_INCOMPLETE;>> + atomic_set(&neigh->probes, neigh_max_probes(neigh));> > Wouldn't it be better if we neigh_suspect the neighbour and leaving the state in NUD_PROBE? We call down to ->output in case neighbour is in NUD_PROBE state, so we must just disable connected 'fast-path'.>
You can look into neigh_event_send() called in neigh_resolve_output(), and if neigh->nud_state
still is NUD_PROBE, the neigh_event_send() will return 0, so the packet will still be sent out
without probe.
So, using neigh_suspect is not a good idea.
Thanks,
Duan
> Greetings,> > Hannes> .>
--
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

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Mon, 12 May 2014 11:26:10 +0800
> 于 2014年05月12日 06:04, Hannes Frederic Sowa 写道:>> On Thu, May 8, 2014, at 22:16, Duan Jiong wrote:>>>>>> Since commit 7e98056964("ipv6: router reachability probing"), a router falls>>> into NUD_FAILED will be probed.>>>>>> Now if function rt6_select() selects a router which neighbour state is NUD_FAILED,>>> and at the same time function rt6_probe() changes the neighbour state to NUD_PROBE,>>> then function dst_neigh_output() can directly send packets, but actually the>>> neighbour still is unreachable. If we set nud_state to NUD_INCOMPLETE instead>>> NUD_PROBE, packets will not be sent out until the neihbour is reachable.>>>>>> In addition, because the route should be probes with a single NS, so we must>>> set neigh->probes to neigh_max_probes(), then the neigh timer timeout and function>>> neigh_timer_handler() will not send other NS Messages.>>>>>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>>>> --->>> Changes from v1:>>> *modify changelog to explain in detail why use neigh_max_probes().>>>>>> net/core/neighbour.c | 4 ++-->>> 1 file changed, 2 insertions(+), 2 deletions(-)>>>>>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c>>> index 8f8a96e..32d872e 100644>>> --- a/net/core/neighbour.c>>> +++ b/net/core/neighbour.c>>> @@ -1248,8 +1248,8 @@ void __neigh_set_probe_once(struct neighbour *neigh)>>> neigh->updated = jiffies;>>> if (!(neigh->nud_state & NUD_FAILED))>>> return;>>> - neigh->nud_state = NUD_PROBE;>>> - atomic_set(&neigh->probes, NEIGH_VAR(neigh->parms, UCAST_PROBES));>>> + neigh->nud_state = NUD_INCOMPLETE;>>> + atomic_set(&neigh->probes, neigh_max_probes(neigh));>> >> Wouldn't it be better if we neigh_suspect the neighbour and leaving the state in NUD_PROBE? We call down to ->output in case neighbour is in NUD_PROBE state, so we must just disable connected 'fast-path'.>> > > You can look into neigh_event_send() called in neigh_resolve_output(), and if neigh->nud_state> still is NUD_PROBE, the neigh_event_send() will return 0, so the packet will still be sent out> without probe.> > So, using neigh_suspect is not a good idea.
If you set it to NUD_INCOMPLETE however, neigh_event_send() is going to add the packet
to the neigh's ARP queue and return '1'.
Is that really what you want to happen in this case?
--
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

于 2014年05月13日 02:37, David Miller 写道:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>> Date: Mon, 12 May 2014 11:26:10 +0800> >> 于 2014年05月12日 06:04, Hannes Frederic Sowa 写道:>>> On Thu, May 8, 2014, at 22:16, Duan Jiong wrote:>>>>>>>> Since commit 7e98056964("ipv6: router reachability probing"), a router falls>>>> into NUD_FAILED will be probed.>>>>>>>> Now if function rt6_select() selects a router which neighbour state is NUD_FAILED,>>>> and at the same time function rt6_probe() changes the neighbour state to NUD_PROBE,>>>> then function dst_neigh_output() can directly send packets, but actually the>>>> neighbour still is unreachable. If we set nud_state to NUD_INCOMPLETE instead>>>> NUD_PROBE, packets will not be sent out until the neihbour is reachable.>>>>>>>> In addition, because the route should be probes with a single NS, so we must>>>> set neigh->probes to neigh_max_probes(), then the neigh timer timeout and function>>>> neigh_timer_handler() will not send other NS Messages.>>>>>>>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>>>>> --->>>> Changes from v1:>>>> *modify changelog to explain in detail why use neigh_max_probes().>>>>>>>> net/core/neighbour.c | 4 ++-->>>> 1 file changed, 2 insertions(+), 2 deletions(-)>>>>>>>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c>>>> index 8f8a96e..32d872e 100644>>>> --- a/net/core/neighbour.c>>>> +++ b/net/core/neighbour.c>>>> @@ -1248,8 +1248,8 @@ void __neigh_set_probe_once(struct neighbour *neigh)>>>> neigh->updated = jiffies;>>>> if (!(neigh->nud_state & NUD_FAILED))>>>> return;>>>> - neigh->nud_state = NUD_PROBE;>>>> - atomic_set(&neigh->probes, NEIGH_VAR(neigh->parms, UCAST_PROBES));>>>> + neigh->nud_state = NUD_INCOMPLETE;>>>> + atomic_set(&neigh->probes, neigh_max_probes(neigh));>>>>>> Wouldn't it be better if we neigh_suspect the neighbour and leaving the state in NUD_PROBE? We call down to ->output in case neighbour is in NUD_PROBE state, so we must just disable connected 'fast-path'.>>>>>>> You can look into neigh_event_send() called in neigh_resolve_output(), and if neigh->nud_state>> still is NUD_PROBE, the neigh_event_send() will return 0, so the packet will still be sent out>> without probe.>>>> So, using neigh_suspect is not a good idea.> > If you set it to NUD_INCOMPLETE however, neigh_event_send() is going to add the packet> to the neigh's ARP queue and return '1'.> > Is that really what you want to happen in this case?
Yes, packets should not be sent out until the neihbour is reachable.
> --> 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> .>
--
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

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Tue, 13 May 2014 09:07:12 +0800
> 于 2014年05月13日 02:37, David Miller 写道:>> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>>> Date: Mon, 12 May 2014 11:26:10 +0800>> >>> 于 2014年05月12日 06:04, Hannes Frederic Sowa 写道:>>>> On Thu, May 8, 2014, at 22:16, Duan Jiong wrote:>>>>>>>>>> Since commit 7e98056964("ipv6: router reachability probing"), a router falls>>>>> into NUD_FAILED will be probed.>>>>>>>>>> Now if function rt6_select() selects a router which neighbour state is NUD_FAILED,>>>>> and at the same time function rt6_probe() changes the neighbour state to NUD_PROBE,>>>>> then function dst_neigh_output() can directly send packets, but actually the>>>>> neighbour still is unreachable. If we set nud_state to NUD_INCOMPLETE instead>>>>> NUD_PROBE, packets will not be sent out until the neihbour is reachable.>>>>>>>>>> In addition, because the route should be probes with a single NS, so we must>>>>> set neigh->probes to neigh_max_probes(), then the neigh timer timeout and function>>>>> neigh_timer_handler() will not send other NS Messages.>>>>>>>>>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>>>>>> --->>>>> Changes from v1:>>>>> *modify changelog to explain in detail why use neigh_max_probes().>>>>>>>>>> net/core/neighbour.c | 4 ++-->>>>> 1 file changed, 2 insertions(+), 2 deletions(-)>>>>>>>>>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c>>>>> index 8f8a96e..32d872e 100644>>>>> --- a/net/core/neighbour.c>>>>> +++ b/net/core/neighbour.c>>>>> @@ -1248,8 +1248,8 @@ void __neigh_set_probe_once(struct neighbour *neigh)>>>>> neigh->updated = jiffies;>>>>> if (!(neigh->nud_state & NUD_FAILED))>>>>> return;>>>>> - neigh->nud_state = NUD_PROBE;>>>>> - atomic_set(&neigh->probes, NEIGH_VAR(neigh->parms, UCAST_PROBES));>>>>> + neigh->nud_state = NUD_INCOMPLETE;>>>>> + atomic_set(&neigh->probes, neigh_max_probes(neigh));>>>>>>>> Wouldn't it be better if we neigh_suspect the neighbour and leaving the state in NUD_PROBE? We call down to ->output in case neighbour is in NUD_PROBE state, so we must just disable connected 'fast-path'.>>>>>>>>>> You can look into neigh_event_send() called in neigh_resolve_output(), and if neigh->nud_state>>> still is NUD_PROBE, the neigh_event_send() will return 0, so the packet will still be sent out>>> without probe.>>>>>> So, using neigh_suspect is not a good idea.>> >> If you set it to NUD_INCOMPLETE however, neigh_event_send() is going to add the packet>> to the neigh's ARP queue and return '1'.>> >> Is that really what you want to happen in this case?> > Yes, packets should not be sent out until the neihbour is reachable.
But shouldn't we be using the router in the list which did not enter
NUD_FAILED in this situation?
--
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

于 2014年05月13日 11:25, David Miller 写道:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>> Date: Tue, 13 May 2014 09:07:12 +0800> >> 于 2014年05月13日 02:37, David Miller 写道:>>> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>>>> Date: Mon, 12 May 2014 11:26:10 +0800>>>>>>> 于 2014年05月12日 06:04, Hannes Frederic Sowa 写道:>>>>> On Thu, May 8, 2014, at 22:16, Duan Jiong wrote:>>>>>>>>>>>> Since commit 7e98056964("ipv6: router reachability probing"), a router falls>>>>>> into NUD_FAILED will be probed.>>>>>>>>>>>> Now if function rt6_select() selects a router which neighbour state is NUD_FAILED,>>>>>> and at the same time function rt6_probe() changes the neighbour state to NUD_PROBE,>>>>>> then function dst_neigh_output() can directly send packets, but actually the>>>>>> neighbour still is unreachable. If we set nud_state to NUD_INCOMPLETE instead>>>>>> NUD_PROBE, packets will not be sent out until the neihbour is reachable.>>>>>>>>>>>> In addition, because the route should be probes with a single NS, so we must>>>>>> set neigh->probes to neigh_max_probes(), then the neigh timer timeout and function>>>>>> neigh_timer_handler() will not send other NS Messages.>>>>>>>>>>>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>>>>>>> --->>>>>> Changes from v1:>>>>>> *modify changelog to explain in detail why use neigh_max_probes().>>>>>>>>>>>> net/core/neighbour.c | 4 ++-->>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)>>>>>>>>>>>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c>>>>>> index 8f8a96e..32d872e 100644>>>>>> --- a/net/core/neighbour.c>>>>>> +++ b/net/core/neighbour.c>>>>>> @@ -1248,8 +1248,8 @@ void __neigh_set_probe_once(struct neighbour *neigh)>>>>>> neigh->updated = jiffies;>>>>>> if (!(neigh->nud_state & NUD_FAILED))>>>>>> return;>>>>>> - neigh->nud_state = NUD_PROBE;>>>>>> - atomic_set(&neigh->probes, NEIGH_VAR(neigh->parms, UCAST_PROBES));>>>>>> + neigh->nud_state = NUD_INCOMPLETE;>>>>>> + atomic_set(&neigh->probes, neigh_max_probes(neigh));>>>>>>>>>> Wouldn't it be better if we neigh_suspect the neighbour and leaving the state in NUD_PROBE? We call down to ->output in case neighbour is in NUD_PROBE state, so we must just disable connected 'fast-path'.>>>>>>>>>>>>> You can look into neigh_event_send() called in neigh_resolve_output(), and if neigh->nud_state>>>> still is NUD_PROBE, the neigh_event_send() will return 0, so the packet will still be sent out>>>> without probe.>>>>>>>> So, using neigh_suspect is not a good idea.>>>>>> If you set it to NUD_INCOMPLETE however, neigh_event_send() is going to add the packet>>> to the neigh's ARP queue and return '1'.>>>>>> Is that really what you want to happen in this case?>>>> Yes, packets should not be sent out until the neihbour is reachable.> > But shouldn't we be using the router in the list which did not enter> NUD_FAILED in this situation?> .
In situation of connected router, the router which enter NUD_FAILED will
never be choosed, and we only probe NUD_FAILED router's reachability by sending
a single Neighbor Solicitation to that router's address. Finally， we
still send packets out through the connected router.
--
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

On Mon, May 12, 2014, at 22:12, Duan Jiong wrote:
> In situation of connected router, the router which enter NUD_FAILED will> never be choosed, and we only probe NUD_FAILED router's reachability by> sending> a single Neighbor Solicitation to that router's address. Finally， we > still send packets out through the connected router.
I agree with the patch mostly. The reason why I would liked to see
another change without changing nud_state is that we export those states
to user space.
Greetings,
Hannes
--
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

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Fri, 9 May 2014 13:16:48 +0800
> Since commit 7e98056964("ipv6: router reachability probing"), a router falls> into NUD_FAILED will be probed.> > Now if function rt6_select() selects a router which neighbour state is NUD_FAILED,> and at the same time function rt6_probe() changes the neighbour state to NUD_PROBE,> then function dst_neigh_output() can directly send packets, but actually the> neighbour still is unreachable. If we set nud_state to NUD_INCOMPLETE instead> NUD_PROBE, packets will not be sent out until the neihbour is reachable.> > In addition, because the route should be probes with a single NS, so we must> set neigh->probes to neigh_max_probes(), then the neigh timer timeout and function> neigh_timer_handler() will not send other NS Messages.> > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Applied and queued up for -stable, thanks.
--
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