However 6 is probably too low - in nbdcopy we use 64. I kept it low just to verify the system works in the way I intended it to.

So you can open multiple TCP connections on each side and issue multiple commands in flight on each of those connections. I will look into this, so we open multiple sockets hence get multiple socket_fds which we can read from and write to?

The code looks very minimal at the moment. I'm not very familiar with the fmt:: class. It's just for formatting and output like printf.

Does it successfully make a handshake to ‘nbdkit -o’ yet? I read in the protocol document https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md#oldstyle-... that handshake is just about the server sending data to the client on accept, I open a socket and connect then read the handshake, and with the data, then populate NbdConnection variables. All this is done currently in the constructor of NbdConnection synchronously. So I guess the answer is yes.

Current data I receive after handshake from source and destination ports Copying from port 1234 to port 2345. ┌──────────────────────────────┐ │ source │ └──────────────────────────────┘ NBDMAGIC (0x4e42444d41474943) 0x420281861253 4096 0b0000000000000000 0b0000110101101101 ┌──────────────────────────────┐ │ destination │ └──────────────────────────────┘ NBDMAGIC (0x4e42444d41474943) 0x420281861253 8192 0b0000000000000000 0b0000110101101101 and then the program ends, destructors call disconnecting code. command to run nbkit server nbdkit data ' ( 0x55 0xAA )*2048 ' -o -f -p 1234

I notice that although you create an io_uring you're not using it. But I guess that's still to come. I am making it now done with the NBD read request submission should be ready soon.

Thanks and regards, Abhay

