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

[Scott Godin]

Ah - I didn't realize that initial was different than max -
that definitely helps.

On Tue, Feb 24, 2009 at 2:15 PM, Adam Roach <adam at nostrum.com> wrote:

> Hmmm... given that initial CSeq must be in the range of [0,2^31), and max
> CSeq is in the range of [0,2^32), you have somewhat more than 2 billion
> transactions before you roll the CSeq space. I know the US Congress has
> kinda numbed us to the real meaning of "billion," but that would take a long
> time... about 68 years, if you send one transaction per second.
>
> /a
>
> Scott Godin 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 at nostrum.com<mailto:
>> rjsparks at nostrum.com>> 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 at nostrum.com
>>                <mailto:adam at nostrum.com>> 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 at resiprocate.org
>>                        <mailto:resiprocate-devel at resiprocate.org>
>>
>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>>
>>                    _______________________________________________
>>                    resiprocate-devel mailing list
>>                    resiprocate-devel at resiprocate.org
>>                    <mailto:resiprocate-devel at resiprocate.org>
>>
>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>>                _______________________________________________
>>                resiprocate-devel mailing list
>>                resiprocate-devel at resiprocate.org
>>                <mailto:resiprocate-devel at resiprocate.org>
>>
>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>>
>>
>>
>>    _______________________________________________
>>    resiprocate-devel mailing list
>>    resiprocate-devel at resiprocate.org
>>    <mailto:resiprocate-devel at resiprocate.org>
>>    https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20090224/eca1b82e/attachment.htm>