On 16-07-14 08:11 AM, Jesper Dangaard Brouer wrote:
> On Wed, 13 Jul 2016 23:23:12 -0700
> John Fastabend <john.fastab...@gmail.com> wrote:
>
>> This converts the pfifo_fast qdisc to use the alf_queue enqueue and
>> dequeue routines then sets the NOLOCK bit.
>
> Should have said skb_array, not alf_queue ;-)
I think I'll get numbers for the alf_queue to so we can compare them
as well. On the todo list for tomorrow.
>
> > 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 <john.r.fastab...@intel.com>
>> ---
>> net/sched/sch_generic.c | 131
>> +++++++++++++++++++++++++++--------------------
>> 1 file changed, 75 insertions(+), 56 deletions(-)
>>
>> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
>> index 7dcd066..2ac3eb9 100644
>> --- a/net/sched/sch_generic.c
>> +++ b/net/sched/sch_generic.c
>> @@ -26,6 +26,7 @@
>> #include <linux/list.h>
>> #include <linux/slab.h>
>> #include <linux/if_vlan.h>
>> +#include <linux/skb_array.h>
>> #include <net/sch_generic.h>
>> #include <net/pkt_sched.h>
>> #include <net/dst.h>
>> @@ -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 &priv->q[band];
>> }
>>
>> static int pfifo_fast_enqueue(struct sk_buff *skb, struct Qdisc *qdisc,
>> struct sk_buff **to_free)
>> {
>> - if (skb_queue_len(&qdisc->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_bh(q, skb);
>
> Do you need the _bh variant here? (Doesn't the qdisc run with BH disabled?)
>
>
Yep its inside rcu_read_lock_bh().
