On Mon, Oct 28, 2013 at 01:55:16AM +0100, Martin Pelikan wrote:
> Hi,
>
> if you noticed weak newqueue performance, it was because one component
> of it was missing. After a discussion with claudio I made this diff,
> which makes a timeout per HFSC-enabled interface and pushes the data
> every hardclock tick, similarly to what oldtbr_timeout() used to do.
>
> This way, in the future, we can:
> - Replace splnet() with mtx_enter(&ifp->lock) directly (here).
> - Replace the current coarse 1/HZ timing with a proper high resolution
> one. This should be done directly in hfsc_dequeue by setting the
> exact value (or maybe slightly more, to accumulate more dequeues into
> one if_start call) instead of just "1 tick all the time".
>
> The timing details will probably become clearer when we switch from
> TAILQs to RB trees and stop spending huge amounts of time going back
> and forth over all of the active classes.
>
> This diff applies to our current tree, to make it work with unlimited v3
> diff, you obviously need to substitute some ->'s for .'s.
>
> comments? ok?
Diff itself is OK. I think there are other options to get higher
resolution tickers by running the ticker in something that is called more
often (for example the softintr handler of the network stack).
But something like this needed and should go in.
> --
> Martin Pelikan
>
>
> Index: hfsc.c
> ===================================================================
> RCS file: /cvs/src/sys/net/hfsc.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 hfsc.c
> --- hfsc.c 12 Oct 2013 11:39:17 -0000 1.1
> +++ hfsc.c 27 Oct 2013 22:53:50 -0000
> @@ -74,6 +74,7 @@ int hfsc_addq(struct hfsc_class *, str
> struct mbuf *hfsc_getq(struct hfsc_class *);
> struct mbuf *hfsc_pollq(struct hfsc_class *);
> void hfsc_purgeq(struct hfsc_class *);
> +void hfsc_deferred(void *);
> void hfsc_update_cfmin(struct hfsc_class *);
> void hfsc_set_active(struct hfsc_class *, int);
> void hfsc_set_passive(struct hfsc_class *);
> @@ -140,6 +141,9 @@ hfsc_attach(struct ifnet *ifp)
> hif->hif_eligible = hfsc_ellist_alloc();
> hif->hif_ifq = (struct ifqueue *)&ifp->if_snd; /* XXX cast temp */
> ifp->if_snd.ifq_hfsc = hif;
> + timeout_set(&hif->hif_defer, hfsc_deferred, ifp);
> + /* XXX HRTIMER don't schedule it yet, only when some packets wait. */
> + timeout_add(&hif->hif_defer, 1);
>
> return (0);
> }
> @@ -147,6 +151,7 @@ hfsc_attach(struct ifnet *ifp)
> int
> hfsc_detach(struct ifnet *ifp)
> {
> + timeout_del(&ifp->if_snd.ifq_hfsc->hif_defer);
> free(ifp->if_snd.ifq_hfsc, M_DEVBUF);
> ifp->if_snd.ifq_hfsc = NULL;
>
> @@ -557,6 +562,7 @@ hfsc_dequeue(struct ifqueue *ifq, int re
>
> cl = tcl;
> }
> + /* XXX HRTIMER plan hfsc_deferred precisely here. */
> if (cl == NULL)
> return (NULL);
> }
> @@ -601,6 +607,21 @@ hfsc_dequeue(struct ifqueue *ifq, int re
> }
>
> return (m);
> +}
> +
> +void
> +hfsc_deferred(void *arg)
> +{
> + struct ifnet *ifp = arg;
> + int s;
> +
> + s = splnet();
> + if (HFSC_ENABLED(&ifp->if_snd) && !IFQ_IS_EMPTY(&ifp->if_snd))
> + if_start(ifp);
> + splx(s);
> +
> + /* XXX HRTIMER nearest virtual/fit time is likely less than 1/HZ. */
> + timeout_add(&ifp->if_snd.ifq_hfsc->hif_defer, 1);
> }
>
> int
> Index: hfsc.h
> ===================================================================
> RCS file: /cvs/src/sys/net/hfsc.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 hfsc.h
> --- hfsc.h 12 Oct 2013 11:39:17 -0000 1.1
> +++ hfsc.h 27 Oct 2013 22:53:50 -0000
> @@ -33,6 +33,8 @@
> #ifndef _HFSC_H_
> #define _HFSC_H_
>
> +#include <sys/timeout.h>
> +
> /* hfsc class flags */
> #define HFSC_RED 0x0001 /* use RED */
> #define HFSC_ECN 0x0002 /* use RED/ECN */
> @@ -242,6 +244,7 @@ struct hfsc_if {
> u_int hif_classid; /* class id sequence number */
>
> hfsc_ellist_t *hif_eligible; /* eligible list */
> + struct timeout hif_defer; /* for queues that weren't ready */
> };
>
> #define HFSC_CLK_SHIFT 8
--
:wq Claudio
