Harald Welte has submitted this change and it was merged.
Change subject: VIRT-PHY: Further simplify mcast_sock code
......................................................................
VIRT-PHY: Further simplify mcast_sock code
By avoiding dynamic allocations and relying on osmo_fd, we can
significantly simplify the code.
Change-Id: Iad653686ac2bda5b3c92c30b4386ee7727d16271
---
M src/host/virt_phy/include/virtphy/osmo_mcast_sock.h
M src/host/virt_phy/src/shared/osmo_mcast_sock.c
2 files changed, 69 insertions(+), 97 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/host/virt_phy/include/virtphy/osmo_mcast_sock.h
b/src/host/virt_phy/include/virtphy/osmo_mcast_sock.h
index ba5237a..31b2fd4 100644
--- a/src/host/virt_phy/include/virtphy/osmo_mcast_sock.h
+++ b/src/host/virt_phy/include/virtphy/osmo_mcast_sock.h
@@ -3,42 +3,25 @@
#include <netinet/in.h>
#include <osmocom/core/select.h>
-struct mcast_server_sock {
- struct osmo_fd osmo_fd;
-};
-
-struct mcast_client_sock {
- struct osmo_fd osmo_fd;
-};
-
struct mcast_bidir_sock {
- struct mcast_server_sock *tx_sock;
- struct mcast_client_sock *rx_sock;
+ struct osmo_fd tx_ofd;
+ struct osmo_fd rx_ofd;
};
-struct mcast_bidir_sock *mcast_bidir_sock_setup(
- void *ctx, char* tx_mcast_group, int tx_mcast_port,
- char* rx_mcast_group, int rx_mcast_port, int loopback,
+struct mcast_bidir_sock *mcast_bidir_sock_setup(void *ctx,
+ const char *tx_mcast_group, int tx_mcast_port,
+ const char *rx_mcast_group, int rx_mcast_port, int loopback,
int (*fd_rx_cb)(struct osmo_fd *ofd, unsigned int what),
void *osmo_fd_data);
-struct mcast_server_sock *mcast_server_sock_setup(void *ctx,
- char* tx_mcast_group,
- int tx_mcast_port,
- int loopback);
-struct mcast_client_sock *mcast_client_sock_setup(
- void *ctx, char* mcast_group, int mcast_port,
- int (*fd_rx_cb)(struct osmo_fd *ofd, unsigned int what),
- void *osmo_fd_data);
-int mcast_client_sock_rx(struct mcast_client_sock *client_sock, void* buf,
- int buf_len);
-int mcast_server_sock_tx(struct mcast_server_sock *serv_sock, void* data,
- int data_len);
-int mcast_bidir_sock_tx(struct mcast_bidir_sock *bidir_sock, void* data,
- int data_len);
-int mcast_bidir_sock_rx(struct mcast_bidir_sock *bidir_sock, void* buf,
- int buf_len);
-void mcast_client_sock_close(struct mcast_client_sock* client_sock);
-void mcast_server_sock_close(struct mcast_server_sock* server_sock);
+int mcast_server_sock_setup(struct osmo_fd *ofd, const char *tx_mcast_group,
+ int tx_mcast_port, int loopback);
+
+int mcast_client_sock_setup(struct osmo_fd *ofd, const char *mcast_group, int
mcast_port,
+ int (*fd_rx_cb)(struct osmo_fd *ofd, unsigned int
what),
+ void *osmo_fd_data);
+
+int mcast_bidir_sock_tx(struct mcast_bidir_sock *bidir_sock, void *data, int
data_len);
+int mcast_bidir_sock_rx(struct mcast_bidir_sock *bidir_sock, void *buf, int
buf_len);
void mcast_bidir_sock_close(struct mcast_bidir_sock* bidir_sock);
diff --git a/src/host/virt_phy/src/shared/osmo_mcast_sock.c
b/src/host/virt_phy/src/shared/osmo_mcast_sock.c
index 6ef3969..11a1aa9 100644
--- a/src/host/virt_phy/src/shared/osmo_mcast_sock.c
+++ b/src/host/virt_phy/src/shared/osmo_mcast_sock.c
@@ -10,149 +10,138 @@
#include <unistd.h>
#include <virtphy/osmo_mcast_sock.h>
+/* convenience wrapper */
+static void fd_close(struct osmo_fd *ofd)
+{
+ /* multicast memberships of socket are implicitly dropped when
+ * socket is closed */
+ osmo_fd_unregister(ofd);
+ close(ofd->fd);
+ ofd->fd = -1;
+ ofd->when = 0;
+}
+
/* server socket is what we use for transmission. It is not subscribed
* to a multicast group or locally bound, but it is just a normal UDP
* socket that's connected to the remote mcast group + port */
-struct mcast_server_sock *
-mcast_server_sock_setup(void *ctx, char* tx_mcast_group, int tx_mcast_port,
int loopback)
+int mcast_server_sock_setup(struct osmo_fd *ofd, const char* tx_mcast_group,
int tx_mcast_port,
+ int loopback)
{
- struct mcast_server_sock *serv_sock = talloc_zero(ctx, struct
mcast_server_sock);
int rc;
/* setup mcast server socket */
- rc = osmo_sock_init_ofd(&serv_sock->osmo_fd, AF_INET, SOCK_DGRAM,
IPPROTO_UDP,
+ rc = osmo_sock_init_ofd(ofd, AF_INET, SOCK_DGRAM, IPPROTO_UDP,
tx_mcast_group, tx_mcast_port,
OSMO_SOCK_F_CONNECT);
if (rc < 0) {
perror("Failed to create Multicast Server Socket");
- return NULL;
+ return rc;
}
/* determines whether sent mcast packets should be looped back to the
local sockets.
* loopback must be enabled if the mcast client is on the same machine
*/
- if (setsockopt(serv_sock->osmo_fd.fd, IPPROTO_IP, IP_MULTICAST_LOOP,
- &loopback, sizeof(loopback)) < 0) {
+ rc = setsockopt(ofd->fd, IPPROTO_IP, IP_MULTICAST_LOOP, &loopback,
sizeof(loopback));
+ if (rc < 0) {
perror("Failed to configure multicast loopback.\n");
- return NULL;
+ return rc;
}
- return serv_sock;
+ return 0;
}
/* the client socket is what we use for reception. It is a UDP socket
* that's bound to the GSMTAP UDP port and subscribed to the respective
* multicast group */
-struct mcast_client_sock *
-mcast_client_sock_setup(void *ctx, char* mcast_group, int mcast_port,
- int (*fd_rx_cb)(struct osmo_fd *ofd, unsigned int what),
- void *osmo_fd_data)
+int mcast_client_sock_setup(struct osmo_fd *ofd, const char *mcast_group, int
mcast_port,
+ int (*fd_rx_cb)(struct osmo_fd *ofd, unsigned int
what),
+ void *osmo_fd_data)
{
- struct mcast_client_sock *client_sock = talloc_zero(ctx, struct
mcast_client_sock);
struct ip_mreq mreq;
int rc, loopback = 1, all = 0;
- client_sock->osmo_fd.cb = fd_rx_cb;
- client_sock->osmo_fd.when = BSC_FD_READ;
- client_sock->osmo_fd.data = osmo_fd_data;
+ ofd->cb = fd_rx_cb;
+ ofd->when = BSC_FD_READ;
+ ofd->data = osmo_fd_data;
/* Create mcast client socket */
- rc = osmo_sock_init_ofd(&client_sock->osmo_fd, AF_INET, SOCK_DGRAM,
IPPROTO_UDP,
+ rc = osmo_sock_init_ofd(ofd, AF_INET, SOCK_DGRAM, IPPROTO_UDP,
NULL, mcast_port, OSMO_SOCK_F_BIND);
if (rc < 0) {
perror("Could not create mcast client socket");
- return NULL;
+ return rc;
}
/* Enable loopback of msgs to the host. */
/* Loopback must be enabled for the client, so multiple
* processes are able to receive a mcast package. */
- rc = setsockopt(client_sock->osmo_fd.fd, IPPROTO_IP, IP_MULTICAST_LOOP,
+ rc = setsockopt(ofd->fd, IPPROTO_IP, IP_MULTICAST_LOOP,
&loopback, sizeof(loopback));
if (rc < 0) {
perror("Failed to enable IP_MULTICAST_LOOP");
- return NULL;
+ return rc;
}
/* Configure and join the multicast group */
memset(&mreq, 0, sizeof(mreq));
mreq.imr_multiaddr.s_addr = inet_addr(mcast_group);
mreq.imr_interface.s_addr = htonl(INADDR_ANY);
- rc = setsockopt(client_sock->osmo_fd.fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
&mreq, sizeof(mreq));
+ rc = setsockopt(ofd->fd, IPPROTO_IP, IP_ADD_MEMBERSHIP, &mreq,
sizeof(mreq));
if (rc < 0) {
perror("Failed to join to mcast goup");
- return NULL;
+ return rc;
}
/* this option will set the delivery option so that only packets
* from sockets we are subscribed to via IP_ADD_MEMBERSHIP are received
*/
- if (setsockopt(client_sock->osmo_fd.fd, IPPROTO_IP, IP_MULTICAST_ALL,
&all, sizeof(all)) < 0) {
+ if (setsockopt(ofd->fd, IPPROTO_IP, IP_MULTICAST_ALL, &all,
sizeof(all)) < 0) {
perror("Failed to modify delivery policy to explicitly
joined.\n");
- return NULL;
+ return rc;
}
- return client_sock;
+ return 0;
}
struct mcast_bidir_sock *
-mcast_bidir_sock_setup(void *ctx, char* tx_mcast_group, int tx_mcast_port,
- char* rx_mcast_group, int rx_mcast_port, int loopback,
+mcast_bidir_sock_setup(void *ctx, const char *tx_mcast_group, int
tx_mcast_port,
+ const char *rx_mcast_group, int rx_mcast_port, int
loopback,
int (*fd_rx_cb)(struct osmo_fd *ofd, unsigned int what),
void *osmo_fd_data)
{
struct mcast_bidir_sock *bidir_sock = talloc(ctx, struct
mcast_bidir_sock);
- bidir_sock->rx_sock = mcast_client_sock_setup(ctx, rx_mcast_group,
- rx_mcast_port, fd_rx_cb,
osmo_fd_data);
- bidir_sock->tx_sock = mcast_server_sock_setup(ctx, tx_mcast_group,
- tx_mcast_port, loopback);
- if (!bidir_sock->rx_sock || !bidir_sock->tx_sock) {
+ int rc;
+
+ if (!bidir_sock)
+ return NULL;
+
+ rc = mcast_client_sock_setup(&bidir_sock->rx_ofd, rx_mcast_group,
rx_mcast_port,
+ fd_rx_cb, osmo_fd_data);
+ if (rc < 0) {
+ talloc_free(bidir_sock);
+ return NULL;
+ }
+ rc = mcast_server_sock_setup(&bidir_sock->tx_ofd, tx_mcast_group,
tx_mcast_port, loopback);
+ if (rc < 0) {
+ fd_close(&bidir_sock->rx_ofd);
+ talloc_free(bidir_sock);
return NULL;
}
return bidir_sock;
}
-int mcast_client_sock_rx(struct mcast_client_sock *client_sock, void* buf,
- int buf_len)
-{
- return recv(client_sock->osmo_fd.fd, buf, buf_len, 0);
-}
-
-int mcast_server_sock_tx(struct mcast_server_sock *serv_sock, void* data,
- int data_len)
-{
- return send(serv_sock->osmo_fd.fd, data, data_len, 0);
-}
-
int mcast_bidir_sock_tx(struct mcast_bidir_sock *bidir_sock, void* data,
int data_len)
{
- return mcast_server_sock_tx(bidir_sock->tx_sock, data, data_len);
+ return send(bidir_sock->tx_ofd.fd, data, data_len, 0);
}
int mcast_bidir_sock_rx(struct mcast_bidir_sock *bidir_sock, void* buf, int
buf_len)
{
- return mcast_client_sock_rx(bidir_sock->rx_sock, buf, buf_len);
-}
-
-void mcast_client_sock_close(struct mcast_client_sock *client_sock)
-{
- /* multicast memberships of socket are implicitly dropped when
- * socket is closed */
- osmo_fd_unregister(&client_sock->osmo_fd);
- close(client_sock->osmo_fd.fd);
- client_sock->osmo_fd.fd = -1;
- client_sock->osmo_fd.when = 0;
- talloc_free(client_sock);
-
-}
-void mcast_server_sock_close(struct mcast_server_sock *serv_sock)
-{
- close(serv_sock->osmo_fd.fd);
- talloc_free(serv_sock);
+ return recv(bidir_sock->rx_ofd.fd, buf, buf_len, 0);
}
void mcast_bidir_sock_close(struct mcast_bidir_sock *bidir_sock)
{
- mcast_client_sock_close(bidir_sock->rx_sock);
- mcast_server_sock_close(bidir_sock->tx_sock);
+ fd_close(&bidir_sock->tx_ofd);
+ fd_close(&bidir_sock->rx_ofd);
talloc_free(bidir_sock);
}
--
To view, visit https://gerrit.osmocom.org/3235
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iad653686ac2bda5b3c92c30b4386ee7727d16271
Gerrit-PatchSet: 1
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder
