This is hokey in the way we use krefs, due to racey code inherited from Plan 9. But it works well enough now that ipconfig runs and gets us a DHCP lease.
Change-Id: I966a41f90a2d129eb9f07d9a7993fda990af1dbb Google-Bug-Id: 28667887 Signed-off-by: Dan Cross <[email protected]> --- kern/src/net/ipifc.c | 35 ++++++++++++++++++++++++++++++++--- kern/src/net/iproute.c | 15 ++++++++++++++- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/kern/src/net/ipifc.c b/kern/src/net/ipifc.c index eea8d2a..f8267eb 100644 --- a/kern/src/net/ipifc.c +++ b/kern/src/net/ipifc.c @@ -864,6 +864,17 @@ void ipifcinit(struct Fs *f) } /* + * TODO: Change this to a proper release. + * At the moment, this is difficult since link removal + * requires access to more than just the kref/struct Iplink. + * E.g., the self and interface pointers. + */ +static void link_release(struct kref *kref) +{ + (void)kref; +} + +/* * add to self routing cache * called with c locked */ @@ -904,7 +915,7 @@ addselfcache(struct Fs *f, struct Ipifc *ifc, /* allocate a lifc-to-local link and link to both */ if (lp == NULL) { lp = kzmalloc(sizeof(*lp), 0); - kref_init(&lp->ref, fake_release, 1); + kref_init(&lp->ref, link_release, 1); lp->lifc = lifc; lp->self = p; lp->selflink = p->link; @@ -981,7 +992,7 @@ static void ipselffree(struct Ipself *p) * called with c locked */ static void -remselfcache(struct Fs *f, struct Ipifc *ifc, struct Iplifc *lifc, uint8_t * a) +remselfcache(struct Fs *f, struct Ipifc *ifc, struct Iplifc *lifc, uint8_t *a) { struct Ipself *p, **l; struct Iplink *link, **l_self, **l_lifc; @@ -1027,7 +1038,25 @@ remselfcache(struct Fs *f, struct Ipifc *ifc, struct Iplifc *lifc, uint8_t * a) if (link == NULL) panic("remselfcache"); - if (kref_refcnt(&link->ref) > 1) + /* + * TODO: The check for 'refcnt > 0' here is awkward. It + * exists so that 'remselfcache' can be called concurrently. + * In the original plan 9 code, the 'goto out' branch was + * taken if the decremented reference count was exactly zero. + * In other threads this could become -1, which plan 9 didn't + * care about since the logic would be skipped over, and since + * 'iplinkfree' won't _actually_ free the link for five seconds + * (see comments in that function), and since all of the actual + * link manipulation happens in the thread where the reference + * is exactly equal to zero. But on Akaros, a negative kref + * will panic; hence checking for a positive ref count before + * decrementing. This entire mechanism is dubious. But Since + * this function is protected by a lock this is probably OK for + * the time being. + * + * However, it is a terrible design and we should fix it. + */ + if (kref_refcnt(&link->ref) > 0 && kref_put(&link->ref) != 0) goto out; if ((p->type & Rmulti) && ifc->m->remmulti != NULL) diff --git a/kern/src/net/iproute.c b/kern/src/net/iproute.c index 0e375fb..746c826 100644 --- a/kern/src/net/iproute.c +++ b/kern/src/net/iproute.c @@ -50,6 +50,19 @@ struct route *v6freelist; rwlock_t routelock; uint32_t v4routegeneration, v6routegeneration; +/* + * TODO: Change this to a proper release. + * At the moment this is difficult to do since deleting + * a route involves manipulating more data structures than + * a kref/struct route. E.g., unlinking from the route tree + * requires access to a parent pointer, which doesn't exist + * in the route structure itself. + */ +static void route_release(struct kref *kref) +{ + (void)kref; +} + static void freeroute(struct route *r) { struct route **l; @@ -89,7 +102,7 @@ static struct route *allocroute(int type) memset(r, 0, n); r->rt.type = type; r->rt.ifc = NULL; - kref_init(&r->rt.kref, fake_release, 1); + kref_init(&r->rt.kref, route_release, 1); return r; } -- 2.8.0.rc3.226.g39d4020 -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
