implement io_uring notification (ubuf_info) stacking

We'll need to associate additional callbacks with ubuf_info, introduce a structure holding ubuf_info callbacks. Apart from a more smarter io_uring notification management introduced in next patches, it can be used to generalise msg_zerocopy_put_abort() and also store ->sg_from_iter, which is currently passed in struct msghdr.

Signed-off-by Pavel Begunkov <[email protected]>

---
 drivers/net/tap.c      |  2 +-
 drivers/net/tun.c      |  2 +-
 drivers/vhost/net.c    |  8 ++++++--
 include/linux/skbuff.h | 19 +++++++++++--------
 io_uring/notif.c       |  8 ++++++--
 net/core/skbuff.c      | 17 +++++++++++------
 6 files changed, 36 insertions(+), 20 deletions(-)

We'll need to associate additional callbacks with ubuf_info, introduce a structure holding ubuf_info callbacks. Apart from a more smarter io_uring notification management introduced in next patches, it can be used to generalise msg_zerocopy_put_abort() and also store ->sg_from_iter, which is currently passed in struct msghdr.

Quoted 5 lines · Pavel Begunkov

We'll need to associate additional callbacks with ubuf_info, introduce a structure holding ubuf_info callbacks. Apart from a more smarter io_uring notification management introduced in next patches, it can be used to generalise msg_zerocopy_put_abort() and also store ->sg_from_iter, which is currently passed in struct msghdr.

This adds an extra indirection for all other ubuf implementations. Can that be avoided?

Quoted 7 lines · Willem de Bruijn

Quoted 5 lines · Pavel Begunkov

We'll need to associate additional callbacks with ubuf_info, introduce a structure holding ubuf_info callbacks. Apart from a more smarter io_uring notification management introduced in next patches, it can be used to generalise msg_zerocopy_put_abort() and also store ->sg_from_iter, which is currently passed in struct msghdr.

Quoted 2 lines

This adds an extra indirection for all other ubuf implementations. Can that be avoided?

It could be fitted directly into ubuf_info, but that doesn't feel right. It should be hot, so does it even matter?

On the bright side,
with the patch I'll also ->sg_from_iter from msghdr into it, so it doesn't have to be in the generic path.

I think it's the right approach, but if you have a strong opinion I can fit it as a new field in ubuf_info.

Quoted 8 lines · Pavel Begunkov

Quoted 7 lines · Willem de Bruijn

Quoted 5 lines · Pavel Begunkov

We'll need to associate additional callbacks with ubuf_info, introduce a structure holding ubuf_info callbacks. Apart from a more smarter io_uring notification management introduced in next patches, it can be used to generalise msg_zerocopy_put_abort() and also store ->sg_from_iter, which is currently passed in struct msghdr.

Quoted 5 lines

Quoted 2 lines

This adds an extra indirection for all other ubuf implementations. Can that be avoided?

It could be fitted directly into ubuf_info, but that doesn't feel right. It should be hot, so does it even matter?

That depends on the workload (working set size)?

Quoted 3 lines

Quoted 1 line

On the bright side,

with the patch I'll also ->sg_from_iter from msghdr into it, so it doesn't have to be in the generic path.

I don't follow this: is this suggested future work?

Quoted 2 lines

I think it's the right approach, but if you have a strong opinion I can fit it as a new field in ubuf_info.

If there is a significant cost, I suppose we could use INDIRECT_CALL or go one step further and demultiplex based on the new ops

     if (uarg->ops == &msg_zerocopy_ubuf_ops)
         msg_zerocopy_callback(..);

Quoted 15 lines · Willem de Bruijn

Quoted 8 lines · Pavel Begunkov

Quoted 7 lines · Willem de Bruijn

Quoted 5 lines · Pavel Begunkov

We'll need to associate additional callbacks with ubuf_info, introduce a structure holding ubuf_info callbacks. Apart from a more smarter io_uring notification management introduced in next patches, it can be used to generalise msg_zerocopy_put_abort() and also store ->sg_from_iter, which is currently passed in struct msghdr.

Quoted 5 lines

Quoted 2 lines

This adds an extra indirection for all other ubuf implementations. Can that be avoided?

It could be fitted directly into ubuf_info, but that doesn't feel right. It should be hot, so does it even matter?

Quoted 6 lines

That depends on the workload (working set size)?

Quoted 3 lines

Quoted 1 line

On the bright side,

with the patch I'll also ->sg_from_iter from msghdr into it, so it doesn't have to be in the generic path.

I don't follow this: is this suggested future work?

Right, a small change I will add later. Without ops though having 3 callback fields in uargs would be out of hands.

