Hi Razvan!

I understand it’s just the beginning, hence my comments, targeted at 
(hopefully) improving things :-)

>> - If TLS is taken out of the core it would be best to rewrite it without 
>> looking at the current code. If we want to get OpenSIPS as an official 
>> Debian package, the TLS code needs to be a separate module, with a different 
>> license (GPL with the OpenSSL exception thing), but in order to relicense it 
>> all developers have to agree, doing it from scratch would avoid this.
> I'm not sure how rewriting the whole module will solve this. Even though it's 
> a different code, it will still have the OpenSSL exception so it can't be 
> part of a Debian package. However, this time we will be able to add the 
> OpenSIPS core in the Debian repo.

It would solve the problem. Having the exception if fine, but if TLS is in the 
core, OpenSIPS would need to be relicensed in its entirety, but having it as a 
module, the exception would only apply to that module.

>> 
>> - If TLS is a module now, has there been any thought on supporting 
>> implementations other than OpenSSL? gnuTLS and libreSSL come to mind.
> Since the new interface will be very modular, having a different TLS 
> implementation will be very simple. All they have to do is to implement the 
> Transport Interface functions.
>> 

Awesome!

>> - I see that the send / receive API is not designed with scatter / gather 
>> I/O in mind. It might be beneficial in the long run to support sending or 
>> receiving an array of buffers, and use readv / writev underneath.
> This would be a nice feature, but the current code is not yet ready to 
> support this. But we should keep track of this for further releases.
>> 

While the current code might not *use* it, I think allowing for it at the API 
level would be nice to have, it basically revolves around using the vectored 
functions if the number of buffers is > 1.

>> - sockaddr_union is defined by OpenSIPS with no real purpose these days, 
>> since we can use sockaddr_storage for allocating space and sockaddr for 
>> passing around pointers.
> Currently sockaddr_union is not only used for storage, but is also allows you 
> to access the structure as sockaddr or sockaddr_in without doing any explicit 
> casts.

So? Is casting to struct sockaddr* in a few places necessarily wrong?

>> 
>> - At what level are the TLS settings considered? Network or transport? 
>> Either way, all init functions get void, but where will the config be taken 
>> from, if it’s different per domain?
> The TLS settings will be considered at the transport level. We could 
> implement the domain logic in the mod_init() function, that is not part of 
> the Transport Interface but of the general Modules Interface.
>> 

Ok.

>> - The transport API has some inconsistencies in what parameters it takes: 
>> connect gets a sockaddr, but bind gets host and port.
> Well, this is not the final version of the API, and most likely during 
> development some of those signatures will change. What we have there is a 
> roughly design we think should be used to implement this interface.But 
> indeed, during development we'll pay attention to these inconsitencies and 
> try to avoid them.
>> 
>> - Where do DNS lookups happen? If the transport API only knows about IPs and 
>> ports then all functions should take sockaddr structures and the app would 
>> do the lookup.
> The DNS lookup is already done on a higher level, by the OpenSIPS Core. At 
> this level we have the IP and ports already resolved.
>> 

Cool. In that case I suggest the transport API just gets sockaddr parameters 
and the conversion be done elsewhere. consistency++ :-)

>> - Any thoughts on using “connected” UDP sockets, so the kernel does the 
>> filtering for incoming traffic?
> We haven't considered this and I am not sure how this would should work. 
> Currently OpenSIPS only has a single socket per UDP listening interface. 
> Using "connected" UDP sockets would require having multiple sockets, each 
> connected to an "authorized" client. Therefore we'd have to implement a 
> socket management logic in the UDP transport interface to handle this. This 
> will increase the complexity of the UDP communication and most likely 
> decrease it's performance.
> 

I didn’t think this all the way through, but the advantage would be that the 
kernel does the filtering. It’s not a big deal anyway, just checking if you 
folks gave the idea any thought.


Keep up the good work!

--
Saúl Ibarra Corretgé
AG Projects



Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

_______________________________________________
Devel mailing list
Devel@lists.opensips.org
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel

Reply via email to