search

xapi-project / nbd

Ocaml NBD library
https://xapi-project.github.io/nbd/
Other
14 stars 16 forks source link

Server ignores NBD_FLAG_C_NO_ZEROES (and unknown flags) in initial client flags #63

Open thomassa opened 7 years ago

thomassa commented 7 years ago

Quoting from the NBD protocol,

Client flags

This field of 32 bits is sent after initial connection and after receiving the handshake flags from the server.

  • [irrelevant part snipped]
  • bit 1, NBD_FLAG_C_NO_ZEROES; MUST NOT be set if the server did not set NBD_FLAG_NO_ZEROES. If set, the server MUST NOT send the 124 bytes of zeroes at the end of the negotiation.

Clients MUST NOT set any other flags; the server MUST drop the TCP connection if the client sets an unknown flag, or a flag that does not match something advertised by the server.

The server fails to do the action I have emphasised above. It does not set NBD_FLAG_NO_ZEROES in the intial handshake flags it sends, but then it makes no decisions that depend on whether the client set NBD_FLAG_C_NO_ZEROES: instead the server carries on regardless and sends the zeroes.

If we were to add support for this flag and feature, then Protocol.Diskinfo.t should omit the padding if the client flag is set.

gaborigloi commented 7 years ago

Good catch! We should obey the NBD protocol.

gaborigloi commented 7 years ago

Actually I've been looking for a test suite for NBD server that tests that the server correctly implements the NBD protocol (e.g. tests all the MUSTs, SHOULDs in the design, and all the corner cases too, like this one), and I couldn't find any. I was discussing with @thomasmck that we could/should bring up this topic on the NBD mailing list - maybe a community effort could be started. Having such a test suite would be really useful for all community members who are implementing an NBD server. And same for benchmarks, but I didn't check whether there is a benchmark suite for NBD servers.