Quoted 7 lines

Quoted 2 lines

I think it's the right approach, but if you have a strong opinion I can fit it as a new field in ubuf_info.

If there is a significant cost, I suppose we could use INDIRECT_CALL or go one step further and demultiplex based on the new ops

Quoted 2 lines

     if (uarg->ops == &msg_zerocopy_ubuf_ops)
         msg_zerocopy_callback(..);

Let me note that the patch doesn't change the number of indirect calls but only adds one extra deref to get the callback, i.e. uarg->ops->callback() instead of uarg->callback().

Your snippet
goes an extra mile and removes the indirect call.

Can I take it as that you're fine with the direction of the patch? Or do you want me to change anything?

On the bright side,
with the patch I'll also ->sg_from_iter from msghdr into it, so it doesn't have to be in the generic path.

Quoted 3 lines · Pavel Begunkov

On the bright side,
with the patch I'll also ->sg_from_iter from msghdr into it, so it doesn't have to be in the generic path.

So, what's old is new again? That's where it started:

https://lore.kernel.org/netdev/20220628225204.GA27554@u2004-local/

At the moment an skb can only have one ubuf_info associated with it, which might be a performance problem for zerocopy sends in cases like TCP via io_uring. Add a callback for assigning ubuf_info to skb, this way we will implement smarter assignment later like linking ubuf_info together.

Note, it's an optional callback, which should be compatible with skb_zcopy_set(), that's because the net stack might potentially decide to clone an skb and take another reference to ubuf_info whenever it wishes. Also, a correct implementation should always be able to bind to an skb without prior ubuf_info, otherwise we could end up in a situation when the send would not be able to progress.

Signed-off-by Pavel Begunkov <[email protected]>

---
 include/linux/skbuff.h |  2 ++
 net/core/skbuff.c      | 20 ++++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

The network stack allows only one ubuf_info per skb, and unlike MSG_ZEROCOPY, each io_uring zerocopy send will carry a separate ubuf_info. That means that send requests can't reuse a previosly allocated skb and need to get one more or more of new ones. That's fine for large sends, but otherwise it would spam the stack with lots of skbs carrying just a little data each.

Quoted 6 lines · Pavel Begunkov

The network stack allows only one ubuf_info per skb, and unlike MSG_ZEROCOPY, each io_uring zerocopy send will carry a separate ubuf_info. That means that send requests can't reuse a previosly allocated skb and need to get one more or more of new ones. That's fine for large sends, but otherwise it would spam the stack with lots of skbs carrying just a little data each.

Can you give a little context why each send request has to be a separate ubuf_info?

This patch series aims to make that model more efficient. Would it be possible to just change the model instead? I assume you tried that and it proved unworkable, but is it easy to explain what the fundamental blocker is?

MSG_ZEROCOPY uses uarg->len to identify multiple consecutive send operations that can be notified at once.

Quoted 8 lines · Willem de Bruijn

Quoted 6 lines · Pavel Begunkov

The network stack allows only one ubuf_info per skb, and unlike MSG_ZEROCOPY, each io_uring zerocopy send will carry a separate ubuf_info. That means that send requests can't reuse a previosly allocated skb and need to get one more or more of new ones. That's fine for large sends, but otherwise it would spam the stack with lots of skbs carrying just a little data each.

Quoted 7 lines

Can you give a little context why each send request has to be a separate ubuf_info?

This patch series aims to make that model more efficient. Would it be possible to just change the model instead? I assume you tried that and it proved unworkable, but is it easy to explain what the fundamental blocker is?

The uapi is so that you get a buffer completion (analogous to what you get with recv(MSG_ERRQUEUE)) for each send request. With that, for skb to serve multiple send requests it'd need to store a list of completions in some way.

One could try to track sockets, have one "active" ubuf_info per socket which all sends would use, and then eventually flush the active ubuf so it can post completions and create a new one.

but io_uring
wouldn't know when it needs to "flush", whenever in the net stack it happens naturally when it pushes skbs from the queue. Not to mention that socket tracking has its own complications.

As for uapi, in early versions of io_uring's SEND_ZC, ubuf_info and requests weren't entangled, roughly speaking, the user could choose that this request should use this ubuf_info (I can elaborate if interesting). It wasn't too complex, but all feedback was pointing that it's much easier to use hot it is now, and honestly it does buy with simplicity.

I'm not sure what a different model would give. We wouldn't win in efficiency comparing to this patch, I can go into details how there are no extra atomics/locks/kmalloc/etc., the only bit is waking up waiting tasks, but that still would need to happen. I can even optimise / ammortise ubuf refcounting if that would matter.

