Re: [Bugme-new] [Bug 11469] New: TUN with 1024 neighbours: ip6_dst_lookup_tail NULL crash

[Evgeniy Polyakov <johnpol@2ka.mipt.ru>]

Hi David.

On Mon, Sep 08, 2008 at 01:15:47PM -0700, David Miller (davem@davemloft.net) wrote:
> But I don't like this patch for several reasons:
> 
> 1) Slapping on a NULL check in response to a OOPS at that exact
>    location is usually a very big red flag, and deserves high scrutiny
>    instead of blind acceptance.
> 
> 2) Looking at the indentation of this DAD code block (it's all one tab
>    too much) it's obviously a very shitty cut and paste job.  If the
>    coding style was too difficult to get right, what does this say
>    about that change that brought the code here, semantically? :-/
> 
>    This means we should figure out how this code got to this place,
>    and what kind of invariants existed at the old location that might
>    make this dst->neighbour dereference valid, and what implications
>    there are for the fact that it can now be NULL.
> 
> Really, we really need to understand much more deeply this situation.

Well, yes. The whole 'optimistic' dad looks a bit suspicious. I think
failed dst entry without neighbour is a result of the 'static' dst entry
returned by the above route lookup and previously neighbour was not
used at all. This patch fixes the opps, but may be just hiding a
problem, but reading how this optimistic duplicate address detection
works, I see no strict requirements that returned route entry has to
have neighbour, so this check actually can be a right fix.

I've added Neil Horman, who created the patch 1.5 years ago, to the
copy list.

-- 
	Evgeniy Polyakov