Re: Stop calling IPsec and pf under splnet
On Tue, Jul 30, 2013 at 11:30:51AM +0200, Martin Pieuchot wrote:
On 12/07/13(Fri) 11:35, Mike Belopuhov wrote:
Hi,
As it was pointed out by dhill there are some rogue splnets in
the tcp_input that shouldn't be there really. The only reason
they're still there is to match overzealous splnets in bridge_
broadcast. bridge_ifenqueue is the only function call in there
that requires splnet protection since it's dealing with send
queues. Narrowing the range of the splnet protection allows us
to remove all splnet protection of the IPsec SPD and TDB code.
This as well removes the only pf_test call done under IPL_NET.
Below are essentially two diffs that are rather hard to separate.
I've tested the diff with the gif-to-ethernet IPsec bridge but
some additional IPsec and bridge testing won't hurt.
mpi@ has provided some feedback already, so I'm really looking
for OK's on this.
ok mpi@
been running this on i386 for a while with plenty of ipsec traffic. no
issues.
diff --git sys/net/if_bridge.c sys/net/if_bridge.c
index 41d7b67..0ca2710 100644
--- sys/net/if_bridge.c
+++ sys/net/if_bridge.c
@@ -969,12 +969,10 @@ bridge_output(struct ifnet *ifp, struct mbuf *m,
struct sockaddr *sa,
return (ENOBUFS);
}
eh = mtod(m, struct ether_header *);
dst = (struct ether_addr *)eh-ether_dhost[0];
- s = splnet();
-
/*
* If bridge is down, but original output interface is up,
* go ahead and send out that interface. Otherwise the packet
* is dropped below.
*/
@@ -1007,11 +1005,10 @@ bridge_output(struct ifnet *ifp, struct mbuf *m,
struct sockaddr *sa,
*/
if ((mtag = m_tag_find(m, PACKET_TAG_IPSEC_OUT_CRYPTO_NEEDED,
NULL)) != NULL) {
ipsp_skipcrypto_unmark((struct tdb_ident *)(mtag + 1));
m_freem(m);
- splx(s);
return (0);
}
#endif /* IPSEC */
bridge_span(sc, NULL, m);
@@ -1074,22 +1071,24 @@ bridge_output(struct ifnet *ifp, struct mbuf *m,
struct sockaddr *sa,
m1-m_pkthdr.len = len;
}
mc = m1;
}
+ s = splnet();
error = bridge_ifenqueue(sc, dst_if, mc);
+ splx(s);
if (error)
continue;
}
if (!used)
m_freem(m);
- splx(s);
return (0);
}
sendunicast:
bridge_span(sc, NULL, m);
+ s = splnet();
if ((dst_if-if_flags IFF_RUNNING) == 0) {
m_freem(m);
splx(s);
return (ENETDOWN);
}
@@ -1251,13 +1250,11 @@ bridgeintr_frame(struct bridge_softc *sc, struct
mbuf *m)
* If the packet is a multicast or broadcast OR if we don't
* know any better, forward it to all interfaces.
*/
if ((m-m_flags (M_BCAST | M_MCAST)) || dst_if == NULL) {
sc-sc_if.if_imcasts++;
- s = splnet();
bridge_broadcast(sc, src_if, eh, m);
- splx(s);
return;
}
/*
* At this point, we're dealing with a unicast frame going to a
@@ -1496,13 +1493,11 @@ bridge_broadcast(struct bridge_softc *sc, struct
ifnet *ifp,
struct ether_header *eh, struct mbuf *m)
{
struct bridge_iflist *p;
struct mbuf *mc;
struct ifnet *dst_if;
- int len, used = 0;
-
- splassert(IPL_NET);
+ int len, s, used = 0;
TAILQ_FOREACH(p, sc-sc_iflist, next) {
/*
* Don't retransmit out of the same interface where
* the packet was received from.
@@ -1587,11 +1582,13 @@ bridge_broadcast(struct bridge_softc *sc, struct
ifnet *ifp,
len += ETHER_VLAN_ENCAP_LEN;
#endif
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);
}
}
if (!used)
m_freem(m);
@@ -1643,11 +1640,11 @@ bridge_span(struct bridge_softc *sc, struct
ether_header *eh,
struct mbuf *morig)
{
struct bridge_iflist *p;
struct ifnet *ifp;
struct mbuf *mc, *m;
- int error;
+ int s, error;
if (TAILQ_EMPTY(sc-sc_spanlist))
return;
m = m_copym2(morig, 0, M_COPYALL, M_NOWAIT);
@@ -1679,11 +1676,13 @@ bridge_span(struct bridge_softc *sc, struct
ether_header *eh,
if (mc == NULL) {
sc-sc_if.if_oerrors++;
continue;
}
+ s =
Re: Stop calling IPsec and pf under splnet
On 12/07/13(Fri) 11:35, Mike Belopuhov wrote:
Hi,
As it was pointed out by dhill there are some rogue splnets in
the tcp_input that shouldn't be there really. The only reason
they're still there is to match overzealous splnets in bridge_
broadcast. bridge_ifenqueue is the only function call in there
that requires splnet protection since it's dealing with send
queues. Narrowing the range of the splnet protection allows us
to remove all splnet protection of the IPsec SPD and TDB code.
This as well removes the only pf_test call done under IPL_NET.
Below are essentially two diffs that are rather hard to separate.
I've tested the diff with the gif-to-ethernet IPsec bridge but
some additional IPsec and bridge testing won't hurt.
mpi@ has provided some feedback already, so I'm really looking
for OK's on this.
ok mpi@
diff --git sys/net/if_bridge.c sys/net/if_bridge.c
index 41d7b67..0ca2710 100644
--- sys/net/if_bridge.c
+++ sys/net/if_bridge.c
@@ -969,12 +969,10 @@ bridge_output(struct ifnet *ifp, struct mbuf *m, struct
sockaddr *sa,
return (ENOBUFS);
}
eh = mtod(m, struct ether_header *);
dst = (struct ether_addr *)eh-ether_dhost[0];
- s = splnet();
-
/*
* If bridge is down, but original output interface is up,
* go ahead and send out that interface. Otherwise the packet
* is dropped below.
*/
@@ -1007,11 +1005,10 @@ bridge_output(struct ifnet *ifp, struct mbuf *m,
struct sockaddr *sa,
*/
if ((mtag = m_tag_find(m, PACKET_TAG_IPSEC_OUT_CRYPTO_NEEDED,
NULL)) != NULL) {
ipsp_skipcrypto_unmark((struct tdb_ident *)(mtag + 1));
m_freem(m);
- splx(s);
return (0);
}
#endif /* IPSEC */
bridge_span(sc, NULL, m);
@@ -1074,22 +1071,24 @@ bridge_output(struct ifnet *ifp, struct mbuf *m,
struct sockaddr *sa,
m1-m_pkthdr.len = len;
}
mc = m1;
}
+ s = splnet();
error = bridge_ifenqueue(sc, dst_if, mc);
+ splx(s);
if (error)
continue;
}
if (!used)
m_freem(m);
- splx(s);
return (0);
}
sendunicast:
bridge_span(sc, NULL, m);
+ s = splnet();
if ((dst_if-if_flags IFF_RUNNING) == 0) {
m_freem(m);
splx(s);
return (ENETDOWN);
}
@@ -1251,13 +1250,11 @@ bridgeintr_frame(struct bridge_softc *sc, struct mbuf
*m)
* If the packet is a multicast or broadcast OR if we don't
* know any better, forward it to all interfaces.
*/
if ((m-m_flags (M_BCAST | M_MCAST)) || dst_if == NULL) {
sc-sc_if.if_imcasts++;
- s = splnet();
bridge_broadcast(sc, src_if, eh, m);
- splx(s);
return;
}
/*
* At this point, we're dealing with a unicast frame going to a
@@ -1496,13 +1493,11 @@ bridge_broadcast(struct bridge_softc *sc, struct
ifnet *ifp,
struct ether_header *eh, struct mbuf *m)
{
struct bridge_iflist *p;
struct mbuf *mc;
struct ifnet *dst_if;
- int len, used = 0;
-
- splassert(IPL_NET);
+ int len, s, used = 0;
TAILQ_FOREACH(p, sc-sc_iflist, next) {
/*
* Don't retransmit out of the same interface where
* the packet was received from.
@@ -1587,11 +1582,13 @@ bridge_broadcast(struct bridge_softc *sc, struct
ifnet *ifp,
len += ETHER_VLAN_ENCAP_LEN;
#endif
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);
}
}
if (!used)
m_freem(m);
@@ -1643,11 +1640,11 @@ bridge_span(struct bridge_softc *sc, struct
ether_header *eh,
struct mbuf *morig)
{
struct bridge_iflist *p;
struct ifnet *ifp;
struct mbuf *mc, *m;
- int error;
+ int s, error;
if (TAILQ_EMPTY(sc-sc_spanlist))
return;
m = m_copym2(morig, 0, M_COPYALL, M_NOWAIT);
@@ -1679,11 +1676,13 @@ bridge_span(struct bridge_softc *sc, struct
ether_header *eh,
if (mc == NULL) {
sc-sc_if.if_oerrors++;
continue;
}
+ s = splnet();
error = bridge_ifenqueue(sc, ifp, mc);
+ splx(s);
if (error)
Stop calling IPsec and pf under splnet
Hi,
As it was pointed out by dhill there are some rogue splnets in
the tcp_input that shouldn't be there really. The only reason
they're still there is to match overzealous splnets in bridge_
broadcast. bridge_ifenqueue is the only function call in there
that requires splnet protection since it's dealing with send
queues. Narrowing the range of the splnet protection allows us
to remove all splnet protection of the IPsec SPD and TDB code.
This as well removes the only pf_test call done under IPL_NET.
Below are essentially two diffs that are rather hard to separate.
I've tested the diff with the gif-to-ethernet IPsec bridge but
some additional IPsec and bridge testing won't hurt.
mpi@ has provided some feedback already, so I'm really looking
for OK's on this.
diff --git sys/net/if_bridge.c sys/net/if_bridge.c
index 41d7b67..0ca2710 100644
--- sys/net/if_bridge.c
+++ sys/net/if_bridge.c
@@ -969,12 +969,10 @@ bridge_output(struct ifnet *ifp, struct mbuf *m, struct
sockaddr *sa,
return (ENOBUFS);
}
eh = mtod(m, struct ether_header *);
dst = (struct ether_addr *)eh-ether_dhost[0];
- s = splnet();
-
/*
* If bridge is down, but original output interface is up,
* go ahead and send out that interface. Otherwise the packet
* is dropped below.
*/
@@ -1007,11 +1005,10 @@ bridge_output(struct ifnet *ifp, struct mbuf *m, struct
sockaddr *sa,
*/
if ((mtag = m_tag_find(m, PACKET_TAG_IPSEC_OUT_CRYPTO_NEEDED,
NULL)) != NULL) {
ipsp_skipcrypto_unmark((struct tdb_ident *)(mtag + 1));
m_freem(m);
- splx(s);
return (0);
}
#endif /* IPSEC */
bridge_span(sc, NULL, m);
@@ -1074,22 +1071,24 @@ bridge_output(struct ifnet *ifp, struct mbuf *m, struct
sockaddr *sa,
m1-m_pkthdr.len = len;
}
mc = m1;
}
+ s = splnet();
error = bridge_ifenqueue(sc, dst_if, mc);
+ splx(s);
if (error)
continue;
}
if (!used)
m_freem(m);
- splx(s);
return (0);
}
sendunicast:
bridge_span(sc, NULL, m);
+ s = splnet();
if ((dst_if-if_flags IFF_RUNNING) == 0) {
m_freem(m);
splx(s);
return (ENETDOWN);
}
@@ -1251,13 +1250,11 @@ bridgeintr_frame(struct bridge_softc *sc, struct mbuf
*m)
* If the packet is a multicast or broadcast OR if we don't
* know any better, forward it to all interfaces.
*/
if ((m-m_flags (M_BCAST | M_MCAST)) || dst_if == NULL) {
sc-sc_if.if_imcasts++;
- s = splnet();
bridge_broadcast(sc, src_if, eh, m);
- splx(s);
return;
}
/*
* At this point, we're dealing with a unicast frame going to a
@@ -1496,13 +1493,11 @@ bridge_broadcast(struct bridge_softc *sc, struct ifnet
*ifp,
struct ether_header *eh, struct mbuf *m)
{
struct bridge_iflist *p;
struct mbuf *mc;
struct ifnet *dst_if;
- int len, used = 0;
-
- splassert(IPL_NET);
+ int len, s, used = 0;
TAILQ_FOREACH(p, sc-sc_iflist, next) {
/*
* Don't retransmit out of the same interface where
* the packet was received from.
@@ -1587,11 +1582,13 @@ bridge_broadcast(struct bridge_softc *sc, struct ifnet
*ifp,
len += ETHER_VLAN_ENCAP_LEN;
#endif
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);
}
}
if (!used)
m_freem(m);
@@ -1643,11 +1640,11 @@ bridge_span(struct bridge_softc *sc, struct
ether_header *eh,
struct mbuf *morig)
{
struct bridge_iflist *p;
struct ifnet *ifp;
struct mbuf *mc, *m;
- int error;
+ int s, error;
if (TAILQ_EMPTY(sc-sc_spanlist))
return;
m = m_copym2(morig, 0, M_COPYALL, M_NOWAIT);
@@ -1679,11 +1676,13 @@ bridge_span(struct bridge_softc *sc, struct
ether_header *eh,
if (mc == NULL) {
sc-sc_if.if_oerrors++;
continue;
}
+ s = splnet();
error = bridge_ifenqueue(sc, ifp, mc);
+ splx(s);
if (error)
