tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Bridge forwarding employing pktqueue



Hi all (esp. rmind),

Here is another patch set that employs rmind's
pktqueue for bridge forwarding (bridge_forward).
It simplifies the codes of packet queuing and
softint, and makes it easy to make bridge
forwarding MP-safe. I want to commit this change
prior to MP-safe work because this change is
relatively small amount and easy to review.

The summary of the changes:
- Add pktqueue_t *sc_fwd_pktq to bridge_softc
- Add net.interfaces.bridgeN.fwdq.{maxlen,len,drops}
  sysctl
- Add 3rd argument to pktq_create to pass sc
  to softint_establish
- Export sysctl_pktq_maxlen and sysctl_pktq_count
  to use them in if_bridge.c

rmind, could you check if the latter two changes to
pktqueue are appropriate?

The patch set can be found here:
https://github.com/ozaki-r/netbsd-src/tree/bridge-pktq

An integrated patch is attached and available here:
http://www.netbsd.org/~ozaki-r/bridge-pktq.diff

Thanks,
  ozaki-r
diff --git a/sys/net/if.c b/sys/net/if.c
index 89af5f0..7a712f7 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -2340,7 +2340,7 @@ bad:
 
 #if defined(INET) || defined(INET6)
 
-static int
+int
 sysctl_pktq_maxlen(SYSCTLFN_ARGS, pktqueue_t *pq)
 {
        u_int nmaxlen = pktq_get_count(pq, PKTQ_MAXLEN);
@@ -2354,7 +2354,7 @@ sysctl_pktq_maxlen(SYSCTLFN_ARGS, pktqueue_t *pq)
        return pktq_set_maxlen(pq, nmaxlen);
 }
 
