search for: nbd_structured_reply_magic

.../* command processed ok */ } @@ -433,6 +446,11 @@ send_structured_reply_read (struct connection *conn, assert (cmd == NBD_CMD_READ); + if (conn->cork) { + r = conn->cork (conn, true); + assert (r == 0); /* For now, only uncorking can fail */ + } + reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); reply.handle = handle; reply.flags = htobe16 (NBD_REPLY_FLAG_DONE); @@ -459,6 +477,14 @@ send_structured_reply_read (struct connection *conn, return connection_set_status (conn, -1); } + if (conn->cork) { + r = conn->cork (conn, false); + if (r == -1) { + nbdkit_e...

...BD_SUCCESS or one of NBD_E*. */ uint64_t handle; /* Opaque handle. */ } __attribute__((packed)); -#define NBD_REQUEST_MAGIC 0x25609513 -#define NBD_REPLY_MAGIC 0x67446698 +/* Structured reply (server -> client). */ +struct structured_reply { + uint32_t magic; /* NBD_STRUCTURED_REPLY_MAGIC. */ + uint16_t flags; /* NBD_REPLY_FLAG_* */ + uint16_t type; /* NBD_REPLY_TYPE_* */ + uint64_t handle; /* Opaque handle. */ + uint32_t length; /* Length of payload which follows. */ +} __attribute__((packed)); + +struct structured_reply_of...

...e `NBD_REPLY_FLAG_DONE` reply flag is used to identify the final chunk. Unless further documented by individual requests below, the chunks MAY be sent in any order, except that the chunk with @@ -418,7 +418,7 @@ A structured reply chunk message looks as follows: S: 32 bits, 0x668e33ef, magic (`NBD_STRUCTURED_REPLY_MAGIC`) S: 16 bits, flags S: 16 bits, type -S: 64 bits, handle +S: 64 bits, cookie S: 32 bits, length of payload (unsigned) S: *length* bytes of payload data (if *length* is nonzero) -- 2.39.2

Slightly RFC, as I need more time to investigate why Unix sockets appeared to degrade with this patch. But as TCP sockets (over loopback to localhost) and TLS sessions (regardless of underlying Unix or TCP) both showed improvements, this looks like a worthwhile series. Eric Blake (2): server: Add support for corking server: Cork around grouped transmission send()s server/internal.h | 3

...C. */ uint32_t error; /* NBD_SUCCESS or one of NBD_E*. */ uint64_t handle; /* Opaque handle. */ } __attribute__((packed)); /* Structured reply (server -> client). */ -struct structured_reply { +struct nbd_structured_reply { uint32_t magic; /* NBD_STRUCTURED_REPLY_MAGIC. */ uint16_t flags; /* NBD_REPLY_FLAG_* */ uint16_t type; /* NBD_REPLY_TYPE_* */ @@ -176,15 +189,20 @@ struct structured_reply { uint32_t length; /* Length of payload which follows. */ } __attribute__((packed)); -struct structured_reply_offset_d...

...e function. > + */ Fair enough. > + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); > + struct structured_reply reply; > + struct structured_reply_offset_data offset_data; > + int r; > + > + assert (cmd == NBD_CMD_READ); > + > + reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); > + reply.handle = handle; > + reply.flags = htobe16 (NBD_REPLY_FLAG_DONE); > + reply.type = htobe16 (NBD_REPLY_TYPE_OFFSET_DATA); > + reply.length = htobe32 (sizeof offset_data + count); This line is correct, but I had to remind myself of C precedence rules on this one; writing...

...5) | value)? Or would that mess up the generated > protocol-to-lookup magic? Could do it either way really. The sed magic uses the symbol (eg. NBD_REPLY_TYPE_ERROR) not the value so either should work. I'll play around with it to see which looks nicer. > > + reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); > > + reply.handle = handle; > > + reply.flags = htobe16 (NBD_REPLY_FLAG_DONE); > > + reply.type = htobe16 (NBD_REPLY_TYPE_OFFSET_DATA); > > + reply.length = htobe32 (sizeof offset_data + count); > > This line is correct, but I had to remind myself of C preceden...

...on-compliant server that is > attempting to cause an information leak. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > plugins/nbd/nbd.c | 232 ++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 212 insertions(+), 20 deletions(-) > > + case NBD_STRUCTURED_REPLY_MAGIC: > + if (!h->structured) { > + nbdkit_error ("structured response without negotiation"); > + return nbd_mark_dead (h); > + } > + if (read_full (h->fd, sizeof rep.simple + (char *) &rep, > + sizeof rep - sizeof rep.simple)) &...

