GrumpyOldTroll / draft-jholland-quic-multicast

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

Frame spec cleanup #27

Closed GrumpyOldTroll closed 2 years ago

GrumpyOldTroll commented 2 years ago

There's a number of problems with the frames:

  1. Channel IDs need a length preceding the 8..160.
  2. Some missing reasons in CLIENT_CHANNEL_STATE
  3. Better extensibility if we make the bit fields into varints and define their selector bits, I think. Is this useful?

(Note: working thru these in the course of frame implementation (https://github.com/GrumpyOldTroll/quiche/issues/5), proposal forthcoming...)

For point 3 I'm tentatively thinking something like this:

---
MC_CLIENT_LIMITS Frame {
  Type (i) = TBD-09 (experiments use 0xff3e809),
  Client Limits Sequence Number (i),
  Capabilities Flags(i),
  Max Aggregate Rate (i),
  Max Channel IDs (i),
  Max Joined Count (i),
}
---
{: #fig-mc-client-limits-format title="MC_CLIENT_LIMITS Frame Format"}

The sequence number is implicitly 0 before the first MC_CLIENT_LIMITS frame from the client, and increases by 1 each new frame that's sent.
Newer frames override older ones.

Limit Support Flags is a bit field computed as follows:

 - 0x1 is set if IPv4 channels are permitted
 - 0x2 is set if IPv6 channels are permitted
 - 0x4 is set if SSM channels are permitted
 - 0x8 is set if ASM channels are permitted

Plus something similar in CHANNEL_PROPERTIES. Nice part is this doesn't change encoding if we add flags that go beyond the reserved space, it just ends up taking more varint space when they're set.

MaxF12 commented 2 years ago

I guess the same would also make sense in the transport property then. I am not sure what the trade-off between better extensibility and less available space is, a 1 byte sized var int only has 6 usable bits for example. I guess at the moment we still have 6 unused bits anyway so maybe there isn't much need for extensibility anyway?

It would come down to 4 if you include the SSM/ASM selector though I still can't think of any cases where you would rather have ASM than SSM (this goes into #23). The server needs to know of all the senders anyway since it has to send the integrity frames for all streams, so it could just use multiple SSM joins for all the senders. Allowing ASM just seems like an enormous hassle and security vulnerability. I guess the use case would be somewhere within a domain where data might come from any number of senders, but again this wouldn't work as everything has to go through the server for the integrity frames.

GrumpyOldTroll commented 2 years ago

Yeah, I'm not sure what the trade-off is either. I found the WriteBytes function, so I guess I didn't really need to do this change, but now that it's done locally I'm not sure it's worth changing back, I think it's the same either way.

Regarding ASM: The only interesting use case I know of is to allow P2P recovery between local neighbors. As long as packets are unmodified by peers, they could still match a server's value for the integrity checks. But you'd still need some kind of coordination. NORM did this, and you could imagine something similar based on clients probing for others nearby, and sending each other NACKs when they think they missed data, but that would be another new thing beyond scope of the first version, and probably not worthwhile.

So yes, I think you're right it's probably more trouble than it's worth and we should probably just strip out ASM support as the best answer for #23. I guess it leaves a couple extra unused bits for growth in a few spots, but mostly it prevents anyone from trying to use it, so we don't have to analyze how it could be used on purpose. (However, some networks convert SSM membership reports to ASM on the network, so we still might need to talk about what kinds of problems are possible to see in the client, but I think this just falls under handling for spurious traffic--ignore if it's light and leave if it's heavy I think is the advice we probably want in the end.)