-static int
+int
 sysctl_pktq_count(SYSCTLFN_ARGS, pktqueue_t *pq, u_int count_id)
 {
        int count = pktq_get_count(pq, count_id);
diff --git a/sys/net/if.h b/sys/net/if.h
index 67e78d4..c5d8958 100644
--- a/sys/net/if.h
+++ b/sys/net/if.h
@@ -79,8 +79,11 @@
 
 #include <sys/socket.h>
 #include <sys/queue.h>
+#include <sys/sysctl.h>
+
 #include <net/dlt.h>
 #include <net/pfil.h>
+#include <net/pktqueue.h>
 
 /*
  * Always include ALTQ glue here -- we use the ALTQ interface queue
@@ -967,6 +970,9 @@ int sysctl_ifq(int *name, u_int namelen, void *oldp,
 #define IFQCTL_DROPS 4
 #define IFQCTL_MAXID 5
 
+int sysctl_pktq_maxlen(SYSCTLFN_PROTO, pktqueue_t *);
+int sysctl_pktq_count(SYSCTLFN_PROTO, pktqueue_t *, u_int);
+
 #endif /* _KERNEL */
 
 #ifdef _NETBSD_SOURCE
diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
index ea7b4fc..b813019 100644
--- a/sys/net/if_bridge.c
+++ b/sys/net/if_bridge.c
@@ -100,12 +100,14 @@ __KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.77 2013/06/29 
21:06:58 rmind Exp $")
 #include <sys/kauth.h>
 #include <sys/cpu.h>
 #include <sys/cprng.h>
+#include <sys/xcall.h>
 
 #include <net/bpf.h>
 #include <net/if.h>
 #include <net/if_dl.h>
 #include <net/if_types.h>
 #include <net/if_llc.h>
+#include <net/pktqueue.h>
 
 #include <net/if_ether.h>
 #include <net/if_bridgevar.h>
@@ -252,6 +254,9 @@ static int  bridge_ip6_checkbasic(struct mbuf **mp);
 # endif /* INET6 */
 #endif /* BRIDGE_IPF */
 
+static void bridge_sysctl_fwdq_setup(struct sysctllog **clog,
+    struct bridge_softc *sc);
+
 struct bridge_control {
        int     (*bc_func)(struct bridge_softc *, void *);
        int     bc_argsize;
@@ -351,13 +356,6 @@ bridge_clone_create(struct if_clone *ifc, int unit)
        sc->sc_hold_time = BSTP_DEFAULT_HOLD_TIME;
        sc->sc_filter_flags = 0;
 
-       /* software interrupt to do the work */
-       sc->sc_softintr = softint_establish(SOFTINT_NET, bridge_forward, sc);
-       if (sc->sc_softintr == NULL) {
-               free(sc, M_DEVBUF);
-               return ENOMEM;
-       }
-
        /* Initialize our routing table. */
        bridge_rtable_init(sc);
 
@@ -380,6 +378,11 @@ bridge_clone_create(struct if_clone *ifc, int unit)
        ifp->if_hdrlen = ETHER_HDR_LEN;
        IFQ_SET_READY(&ifp->if_snd);
 
+       sc->sc_fwd_pktq = pktq_create(IFQ_MAXLEN, bridge_forward, sc);
+       KASSERT(sc->sc_fwd_pktq != NULL);
+
+       bridge_sysctl_fwdq_setup(&ifp->if_sysctl_log, sc);
+
        if_attach(ifp);
 
        if_alloc_sadl(ifp);
@@ -402,6 +405,12 @@ bridge_clone_destroy(struct ifnet *ifp)
        struct bridge_softc *sc = ifp->if_softc;
        struct bridge_iflist *bif;
        int s;
+       uint64_t xc;
+
+       /* Must be called during IFF_RUNNING, i.e., before bridge_stop */
+       pktq_barrier(sc->sc_fwd_pktq);
+       xc = xc_broadcast(0, (xcfunc_t)nullop, NULL, NULL);
+       xc_wait(xc);
 
        s = splnet();
 
@@ -416,16 +425,104 @@ bridge_clone_destroy(struct ifnet *ifp)
 
        if_detach(ifp);
 
+       /* Should be called after if_detach for safe */
+       pktq_flush(sc->sc_fwd_pktq);
+       pktq_destroy(sc->sc_fwd_pktq);
+
        /* Tear down the routing table. */
        bridge_rtable_fini(sc);
 
-       softint_disestablish(sc->sc_softintr);
-
        free(sc, M_DEVBUF);
 
        return (0);
 }
 
+static int
+bridge_sysctl_fwdq_maxlen(SYSCTLFN_ARGS)
+{
+       struct sysctlnode node = *rnode;
+       const struct bridge_softc *sc = node.sysctl_data;
+       return sysctl_pktq_maxlen(SYSCTLFN_CALL(rnode), sc->sc_fwd_pktq);
+}
+
+#define        SYSCTL_BRIDGE_PKTQ(cn, c)                                       
\
+       static int                                                      \
+       bridge_sysctl_fwdq_##cn(SYSCTLFN_ARGS)                          \
+       {                                                               \
+               struct sysctlnode node = *rnode;                        \
+               const struct bridge_softc *sc = node.sysctl_data;       \
+               return sysctl_pktq_count(SYSCTLFN_CALL(rnode),          \
+                                        sc->sc_fwd_pktq, c);           \
+       }
+
+SYSCTL_BRIDGE_PKTQ(items, PKTQ_NITEMS)
+SYSCTL_BRIDGE_PKTQ(drops, PKTQ_DROPS)
+
+static void
+bridge_sysctl_fwdq_setup(struct sysctllog **clog, struct bridge_softc *sc)
+{
+       const struct sysctlnode *cnode, *rnode;
+       sysctlfn len_func = NULL, maxlen_func = NULL, drops_func = NULL;
+       const char *ifname = sc->sc_if.if_xname;
+
+       len_func = bridge_sysctl_fwdq_items;
+       maxlen_func = bridge_sysctl_fwdq_maxlen;
+       drops_func = bridge_sysctl_fwdq_drops;
+
+       if (sysctl_createv(clog, 0, NULL, &rnode,
+                          CTLFLAG_PERMANENT,
+                          CTLTYPE_NODE, "interfaces",
+                          SYSCTL_DESCR("Per-interface controls"),
+                          NULL, 0, NULL, 0,
+                          CTL_NET, CTL_CREATE, CTL_EOL) != 0)
+               goto bad;
+
+       if (sysctl_createv(clog, 0, &rnode, &rnode,
+                          CTLFLAG_PERMANENT,
+                          CTLTYPE_NODE, ifname,
+                          SYSCTL_DESCR("Interface controls"),
+                          NULL, 0, NULL, 0,
+                          CTL_CREATE, CTL_EOL) != 0)
+               goto bad;
+
+       if (sysctl_createv(clog, 0, &rnode, &rnode,
+                          CTLFLAG_PERMANENT,
+                          CTLTYPE_NODE, "fwdq",
+                          SYSCTL_DESCR("Protocol input queue controls"),
+                          NULL, 0, NULL, 0,
+                          CTL_CREATE, CTL_EOL) != 0)
+               goto bad;
+
+       if (sysctl_createv(clog, 0, &rnode, &cnode,
+                          CTLFLAG_PERMANENT,
+                          CTLTYPE_INT, "len",
+                          SYSCTL_DESCR("Current forwarding queue length"),
+                          len_func, 0, (void *)sc, 0,
+                          CTL_CREATE, IFQCTL_LEN, CTL_EOL) != 0)
+               goto bad;
+
+       if (sysctl_createv(clog, 0, &rnode, &cnode,
+                          CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
+                          CTLTYPE_INT, "maxlen",
+                          SYSCTL_DESCR("Maximum allowed forwarding queue 
length"),
+                          maxlen_func, 0, (void *)sc, 0,
+                          CTL_CREATE, IFQCTL_MAXLEN, CTL_EOL) != 0)
+               goto bad;
+
+       if (sysctl_createv(clog, 0, &rnode, &cnode,
+                          CTLFLAG_PERMANENT,
+                          CTLTYPE_INT, "drops",
+                          SYSCTL_DESCR("Packets dropped due to full forwarding 
queue"),
+                          drops_func, 0, (void *)sc, 0,
+                          CTL_CREATE, IFQCTL_DROPS, CTL_EOL) != 0)
+               goto bad;
+
+       return;
+bad:
+       aprint_error("%s: could not attach sysctl nodes\n", ifname);
+       return;
+}
+
 /*
  * bridge_ioctl:
  *
@@ -1340,19 +1437,16 @@ bridge_forward(void *v)
        struct ether_header *eh;
        int s;
 
+       KERNEL_LOCK(1, NULL);
        mutex_enter(softnet_lock);
        if ((sc->sc_if.if_flags & IFF_RUNNING) == 0) {
                mutex_exit(softnet_lock);
+               KERNEL_UNLOCK_ONE(NULL);
                return;
        }
 
        s = splnet();
-       while (1) {
-               IFQ_POLL(&sc->sc_if.if_snd, m);
-               if (m == NULL)
-                       break;
-               IFQ_DEQUEUE(&sc->sc_if.if_snd, m);
-
+       while ((m = pktq_dequeue(sc->sc_fwd_pktq)) != NULL) {
                src_if = m->m_pkthdr.rcvif;
 
                sc->sc_if.if_ipackets++;
@@ -1466,6 +1560,7 @@ bridge_forward(void *v)
        }
        splx(s);
        mutex_exit(softnet_lock);
+       KERNEL_UNLOCK_ONE(NULL);
 }
 
 /*
@@ -1515,17 +1610,15 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
                 * for bridge processing; return the original packet for
                 * local processing.
                 */
-               if (IF_QFULL(&sc->sc_if.if_snd)) {
-                       IF_DROP(&sc->sc_if.if_snd);
-                       return (m);
-               }
                mc = m_dup(m, 0, M_COPYALL, M_NOWAIT);
                if (mc == NULL)
-                       return (m);
+                       return m;
 
                /* Perform the bridge forwarding function with the copy. */
-               IF_ENQUEUE(&sc->sc_if.if_snd, mc);
-               softint_schedule(sc->sc_softintr);
+               if (__predict_false(!pktq_enqueue(sc->sc_fwd_pktq, mc, 0))) {
+                       m_freem(mc);
+                       return m;
+               }
 
                /* Return the original packet for local processing. */
                return (m);
@@ -1573,13 +1666,8 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
        }
 
        /* Perform the bridge forwarding function. */
-       if (IF_QFULL(&sc->sc_if.if_snd)) {
-               IF_DROP(&sc->sc_if.if_snd);
+       if (__predict_false(!pktq_enqueue(sc->sc_fwd_pktq, m, 0)))
                m_freem(m);
-               return (NULL);
-       }
-       IF_ENQUEUE(&sc->sc_if.if_snd, m);
-       softint_schedule(sc->sc_softintr);
 
        return (NULL);
 }
diff --git a/sys/net/if_bridgevar.h b/sys/net/if_bridgevar.h
index b84d0b8..eb5ed14 100644
--- a/sys/net/if_bridgevar.h
+++ b/sys/net/if_bridgevar.h
@@ -78,6 +78,8 @@
 #include <sys/callout.h>
 #include <sys/queue.h>
 
+#include <net/pktqueue.h>
+
 /*
  * Commands used in the SIOCSDRVSPEC ioctl.  Note the lookup of the
  * bridge interface itself is keyed off the ifdrv structure.
@@ -302,6 +304,7 @@ struct bridge_softc {
        uint32_t                sc_rthash_key;  /* key for hash */
        uint32_t                sc_filter_flags; /* ipf and flags */
        void                    *sc_softintr;
+       pktqueue_t *            sc_fwd_pktq;
 };
 
 extern const uint8_t bstp_etheraddr[];
