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

[Nikos Mavrogiannopoulos]

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 http://x2a.org/git/gnutls/?h=buffers-redux 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:

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

- 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).

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) {
                gnutls_free(cipher);
        }

and will allow the reuse of the already allocated data.

regards,
Nikos