Re: [RFC PATCH 11/13] net: sched: pfifo_fast use alf_queue

[John Fastabend]

On 16-08-19 03:13 AM, Jesper Dangaard Brouer wrote:
> On Wed, 17 Aug 2016 12:38:10 -0700
> John Fastabend  wrote:
> 
>> This converts the pfifo_fast qdisc to use the alf_queue enqueue and
> ^
>> dequeue routines then sets the NOLOCK bit.
>>
>> This also removes the logic used to pick the next band to dequeue from
>> and instead just checks each alf_queue for packets from top priority
>^
>> to lowest. This might need to be a bit more clever but seems to work
>> for now.
> 
> You need to fix the description, as you are no longer using my
> alf_queue implementation but instead are using the skb_array/ptr_ring
> queue (by MST).
> 

Yep I forgot to change this even though iirc you may have had the same
comment in the last rev. Thanks! I'll get it fixed up this time.

.John

Re: [RFC PATCH 11/13] net: sched: pfifo_fast use alf_queue

[Jesper Dangaard Brouer]

On Wed, 17 Aug 2016 12:38:10 -0700
John Fastabend  wrote:

> This converts the pfifo_fast qdisc to use the alf_queue enqueue and
^
> dequeue routines then sets the NOLOCK bit.
> 
> This also removes the logic used to pick the next band to dequeue from
> and instead just checks each alf_queue for packets from top priority
   ^
> to lowest. This might need to be a bit more clever but seems to work
> for now.

You need to fix the description, as you are no longer using my
alf_queue implementation but instead are using the skb_array/ptr_ring
queue (by MST).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

[RFC PATCH 11/13] net: sched: pfifo_fast use alf_queue

[John Fastabend]

This converts the pfifo_fast qdisc to use the alf_queue enqueue and
dequeue routines then sets the NOLOCK bit.

This also removes the logic used to pick the next band to dequeue from
and instead just checks each alf_queue for packets from top priority
to lowest. This might need to be a bit more clever but seems to work
for now.

Signed-off-by: John Fastabend 
---
 net/sched/sch_generic.c |  133 +++
 1 file changed, 77 insertions(+), 56 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index f5b7254..c41a5dd 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -555,88 +556,79 @@ static const u8 prio2band[TC_PRIO_MAX + 1] = {
 
 /*
  * Private data for a pfifo_fast scheduler containing:
- * - queues for the three band
- * - bitmap indicating which of the bands contain skbs
+ * - rings for priority bands
  */
 struct pfifo_fast_priv {
-   u32 bitmap;
-   struct sk_buff_head q[PFIFO_FAST_BANDS];
+   struct skb_array q[PFIFO_FAST_BANDS];
 };
 
-/*
- * Convert a bitmap to the first band number where an skb is queued, where:
- * bitmap=0 means there are no skbs on any band.
- * bitmap=1 means there is an skb on band 0.
- * bitmap=7 means there are skbs on all 3 bands, etc.
- */
-static const int bitmap2band[] = {-1, 0, 1, 0, 2, 0, 1, 0};
-
-static inline struct sk_buff_head *band2list(struct pfifo_fast_priv *priv,
-int band)
+static inline struct skb_array *band2list(struct pfifo_fast_priv *priv,
+ int band)
 {
-   return priv->q + band;
+   return >q[band];
 }
 
 static int pfifo_fast_enqueue(struct sk_buff *skb, struct Qdisc *qdisc,
  struct sk_buff **to_free)
 {
-   if (skb_queue_len(>q) < qdisc_dev(qdisc)->tx_queue_len) {
-   int band = prio2band[skb->priority & TC_PRIO_MAX];
-   struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
-   struct sk_buff_head *list = band2list(priv, band);
-
-   priv->bitmap |= (1 << band);
-   qdisc->q.qlen++;
-   return __qdisc_enqueue_tail(skb, qdisc, list);
-   }
+   int band = prio2band[skb->priority & TC_PRIO_MAX];
+   struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
+   struct skb_array *q = band2list(priv, band);
+   int err;
 
-   return qdisc_drop(skb, qdisc, to_free);
+   err = skb_array_produce(q, skb);
+
+   if (unlikely(err))
+   return qdisc_drop_cpu(skb, qdisc, to_free);
+
+   qdisc_qstats_cpu_qlen_inc(qdisc);
+   qdisc_qstats_cpu_backlog_inc(qdisc, skb);
+   return NET_XMIT_SUCCESS;
 }
 
 static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc)
 {
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
-   int band = bitmap2band[priv->bitmap];
+   struct sk_buff *skb = NULL;
+   int band;
 
-   if (likely(band >= 0)) {
-   struct sk_buff_head *list = band2list(priv, band);
-   struct sk_buff *skb = __qdisc_dequeue_head(qdisc, list);
+   for (band = 0; band < PFIFO_FAST_BANDS && !skb; band++) {
+   struct skb_array *q = band2list(priv, band);
 
-   qdisc->q.qlen--;
-   if (skb_queue_empty(list))
-   priv->bitmap &= ~(1 << band);
+   if (__skb_array_empty(q))
+   continue;
 
-   return skb;
+   skb = skb_array_consume(q);
}
 
-   return NULL;
-}
-
-static struct sk_buff *pfifo_fast_peek(struct Qdisc *qdisc)
-{
-   struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
-   int band = bitmap2band[priv->bitmap];
-
-   if (band >= 0) {
-   struct sk_buff_head *list = band2list(priv, band);
-
-   return skb_peek(list);
+   if (likely(skb)) {
+   qdisc_qstats_cpu_backlog_dec(qdisc, skb);
+   qdisc_bstats_cpu_update(qdisc, skb);
+   qdisc_qstats_cpu_qlen_dec(qdisc);
}
 
-   return NULL;
+   return skb;
 }
 
 static void pfifo_fast_reset(struct Qdisc *qdisc)
 {
-   int prio;
+   int i, band;
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
 
-   for (prio = 0; prio < PFIFO_FAST_BANDS; prio++)
-   __qdisc_reset_queue(band2list(priv, prio));
+   for (band = 0; band < PFIFO_FAST_BANDS; band++) {
+   struct skb_array *q = band2list(priv, band);
+   struct sk_buff *skb;
 
-   priv->bitmap = 0;
-   qdisc->qstats.backlog = 0;
-   qdisc->q.qlen = 0;
+   while ((skb = skb_array_consume(q)) != NULL)
+   kfree_skb(skb);
+   }
+
+   for_each_possible_cpu(i) {
+   struct gnet_stats_queue *q =