Quoted 3 lines

MSG_ZEROCOPY uses uarg->len to identify multiple consecutive send operations that can be notified at once.

Signature · 1 line

Pavel Begunkov

Quoted 17 lines · Pavel Begunkov

Quoted 8 lines · Willem de Bruijn

Quoted 6 lines · Pavel Begunkov

The network stack allows only one ubuf_info per skb, and unlike MSG_ZEROCOPY, each io_uring zerocopy send will carry a separate ubuf_info. That means that send requests can't reuse a previosly allocated skb and need to get one more or more of new ones. That's fine for large sends, but otherwise it would spam the stack with lots of skbs carrying just a little data each.

Quoted 7 lines

Can you give a little context why each send request has to be a separate ubuf_info?

This patch series aims to make that model more efficient. Would it be possible to just change the model instead? I assume you tried that and it proved unworkable, but is it easy to explain what the fundamental blocker is?

Quoted 4 lines

The uapi is so that you get a buffer completion (analogous to what you get with recv(MSG_ERRQUEUE)) for each send request. With that, for skb to serve multiple send requests it'd need to store a list of completions in some way.

I probably don't know the io_uring implementation well enough yet, so take this with a huge grain of salt.

MSG_ZEROCOPY can generate completions for multiple send calls from a single uarg, by virtue of completions being incrementing IDs.

Is there a fundamental reason why io_uring needs a 1:1 mapping between request slots in the API and uarg in the datapath?

Or differently, is
there no trivial way to associate a range of completions with a single uarg?

Quoted 3 lines

One could try to track sockets, have one "active" ubuf_info per socket which all sends would use, and then eventually flush the active ubuf so it can post completions and create a new one.

This is basically what MSG_ZEROCOPY does for TCP. It signals POLLERR as soon as one completion arrives. Then when a process gets around to calling MSG_ERRQUEUE, it returns the range of completions that have arrived in the meantime. A process can thus decide to postpone completion handling to increase batching.

Quoted 11 lines

but io_uring
wouldn't know when it needs to "flush", whenever in the net stack it happens naturally when it pushes skbs from the queue. Not to mention that socket tracking has its own complications.

As for uapi, in early versions of io_uring's SEND_ZC, ubuf_info and requests weren't entangled, roughly speaking, the user could choose that this request should use this ubuf_info (I can elaborate if interesting). It wasn't too complex, but all feedback was pointing that it's much easier to use hot it is now, and honestly it does buy with simplicity.

I see. I suppose that answers the 1:1 mapping the ABI question I asked above. I should reread that patch.

Quoted 6 lines

I'm not sure what a different model would give. We wouldn't win in efficiency comparing to this patch, I can go into details how there are no extra atomics/locks/kmalloc/etc., the only bit is waking up waiting tasks, but that still would need to happen. I can even optimise / ammortise ubuf refcounting if that would matter.

Slight aside: we know that MSG_ZEROCOPY is quite inefficient for small sends. Very rough rule of thumb is you need around 16KB or larger sends for it to outperform regular copy. Part of that is the memory pinning. The other part is the notification handling. MSG_ERRQUEUE is expensive. I hope that io_uring cannot just match, but improve on MSG_ZEROCOPY, especially for smaller packets.

Quoted 7 lines

Slight aside: we know that MSG_ZEROCOPY is quite inefficient for small sends. Very rough rule of thumb is you need around 16KB or larger sends for it to outperform regular copy. Part of that is the memory pinning. The other part is the notification handling. MSG_ERRQUEUE is expensive. I hope that io_uring cannot just match, but improve on MSG_ZEROCOPY, especially for smaller packets.

I has some numbers left from this patchset benchmarking. Not too well suited to answer your question, but still gives an idea. Just a benchmark, single buffer, 100g broadcom NIC IIRC. All is io_uring based, -z<bool> switches copy vs zerocopy. Zero copy uses registered buffers, so no page pinning and page table traversal at runtime. 10s per run is not ideal, but was matching longer runs.

# 1200 bytes
./send-zerocopy -4 tcp -D <ip> -t 10 -n 1 -l0 -b1 -d -s1200 -z0 packets=15004160 (MB=17170), rps=1470996 (MB/s=1683) ./send-zerocopy -4 tcp -D <ip> -t 10 -n 1 -l0 -b1 -d -s1200 -z1 packets=10440224 (MB=11947), rps=1023551 (MB/s=1171)

