On 03/21/2018 12:44 PM, Jakob Unterwurzacher wrote: > On 21.03.18 19:43, John Fastabend wrote: >> Thats my theory at least. Are you able to test a patch if I generate >> one to fix this? > > Yes, no problem.
Can you try this, diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index d4907b5..1e596bd 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -30,6 +30,7 @@ struct qdisc_rate_table { enum qdisc_state_t { __QDISC_STATE_SCHED, __QDISC_STATE_DEACTIVATED, + __QDISC_STATE_RUNNING, }; struct qdisc_size_table { diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index 190570f..cf7c37d 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -377,20 +377,26 @@ static inline bool qdisc_restart(struct Qdisc *q, int *packets) struct netdev_queue *txq; struct net_device *dev; struct sk_buff *skb; - bool validate; + bool more, validate; /* Dequeue packet */ + if (test_and_set_bit(__QDISC_STATE_RUNNING, &q->state)) + return false; + skb = dequeue_skb(q, &validate, packets); - if (unlikely(!skb)) + if (unlikely(!skb)) { + clear_bit(__QDISC_STATE_RUNNING, &q->state); return false; + } if (!(q->flags & TCQ_F_NOLOCK)) root_lock = qdisc_lock(q); dev = qdisc_dev(q); txq = skb_get_tx_queue(dev, skb); - - return sch_direct_xmit(skb, q, dev, txq, root_lock, validate); + more = sch_direct_xmit(skb, q, dev, txq, root_lock, validate); + clear_bit(__QDISC_STATE_RUNNING, &q->state); + return more; } > > I just tested with the flag change you suggested (see below, I had to keep > TCQ_F_CPUSTATS to prevent a crash) and I have NOT seen OOO so far. > Right because the code expects per cpu stats if the CPUSTATS flag is removed it will crash. > Thanks, > Jakob > > > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > index 190570f21b20..51b68ef4977b 100644 > --- a/net/sched/sch_generic.c > +++ b/net/sched/sch_generic.c > @@ -792,7 +792,7 @@ struct Qdisc_ops pfifo_fast_ops __read_mostly = { > .dump = pfifo_fast_dump, > .change_tx_queue_len = pfifo_fast_change_tx_queue_len, > .owner = THIS_MODULE, > - .static_flags = TCQ_F_NOLOCK | TCQ_F_CPUSTATS, > + .static_flags = TCQ_F_CPUSTATS, > }; > EXPORT_SYMBOL(pfifo_fast_ops);