[SCM] Samba Shared Repository - branch master updated - release-4-0-0alpha6-954-gbb7e6f0

[Stefan (metze) Metzmacher]

Hi Volker,

> On Wed, Feb 18, 2009 at 02:06:02PM +0100, Stefan (metze) Metzmacher wrote:
>> I'm in progress to fix the whole problem correctly, by fixing the socket
>> abstraction. The function socket_get_fd() is stupid and breaks the
>> abstraction.
>>
>> The caller should give a tevent_context to the socket layer and
>> callbacks for read and write, it's up to the socket layer how to trigger
>> them. The packet.c code will also be obsoleted by a generic
>> tsocket_send[to]_queue_send/recv() and tsocket_recv_pkt_send/recv() code.
> 
> While you're rewriting this: I'm not convinced that
> statically attaching an event context and fd event to a
> socket is necessarily a good idea. At least for me this has
> in the past caused some confusion about the code path when
> receiving something from the network. The way I've done it
> in s3 so far is a bit different: The socket knows nothing
> about event contexts, only requests trigger the creation of
> an fd event.

I don't think it's true that the socket knows nothing about
event contexts, at least the socket (within the kernel) knows
how to trigger events through the fd.

> I know that this makes for example an oplock
> break listener difficult, and you have to take care of
> queueing at a higher level,

Yes queuing at a higher level is a nice way to solve this.
I also think the higher level needs to make sure
that there's only one function that accesses the socket
(read and write access can be splitted). As it's the job of
the higher level to garantie that, I think the higher level
should also make sure that only one event context is used at a time
(which means that waiting for readable and waiting for writeable
need to be on the same event context).

But it doesn't mean that event context has to be the same over the
lifetime of the socket, the higher level can change it.

tsocket_set_event_context() will internally remove an existing fd event
and create a new one.

> but for me it is easier to
> understand than an event context attached to the socket.

I'm trying hard to implement almost the same logik you've used,
as I also like this style.

I identified this as your "socket api":

1.) create socket:
      async_sock: int socket(...)

      tsocket: int tsocket_context_create(&sock, ...)

2.) connect to an endpoint:
      async_sock: int connect(fd, ...)

      tsocket: int tsocket_connect(sock, ...)

3.) check if async connect has finished
      async_sock: int getsockopt(fd, SOL_SOCKET, SO_ERROR, ...)

      tsocket: int tsocket_get_status(sock, ...)

4.) write data into a socket:
      async_sock: ssize_t send(fd, buf, len, ...)

      tsocket: ssize_t tsocket_send(sock, buf, len, ...)

5.) read data from a socket:
      async_sock: ssize_t recv(fd, buf, len, ...)

      tsocket: ssize_t tsocket_recv(sock, buf, len, ....)

6.) close a socket:
      async_sock: close(fd)

      tsocket: tsocket_disconnect(sock)

7.) attach event context to the socket:
      async_sock:
         fde = tevent_add_fd(event_ctx, fd, 0,
                             read_write_handler, private, ...)
         (read_write_handler dispatches to read_handler
          and write_handler)

      tsocket:
         tsocket_set_event_context(sock, event_ctx, ...)

8.) wait to recv data
      async_sock:
         TEVENT_FD_READABLE(fde)
         TEVENT_FD_NOT_READABLE(fde)

      tsocket:
         tsocket_set_recv_handler(sock, read_handler, private)
         tsocket_set_recv_handler(sock, NULL, NULL)

9.) wait to send data
      async_sock:
         TEVENT_FD_WRITEABLE(fde)
         TEVENT_FD_NOT_WRITEABLE(fde)

      tsocket:
         tsocket_set_send_handler(sock, write_handler, private)
         tsocket_set_send_handler(sock, NULL, NULL)


The thing I don't like about the async_sock event handling is,
that caller can setup multiple fd event handlers at the same time.
As we agree that the higher level will take care of it, it's
ok socket abstraction doesn't provide the "feature" to register multiple
event handlers.

In order to hide SASL, TLS/SSL, named pipes and other virtual sockets
behind this abstraction, we need to hide the fd (behind tsocket_context).

My tsocket_send_buf_send/recv() will match your sendall_send/recv()
functions. And tsocket_send_buf_queue_send/recv() will be based on top
of them.

The same will apply for tsocket_recv_pkt_send/recv() and your
read_pkt_send/recv().

metze




-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 252 bytes
Desc: OpenPGP digital signature
Url : http://lists.samba.org/archive/samba-technical/attachments/20090218/7409b67f/signature.bin