Re: [Wish] Use entity-tags for solving out of order PATCH requests

[Lorenzo Miniero <[e-mail]>]

On Thu, 03 Mar 2022 12:27:42 +0100
Juliusz Chroboczek <[e-mail]> wrote:

> Thanks a lot for your work, Sergio.  I think I got it now.
> 


Hi Juliusz,

chiming in with some responses (from my understanding of the document)
and comments, waiting for Sergio to answer.


> 1. Would you consider adding guidance for server implementers about
> how to generate the ETag ?  For example, something like
> 
> "The ETag could, for example, be generated by appending the ICE ufrag
> with the ice password, or by using the timestamp of the last ICE
> restart."
> 
> (I'm not very familiar with HTTP, but it is my understanding that a
> strong ETag must be unique over the lifetime of a resource.  I'm not
> quite sure what a resource is, and I'm not sure what data can be used
> to guarantee the properties of the ETag that are required by HTTP.
> I'm sure I'm not the only person who would find such guidance useful.)
> 


It's my understanding that any random string would work, so if
concatenating ICE ufrag and pwd (which is what Sergio does in the
draft) works for you, then it's ok. Even if the ETag is supposed to be
unique over the lifetime of a resource, as you say, a WHIP resource has
a short life, since once the PeerConnection is closed it goes away
(further attempts to publish to the same WHIP endpoint will in practice
create a new resource, even if the URI is the same). As such, it should
be easy to enforce those unique IDs in that limited scope.


> 2. Could you please spell out explicitly how the server distinguishes
> between restarts and candidates?  My understanding is that one says
> "If-Match: "*"" while the other carries a strong ETag, but I'd like
> to see it spelled out explicitly.
> 


The right way to do that would be looking at the payload, I suppose: if
the provided ICE credentials differ from the ones you knew, it's a
restart. In my prototype, for instance, I'm only allowing "*" as a
value if the payload _actually_ contains a restart: not sure if that's
overly zealous, or even plain wrong (maybe 412 makes no sense if the
client is sending "*"?).


> 3. What should a server do if it cannot parse a candidate (say, the
> client sends an mDNS candidate, and the server doesn't implement
> mDNS, or the server is IPv6-only and the client sends an IPv4
> candidate).  Should it return success and ignore the candidate, or
> should it return an error?  If so, which one?
> 


I think it's up to the implementation: in my case I'm just sending a
204 back whether Janus will accept the trickle candidate or not,
without waiting for a response.


> 4. What should the server do if restarting ICE fails?  Should it
> ignore the restart and return the same ETag as before, should it
> return an error to the client and continue with the previous ICE
> generation, or should it return the error and close the RTP stream
> (with RTCP BYE)?
> 


That's a good point: in my implementation, I'm sending a 400 back
(arbitrary choice, some text in the draft would indeed help), but I'm
not actively tearing down the PeerConnection. In theory, if an ICE
restart is required and the ICE restart itself fails, I guess it's
reasonable to assume the PC will eventually go down by itself anyway,
but maybe some guidance in the draft would indeed help.

Lorenzo

-- 
I'm getting older but, unlike whisky, I'm not getting any better
https://twitter.com/elminiero