Provide pluggable queueing interface for pf

By hiding H-FSC behind pfq_ops structure similar to the ifq_ops,
we provide a possibility to plug alternative queueing interfaces
for use in pf.  This reduces amount of H-FSC specific code in the
pf ioctl handler

While here, change the the order of elements in hfsc_class_stats
to provide some compatibility between queue stat structures of
different traffic conditioners.

No objections from henning@, ok sthen@

diff --git a/sys/net/hfsc.c b/sys/net/hfsc.c
index f28b6a5..242f13e 100644 (file)
--- a/sys/net/hfsc.c
+++ b/sys/net/hfsc.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: hfsc.c,v 1.37 2017/04/26 15:50:59 mikeb Exp $ */
+/*     $OpenBSD: hfsc.c,v 1.38 2017/05/02 12:27:37 mikeb Exp $ */
 
 /*
  * Copyright (c) 2012-2013 Henning Brauer <henning@openbsd.org>
@@ -279,6 +279,24 @@ const struct ifq_ops hfsc_ops = {
 
 const struct ifq_ops * const ifq_hfsc_ops = &hfsc_ops;
 
+/*
+ * pf queue glue.
+ */
+
+void           *hfsc_pf_alloc(struct ifnet *);
+int             hfsc_pf_addqueue(void *, struct pf_queuespec *);
+void            hfsc_pf_free(void *);
+int             hfsc_pf_qstats(struct pf_queuespec *, void *, int *);
+
+const struct pfq_ops hfsc_pf_ops = {
+       hfsc_pf_alloc,
+       hfsc_pf_addqueue,
+       hfsc_pf_free,
+       hfsc_pf_qstats
+};
+
+const struct pfq_ops * const pfq_hfsc_ops = &hfsc_pf_ops;
+
 u_int64_t
 hfsc_microuptime(void)
 {
@@ -323,7 +341,7 @@ hfsc_initialize(void)
            IPL_NONE, PR_WAITOK, "hfscintsc", NULL);
 }
 
-struct hfsc_if *
+void *
 hfsc_pf_alloc(struct ifnet *ifp)
 {
        struct hfsc_if *hif;
@@ -342,8 +360,9 @@ hfsc_pf_alloc(struct ifnet *ifp)
 }
 
 int
-hfsc_pf_addqueue(struct hfsc_if *hif, struct pf_queuespec *q)
+hfsc_pf_addqueue(void *arg, struct pf_queuespec *q)
 {
+       struct hfsc_if *hif = arg;
        struct hfsc_class *cl, *parent;
        struct hfsc_sc rtsc, lssc, ulsc;
 
@@ -416,8 +435,10 @@ hfsc_pf_qstats(struct pf_queuespec *q, void *ubuf, int *nbytes)
 }
 
 void
-hfsc_pf_free(struct hfsc_if *hif)
+hfsc_pf_free(void *arg)
 {
+       struct hfsc_if *hif = arg;
+
        hfsc_free(0, hif);
 }
 

diff --git a/sys/net/hfsc.h b/sys/net/hfsc.h
index 88108f6..ecac57d 100644 (file)
--- a/sys/net/hfsc.h
+++ b/sys/net/hfsc.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: hfsc.h,v 1.12 2017/04/26 15:50:59 mikeb Exp $ */
+/*     $OpenBSD: hfsc.h,v 1.13 2017/05/02 12:27:37 mikeb Exp $ */
 
 /*
  * Copyright (c) 2012-2013 Henning Brauer <henning@openbsd.org>
@@ -34,9 +34,6 @@
 #define        _HFSC_H_
 
 /* hfsc class flags */
