GrumpyOldTroll / draft-jholland-quic-multicast

Work in progress to propose a multicast extension to quic.
Other
6 stars 6 forks source link

MC_STATE: reason can be 0-length? #94

Closed LPardue closed 2 years ago

LPardue commented 2 years ago

https://www.ietf.org/archive/id/draft-jholland-quic-multicast-01.html#figure-12

MC_STATE Frame {
  Type (i) = TBD-0b (experiments use 0xff3e80b),
  Client Channel State Sequence Number (i),
  ID Length (8),
  Channel ID (8..160),
  State (i),
  Reason (0..i)
}

This implies Reason can be 0-length, which seems odd and I'm not sure a frame parser could acutally handle this unless it new the field was going to be 0-length (I.e. the field is optional, usually indicated by the frame type or another field in the frame.

Suggest this is just made to be Reason (i), unless I'm missing something.

MaxF12 commented 2 years ago

I think the intention is that for cases when the state is Joined or Retired the reason field is 0-length, so the parser could learn it off of that I guess. It would always have to be present for the Left or Declined Join cases. Probably needs rephrasing to be more clear on that.

LPardue commented 2 years ago

Thanks for the explanation. That's a deviation from the way other QUIC frames are described or handled.

I would suggest avoiding deviations and following familiar patterns. Either always send an error code, or model the frame type around a bit field model - I.e. you encode the State field in the type and then that determines if there is a Reason field or not

GrumpyOldTroll commented 2 years ago

Yeah, we missed this one when refactoring out a similar problem in an earlier draft, thanks. I like the idea of:

I'm even kinda thinking it might also be useful to include another 2 types or a length+bytes to support an optional Reason Phrase, like connection close. Not sure we need it, but it might be helpful.