Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 9 Nov 2012 17:51:49 +0100
From:      Fabien Thomas <fabien.thomas@netasq.com>
To:        Ingo Flaschberger <if@xip.at>
Cc:        freebsd-net@freebsd.org
Subject:   Re: [patch] reducing arp locking
Message-ID:  <AA0D6054-D300-409E-918C-98B206D11F55@netasq.com>
In-Reply-To: <509D32CC.6000201@xip.at>
References:  <509AEDAC.10002@FreeBSD.org> <509B884F.7040106@networx.ch> <509B88B1.3070905@FreeBSD.org> <49EE4F42-6162-40F4-9DE0-1ACA1289B225@netasq.com> <509CC776.9010200@FreeBSD.org> <37E1F76F-D951-4B36-AF00-039DA1CC5CF3@netasq.com> <509CE6A2.2040200@FreeBSD.org> <8169CD67-E444-46FC-A7C8-DD6FB59091E1@netasq.com> <509D32CC.6000201@xip.at>

next in thread | previous in thread | raw e-mail | index | archive | help

Le 9 nov. 2012 =E0 17:43, Ingo Flaschberger a =E9crit :

> Am 09.11.2012 15:03, schrieb Fabien Thomas:
>> In in_arpinput only exclusive access to the entry is taken during the =
update no IF_AFDATA_LOCK that's why i was surprised.
>=20
> what about this:

I'm not against optimizing but an API that seems clear (correct this if =
i'm wrong):
- one lock for list modification
- one RW lock for lle entry access
- one refcount for ptr unref

is now a lot more unclear and from my point of view dangerous.

My next question is why do we need a per entry lock if we use the table =
lock to protect entry access?

Fabien
=20
> --
> --- /usr/src/sys/netinet/if_ether.c_org 2012-11-09 16:15:43.000000000 =
+0000
> +++ /usr/src/sys/netinet/if_ether.c     2012-11-09 16:16:37.000000000 =
+0000
> @@ -685,7 +685,7 @@
>        flags |=3D LLE_EXCLUSIVE;
>        IF_AFDATA_LOCK(ifp);
>        la =3D lla_lookup(LLTABLE(ifp), flags, (struct sockaddr =
*)&sin);
> -       IF_AFDATA_UNLOCK(ifp);
> +
>        if (la !=3D NULL) {
>                /* the following is not an error when doing bridging */
>                if (!bridged && la->lle_tbl->llt_ifp !=3D ifp && =
!carp_match) {
> @@ -697,12 +697,14 @@
>                                    ifp->if_addrlen, (u_char =
*)ar_sha(ah), ":",
>                                    ifp->if_xname);
>                        LLE_WUNLOCK(la);
> +                       IF_AFDATA_UNLOCK(ifp);
>                        goto reply;
>                }
>                if ((la->la_flags & LLE_VALID) &&
>                    bcmp(ar_sha(ah), &la->ll_addr, ifp->if_addrlen)) {
>                        if (la->la_flags & LLE_STATIC) {
>                                LLE_WUNLOCK(la);
> +                               IF_AFDATA_UNLOCK(ifp);
>                                if (log_arp_permanent_modify)
>                                        log(LOG_ERR,
>                                            "arp: %*D attempts to =
modify "
> @@ -725,6 +727,7 @@
>=20
>                if (ifp->if_addrlen !=3D ah->ar_hln) {
>                        LLE_WUNLOCK(la);
> +                       IF_AFDATA_UNLOCK(ifp);
>                        log(LOG_WARNING, "arp from %*D: addr len: new =
%d, "
>                            "i/f %d (ignored)\n", ifp->if_addrlen,
>                            (u_char *) ar_sha(ah), ":", ah->ar_hln,
> @@ -763,14 +766,19 @@
>                        la->la_numheld =3D 0;
>                        memcpy(&sa, L3_ADDR(la), sizeof(sa));
>                        LLE_WUNLOCK(la);
> +                       IF_AFDATA_UNLOCK(ifp);
>                        for (; m_hold !=3D NULL; m_hold =3D =
m_hold_next) {
>                                m_hold_next =3D m_hold->m_nextpkt;
>                                m_hold->m_nextpkt =3D NULL;
>                                (*ifp->if_output)(ifp, m_hold, &sa, =
NULL);
>                        }
> -               } else
> +               } else {
>                        LLE_WUNLOCK(la);
> -       }
> +                       IF_AFDATA_UNLOCK(ifp);
> +                }
> +       } else {
> +               IF_AFDATA_UNLOCK(ifp);
> +        }
> reply:
>        if (op !=3D ARPOP_REQUEST)
>                goto drop;
> --
>=20
> Kind regards,
>    Ingo Flaschberger
>=20
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AA0D6054-D300-409E-918C-98B206D11F55>