Skip to content

Commit 0f7168f

Browse files
author
Ole John Aske
committed
Bug#34854869 Incorrect handling of SSL_WANT_WRITE in ssl_writev()
ssl_writev() behaved incorrectly if it did only a partial write of the buffered data it was supposed to write. The Transporter layers protocol is such that a partial write should not be retried internaly in the write implementations. Instead the 'written' bytes should be returned to the upper layers which does the book keeping of the buffered data for each transporter, which transporter having buffered data to read/write, balance the read/write handling over the nodes/transporters, as well as handling the errors/exception codes returned from the transportes - deciding if the errors are retriable, or if the connection need to be dropped. NdbSocket::ssl_writev() did its own send-retries in case of partial writes, which breaks the above protocol. More severe; ssl_writev() made the incorrect assumption that all data supplied to ssl_send() / send_several_iov() were actually sent. Thus the iteration over the 'n' iovec[] were incorrectly maintained. Patch removes the write-looping in ssl_writev() in case of partial writes. The problem with maintaining sent iovec[] data goes away as a side effect. (Upper transporter layers maintains it and calls back with updated data pointers and length.) Change-Id: If55afbeee72d747dc52cf779fdde5bd3a381352a
1 parent 9c2ed80 commit 0f7168f

File tree

1 file changed

+12
-25
lines changed

1 file changed

+12
-25
lines changed

storage/ndb/src/common/util/NdbSocket.cpp

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -318,34 +318,21 @@ int NdbSocket::consolidate(const struct iovec *vec,
318318

319319
ssize_t NdbSocket::ssl_writev(const struct iovec *vec, int nvec) const
320320
{
321-
while(nvec > 0 && vec->iov_len == 0) {
322-
vec++;
323-
nvec--;
321+
if (unlikely(nvec <= 0)) return 0;
322+
323+
int sent;
324+
const int n = consolidate(vec, nvec);
325+
if(n > 1) {
326+
sent = send_several_iov(vec, n);
327+
} else {
328+
sent = ssl_send((const char *) vec[0].iov_base, vec[0].iov_len);
324329
}
325-
326-
ssize_t total = 0;
327-
while(nvec > 0) {
328-
int sent;
329-
int n = consolidate(vec, nvec);
330-
if(n > 1) {
331-
sent = send_several_iov(vec, n);
332-
} else {
333-
sent = ssl_send((const char *) vec[0].iov_base, vec[0].iov_len);
334-
}
335-
336-
if(sent > 0) {
337-
vec += n;
338-
nvec -= n;
339-
total += sent;
340-
} else if(total > 0) {
341-
break; // return total bytes sent prior to error
342-
} else {
343-
return sent; // no data has been sent; return error code
344-
}
345-
}
346-
return total;
330+
// Note that handling of partial sends is the responsibility of
331+
// upper transporter levels -> We should not retry here!
332+
return sent;
347333
}
348334

335+
/* Bundle several small iovecs into a single TLS record < 16 KB */
349336
int NdbSocket::send_several_iov(const struct iovec * vec, int n) const {
350337
size_t len = 0;
351338
char buff[MaxTlsRecord];

0 commit comments

Comments
 (0)