* [PATCH net-next] Better handling of transition to NUD_PROBE state
@ 2015-05-18 10:44 Erik Kline
2015-05-20 3:42 ` Lorenzo Colitti
2015-05-21 20:53 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Erik Kline @ 2015-05-18 10:44 UTC (permalink / raw)
To: hannes; +Cc: lorenzo, netdev, davem, Erik Kline
[1] When entering NUD_PROBE state via neigh_update(), perhaps received
from userspace, correctly (re)initialize the probes count to zero.
This is useful for forcing revalidation of a neighbor (for example
if the host is attempting to do DNA [IPv4 4436, IPv6 6059]).
[2] Notify listeners when a neighbor goes into NUD_PROBE state.
By sending notifications on entry to NUD_PROBE state listeners get
more timely warnings of imminent connectivity issues.
The current notifications on entry to NUD_STALE have somewhat
limited usefulness: NUD_STALE is a perfectly normal state, as is
NUD_DELAY, whereas notifications on entry to NUD_FAILURE come after
a neighbor reachability problem has been confirmed (typically after
three probes).
Signed-off-by: Erik Kline <ek@google.com>
---
net/core/neighbour.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 3de6542..3a74df7 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -913,6 +913,7 @@ static void neigh_timer_handler(unsigned long arg)
neigh->nud_state = NUD_PROBE;
neigh->updated = jiffies;
atomic_set(&neigh->probes, 0);
+ notify = 1;
next = now + NEIGH_VAR(neigh->parms, RETRANS_TIME);
}
} else {
@@ -1144,6 +1145,8 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
if (new != old) {
neigh_del_timer(neigh);
+ if (new & NUD_PROBE)
+ atomic_set(&neigh->probes, 0);
if (new & NUD_IN_TIMER)
neigh_add_timer(neigh, (jiffies +
((new & NUD_REACHABLE) ?
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] Better handling of transition to NUD_PROBE state
2015-05-18 10:44 [PATCH net-next] Better handling of transition to NUD_PROBE state Erik Kline
@ 2015-05-20 3:42 ` Lorenzo Colitti
2015-05-20 16:10 ` Hannes Frederic Sowa
2015-05-21 20:53 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Lorenzo Colitti @ 2015-05-20 3:42 UTC (permalink / raw)
To: Erik Kline; +Cc: Hannes Frederic Sowa, netdev, David Miller
On Mon, May 18, 2015 at 7:44 PM, Erik Kline <ek@google.com> wrote:
> [1] When entering NUD_PROBE state via neigh_update(), perhaps received
> from userspace, correctly (re)initialize the probes count to zero.
>
> This is useful for forcing revalidation of a neighbor (for example
> if the host is attempting to do DNA [IPv4 4436, IPv6 6059]).
>
> [2] Notify listeners when a neighbor goes into NUD_PROBE state.
>
> By sending notifications on entry to NUD_PROBE state listeners get
> more timely warnings of imminent connectivity issues.
>
> The current notifications on entry to NUD_STALE have somewhat
> limited usefulness: NUD_STALE is a perfectly normal state, as is
> NUD_DELAY, whereas notifications on entry to NUD_FAILURE come after
> a neighbor reachability problem has been confirmed (typically after
> three probes).
>
> Signed-off-by: Erik Kline <ek@google.com>
> ---
> net/core/neighbour.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 3de6542..3a74df7 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -913,6 +913,7 @@ static void neigh_timer_handler(unsigned long arg)
> neigh->nud_state = NUD_PROBE;
> neigh->updated = jiffies;
> atomic_set(&neigh->probes, 0);
> + notify = 1;
+1. Currently, the code notifies when going from REACHABLE into STALE,
which is not necessarily something userspace might want to know about
(all it means is "we haven't sent any packets to this neighbour
recently"), but it doesn't notify when going into PROBE, which is a
more important event (it means "we've been sending this neighbour
packets for (by default) 5 seconds and we still haven't found out if
it's stilll there, so we're probing it").
> next = now + NEIGH_VAR(neigh->parms, RETRANS_TIME);
> }
> } else {
> @@ -1144,6 +1145,8 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
>
> if (new != old) {
> neigh_del_timer(neigh);
> + if (new & NUD_PROBE)
> + atomic_set(&neigh->probes, 0);
+1. The normal code path (from STALE to DELAY to PROBE) obviously
already sets the probes to 0. Userspace can put a neighbour into
NUD_PROBE to cause the kernel to probe it, but this only works (by
default) three times because the probe counter is never reset to 0. So
the first three times, the neighbour goes from (say) STALE to PROBE
and then back to REACHABLE (good), but then the fourth time, the
neighbour goes from STALE to PROBE and then immediately to FAILED.
> if (new & NUD_IN_TIMER)
> neigh_add_timer(neigh, (jiffies +
> ((new & NUD_REACHABLE) ?
> --
> 2.2.0.rc0.207.ga3a616c
>
Acked-By: Lorenzo Colitti <lorenzo@google.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] Better handling of transition to NUD_PROBE state
2015-05-20 3:42 ` Lorenzo Colitti
@ 2015-05-20 16:10 ` Hannes Frederic Sowa
0 siblings, 0 replies; 5+ messages in thread
From: Hannes Frederic Sowa @ 2015-05-20 16:10 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: Erik Kline, netdev, David Miller
On Mi, 2015-05-20 at 12:42 +0900, Lorenzo Colitti wrote:
> On Mon, May 18, 2015 at 7:44 PM, Erik Kline <ek@google.com> wrote:
> > [1] When entering NUD_PROBE state via neigh_update(), perhaps received
> > from userspace, correctly (re)initialize the probes count to zero.
> >
> > This is useful for forcing revalidation of a neighbor (for example
> > if the host is attempting to do DNA [IPv4 4436, IPv6 6059]).
> >
> > [2] Notify listeners when a neighbor goes into NUD_PROBE state.
> >
> > By sending notifications on entry to NUD_PROBE state listeners get
> > more timely warnings of imminent connectivity issues.
> >
> > The current notifications on entry to NUD_STALE have somewhat
> > limited usefulness: NUD_STALE is a perfectly normal state, as is
> > NUD_DELAY, whereas notifications on entry to NUD_FAILURE come after
> > a neighbor reachability problem has been confirmed (typically after
> > three probes).
> >
> > Signed-off-by: Erik Kline <ek@google.com>
> > ---
> > net/core/neighbour.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index 3de6542..3a74df7 100644
> > --- a/net/core/neighbour.c
> > +++ b/net/core/neighbour.c
> > @@ -913,6 +913,7 @@ static void neigh_timer_handler(unsigned long arg)
> > neigh->nud_state = NUD_PROBE;
> > neigh->updated = jiffies;
> > atomic_set(&neigh->probes, 0);
> > + notify = 1;
>
> +1. Currently, the code notifies when going from REACHABLE into STALE,
> which is not necessarily something userspace might want to know about
> (all it means is "we haven't sent any packets to this neighbour
> recently"), but it doesn't notify when going into PROBE, which is a
> more important event (it means "we've been sending this neighbour
> packets for (by default) 5 seconds and we still haven't found out if
> it's stilll there, so we're probing it").
>
> > next = now + NEIGH_VAR(neigh->parms, RETRANS_TIME);
> > }
> > } else {
> > @@ -1144,6 +1145,8 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
> >
> > if (new != old) {
> > neigh_del_timer(neigh);
> > + if (new & NUD_PROBE)
> > + atomic_set(&neigh->probes, 0);
>
> +1. The normal code path (from STALE to DELAY to PROBE) obviously
> already sets the probes to 0. Userspace can put a neighbour into
> NUD_PROBE to cause the kernel to probe it, but this only works (by
> default) three times because the probe counter is never reset to 0. So
> the first three times, the neighbour goes from (say) STALE to PROBE
> and then back to REACHABLE (good), but then the fourth time, the
> neighbour goes from STALE to PROBE and then immediately to FAILED.
>
> > if (new & NUD_IN_TIMER)
> > neigh_add_timer(neigh, (jiffies +
> > ((new & NUD_REACHABLE) ?
> > --
> > 2.2.0.rc0.207.ga3a616c
> >
>
> Acked-By: Lorenzo Colitti <lorenzo@google.com>
I agree with Lorenzo, these changes look fine.
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] Better handling of transition to NUD_PROBE state
2015-05-18 10:44 [PATCH net-next] Better handling of transition to NUD_PROBE state Erik Kline
2015-05-20 3:42 ` Lorenzo Colitti
@ 2015-05-21 20:53 ` David Miller
2015-05-22 4:32 ` Erik Kline
1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2015-05-21 20:53 UTC (permalink / raw)
To: ek; +Cc: hannes, lorenzo, netdev
From: Erik Kline <ek@google.com>
Date: Mon, 18 May 2015 19:44:41 +0900
> [1] When entering NUD_PROBE state via neigh_update(), perhaps received
> from userspace, correctly (re)initialize the probes count to zero.
>
> This is useful for forcing revalidation of a neighbor (for example
> if the host is attempting to do DNA [IPv4 4436, IPv6 6059]).
>
> [2] Notify listeners when a neighbor goes into NUD_PROBE state.
>
> By sending notifications on entry to NUD_PROBE state listeners get
> more timely warnings of imminent connectivity issues.
>
> The current notifications on entry to NUD_STALE have somewhat
> limited usefulness: NUD_STALE is a perfectly normal state, as is
> NUD_DELAY, whereas notifications on entry to NUD_FAILURE come after
> a neighbor reachability problem has been confirmed (typically after
> three probes).
>
> Signed-off-by: Erik Kline <ek@google.com>
Please, in the future, put a proper subsystem prefix in the Subject
line of your patch submissions. In this particular case "neigh: "
would have been appropriate and is what I added when applying your
patch.
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] Better handling of transition to NUD_PROBE state
2015-05-21 20:53 ` David Miller
@ 2015-05-22 4:32 ` Erik Kline
0 siblings, 0 replies; 5+ messages in thread
From: Erik Kline @ 2015-05-22 4:32 UTC (permalink / raw)
To: David Miller; +Cc: Hannes Frederic Sowa, Lorenzo Colitti, netdev
> Please, in the future, put a proper subsystem prefix in the Subject
> line of your patch submissions. In this particular case "neigh: "
> would have been appropriate and is what I added when applying your
> patch.
My apologies.
> Applied, thanks.
Appreciated.
-ek
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-05-22 4:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-18 10:44 [PATCH net-next] Better handling of transition to NUD_PROBE state Erik Kline
2015-05-20 3:42 ` Lorenzo Colitti
2015-05-20 16:10 ` Hannes Frederic Sowa
2015-05-21 20:53 ` David Miller
2015-05-22 4:32 ` Erik Kline
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.