| Message ID | 536C64C0.5050204@cn.fujitsu.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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
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)); neigh_add_timer(neigh, jiffies + NEIGH_VAR(neigh->parms, RETRANS_TIME)); }
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(-)