svn commit: r198111 - head/sys/netinet

[Qing Li]

Forgot to mention the return code was incorrect. The function was returning
EHOSTUNEACH when it should be returning EHOSTDOWN. This is also
fixed by this patch.

-- Qing



On Wed, Oct 14, 2009 at 11:12 PM, Qing Li <qingli at freebsd.org> wrote:
> Author: qingli
> Date: Thu Oct 15 06:12:04 2009
> New Revision: 198111
> URL: http://svn.freebsd.org/changeset/base/198111
>
> Log:
>  This patch fixes the following issues in the ARP operation:
>
>  1. There is a regression issue in the ARP code. The incomplete
>     ARP entry was timing out too quickly (1 second timeout), as
>     such, a new entry is created each time arpresolve() is called.
>     Therefore the maximum attempts made is always 1. Consequently
>     the error code returned to the application is always 0.
>  2. Set the expiration of each incomplete entry to a 20-second
>     lifetime.
>  3. Return "incomplete" entries to the application.
>
>  Reviewed by:  kmacy
>  MFC after:    3 days
>
> Modified:
>  head/sys/netinet/if_ether.c
>  head/sys/netinet/in.c
>
> Modified: head/sys/netinet/if_ether.c
> ==============================================================================
> --- head/sys/netinet/if_ether.c Thu Oct 15 06:02:37 2009        (r198110)
> +++ head/sys/netinet/if_ether.c Thu Oct 15 06:12:04 2009        (r198111)
> @@ -88,11 +88,14 @@ VNET_DEFINE(int, useloopback) = 1;  /* us
>  /* timer values */
>  static VNET_DEFINE(int, arpt_keep) = (20*60);  /* once resolved, good for 20
>                                                 * minutes */
> +static VNET_DEFINE(int, arpt_down) = 20;      /* keep incomplete entries for
> +                                              * 20 seconds */
>  static VNET_DEFINE(int, arp_maxtries) = 5;
>  static VNET_DEFINE(int, arp_proxyall);
>  static VNET_DEFINE(struct arpstat, arpstat);  /* ARP statistics, see if_arp.h */
>
>  #define        V_arpt_keep             VNET(arpt_keep)
> +#define        V_arpt_down             VNET(arpt_down)
>  #define        V_arp_maxtries          VNET(arp_maxtries)
>  #define        V_arp_proxyall          VNET(arp_proxyall)
>  #define        V_arpstat               VNET(arpstat)
> @@ -309,7 +312,7 @@ retry:
>        }
>
>        if ((la->la_flags & LLE_VALID) &&
> -           ((la->la_flags & LLE_STATIC) || la->la_expire > time_uptime)) {
> +           ((la->la_flags & LLE_STATIC) || la->la_expire > time_second)) {
>                bcopy(&la->ll_addr, desten, ifp->if_addrlen);
>                /*
>                 * If entry has an expiry time and it is approaching,
> @@ -317,7 +320,7 @@ retry:
>                 * arpt_down interval.
>                 */
>                if (!(la->la_flags & LLE_STATIC) &&
> -                   time_uptime + la->la_preempt > la->la_expire) {
> +                   time_second + la->la_preempt > la->la_expire) {
>                        arprequest(ifp, NULL,
>                            &SIN(dst)->sin_addr, IF_LLADDR(ifp));
>
> @@ -337,7 +340,7 @@ retry:
>                goto done;
>        }
>
> -       renew = (la->la_asked == 0 || la->la_expire != time_uptime);
> +       renew = (la->la_asked == 0 || la->la_expire != time_second);
>        if ((renew || m != NULL) && (flags & LLE_EXCLUSIVE) == 0) {
>                flags |= LLE_EXCLUSIVE;
>                LLE_RUNLOCK(la);
> @@ -370,12 +373,12 @@ retry:
>                error = EWOULDBLOCK;    /* First request. */
>        else
>                error =
> -                   (rt0->rt_flags & RTF_GATEWAY) ? EHOSTDOWN : EHOSTUNREACH;
> +                       (rt0->rt_flags & RTF_GATEWAY) ? EHOSTUNREACH : EHOSTDOWN;
>
>        if (renew) {
>                LLE_ADDREF(la);
> -               la->la_expire = time_uptime;
> -               callout_reset(&la->la_timer, hz, arptimer, la);
> +               la->la_expire = time_second;
> +               callout_reset(&la->la_timer, hz * V_arpt_down, arptimer, la);
>                la->la_asked++;
>                LLE_WUNLOCK(la);
>                arprequest(ifp, NULL, &SIN(dst)->sin_addr,
> @@ -687,7 +690,7 @@ match:
>                EVENTHANDLER_INVOKE(arp_update_event, la);
>
>                if (!(la->la_flags & LLE_STATIC)) {
> -                       la->la_expire = time_uptime + V_arpt_keep;
> +                       la->la_expire = time_second + V_arpt_keep;
>                        callout_reset(&la->la_timer, hz * V_arpt_keep,
>                            arptimer, la);
>                }
>
> Modified: head/sys/netinet/in.c
> ==============================================================================
> --- head/sys/netinet/in.c       Thu Oct 15 06:02:37 2009        (r198110)
> +++ head/sys/netinet/in.c       Thu Oct 15 06:12:04 2009        (r198111)
> @@ -1440,7 +1440,7 @@ in_lltable_dump(struct lltable *llt, str
>                        struct sockaddr_dl *sdl;
>
>                        /* skip deleted entries */
> -                       if ((lle->la_flags & (LLE_DELETED|LLE_VALID)) != LLE_VALID)
> +                       if ((lle->la_flags & LLE_DELETED) == LLE_DELETED)
>                                continue;
>                        /* Skip if jailed and not a valid IP of the prison. */
>                        if (prison_if(wr->td->td_ucred, L3_ADDR(lle)) != 0)
> @@ -1472,10 +1472,15 @@ in_lltable_dump(struct lltable *llt, str
>                        sdl = &arpc.sdl;
>                        sdl->sdl_family = AF_LINK;
>                        sdl->sdl_len = sizeof(*sdl);
> -                       sdl->sdl_alen = ifp->if_addrlen;
>                        sdl->sdl_index = ifp->if_index;
>                        sdl->sdl_type = ifp->if_type;
> -                       bcopy(&lle->ll_addr, LLADDR(sdl), ifp->if_addrlen);
> +                       if ((lle->la_flags & LLE_VALID) == LLE_VALID) {
> +                               sdl->sdl_alen = ifp->if_addrlen;
> +                               bcopy(&lle->ll_addr, LLADDR(sdl), ifp->if_addrlen);
> +                       } else {
> +                               sdl->sdl_alen = 0;
> +                               bzero(LLADDR(sdl), ifp->if_addrlen);
> +                       }
>
>                        arpc.rtm.rtm_rmx.rmx_expire =
>                            lle->la_flags & LLE_STATIC ? 0 : lle->la_expire;
>