< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

Re: [reSIProcate] CSeq is incorrect in NOTIFY generated by DUM on SUBSCRIBE


I think we could do something fairly simple(probabilistic) for CSEQ roll-overs....if the difference is greater than 2^16, say(or whatever the right number is), and the current remote CSeq is 'close' to the edge,  treat it as a roll-over.

On Tue, Feb 24, 2009 at 10:51 AM, Scott Godin <sgodin@xxxxxxxxxxxxxxx> wrote:
I just did a quick scan of the code - it seems that the only time we check CSeq ordering is for ClientISubscriptions when receiving NOTIFY requests.

Perhaps we should add a CSeq order check to Dialog.cxx, instead of just removing mRemoteCSeq.  We will just need to be careful about being tolerant of CSeq roll overs.

Scott


On Tue, Feb 24, 2009 at 12:06 PM, Robert Sparks <rjsparks@xxxxxxxxxxx> wrote:
We don't check for out-of-order requests?


On Feb 24, 2009, at 10:59 AM, Adam Roach wrote:

I think (hope) Jason was talking about removing the "remote cseq" field. I'd wait for him to get to a non-virtual keyboard -- where he can examine the code himself -- for clarification.

/a


Robert Sparks wrote:
Well, if there was to be a change, it would not be to use the CSeq from the other side, but rather to start at somewhere random instead of always at 1.

While I don't really see a need to change to this random start (because I don't see any benefit), I'm really uncomfortable with the claim you're making here that it might do harm.

(I agree with watching out for unintended consequences in general, but for this particular instance, if the change broke some peer, then introducing a challenging proxy, for instance,
would break that same peer).

RjS

On Feb 24, 2009, at 9:52 AM, Jason Fischl wrote:

My gut feeling here is to leave it unchanged. These kind of changes may have unintended consequences.

Sent from my iPhone

On Feb 24, 2009, at 6:55, Adam Roach <adam@xxxxxxxxxxx> wrote:

Volodymyr:

Thanks for the suggestion.

In terms of the local CSeq: According to RFC 3261, The CSeq number space is unique in each direction. In other words, the CSeq in the NOTIFY is completely unrelated to the CSeq in the SUBSCRIBE. The original code is correct.

I agree with you that the remote CSeq is not used, and can likely be removed. I'd be interested to have Scott, Jason, and Derek weigh in on whether this can be safely removed, or if there are some future plans for it.

/a

Volodymyr Tarasenko wrote:
Hi All,

As I see dum is incorrectly generates CSeq in NOTIFY in case when SUBSCRIBE's SCeq was not 1. NOTIFY's CSeq always starts from 2 in spite of CSeq in SUBSCRIBE was not 1.
The fast patch is very simple:

--- resip/dum/Dialog.cxx
+++ resip/dum/Dialog.cxx
@@ -136,7 +136,7 @@
   }

   mRemoteCSeq = request.header(h_CSeq).sequence();
-      mLocalCSeq = 1;
+      mLocalCSeq = request.header(h_CSeq).sequence();

   DebugLog ( << "************** Created Dialog as UAS **************" );
   DebugLog ( << "mRemoteNameAddr: " << mRemoteNameAddr );

Also, after code review I've found that mRemoteCSeq is never used and looks like it could be safety removed.

Regards,
Volodymyr!


  _______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel


_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel



_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel


_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel