On Mon, Oct 16, 2017 at 12:47:06PM +0200, 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.
>
> Index: net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.517
> diff -u -p -r1.517 if.c
> --- net/if.c 16 Oct 2017 08:19:15 -0000 1.517
> +++ net/if.c 16 Oct 2017 10:37:37 -0000
> @@ -871,9 +871,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)
> @@ -900,22 +897,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
> @@ -932,11 +913,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 16 Oct 2017 10:41:09 -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 16 Oct 2017 10:41:22 -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.203
> diff -u -p -r1.203 ip6_input.c
> --- netinet6/ip6_input.c 9 Oct 2017 08:35:38 -0000 1.203
> +++ netinet6/ip6_input.c 16 Oct 2017 10:41:26 -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,40 +1454,16 @@ 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) {
> ip6_output(m, NULL, NULL, IPV6_MINMTU, NULL, NULL);
> }
> NET_UNLOCK();
> -
> -#ifdef IPSEC
> - if (locked)
> - KERNEL_UNLOCK();
> -#endif /* IPSEC */
> }
>
> void
>
I've been running this on my daily road warrior for the past three
days without any regressions so far. The gateway is still running a
slightly older snapshot though; will update soon and report back with
your diff on both sides.
