Re: Unneeded splnet()/splx() in carp(4)
On Tue, Mar 07, 2017 at 11:08:43AM +0100, Martin Pieuchot wrote:
> carp(4), as a pseudo-interface, is always executed in the 'softnet'
> thread. Using splnet()/splx() might have been relevant when link-state
> handlers where directly executed from hardware interrupt handlers. But
> nowadays everything is run under the NET_LOCK() in a thread context, so
> let's get rid of these superfluous splnet()/splx() dances.
>
> ok?
OK bluhm@
>
> Index: netinet/ip_carp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.302
> diff -u -p -r1.302 ip_carp.c
> --- netinet/ip_carp.c 20 Feb 2017 06:29:42 - 1.302
> +++ netinet/ip_carp.c 7 Mar 2017 10:05:08 -
> @@ -898,7 +898,6 @@ carpdetach(struct carp_softc *sc)
> {
> struct ifnet *ifp0;
> struct carp_if *cif;
> - int s;
>
> carp_del_all_timeouts(sc);
>
> @@ -926,7 +925,6 @@ carpdetach(struct carp_softc *sc)
> /* Restore previous input handler. */
> if_ih_remove(ifp0, carp_input, cif);
>
> - s = splnet();
> if (sc->lh_cookie != NULL)
> hook_disestablish(ifp0->if_linkstatehooks, sc->lh_cookie);
>
> @@ -938,7 +936,6 @@ carpdetach(struct carp_softc *sc)
> free(cif, M_IFADDR, sizeof(*cif));
> }
> sc->sc_carpdev = NULL;
> - splx(s);
> }
>
> /* Detach an interface from the carp. */
> @@ -1680,7 +1677,6 @@ carp_set_ifp(struct carp_softc *sc, stru
> struct carp_if *cif, *ncif = NULL;
> struct carp_softc *vr, *last = NULL, *after = NULL;
> int myself = 0, error = 0;
> - int s;
>
> KASSERT(ifp0 != sc->sc_carpdev);
> KERNEL_ASSERT_LOCKED(); /* touching vhif_vrs */
> @@ -1754,9 +1750,7 @@ carp_set_ifp(struct carp_softc *sc, stru
> /* Change input handler of the physical interface. */
> if_ih_insert(ifp0, carp_input, cif);
>
> - s = splnet();
> carp_carpdev_state(ifp0);
> - splx(s);
>
> return (0);
> }
Re: Unneeded splnet()/splx() in carp(4)
On 7 March 2017 at 11:08, Martin Pieuchot wrote: > carp(4), as a pseudo-interface, is always executed in the 'softnet' > thread. Using splnet()/splx() might have been relevant when link-state > handlers where directly executed from hardware interrupt handlers. But > nowadays everything is run under the NET_LOCK() in a thread context, so > let's get rid of these superfluous splnet()/splx() dances. > > ok? > Looks good to me. Hooray for the last pair of splnets in netinet going the way of the Dodo.
Re: Unneeded splnet()/splx() in carp(4)
On 07/03/17(Tue) 11:08, Martin Pieuchot wrote:
> carp(4), as a pseudo-interface, is always executed in the 'softnet'
> thread. Using splnet()/splx() might have been relevant when link-state
> handlers where directly executed from hardware interrupt handlers. But
> nowadays everything is run under the NET_LOCK() in a thread context, so
> let's get rid of these superfluous splnet()/splx() dances.
>
> ok?
Anyone?
>
> Index: netinet/ip_carp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.302
> diff -u -p -r1.302 ip_carp.c
> --- netinet/ip_carp.c 20 Feb 2017 06:29:42 - 1.302
> +++ netinet/ip_carp.c 7 Mar 2017 10:05:08 -
> @@ -898,7 +898,6 @@ carpdetach(struct carp_softc *sc)
> {
> struct ifnet *ifp0;
> struct carp_if *cif;
> - int s;
>
> carp_del_all_timeouts(sc);
>
> @@ -926,7 +925,6 @@ carpdetach(struct carp_softc *sc)
> /* Restore previous input handler. */
> if_ih_remove(ifp0, carp_input, cif);
>
> - s = splnet();
> if (sc->lh_cookie != NULL)
> hook_disestablish(ifp0->if_linkstatehooks, sc->lh_cookie);
>
> @@ -938,7 +936,6 @@ carpdetach(struct carp_softc *sc)
> free(cif, M_IFADDR, sizeof(*cif));
> }
> sc->sc_carpdev = NULL;
> - splx(s);
> }
>
> /* Detach an interface from the carp. */
> @@ -1680,7 +1677,6 @@ carp_set_ifp(struct carp_softc *sc, stru
> struct carp_if *cif, *ncif = NULL;
> struct carp_softc *vr, *last = NULL, *after = NULL;
> int myself = 0, error = 0;
> - int s;
>
> KASSERT(ifp0 != sc->sc_carpdev);
> KERNEL_ASSERT_LOCKED(); /* touching vhif_vrs */
> @@ -1754,9 +1750,7 @@ carp_set_ifp(struct carp_softc *sc, stru
> /* Change input handler of the physical interface. */
> if_ih_insert(ifp0, carp_input, cif);
>
> - s = splnet();
> carp_carpdev_state(ifp0);
> - splx(s);
>
> return (0);
> }
>
Unneeded splnet()/splx() in carp(4)
carp(4), as a pseudo-interface, is always executed in the 'softnet'
thread. Using splnet()/splx() might have been relevant when link-state
handlers where directly executed from hardware interrupt handlers. But
nowadays everything is run under the NET_LOCK() in a thread context, so
let's get rid of these superfluous splnet()/splx() dances.
ok?
Index: netinet/ip_carp.c
===
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.302
diff -u -p -r1.302 ip_carp.c
--- netinet/ip_carp.c 20 Feb 2017 06:29:42 - 1.302
+++ netinet/ip_carp.c 7 Mar 2017 10:05:08 -
@@ -898,7 +898,6 @@ carpdetach(struct carp_softc *sc)
{
struct ifnet *ifp0;
struct carp_if *cif;
- int s;
carp_del_all_timeouts(sc);
@@ -926,7 +925,6 @@ carpdetach(struct carp_softc *sc)
/* Restore previous input handler. */
if_ih_remove(ifp0, carp_input, cif);
- s = splnet();
if (sc->lh_cookie != NULL)
hook_disestablish(ifp0->if_linkstatehooks, sc->lh_cookie);
@@ -938,7 +936,6 @@ carpdetach(struct carp_softc *sc)
free(cif, M_IFADDR, sizeof(*cif));
}
sc->sc_carpdev = NULL;
- splx(s);
}
/* Detach an interface from the carp. */
@@ -1680,7 +1677,6 @@ carp_set_ifp(struct carp_softc *sc, stru
struct carp_if *cif, *ncif = NULL;
struct carp_softc *vr, *last = NULL, *after = NULL;
int myself = 0, error = 0;
- int s;
KASSERT(ifp0 != sc->sc_carpdev);
KERNEL_ASSERT_LOCKED(); /* touching vhif_vrs */
@@ -1754,9 +1750,7 @@ carp_set_ifp(struct carp_softc *sc, stru
/* Change input handler of the physical interface. */
if_ih_insert(ifp0, carp_input, cif);
- s = splnet();
carp_carpdev_state(ifp0);
- splx(s);
return (0);
}
Unneeded splnet()
bridge_ifenqueue() does not need any spl protection, if_output()
already raises it.
ok?
Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.241
diff -u -p -r1.241 if_bridge.c
--- net/if_bridge.c 8 Jun 2015 13:44:08 - 1.241
+++ net/if_bridge.c 8 Jun 2015 13:48:29 -
@@ -967,7 +967,7 @@ bridge_output(struct ifnet *ifp, struct
struct bridge_rtnode *dst_p = NULL;
struct ether_addr *dst;
struct bridge_softc *sc;
- int s, error, len;
+ int error, len;
/* ifp must be a member interface of the bridge. */
if (ifp->if_bridgeport == NULL) {
@@ -1072,9 +1072,7 @@ bridge_output(struct ifnet *ifp, struct
mc = m1;
}
- s = splnet();
error = bridge_ifenqueue(sc, dst_if, mc);
- splx(s);
if (error)
continue;
}
@@ -1093,9 +1091,7 @@ sendunicast:
m_freem(m);
return (ENETDOWN);
}
- s = splnet();
bridge_ifenqueue(sc, dst_if, m);
- splx(s);
return (0);
}
@@ -1135,12 +1131,12 @@ bridgeintr(void)
void
bridgeintr_frame(struct bridge_softc *sc, struct mbuf *m)
{
- int s, len;
struct ifnet *src_if, *dst_if;
struct bridge_iflist *ifl;
struct bridge_rtnode *dst_p;
struct ether_addr *dst, *src;
struct ether_header eh;
+ int len;
if ((sc->sc_if.if_flags & IFF_RUNNING) == 0) {
m_freem(m);
@@ -1293,9 +1289,7 @@ bridgeintr_frame(struct bridge_softc *sc
if ((len - ETHER_HDR_LEN) > dst_if->if_mtu)
bridge_fragment(sc, dst_if, &eh, m);
else {
- s = splnet();
bridge_ifenqueue(sc, dst_if, m);
- splx(s);
}
}
@@ -1499,7 +1493,7 @@ bridge_broadcast(struct bridge_softc *sc
struct bridge_iflist *p;
struct mbuf *mc;
struct ifnet *dst_if;
- int len, s, used = 0;
+ int len, used = 0;
TAILQ_FOREACH(p, &sc->sc_iflist, next) {
/*
@@ -1585,9 +1579,7 @@ bridge_broadcast(struct bridge_softc *sc
if ((len - ETHER_HDR_LEN) > dst_if->if_mtu)
bridge_fragment(sc, dst_if, eh, mc);
else {
- s = splnet();
bridge_ifenqueue(sc, dst_if, mc);
- splx(s);
}
}
@@ -1638,7 +1630,7 @@ bridge_span(struct bridge_softc *sc, str
struct bridge_iflist *p;
struct ifnet *ifp;
struct mbuf *mc, *m;
- int s, error;
+ int error;
if (TAILQ_EMPTY(&sc->sc_spanlist))
return;
@@ -1665,9 +1657,7 @@ bridge_span(struct bridge_softc *sc, str
continue;
}
- s = splnet();
error = bridge_ifenqueue(sc, ifp, mc);
- splx(s);
if (error)
continue;
}
@@ -2555,7 +2545,7 @@ bridge_fragment(struct bridge_softc *sc,
{
struct llc llc;
struct mbuf *m0;
- int s, error = 0;
+ int error = 0;
int hassnap = 0;
u_int16_t etype;
struct ip *ip;
@@ -2570,9 +2560,7 @@ bridge_fragment(struct bridge_softc *sc,
len += ETHER_VLAN_ENCAP_LEN;
if ((ifp->if_capabilities & IFCAP_VLAN_MTU) &&
(len - sizeof(struct ether_vlan_header) <= ifp->if_mtu)) {
- s = splnet();
bridge_ifenqueue(sc, ifp, m);
- splx(s);
return;
}
goto dropit;
@@ -2640,13 +2628,10 @@ bridge_fragment(struct bridge_softc *sc,
continue;
}
bcopy(eh, mtod(m, caddr_t), sizeof(*eh));
- s = splnet();
error = bridge_ifenqueue(sc, ifp, m);
if (error) {
- splx(s);
continue;
}
- splx(s);
} else
m_freem(m);
}
