Re: Moving away from softnet interrupts

2016-05-02 Thread Alexander Bluhm 
On Mon, May 02, 2016 at 11:27:36AM +0200, Martin Pieuchot wrote:
> On 18/04/16(Mon) 10:50, Martin Pieuchot wrote:
> > The current goal of the Network SMP effort is to have a single CPU
> > process the IP forwarding path in a process context without holding
> > the KERNEL_LOCK().  To achieve this goal we're progressively moving
> > code from the softnet interrupt context to the if_input_task.  In
> > the end we'll completely get rid of this soft-interrupt.
> > 
> > So now would be a good time to know if moving all the code currently
> > run in a soft-interrupt context to a task uncovers any bug.  I'm
> > happily running the diff below on amd64 and macppc, it even gives me
> > a small performance boost.
> > 
> > I'd appreciate more tests especially on exotic archs.
> 
> So far this has been tested on amd64, i386, powerpc, mips64 and sparc.
> 
> So I'm looking for oks.

OK bluhm@

> 
> > Index: net/if.c
> > ===
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.429
> > diff -u -p -r1.429 if.c
> > --- net/if.c16 Mar 2016 12:08:09 -  1.429
> > +++ net/if.c18 Apr 2016 08:00:08 -
> > @@ -64,9 +64,12 @@
> >  #include "bpfilter.h"
> >  #include "bridge.h"
> >  #include "carp.h"
> > +#include "ether.h"
> >  #include "pf.h"
> > +#include "pfsync.h"
> > +#include "ppp.h"
> > +#include "pppoe.h"
> >  #include "trunk.h"
> > -#include "ether.h"
> >  
> >  #include 
> >  #include 
> > @@ -148,6 +151,7 @@ int if_group_egress_build(void);
> >  void   if_watchdog_task(void *);
> >  
> >  void   if_input_process(void *);
> > +void   if_netisr(void *);
> >  
> >  #ifdef DDB
> >  void   ifa_print_all(void);
> > @@ -216,13 +220,15 @@ void  net_tick(void *);
> >  intnet_livelocked(void);
> >  intifq_congestion;
> >  
> > -struct taskq *softnettq;
> > +int netisr;
> > +struct taskq   *softnettq;
> > +
> > +struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
> > +struct task if_input_task = TASK_INITIALIZER(if_input_process, 
> > _input_queue);
> > +struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL);
> >  
> >  /*
> >   * Network interface utility routines.
> > - *
> > - * Routines with ifa_ifwith* names take sockaddr *'s as
> > - * parameters.
> >   */
> >  void
> >  ifinit(void)
> > @@ -590,9 +596,6 @@ if_enqueue(struct ifnet *ifp, struct mbu
> > return (0);
> >  }
> >  
> > -struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
> > -struct task if_input_task = TASK_INITIALIZER(if_input_process, 
> > _input_queue);
> > -
> >  void
> >  if_input(struct ifnet *ifp, struct mbuf_list *ml)
> >  {
> > @@ -803,6 +806,50 @@ if_input_process(void *xmq)
> > if_put(ifp);
> > }
> > splx(s);
> > +}
> > +
> > +void
> > +if_netisr(void *unused)
> > +{
> > +   int n, t = 0;
> > +   int s;
> > +
> > +   KERNEL_LOCK();
> > +   s = splsoftnet();
> > +
> > +   while ((n = netisr) != 0) {
> > +   sched_pause();
> > +
> > +   atomic_clearbits_int(, n);
> > +
> > +   if (n & (1 << NETISR_IP))
> > +   ipintr();
> > +#ifdef INET6
> > +   if (n & (1 << NETISR_IPV6))
> > +   ip6intr();
> > +#endif
> > +#if NPPP > 0
> > +   if (n & (1 << NETISR_PPP))
> > +   pppintr();
> > +#endif
> > +#if NBRIDGE > 0
> > +   if (n & (1 << NETISR_BRIDGE))
> > +   bridgeintr();
> > +#endif
> > +#if NPPPOE > 0
> > +   if (n & (1 << NETISR_PPPOE))
> > +   pppoeintr();
> > +#endif
> > +   t |= n;
> > +   }
> > +
> > +#if NPFSYNC > 0
> > +   if (t & (1 << NETISR_PFSYNC))
> > +   pfsyncintr();
> > +#endif
> > +
> > +   splx(s);
> > +   KERNEL_UNLOCK();
> >  }
> >  
> >  void
> > Index: net/netisr.c
> > ===
> > RCS file: net/netisr.c
> > diff -N net/netisr.c
> > --- net/netisr.c8 Jan 2016 13:53:24 -   1.10
> > +++ /dev/null   1 Jan 1970 00:00:00 -
> > @@ -1,74 +0,0 @@
> > -/*
> > - * Copyright (c) 2010 Owain G. Ainsworth 
> > - *
> > - * Permission to use, copy, modify, and distribute this software for any
> > - * purpose with or without fee is hereby granted, provided that the above
> > - * copyright notice and this permission notice appear in all copies.
> > - *
> > - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> > - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> > - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> > - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> > - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> > - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> > - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS

Re: Moving away from softnet interrupts

2016-05-02 Thread Mark Kettenis 
> Date: Mon, 2 May 2016 11:27:36 +0200
> From: Martin Pieuchot 
> 
> On 18/04/16(Mon) 10:50, Martin Pieuchot wrote:
> > The current goal of the Network SMP effort is to have a single CPU
> > process the IP forwarding path in a process context without holding
> > the KERNEL_LOCK().  To achieve this goal we're progressively moving
> > code from the softnet interrupt context to the if_input_task.  In
> > the end we'll completely get rid of this soft-interrupt.
> > 
> > So now would be a good time to know if moving all the code currently
> > run in a soft-interrupt context to a task uncovers any bug.  I'm
> > happily running the diff below on amd64 and macppc, it even gives me
> > a small performance boost.
> > 
> > I'd appreciate more tests especially on exotic archs.
> 
> So far this has been tested on amd64, i386, powerpc, mips64 and sparc.

And sparc64 (with em, vnet and bridge).  No real performance testing
done, but I noticed no issues.

> So I'm looking for oks.

ok kettenis@

> > Index: net/if.c
> > ===
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.429
> > diff -u -p -r1.429 if.c
> > --- net/if.c16 Mar 2016 12:08:09 -  1.429
> > +++ net/if.c18 Apr 2016 08:00:08 -
> > @@ -64,9 +64,12 @@
> >  #include "bpfilter.h"
> >  #include "bridge.h"
> >  #include "carp.h"
> > +#include "ether.h"
> >  #include "pf.h"
> > +#include "pfsync.h"
> > +#include "ppp.h"
> > +#include "pppoe.h"
> >  #include "trunk.h"
> > -#include "ether.h"
> >  
> >  #include 
> >  #include 
> > @@ -148,6 +151,7 @@ int if_group_egress_build(void);
> >  void   if_watchdog_task(void *);
> >  
> >  void   if_input_process(void *);
> > +void   if_netisr(void *);
> >  
> >  #ifdef DDB
> >  void   ifa_print_all(void);
> > @@ -216,13 +220,15 @@ void  net_tick(void *);
> >  intnet_livelocked(void);
> >  intifq_congestion;
> >  
> > -struct taskq *softnettq;
> > +int netisr;
> > +struct taskq   *softnettq;
> > +
> > +struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
> > +struct task if_input_task = TASK_INITIALIZER(if_input_process, 
> > _input_queue);
> > +struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL);
> >  
> >  /*
> >   * Network interface utility routines.
> > - *
> > - * Routines with ifa_ifwith* names take sockaddr *'s as
> > - * parameters.
> >   */
> >  void
> >  ifinit(void)
> > @@ -590,9 +596,6 @@ if_enqueue(struct ifnet *ifp, struct mbu
> > return (0);
> >  }
> >  
> > -struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
> > -struct task if_input_task = TASK_INITIALIZER(if_input_process, 
> > _input_queue);
> > -
> >  void
> >  if_input(struct ifnet *ifp, struct mbuf_list *ml)
> >  {
> > @@ -803,6 +806,50 @@ if_input_process(void *xmq)
> > if_put(ifp);
> > }
> > splx(s);
> > +}
> > +
> > +void
> > +if_netisr(void *unused)
> > +{
> > +   int n, t = 0;
> > +   int s;
> > +
> > +   KERNEL_LOCK();
> > +   s = splsoftnet();
> > +
> > +   while ((n = netisr) != 0) {
> > +   sched_pause();
> > +
> > +   atomic_clearbits_int(, n);
> > +
> > +   if (n & (1 << NETISR_IP))
> > +   ipintr();
> > +#ifdef INET6
> > +   if (n & (1 << NETISR_IPV6))
> > +   ip6intr();
> > +#endif
> > +#if NPPP > 0
> > +   if (n & (1 << NETISR_PPP))
> > +   pppintr();
> > +#endif
> > +#if NBRIDGE > 0
> > +   if (n & (1 << NETISR_BRIDGE))
> > +   bridgeintr();
> > +#endif
> > +#if NPPPOE > 0
> > +   if (n & (1 << NETISR_PPPOE))
> > +   pppoeintr();
> > +#endif
> > +   t |= n;
> > +   }
> > +
> > +#if NPFSYNC > 0
> > +   if (t & (1 << NETISR_PFSYNC))
> > +   pfsyncintr();
> > +#endif
> > +
> > +   splx(s);
> > +   KERNEL_UNLOCK();
> >  }
> >  
> >  void
> > Index: net/netisr.c
> > ===
> > RCS file: net/netisr.c
> > diff -N net/netisr.c
> > --- net/netisr.c8 Jan 2016 13:53:24 -   1.10
> > +++ /dev/null   1 Jan 1970 00:00:00 -
> > @@ -1,74 +0,0 @@
> > -/*
> > - * Copyright (c) 2010 Owain G. Ainsworth 
> > - *
> > - * Permission to use, copy, modify, and distribute this software for any
> > - * purpose with or without fee is hereby granted, provided that the above
> > - * copyright notice and this permission notice appear in all copies.
> > - *
> > - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> > - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> > - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> > - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> > - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> > - * ACTION OF

Re: Moving away from softnet interrupts

2016-05-02 Thread Martin Pieuchot 
On 18/04/16(Mon) 10:50, Martin Pieuchot wrote:
> The current goal of the Network SMP effort is to have a single CPU
> process the IP forwarding path in a process context without holding
> the KERNEL_LOCK().  To achieve this goal we're progressively moving
> code from the softnet interrupt context to the if_input_task.  In
> the end we'll completely get rid of this soft-interrupt.
> 
> So now would be a good time to know if moving all the code currently
> run in a soft-interrupt context to a task uncovers any bug.  I'm
> happily running the diff below on amd64 and macppc, it even gives me
> a small performance boost.
> 
> I'd appreciate more tests especially on exotic archs.

So far this has been tested on amd64, i386, powerpc, mips64 and sparc.

So I'm looking for oks.

> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.429
> diff -u -p -r1.429 if.c
> --- net/if.c  16 Mar 2016 12:08:09 -  1.429
> +++ net/if.c  18 Apr 2016 08:00:08 -
> @@ -64,9 +64,12 @@
>  #include "bpfilter.h"
>  #include "bridge.h"
>  #include "carp.h"
> +#include "ether.h"
>  #include "pf.h"
> +#include "pfsync.h"
> +#include "ppp.h"
> +#include "pppoe.h"
>  #include "trunk.h"
> -#include "ether.h"
>  
>  #include 
>  #include 
> @@ -148,6 +151,7 @@ int   if_group_egress_build(void);
>  void if_watchdog_task(void *);
>  
>  void if_input_process(void *);
> +void if_netisr(void *);
>  
>  #ifdef DDB
>  void ifa_print_all(void);
> @@ -216,13 +220,15 @@ voidnet_tick(void *);
>  int  net_livelocked(void);
>  int  ifq_congestion;
>  
> -struct taskq *softnettq;
> +int   netisr;
> +struct taskq *softnettq;
> +
> +struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
> +struct task if_input_task = TASK_INITIALIZER(if_input_process, 
> _input_queue);
> +struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL);
>  
>  /*
>   * Network interface utility routines.
> - *
> - * Routines with ifa_ifwith* names take sockaddr *'s as
> - * parameters.
>   */
>  void
>  ifinit(void)
> @@ -590,9 +596,6 @@ if_enqueue(struct ifnet *ifp, struct mbu
>   return (0);
>  }
>  
> -struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
> -struct task if_input_task = TASK_INITIALIZER(if_input_process, 
> _input_queue);
> -
>  void
>  if_input(struct ifnet *ifp, struct mbuf_list *ml)
>  {
> @@ -803,6 +806,50 @@ if_input_process(void *xmq)
>   if_put(ifp);
>   }
>   splx(s);
> +}
> +
> +void
> +if_netisr(void *unused)
> +{
> + int n, t = 0;
> + int s;
> +
> + KERNEL_LOCK();
> + s = splsoftnet();
> +
> + while ((n = netisr) != 0) {
> + sched_pause();
> +
> + atomic_clearbits_int(, n);
> +
> + if (n & (1 << NETISR_IP))
> + ipintr();
> +#ifdef INET6
> + if (n & (1 << NETISR_IPV6))
> + ip6intr();
> +#endif
> +#if NPPP > 0
> + if (n & (1 << NETISR_PPP))
> + pppintr();
> +#endif
> +#if NBRIDGE > 0
> + if (n & (1 << NETISR_BRIDGE))
> + bridgeintr();
> +#endif
> +#if NPPPOE > 0
> + if (n & (1 << NETISR_PPPOE))
> + pppoeintr();
> +#endif
> + t |= n;
> + }
> +
> +#if NPFSYNC > 0
> + if (t & (1 << NETISR_PFSYNC))
> + pfsyncintr();
> +#endif
> +
> + splx(s);
> + KERNEL_UNLOCK();
>  }
>  
>  void
> Index: net/netisr.c
> ===
> RCS file: net/netisr.c
> diff -N net/netisr.c
> --- net/netisr.c  8 Jan 2016 13:53:24 -   1.10
> +++ /dev/null 1 Jan 1970 00:00:00 -
> @@ -1,74 +0,0 @@
> -/*
> - * Copyright (c) 2010 Owain G. Ainsworth 
> - *
> - * Permission to use, copy, modify, and distribute this software for any
> - * purpose with or without fee is hereby granted, provided that the above
> - * copyright notice and this permission notice appear in all copies.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> - */
> -#include 
> -#include 
> -
> -#include 
> -
> -#include 
> -
> -#include "ppp.h"
> -#include "bridge.h"
> -#include "pppoe.h"
> -#include "pfsync.h"
> -
> -void  netintr(void *);
> -
> -int   netisr;
> -void *netisr_intr;
> -
> -void
> -netintr(void *unused)
> -{
> - int n, t = 0;
> -
> - while ((n = netisr) != 0) {
> - atomic_clearbits_int(, n);

Re: Moving away from softnet interrupts

2016-04-26 Thread Janne Johansson 
2016-04-25 9:59 GMT+02:00 Martin Pieuchot :

> > > The current goal of the Network SMP effort is to have a single CPU
> > > process the IP forwarding path in a process context without holding
> > > the KERNEL_LOCK().  To achieve this goal we're progressively moving
> > > code from the softnet interrupt context to the if_input_task.  In
> > > the end we'll completely get rid of this soft-interrupt.
> > >
> > > So now would be a good time to know if moving all the code currently
> > > run in a soft-interrupt context to a task uncovers any bug.  I'm
> > > happily running the diff below on amd64 and macppc, it even gives me
> > > a small performance boost.
> > >
> > > I'd appreciate more tests especially on exotic archs.
>
>
> I'm still looking for reports on different architectures.
>
>
I ran this overnight on my edgerouter lite, it has survived a few cvs-up's
and building a kernel over NFS.
So octeon doesn't seem to mind the patch.

-- 
May the most significant bit of your life be positive.

Re: Moving away from softnet interrupts

2016-04-25 Thread Martin Pieuchot 
On 20/04/16(Wed) 09:33, Dimitris Papastamos wrote:
> On Mon, Apr 18, 2016 at 10:50:46AM +0200, Martin Pieuchot wrote:
> > The current goal of the Network SMP effort is to have a single CPU
> > process the IP forwarding path in a process context without holding
> > the KERNEL_LOCK().  To achieve this goal we're progressively moving
> > code from the softnet interrupt context to the if_input_task.  In
> > the end we'll completely get rid of this soft-interrupt.
> > 
> > So now would be a good time to know if moving all the code currently
> > run in a soft-interrupt context to a task uncovers any bug.  I'm
> > happily running the diff below on amd64 and macppc, it even gives me
> > a small performance boost.
> > 
> > I'd appreciate more tests especially on exotic archs.
> 
> I've been running with this diff since you posted it on my home router.
> I have not encountered any issues.

Thanks for testing.

I'm still looking for reports on different architectures.

Re: Moving away from softnet interrupts

2016-04-20 Thread Dimitris Papastamos 
On Mon, Apr 18, 2016 at 10:50:46AM +0200, Martin Pieuchot wrote:
> The current goal of the Network SMP effort is to have a single CPU
> process the IP forwarding path in a process context without holding
> the KERNEL_LOCK().  To achieve this goal we're progressively moving
> code from the softnet interrupt context to the if_input_task.  In
> the end we'll completely get rid of this soft-interrupt.
> 
> So now would be a good time to know if moving all the code currently
> run in a soft-interrupt context to a task uncovers any bug.  I'm
> happily running the diff below on amd64 and macppc, it even gives me
> a small performance boost.
> 
> I'd appreciate more tests especially on exotic archs.

I've been running with this diff since you posted it on my home router.
I have not encountered any issues.

See ifconfig/route show/sysctl.conf/dmesg output below.

ifconfig:

lo0: flags=8049 mtu 32768
priority: 0
groups: lo
inet6 ::1 prefixlen 128
inet6 fe80::1%lo0 prefixlen 64 scopeid 0x4
inet 127.0.0.1 netmask 0xff00
re0: flags=18843 mtu 1500
lladdr 90:e6:ba:74:0e:38
description: WAN
priority: 0
groups: egress
media: Ethernet autoselect (1000baseT full-duplex,rxpause,txpause)
status: active
inet 82.13.64.4 netmask 0xf000 broadcast 82.13.79.255
re1: 
flags=18b43 mtu 
1500
lladdr 80:ee:73:9f:1d:3d
description: Main subnet
priority: 0
media: Ethernet autoselect (1000baseT 
full-duplex,master,rxpause,txpause)
status: active
inet 10.0.0.1 netmask 0xff00 broadcast 10.0.0.255
inet6 fe80::82ee:73ff:fe9f:1d3d%re1 prefixlen 64 scopeid 0x2
inet6 2001:470:1f15:b42::1 prefixlen 64
inet6 fdfc::1 prefixlen 64
enc0: flags=0<>
priority: 0
groups: enc
status: active
axe0: flags=8843 mtu 1500
lladdr d8:eb:97:bd:8a:77
description: VoIP subnet
priority: 0
media: Ethernet autoselect (100baseTX full-duplex)
status: active
inet 10.0.1.1 netmask 0xff00 broadcast 10.0.1.255
tun0: flags=8043 mtu 1500
description: tinc VPN
priority: 0
groups: tun
status: active
inet 172.17.0.2 netmask 0xff00 broadcast 172.17.0.255
tun1: flags=8051 mtu 1304
description: cjdns
priority: 0
groups: tun
status: active
inet6 fe80::92e6:baff:fe74:e38%tun1 ->  prefixlen 64 scopeid 0x7
inet6 fcac:2580:3f14:3393:f405:d913:98af:ec5f ->  prefixlen 8
tun2: flags=8051 mtu 1500
description: stun
priority: 0
groups: tun
status: down
inet 10.10.0.2 --> 10.10.0.1 netmask 0x
gif0: flags=8051 mtu 1480
description: Hurricane Electric 6in4 link
priority: 0
groups: gif egress
tunnel: inet 82.13.64.4 -> 216.66.84.46
inet6 fe80::92e6:baff:fe74:e38%gif0 ->  prefixlen 64 scopeid 0x9
inet6 2001:470:1f14:b42::2 -> 2001:470:1f14:b42::1 prefixlen 128
bridge0: flags=41
description: Dedi bridge
groups: bridge
priority 32768 hellotime 2 fwddelay 15 maxage 20 holdcnt 6 proto rstp
re1 flags=3
port 2 ifpriority 0 ifcost 0
tap0 flags=3
port 13 ifpriority 0 ifcost 0
pflow0: flags=41 mtu 1492
priority: 0
pflow: sender: 10.0.0.1 receiver: 10.0.0.7:3001 version: 5
groups: pflow
pflog0: flags=141 mtu 33144
priority: 0
groups: pflog
tap0: flags=8943 mtu 1500
lladdr fe:e1:ba:d0:b8:5e
priority: 0
groups: tap
status: active
inet 10.0.0.14 netmask 0xff00 broadcast 10.0.0.255

route show:

Routing tables

Internet:
DestinationGatewayFlags   Refs  Use   Mtu  Prio Iface
defaultcpc92320-cmbg19-2- UGS   13 36158691 - 8 re0  
BASE-ADDRESS.MCAST localhost  URS028680 32768 8 lo0  
10.0.0/24  sunUCP410431 - 4 re1  
10.0.0/24  10.0.0.14  UCP00 - 4 tap0 
sun80:ee:73:9f:1d:3d  UHLl   024952 - 1 re1  
jupiter34:64:a9:9a:5f:1c  UHLc   3 14403576 - 4 re1  
callisto   04:18:d6:f0:47:cd  UHLc   016453 - 4 re1  
pluto-mgmt e4:8d:8c:19:38:af  UHLc   010449 - 4 re1  
10.0.0.14  fe:e1:ba:d0:b8:5e  UHLl   0  125 - 1 tap0

Re: Moving away from softnet interrupts

2016-04-18 Thread Hrvoje Popovski 
On 18.4.2016. 15:31, Hrvoje Popovski wrote:
> On 18.4.2016. 10:50, Martin Pieuchot wrote:
>> The current goal of the Network SMP effort is to have a single CPU
>> process the IP forwarding path in a process context without holding
>> the KERNEL_LOCK().  To achieve this goal we're progressively moving
>> code from the softnet interrupt context to the if_input_task.  In
>> the end we'll completely get rid of this soft-interrupt.
>>
>> So now would be a good time to know if moving all the code currently
>> run in a soft-interrupt context to a task uncovers any bug.  I'm
>> happily running the diff below on amd64 and macppc, it even gives me
>> a small performance boost.
>>
>> I'd appreciate more tests especially on exotic archs.
> 
> Hi,
> 
> i have tested this patch over ix interfaces on box with 12 cores
> Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.39 MHz
> dmesg is below results ..
> 
> 
> pf=NO
> ddb.console=1
> kern.pool_debug=0
> kern.maxclusters=32768
> net.inet.ip.forwarding=1
> net.inet.ip.ifq.maxlen=8192
> 
> while sending traffic i was shutting down physical interfaces and bridge
> and box survived this ...
> 
> routing over ix
> 
> send  receive
> 400kpps   400kpps
> 500kpps   500kpps
> 600kpps   600kpps - after cca 5 min drops down to 490kpps
> 650kpps   650kpps
> 700kpps   640kpps
> 800kpps   640kpps
> 1.4Mpps   600kpps
> 14Mpps600kpps
> 
> 
> bridge over ix
> 
> send  receive
> 400kpps   400kpps
> 500kpps   500kpps - after cca 1min drops down to 450kpps
> 600kpps   600kpps
> 650kpps   640kpps
> 700kpps   610kpps
> 800kpps   560kpps
> 1.4Mpps   370kpps
> 14Mpps370kpps
> 

routing over vlan

sendreceive
400kpps 400kpps
480kpps 480kpps
500kpps 450kpps
600kpps 450kpps
800kpps 450kpps
1.4Mpps 310kpps
14Mpps  310kpps

# ifconfig vlan
vlan111: flags=18843 mtu 1500
lladdr a0:36:9f:2e:96:a0
index 14 priority 0
vlan: 111 parent interface: ix0
vnetid: 111
parent: ix0
groups: vlan
status: active
inet 10.113.0.1 netmask 0x broadcast 10.113.255.255
vlan112: flags=18843 mtu 1500
lladdr a0:36:9f:2e:96:a1
index 15 priority 0
vlan: 112 parent interface: ix1
vnetid: 112
parent: ix1
groups: vlan
status: active
inet 10.114.0.1 netmask 0x broadcast 10.114.255.255

Re: Moving away from softnet interrupts

2016-04-18 Thread Hrvoje Popovski 
On 18.4.2016. 10:50, Martin Pieuchot wrote:
> The current goal of the Network SMP effort is to have a single CPU
> process the IP forwarding path in a process context without holding
> the KERNEL_LOCK().  To achieve this goal we're progressively moving
> code from the softnet interrupt context to the if_input_task.  In
> the end we'll completely get rid of this soft-interrupt.
> 
> So now would be a good time to know if moving all the code currently
> run in a soft-interrupt context to a task uncovers any bug.  I'm
> happily running the diff below on amd64 and macppc, it even gives me
> a small performance boost.
> 
> I'd appreciate more tests especially on exotic archs.

Hi,

i have tested this patch over ix interfaces on box with 12 cores
Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.39 MHz
dmesg is below results ..


pf=NO
ddb.console=1
kern.pool_debug=0
kern.maxclusters=32768
net.inet.ip.forwarding=1
net.inet.ip.ifq.maxlen=8192

while sending traffic i was shutting down physical interfaces and bridge
and box survived this ...

routing over ix

sendreceive
400kpps 400kpps
500kpps 500kpps
600kpps 600kpps - after cca 5 min drops down to 490kpps
650kpps 650kpps
700kpps 640kpps
800kpps 640kpps
1.4Mpps 600kpps
14Mpps  600kpps


bridge over ix

sendreceive
400kpps 400kpps
500kpps 500kpps - after cca 1min drops down to 450kpps
600kpps 600kpps
650kpps 640kpps
700kpps 610kpps
800kpps 560kpps
1.4Mpps 370kpps
14Mpps  370kpps


# ifconfig ix
ix0:
flags=18b43
mtu 1500
lladdr a0:36:9f:2e:96:a0
description: connected to test1.srce.hr (p2p1)
index 3 priority 0
media: Ethernet autoselect (10GSFP+Cu full-duplex)
status: active
ix1:
flags=18b43
mtu 1500
lladdr a0:36:9f:2e:96:a1
index 4 priority 0
media: Ethernet autoselect (10GSFP+Cu full-duplex)
status: active

# ifconfig bridge0
bridge0: flags=41
index 14
groups: bridge
priority 32768 hellotime 2 fwddelay 15 maxage 20 holdcnt 6 proto
rstp
designated: id 00:00:00:00:00:00 priority 0
ix0 flags=3
port 3 ifpriority 0 ifcost 0
ix1 flags=3
port 4 ifpriority 0 ifcost 0
Addresses (max cache: 100, timeout: 240):
90:e2:ba:33:af:ed ix1 1 flags=0<>
90:e2:ba:33:af:ec ix0 1 flags=0<>



OpenBSD 5.9-current (GENERIC.MP) #0: Mon Apr 18 11:15:43 CEST 2016
r...@x3550m4.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
RTC BIOS diagnostic error 80
real mem = 34315018240 (32725MB)
avail mem = 33270788096 (31729MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0x7e67c000 (84 entries)
bios0: vendor IBM version "-[D7E146CUS-1.82]-" date 04/09/2015
bios0: IBM IBM System x3550 M4 Server -[7914T91]-
acpi0 at bios0: rev 2
acpi0: sleep states S0 S5
acpi0: tables DSDT FACP TCPA ERST HEST HPET APIC MCFG OEM0 OEM1 SLIT
SRAT SLIC SSDT SSDT SSDT SSDT SSDT SSDT DMAR
acpi0: wakeup devices MRP1(S4) DCC0(S4) MRP3(S4) MRP5(S4) EHC2(S5)
PEX0(S5) PEX7(S5) EHC1(S5) IP2P(S3) MRPB(S4) MRPC(S4) MRPD(S4) MRPF(S4)
MRPG(S4) MRPH(S4) MRPI(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.35 MHz
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 100MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.00 MHz
cpu1:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.00 MHz
cpu2:

Re: Moving away from softnet interrupts

2016-04-18 Thread Martin Pieuchot 
On 18/04/16(Mon) 11:33, Mark Kettenis wrote:
> [...] 
> However, this diff uses the same taskq for both the "unlocked" and
> "locked" parts.  My experience with crypto code was that frequently
> grabbing and releasing the kernel lock did affect latency in a bad
> way.  Some time ago dlg@ had a somewhat similar diff that had a
> separate taskq for the "locked" bits.  That would avoid blocking of
> the "unlocked" bits when the "locked" code is grabbing the kernel
> lock.  Of course such an approach would probably introduce additional
> context switches.  Did you consider such an approach?

I don't think this diff will increase the latency because currently 
softnet interrupts are processed right after dropping the SPL level
at the end of the if_input_task loop.

I also strongly believe that moving the "locked" bits to another task
should be done in a separate step, if at all.  The reason is that we
did some of the work to make the incoming path run in parallel of the
rest of the kernel, not in parallel of itself.  So I'd rather wait to
not expose too many bugs at once.

Re: Moving away from softnet interrupts

2016-04-18 Thread Mark Kettenis 
> Date: Mon, 18 Apr 2016 10:50:46 +0200
> From: Martin Pieuchot 
> 
> The current goal of the Network SMP effort is to have a single CPU
> process the IP forwarding path in a process context without holding
> the KERNEL_LOCK().  To achieve this goal we're progressively moving
> code from the softnet interrupt context to the if_input_task.  In
> the end we'll completely get rid of this soft-interrupt.
> 
> So now would be a good time to know if moving all the code currently
> run in a soft-interrupt context to a task uncovers any bug.  I'm
> happily running the diff below on amd64 and macppc, it even gives me
> a small performance boost.
> 
> I'd appreciate more tests especially on exotic archs.

Yes, please test this.  And report back how this changes the results
of your favourite benchmark.

However, this diff uses the same taskq for both the "unlocked" and
"locked" parts.  My experience with crypto code was that frequently
grabbing and releasing the kernel lock did affect latency in a bad
way.  Some time ago dlg@ had a somewhat similar diff that had a
separate taskq for the "locked" bits.  That would avoid blocking of
the "unlocked" bits when the "locked" code is grabbing the kernel
lock.  Of course such an approach would probably introduce additional
context switches.  Did you consider such an approach?


> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.429
> diff -u -p -r1.429 if.c
> --- net/if.c  16 Mar 2016 12:08:09 -  1.429
> +++ net/if.c  18 Apr 2016 08:00:08 -
> @@ -64,9 +64,12 @@
>  #include "bpfilter.h"
>  #include "bridge.h"
>  #include "carp.h"
> +#include "ether.h"
>  #include "pf.h"
> +#include "pfsync.h"
> +#include "ppp.h"
> +#include "pppoe.h"
>  #include "trunk.h"
> -#include "ether.h"
>  
>  #include 
>  #include 
> @@ -148,6 +151,7 @@ int   if_group_egress_build(void);
>  void if_watchdog_task(void *);
>  
>  void if_input_process(void *);
> +void if_netisr(void *);
>  
>  #ifdef DDB
>  void ifa_print_all(void);
> @@ -216,13 +220,15 @@ voidnet_tick(void *);
>  int  net_livelocked(void);
>  int  ifq_congestion;
>  
> -struct taskq *softnettq;
> +int   netisr;
> +struct taskq *softnettq;
> +
> +struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
> +struct task if_input_task = TASK_INITIALIZER(if_input_process, 
> _input_queue);
> +struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL);
>  
>  /*
>   * Network interface utility routines.
> - *
> - * Routines with ifa_ifwith* names take sockaddr *'s as
> - * parameters.
>   */
>  void
>  ifinit(void)
> @@ -590,9 +596,6 @@ if_enqueue(struct ifnet *ifp, struct mbu
>   return (0);
>  }
>  
> -struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
> -struct task if_input_task = TASK_INITIALIZER(if_input_process, 
> _input_queue);
> -
>  void
>  if_input(struct ifnet *ifp, struct mbuf_list *ml)
>  {
> @@ -803,6 +806,50 @@ if_input_process(void *xmq)
>   if_put(ifp);
>   }
>   splx(s);
> +}
> +
> +void
> +if_netisr(void *unused)
> +{
> + int n, t = 0;
> + int s;
> +
> + KERNEL_LOCK();
> + s = splsoftnet();
> +
> + while ((n = netisr) != 0) {
> + sched_pause();
> +
> + atomic_clearbits_int(, n);
> +
> + if (n & (1 << NETISR_IP))
> + ipintr();
> +#ifdef INET6
> + if (n & (1 << NETISR_IPV6))
> + ip6intr();
> +#endif
> +#if NPPP > 0
> + if (n & (1 << NETISR_PPP))
> + pppintr();
> +#endif
> +#if NBRIDGE > 0
> + if (n & (1 << NETISR_BRIDGE))
> + bridgeintr();
> +#endif
> +#if NPPPOE > 0
> + if (n & (1 << NETISR_PPPOE))
> + pppoeintr();
> +#endif
> + t |= n;
> + }
> +
> +#if NPFSYNC > 0
> + if (t & (1 << NETISR_PFSYNC))
> + pfsyncintr();
> +#endif
> +
> + splx(s);
> + KERNEL_UNLOCK();
>  }
>  
>  void
> Index: net/netisr.c
> ===
> RCS file: net/netisr.c
> diff -N net/netisr.c
> --- net/netisr.c  8 Jan 2016 13:53:24 -   1.10
> +++ /dev/null 1 Jan 1970 00:00:00 -
> @@ -1,74 +0,0 @@
> -/*
> - * Copyright (c) 2010 Owain G. Ainsworth 
> - *
> - * Permission to use, copy, modify, and distribute this software for any
> - * purpose with or without fee is hereby granted, provided that the above
> - * copyright notice and this permission notice appear in all copies.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> - * WHATSOEVER RESULTING FROM