From mpi@openbsd.org Thu Sep 10 23:14:23 2015 Delivered-To: david@gwynne.id.au Received: by 10.27.85.143 with SMTP id l15csp390838wli; Thu, 10 Sep 2015 06:14:23 -0700 (PDT) X-Received: by 10.202.198.131 with SMTP id w125mr30893361oif.10.1441890863791; Thu, 10 Sep 2015 06:14:23 -0700 (PDT) Return-Path: Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net. [2001:4b98:c:538::197]) by mx.google.com with ESMTPS id b6si19445324pas.61.2015.09.10.06.14.21 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Thu, 10 Sep 2015 06:14:23 -0700 (PDT) Received-SPF: softfail (google.com: domain of transitioning mpi@openbsd.org does not designate 2001:4b98:c:538::197 as permitted sender) client-ip=2001:4b98:c:538::197; Authentication-Results: mx.google.com; spf=softfail (google.com: domain of transitioning mpi@openbsd.org does not designate 2001:4b98:c:538::197 as permitted sender) smtp.mailfrom=mpi@openbsd.org Received: from mfilter18-d.gandi.net (mfilter18-d.gandi.net [217.70.178.146]) by relay5-d.mail.gandi.net (Postfix) with ESMTP id 92A3141C0A5; Thu, 10 Sep 2015 15:14:18 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter18-d.gandi.net Received: from relay5-d.mail.gandi.net ([IPv6:::ffff:217.70.183.197]) by mfilter18-d.gandi.net (mfilter18-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id Hr6xso4BgnPk; Thu, 10 Sep 2015 15:14:16 +0200 (CEST) X-Originating-IP: 195.132.27.120 Received: from figo.nolizard.org (195-132-27-120.rev.numericable.fr [195.132.27.120]) (Authenticated sender: mpieuchot@nolizard.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id F2B5A41C0D8; Thu, 10 Sep 2015 15:14:12 +0200 (CEST) Received: from localhost (figo.nolizard.org [local]); by figo.nolizard.org (OpenSMTPD) with ESMTPA id b2c5f27d; Thu, 10 Sep 2015 15:14:12 +0200 (CEST) Date: Thu, 10 Sep 2015 15:14:12 +0200 From: Martin Pieuchot To: David Gwynne Cc: sashan@openbsd.org, mikeb@openbsd.org, claudio@openbsd.org Subject: Re: ifih again Message-ID: <20150910131412.GD6144@figo.nolizard.org> References: <20150910114927.GQ21757@animata.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150910114927.GQ21757@animata.net> User-Agent: Mutt/1.5.23 (2014-03-12) Status: RO Content-Length: 15070 Lines: 519 On 10/09/15(Thu) 21:49, David Gwynne wrote: > this renames the functions as per mpi@s request, and cleans up a > couple of niggly things found by mikeb@ > > i want to put this in. ok? I would prefer if you could commit the diff below. It compiles and I changed the rw_enter* by a KERNEL_ASSERT_LOCKED(). I tested it with carp on top of trunk deleting interfaces while sending traffic. Index: net/if.c =================================================================== RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.365 diff -u -p -r1.365 if.c --- net/if.c 10 Sep 2015 06:00:37 -0000 1.365 +++ net/if.c 10 Sep 2015 13:12:52 -0000 @@ -80,6 +80,7 @@ #include #include #include +#include #include #include @@ -388,7 +389,7 @@ if_attach_common(struct ifnet *ifp) ifp->if_linkstatetask = malloc(sizeof(*ifp->if_linkstatetask), M_TEMP, M_WAITOK|M_ZERO); - SLIST_INIT(&ifp->if_inputs); + SRPL_INIT(&ifp->if_inputs); } void @@ -485,6 +486,89 @@ if_input(struct ifnet *ifp, struct mbuf_ task_add(softnettq, &if_input_task); } +struct ifih { + struct srpl_entry ifih_next; + int (*ifih_input)(struct ifnet *, struct mbuf *); + int ifih_refcnt; + int ifih_srpcnt; +}; + +void if_ih_ref(void *, void *); +void if_ih_unref(void *, void *); + +struct srpl_rc ifih_rc = SRPL_RC_INITIALIZER(if_ih_ref, if_ih_unref, NULL); + +void +if_ih_insert(struct ifnet *ifp, int (*input)(struct ifnet *, struct mbuf *)) +{ + struct ifih *ifih; + + KERNEL_ASSERT_LOCKED(); + SRPL_FOREACH_LOCKED(ifih, &ifp->if_inputs, ifih_next) { + if (ifih->ifih_input == input) { + ifih->ifih_refcnt++; + break; + } + } + + if (ifih == NULL) { + ifih = malloc(sizeof(*ifih), M_DEVBUF, M_WAITOK); + + ifih->ifih_input = input; + ifih->ifih_refcnt = 1; + ifih->ifih_srpcnt = 0; + SRPL_INSERT_HEAD_LOCKED(&ifih_rc, &ifp->if_inputs, + ifih, ifih_next); + } +} + +void +if_ih_ref(void *null, void *i) +{ + struct ifih *ifih = i; + + atomic_inc_int(&ifih->ifih_srpcnt); +} + +void +if_ih_unref(void *null, void *i) +{ + struct ifih *ifih = i; + + if (atomic_dec_int_nv(&ifih->ifih_srpcnt) == 0) + wakeup(&ifih->ifih_srpcnt); +} + +void +if_ih_remove(struct ifnet *ifp, int (*input)(struct ifnet *, struct mbuf *)) +{ + struct sleep_state sls; + struct ifih *ifih; + int refs; + + KERNEL_ASSERT_LOCKED(); + SRPL_FOREACH_LOCKED(ifih, &ifp->if_inputs, ifih_next) { + if (ifih->ifih_input == input) + break; + } + + KASSERT(ifih != NULL); + + if (--ifih->ifih_refcnt == 0) { + SRPL_REMOVE_LOCKED(&ifih_rc, &ifp->if_inputs, ifih, + ifih, ifih_next); + + refs = ifih->ifih_srpcnt; + while (refs) { + sleep_setup(&sls, &ifih->ifih_srpcnt, PWAIT, "ifihrm"); + refs = ifih->ifih_srpcnt; + sleep_finish(&sls, refs); + } + + free(ifih, M_DEVBUF, sizeof(*ifih)); + } +} + void if_input_process(void *xmq) { @@ -493,6 +577,7 @@ if_input_process(void *xmq) struct mbuf *m; struct ifnet *ifp; struct ifih *ifih; + struct srpl_iter i; int s; mq_delist(mq, &ml); @@ -525,10 +610,12 @@ if_input_process(void *xmq) * Pass this mbuf to all input handlers of its * interface until it is consumed. */ - SLIST_FOREACH(ifih, &ifp->if_inputs, ifih_next) { + SRPL_FOREACH(ifih, &ifp->if_inputs, &i, ifih_next) { if ((*ifih->ifih_input)(ifp, m)) break; } + SRPL_LEAVE(&i, ifih); + if (ifih == NULL) m_freem(m); } Index: net/if_bridge.c =================================================================== RCS file: /cvs/src/sys/net/if_bridge.c,v retrieving revision 1.261 diff -u -p -r1.261 if_bridge.c --- net/if_bridge.c 9 Sep 2015 12:50:08 -0000 1.261 +++ net/if_bridge.c 10 Sep 2015 12:43:05 -0000 @@ -183,7 +183,6 @@ int bridge_clone_create(struct if_clone *ifc, int unit) { struct bridge_softc *sc; - struct ifih *bridge_ifih; struct ifnet *ifp; int i; @@ -191,15 +190,8 @@ bridge_clone_create(struct if_clone *ifc if (!sc) return (ENOMEM); - bridge_ifih = malloc(sizeof(*bridge_ifih), M_DEVBUF, M_NOWAIT); - if (bridge_ifih == NULL) { - free(sc, M_DEVBUF, 0); - return (ENOMEM); - } - sc->sc_stp = bstp_create(&sc->sc_if); if (!sc->sc_stp) { - free(bridge_ifih, M_DEVBUF, sizeof(*bridge_ifih)); free(sc, M_DEVBUF, 0); return (ENOMEM); } @@ -231,8 +223,7 @@ bridge_clone_create(struct if_clone *ifc DLT_EN10MB, ETHER_HDR_LEN); #endif - bridge_ifih->ifih_input = ether_input; - SLIST_INSERT_HEAD(&ifp->if_inputs, bridge_ifih, ifih_next); + if_ih_insert(ifp, ether_input); return (0); } @@ -242,7 +233,6 @@ bridge_clone_destroy(struct ifnet *ifp) { struct bridge_softc *sc = ifp->if_softc; struct bridge_iflist *bif; - struct ifih *bridge_ifih; bridge_stop(sc); bridge_rtflush(sc, IFBF_FLUSHALL); @@ -258,12 +248,9 @@ bridge_clone_destroy(struct ifnet *ifp) /* Undo pseudo-driver changes. */ if_deactivate(ifp); - bridge_ifih = SLIST_FIRST(&ifp->if_inputs); - SLIST_REMOVE_HEAD(&ifp->if_inputs, ifih_next); - - KASSERT(SLIST_EMPTY(&ifp->if_inputs)); + if_ih_remove(ifp, ether_input); - free(bridge_ifih, M_DEVBUF, sizeof(*bridge_ifih)); + KASSERT(SRPL_EMPTY_LOCKED(&ifp->if_inputs)); if_detach(ifp); Index: net/if_ethersubr.c =================================================================== RCS file: /cvs/src/sys/net/if_ethersubr.c,v retrieving revision 1.221 diff -u -p -r1.221 if_ethersubr.c --- net/if_ethersubr.c 29 Jul 2015 00:04:03 -0000 1.221 +++ net/if_ethersubr.c 10 Sep 2015 12:43:05 -0000 @@ -492,7 +492,6 @@ void ether_ifattach(struct ifnet *ifp) { struct arpcom *ac = (struct arpcom *)ifp; - struct ifih *ether_ifih; /* * Any interface which provides a MAC address which is obviously @@ -507,9 +506,7 @@ ether_ifattach(struct ifnet *ifp) ifp->if_mtu = ETHERMTU; ifp->if_output = ether_output; - ether_ifih = malloc(sizeof(*ether_ifih), M_DEVBUF, M_WAITOK); - ether_ifih->ifih_input = ether_input; - SLIST_INSERT_HEAD(&ifp->if_inputs, ether_ifih, ifih_next); + if_ih_insert(ifp, ether_input); if (ifp->if_hardmtu == 0) ifp->if_hardmtu = ETHERMTU; @@ -526,18 +523,14 @@ void ether_ifdetach(struct ifnet *ifp) { struct arpcom *ac = (struct arpcom *)ifp; - struct ifih *ether_ifih; struct ether_multi *enm; /* Undo pseudo-driver changes. */ if_deactivate(ifp); - ether_ifih = SLIST_FIRST(&ifp->if_inputs); - SLIST_REMOVE_HEAD(&ifp->if_inputs, ifih_next); + if_ih_remove(ifp, ether_input); - KASSERT(SLIST_EMPTY(&ifp->if_inputs)); - - free(ether_ifih, M_DEVBUF, sizeof(*ether_ifih)); + KASSERT(SRPL_EMPTY_LOCKED(&ifp->if_inputs)); for (enm = LIST_FIRST(&ac->ac_multiaddrs); enm != NULL; Index: net/if_mpw.c =================================================================== RCS file: /cvs/src/sys/net/if_mpw.c,v retrieving revision 1.3 diff -u -p -r1.3 if_mpw.c --- net/if_mpw.c 9 Sep 2015 20:13:20 -0000 1.3 +++ net/if_mpw.c 10 Sep 2015 12:43:05 -0000 @@ -82,18 +82,11 @@ mpw_clone_create(struct if_clone *ifc, i { struct mpw_softc *sc; struct ifnet *ifp; - struct ifih *ifih; sc = malloc(sizeof(*sc), M_DEVBUF, M_NOWAIT | M_ZERO); if (sc == NULL) return (ENOMEM); - ifih = malloc(sizeof(*ifih), M_DEVBUF, M_NOWAIT | M_ZERO); - if (ifih == NULL) { - free(sc, M_DEVBUF, sizeof(*sc)); - return (ENOMEM); - } - ifp = &sc->sc_if; snprintf(ifp->if_xname, sizeof(ifp->if_xname), "mpw%d", unit); ifp->if_softc = sc; @@ -116,8 +109,7 @@ mpw_clone_create(struct if_clone *ifc, i sc->sc_smpls.smpls_len = sizeof(sc->sc_smpls); sc->sc_smpls.smpls_family = AF_MPLS; - ifih->ifih_input = mpw_input; - SLIST_INSERT_HEAD(&ifp->if_inputs, ifih, ifih_next); + if_ih_insert(ifp, mpw_input); #if NBPFILTER > 0 bpfattach(&ifp->if_bpf, ifp, DLT_EN10MB, ETHER_HDR_LEN); @@ -130,7 +122,6 @@ int mpw_clone_destroy(struct ifnet *ifp) { struct mpw_softc *sc = ifp->if_softc; - struct ifih *ifih = SLIST_FIRST(&ifp->if_inputs); int s; ifp->if_flags &= ~IFF_RUNNING; @@ -142,8 +133,7 @@ mpw_clone_destroy(struct ifnet *ifp) splx(s); } - SLIST_REMOVE(&ifp->if_inputs, ifih, ifih, ifih_next); - free(ifih, M_DEVBUF, sizeof(*ifih)); + if_ih_remove(ifp, mpw_input); if_detach(ifp); free(sc, M_DEVBUF, sizeof(*sc)); Index: net/if_trunk.c =================================================================== RCS file: /cvs/src/sys/net/if_trunk.c,v retrieving revision 1.109 diff -u -p -r1.109 if_trunk.c --- net/if_trunk.c 17 Jul 2015 23:32:18 -0000 1.109 +++ net/if_trunk.c 10 Sep 2015 12:43:05 -0000 @@ -344,8 +344,7 @@ trunk_port_create(struct trunk_softc *tr ifp->if_type = IFT_IEEE8023ADLAG; /* Change input handler of the physical interface. */ - tp->tp_ifih.ifih_input = trunk_input; - SLIST_INSERT_HEAD(&ifp->if_inputs, &tp->tp_ifih, ifih_next); + if_ih_insert(ifp, trunk_input); ifp->if_tp = (caddr_t)tp; tp->tp_ioctl = ifp->if_ioctl; @@ -438,7 +437,7 @@ trunk_port_destroy(struct trunk_port *tp ifp->if_type = tp->tp_iftype; /* Restore previous input handler. */ - SLIST_REMOVE(&ifp->if_inputs, &tp->tp_ifih, ifih, ifih_next); + if_ih_remove(ifp, trunk_input); ifp->if_watchdog = tp->tp_watchdog; ifp->if_ioctl = tp->tp_ioctl; Index: net/if_trunk.h =================================================================== RCS file: /cvs/src/sys/net/if_trunk.h,v retrieving revision 1.23 diff -u -p -r1.23 if_trunk.h --- net/if_trunk.h 26 May 2015 11:39:07 -0000 1.23 +++ net/if_trunk.h 10 Sep 2015 12:38:13 -0000 @@ -137,7 +137,6 @@ struct trunk_port { u_int32_t tp_flags; /* port flags */ void *lh_cookie; /* if state hook */ void *dh_cookie; /* if detach hook */ - struct ifih tp_ifih; /* input handler */ /* Redirected callbacks */ void (*tp_watchdog)(struct ifnet *); Index: net/if_var.h =================================================================== RCS file: /cvs/src/sys/net/if_var.h,v retrieving revision 1.35 diff -u -p -r1.35 if_var.h --- net/if_var.h 9 Sep 2015 16:01:10 -0000 1.35 +++ net/if_var.h 10 Sep 2015 12:38:13 -0000 @@ -110,15 +110,6 @@ struct ifqueue { }; /* - * Interface input hooks. - */ -struct ifih { - SLIST_ENTRY(ifih) ifih_next; - int (*ifih_input)(struct ifnet *, struct mbuf *); - int ifih_refcnt; -}; - -/* * Structure defining a queue for a network interface. * * (Would like to call this struct ``if'', but C isn't PL/1.) @@ -161,7 +152,7 @@ struct ifnet { /* and the entries */ struct task *if_linkstatetask; /* task to do route updates */ /* procedure handles */ - SLIST_HEAD(, ifih) if_inputs; /* input routines (dequeue) */ + struct srpl if_inputs; /* input routines (dequeue) */ /* output routine (enqueue) */ int (*if_output)(struct ifnet *, struct mbuf *, struct sockaddr *, @@ -448,6 +439,9 @@ void ifa_add(struct ifnet *, struct ifad void ifa_del(struct ifnet *, struct ifaddr *); void ifa_update_broadaddr(struct ifnet *, struct ifaddr *, struct sockaddr *); + +void if_ih_insert(struct ifnet *, int (*)(struct ifnet *, struct mbuf *)); +void if_ih_remove(struct ifnet *, int (*)(struct ifnet *, struct mbuf *)); void if_rxr_init(struct if_rxring *, u_int, u_int); u_int if_rxr_get(struct if_rxring *, u_int); Index: net/if_vlan.c =================================================================== RCS file: /cvs/src/sys/net/if_vlan.c,v retrieving revision 1.135 diff -u -p -r1.135 if_vlan.c --- net/if_vlan.c 20 Jul 2015 22:16:41 -0000 1.135 +++ net/if_vlan.c 10 Sep 2015 12:43:05 -0000 @@ -357,17 +357,6 @@ vlan_config(struct ifvlan *ifv, struct i if (ifv->ifv_p == p && ifv->ifv_tag == tag) /* noop */ return (0); - /* Can we share an ifih between multiple vlan(4) instances? */ - ifv->ifv_ifih = SLIST_FIRST(&p->if_inputs); - if (ifv->ifv_ifih->ifih_input != vlan_input) { - ifv->ifv_ifih = malloc(sizeof(*ifv->ifv_ifih), M_DEVBUF, - M_NOWAIT); - if (ifv->ifv_ifih == NULL) - return (ENOMEM); - ifv->ifv_ifih->ifih_input = vlan_input; - ifv->ifv_ifih->ifih_refcnt = 0; - } - /* Remember existing interface flags and reset the interface */ flags = ifv->ifv_flags; vlan_unconfig(&ifv->ifv_if, p); @@ -436,13 +425,12 @@ vlan_config(struct ifvlan *ifv, struct i tagh = ifv->ifv_type == ETHERTYPE_QINQ ? svlan_tagh : vlan_tagh; s = splnet(); - /* Change input handler of the physical interface. */ - if (++ifv->ifv_ifih->ifih_refcnt == 1) - SLIST_INSERT_HEAD(&p->if_inputs, ifv->ifv_ifih, ifih_next); - LIST_INSERT_HEAD(&tagh[TAG_HASH(tag)], ifv, ifv_list); splx(s); + /* Change input handler of the physical interface. */ + if_ih_insert(p, vlan_input); + return (0); } @@ -466,13 +454,10 @@ vlan_unconfig(struct ifnet *ifp, struct s = splnet(); LIST_REMOVE(ifv, ifv_list); - - /* Restore previous input handler. */ - if (--ifv->ifv_ifih->ifih_refcnt == 0) { - SLIST_REMOVE(&p->if_inputs, ifv->ifv_ifih, ifih, ifih_next); - free(ifv->ifv_ifih, M_DEVBUF, sizeof(*ifv->ifv_ifih)); - } splx(s); + + /* Restore previous input handler. */ + if_ih_remove(p, vlan_input); hook_disestablish(p->if_linkstatehooks, ifv->lh_cookie); hook_disestablish(p->if_detachhooks, ifv->dh_cookie); Index: netinet/ip_carp.c =================================================================== RCS file: /cvs/src/sys/netinet/ip_carp.c,v retrieving revision 1.264 diff -u -p -r1.264 ip_carp.c --- netinet/ip_carp.c 2 Jul 2015 09:40:03 -0000 1.264 +++ netinet/ip_carp.c 10 Sep 2015 12:43:05 -0000 @@ -120,7 +120,6 @@ struct carp_softc { #define sc_carpdev sc_ac.ac_if.if_carpdev void *ah_cookie; void *lh_cookie; - struct ifih *sc_ifih; struct ip_moptions sc_imo; #ifdef INET6 struct ip6_moptions sc_im6o; @@ -864,13 +863,10 @@ carpdetach(struct carp_softc *sc) if (ifp == NULL) return; - s = splnet(); /* Restore previous input handler. */ - if (--sc->sc_ifih->ifih_refcnt == 0) { - SLIST_REMOVE(&ifp->if_inputs, sc->sc_ifih, ifih, ifih_next); - free(sc->sc_ifih, M_DEVBUF, sizeof(*sc->sc_ifih)); - } + if_ih_remove(ifp, carp_input); + s = splnet(); if (sc->lh_cookie != NULL) hook_disestablish(ifp->if_linkstatehooks, sc->lh_cookie); @@ -1686,18 +1682,6 @@ carp_set_ifp(struct carp_softc *sc, stru return (EINVAL); } - /* Can we share an ifih between multiple carp(4) instances? */ - sc->sc_ifih = SLIST_FIRST(&ifp->if_inputs); - if (sc->sc_ifih->ifih_input != carp_input) { - sc->sc_ifih = malloc(sizeof(*sc->sc_ifih), M_DEVBUF, M_NOWAIT); - if (sc->sc_ifih == NULL) { - free(ncif, M_IFADDR, sizeof(*ncif)); - return (ENOMEM); - } - sc->sc_ifih->ifih_input = carp_input; - sc->sc_ifih->ifih_refcnt = 0; - } - /* detach from old interface */ if (sc->sc_carpdev != NULL) carpdetach(sc); @@ -1734,11 +1718,10 @@ carp_set_ifp(struct carp_softc *sc, stru sc->lh_cookie = hook_establish(ifp->if_linkstatehooks, 1, carp_carpdev_state, ifp); - s = splnet(); /* Change input handler of the physical interface. */ - if (++sc->sc_ifih->ifih_refcnt == 1) - SLIST_INSERT_HEAD(&ifp->if_inputs, sc->sc_ifih, ifih_next); + if_ih_insert(ifp, carp_input); + s = splnet(); carp_carpdev_state(ifp); splx(s);