[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC] Suggestion for the improvement of the buffering layer

From: Jonathan Bastien-Filiatrault
Subject: Re: [RFC] Suggestion for the improvement of the buffering layer
Date: Sun, 09 Aug 2009 14:23:32 -0400
User-agent: Mozilla-Thunderbird (X11/20090103)

Nikos Mavrogiannopoulos wrote:
Jonathan Bastien-Filiatrault wrote:

Indeed but don't underestimate memcpy. Both for low latency and high
speed links (especially with crypto accelerators) that could be the
bottleneck. Keep that in mind in your design. In any case it doesn't
look easy task but that's always in the eye of the beholder.
Agreed. You can check for
preliminary work. I have squashed _gnutls_io_write_buffered and down to
less than a screenful each. The mbuffers code is now handling partial
socket writes without problem (and regressions). The code is still
missing some comments, but you should find it clear after a little
while. The main benefit at this point is that the buffer logic is
abstracted away in the mbuffers code and is kept out of the code that
uses mbuffers.

Your input is very important to me at this point.

Hello Jonathan,
 I like the overall change due to simplicity it introduces. Some comments:
Thanks !

- You could use const gnutls_datum* to pass datums that will not be
changed, just to save few bytes from stack.
No problem, I will do this.

- I don't like the malloc for each mbuffer_st being added. I'd suggest
to be avoided even if it results to some fixed number of mbuffers per
session (which seems quite realistic requirement).
So do we just allocate a series of maximum-sized buffers (2^14+2048 for regular TLS and ~2^16 for DTLS) and keep a free list ? Do we keep a free list and realloc if we need more memory ? I do not want to reimplement malloc... Is the added complexity worth it ? In the general case, mbuffer chains should not be very long...

Further I'd suggest to avoid the _gnutls_mbuffer_enqueue_copy() in
io_write_buffered() by passing a datum to the latter as well as flag on
whether this datum can be assigned without copy to an mbuffer. Thus in
gnutls_record.c you could convert
      ret = _gnutls_io_write_buffered (session, cipher, cipher_size);
      gnutls_free (cipher);
to something like:
        gnutls_datum tmp = {cipher, cipher_size};
        ret = _gnutls_io_write_buffered (session, &tmp, 1);
        if (ret < 0) {
Yes, I agree. I avoided this modification in order to keep the internal API constant for now. I will apply this modification.

and will allow the reuse of the already allocated data.


Thanks for your input,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]