Bug #6437
openosmo_io_ops segmentation_cb missing parameter holding state
90%
Description
struct osmo_io_ops { ... void (*read_cb)(struct osmo_io_fd *iofd, int res, struct msgb *msg); ... void (*write_cb)(struct osmo_io_fd *iofd, int res, struct msgb *msg); ... int (*segmentation_cb)(struct msgb *msg);
The segmentation_cb is missing the "struct osmo_io_fd *iofd" in front. This puts several limitations for future users of the cb API, since there's no way to access related objects like iofd (private_nr, data ptr, etc.), which in turn doesn't allow higher level APIs (such as osmo_stream) to provide its own wrapper API passing eg. osmo_stream_cli/srv as a first parameter.
That means there's no way to store inter-msg state which may be needed at some point for segmentation, which doesn't come initially from first msgb.
segmentation_cb was added in libosmocore d4d03705f90e4a346f21ee2f328348b9ba37a201, between release 1.8.0 and 1.9.0.
Relevant users of segmentation_cb outside libosmocore which could be affected are:
libosmo-netif.git:
libosmo-netif/src/stream_srv.c 977: * \param[in] segmentation_cb Segmentation callback to be set */ 978:void osmo_stream_srv_set_segmentation_cb(struct osmo_stream_srv *conn, 979: osmo_stream_srv_segmentation_cb_t segmentation_cb) 988: conn_ops.segmentation_cb = segmentation_cb; libosmo-netif/src/stream_cli.c 110: osmo_stream_cli_segmentation_cb_t segmentation_cb; 433: cli->segmentation_cb = NULL; 490: .segmentation_cb = NULL, 527: .segmentation_cb = NULL, 663:static void configure_cli_segmentation_cb(struct osmo_stream_cli *cli, 664: osmo_stream_cli_segmentation_cb_t segmentation_cb) 670: client_ops.segmentation_cb = segmentation_cb; 676: * \param[in] segmentation_cb Target segmentation callback 678:void osmo_stream_cli_set_segmentation_cb(struct osmo_stream_cli *cli, 679: osmo_stream_cli_segmentation_cb_t segmentation_cb) 681: cli->segmentation_cb = segmentation_cb; libosmo-netif/src/ipa.c 411:int osmo_ipa_segmentation_cb(struct msgb *msg) libosmo-netif/include/osmocom/netif/ipa.h 66:int osmo_ipa_segmentation_cb(struct msgb *msg);
libosmo-sccp.git:
libosmo-sccp/src/ss7_internal.h 41:int xua_tcp_segmentation_cb(struct msgb *msg); libosmo-sccp/src/osmo_ss7_xua_srv.c 89: osmo_stream_srv_set_segmentation_cb(srv, osmo_ipa_segmentation_cb); 96: osmo_stream_srv_set_segmentation_cb(srv, xua_tcp_segmentation_cb); 103: osmo_stream_srv_set_segmentation_cb(srv, NULL); libosmo-sccp/src/osmo_ss7_asp.c 652: osmo_stream_cli_set_segmentation_cb(asp->client, osmo_ipa_segmentation_cb); 657: osmo_stream_cli_set_segmentation_cb(asp->client, NULL); 660: osmo_stream_cli_set_segmentation_cb(asp->client, xua_tcp_segmentation_cb); 667: osmo_stream_cli_set_segmentation_cb(asp->client, NULL); 856:int xua_tcp_segmentation_cb(struct msgb *msg)
osmo-bsc.git:
osmo-bsc/src/osmo-bsc/cbsp_link.c 109: osmo_stream_srv_set_segmentation_cb(srv, osmo_cbsp_segmentation_cb); 231: osmo_stream_cli_set_segmentation_cb(cbc->client.cli, osmo_cbsp_segmentation_cb);
Updated by pespin 16 days ago
We are anyway already breaking osmo_io_ops ABI with next upcoming release of libosmocore in several ways, eg from libosmocore.git TODO-RELEASE:
"core ABI change osmo_io_ops now contains a struct of structs, not union of structs"
So I think it makes sense to change the callback now anyway.
Alternatively, we could keep the old segmentation_cb and add a segmentation_cb2 field with the new signature, and osmo_io code calls whichever of the 2 is available, giving preference to the new one.
Updated by pespin 16 days ago
- Status changed from New to Feedback
RFC patch submitted here: https://gerrit.osmocom.org/c/libosmocore/+/36582 osmo_io: Add iofd param to segmentation_cb
We can also change it to have a new segmentation_cb2 callback to avoid breaking API (still breaking ABI but we are anyway already breaking it regardless of merging this).