[reSIProcate] Matching INFO requests and responses

Thu Dec 9 11:27:44 CST 2010

The other InviteSession API's return objects by reference (ie. SipMessage&)
- you just need to handle the case when a caller accesses the new API and
you don't have a previous NIT request to return - returning a reference to a
static empty SipMessage makes sense for this case
(see InviteSession::getLocalOfferAnswer for an example.

Scott

On Thu, Dec 9, 2010 at 12:02 PM, Aron Rosenberg <arosenberg at logitech.com>wrote:

> The only "main" reason to expand the callback API was to try and match
> ClientPagerMessageHandler...but I can rework the API to still keep the
> last request in the NIT queue but expose a new method on InviteSession
> called getLastNITRequest() . Should that return a
> auto_ptr<SipMessage>, const SipMessage* or something else?
>
> -Aron
>
>
> On Thu, Dec 9, 2010 at 6:53 AM, Scott Godin <sgodin at sipspectrum.com>
> wrote:
> > That works for me too - in fact if you are going with this approach, then
> > you may as well store the entire last IT request and make it available
> > instead of just the contents.  A branch sounds like a good idea.  : )
> > Thanks,
> > Scott
> >
> > On Thu, Dec 9, 2010 at 9:44 AM, Francis Joanis <francis.joanis at gmail.com
> >
> > wrote:
> >>
> >> Hi,
> >> What about the following: rather than changing the DUM callbacks we
> could
> >> expose a reader to get the last Contents that was sent as a NIT? A bit
> like
> >> InviteSession::getRemoteSdp().
> >> That way, if someone wants to access it, it can simply be done in the
> on*
> >> callbacks. It would be cleaner in the sense that it won't change the
> >> interfaces at all.
> >> I will take care of ensuring that the memory is properly managed (helped
> >> by valgrind). I was thinking of cloning the contents of the message then
> >> assigning it to an auto_ptr as a member of InviteSession.
> >> I also want to code a test suite to ensure that it works for INFO,
> >> MESSAGE. We also need to ensure that it doesn't screw up the REFERs
> since
> >> currently nitComplete() is also called from Dialog.cxx.
> >> I was thinking of creating a branch to do this: b-nit-contents-20101209.
> >> Scott, let me know if you think that would be a bad idea ;)
> >> I'll let you know of my progress.
> >> Francis
> >>
> >> On Thu, Dec 9, 2010 at 1:43 AM, Aron Rosenberg <arosenberg at logitech.com
> >
> >> wrote:
> >>>
> >>> I coded up a working patch based on Scott's feedback. The only
> >>> difference is that neither set of callbacks are pure-virtual now. I
> >>> figure this was better moving forward so that new code didn't need to
> >>> reference the old handler. It also seems that dealing with the
> >>> responses to INFO and MESSAGE aren't needed by most normal UA's. I
> >>> tested this patch with MESSAGE requests on the wire and it works.
> >>>
> >>> My only worry with this patch is that the memory handling semantics
> >>> are correct. mNITQueue holds SharedPtr<SipMessage> and it seems ok for
> >>> that to stay valid after dum.send() has been called. On the callback
> >>> we pass a const Contents* rather than an auto_ptr since we can't get
> >>> an auto_ptr from SipMessage for Contents without a clone call. I
> >>> figure the developer who needs this longer than the life of the
> >>> callback can clone themselves.
> >>>
> >>> Francis: Can you test the INFO side to make sure you are getting the
> >>> right data back.
> >>>
> >>> -Aron
> >>>
> >>>
> >>> On Wed, Dec 8, 2010 at 1:56 PM, Scott Godin <sgodin at sipspectrum.com>
> >>> wrote:
> >>> > I think we could add the new callback and leave the old one place.
>  The
> >>> > new
> >>> > handler would not be virtual, and the default handler for the new
> >>> > callback
> >>> > would just throw away the contents and call the old callback.  This
> >>> > will
> >>> > allow current applications to use the new dum version without any
> code
> >>> > changes.  For applications that want to see the Contents, they would
> >>> > need to
> >>> > implement both callbacks, but the old callback can be left blank as
> it
> >>> > will
> >>> > never get called.
> >>> > Scott
> >>> >
> >>> > On Wed, Dec 8, 2010 at 4:36 PM, Aron Rosenberg
> >>> > <arosenberg at logitech.com>
> >>> > wrote:
> >>> >>
> >>> >> The major use-case we need to fix is this:
> >>> >>
> >>> >> InviteSession based MESSAGE requests where the far end returns 415
> >>> >> (mime-type invalid) for some types (text/plain works, but text/html
> >>> >> doesn't, application/im-iscomposing+xml doesn't). Without the
> >>> >> Contents* of the request that failed, we can't tell which type
> failed
> >>> >> to determine if we need to notify the user sending the message.
> >>> >>
> >>> >> Scott: You have an opinion on if changing the callback type is an ok
> >>> >> way to proceed?
> >>> >>
> >>> >> Francis: I have attached a prototype patch for this, it has only
> been
> >>> >> compile tested if you want to try it and flesh it out some more.
> >>> >>
> >>> >> -Aron
> >>> >>
> >>> >> On Wed, Dec 8, 2010 at 12:39 PM, Francis Joanis
> >>> >> <francis.joanis at gmail.com> wrote:
> >>> >> > Hi,
> >>> >> > I like what you are proposing since any user-specific information
> >>> >> > would
> >>> >> > go
> >>> >> > in the user-derived Contents object: it acts a bit like an
> >>> >> > AppDialog/Set
> >>> >> > in
> >>> >> > that sense.
> >>> >> > We could even implement this by only caching the Contents of the
> >>> >> > last
> >>> >> > sent
> >>> >> > NIT request, since they're all sent serially (so no need to do the
> >>> >> > extra
> >>> >> > management on mNITQueue). This would mainly require adding a new
> >>> >> > member
> >>> >> > variable (probably an auto_ptr<Contents>) to InviteSession and
> >>> >> > changing
> >>> >> > the
> >>> >> > various on* callbacks in InviteSessionHandler.
> >>> >> > In the case of the REFER (which is also using mNITQueue), it would
> >>> >> > most
> >>> >> > likely have a NULL Contents, so it could get ignored on the
> >>> >> > callbacks
> >>> >> > (i.e.
> >>> >> > no need to modify the onRefer* callbacks).
> >>> >> > I started playing around with the code but I'd like to know if
> such
> >>> >> > a
> >>> >> > change
> >>> >> > would be a good one or not from the perspective of the more
> >>> >> > experienced
> >>> >> > developers. Changing callback APIs seems like a somewhat major
> >>> >> > change
> >>> >> > (i.e.
> >>> >> > it will break current apps) and I wouldn't want to do it unless
> >>> >> > there is
> >>> >> > really a good use case.
> >>> >> > Regards,
> >>> >> > Francis
> >>> >> >
> >>> >> >
> >>> >> > On Wed, Dec 8, 2010 at 1:32 PM, Aron Rosenberg
> >>> >> > <arosenberg at logitech.com>
> >>> >> > wrote:
> >>> >> >>
> >>> >> >> For a slightly different issue (matching 415's to outbound
> MESSAGE
> >>> >> >> requests) I was looking at modifying InviteSession to remove the
> >>> >> >> SipMessage* from the mNITQueue upon end of transaction rather
> than
> >>> >> >> beginning of transaction. We could then pass up the Contents*
> like
> >>> >> >> is
> >>> >> >> done with ClientPagerMessage.
> >>> >> >>
> >>> >> >> onMessageSuccess(resip::InviteSessionHandle ish, const
> >>> >> >> resip::SipMessage&status) would become
> >>> >> >> onMessageSuccess(resip::InviteSessionHandle ish, const
> >>> >> >> resip::SipMessage&status, std::auto_ptr<Contents> contents)
> >>> >> >> onMessageFailure(resip::InviteSessionHandle ish, const
> >>> >> >> resip::SipMessage &status) would become
> >>> >> >> onMessageFailure(resip::InviteSessionHandle ish, const
> >>> >> >> resip::SipMessage &status, std::auto_ptr<Contents> contents)
> >>> >> >>
> >>> >> >> This would also work in the INFO cases with the same changes to
> add
> >>> >> >> std::auto_ptr<Contents> contents to the callbacks.
> >>> >> >>
> >>> >> >> I also attempted to try the other way, dig into the
> >>> >> >> TransactionController / TU and create a "SipMessage*
> >>> >> >> findLastRequestForResponse(SipMessage*)" that used the tid, but
> >>> >> >> this
> >>> >> >> would require adding a number of public functions to SipStack,
> >>> >> >> TransactionMap and some other classes.
> >>> >> >>
> >>> >> >> -Aron
> >>> >> >>
> >>> >> >> Aron Rosenberg
> >>> >> >> Logitech Inc. (SightSpeed Group)
> >>> >> >>
> >>> >> >>
> >>> >> >>
> >>> >> >> On Wed, Dec 8, 2010 at 10:08 AM, Francis Joanis
> >>> >> >> <francis.joanis at gmail.com> wrote:
> >>> >> >> > So I guess it leaves me with the original workaround of
> >>> >> >> > leveraging
> >>> >> >> > the
> >>> >> >> > fact
> >>> >> >> > that all NITs are sequential. Also I think I could use an
> >>> >> >> > outgoing
> >>> >> >> > DumFeature to inspect outgoing messages, but that would be
> called
> >>> >> >> > for
> >>> >> >> > all
> >>> >> >> > outgoing messages, which will impact performance.
> >>> >> >> > Thanks,
> >>> >> >> > Francis
> >>> >> >> >
> >>> >> >> > On Wed, Dec 8, 2010 at 11:28 AM, Scott Godin
> >>> >> >> > <sgodin at sipspectrum.com>
> >>> >> >> > wrote:
> >>> >> >> >>
> >>> >> >> >> The branch parameter is "reset" in DialogUsageManager send (ln
> >>> >> >> >> 824)
> >>> >> >> >> -
> >>> >> >> >> so
> >>> >> >> >> it won't help to store the branch parameter in the message
> >>> >> >> >> created
> >>> >> >> >> (same
> >>> >> >> >> applies for onReadyToSend).
> >>> >> >> >> Scott
> >>> >> >> >>
> >>> >> >> >> On Wed, Dec 8, 2010 at 11:08 AM, Francis Joanis
> >>> >> >> >> <francis.joanis at gmail.com>
> >>> >> >> >> wrote:
> >>> >> >> >>>
> >>> >> >> >>> Hi Scott,
> >>> >> >> >>> Thanks for your reply.
> >>> >> >> >>> What about if we changed InviteSession::info/refer/message to
> >>> >> >> >>> return a
> >>> >> >> >>> _copy_ of the constructed SipMessage rather than returning
> >>> >> >> >>> void?
> >>> >> >> >>> I ran a quick test and it looks like the transaction id is
> set
> >>> >> >> >>> when
> >>> >> >> >>> the
> >>> >> >> >>> message is constructed. That way, I can immediately cache the
> >>> >> >> >>> SIP
> >>> >> >> >>> transaction id to pair it with my "key". This removes the
> need
> >>> >> >> >>> for
> >>> >> >> >>> my
> >>> >> >> >>> extra
> >>> >> >> >>> "key list" and does not require message inspection in
> >>> >> >> >>> onReadyToSend.
> >>> >> >> >>> (I've also attached a patch to do this.)
> >>> >> >> >>> Thanks,
> >>> >> >> >>> Francis
> >>> >> >> >>>
> >>> >> >> >>> On Wed, Dec 8, 2010 at 9:12 AM, Scott Godin
> >>> >> >> >>> <sgodin at sipspectrum.com>
> >>> >> >> >>> wrote:
> >>> >> >> >>>>
> >>> >> >> >>>> ...inline...
> >>> >> >> >>>>
> >>> >> >> >>>> On Tue, Dec 7, 2010 at 8:40 PM, Francis Joanis
> >>> >> >> >>>> <francis.joanis at gmail.com> wrote:
> >>> >> >> >>>>>
> >>> >> >> >>>>> Hi guys,
> >>> >> >> >>>>> I have a question about sending multiple NIT messages
> >>> >> >> >>>>> logically
> >>> >> >> >>>>> "at
> >>> >> >> >>>>> the
> >>> >> >> >>>>> same time" (with DUM). I do know that the current code
> >>> >> >> >>>>> doesn't
> >>> >> >> >>>>> allow
> >>> >> >> >>>>> for
> >>> >> >> >>>>> real parallel NIT requests (i.e. the NIT queue in
> >>> >> >> >>>>> InviteSession)
> >>> >> >> >>>>> -
> >>> >> >> >>>>> but
> >>> >> >> >>>>> that's not my issue.
> >>> >> >> >>>>> The issue I have is regarding how to match incoming INFO
> >>> >> >> >>>>> responses
> >>> >> >> >>>>> to
> >>> >> >> >>>>> their request. Imagine I need to send INFO messages
> regarding
> >>> >> >> >>>>> application-specific transactions (like DB transactions or
> >>> >> >> >>>>> whatever
> >>> >> >> >>>>> application specific thing like a button click). I'll call
> >>> >> >> >>>>> those
> >>> >> >> >>>>> non-SIP
> >>> >> >> >>>>> transaction ids "keys".
> >>> >> >> >>>>> If I can't have the keys passed into the INFO body and then
> >>> >> >> >>>>> have
> >>> >> >> >>>>> them
> >>> >> >> >>>>> resupplied back into the INFO response, there is no easy
> way
> >>> >> >> >>>>> of
> >>> >> >> >>>>> matching
> >>> >> >> >>>>> which response is for which request (especially if my
> >>> >> >> >>>>> application
> >>> >> >> >>>>> would
> >>> >> >> >>>>> allow multiple requests to be sent (queued in NIT queue) at
> >>> >> >> >>>>> the
> >>> >> >> >>>>> same
> >>> >> >> >>>>> time).
> >>> >> >> >>>>> The "workaround" would be to leverage the fact that NITs in
> >>> >> >> >>>>> an
> >>> >> >> >>>>> INVITE
> >>> >> >> >>>>> dialog are all serialized: I could keep a separate list of
> >>> >> >> >>>>> "keys".
> >>> >> >> >>>>> If I were
> >>> >> >> >>>>> to add my key to the list right before sending the INFO
> >>> >> >> >>>>> message,
> >>> >> >> >>>>> I
> >>> >> >> >>>>> would
> >>> >> >> >>>>> then know that the next INFO response will relate to that
> key
> >>> >> >> >>>>> (oldest
> >>> >> >> >>>>> element of the key list). But this can get messy since I
> >>> >> >> >>>>> would
> >>> >> >> >>>>> need
> >>> >> >> >>>>> to make
> >>> >> >> >>>>> sure that the "key list" is properly managed.
> >>> >> >> >>>>
> >>> >> >> >>>> [Scott]  This solution works without having to modify DUM,
> and
> >>> >> >> >>>> its
> >>> >> >> >>>> likely very similar to how it would be implemented in DUM if
> >>> >> >> >>>> we
> >>> >> >> >>>> added
> >>> >> >> >>>> some
> >>> >> >> >>>> kind of key/id to the info() call and response callback.
> >>> >> >> >>>>  Alternatively, you
> >>> >> >> >>>> could track the transaction id's as the INFO messages flow
> out
> >>> >> >> >>>> of
> >>> >> >> >>>> the
> >>> >> >> >>>> invite
> >>> >> >> >>>> session by watching the onReadyToSend callback, but I don't
> >>> >> >> >>>> think
> >>> >> >> >>>> that
> >>> >> >> >>>> offers any advantage over assuming the requests are
> >>> >> >> >>>> serialized.
> >>> >> >> >>>>
> >>> >> >> >>>>>
> >>> >> >> >>>>> If I look at the similar scenario but with INVITE messages,
> >>> >> >> >>>>> one
> >>> >> >> >>>>> can
> >>> >> >> >>>>> easily leverage the AppDialogSet/... to do this: set the
> >>> >> >> >>>>> "key" on
> >>> >> >> >>>>> the
> >>> >> >> >>>>> AppDialogSet and then it will be easily accessible once
> >>> >> >> >>>>> INVITE
> >>> >> >> >>>>> responses
> >>> >> >> >>>>> arrive (extracted from the INVITE session).
> >>> >> >> >>>>> I was thinking of extracting the id of the SIP transaction
> >>> >> >> >>>>> created
> >>> >> >> >>>>> for
> >>> >> >> >>>>> the INFO to use it for a lookup between the SIP transaction
> >>> >> >> >>>>> id
> >>> >> >> >>>>> and
> >>> >> >> >>>>> my "key".
> >>> >> >> >>>>> That way, when handling onInfo* callbacks I would be able
> to
> >>> >> >> >>>>> access
> >>> >> >> >>>>> the SIP
> >>> >> >> >>>>> transaction id from the response message and map it with my
> >>> >> >> >>>>> "key".
> >>> >> >> >>>>> However, this might not be feasible if the SIP transaction
> id
> >>> >> >> >>>>> is
> >>> >> >> >>>>> unknown to the DUM at this time (prior to sending it).
> >>> >> >> >>>>> What do you make out of this?
> >>> >> >> >>>>> I suspect this will get worst if reSIProcate ever allows
> >>> >> >> >>>>> multiple
> >>> >> >> >>>>> NITs
> >>> >> >> >>>>> in parallel at the same time (in an INVITE dialog), since
> >>> >> >> >>>>> then
> >>> >> >> >>>>> there
> >>> >> >> >>>>> would
> >>> >> >> >>>>> really be no easy way of matching them up to application
> >>> >> >> >>>>> specific
> >>> >> >> >>>>> data (or
> >>> >> >> >>>>> am I asking for an AppTransaction class ;)?)
> >>> >> >> >>>>
> >>> >> >> >>>> [Scott] Not sure we would ever allow this - I remember this
> >>> >> >> >>>> being
> >>> >> >> >>>> discouraged by the IETF.
> >>> >> >> >>>>
> >>> >> >> >>>>>
> >>> >> >> >>>>> Thanks a lot,
> >>> >> >> >>>>> Francis
> >>> >> >> >>>>> _______________________________________________
> >>> >> >> >>>>> resiprocate-devel mailing list
> >>> >> >> >>>>> resiprocate-devel at resiprocate.org
> >>> >> >> >>>>>
> >>> >> >> >>>>>
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
> >>> >> >> >>>>
> >>> >> >> >>>
> >>> >> >> >>
> >>> >> >> >
> >>> >> >> >
> >>> >> >> > _______________________________________________
> >>> >> >> > resiprocate-devel mailing list
> >>> >> >> > resiprocate-devel at resiprocate.org
> >>> >> >> >
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
> >>> >> >> >
> >>> >> >
> >>> >> >
> >>> >>
> >>> >> _______________________________________________
> >>> >> resiprocate-devel mailing list
> >>> >> resiprocate-devel at resiprocate.org
> >>> >> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
> >>> >
> >>> >
> >>>
> >>> _______________________________________________
> >>> resiprocate-devel mailing list
> >>> resiprocate-devel at resiprocate.org
> >>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
> >>
> >>
> >> _______________________________________________
> >> resiprocate-devel mailing list
> >> 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/20101209/13fe364b/attachment.htm>