svn commit: r225947 - head/sys/netinet

[Qing Li]

Hi Gleb,

>
> On Mon, Oct 03, 2011 at 07:51:19PM +0000, Qing Li wrote:
> Q> Author: qingli
> Q> Date: Mon Oct  3 19:51:18 2011
> Q> New Revision: 225947
> Q> URL: http://svn.freebsd.org/changeset/base/225947
> Q>
> Q> Log:
> Q>   A system may have multiple physical interfaces, all of which are on the
> Q>   same prefix. Since a single route entry is installed for the prefix
> Q>   (without RADIX_MPATH), incoming packets on the interfaces that are not
> Q>   associated with the prefix route may trigger an error message about
> Q>   unable to allocation LLE entry, and fails L2. This patch makes sure a
> Q>   valid route is present in the system, and allow the aforementioned
> Q>   condition to exist and treats as valid.
> Q>
> Q>   Reviewed by:       bz
> Q>   MFC after: 5 days
>
>  this commit together with r225946 makes the in_lltable_rtcheck()
> quite difficult to understand.
>
>  What confuses me most, is that in lines 1435-1445 you are
> assigning error to a positive value, BUT proceeding further
> with function.
>

   This is what was there before (meaning returning error immediately),
   but I guess a couple of folks felt it looked a bit cluttered.
   This is mostly due to the fact the "RTFREE_LOCKED()" operation
   has to be performed before returning.

>
> Well, after third review it is clear, that
> next if() case would definitely be true, and you would proceed
> with return. But that is difficult to see from first glance.
>

   Not so, only for an indirect prefix route.

>
> I'd suggest to remove error variable, return immediately in
> all error cases, and also the RTF_GATEWAY check can be shifted up,
> since it is the most simple and the most usual to be true.
>

  No, the RTF_GATEWAY check cannot be shifted up because if we did
  that, the (indirect host route, with destination matching the gateway IP)
  would never be executed, if when that set of conditions are true, which is
  allowed and the reason for the patch in the first place.

>
> Also, in this commit you really do not need the __DECONST hacks.
>

  Okay, I must had a temporary short circuit. I will fix that.

  Thanks

  --Qing