On Tue, Aug 2, 2016 at 5:00 AM, Carsten Haitzler <ras...@rasterman.com> wrote:
> On Mon, 1 Aug 2016 00:50:31 -0300 Gustavo Sverzut Barbieri 
> <barbi...@gmail.com>
[...]
>> struct Efl.Net.Address {
>>     family: Efl.Net.Family;
>>     address: union { in, in6, bt... };
>> }
>
> are you sure this shouldn't just be a string above for address? and as onefang
> said - the string can tell us what we need. it can just be a domain name of an
> ipv4 addr or an ipv6 addr and so on... ? the actual representation like this
> should be internal and not at any API level. at an API return a stringified
> thing. set the destination address, get it back as-is with a string name.
> request a result and once resolve is done (callback or promise) you can get 
> the
> string resolution of the address. eg ipv4 addr string or ipv6 addr string for
> example. this just de-complicates API across bindings to not need a union (a
> bit of an ugly issue) and who really deals with addresses at this 
> format/level?

No hard opinion on string versus struct. The good part of strings is
that it's easy to extend without going into unions and the likes. But
it doesn't allow compile-time checks and simple switch/if on the
family (which can be solved by using some functions to parse or check)

If you think a string is good to go, I'll change to that.



>> struct Eina.Blob {
[...]
> this is a big problem for bindings. a big fat "no - find another way". no
> function pointers in structs or parameters to methods etc. no void *'s
> (reserved for base eo classes only). use eina_binbuf - a list/array or chain 
> of
> binbufs or perhaps something else.

see my other emails, the idea is to allow different "free" for
different kinds of memory (ie: mmap) and windowing/slicing (ie: line
of a file).

It's not a hard requirement either, just my efficiency advice after
invested lots of time to test this use case with Soletta.


>>                 @out error: int; [[0 on success, ENOSPC if no space left]]
>>             }
>
> why not return error? why have it as an @out param? why not just return a bool
> and be done with it?

he, because I forgot about it :-D will be fixed for v2, thanks!


failure here can ONLY mean the inability to store the
> binbuf in a list internally or the object is totally invalid. either way out 
> of
> memory or another pretty fatal condition for this connection object.





>>         flush @virtual_pure {
[...]
> return bool value please - much better that @out param.

sure! fixed :-)


>>         @property local_address {
>>             [[the local IP or unix-local address]]
>>             get @virtual_pure {
>>             }
>>             values {
>>                 address: Efl.Net.Address;
>>             }
>>         }
>
> naming: address_local

ouch! keep an eye on that for me, in Soletta we changed the order and
it will bite me for sure ;-) Fixed.


> what local address is this exactly? if tis a server i'ts the address being
> listened on? the ip address (ipv4/6/whatever) of the outgoing interface or
> something?

local: getsockname()
remote: getpeername()


>>         @property remote_address {
>>             [[the remote IP or unix-local address]]
>>             get @virtual_pure {
>>             }
>>             values {
>>                 address: Efl.Net.Address;
>>             }
>>         }
>
> naming: address_remote
>
> and remote - this is the address you probably want to SET right so its set {}
> AND get {}. the get gets what you set. now i mention - what about a
> address_resolved_remote property you can just get {} only that tells you the
> resolving of the name or whatever specified by setting the address_remote
> property.

As per above, it would be the already resolved and actually connected
name, it's in the base class (Efl.Net.BaseSocket), so also means a
socket from a connecting/incoming client. Thus the "address_remote"
like that is consistent.

However I see that for connect-to-server role
(ecore_con_server_connect() and current Efl.Base.Socket, the given
parameter (ie: address_destination? address_connect?) could be
remembered for reference purposes. Example

        @property address_connected {
            [[The address used to create and connect this socket. It
            is the unresolved address, see the property address_remote
            for the actual resolved address]]
            get @virtual_pure {
            }
            values {
                address: Efl.Net.Address;
            }
        }


> how do we deal with resolving? a promise? an event callback? we have
> multiple stages here. first a resolve (async) then a connect (async) then
> finally we are established.

In the current proposal it's not there, it would be another class that
resolves the string to an Efl.Net.Address and that could be with a
promise.

That could be more cumbersome to use, but would remember people to
cache the resolved address.

Node.js and Qt both use an event to notify they were resolved.



>>         @property internal_fd {
>>             [[the internal file descriptor (use with caution!)]]
>>             /* it seems the usage is to steal the fd to do something
>>              * else or set some flags. If so we could expose some
>>              * primitives such as fcntl_add(type, flags) and
>>              * fcntl_del(type, flags) as well as a destructor that
>>              * will return the stolen fd (destroys the object and
>>              * keeps fd alive).
>>              */
>>             get @virtual_pure {
>>             }
>>             values {
>>                 fd: int;
>>             }
>>         }
>
> this has issues possibly on windows. fd's don't quote behave the same way 
> here.
> can we avoid this? if we need to do deep down ugly things with the fd... can 
> we
> just support them with the API? like fd passing on local unix sockets. have 
> our
> api deal with sending and getting fd's as a specific send method and an input
> event. i get why you have this - as a "way out" for when we don't support
> something. this is kind of bad for bindings though and portability. :(

don't get me wrong, I tried to justify to not have it there!

I just put it here because Ecore_Con.h provides that and from my
investigation, empc is the only user (gets the connected fd and
handles the socket to mpd library:

https://git.enlightenment.org/apps/empc.git/tree/src/bin/empdd.c#n828

Then in this case we could provide a way to steal the fd and delete
the client, keeping the fd alive.


>>         @property timeout { /* this usually doesn't change,
>>                                should be constructor-only */
>>             [[timeout to close the connection.]]
>>             values {
>>                 timeout: double;
>>             }
>>         }
>
> i see no reason that this can't change. a timeout for a connect or a timeout
> for buffers being able to send ANYTHING (if buffers can't drain for writes for
> more than N sec - consider this connection dead? should they be the same 
> timer?)

I still have to see when these would be dynamically changed in real
usages. Usually they are a constant, eventually user configurable but
you could apply to new sockets only.

The benefit of never changing is that your code will be reduced to a
simple ecore_timer_reset() instead of the slightly more complex
_ecore_con_server_timer_update():

https://git.enlightenment.org/core/efl.git/tree/src/lib/ecore_con/ecore_con.c#n1456



>>         @property uptime { /* is this really required?
>>                               currently unused in our code!
>>                               Could be done outside with ease */
>>             [[the time since the connection was established]]
>>             get @virtual_pure {
>>             }
>>             values {
>>                 uptime: double;
>>             }
>>         }
>>     }
>> }
>
> i see very little value in the connection tracking it's own uptime at this
> point. it can be done some time in the future if people really want it.

great, so will remove :-)


>> class Efl.Net.Socket (Efl.Net.BaseSocket) {
>>     [[This is a client socket that is created and connected to a
>>       specific server given to the constructor]]
>>
>>     methods {
>>         connect {
>>             [[The constructor that connects the socket to a given server]]
>>             params {
>>                 @in type: Efl.Net.Socket.Type;
>>                 @in address: Efl.Net.SocketAddress;
>>                 @in timeout: double;
>>             }
>>         }
>>     }
>
> as people mentioned. we could have type implicit in socketaddress with 
> strings.
> but let's talk constructors. shouldn't we just have connect() take no params
> and use the currently set up remote address, timeout and other parameters at
> the time? this works well with eo's finalize constructing mechanisms. these
> constructors atm are only really useful for c++ even then we'r elooking at
> using lambdas for construction calling methods.

could be, but is there a way to limit the "set" method of a property
to be only usable in construction time (ie: before finalize)?

like resetting the address... once the connection is done, this
operation is not supported.


>> class Efl.Net.Server (Eo.Base) {
>>     events {
>>         connection @hot: Efl.Net.BaseSocket; [[new connection was accepted]]
>>         error: int; [[errno]]
>>         /* no disconnect, listen on each individual connection */
>>     }
>
> i really don't like error as an int. really don't. a proper enum and/or struct
> with fields like enum for error type and maybe a string for error extra info
> (human readable from perror() style?)

is Eina_Error ok?


> what about disconnects? you expect peolpe to listen on del events on the 
> connection objects?

yes, IMO makes more sense unless there is a common pattern to not do so.


>>         @property connections {
>>             [[active connections]]
>>             /* Needed?
>>              *
>>              * actual members or just a counter? It seems all others
>>              * do not keep a list and we should be careful if
>>              * iterating over the list happens to modify it.
>>              *
>>              * count would be useful to gracefully quit if no
>>              * connections, but is easily done elswhere.
>>              *
>>              * the only user in our repo is eeze_scanner.c that uses
>>              * to send a single eet to all connections. Also easily
>>              * done at its own side (actually it already stores the
>>              * clients in a hash, could foreach() that one).
>>              */
>>         }
>
> how about return an eina iterator for connections that will list children. 
> that
> would help and not define the storage type. i think its good to have this in
> some form, but best might be an iterator.

my worry is that it can be tricky if people may change the storage
during the iteration, such as loop in a list of connections and you
can delete the next element.

It's easy to create your own list if you wish, by listening to
"connection" event and append to eina_list.

The only user I found (eeze_scanner) already have a hash, so could do
a foreach() there.


> i'm going to skip the analysis below... and focus on...
>
> 1. missing anything to do with SSL. events for certificates, validity, being
> able to register certs, verify etc. etc. etc. you touch on this in analysis of
> ecore_con server/client.

it's written on the first part of the email :-) I'll come to this, but
let's first get the basics right and the SSL must follow the pattern.


> 2. no way to request a resolve FIRST separately to a connect. this would be
> good i think (a connect implies a resolve then connect if not already 
> resolved,
> but allow a stand-alone resolve to then verify results before doing a 
> connect).

indeed I forgot to write that, but as stated above it would be another
function that does string -> Efl.Net.Address.

If the methods and properties should move to strings, then a way to
set the exact resolved address could be used, as well as an event to
notify string was resolved. An explicit resolver (like the one in
Golang) could always be of use.



> 3. addresses i think should be just strings. we could then overload this to do
> things like websockets just by a special address format (ie url) and the
> details are hidden

ok, I can change if others want that as well.


> 4. how do you disconnect a client? del the client object you see in the event
> info?

yes. I see that other tools offer some "graceful shutdown" and
"immediate abort". The graceful shutdown will stop reading and will do
the actual close once all pending data is sent. The immediate abort
will ignore pending stuff.

we can do 2 distinct methods if you wish, or we can let the user deal
with graceful shutdown on his own by using the events (or promise as
cedric wants).


> 5. i think controls on max number of clients would be nice so it auto drops
> clients without you bothering to listen for the connect cb's then disconnect
> them by deleting the objects.

You did skip the constructor method, it does contain:

                @in client_limit: int;
                @in reject_excess_clients: bool;

:-)



> 6. have you considered freezing or thawing clients so write buffers are frozen
> (and input buffers are ignored) while frozen? this would be like pause/resume
> in node.js

could add, but currently we do not provide that, thus no users on our codebase.



> 7. missing things like tcp keepalive and nodelay.

also no users in our code base. We could add these to some "flags" to
be given to the constructor. Actually NODELAY is currently implemented
like that, it's a Ecore_Con_Type.


> 8. what about udp?

You mean sendmsg()/sendto()?

For other tools they all offer different classes with specific
methods, thus we could create:

Efl.Net.UDPSocket interface that implements
sendto/sendmsg/recvfrom/recvmsg. We could map to regular events by
taking and sending the whole datagram in/from data.


> 9. no way to GET the current buffer status of a connection you made to a
> server. e.g. how much data is buffers and not sent yet (not sent to kernel).

indeed, you'd have to keep that information elsewhere. But it's a
simple getter, so we can add if wanted.

what name to use?

 - pending_send_bytes
 - send_buffer_usage


> 10. perhaps add a drain event to know when all write buffers are finished 
> being
> written to kernel? or maybe sent out of interface to other end?

added


> 11. should we consider things like socks proxies - how they work with this?

Qt sets a proxy object to socket. I did not check their
implementation. Others do not deal with that in their API.

Node.js does for http/https using an "agent" option:
https://github.com/TooTallNate/node-socks-proxy-agent

Go does http/https using a "transport" option and for regular SOCKS
they use another class https://godoc.org/golang.org/x/net/proxy#Dialer
(analogous to our ecore_con_server_connect()).



> 12. what about api's so do things like fd pass (and receive), do things like
> get the uid/pid and other process unfo or the other end of the connection of a
> unix socket?

These should be an extra interface that only the UnixSocket would
provide, they would do things like SCM_CREDENTIALS.



> what should we do about things like mqtt, mq/zeromq, ocf/oic - they belong in
> another interface layer? http is a bit different too due to its nature.
> websockets i can see fitting underneath the above, but not real http itself

mqtt and the others are all PITA to get all the quirks right... pretty
much like http they look simple to implement the basics, but then the
other peers rely on bug-compatibility with mainstream projects such as
mosquitto, iotivity... So implementing on top of pure socket is a
major work, would be simpler to just do bindings for "de facto
standard" libraries.



-- 
Gustavo Sverzut Barbieri
--------------------------------------
Mobile: +55 (16) 99354-9890

------------------------------------------------------------------------------
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to