From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:54:01
This series adds support for building lockless qdiscs. This is
the result of noticing the qdisc lock is a common hot-spot in
perf analysis of the Linux network stack, especially when testing
with high packet per second rates. However, nothing is free and
most qdiscs rely on the qdisc lock for their data structures so
each qdisc must be converted on a case by case basis. In this
series, to kick things off, we make pfifo_fast, mq, and mqprio
lockless. Follow up series can address additional qdiscs as needed.
For example sch_tbf might be useful. To allow this the lockless
design is an opt-in flag. In some future utopia we convert all
qdiscs and we get to drop this case analysis, but in order to
make progress we live in the real-world.
There are also a handful of optimizations I have behind this
series and a few code cleanups that I couldn't figure out how
to fit neatly into this series with out increasing the patch
count. Once this is in additional patches can address this. The
most notable is in skb_dequeue we can push the consumer lock
out a bit and consume multiple skbs off the skb_array in pfifo
fast per iteration. Ideally we could push arrays of packets at
drivers as well but we would need the infrastructure for this.
The other notable improvement is to do less locking in the
overrun cases where bad tx queue list and gso_skb are being
hit. Although, nice in theory in practice this is the error
case and I haven't found a benchmark where this matters yet.
For testing...
My first test case uses multiple containers (via cilium) where
multiple client containers use 'wrk' to benchmark connections with
a server container running lighttpd. Where lighttpd is configured
to use multiple threads, one per core. Additionally this test has
a proxy agent running so all traffic takes an extra hop through a
proxy container. In these cases each TCP packet traverses the egress
qdisc layer at least four times and the ingress qdisc layer an
additional four times. This makes for a good stress test IMO, perf
details below.
The other micro-benchmark I run is injecting packets directly into
qdisc layer using pktgen. This uses the benchmark script,
./pktgen_bench_xmit_mode_queue_xmit.sh
Benchmarks taken in two cases, "base" running latest net-next no
changes to qdisc layer and "qdisc" tests run with qdisc lockless
updates. Numbers reported in req/sec. All virtual 'veth' devices
run with pfifo_fast in the qdisc test case.
`wrk -t16 -c $conns -d30 "http://[$SERVER_IP4]:80"`
conns 16 32 64 1024
-----------------------------------------------
base: 18831 20201 21393 29151
qdisc: 19309 21063 23899 29265
notice in all cases we see performance improvement when running
with qdisc case.
Microbenchmarks using pktgen are as follows,
`pktgen_bench_xmit_mode_queue_xmit.sh -t 1 -i eth2 -c 20000000
base(mq): 2.1Mpps
base(pfifo_fast): 2.1Mpps
qdisc(mq): 2.6Mpps
qdisc(pfifo_fast): 2.6Mpps
notice numbers are the same for mq and pfifo_fast because only
testing a single thread here. In both tests we see a nice bump
in performance gain. The key with 'mq' is it is already per
txq ring so contention is minimal in the above cases. Qdiscs
such as tbf or htb which have more contention will likely show
larger gains when/if lockless versions are implemented.
Thanks to everyone who helped with this work especially Daniel
Borkmann, Eric Dumazet and Willem de Bruijn for discussing the
design and reviewing versions of the code.
Changes from the RFC: dropped a couple patches off the end,
fixed a bug with skb_queue_walk_safe not unlinking skb in all
cases, fixed a lockdep splat with pfifo_fast_destroy not calling
*_bh lock variant, addressed _most_ of Willem's comments, there
was a bug in the bulk locking (final patch) of the RFC series.
@Willem, I left out lockdep annotation for a follow on series
to add lockdep more completely, rather than just in code I
touched.
Comments and feedback welcome.
Thanks,
John
---
John Fastabend (14):
net: sched: cleanup qdisc_run and __qdisc_run semantics
net: sched: allow qdiscs to handle locking
net: sched: remove remaining uses for qdisc_qlen in xmit path
net: sched: provide per cpu qstat helpers
net: sched: a dflt qdisc may be used with per cpu stats
net: sched: explicit locking in gso_cpu fallback
net: sched: drop qdisc_reset from dev_graft_qdisc
net: sched: use skb list for skb_bad_tx
net: sched: check for frozen queue before skb_bad_txq check
net: sched: helpers to sum qlen and qlen for per cpu logic
net: sched: add support for TCQ_F_NOLOCK subqueues to sch_mq
net: sched: add support for TCQ_F_NOLOCK subqueues to sch_mqprio
net: skb_array: expose peek API
net: sched: pfifo_fast use skb_array
include/linux/skb_array.h | 5 +
include/net/gen_stats.h | 3
include/net/pkt_sched.h | 10 +
include/net/sch_generic.h | 79 +++++++-
net/core/dev.c | 31 +++
net/core/gen_stats.c | 9 +
net/sched/sch_api.c | 8 +
net/sched/sch_generic.c | 440 ++++++++++++++++++++++++++++++++-------------
net/sched/sch_mq.c | 34 +++
net/sched/sch_mqprio.c | 69 +++++--
10 files changed, 512 insertions(+), 176 deletions(-)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:54:20
Currently __qdisc_run calls qdisc_run_end() but does not call
qdisc_run_begin(). This makes it hard to track pairs of
qdisc_run_{begin,end} across function calls.
To simplify reading these code paths this patch moves begin/end calls
into qdisc_run().
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/pkt_sched.h | 4 +++-
net/core/dev.c | 5 +++--
net/sched/sch_generic.c | 2 --
3 files changed, 6 insertions(+), 5 deletions(-)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:54:42
This patch adds a flag for queueing disciplines to indicate the stack
does not need to use the qdisc lock to protect operations. This can
be used to build lockless scheduling algorithms and improving
performance.
The flag is checked in the tx path and the qdisc lock is only taken
if it is not set. For now use a conditional if statement. Later we
could be more aggressive if it proves worthwhile and use a static key
or wrap this in a likely().
Also the lockless case drops the TCQ_F_CAN_BYPASS logic. The reason
for this is synchronizing a qlen counter across threads proves to
cost more than doing the enqueue/dequeue operations when tested with
pktgen.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/sch_generic.h | 1 +
net/core/dev.c | 26 ++++++++++++++++++++++----
net/sched/sch_generic.c | 30 ++++++++++++++++++++----------
3 files changed, 43 insertions(+), 14 deletions(-)
@@ -4144,19 +4159,22 @@ static __latent_entropy void net_tx_action(struct softirq_action *h)while(head){structQdisc*q=head;-spinlock_t*root_lock;+spinlock_t*root_lock=NULL;head=head->next_sched;-root_lock=qdisc_lock(q);-spin_lock(root_lock);+if(!(q->flags&TCQ_F_NOLOCK)){+root_lock=qdisc_lock(q);+spin_lock(root_lock);+}/* We need to make sure head->next_sched is read*beforeclearing__QDISC_STATE_SCHED*/smp_mb__before_atomic();clear_bit(__QDISC_STATE_SCHED,&q->state);qdisc_run(q);-spin_unlock(root_lock);+if(root_lock)+spin_unlock(root_lock);}}}
@@ -174,7 +174,8 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,intret=NETDEV_TX_BUSY;/* And release qdisc */-spin_unlock(root_lock);+if(root_lock)+spin_unlock(root_lock);/* Note that we validate skb (GSO, checksum, ...) outside of locks */if(validate)
@@ -187,10 +188,13 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,HARD_TX_UNLOCK(dev,txq);}else{-spin_lock(root_lock);+if(root_lock)+spin_lock(root_lock);returnqdisc_qlen(q);}-spin_lock(root_lock);++if(root_lock)+spin_lock(root_lock);if(dev_xmit_complete(ret)){/* Driver sent out skb successfully or skb was consumed */
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:55:01
sch_direct_xmit() uses qdisc_qlen as a return value but all call sites
of the routine only check if it is zero or not. Simplify the logic so
that we don't need to return an actual queue length value.
This introduces a case now where sch_direct_xmit would have returned
a qlen of zero but now it returns true. However in this case all
call sites of sch_direct_xmit will implement a dequeue() and get
a null skb and abort. This trades tracking qlen in the hotpath for
an extra dequeue operation. Overall this seems to be good for
performance.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/pkt_sched.h | 6 +++---
net/sched/sch_generic.c | 28 +++++++++++++---------------
2 files changed, 16 insertions(+), 18 deletions(-)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:55:20
The per cpu qstats support was added with per cpu bstat support which
is currently used by the ingress qdisc. This patch adds a set of
helpers needed to make other qdiscs that use qstats per cpu as well.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/sch_generic.h | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:55:39
Enable dflt qdisc support for per cpu stats before this patch a dflt
qdisc was required to use the global statistics qstats and bstats.
This adds a static flags field to qdisc_ops that is propagated
into qdisc->flags in qdisc allocate call. This allows the allocation
block to completely allocate the qdisc object so we don't have
dangling allocations after qdisc init.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/sch_generic.h | 1 +
net/sched/sch_generic.c | 16 ++++++++++++++++
2 files changed, 17 insertions(+)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:55:58
This work is preparing the qdisc layer to support egress lockless
qdiscs. If we are running the egress qdisc lockless in the case we
overrun the netdev, for whatever reason, the netdev returns a busy
error code and the skb is parked on the gso_skb pointer. With many
cores all hitting this case at once its possible to have multiple
sk_buffs here so we turn gso_skb into a queue.
This should be the edge case and if we see this frequently then
the netdev/qdisc layer needs to back off.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/sch_generic.h | 20 ++++++-----
net/sched/sch_generic.c | 85 ++++++++++++++++++++++++++++++++++++++-------
2 files changed, 84 insertions(+), 21 deletions(-)
@@ -795,26 +795,30 @@ static inline struct sk_buff *qdisc_peek_head(struct Qdisc *sch)/* generic pseudo peek method for non-work-conserving qdisc */staticinlinestructsk_buff*qdisc_peek_dequeued(structQdisc*sch){+structsk_buff*skb=skb_peek(&sch->gso_skb);+/* we can reuse ->gso_skb because peek isn't called for root qdiscs */-if(!sch->gso_skb){-sch->gso_skb=sch->dequeue(sch);-if(sch->gso_skb){+if(!skb){+skb=sch->dequeue(sch);++if(skb){+__skb_queue_head(&sch->gso_skb,skb);/* it's still part of the queue */-qdisc_qstats_backlog_inc(sch,sch->gso_skb);+qdisc_qstats_backlog_inc(sch,skb);sch->q.qlen++;}}-returnsch->gso_skb;+returnskb;}/* use instead of qdisc->dequeue() for all qdiscs queried with ->peek() */staticinlinestructsk_buff*qdisc_dequeue_peeked(structQdisc*sch){-structsk_buff*skb=sch->gso_skb;+structsk_buff*skb=skb_peek(&sch->gso_skb);if(skb){-sch->gso_skb=NULL;+skb=__skb_dequeue(&sch->gso_skb);qdisc_qstats_backlog_dec(sch,skb);sch->q.qlen--;}else{
@@ -45,10 +45,9 @@*-ingressfilteringisalsoserializedviaqdiscrootlock*-updatestotreeandtreewalkingareonlydoneunderthertnlmutex.*/--staticinlineintdev_requeue_skb(structsk_buff*skb,structQdisc*q)+staticinlineint__dev_requeue_skb(structsk_buff*skb,structQdisc*q){-q->gso_skb=skb;+__skb_queue_head(&q->gso_skb,skb);q->qstats.requeues++;qdisc_qstats_backlog_inc(q,skb);q->q.qlen++;/* it's still part of the queue */
@@ -112,23 +135,50 @@ static void try_bulk_dequeue_skb_slow(struct Qdisc *q,staticstructsk_buff*dequeue_skb(structQdisc*q,bool*validate,int*packets){-structsk_buff*skb=q->gso_skb;conststructnetdev_queue*txq=q->dev_queue;+structsk_buff*skb;*packets=1;-if(unlikely(skb)){+if(unlikely(!skb_queue_empty(&q->gso_skb))){+spinlock_t*lock=NULL;++if(q->flags&TCQ_F_NOLOCK){+lock=qdisc_lock(q);+spin_lock(lock);+}++skb=skb_peek(&q->gso_skb);++/* skb may be null if another cpu pulls gso_skb off in between+*emptycheckandlock.+*/+if(!skb){+if(lock)+spin_unlock(lock);+gotovalidate;+}+/* skb in gso_skb were already validated */*validate=false;/* check the reason of requeuing without tx lock first */txq=skb_get_tx_queue(txq->dev,skb);if(!netif_xmit_frozen_or_stopped(txq)){-q->gso_skb=NULL;-qdisc_qstats_backlog_dec(q,skb);-q->q.qlen--;-}else+skb=__skb_dequeue(&q->gso_skb);+if(qdisc_is_percpu_stats(q)){+qdisc_qstats_cpu_backlog_dec(q,skb);+qdisc_qstats_cpu_qlen_dec(q);+}else{+qdisc_qstats_backlog_dec(q,skb);+q->q.qlen--;+}+}else{skb=NULL;+}+if(lock)+spin_unlock(lock);gototrace;}+validate:*validate=true;skb=q->skb_bad_txq;if(unlikely(skb)){
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:56:17
In qdisc_graft_qdisc a "new" qdisc is attached and the 'qdisc_destroy'
operation is called on the old qdisc. The destroy operation will wait
a rcu grace period and call qdisc_rcu_free(). At which point
gso_cpu_skb is free'd along with all stats so no need to zero stats
and gso_cpu_skb from the graft operation itself.
Further after dropping the qdisc locks we can not continue to call
qdisc_reset before waiting an rcu grace period so that the qdisc is
detached from all cpus. By removing the qdisc_reset() here we get
the correct property of waiting an rcu grace period and letting the
qdisc_destroy operation clean up the qdisc correctly.
Note, a refcnt greater than 1 would cause the destroy operation to
be aborted however if this ever happened the reference to the qdisc
would be lost and we would have a memory leak.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/sched/sch_generic.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
@@ -819,10 +819,6 @@ struct Qdisc *dev_graft_qdisc(struct netdev_queue *dev_queue,root_lock=qdisc_lock(oqdisc);spin_lock_bh(root_lock);-/* Prune old scheduler */-if(oqdisc&&refcount_read(&oqdisc->refcnt)<=1)-qdisc_reset(oqdisc);-/* ... and graft new one */if(qdisc==NULL)qdisc=&noop_qdisc;
@@ -997,20 +1002,25 @@ void dev_deactivate_many(struct list_head *head)&noop_qdisc);dev_watchdog_down(dev);-sync_needed|=!dev->dismantle;}/* Wait for outstanding qdisc-less dev_queue_xmit calls.*Thisisavoidedifalldevicesareindismantlephase:*Callerwillcallsynchronize_net()forus*/-if(sync_needed)-synchronize_net();+synchronize_net();/* Wait for outstanding qdisc_run calls. */-list_for_each_entry(dev,head,close_list)+list_for_each_entry(dev,head,close_list){while(some_qdisc_is_busy(dev))yield();+/* The new qdisc is assigned at this point so we can safely+*unwindstaleskblistsandqdiscstatistics+*/+netdev_for_each_tx_queue(dev,dev_qdisc_reset,NULL);+if(dev_ingress_queue(dev))+dev_qdisc_reset(dev,dev_ingress_queue(dev),NULL);+}}voiddev_deactivate(structnet_device*dev)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:56:36
Similar to how gso is handled use skb list for skb_bad_tx this is
required with lockless qdiscs because we may have multiple cores
attempting to push skbs into skb_bad_tx concurrently
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/sch_generic.h | 2 -
net/sched/sch_generic.c | 106 +++++++++++++++++++++++++++++++++++++--------
2 files changed, 87 insertions(+), 21 deletions(-)
@@ -45,6 +45,68 @@*-ingressfilteringisalsoserializedviaqdiscrootlock*-updatestotreeandtreewalkingareonlydoneunderthertnlmutex.*/++staticinlinestructsk_buff*__skb_dequeue_bad_txq(structQdisc*q)+{+conststructnetdev_queue*txq=q->dev_queue;+spinlock_t*lock=NULL;+structsk_buff*skb;++if(q->flags&TCQ_F_NOLOCK){+lock=qdisc_lock(q);+spin_lock(lock);+}++skb=skb_peek(&q->skb_bad_txq);+if(skb){+/* check the reason of requeuing without tx lock first */+txq=skb_get_tx_queue(txq->dev,skb);+if(!netif_xmit_frozen_or_stopped(txq)){+skb=__skb_dequeue(&q->skb_bad_txq);+if(qdisc_is_percpu_stats(q)){+qdisc_qstats_cpu_backlog_dec(q,skb);+qdisc_qstats_cpu_qlen_dec(q);+}else{+qdisc_qstats_backlog_dec(q,skb);+q->q.qlen--;+}+}else{+skb=NULL;+}+}++if(lock)+spin_unlock(lock);++returnskb;+}++staticinlinestructsk_buff*qdisc_dequeue_skb_bad_txq(structQdisc*q)+{+structsk_buff*skb=skb_peek(&q->skb_bad_txq);++if(unlikely(skb))+skb=__skb_dequeue_bad_txq(q);++returnskb;+}++staticinlinevoidqdisc_enqueue_skb_bad_txq(structQdisc*q,+structsk_buff*skb)+{+spinlock_t*lock=NULL;++if(q->flags&TCQ_F_NOLOCK){+lock=qdisc_lock(q);+spin_lock(lock);+}++__skb_queue_tail(&q->skb_bad_txq,skb);++if(lock)+spin_unlock(lock);+}+staticinlineint__dev_requeue_skb(structsk_buff*skb,structQdisc*q){__skb_queue_head(&q->gso_skb,skb);
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:56:55
I can not think of any reason to pull the bad txq skb off the qdisc if
the txq we plan to send this on is still frozen. So check for frozen
queue first and abort before dequeuing either skb_bad_txq skb or
normal qdisc dequeue() skb.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/sched/sch_generic.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:57:14
Add qdisc qlen helper routines for lockless qdiscs to use.
The qdisc qlen is no longer used in the hotpath but it is reported
via stats query on the qdisc so it still needs to be tracked. This
adds the per cpu operations needed along with a helper to return
the summation of per cpu stats.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/sch_generic.h | 20 ++++++++++++++++++++
net/sched/sch_api.c | 3 ++-
2 files changed, 22 insertions(+), 1 deletion(-)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:57:33
The sch_mq qdisc creates a sub-qdisc per tx queue which are then
called independently for enqueue and dequeue operations. However
statistics are aggregated and pushed up to the "master" qdisc.
This patch adds support for any of the sub-qdiscs to be per cpu
statistic qdiscs. To handle this case add a check when calculating
stats and aggregate the per cpu stats if needed.
Also exports __gnet_stats_copy_queue() to use as a helper function.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/gen_stats.h | 3 +++
net/core/gen_stats.c | 9 +++++----
net/sched/sch_mq.c | 25 ++++++++++++++++++-------
3 files changed, 26 insertions(+), 11 deletions(-)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:57:54
The sch_mqprio qdisc creates a sub-qdisc per tx queue which are then
called independently for enqueue and dequeue operations. However
statistics are aggregated and pushed up to the "master" qdisc.
This patch adds support for any of the sub-qdiscs to be per cpu
statistic qdiscs. To handle this case add a check when calculating
stats and aggregate the per cpu stats if needed.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/sched/sch_mq.c | 35 +++++++++++++++---------
net/sched/sch_mqprio.c | 69 +++++++++++++++++++++++++++++++++---------------
2 files changed, 69 insertions(+), 35 deletions(-)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:58:13
This adds a peek routine to skb_array.h for use with qdisc.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/linux/skb_array.h | 5 +++++
1 file changed, 5 insertions(+)
From: John Fastabend <john.fastabend@gmail.com> Date: 2017-12-07 17:58:33
This converts the pfifo_fast qdisc to use the skb_array data structure
and set the lockless qdisc bit. pfifo_fast is the first qdisc to support
the lockless bit that can be a child of a qdisc requiring locking. So
we add logic to clear the lock bit on initialization in these cases when
the qdisc graft operation occurs.
This also removes the logic used to pick the next band to dequeue from
and instead just checks a per priority array 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.fastabend@gmail.com>
---
net/sched/sch_api.c | 5 ++
net/sched/sch_generic.c | 140 +++++++++++++++++++++++++++++------------------
2 files changed, 92 insertions(+), 53 deletions(-)
@@ -955,6 +955,11 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,}else{conststructQdisc_class_ops*cops=parent->ops->cl_ops;+/* Only support running class lockless if parent is lockless */+if(new&&(new->flags&TCQ_F_NOLOCK)&&+parent&&!(parent->flags&TCQ_F_NOLOCK))+new->flags&=~TCQ_F_NOLOCK;+err=-EOPNOTSUPP;if(cops&&cops->graft){unsignedlongcl=cops->find(parent,classid);
@@ -682,17 +683,48 @@ static int pfifo_fast_dump(struct Qdisc *qdisc, struct sk_buff *skb)staticintpfifo_fast_init(structQdisc*qdisc,structnlattr*opt){-intprio;+unsignedintqlen=qdisc_dev(qdisc)->tx_queue_len;structpfifo_fast_priv*priv=qdisc_priv(qdisc);+intprio;++/* guard against zero length rings */+if(!qlen)+return-EINVAL;-for(prio=0;prio<PFIFO_FAST_BANDS;prio++)-qdisc_skb_head_init(band2list(priv,prio));+for(prio=0;prio<PFIFO_FAST_BANDS;prio++){+structskb_array*q=band2list(priv,prio);+interr;++err=skb_array_init(q,qlen,GFP_KERNEL);+if(err)+return-ENOMEM;+}/* Can by-pass the queue discipline */qdisc->flags|=TCQ_F_CAN_BYPASS;return0;}+staticvoidpfifo_fast_destroy(structQdisc*sch)+{+structpfifo_fast_priv*priv=qdisc_priv(sch);+intprio;++for(prio=0;prio<PFIFO_FAST_BANDS;prio++){+structskb_array*q=band2list(priv,prio);++/* NULL ring is possible if destroy path is due to a failed+*skb_array_init()inpfifo_fast_init()case.+*/+if(!&q->ring.queue)+continue;+/* Destroy ring but no need to kfree_skb because a call to+*pfifo_fast_reset()hasalreadydonethatwork.+*/+ptr_ring_cleanup(&q->ring,NULL);+}+}+structQdisc_opspfifo_fast_ops__read_mostly={.id="pfifo_fast",.priv_size=sizeof(structpfifo_fast_priv),
From: Cong Wang <hidden> Date: 2017-12-12 22:07:11
On Thu, Dec 7, 2017 at 9:58 AM, John Fastabend [off-list ref] wrote:
This converts the pfifo_fast qdisc to use the skb_array data structure
and set the lockless qdisc bit. pfifo_fast is the first qdisc to support
the lockless bit that can be a child of a qdisc requiring locking. So
we add logic to clear the lock bit on initialization in these cases when
the qdisc graft operation occurs.
This also removes the logic used to pick the next band to dequeue from
and instead just checks a per priority array for packets from top priority
to lowest. This might need to be a bit more clever but seems to work
for now.
A very dumb question:
Why do we call it lockless? With skb_array you just shift the per qdisc
spinlock down to each pfifo band, skb_array still uses a spinlock underneath.
What am I missing here?