search

moq-wg / moq-transport

draft-ietf-moq-transport
Other
87 stars 22 forks source link

Reduce use of x(b) notation #544

Closed LPardue closed 2 months ago

LPardue commented 2 months ago

This is a proposal to illustrate the changes needed to address one aspect of https://github.com/moq-wg/moq-transport/issues/536 .

In creating this, I realised that the recent change to add a length to control messages used the x(b) notation. This makes it awkward to then fully expand the full message structure as is done in RFC 9000 and RFC 9114 (and proposed for the Type field here on https://github.com/moq-wg/moq-transport/pull/541). Its also arguable if the "followed by that many bytes of binary data" description for the x (b) notation is really supposed to apply to fields that are also formally defined but I think that debate is a distraction.

This PR reduces usage of x (b) by adding an explicit length field before the respective value. This reveals that some control message types have a redundant length field, which is rather pointless in some cases. For example, with this change we have

GOAWAY Message {
  Length (i),
  New Session URI Length (i),
  New Session URI (..),
}

this could be simplified to just 

GOAWAY Message {
  Length (i),
  New Session URI (..),
}

However, I avoided doing that in this PR because it is a breaking wire format change.

I'd suggest we do the editorial changes in this PR first, and then survey the control types for opportunities to remove redundant lengths. We could even go so far as rearranging the order of fields so that the variable length ones go at the end, for example HTTP/3 does this with HEADERS and PUSH_PROMISE

The last remaining usage of x (b) after this change relates to x (tuple). I left that one since I didn't want to conflate things.

suhasHere commented 2 months ago

+1 with moving away from (b) as that was the original structure of the messages. I would see if we can combine this change with Type change as well to handle all the control message changes in one place.

martinduke commented 2 months ago

25 Sep 2024 interim: No objections to this direction. @ianswett Merge when github discussion has petered out.

LPardue commented 2 months ago

closing in favor of the mega PR that people asked for: https://github.com/moq-wg/moq-transport/pull/550