< 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


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@xxxxxxxxxxx <mailto: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
                <mailto: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
                        <mailto:resiprocate-devel@xxxxxxxxxxxxxxx>
                        
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel


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

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




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