Date: Fri, 9 Mar 2012 09:30:12 GMT From: Gleb Smirnoff <glebius@FreeBSD.org> To: freebsd-net@FreeBSD.org Subject: kern/165863 Message-ID: <201203090930.q299UCPX057950@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/165863; it has been noted by GNATS.
From: Gleb Smirnoff <glebius@FreeBSD.org>
To: Eric van Gyzen <eric_van_gyzen@dell.com>,
Eric van Gyzen <eric@vangyzen.net>, emaste@FreeBSD.org
Cc: bug-followup@FreeBSD.org
Subject: kern/165863
Date: Fri, 9 Mar 2012 13:20:56 +0400
--BXVAT5kNtrzKuDFl
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
Hello, Eric and Ed.
Can you look at this patch? I decided to utilize newer callout API,
that allows to delegate lock retrieval to the callout subsystem, and
this makes things simplier. Hope that should work.
Patch is against head.
Eric, can you please send me your test programs, that you use to
reproduce the bug?
--
Totus tuus, Glebius.
--BXVAT5kNtrzKuDFl
Content-Type: text/x-diff; charset=koi8-r
Content-Disposition: attachment; filename="kern165863.diff"
Index: in.c
===================================================================
--- in.c (revision 232689)
+++ in.c (working copy)
@@ -1279,11 +1279,12 @@
{
struct in_llentry *lle;
- lle = malloc(sizeof(struct in_llentry), M_LLTABLE, M_DONTWAIT | M_ZERO);
+ lle = malloc(sizeof(struct in_llentry), M_LLTABLE, M_NOWAIT | M_ZERO);
if (lle == NULL) /* NB: caller generates msg */
return NULL;
- callout_init(&lle->base.la_timer, CALLOUT_MPSAFE);
+ LLE_LOCK_INIT(&lle->base);
+ callout_init_rw(&lle->base.la_timer, &lle->base.lle_lock, 0);
/*
* For IPv4 this will trigger "arpresolve" to generate
* an ARP request.
@@ -1292,46 +1293,44 @@
lle->l3_addr4 = *(const struct sockaddr_in *)l3addr;
lle->base.lle_refcnt = 1;
lle->base.lle_free = in_lltable_free;
- LLE_LOCK_INIT(&lle->base);
- return &lle->base;
+
+ return (&lle->base);
}
#define IN_ARE_MASKED_ADDR_EQUAL(d, a, m) ( \
(((ntohl((d)->sin_addr.s_addr) ^ (a)->sin_addr.s_addr) & (m)->sin_addr.s_addr)) == 0 )
static void
-in_lltable_prefix_free(struct lltable *llt,
- const struct sockaddr *prefix,
- const struct sockaddr *mask,
- u_int flags)
+in_lltable_prefix_free(struct lltable *llt, const struct sockaddr *prefix,
+ const struct sockaddr *mask, u_int flags)
{
const struct sockaddr_in *pfx = (const struct sockaddr_in *)prefix;
const struct sockaddr_in *msk = (const struct sockaddr_in *)mask;
struct llentry *lle, *next;
- register int i;
+ int i;
size_t pkts_dropped;
- for (i=0; i < LLTBL_HASHTBL_SIZE; i++) {
+ IF_AFDATA_WLOCK(llt->llt_ifp);
+ for (i = 0; i < LLTBL_HASHTBL_SIZE; i++) {
LIST_FOREACH_SAFE(lle, &llt->lle_head[i], lle_next, next) {
/*
* (flags & LLE_STATIC) means deleting all entries
- * including static ARP entries
+ * including static ARP entries.
*/
- if (IN_ARE_MASKED_ADDR_EQUAL((struct sockaddr_in *)L3_ADDR(lle),
- pfx, msk) &&
- ((flags & LLE_STATIC) || !(lle->la_flags & LLE_STATIC))) {
- int canceled;
+ if (IN_ARE_MASKED_ADDR_EQUAL(satosin(L3_ADDR(lle)),
+ pfx, msk) && ((flags & LLE_STATIC) ||
+ !(lle->la_flags & LLE_STATIC))) {
- canceled = callout_drain(&lle->la_timer);
LLE_WLOCK(lle);
- if (canceled)
+ if (callout_stop(&lle->la_timer))
LLE_REMREF(lle);
pkts_dropped = llentry_free(lle);
ARPSTAT_ADD(dropped, pkts_dropped);
}
}
}
+ IF_AFDATA_WUNLOCK(llt->llt_ifp);
}
Index: if_ether.c
===================================================================
--- if_ether.c (revision 232689)
+++ if_ether.c (working copy)
@@ -163,38 +163,23 @@
static void
arptimer(void *arg)
{
+ struct llentry *lle = (struct llentry *)arg;
struct ifnet *ifp;
- struct llentry *lle;
- int pkts_dropped;
+ size_t pkts_dropped;
- KASSERT(arg != NULL, ("%s: arg NULL", __func__));
- lle = (struct llentry *)arg;
+ LLE_WLOCK_ASSERT(lle);
+
+ if (lle->la_flags & LLE_STATIC)
+ return;
+
ifp = lle->lle_tbl->llt_ifp;
CURVNET_SET(ifp->if_vnet);
- IF_AFDATA_LOCK(ifp);
- LLE_WLOCK(lle);
- if (lle->la_flags & LLE_STATIC)
- LLE_WUNLOCK(lle);
- else {
- if (!callout_pending(&lle->la_timer) &&
- callout_active(&lle->la_timer)) {
- callout_stop(&lle->la_timer);
- LLE_REMREF(lle);
- pkts_dropped = llentry_free(lle);
- ARPSTAT_ADD(dropped, pkts_dropped);
- ARPSTAT_INC(timeouts);
- } else {
-#ifdef DIAGNOSTIC
- struct sockaddr *l3addr = L3_ADDR(lle);
- log(LOG_INFO,
- "arptimer issue: %p, IPv4 address: \"%s\"\n", lle,
- inet_ntoa(
- ((const struct sockaddr_in *)l3addr)->sin_addr));
-#endif
- LLE_WUNLOCK(lle);
- }
- }
- IF_AFDATA_UNLOCK(ifp);
+
+ LLE_REMREF(lle);
+ pkts_dropped = llentry_free(lle);
+ ARPSTAT_ADD(dropped, pkts_dropped);
+ ARPSTAT_INC(timeouts);
+
CURVNET_RESTORE();
}
--BXVAT5kNtrzKuDFl--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203090930.q299UCPX057950>