Le 9 nov. 2012 à 17:43, Ingo Flaschberger a écrit :
> 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.
>
> 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
> --
> --- /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 |= LLE_EXCLUSIVE;
> IF_AFDATA_LOCK(ifp);
> la = lla_lookup(LLTABLE(ifp), flags, (struct sockaddr *)&sin);
> - IF_AFDATA_UNLOCK(ifp);
> +
> if (la != NULL) {
> /* the following is not an error when doing bridging */
> if (!bridged && la->lle_tbl->llt_ifp != 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 @@
>
> if (ifp->if_addrlen != 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 = 0;
> memcpy(&sa, L3_ADDR(la), sizeof(sa));
> LLE_WUNLOCK(la);
> + IF_AFDATA_UNLOCK(ifp);
> for (; m_hold != NULL; m_hold = m_hold_next) {
> m_hold_next = m_hold->m_nextpkt;
> m_hold->m_nextpkt = 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 != ARPOP_REQUEST)
> goto drop;
> --
>
> Kind regards,
> Ingo Flaschberger
>
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
