All of lore.kernel.org
 help / color / mirror / Atom feed
From: Magnus Karlsson <magnus.karlsson@gmail.com>
To: jakub.kicinski@netronome.com
Cc: "Karlsson, Magnus" <magnus.karlsson@intel.com>,
	"Björn Töpel" <bjorn.topel@intel.com>,
	ast@kernel.org, "Daniel Borkmann" <daniel@iogearbox.net>,
	"Network Development" <netdev@vger.kernel.org>,
	"Jesper Dangaard Brouer" <brouer@redhat.com>
Subject: Re: [PATCH bpf-next v2 0/5] xsk: fix bug when trying to use both copy and zero-copy mode
Date: Tue, 2 Oct 2018 14:49:13 +0200	[thread overview]
Message-ID: <CAJ8uoz2Bwrj2iMBn-DA6km=Zac45XigegdAOQQ9rWQkJMz8P=Q@mail.gmail.com> (raw)
In-Reply-To: <20181001133146.1b8f3810@cakuba.netronome.com>

On Mon, Oct 1, 2018 at 10:34 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Mon,  1 Oct 2018 14:51:32 +0200, Magnus Karlsson wrote:
> > Jakub, please take a look at your patches. The last one I had to
> > change slightly to make it fit with the new interface
> > xdp_get_umem_from_qid(). An added bonus with this function is that we,
> > in the future, can also use it from the driver to get a umem, thus
> > simplifying driver implementations (and later remove the umem from the
> > NDO completely). Björn will mail patches, at a later point in time,
> > using this in the i40e and ixgbe drivers, that removes a good chunk of
> > code from the ZC implementations.
>
> Nice, drivers which don't follow the prepare/commit model of handling
> reconfigurations will benefit!
>
> > I also made your code aware of Tx queues. If we create a socket that
> > only has a Tx queue, then the queue id will refer to a Tx queue id
> > only and could be larger than the available amount of Rx queues.
> > Please take a look at it.
>
> The semantics of Tx queue id are slightly unclear.  To me XDP is
> associated with Rx, so the qid in driver context can only refer to
> Rx queue and its associated XDP Tx queue.  It does not mean the Tx
> queue stack uses, like it does for copy fallback.  If one doesn't have
> a Rx queue $id, there will be no associated XDP Tx queue $id (in all
> drivers but Intel, and virtio, which use per-CPU Tx queues making TX
> queue even more meaningless).
>
> Its to be seen how others implement AF_XDP.  My general feeling is
> that we should only talk about Rx queues in context of driver XDP.

This is the way I see it. From an uapi point of view we can create a
socket that can only do Rx, only Tx or both. We then bind this socket
to a specific queue id on a device. If a packet is received on this
queue id it is sent (by the default xdpsock sample program) to the
socket. If a packet is sent on this socket it goes out on this same
queue id. If you have not registered an Rx ring (in user space) for
this socket, you cannot receive anything on this socket. And
conversely, if you have no Tx ring, you will not be able to send
anything.

But if we take a look at this from the driver perspective and the NDO
XDP_SETUP_XSK_UMEM, today it does not know anything about if Rx and Tx
rings have been setup in the socket. It will always initialize the HW
Rx and Tx queues of the supplied queue id. So with today's NDO
interface you will always get a Rx/Tx queue pair. In order to realize
the uapi above in an efficient manner and to support devices with more
Tx queues than Rx, we need to change the NDO.

Just as a note, in the applications I am used to work on, radio base
stations and other telecom apps, it is the common case to have many
more Tx queues than Rx queues just to be able to use scheduling,
shaping and other QoS features that are important on egress in those
systems. Therefore the interest in supporting Tx only queues. But
maybe this is just a weird case, do not know.

  reply	other threads:[~2018-10-02 19:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-01 12:51 [PATCH bpf-next v2 0/5] xsk: fix bug when trying to use both copy and zero-copy mode Magnus Karlsson
2018-10-01 12:51 ` [PATCH bpf-next v2 1/5] net: add umem reference in netdev{_rx}_queue Magnus Karlsson
2018-10-01 12:51 ` [PATCH bpf-next v2 2/5] xsk: fix bug when trying to use both copy and zero-copy on one queue id Magnus Karlsson
2018-10-01 12:51 ` [PATCH bpf-next v2 3/5] ethtool: rename local variable max -> curr Magnus Karlsson
2018-10-01 12:51 ` [PATCH bpf-next v2 4/5] ethtool: don't allow disabling queues with umem installed Magnus Karlsson
2018-10-01 12:51 ` [PATCH bpf-next v2 5/5] xsk: simplify xdp_clear_umem_at_qid implementation Magnus Karlsson
2018-10-01 20:31 ` [PATCH bpf-next v2 0/5] xsk: fix bug when trying to use both copy and zero-copy mode Jakub Kicinski
2018-10-02 12:49   ` Magnus Karlsson [this message]
2018-10-02 16:58     ` Jakub Kicinski
2018-10-05  7:35 ` Daniel Borkmann

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='CAJ8uoz2Bwrj2iMBn-DA6km=Zac45XigegdAOQQ9rWQkJMz8P=Q@mail.gmail.com' \
    --to=magnus.karlsson@gmail.com \
    --cc=ast@kernel.org \
    --cc=bjorn.topel@intel.com \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.