drivers/modem: Patch fix for sock id and fd

There is is an error in the way that the sock id is
determined. A unique fd is reserved and assigned appropriatly, but
the id, which should correspond to a socket number within the modem,
is set to an invalid, and identical for all sockets, value, and then
not used appropriatly in the drivers, instead, the sock_fd is used,
which can be any value really, it is not related to the socket number
in any way.

This results in the drivers only working if the reserved fd happens
to be between base_socket_num and (socket_len - 1)

This patch assignes the id to the index of the socket + the
base_socket_num, the socket at index 0 will get the id 1 if the
base_socket_num is 1, and the modem_socket_from_id should then
be used to get a pointer to the socket, since the id is not
neccesarily equal to the index.

The FIXME has been solved by adding a note both at the start
of the modem_socket_get function and inside the Kconfig file
for the MODEM_SOCKET option description. It is not an error,
but the user must be aware that it uses the POSIX file
descriptors, for which only 4 are allocated by default.

This patch fixes the bug, without breaking the brittle modem
drivers which currently are built around this bug.

The modem drivers should be updated to use the id as the
socket num instead of the sock_fd after this fix has been merged.

The "socket # needs assigning" has been removed, as that is what
this patch is doing

I also added comments to the id and sock_fd in the modem_socket
structure to help developers use the id and fd appropriately.

Signed-off-by: Bjarki Arge Andreasen <bjarkix123@gmail.com>
This commit is contained in:
Bjarki Arge Andreasen 2022-07-08 21:28:07 +02:00 committed by Carles Cufí
commit 52dd2bb3a3
3 changed files with 13 additions and 3 deletions

View file

@ -151,6 +151,10 @@ config MODEM_SOCKET
To configure this layer for use, create a modem_socket_config
object with your socket data and pass it's reference to
modem_socket_init().
Note that the modem socket uses runtime allocated file descriptors
reserved from the fdtable, for which the max count is set using the
Kconfig option POSIX_MAX_FDS. Make sure to update this value as both
the modem sockets and the POSIX_API, if used, share them.
config MODEM_SOCKET_PACKET_COUNT
int "Maximum number of stored packet sizes per socket"

View file

@ -143,6 +143,10 @@ data_ready:
* Socket Support Functions
*/
/*
* This function reserves a file descriptor from the fdtable, make sure to update the
* POSIX_FDS_MAX Kconfig option to support at minimum the required amount of sockets
*/
int modem_socket_get(struct modem_socket_config *cfg, int family, int type, int proto)
{
int i;
@ -160,7 +164,6 @@ int modem_socket_get(struct modem_socket_config *cfg, int family, int type, int
return -ENOMEM;
}
/* FIXME: 4 fds max now due to POSIX_OS conflict */
cfg->sockets[i].sock_fd = z_reserve_fd();
if (cfg->sockets[i].sock_fd < 0) {
k_sem_give(&cfg->sem_lock);
@ -170,8 +173,7 @@ int modem_socket_get(struct modem_socket_config *cfg, int family, int type, int
cfg->sockets[i].family = family;
cfg->sockets[i].type = type;
cfg->sockets[i].ip_proto = proto;
/* socket # needs assigning */
cfg->sockets[i].id = cfg->sockets_len + 1;
cfg->sockets[i].id = i + cfg->base_socket_num;
z_finalize_fd(cfg->sockets[i].sock_fd, &cfg->sockets[i],
(const struct fd_op_vtable *)cfg->vtable);

View file

@ -29,7 +29,11 @@ __net_socket struct modem_socket {
int ip_proto;
struct sockaddr src;
struct sockaddr dst;
/** The number identifying the socket handle inside the modem */
int id;
/** The file descriptor identifying the socket in the fdtable */
int sock_fd;
/** packet data */