# 4000 bytes
./send-zerocopy -4 tcp -D <ip> -t 10 -n 1 -l0 -b1 -d -s4000 -z0 packets=11742688 (MB=44794), rps=1151243 (MB/s=4391) ./send-zerocopy -4 tcp -D <ip> -t 10 -n 1 -l0 -b1 -d -s4000 -z1 packets=14144048 (MB=53955), rps=1386671 (MB/s=5289)

# 8000 bytes
./send-zerocopy -4 tcp -D <ip> -t 10 -n 1 -l0 -b1 -d -s8000 -z0 packets=6868976 (MB=52406), rps=673429 (MB/s=5137) ./send-zerocopy -4 tcp -D <ip> -t 10 -n 1 -l0 -b1 -d -s8000 -z1 packets=10800784 (MB=82403), rps=1058900 (MB/s=8078)

io_uring allocates a ubuf_info per zerocopy send request, it's convenient for the userspace but with how things are it means that every time the TCP stack has to allocate a new skb instead of amending into a previous one. Unless sends are big enough, there will be lots of small skbs straining the stack and dipping performance.

The patchset implements notification, i.e. an io_uring's ubuf_info extension, stacking. It tries to link ubuf_info's into a list, and the entire link will be put down together once all references are gone.

io_uring allocates a ubuf_info per zerocopy send request, it's convenient for the userspace but with how things are it means that every time the TCP stack has to allocate a new skb instead of amending into a previous one. Unless sends are big enough, there will be lots of small skbs straining the stack and dipping performance.

The patchset implements notification, i.e. an io_uring's ubuf_info extension, stacking. It tries to link ubuf_info's into a list, and the entire link will be put down together once all references are gone.

Testing with liburing/examples/send-zerocopy and another custom made tool, with 4K bytes per send it improves performance ~6 times and levels it with MSG_ZEROCOPY. Without the patchset it requires much larger sends to utilise all potential.

bytes  | before | after (Kqps)  
100    | 283    | 936
1200   | 195    | 1023
4000   | 193    | 1386
8000   | 154    | 1058

Pavel Begunkov (6):
net: extend ubuf_info callback to ops structure
net: add callback for setting a ubuf_info to skb io_uring/notif: refactor io_tx_ubuf_complete() io_uring/notif: remove ctx var from io_notif_tw_complete io_uring/notif: simplify io_notif_flush() io_uring/notif: implement notification stacking

 drivers/net/tap.c      |  2 +-
 drivers/net/tun.c      |  2 +-
 drivers/vhost/net.c    |  8 +++-
 include/linux/skbuff.h | 21 ++++++----
 io_uring/notif.c       | 91 +++++++++++++++++++++++++++++++++++-------
 io_uring/notif.h       | 13 +++---
 net/core/skbuff.c      | 37 +++++++++++------
 7 files changed, 129 insertions(+), 45 deletions(-)

Quoted 5 lines · Pavel Begunkov

io_uring allocates a ubuf_info per zerocopy send request, it's convenient for the userspace but with how things are it means that every time the TCP stack has to allocate a new skb instead of amending into a previous one. Unless sends are big enough, there will be lots of small skbs straining the stack and dipping performance.

The ubuf_info forces TCP segmentation at less than MTU boundaries which kills performance with small message sizes as TCP is forced to send small packets. This is an interesting solution to allow the byte stream to flow yet maintain the segmentation boundaries for callbacks.

Quoted 33 lines

The patchset implements notification, i.e. an io_uring's ubuf_info extension, stacking. It tries to link ubuf_info's into a list, and the entire link will be put down together once all references are gone.

Testing with liburing/examples/send-zerocopy and another custom made tool, with 4K bytes per send it improves performance ~6 times and levels it with MSG_ZEROCOPY. Without the patchset it requires much larger sends to utilise all potential.

bytes  | before | after (Kqps)
100    | 283    | 936
1200   | 195    | 1023
4000   | 193    | 1386
8000   | 154    | 1058

Pavel Begunkov (6):
net: extend ubuf_info callback to ops structure
net: add callback for setting a ubuf_info to skb io_uring/notif: refactor io_tx_ubuf_complete() io_uring/notif: remove ctx var from io_notif_tw_complete io_uring/notif: simplify io_notif_flush() io_uring/notif: implement notification stacking

  drivers/net/tap.c      |  2 +-
  drivers/net/tun.c      |  2 +-
  drivers/vhost/net.c    |  8 +++-
  include/linux/skbuff.h | 21 ++++++----
  io_uring/notif.c       | 91 +++++++++++++++++++++++++++++++++++-------
  io_uring/notif.h       | 13 +++---
  net/core/skbuff.c      | 37 +++++++++++------
  7 files changed, 129 insertions(+), 45 deletions(-)