On Thu, Jul 28, 2022 at 10:55:39AM -0700, Gerd Rausch wrote:
> Hi Jason,
>
> On Fri, 2022-06-24 at 17:55 -0700, Gerd Rausch wrote:
> > In other words, I don't see how the STALE case was covered, and I'd have
> > to guess wether the -ENODATA coupling was intentional or accidental.
> >
> > Now it may be perfectly possible to just make the "neigh_event_send"
> > call above unconditional and call it a day.
> >
> > I mean, simpler fixes always win over more complex ones.
> >
> > But, I personally don't know the twists & turns of this code well enough
> > to be able to assert that this won't leave any non-covered conditional
> > corner cases. I certainly hadn't tested that.
> >
>
> I finally got around to trying the much simpler fix, and it appears to
> work just as well:
>
> --------%<--------%<--------%<--------%<--------%<--------%<--------
> --- drivers/infiniband/core/addr.c-
> +++ drivers/infiniband/core/addr.c
> @@ -336,11 +336,11 @@ static int dst_fetch_ha(const struct dst
> return -ENODATA;
>
> if (!(n->nud_state & NUD_VALID)) {
> - neigh_event_send(n, NULL);
> ret = -ENODATA;
> } else {
> neigh_ha_snapshot(dev_addr->dst_dev_addr, n, dst->dev);
> }
> + neigh_event_send(n, NULL);
>
> neigh_release(n);
>
> --------%<--------%<--------%<--------%<--------%<--------%<--------
>
> Tested with IPv4 only, but this should work just as well with IPv6:
>
> STALE neighbor entries transition to DELAY -> REACHABLE with this
> change, i.e. the entries get updated.
It seems OK, but I would still like to see an attempt to explain how
nud_state and neigh_event_send is supposed to be used.. Maybe futile,
but lets try at least.
Jason
