Re: unlimited HFSC v3: more readable, less hacks

2013-10-21 Thread Claudio Jeker 
On Mon, Oct 21, 2013 at 12:04:14AM +0200, Martin Pelikan wrote:
 Hopefully the third time does the charm.
 
 The previous union approach to altq/newq bits was wrong, because
 switching back and forth was racy.  This new diff then concatenates
 these structures like [ifqueue, hfsc_if, altq-bits], has some better
 names, doesn't need renaming stuff in the old code (it can remain 2
 years old), removes the now useless ifq_hfsc pointer.

Can't we not just nuke altq? It is going to die anyway so why try to keep
it alive?
 
 As always, it's being heavily tested.
 --
 Martin Pelikan
 
 
 ? net/hfsc.c.instrumented
 Index: altq/if_altq.h
 ===
 RCS file: /cvs/src/sys/altq/if_altq.h,v
 retrieving revision 1.16
 diff -u -p -r1.16 if_altq.h
 --- altq/if_altq.h12 Oct 2013 12:13:10 -  1.16
 +++ altq/if_altq.h20 Oct 2013 21:25:41 -
 @@ -36,6 +36,13 @@ struct altq_pktattr; struct oldtb_regula
  /*
   * Structure defining a queue for a network interface.
   */
 +
 +/* XXX hack, because we need the structure definition */
 +#define ALTQ_IS_ENABLED  1
 +#include net/hfsc.h
 +#undef ALTQ_IS_ENABLED
 +/* XXX hack */
 +
  struct   ifaltq {
   /* fields compatible with struct ifqueue */
   struct {
 @@ -45,8 +52,8 @@ struct  ifaltq {
   int ifq_len;
   int ifq_maxlen;
   int ifq_drops;
 - struct  hfsc_if *ifq_hfsc;
   struct  timeout *ifq_congestion;
 + struct  hfsc_if  ifq_hfsc;
  
   /* alternate queueing related fields */
   int altq_type;  /* discipline type */
 Index: net/hfsc.c
 ===
 RCS file: /cvs/src/sys/net/hfsc.c,v
 retrieving revision 1.1
 diff -u -p -r1.1 hfsc.c
 --- net/hfsc.c12 Oct 2013 11:39:17 -  1.1
 +++ net/hfsc.c20 Oct 2013 21:25:42 -
 @@ -63,7 +63,7 @@
  /*
   * function prototypes
   */
 -struct hfsc_class*hfsc_class_create(struct hfsc_if *,
 +struct hfsc_class*hfsc_class_create(struct ifqueue *,
   struct hfsc_sc *, struct hfsc_sc *,
   struct hfsc_sc *, struct hfsc_class *, int,
   int, int);
 @@ -128,18 +128,49 @@ hfsc_microuptime(void)
   HFSC_CLK_SHIFT);
  }
  
 +/*
 + * The new table will be exactly one page larger, so in the most
 + * common case of 8B pointers and 4KB pages it's 512 more classes.
 + * Returns the amount of classes, so all new pages are 100% utilized.
 + */
 +static inline u_int
 +hfsc_more_slots(u_int current)
 +{
 + u_int was_pages = current * sizeof(void *) / PAGE_SIZE;
 + u_int n = ((was_pages + 1) * PAGE_SIZE) / sizeof(void *);
 +
 + return (n);
 +}
 +
 +static void
 +hfsc_grow_class_tbl(struct hfsc_if *hif)
 +{
 + struct hfsc_class **newtbl, **old = hif-hif_class_tbl;
 + const u_int slots = hfsc_more_slots(hif-hif_allocated);
 +
 + newtbl = malloc(slots * sizeof(void *), M_DEVBUF, M_WAITOK | M_ZERO);
 + memcpy(newtbl, old, hif-hif_allocated * sizeof(void *));
 +
 + hif-hif_allocated = slots;
 + hif-hif_class_tbl = newtbl;
 +
 + free(old, M_DEVBUF);
 +}
 +
  int
  hfsc_attach(struct ifnet *ifp)
  {
 - struct hfsc_if *hif;
 + const u_int slots = hfsc_more_slots(0);
 + const size_t sz = slots * sizeof(void *);
 + struct hfsc_if *hif = ifp-if_snd.ifq_hfsc;
  
 - if (ifp-if_snd.ifq_hfsc != NULL)
 + if (hif-hif_class_tbl != NULL)
   return (0);
  
 - hif = malloc(sizeof(struct hfsc_if), M_DEVBUF, M_WAITOK|M_ZERO);
 + hif-hif_class_tbl = malloc(sz, M_DEVBUF, M_WAITOK | M_ZERO);
 + hif-hif_allocated = slots;
 + hif-hif_classes = 0;
   hif-hif_eligible = hfsc_ellist_alloc();
 - hif-hif_ifq = (struct ifqueue *)ifp-if_snd; /* XXX cast temp */
 - ifp-if_snd.ifq_hfsc = hif;
  
   return (0);
  }
 @@ -147,8 +178,11 @@ hfsc_attach(struct ifnet *ifp)
  int
  hfsc_detach(struct ifnet *ifp)
  {
 - free(ifp-if_snd.ifq_hfsc, M_DEVBUF);
 - ifp-if_snd.ifq_hfsc = NULL;
 + struct hfsc_if *hif = ifp-if_snd.ifq_hfsc;
 +
 + hif-hif_allocated = hif-hif_classes = 0;
 + free(hif-hif_class_tbl, M_DEVBUF);
 + hif-hif_class_tbl = NULL;
  
   return (0);
  }
 @@ -156,11 +190,13 @@ hfsc_detach(struct ifnet *ifp)
  int
  hfsc_addqueue(struct pf_queuespec *q)
  {
 - struct hfsc_if *hif;
 + /* XXX remove the cast when ifaltq is gone. */
 + struct ifqueue *ifq = (struct ifqueue *)q-kif-pfik_ifp-if_snd;
 + struct hfsc_if *hif = ifq-ifq_hfsc;
   struct hfsc_class *cl, *parent;
   struct hfsc_sc rtsc, lssc, ulsc;
  
 - if ((hif = q-kif-pfik_ifp-if_snd.ifq_hfsc) == NULL)
 + if (hif-hif_allocated == 0)
   return (EINVAL);
  
   if (q-parent_qid == HFSC_NULLCLASS_HANDLE 
 @@ -185,7 +221,7 @@ hfsc_addqueue(struct pf_queuespec *q)
   ulsc.d  = q-upperlimit.d;
   ulsc.m2 = q-upperlimit.m2.absolute;

Re: unlimited HFSC v3: more readable, less hacks

2013-10-21 Thread Henning Brauer 
* Claudio Jeker cje...@diehard.n-r-g.com [2013-10-21 11:20]:
 Can't we not just nuke altq? It is going to die anyway so why try to keep
 it alive?

no, sorry.

as much as keeping it was a mistake (wearing my programmer hat), the
painful work has been done already, and the feedback i got on it is
clear.

besides, newqueue isn't a 100% replacement yet. last not least RED (or
sth similiar) is missing.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/

unlimited HFSC v3: more readable, less hacks

2013-10-20 Thread Martin Pelikan 
Hopefully the third time does the charm.

The previous union approach to altq/newq bits was wrong, because
switching back and forth was racy.  This new diff then concatenates
these structures like [ifqueue, hfsc_if, altq-bits], has some better
names, doesn't need renaming stuff in the old code (it can remain 2
years old), removes the now useless ifq_hfsc pointer.

As always, it's being heavily tested.
--
Martin Pelikan


? net/hfsc.c.instrumented
Index: altq/if_altq.h
===
RCS file: /cvs/src/sys/altq/if_altq.h,v
retrieving revision 1.16
diff -u -p -r1.16 if_altq.h
--- altq/if_altq.h  12 Oct 2013 12:13:10 -  1.16
+++ altq/if_altq.h  20 Oct 2013 21:25:41 -
@@ -36,6 +36,13 @@ struct altq_pktattr; struct oldtb_regula
 /*
  * Structure defining a queue for a network interface.
  */
+
+/* XXX hack, because we need the structure definition */
+#define ALTQ_IS_ENABLED1
+#include net/hfsc.h
+#undef ALTQ_IS_ENABLED
+/* XXX hack */
+
 struct ifaltq {
/* fields compatible with struct ifqueue */
struct {
@@ -45,8 +52,8 @@ structifaltq {
int ifq_len;
int ifq_maxlen;
int ifq_drops;
-   struct  hfsc_if *ifq_hfsc;
struct  timeout *ifq_congestion;
+   struct  hfsc_if  ifq_hfsc;
 
/* alternate queueing related fields */
int altq_type;  /* discipline type */
Index: net/hfsc.c
===
RCS file: /cvs/src/sys/net/hfsc.c,v
retrieving revision 1.1
diff -u -p -r1.1 hfsc.c
--- net/hfsc.c  12 Oct 2013 11:39:17 -  1.1
+++ net/hfsc.c  20 Oct 2013 21:25:42 -
@@ -63,7 +63,7 @@
 /*
  * function prototypes
  */
-struct hfsc_class  *hfsc_class_create(struct hfsc_if *,
+struct hfsc_class  *hfsc_class_create(struct ifqueue *,
struct hfsc_sc *, struct hfsc_sc *,
struct hfsc_sc *, struct hfsc_class *, int,
int, int);
@@ -128,18 +128,49 @@ hfsc_microuptime(void)
HFSC_CLK_SHIFT);
 }
 
+/*
+ * The new table will be exactly one page larger, so in the most
+ * common case of 8B pointers and 4KB pages it's 512 more classes.
+ * Returns the amount of classes, so all new pages are 100% utilized.
+ */
+static inline u_int
+hfsc_more_slots(u_int current)
+{
+   u_int was_pages = current * sizeof(void *) / PAGE_SIZE;
+   u_int n = ((was_pages + 1) * PAGE_SIZE) / sizeof(void *);
+
+   return (n);
+}
+
+static void
+hfsc_grow_class_tbl(struct hfsc_if *hif)
+{
+   struct hfsc_class **newtbl, **old = hif-hif_class_tbl;
+   const u_int slots = hfsc_more_slots(hif-hif_allocated);
+
+   newtbl = malloc(slots * sizeof(void *), M_DEVBUF, M_WAITOK | M_ZERO);
+   memcpy(newtbl, old, hif-hif_allocated * sizeof(void *));
+
+   hif-hif_allocated = slots;
+   hif-hif_class_tbl = newtbl;
+
+   free(old, M_DEVBUF);
+}
+
 int
 hfsc_attach(struct ifnet *ifp)
 {
-   struct hfsc_if *hif;
+   const u_int slots = hfsc_more_slots(0);
+   const size_t sz = slots * sizeof(void *);
+   struct hfsc_if *hif = ifp-if_snd.ifq_hfsc;
 
-   if (ifp-if_snd.ifq_hfsc != NULL)
+   if (hif-hif_class_tbl != NULL)
return (0);
 
-   hif = malloc(sizeof(struct hfsc_if), M_DEVBUF, M_WAITOK|M_ZERO);
+   hif-hif_class_tbl = malloc(sz, M_DEVBUF, M_WAITOK | M_ZERO);
+   hif-hif_allocated = slots;
+   hif-hif_classes = 0;
hif-hif_eligible = hfsc_ellist_alloc();
-   hif-hif_ifq = (struct ifqueue *)ifp-if_snd; /* XXX cast temp */
-   ifp-if_snd.ifq_hfsc = hif;
 
return (0);
 }
@@ -147,8 +178,11 @@ hfsc_attach(struct ifnet *ifp)
 int
 hfsc_detach(struct ifnet *ifp)
 {
-   free(ifp-if_snd.ifq_hfsc, M_DEVBUF);
-   ifp-if_snd.ifq_hfsc = NULL;
+   struct hfsc_if *hif = ifp-if_snd.ifq_hfsc;
+
+   hif-hif_allocated = hif-hif_classes = 0;
+   free(hif-hif_class_tbl, M_DEVBUF);
+   hif-hif_class_tbl = NULL;
 
return (0);
 }
@@ -156,11 +190,13 @@ hfsc_detach(struct ifnet *ifp)
 int
 hfsc_addqueue(struct pf_queuespec *q)
 {
-   struct hfsc_if *hif;
+   /* XXX remove the cast when ifaltq is gone. */
+   struct ifqueue *ifq = (struct ifqueue *)q-kif-pfik_ifp-if_snd;
+   struct hfsc_if *hif = ifq-ifq_hfsc;
struct hfsc_class *cl, *parent;
struct hfsc_sc rtsc, lssc, ulsc;
 
-   if ((hif = q-kif-pfik_ifp-if_snd.ifq_hfsc) == NULL)
+   if (hif-hif_allocated == 0)
return (EINVAL);
 
if (q-parent_qid == HFSC_NULLCLASS_HANDLE 
@@ -185,7 +221,7 @@ hfsc_addqueue(struct pf_queuespec *q)
ulsc.d  = q-upperlimit.d;
ulsc.m2 = q-upperlimit.m2.absolute;
 
-   cl = hfsc_class_create(hif, rtsc, lssc, ulsc,
+   cl = hfsc_class_create(ifq, rtsc, lssc, ulsc,
parent, q-qlimit, q-flags, q-qid);
if (cl == NULL)