On 16/10/17(Mon) 12:47, Martin Pieuchot wrote:
> On 11/10/17(Wed) 17:01, Martin Pieuchot wrote:
> > OpenBSD 6.2 includes nice performance and latency improvements due to
> > the work done in the Network Stack in the previous years. However as
> > soon as IPsec is enabled, all network related processing are affected.
> > In other words you cannot profit from the last MP work in the Network
> > stack if you use IPsec.
> >
> > During the last 6 months I hoped that somebody else would look at the
> > IPsec stack and tell us what needs to be done. This didn't happen.
> >
> > Now that 6.2 is released, we cannot afford to continue to parallelize
> > the Network Stack if some of our users and testers still run it under
> > KERNEL_LOCK().
> >
> > So I did an audit of the IPsec stack and came with the small diff below.
> > This diff doesn't remove the KERNEL_LOCK() (yet), but add some asserts
> > to make sure that the global data structure are all accessed while
> > holding the NET_LOCK().
>
> Here's the diff to stop grabbing the KERNEL_LOCK(), please test and
> report back.
Updated diff to apply on top of recent changes. I'm still looking for
tests and oks.
Index: net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.519
diff -u -p -r1.519 if.c
--- net/if.c 16 Oct 2017 13:40:58 -0000 1.519
+++ net/if.c 25 Oct 2017 09:36:14 -0000
@@ -873,9 +873,6 @@ if_input_process(void *xifidx)
struct ifih *ifih;
struct srp_ref sr;
int s;
-#ifdef IPSEC
- int locked = 0;
-#endif /* IPSEC */
ifp = if_get(ifidx);
if (ifp == NULL)
@@ -902,22 +899,6 @@ if_input_process(void *xifidx)
*/
NET_LOCK();
s = splnet();
-
-#ifdef IPSEC
- /*
- * IPsec is not ready to run without KERNEL_LOCK(). So all
- * the traffic on your machine is punished if you have IPsec
- * enabled.
- */
- extern int ipsec_in_use;
- if (ipsec_in_use) {
- NET_UNLOCK();
- KERNEL_LOCK();
- NET_LOCK();
- locked = 1;
- }
-#endif /* IPSEC */
-
while ((m = ml_dequeue(&ml)) != NULL) {
/*
* Pass this mbuf to all input handlers of its
@@ -934,11 +915,6 @@ if_input_process(void *xifidx)
}
splx(s);
NET_UNLOCK();
-
-#ifdef IPSEC
- if (locked)
- KERNEL_UNLOCK();
-#endif /* IPSEC */
out:
if_put(ifp);
}
Index: netinet/ip_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.323
diff -u -p -r1.323 ip_input.c
--- netinet/ip_input.c 9 Oct 2017 08:35:38 -0000 1.323
+++ netinet/ip_input.c 25 Oct 2017 09:36:14 -0000
@@ -482,8 +482,6 @@ ip_input_if(struct mbuf **mp, int *offp,
if (ipsec_in_use) {
int rv;
- KERNEL_ASSERT_LOCKED();
-
rv = ipsec_forward_check(m, hlen, AF_INET);
if (rv != 0) {
ipstat_inc(ips_cantforward);
@@ -1825,40 +1823,16 @@ ip_send_dispatch(void *xmq)
struct mbuf_queue *mq = xmq;
struct mbuf *m;
struct mbuf_list ml;
-#ifdef IPSEC
- int locked = 0;
-#endif /* IPSEC */
mq_delist(mq, &ml);
if (ml_empty(&ml))
return;
NET_LOCK();
-
-#ifdef IPSEC
- /*
- * IPsec is not ready to run without KERNEL_LOCK(). So all
- * the traffic on your machine is punished if you have IPsec
- * enabled.
- */
- extern int ipsec_in_use;
- if (ipsec_in_use) {
- NET_UNLOCK();
- KERNEL_LOCK();
- NET_LOCK();
- locked = 1;
- }
-#endif /* IPSEC */
-
while ((m = ml_dequeue(&ml)) != NULL) {
ip_output(m, NULL, NULL, 0, NULL, NULL, 0);
}
NET_UNLOCK();
-
-#ifdef IPSEC
- if (locked)
- KERNEL_UNLOCK();
-#endif /* IPSEC */
}
void
Index: netinet/ip_output.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.342
diff -u -p -r1.342 ip_output.c
--- netinet/ip_output.c 20 Sep 2017 16:22:02 -0000 1.342
+++ netinet/ip_output.c 25 Oct 2017 09:36:14 -0000
@@ -233,7 +233,6 @@ reroute:
#ifdef IPSEC
if (ipsec_in_use || inp != NULL) {
- KERNEL_ASSERT_LOCKED();
/* Do we have any pending SAs to apply ? */
tdb = ip_output_ipsec_lookup(m, hlen, &error, inp,
ipsecflowinfo);
@@ -404,7 +403,6 @@ sendit:
* Check if the packet needs encapsulation.
*/
if (tdb != NULL) {
- KERNEL_ASSERT_LOCKED();
/* Callee frees mbuf */
error = ip_output_ipsec_send(tdb, m, ifp, ro);
goto done;
Index: netinet6/ip6_input.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.204
diff -u -p -r1.204 ip6_input.c
--- netinet6/ip6_input.c 18 Oct 2017 13:16:35 -0000 1.204
+++ netinet6/ip6_input.c 25 Oct 2017 09:36:47 -0000
@@ -521,7 +521,6 @@ ip6_input_if(struct mbuf **mp, int *offp
if (ipsec_in_use) {
int rv;
- KERNEL_ASSERT_LOCKED();
rv = ipsec_forward_check(m, *offp, AF_INET6);
if (rv != 0) {
ip6stat_inc(ip6s_cantforward);
@@ -1455,31 +1454,12 @@ ip6_send_dispatch(void *xmq)
struct mbuf_queue *mq = xmq;
struct mbuf *m;
struct mbuf_list ml;
-#ifdef IPSEC
- int locked = 0;
-#endif /* IPSEC */
mq_delist(mq, &ml);
if (ml_empty(&ml))
return;
NET_LOCK();
-
-#ifdef IPSEC
- /*
- * IPsec is not ready to run without KERNEL_LOCK(). So all
- * the traffic on your machine is punished if you have IPsec
- * enabled.
- */
- extern int ipsec_in_use;
- if (ipsec_in_use) {
- NET_UNLOCK();
- KERNEL_LOCK();
- NET_LOCK();
- locked = 1;
- }
-#endif /* IPSEC */
-
while ((m = ml_dequeue(&ml)) != NULL) {
/*
* To avoid a "too big" situation at an intermediate router and
@@ -1491,11 +1471,6 @@ ip6_send_dispatch(void *xmq)
ip6_output(m, NULL, NULL, IPV6_MINMTU, NULL, NULL);
}
NET_UNLOCK();
-
-#ifdef IPSEC
- if (locked)
- KERNEL_UNLOCK();
-#endif /* IPSEC */
}
void
