[Nice] New API and refactor
youness.alaoui at collabora.co.uk
Mon Jun 23 14:09:39 PDT 2008
Rémi Denis-Courmont wrote:
> Le Saturday 21 June 2008 00:13:59 ext Youness Alaoui, vous avez écrit :
>> Other changes include the fact that the NONCE/REALM/USERNAME attributes
>> should be added by the user of the library, and not by the library
>> itself.. the STUN module will only add the message-integrity and the
>> fingerprint attributes if necessary.
>> The main problem I see now is with the stun 'bind' and 'stun-ice'
>> usages.. those should not be in the stun module, they should be part of
>> the libnice core functions and should be a lot simpler.
> They used to be simple, until we found half a dozen different cases that need
> to be handled in different ways for different reasons. The "large" number of
> API mostly comes from the fact that we want a non-blocking API, so we have to
> expose the timer and the transaction state (completed, failed, resending,
> waiting for answer...) to the caller. In fact, the blocking API is way
> simpler... but it is obviously not usable within nice/
Yes, of course, the "should be simpler" wasn't much about the code
there, but actually about the API. the transport, the timers, etc..
those shouldn't be in the stun module. I probably should have explained
my views about this earlier.
I think the stun module should be as simple as possible, it should just
be a "create and parse stun messages" library.
The bind usage should be as simple as :
stun_agent_init_request(agent, msg, STUN_BINDING);
stun_agent_finish_message(agent, msg, NULL, 0);
stun_agent_validate(agent, msg, buf, buf_len, NULL, NULL);
if (stun_message_find_addr(msg, STUN_ATTRIBUTE_MAPPED_ADDRESS, &addr) !=
stun_message_find_addr(msg, STUN_ATTRIBUTE_XOR_MAPPED_ADDRESS, &addr);
That's it (a bit simplified of course), we don't really need much more
than that, the whole transport and timer, etc.. are not something that
should be part of a simple library like STUN, it's really the job of the
user of the lib.
The bind usage is useful here for the stunbdc tool and I think it should
be kept as part of that tool and that's it. Also, libnice doesn't seem
to use much of the bind stuff, since it does everything itself, so the
code is really only used by stunbdc.
About the ICE usage, all of that is not stun specific, it's really about
ICE and it's specific to libnice.. if anyone wanted to create an ICE
library and use our libstun, they will probably implement their own ICE
usage code.. Also, having the ice usage there is not something that
helps our modularity because the usages still have a lot of calls with
the compat argument for gtalk compatibility.
If we want ICE and bind usages, I suggest something as simple as this :
stun_usage_ice_create(agent, msg, buffer, buffer_len, use_candidate,
stun_usage_ice_parse(agent, msg, &use_candidate, &controlling, etc..)
stun_usage_bind_create(agent, msg, buffer, buffer_len)
stun_usage_bind_parse(agent, msg, &addr);
and let the library using those functions pass whatever argument they
want (with a NULL or '-1' if we don't want some attribute (controlling
for example) to be added to the message).
Simply helper functions for the usages instead of the overly complex
system we currently have.
> Then it is also true that, for instance, stun_conncheck_start has an horrific
> prototype. But unless we're to create even more APIs, or expose STUN message
> formatting upstream, we simply cannot remove them. In all three cases (more
> API, expose STUN messages format, change nothing), I'm afraid it will remain
> intricate because it intrinsically is.
Well, apart from its horrific prototype (which is ok when justified),
there's the fact that stun_conncheck_start is in bind.c and needs a
stun_bind_t context to work.. while stun_conncheck_reply is in
stun-ice.c and doesn't need it.. it's all very confusing.. is the ice
conncheck stuff a bind usage thing or an ice usage thing.. hehe..
(stun_conncheck_start adds ICE_CONTROLLING and other ice specific
> As for being part of the core - I have to disagree. Binding discovery is part
> of STUN. Logically: it avoids exposing the STUN message formats to ICE.
> Architecturally: it is part of the STUN spec, not the ICE one. In real life:
> it can be used independent of ICE. For testing purpose: it is a lot easier to
> test in-depth without booting the full ICE layers.
As I said before, the binding discovery is just a usage... STUN was
"Simple Traversal of UDP over NAT", but it was renamed as "Session
Traversal Utilities for NAT", so it's purpose is to be a utility, not do
the traversal itself.. anyways, that's just semantics. As I said before,
the binding usage is very simple, it's just two lines for sending, two
or three lines for receiving.
The bind stuff with transport and timer are useful only for stunbdc. I
had at some point moved those files into the tools/ directory and made
stunbdc link to their .o without them being in libstun.a... I also made
the test-bind link with the .o files..
In the latest revision, I've put the usages back into a usages/
directory to make it easier to port libnice to the new API.
> The STUN-ICE usage is a corner case. It is of course ICE-specific. We put it
> to the STUN part because it had a lot of common code with the binding
> discovery, and because we did not want to expose (as far as possible) the ICE
> agent to the STUN messages.
yeah, it's ICE specific and should be in libnice, at least, that's what
To summarize what I just said :
- the usages are just "how to use libstun" and should not be part of
- whoever wants to do binding requests, would only need a few lines of
code, so no need for the bind usage
- whoever wants to do ice will probably implement it himself instead of
using the ice usage since it's specific to libnice
- we could provide helper functions (1 for creating, 1 for parsing) for
Tell me what you think about all that I said, and if you share my thoughts.
If you're ok with it, I'll then proceed to remove the usages from
libstun and move/copy that code into libnice, while keeping the whole
bind usage stack for the stunbdc tool.
More information about the Nice