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

Re: [reSIProcate] Fwd: [reSIProcate-commit] COMMIT: resiprocate 6128moetje: Added STUN client support


I had been looking at this, too, but got totally distracted by IETF along the way.

Actually, I don't understand what the lock is doing here.  There's a general issue, though, that this interface only supports a single outstanding stun request, which really isn't going to be sufficient for more complicated UAC/UAS code.   So if there's a lock, it actually needs to be in place to prevent a second call to stunSendTest from returning until stunResult has completed successfully.  But I'm pretty sure we don't want to do that, either.

A few thoughts on this:
- Primarily, have you taken a look at rfc3489bis-03 or recent drafts? The usage has changed a bit.  I have a student who's working on implementing the new version, and we hope to be able to contribute that in the near future.
- One of the key behaviors needed is the ability to issue a binding query/connectivity check to any arbitrary destination, and in fact there may be multiple outstanding queries.  This implies that the stun request needs to probably return a handle or require a conventional event handler so that replies can invoke the handler with a handle to match the specific stun request.
- We should make sure that whatever the design is, it provides the services necessary to higher layers can implement ICE on top of this.


possible interface at the dum level (very rough):

StunMessage DialogUsageManager::makeStunMessage(Transport& t, const Tuple& dest, StunRequestHandle& h,

(I'm not sure what goes here.  A simple interface would be an enum of the stun usages defined in sec 12 of 3489bis to fill in the values for the typical usages. )

DialogUsageManager::send(StunMessage& )

STUNResponseHandler::onStunResponse(StunRequestHandle& h,

STUNResponseHandler::onStunFailure(h)

I like the ability to create a StunMessage the same way a SipMessage is created for something like Register, which allows either a simple call to create the standard STUN messages, but a more advanced use can be made by manipulating the StunMessage directly.  That way if someone wants to manipulate the message directly and use things like response-address that have no official use, they can.

(I realize there's still the issue of the exposed transport in the call above.  I'm not sure what the right answer is for solving that.)

There's also an interesting question of whether we would want to implement KeepAlive messages using the same interface.

Bruce



On 4/9/06, Matthias Moetje - TERASENS GmbH <moetje@xxxxxxxxxxxx> wrote:
Jason,
 
I had thought about both issues before and I had asked for comments
on this before I commited these changes.
 
Regarding the transport pointer: As long as you know that the lifetime
of this pointer is identical to that of the stack, nothing bad will ever
happen. Looking at the complexity of the stack and the expertise
required to find out how it is supposed to work and how to use it,
I'm not sure if especially this part would need to be made that "safe"
because low-skilled people will probably never be able to use the
stack anyway.
If someone creates his own transport class we already have a similar
situation: the custom transport's lifetime is controlled from outside
of the stack.
The only alternative that came to my mind was to send STUN messages
all the way through the stack same like SIP messages, but I'm not
sure if the amount of changes required to make this work really pays.
Of course I'm open for other alternatives!
 
 
Regarding the mutex: If we assume that each time the "process"
method is called, a single SIP message is processed, and if we compare
the amount of processing that is done on each SIP message, I don't
think that switching the mutex affects the processing time in a significant
way. Perhaps we could avoid the Lock object creation by switching
the mutex on and off manually?
 

Best regards,

Matthias Moetje



From: resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Jason Fischl
Sent: Sunday, April 09, 2006 11:48 PM
To: resiprocate
Subject: [reSIProcate] Fwd: [reSIProcate-commit] COMMIT: resiprocate 6128moetje: Added STUN client support

I'd like to propose that we reconsider this particular interface change. It has a few implications that I am not sure are consistent with our design goals:

- exposes a pointer to a data structure that is not meant to be shared with the application
- requires a mutex to be used in every process call for the UdpTransport

I think we need to consider alternatives and am willing to spend some time this week to have a conference call to review alternatives.

Thanks,
Jason


---------- Forwarded message ----------
From: svn@xxxxxxxxxxxxxx < svn@xxxxxxxxxxxxxx>
Date: Apr 9, 2006 10:20 AM
Subject: [reSIProcate-commit] COMMIT: resiprocate 6128 moetje: Added STUN client support
To: resiprocate-commit@xxxxxxxxxxxxxxxxxxx

Project resiprocate
New Revision 6128
Committer moetje (Matthias Moetje)
Date 2006-04-09 10:20:40 -0700 (Sun, 09 Apr 2006)

Log

 Added STUN client support


Modified:


_______________________________________________
resiprocate-commit mailing list
resiprocate-commit@xxxxxxxxxxxxxxxxxxx
https://list.sipfoundry.org/mailman/listinfo/resiprocate-commit



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