-#define        HFSC_RED                0x0001  /* use RED */
-#define        HFSC_ECN                0x0002  /* use RED/ECN */
-#define        HFSC_RIO                0x0004  /* use RIO */
 #define        HFSC_DEFAULTCLASS       0x1000  /* default class */
 
 struct hfsc_pktcntr {
@@ -65,6 +62,13 @@ struct hfsc_sc {
 #define        HFSC_DEFAULTSC          (HFSC_REALTIMESC|HFSC_LINKSHARINGSC)
 
 struct hfsc_class_stats {
+       struct hfsc_pktcntr     xmit_cnt;
+       struct hfsc_pktcntr     drop_cnt;
+
+       u_int                   qlength;
+       u_int                   qlimit;
+       u_int                   period;
+
        u_int                   class_id;
        u_int32_t               class_handle;
        struct hfsc_sc          rsc;
@@ -91,12 +95,6 @@ struct hfsc_class_stats {
        u_int64_t               cur_time;
        u_int32_t               machclk_freq;
 
-       u_int                   qlength;
-       u_int                   qlimit;
-       struct hfsc_pktcntr     xmit_cnt;
-       struct hfsc_pktcntr     drop_cnt;
-       u_int                   period;
-
        u_int                   vtperiod;       /* vt period sequence no */
        u_int                   parentperiod;   /* parent's vt period seqno */
        int                     nactive;        /* number of active children */
@@ -113,15 +111,11 @@ struct pf_queuespec;
 struct hfsc_if;
 
 extern const struct ifq_ops * const ifq_hfsc_ops;
+extern const struct pfq_ops * const pfq_hfsc_ops;
 
 #define        HFSC_ENABLED(ifq)       ((ifq)->ifq_ops == ifq_hfsc_ops)
 #define        HFSC_DEFAULT_QLIMIT     50
 
-struct hfsc_if *hfsc_pf_alloc(struct ifnet *);
-int             hfsc_pf_addqueue(struct hfsc_if *, struct pf_queuespec *);
-void            hfsc_pf_free(struct hfsc_if *);
-int             hfsc_pf_qstats(struct pf_queuespec *, void *, int *);
-
 void            hfsc_initialize(void);
 u_int64_t       hfsc_microuptime(void);
 

diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index c5ec002..a880590 100644 (file)
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf_ioctl.c,v 1.309 2017/04/21 23:21:02 yasuoka Exp $ */
+/*     $OpenBSD: pf_ioctl.c,v 1.310 2017/05/02 12:27:37 mikeb Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -90,7 +90,6 @@ int                    pfclose(dev_t, int, int, struct proc *);
 int                     pfioctl(dev_t, u_long, caddr_t, int, struct proc *);
 int                     pf_begin_rules(u_int32_t *, const char *);
 int                     pf_rollback_rules(u_int32_t, char *);
-int                     pf_enable_queues(void);
 void                    pf_remove_queues(void);
 int                     pf_commit_queues(void);
 void                    pf_free_queues(struct pf_queuehead *);
@@ -549,31 +548,31 @@ pf_remove_queues(void)
                if (ifp == NULL)
                        continue;
 
-               KASSERT(HFSC_ENABLED(&ifp->if_snd));
-
                ifq_attach(&ifp->if_snd, ifq_priq_ops, NULL);
        }
 }
 
-struct pf_hfsc_queue {
+struct pf_queue_if {
        struct ifnet            *ifp;
-       struct hfsc_if          *hif;
-       struct pf_hfsc_queue    *next;
+       const struct ifq_ops    *ifqops;
+       const struct pfq_ops    *pfqops;
+       void                    *disc;
+       struct pf_queue_if      *next;
 };
 
-static inline struct pf_hfsc_queue *
-pf_hfsc_ifp2q(struct pf_hfsc_queue *list, struct ifnet *ifp)
+static inline struct pf_queue_if *
+pf_ifp2q(struct pf_queue_if *list, struct ifnet *ifp)
 {
-       struct pf_hfsc_queue *phq = list;
+       struct pf_queue_if *qif = list;
 
-       while (phq != NULL) {
-               if (phq->ifp == ifp)
-                       return (phq);
+       while (qif != NULL) {
+               if (qif->ifp == ifp)
+                       return (qif);
 
-               phq = phq->next;
+               qif = qif->next;
        }
 
-       return (phq);
+       return (qif);
 }
 
 int
@@ -581,10 +580,13 @@ pf_create_queues(void)
 {
        struct pf_queuespec     *q;
        struct ifnet            *ifp;
-       struct pf_hfsc_queue    *list = NULL, *phq;
+       struct pf_queue_if              *list = NULL, *qif;
        int                      error;
 
-       /* find root queues and alloc hfsc for these interfaces */
+       /*
+        * Find root queues and allocate traffic conditioner
+        * private data for these interfaces
+        */
        TAILQ_FOREACH(q, pf_queues_active, entries) {
                if (q->parent_qid != 0)
                        continue;
@@ -593,12 +595,16 @@ pf_create_queues(void)
                if (ifp == NULL)
                        continue;
 
-               phq = malloc(sizeof(*phq), M_TEMP, M_WAITOK);
-               phq->ifp = ifp;
-               phq->hif = hfsc_pf_alloc(ifp);
+               qif = malloc(sizeof(*qif), M_TEMP, M_WAITOK);
+               qif->ifp = ifp;
 
-               phq->next = list;
-               list = phq;
+               qif->ifqops = ifq_hfsc_ops;
+               qif->pfqops = pfq_hfsc_ops;
+
+               qif->disc = qif->pfqops->pfq_alloc(ifp);
+
+               qif->next = list;
+               list = qif;
        }
 
        /* and now everything */
@@ -607,10 +613,10 @@ pf_create_queues(void)
                if (ifp == NULL)
                        continue;
 
-               phq = pf_hfsc_ifp2q(list, ifp);
-               KASSERT(phq != NULL);
+               qif = pf_ifp2q(list, ifp);
+               KASSERT(qif != NULL);
 
-               error = hfsc_pf_addqueue(phq->hif, q);
+               error = qif->pfqops->pfq_addqueue(qif->disc, q);
                if (error != 0)
                        goto error;
        }
@@ -624,8 +630,8 @@ pf_create_queues(void)
                if (ifp == NULL)
                        continue;
 
-               phq = pf_hfsc_ifp2q(list, ifp);
-               if (phq != NULL)
+               qif = pf_ifp2q(list, ifp);
+               if (qif != NULL)
                        continue;
 
                ifq_attach(&ifp->if_snd, ifq_priq_ops, NULL);
@@ -633,24 +639,24 @@ pf_create_queues(void)
 
        /* commit the new queues */
        while (list != NULL) {
-               phq = list;
-               list = phq->next;
+               qif = list;
+               list = qif->next;
 
-               ifp = phq->ifp;
+               ifp = qif->ifp;
 
-               ifq_attach(&ifp->if_snd, ifq_hfsc_ops, phq->hif);
-               free(phq, M_TEMP, sizeof(*phq));
+               ifq_attach(&ifp->if_snd, qif->ifqops, qif->disc);
+               free(qif, M_TEMP, sizeof(*qif));
        }
 
        return (0);
 
 error:
        while (list != NULL) {
-               phq = list;
-               list = phq->next;
+               qif = list;
+               list = qif->next;
 
-               hfsc_pf_free(phq->hif);
-               free(phq, M_TEMP, sizeof(*phq));
+               qif->pfqops->pfq_free(qif->disc);
+               free(qif, M_TEMP, sizeof(*qif));
        }
 
        return (error);
@@ -1082,7 +1088,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
                        break;
                }
                bcopy(qs, &pq->queue, sizeof(pq->queue));
-               error = hfsc_pf_qstats(qs, pq->buf, &nbytes);
+               error = pfq_hfsc_ops->pfq_qstats(qs, pq->buf, &nbytes);
                if (error == 0)
                        pq->nbytes = nbytes;
                break;
@@ -1117,8 +1123,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
                }
                /* XXX resolve bw percentage specs */
                pfi_kif_ref(qs->kif, PFI_KIF_REF_RULE);
-               if (qs->qlimit == 0)
-                       qs->qlimit = HFSC_DEFAULT_QLIMIT;
+
                TAILQ_INSERT_TAIL(pf_queues_inactive, qs, entries);
 
                break;

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 7bf8375..981d778 100644 (file)
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pfvar.h,v 1.450 2017/03/17 17:19:17 mpi Exp $ */
+/*     $OpenBSD: pfvar.h,v 1.451 2017/05/02 12:27:37 mikeb Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1345,6 +1345,13 @@ struct hfsc_opts {
        int             flags;
 };
 
+struct pfq_ops {
+       void            *(*pfq_alloc)(struct ifnet *);
+       int              (*pfq_addqueue)(void *, struct pf_queuespec *);
+       void             (*pfq_free)(void *);
+       int              (*pfq_qstats)(struct pf_queuespec *, void *, int *);
+};
+
 struct pf_tagname {
        TAILQ_ENTRY(pf_tagname) entries;
        char                    name[PF_TAG_NAME_SIZE];