I ran into a problem while working on receiving data from nbd source (Reply to NBD_CMD_READ) As you know we need to parse the error in Reply Header before we can proceed reading the data. Let's say an error occurred so instead of HEADER1_DATA1..._HEADER2_DATA2... we will get HEADER1_HEADER2_ DATA2... (as far as I know) so submitting a recv request to io_uring with length = sizeof(HEADER1+DATA1) would cause problem as it won't detect NBD packet boundaries and will give us as many bytes we ask it (I may be wrong here that's what I read till now). A remedy to this would be just submit 'header reads' to io_uring when we get a read and if header says there were no errors we can be sure there is length bytes ready to be read in the buffer(rest of the NBD packet) and read won't block. Now, as far as I can tell this would work as I expect but our main concern is avoiding copy_user_enhanced_fast_string so this won't be nice. Also attaching metadata (Operation) to read SQE doesn't make sense because As far as I know io_uring won't be able to tell the difference the read is for which io_uring request, Reply Header's handle will tell us which operation in operations vector does this NBD packet belong to. Another solution would be opening multiple sockets one for each slot in operations vector, only one NBD operation runs on a socket i.e. only one inflight request per socket, that too sounds like a bad idea. Thanks and regards, Abhay

On Sun, Jun 27, 2021 at 05:00:00PM +0530, Abhay Raj Singh wrote:

I ran into a problem while working on receiving data from nbd source (Reply to NBD_CMD_READ)

As you know we need to parse the error in Reply Header before we can proceed reading the data. Let's say an error occurred so instead of HEADER1_DATA1..._HEADER2_DATA2... we will get HEADER1_HEADER2_ DATA2... (as far as I know) so submitting a recv request to io_uring with length = sizeof(HEADER1+DATA1) would cause problem as it won't detect NBD packet boundaries and will give us as many bytes we ask it (I may be wrong here that's what I read till now).

Two problems with requesting length = sizeof(HEADER1+DATA1): First, as you pointed out, if the server errors out, you will only get HEADER1 bytes back (including the error indication), and no data bytes. Second, once you start issuing out-of-order requests to the server rather than synchronous waiting for a reply before beginning the next request, then you are also at risk that the server might answer HEADER2 prior to answering HEADER1.

A remedy to this would be just submit 'header reads' to io_uring when we get a read and if header says there were no errors we can be sure there is length bytes ready to be read in the buffer(rest of the NBD packet) and read won't block.

Yes, you really DO have to submit a read request for JUST a header, and then based on what that header tells you are you finally able to decipher what to expect next on the wire (another header, or WHICH read you are getting a reply to). As Rich said, processing the headers via user-space copies is probably fine, where the real savings come into play when processing the data payloads.

Now, as far as I can tell this would work as I expect but our main concern is avoiding copy_user_enhanced_fast_string so this won't be nice.

Also attaching metadata (Operation) to read SQE doesn't make sense because As far as I know io_uring won't be able to tell the difference the read is for which io_uring request, Reply Header's handle will tell us which operation in operations vector does this NBD packet belong to.

Yeah, because of the out-of-order potential of the NBD protocol, you will have to be careful that you are processing headers before knowing where to send payloads.

Another solution would be opening multiple sockets one for each slot in operations vector, only one NBD operation runs on a socket i.e. only one inflight request per socket, that too sounds like a bad idea.

To some extent, multiple sockets is what Rich mentioned in the multi-conn approach, but having one socket per parallel operation is going to be slower than properly handling out-of-order traffic on one socket (there may still be savings by having multiple sockets, but you also want to be sure to handle multiple in-flight commands per socket). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

On Sat, Jul 10, 2021 at 03:03:19AM +0530, Abhay Raj Singh wrote:

I managed to get the reading from the server part of nbdcpy working (queuing new requests after a socket gets free etc.), the writing to destination part is not being executed due to some sneaky bug I hope to find  it soon.

Cool! I was following your git repo and watching some of the changes you'd made. Just to let you know I'm on holiday until Thursday, but you can keep looking at the write bug and sending emails and I'll catch up as soon as I get back.

This zero-copy TCP receive API looks promising should I pursue it further https://lwn.net/Articles/752188/

I wonder how this interacts (if at all?) with io_uring. I'm not committed to any particular way to make nbdcopy faster. Any method that makes it faster on Linux is good as far as I'm concerned. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

On Sat, Jul 31, 2021 at 06:12:16PM +0530, Abhay Raj Singh wrote:

I was able to resolve the issue caused by multiplexing where HEADER1+DATA1... was read as HEADER1+HEADER2+HEADER3...

So nbdcpy is functional! and copies completely.

Though I have been trying to resolve another issue

The command below works fine (16MB) nbdkit -v -o data ' ( 0x41 0x42 )*16777216 ' --run 'bin/nbdcpy $port $port'

But when I use random `nbdkit -v -o random size=16M --run 'bin/nbdcpy $port $port'`

I get error: data written does not match expected

This is a problem :-( It comes from here and indicates that the data you're writing doesn't exactly match what was originally read: https://gitlab.com/nbdkit/nbdkit/-/blob/de8dcd3a34a38b088a0f9a6f8ca754702ad1... I would suggest starting with a very small disk and printing out everything that you read and write. Should help to track down the problem.

So, I assume my code is affected by data somehow which I am not able to find.

As it's functional now I will update the documentation to make it more understandable and true to code and then ask for reviews from io_uring people. I will also resume work on documenting how libnbd works and on how to integrate io_uring with libnbd or nbdcopy.

Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

On Sat, Aug 07, 2021 at 09:53:10PM +0530, Abhay Raj Singh wrote:

Fixed the issue, I needed to learn to use Wireshark so took some time. Turns out I was not updating the offset for an 'operation' / operation slot in a rare case.

As an operation owns and NBD handle (the number) say 7

Initially, it read from offset 512 then 2048 then 4096, etc. But was always writing to offset 512 (in rare cases) because I didn't update the offset field of operation slot.

There's a bug where it gets stuck after reading 300KB or so, should be done by tomorrow.

OK, let me know how it goes :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html

[adding the NBD list into cc] On Mon, Aug 23, 2021 at 09:26:34PM +0530, Abhay Raj Singh wrote:

I had an idea for optimizing my current approach, it's good in some ways but can be faster with some breaking changes to the protocol.

Currently, we read (from socket connected to source) one request at a time the simple flow looks like `read_header(io_uring) ---- success ---> recv(data) --- success ---> send(data) & queue another read header` but it's not as efficient as it could be at best it's a hack.

Another approach I am thinking about is a large buffer where we can read all of the socket's data and process packets from that buffer as all the I/O is handled. this minimizes the number of read requests to the kernel as we do 1 read for multiple NBD packets.

Further optimization requires changing the NBD protocol a bit Current protocol 1. Memory representation of a response (20-byte header + data) 2. Memory representation of a request (28-byte header + data)

HHHHH_DDDDDDDDD... HHHHHHH_DDDDDDDDD...

H and D represent 4 bytes, _ represents 0 bytes

You are correct that requests are currently 28 bytes header plus any payload (where payload is currently only in NBD_CMD_WRITE). But responses are two different lengths: simple responses are 16 bytes + payload (payload only for NBD_CMD_READ, and only if structured replies not negotiated), while structured responses are 20 bytes + payload (but while NBD_CMD_READ and NBD_CMD_BLOCK_STATUS require structured replies, a compliant server can still send simple replies to other commands). So it's even trickier than you represent here, as reading 20-byte headers of a reply is not going to always do the right thing.

With the large buffer approach, we read data into a large buffer, then copy the NBD packet's data to a new buffer, strap a new header to it and send it. This copying is what we wanted to avoid in the first place.

If the response header was 28 bytes or the first 8-bytes of data were useless we could have just overwritten the header part and sent data directly from the large buffer, therefore avoiding the copy.

What are your thoughts?

There's already discussions about what it would take to extend the NBD protocol to support 64-bit requests (not that we'd want to go beyond current server restrictions of 32M or 64M maximum NBD_CMD_READ and NBD_CMD_WRITE, but more so we can permit quick image zeroing via a 64-bit NBD_CMD_WRITE_ZEROES). Your observation that having the request and response headers be equally sized for more efficient handling is worthwhile to consider in making such a protocol extension - of necessity, it would have to be via an NBD_OPT_* option requested by the client during negotiation and responded to affirmatively by the server, before both sides then use the new-size packets in both directions after NBD_OPT_GO (and a client would still have to be prepared to fall back to the unequal-sized headers if the server doesn't understand the option). For that matter, is there a benefit to having cache-line-optimized sizing, where all headers are exactly 32 bytes (both requests and responses, and both simple and structured replies)? I'm thinking maybe NBD_OPT_FIXED_SIZE_HEADER might be a sane name for such an option. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org