osmo_io: Change struct osmo_io_ops to contain struct, not union

As we introduce more modes, and each mode aliases call-back function
pointers to those of another mode, we have more and more error cases
where we (for exampele) access read_cb, but in reality the user has
populated recvfrom_cb.

Let's use a struct, meaning that call-backs of one mode no longer alias
to the same memory locations of call-backs fro another mode.  This
allows us to properly check if the user actually provided the right
callbacks for the given mode of the iofd.

This breaks ABI, but luckily not API.  So a simple recompile of
higher-layer library + application code will work.

Change-Id: I9d302df8d00369e7b30437a52deb205f75882be3
This commit is contained in:
Harald Welte 2024-02-23 16:08:49 +01:00
parent e8ab1b77d8
commit b365b1d094
4 changed files with 74 additions and 36 deletions

View File

@ -12,6 +12,8 @@ core ADD osmo_sock_multiaddr_get_ip_and_port(), osmo_multiaddr_ip_and
core ADD osmo_sock_sctp_get_peer_addr_info()
core ADD gsmtap_inst_fd2() core, DEPRECATE gsmtap_inst_fd()
core behavior change osmo_tdef_fsm_inst_state_chg(): allow millisecond precision
core ABI change osmo_io_ops now contains a struct of structs, not union of structs
core ABI change osmo_iofd_set_ioops() now returns a value (error code)
isdn ABI change add states and flags for external T200 handling
isdn ADD initial implementation of the V.110 Terminal Adapter
gsm ABI change add T200 timer states to lapdm_datalink

View File

