netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: Xingjun Liu <yterrencelau@gmail.com>, john.r.fastabend@intel.com
Cc: netdev@vger.kernel.org
Subject: Re: More info about lockless qdisc patch
Date: Sat, 7 Jan 2017 12:11:27 -0800	[thread overview]
Message-ID: <58714B6F.5000301@gmail.com> (raw)
In-Reply-To: <D1B824B6-91C1-4049-8231-048FD900D160@gmail.com>

On 16-12-30 04:48 AM, Xingjun Liu wrote:
> Hi John,
> 
> I note you have a patch request last august about "support lockless qdisc”,
> 
> see: https://www.mail-archive.com/netdev@vger.kernel.org/msg124489.html

I have another set of patches that fixes a few bugs from the above.

> 
> We take an interest in it. Do you have any info/plan on this patch?
> 

The current state of the patches (the latest I have on my private tree) is
currently
I an issue with re-starting the driver when multiple cores do a dequeue but the
driver overruns the hardware descriptor ring so we push back into the qdisc. When
this happens the patches push the skb onto the gso_skb per cpu list.

Now the problem is how do we wake the correct gso_skb per cpu list up? The driver
will restart the qdisc at some future point but not necessarily from the same
cpu that
the per cpu enqueue happened on. So I tried to fix this by removing the driver
restart
logic and having the qdisc layer poll the driver. This sort of/kind of works
except its not
a very satisfying solution. Tom noted at one point with BQL the above case
should not
happen very much but it is still possible unfortunately under certain
conditions. And the
corner cases get ugly.

I've been kicking around another idea lately. To get rid of the gso_skb entirely
which
would certainly clean up the code and get rid of one of the uglier pieces of
qdisc struct
in my opinion. To make this work I want to add a tx_prep() ndo call the qdisc
could call
into the driver with to reserve "slots" in the driver. Once we have some # of
slots reserved
we can dequeue from the qdisc ring and send to driver "knowing" that the driver has
agreed to consume the packets. The trick here is sk_buffs do not map 1:1 with
descriptors
in the hardware always so a bit of driver magic needs to occur to get
calculations correct.

I'll try to rebase the current set of patches I have on net-next and post it
just so its
available. I can probably get to the above in a month or so. Maybe have something by
March.

Thanks,
John

> Thanks
> Xingjun
> 

      reply	other threads:[~2017-01-07 20:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-30 12:48 More info about lockless qdisc patch Xingjun Liu
2017-01-07 20:11 ` John Fastabend [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=58714B6F.5000301@gmail.com \
    --to=john.fastabend@gmail.com \
    --cc=john.r.fastabend@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=yterrencelau@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).