[PATCH] osmocom-bb[master]: VIRT-PHY: Further simplify mcast_sock code

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Thu Jul 13 12:32:08 UTC 2017


Review at  https://gerrit.osmocom.org/3235

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(-)


  git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/35/3235/1

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: newchange
Gerrit-Change-Id: Iad653686ac2bda5b3c92c30b4386ee7727d16271
Gerrit-PatchSet: 1
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>



More information about the gerrit-log mailing list