@ -35,37 +35,35 @@ static inline const char *osmo_io_backend_name(enum osmo_io_backend val)
{ return get_value_string(osmo_io_backend_names, val); }
struct osmo_io_ops {
union {
/* mode OSMO_IO_FD_MODE_READ_WRITE: */
struct {
/*! call-back function when something was read from fd */
void (*read_cb)(struct osmo_io_fd *iofd, int res, struct msgb *msg);
/*! call-back function when write has completed on fd */
void (*write_cb)(struct osmo_io_fd *iofd, int res,
struct msgb *msg);
/*! 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.
* If a full message was received (segmentation_cb() returns a value <= msgb_length())
* the msgb will be trimmed to size by osmo_io and forwarded to the read call-back. Any
* parsing done to the msgb by segmentation_cb() will be preserved for the read_cb()
* (e.g. setting lxh or msgb->cb). */
int (*segmentation_cb)(struct msgb *msg);
};
/* mode OSMO_IO_FD_MODE_READ_WRITE: */
struct {
/*! call-back function when something was read from fd */
void (*read_cb)(struct osmo_io_fd *iofd, int res, struct msgb *msg);
/*! call-back function when write has completed on fd */
void (*write_cb)(struct osmo_io_fd *iofd, int res,
struct msgb *msg);
/*! 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.
* If a full message was received (segmentation_cb() returns a value <= msgb_length())
* the msgb will be trimmed to size by osmo_io and forwarded to the read call-back. Any
* parsing done to the msgb by segmentation_cb() will be preserved for the read_cb()
* (e.g. setting lxh or msgb->cb). */
int (*segmentation_cb)(struct msgb *msg);
};
/* mode OSMO_IO_FD_MODE_RECVFROM_SENDTO: */
struct {
/*! call-back function emulating recvfrom */
void (*recvfrom_cb)(struct osmo_io_fd *iofd, int res,
struct msgb *msg,
const struct osmo_sockaddr *saddr);
/*! call-back function emulating sendto */
void (*sendto_cb)(struct osmo_io_fd *iofd, int res,
struct msgb *msg,
const struct osmo_sockaddr *daddr);
};
/* mode OSMO_IO_FD_MODE_RECVFROM_SENDTO: */
struct {
/*! call-back function emulating recvfrom */
void (*recvfrom_cb)(struct osmo_io_fd *iofd, int res,
struct msgb *msg,
const struct osmo_sockaddr *saddr);
/*! call-back function emulating sendto */
void (*sendto_cb)(struct osmo_io_fd *iofd, int res,
struct msgb *msg,
const struct osmo_sockaddr *daddr);
};
};
@ -98,4 +96,4 @@ int osmo_iofd_get_fd(const struct osmo_io_fd *iofd);
const char *osmo_iofd_get_name(const struct osmo_io_fd *iofd);
void osmo_iofd_set_name(struct osmo_io_fd *iofd, const char *name);
void osmo_iofd_set_ioops(struct osmo_io_fd *iofd, const struct osmo_io_ops *ioops);
int osmo_iofd_set_ioops(struct osmo_io_fd *iofd, const struct osmo_io_ops *ioops);

View File

@ -306,6 +306,8 @@ void iofd_handle_segmented_read(struct osmo_io_fd *iofd, struct msgb *msg, int r
int res;
struct msgb *pending = NULL;
OSMO_ASSERT(iofd->mode == OSMO_IO_FD_MODE_READ_WRITE);
if (rc <= 0) {
iofd->io_ops.read_cb(iofd, rc, msg);
return;
@ -473,6 +475,24 @@ int osmo_iofd_sendto_msgb(struct osmo_io_fd *iofd, struct msgb *msg, int sendto_
return 0;
}
static int check_mode_callback_compat(enum osmo_io_fd_mode mode, const struct osmo_io_ops *ops)
{
switch (mode) {
case OSMO_IO_FD_MODE_READ_WRITE:
if (ops->recvfrom_cb || ops->sendto_cb)
return false;
break;
case OSMO_IO_FD_MODE_RECVFROM_SENDTO:
if (ops->read_cb || ops->write_cb)
return false;
break;
default:
break;
}
return true;
}
/*! Allocate and setup a new iofd.
* \param[in] ctx the parent context from which to allocate
* \param[in] fd the underlying system file descriptor
@ -496,6 +516,9 @@ struct osmo_io_fd *osmo_iofd_setup(const void *ctx, int fd, const char *name, en
return NULL;
}
if (!check_mode_callback_compat(mode, ioops))
return NULL;
iofd = talloc_zero(ctx, struct osmo_io_fd);
if (!iofd)
return NULL;
@ -543,8 +566,10 @@ int osmo_iofd_register(struct osmo_io_fd *iofd, int fd)
return rc;
IOFD_FLAG_UNSET(iofd, IOFD_FLAG_CLOSED);
if (iofd->io_ops.read_cb)
if ((iofd->mode == OSMO_IO_FD_MODE_READ_WRITE && iofd->io_ops.read_cb) ||
(iofd->mode == OSMO_IO_FD_MODE_RECVFROM_SENDTO && iofd->io_ops.recvfrom_cb)) {
osmo_iofd_ops.read_enable(iofd);
}
if (iofd->tx_queue.current_length > 0)
osmo_iofd_ops.write_enable(iofd);
@ -722,8 +747,11 @@ void osmo_iofd_set_name(struct osmo_io_fd *iofd, const char *name)
/*! Set the osmo_io_ops for an iofd.
* \param[in] iofd Target iofd file descriptor
* \param[in] ioops osmo_io_ops structure to be set */
void osmo_iofd_set_ioops(struct osmo_io_fd *iofd, const struct osmo_io_ops *ioops)
int osmo_iofd_set_ioops(struct osmo_io_fd *iofd, const struct osmo_io_ops *ioops)
{
if (!check_mode_callback_compat(iofd->mode, ioops))
return -EINVAL;
iofd->io_ops = *ioops;
switch (iofd->mode) {
@ -743,6 +771,8 @@ void osmo_iofd_set_ioops(struct osmo_io_fd *iofd, const struct osmo_io_ops *ioop
default:
OSMO_ASSERT(0);
}
return 0;
}
/*! Notify the user if/when the socket is connected.

View File

@ -81,10 +81,18 @@ static void iofd_poll_ofd_cb_recvmsg_sendmsg(struct osmo_fd *ofd, unsigned int w
rc = sendmsg(ofd->fd, &msghdr->hdr, msghdr->flags);
iofd_handle_send_completion(iofd, rc, msghdr);
} else {
if (iofd->mode == OSMO_IO_FD_MODE_READ_WRITE)
/* Socket is writable, but we have no data to send. A non-blocking/async
connect() is signalled this way. */
/* Socket is writable, but we have no data to send. A non-blocking/async
connect() is signalled this way. */
switch (iofd->mode) {
case OSMO_IO_FD_MODE_READ_WRITE:
iofd->io_ops.write_cb(iofd, 0, NULL);
break;
case OSMO_IO_FD_MODE_RECVFROM_SENDTO:
iofd->io_ops.sendto_cb(iofd, 0, NULL, NULL);
break;
default:
break;
}
if (osmo_iofd_txqueue_len(iofd) == 0)
iofd_poll_ops.write_disable(iofd);
}