reintroduce tx mitigation
authordlg <dlg@openbsd.org>
Tue, 2 Jan 2018 07:08:10 +0000 (07:08 +0000)
committerdlg <dlg@openbsd.org>
Tue, 2 Jan 2018 07:08:10 +0000 (07:08 +0000)
to quote the previous commit:

> this replaces ifq_start with code that waits until at least 4 packets
> have been queued on the ifq before calling the drivers start routine.
> if less than 4 packets get queued, the start routine is called from
> a task in a softnet tq.
>
> 4 packets was chosen this time based on testing sephe did in dfly
> which showed no real improvement when bundling more packets.  hrvoje
> popovski tested this on several nics and found an improvement of
> 10 to 20 percent when forwarding across the board.
>
> because some of the ifq's work could be sitting on a softnet tq,
> ifq_barrier now calls taskq_barrier to guarantee any work that was
> pending there has finished.
>
> ok mpi@ visa@

this was backed out because of a race in the net80211 stack that
anton@ hit. mpi@ committed a workaround for it in revision 1.30 of
src/sys/net80211/ieee80211_pae_output.c.

im putting this in again so we can see what breaks next.

sys/net/ifq.c
sys/net/ifq.h

index add143c..6b71376 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ifq.c,v 1.19 2017/12/15 01:40:39 dlg Exp $ */
+/*     $OpenBSD: ifq.c,v 1.20 2018/01/02 07:08:10 dlg Exp $ */
 
 /*
  * Copyright (c) 2015 David Gwynne <dlg@openbsd.org>
@@ -70,9 +70,16 @@ struct priq {
 void   ifq_start_task(void *);
 void   ifq_restart_task(void *);
 void   ifq_barrier_task(void *);
+void   ifq_bundle_task(void *);
 
 #define TASK_ONQUEUE 0x1
 
+static inline void
+ifq_run_start(struct ifqueue *ifq)
+{
+       ifq_serialize(ifq, &ifq->ifq_start);
+}
+
 void
 ifq_serialize(struct ifqueue *ifq, struct task *t)
 {
@@ -113,6 +120,16 @@ ifq_is_serialized(struct ifqueue *ifq)
        return (ifq->ifq_serializer == curcpu());
 }
 
+void
+ifq_start(struct ifqueue *ifq)
+{
+       if (ifq_len(ifq) >= min(4, ifq->ifq_maxlen)) {
+               task_del(ifq->ifq_softnet, &ifq->ifq_bundle);
+               ifq_run_start(ifq);
+       } else
+               task_add(ifq->ifq_softnet, &ifq->ifq_bundle);
+}
+
 void
 ifq_start_task(void *p)
 {
@@ -136,12 +153,32 @@ ifq_restart_task(void *p)
        ifp->if_qstart(ifq);
 }
 
+void
+ifq_bundle_task(void *p)
+{
+       struct ifqueue *ifq = p;
+
+       ifq_run_start(ifq);
+}
+
 void
 ifq_barrier(struct ifqueue *ifq)
 {
        struct cond c = COND_INITIALIZER();
        struct task t = TASK_INITIALIZER(ifq_barrier_task, &c);
 
+       if (!task_del(ifq->ifq_softnet, &ifq->ifq_bundle)) {
+               int netlocked = (rw_status(&netlock) == RW_WRITE);
+
+               if (netlocked) /* XXXSMP breaks atomicity */
+                       NET_UNLOCK();
+
+               taskq_barrier(ifq->ifq_softnet);
+
+               if (netlocked)
+                       NET_LOCK();
+       }
+
        if (ifq->ifq_serializer == NULL)
                return;
 
@@ -166,6 +203,7 @@ void
 ifq_init(struct ifqueue *ifq, struct ifnet *ifp, unsigned int idx)
 {
        ifq->ifq_if = ifp;
+       ifq->ifq_softnet = net_tq(ifp->if_index);
        ifq->ifq_softc = NULL;
 
        mtx_init(&ifq->ifq_mtx, IPL_NET);
@@ -187,6 +225,7 @@ ifq_init(struct ifqueue *ifq, struct ifnet *ifp, unsigned int idx)
        mtx_init(&ifq->ifq_task_mtx, IPL_NET);
        TAILQ_INIT(&ifq->ifq_task_list);
        ifq->ifq_serializer = NULL;
+       task_set(&ifq->ifq_bundle, ifq_bundle_task, ifq);
 
        task_set(&ifq->ifq_start, ifq_start_task, ifq);
        task_set(&ifq->ifq_restart, ifq_restart_task, ifq);
@@ -238,6 +277,8 @@ ifq_destroy(struct ifqueue *ifq)
 {
        struct mbuf_list ml = MBUF_LIST_INITIALIZER();
 
+       ifq_barrier(ifq); /* ensure nothing is running with the ifq */
+
        /* don't need to lock because this is the last use of the ifq */
 
        ifq->ifq_ops->ifqop_purge(ifq, &ml);
index 8e0fcd8..345d106 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ifq.h,v 1.18 2017/12/21 03:59:32 dlg Exp $ */
+/*     $OpenBSD: ifq.h,v 1.19 2018/01/02 07:08:10 dlg Exp $ */
 
 /*
  * Copyright (c) 2015 David Gwynne <dlg@openbsd.org>
@@ -25,6 +25,7 @@ struct ifq_ops;
 
 struct ifqueue {
        struct ifnet            *ifq_if;
+       struct taskq            *ifq_softnet;
        union {
                void                    *_ifq_softc;
                /*
@@ -57,6 +58,7 @@ struct ifqueue {
        struct mutex             ifq_task_mtx;
        struct task_list         ifq_task_list;
        void                    *ifq_serializer;
+       struct task              ifq_bundle;
 
        /* work to be serialised */
        struct task              ifq_start;
@@ -405,6 +407,7 @@ void                 ifq_attach(struct ifqueue *, const struct ifq_ops *, void *);
 void            ifq_destroy(struct ifqueue *);
 void            ifq_add_data(struct ifqueue *, struct if_data *);
 int             ifq_enqueue(struct ifqueue *, struct mbuf *);
+void            ifq_start(struct ifqueue *);
 struct mbuf    *ifq_deq_begin(struct ifqueue *);
 void            ifq_deq_commit(struct ifqueue *, struct mbuf *);
 void            ifq_deq_rollback(struct ifqueue *, struct mbuf *);
@@ -440,12 +443,6 @@ ifq_is_oactive(struct ifqueue *ifq)
        return (ifq->ifq_oactive);
 }
 
-static inline void
-ifq_start(struct ifqueue *ifq)
-{
-       ifq_serialize(ifq, &ifq->ifq_start);
-}
-
 static inline void
 ifq_restart(struct ifqueue *ifq)
 {