diff --git a/sys/net/pktqueue.c b/sys/net/pktqueue.c
index 21ae908..3565180 100644
--- a/sys/net/pktqueue.c
+++ b/sys/net/pktqueue.c
@@ -96,7 +96,7 @@ typedef struct {
     roundup2(offsetof(pktqueue_t, pq_queue[ncpu]), coherency_unit)
 
 pktqueue_t *
-pktq_create(size_t maxlen, void (*intrh)(void *))
+pktq_create(size_t maxlen, void (*intrh)(void *), void *sc)
 {
        const u_int sflags = SOFTINT_NET | SOFTINT_MPSAFE | SOFTINT_RCPU;
        const size_t len = PKTQUEUE_STRUCT_LEN(ncpu);
@@ -107,7 +107,7 @@ pktq_create(size_t maxlen, void (*intrh)(void *))
        if ((pc = percpu_alloc(sizeof(pktq_counters_t))) == NULL) {
                return NULL;
        }
-       if ((sih = softint_establish(sflags, intrh, NULL)) == NULL) {
+       if ((sih = softint_establish(sflags, intrh, sc)) == NULL) {
                percpu_free(pc, sizeof(pktq_counters_t));
                return NULL;
        }
diff --git a/sys/net/pktqueue.h b/sys/net/pktqueue.h
index f246115..c927021 100644
--- a/sys/net/pktqueue.h
+++ b/sys/net/pktqueue.h
@@ -42,7 +42,7 @@ typedef struct pktqueue pktqueue_t;
 
 typedef enum { PKTQ_MAXLEN, PKTQ_NITEMS, PKTQ_DROPS } pktq_count_t;
 
-pktqueue_t *   pktq_create(size_t, void (*)(void *));
+pktqueue_t *   pktq_create(size_t, void (*)(void *), void *);
 void           pktq_destroy(pktqueue_t *);
 
 bool           pktq_enqueue(pktqueue_t *, struct mbuf *, const u_int);
diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index ece1c5d..a5de132 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -304,7 +304,7 @@ ip_init(void)
        pr = pffindproto(PF_INET, IPPROTO_RAW, SOCK_RAW);
        KASSERT(pr != NULL);
 
-       ip_pktq = pktq_create(IFQ_MAXLEN, ipintr);
+       ip_pktq = pktq_create(IFQ_MAXLEN, ipintr, NULL);
        KASSERT(ip_pktq != NULL);
 
        for (u_int i = 0; i < IPPROTO_MAX; i++) {
diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c
index 020ab90..9b3bac3 100644
--- a/sys/netinet6/ip6_input.c
+++ b/sys/netinet6/ip6_input.c
@@ -180,7 +180,7 @@ ip6_init(void)
                    pr->pr_protocol && pr->pr_protocol != IPPROTO_RAW)
                        ip6_protox[pr->pr_protocol] = pr - inet6sw;
 
-       ip6_pktq = pktq_create(IFQ_MAXLEN, ip6intr);
+       ip6_pktq = pktq_create(IFQ_MAXLEN, ip6intr, NULL);
        KASSERT(ip6_pktq != NULL);
 
        scope6_init();


Home | Main Index | Thread Index | Old Index