daniel has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32758 )

Change subject: osmo_io: Improve handling and documentation of segmentation_cb ......................................................................

osmo_io: Improve handling and documentation of segmentation_cb

The read length is not needed in the segmentation callback, msgb already has all the necessary information, the parameter previously was just msgb_length(msg).

Also handle negative return values (except -EAGAIN) of the callback as errors which cause the msg to be dropped. -EAGAIN will defer the msg.

Change-Id: I6a0eebb8d4490f09a3cc6eb97d4ff47b4a8fd377 --- M include/osmocom/core/osmo_io.h M src/core/osmo_io.c 2 files changed, 48 insertions(+), 14 deletions(-)

Approvals: daniel: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified

diff --git a/include/osmocom/core/osmo_io.h b/include/osmocom/core/osmo_io.h index c19ca67..d7402d6 100644 --- a/include/osmocom/core/osmo_io.h +++ b/include/osmocom/core/osmo_io.h @@ -42,8 +42,12 @@ /*! call-back function when write has completed on fd */ void (*write_cb)(struct osmo_io_fd *iofd, int res, const struct msgb *msg); - /*! call-back function to segment the data returned by read_cb */ - int (*segmentation_cb)(struct msgb *msg, int data_len); + /*! call-back function to segment the data at message boundaries. + * Needs to return the size of the next message. If it returns + * -EAGAIN or a value larger than msgb_length() (message is incomplete) + * osmo_io will wait for more data to be read. Other negative values + * cause the msg to be discarded. */ + int (*segmentation_cb)(struct msgb *msg); };

/* mode OSMO_IO_FD_MODE_RECVFROM_SENDTO: */ diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index b06e63c..857644d 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -219,7 +219,7 @@ */ static enum iofd_seg_act iofd_handle_segmentation(struct osmo_io_fd *iofd, struct msgb *msg, struct msgb **pending_out) { - int pending_len, msg_len; + int extra_len, msg_len; struct msgb *msg_pending;

msg_len = msgb_length(msg); @@ -229,27 +229,41 @@ return IOFD_SEG_ACT_HANDLE_ONE; }

- int len = iofd->io_ops.segmentation_cb(msg, msg_len); - - pending_len = msg_len - len; - /* No segmentation needed, return */ - if (pending_len == 0) { + int len = iofd->io_ops.segmentation_cb(msg); + if (len == -EAGAIN) { + goto defer; + } else if (len < 0) { + /* Something is wrong, skip this msgb */ + LOGPIO(iofd, LOGL_ERROR, "segmentation_cb returned error (%d), skipping msg of size %d\n", len, msg_len); *pending_out = NULL; - return IOFD_SEG_ACT_HANDLE_ONE; - } else if (pending_len < 0) { - *pending_out = msg; + msgb_free(msg); return IOFD_SEG_ACT_DEFER; }

- /* Copy the pending data over */ + extra_len = msg_len - len; + /* No segmentation needed, return the whole msgb */ + if (extra_len == 0) { + *pending_out = NULL; + return IOFD_SEG_ACT_HANDLE_ONE; + /* segment is incomplete */ + } else if (extra_len < 0) { + goto defer; + } + + /* msgb contains more than one segment */ + /* Copy the trailing data over */ msg_pending = iofd_msgb_alloc(iofd); - memcpy(msgb_data(msg_pending), msgb_data(msg) + len, pending_len); - msgb_put(msg_pending, pending_len); + memcpy(msgb_data(msg_pending), msgb_data(msg) + len, extra_len); + msgb_put(msg_pending, extra_len); *pending_out = msg_pending;

/* Trim the original msgb to size */ msgb_trim(msg, len); return IOFD_SEG_ACT_HANDLE_MORE; + +defer: + *pending_out = msg; + return IOFD_SEG_ACT_DEFER; }

/*! Restore message boundaries on read() and pass individual messages to the read callback