...n 0; REPLY.CHECK_SIMPLE_OR_STRUCTURED_REPLY: + struct command_in_flight *cmd; uint32_t magic; + uint64_t handle; magic = be32toh (h->sbuf.simple_reply.magic); if (magic == NBD_SIMPLE_REPLY_MAGIC) { SET_NEXT_STATE (%SIMPLE_REPLY.START); - return 0; } else if (magic == NBD_STRUCTURED_REPLY_MAGIC) { SET_NEXT_STATE (%STRUCTURED_REPLY.START); - return 0; } else { SET_NEXT_STATE (%.DEAD); /* We've probably lost synchronization. */ @@ -86,6 +88,29 @@ return -1; } + /* NB: This works for both simple and structured replies because the + * handle is stored at the...

...++ b/server/protocol.c > @@ -503,6 +503,9 @@ send_structured_reply_block_status (struct connection *conn, > if (blocks == NULL) > return connection_set_status (conn, -1); > > + nr_blocks = 32; > + blocks[0].length = htobe32 (4096); > + > reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); > reply.handle = handle; > reply.flags = htobe16 (NBD_REPLY_FLAG_DONE); > @@ -526,7 +529,7 @@ send_structured_reply_block_status (struct connection *conn, > > /* Send each block descriptor. */ > for (i = 0; i < nr_blocks; ++i) { > - r = conn->send (conn...

...-S: 64 bits, handle +S: 64 bits, handle (MUST match the request) S: (*length* bytes of data if the request is of type `NBD_CMD_READ` and *error* is zero) @@ -416,9 +473,9 @@ on the chunks received. A structured reply chunk message looks as follows: S: 32 bits, 0x668e33ef, magic (`NBD_STRUCTURED_REPLY_MAGIC`) -S: 16 bits, flags +S: 16 bits, reply flags S: 16 bits, type -S: 64 bits, handle +S: 64 bits, handle (MUST match the request) S: 32 bits, length of payload (unsigned) S: *length* bytes of payload data (if *length* is nonzero) @@ -426,6 +483,45 @@ The use of *length* in the rep...

...; #define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) #define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5) -extern const char *name_of_nbd_info (int); #define NBD_INFO_EXPORT 0 /* NBD_INFO_EXPORT reply (follows fixed_new_option_reply). */ @@ -197,14 +192,12 @@ struct structured_reply_error { #define NBD_STRUCTURED_REPLY_MAGIC 0x668e33ef /* Structured reply flags. */ -extern const char *name_of_nbd_reply_flag (int); #define NBD_REPLY_FLAG_DONE (1<<0) #define NBD_REPLY_TYPE_ERR(val) ((1<<15) | (val)) #define NBD_REPLY_TYPE_IS_ERR(val) (!!((val) & (1<<15))) /* Structured reply type...

We should have only one NBD protocol file. Let's make nbdkit's version the canonical one, and use it in libnbd. Rich.

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses

On 3/23/19 7:58 AM, Richard W.M. Jones wrote: >> A good point here is what happens with the block-status branch[1]. >> Let's see: >> >> $ ./nbdkit memory size=64M --run '/home/rjones/d/qemu/qemu-img convert $nbd /var/tmp/out' >> qemu-img: Payload too large >> nbdkit: memory.1: error: write reply: NBD_CMD_BLOCK_STATUS: Broken pipe >> >>

...{ + case NBD_SIMPLE_REPLY_MAGIC: + nbdkit_debug ("received simple reply for cookie %#" PRIx64 ", status %s", + rep.simple.handle, + name_of_nbd_error(be32toh (rep.simple.error))); + error = be32toh (rep.simple.error); + break; + case NBD_STRUCTURED_REPLY_MAGIC: + if (!h->structured) { + nbdkit_error ("structured response without negotiation"); + return nbd_mark_dead (h); + } + if (read_full (h->fd, sizeof rep.simple + (char *) &rep, + sizeof rep - sizeof rep.simple)) + return nbd_mark_dead (h)...

...re not doing - * that yet we acquire the lock for the whole function. - */ - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); - struct structured_reply reply; - struct structured_reply_offset_data offset_data; - int r; - - assert (cmd == NBD_CMD_READ); - - reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); - reply.handle = handle; - reply.flags = htobe16 (NBD_REPLY_FLAG_DONE); - reply.type = htobe16 (NBD_REPLY_TYPE_OFFSET_DATA); - reply.length = htobe32 (count + sizeof offset_data); - - r = conn->send (conn, &reply, sizeof reply); - if (r == -1) { - nbdkit_error ("write reply:...

...uct nbdkit_extents *extents) +{ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); + struct structured_reply reply; + size_t nr_extents = nbdkit_extents_size (extents); + uint32_t context_id; + size_t i; + int r; + + assert (cmd == NBD_CMD_BLOCK_STATUS); + + reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); + reply.handle = handle; + reply.flags = htobe16 (NBD_REPLY_FLAG_DONE); + reply.type = htobe16 (NBD_REPLY_TYPE_BLOCK_STATUS); + reply.length = htobe32 (sizeof context_id + + nr_extents * sizeof (struct block_descriptor)); + + r = conn->send (conn, &reply, size...

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

This was v3 patch 2/22, reworked to address the confusion about how a structured reply header is read in two pieces before getting to the payload portion. I'm still working on rebasing the rest of my v3 series (patches 1, 3-22) from other comments given, but this seemed independent enough that it's worth posting now rather than holding it up for the rest of the series. Eric Blake (4):