diff mbox series

[v9,09/16] net: dgram: make dgram_dst generic

Message ID 20220926195048.487915-10-lvivier@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi: net: add unix socket type support to netdev backend | expand

Commit Message

Laurent Vivier Sept. 26, 2022, 7:50 p.m. UTC 
dgram_dst is a sockaddr_in structure. To be able to use it with
unix socket, use a pointer to a generic sockaddr structure.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
 net/dgram.c | 73 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 29 deletions(-)

Comments

David Gibson Sept. 28, 2022, 6:17 a.m. UTC | #1 
On Mon, Sep 26, 2022 at 09:50:41PM +0200, Laurent Vivier wrote:
> dgram_dst is a sockaddr_in structure. To be able to use it with
> unix socket, use a pointer to a generic sockaddr structure.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>

LGTM as far as it goes, but.. how can one actually use this pointer as
a generic sockaddr without also storing a socklen_t somewhere?

> ---
>  net/dgram.c | 73 ++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 44 insertions(+), 29 deletions(-)
> 
> diff --git a/net/dgram.c b/net/dgram.c
> index 45d869efc844..57933bfbd1c1 100644
> --- a/net/dgram.c
> +++ b/net/dgram.c
> @@ -39,7 +39,7 @@ typedef struct NetDgramState {
>      NetClientState nc;
>      int fd;
>      SocketReadState rs;
> -    struct sockaddr_in dgram_dst; /* contains destination iff connectionless */
> +    struct sockaddr *dgram_dst; /* contains destination iff connectionless */
>      bool read_poll;               /* waiting to receive data? */
>      bool write_poll;              /* waiting to transmit data? */
>  } NetDgramState;
> @@ -83,10 +83,9 @@ static ssize_t net_dgram_receive_dgram(NetClientState *nc,
>      ssize_t ret;
>  
>      do {
> -        if (s->dgram_dst.sin_family != AF_UNIX) {
> -            ret = sendto(s->fd, buf, size, 0,
> -                         (struct sockaddr *)&s->dgram_dst,
> -                         sizeof(s->dgram_dst));
> +        if (s->dgram_dst) {
> +            ret = sendto(s->fd, buf, size, 0, s->dgram_dst,
> +                         sizeof(struct sockaddr_in));
>          } else {
>              ret = send(s->fd, buf, size, 0);
>          }
> @@ -243,6 +242,8 @@ static void net_dgram_cleanup(NetClientState *nc)
>          close(s->fd);
>          s->fd = -1;
>      }
> +    g_free(s->dgram_dst);
> +    s->dgram_dst = NULL;
>  }
>  
>  static NetClientInfo net_dgram_socket_info = {
> @@ -259,7 +260,7 @@ static NetDgramState *net_dgram_fd_init_dgram(NetClientState *peer,
>                                                SocketAddress *mcast,
>                                                Error **errp)
>  {
> -    struct sockaddr_in saddr;
> +    struct sockaddr_in *saddr = NULL;
>      int newfd;
>      NetClientState *nc;
>      NetDgramState *s;
> @@ -281,24 +282,25 @@ static NetDgramState *net_dgram_fd_init_dgram(NetClientState *peer,
>       */
>  
>      if (is_fd && mcast != NULL) {
> -            if (convert_host_port(&saddr, mcast->u.inet.host,
> -                                  mcast->u.inet.port, errp) < 0) {
> +            saddr = g_new(struct sockaddr_in, 1);
> +
> +            if (convert_host_port(saddr, mcast->u.inet.host, mcast->u.inet.port,
> +                                  errp) < 0) {
>                  goto err;
>              }
>              /* must be bound */
> -            if (saddr.sin_addr.s_addr == 0) {
> +            if (saddr->sin_addr.s_addr == 0) {
>                  error_setg(errp, "can't setup multicast destination address");
>                  goto err;
>              }
>              /* clone dgram socket */
> -            newfd = net_dgram_mcast_create(&saddr, NULL, errp);
> +            newfd = net_dgram_mcast_create(saddr, NULL, errp);
>              if (newfd < 0) {
>                  goto err;
>              }
>              /* clone newfd to fd, close newfd */
>              dup2(newfd, fd);
>              close(newfd);
> -
>      }
>  
>      nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name);
> @@ -310,16 +312,13 @@ static NetDgramState *net_dgram_fd_init_dgram(NetClientState *peer,
>      net_dgram_read_poll(s, true);
>  
>      /* mcast: save bound address as dst */
> -    if (is_fd && mcast != NULL) {
> -        s->dgram_dst = saddr;
> +    if (saddr) {
> +        g_assert(s->dgram_dst == NULL);
> +        s->dgram_dst = (struct sockaddr *)saddr;
>          snprintf(nc->info_str, sizeof(nc->info_str),
>                   "fd=%d (cloned mcast=%s:%d)",
> -                 fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
> +                 fd, inet_ntoa(saddr->sin_addr), ntohs(saddr->sin_port));
>      } else {
> -        if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
> -            s->dgram_dst.sin_family = AF_UNIX;
> -        }
> -
>          snprintf(nc->info_str, sizeof(nc->info_str), "fd=%d %s", fd,
>                   SocketAddressType_str(sa_type));
>      }
> @@ -327,6 +326,7 @@ static NetDgramState *net_dgram_fd_init_dgram(NetClientState *peer,
>      return s;
>  
>  err:
> +    g_free(saddr);
>      closesocket(fd);
>      return NULL;
>  }
> @@ -340,21 +340,24 @@ static int net_dgram_mcast_init(NetClientState *peer,
>  {
>      NetDgramState *s;
>      int fd, ret;
> -    struct sockaddr_in saddr;
> +    struct sockaddr_in *saddr;
>  
>      if (remote->type != SOCKET_ADDRESS_TYPE_INET) {
>          error_setg(errp, "multicast only support inet type");
>          return -1;
>      }
>  
> -    if (convert_host_port(&saddr, remote->u.inet.host, remote->u.inet.port,
> +    saddr = g_new(struct sockaddr_in, 1);
> +    if (convert_host_port(saddr, remote->u.inet.host, remote->u.inet.port,
>                            errp) < 0) {
> +        g_free(saddr);
>          return -1;
>      }
>  
>      if (!local) {
> -        fd = net_dgram_mcast_create(&saddr, NULL, errp);
> +        fd = net_dgram_mcast_create(saddr, NULL, errp);
>          if (fd < 0) {
> +            g_free(saddr);
>              return -1;
>          }
>      } else {
> @@ -363,13 +366,15 @@ static int net_dgram_mcast_init(NetClientState *peer,
>              struct in_addr localaddr;
>  
>              if (inet_aton(local->u.inet.host, &localaddr) == 0) {
> +                g_free(saddr);
>                  error_setg(errp, "localaddr '%s' is not a valid IPv4 address",
>                             local->u.inet.host);
>                  return -1;
>              }
>  
> -            fd = net_dgram_mcast_create(&saddr, &localaddr, errp);
> +            fd = net_dgram_mcast_create(saddr, &localaddr, errp);
>              if (fd < 0) {
> +                g_free(saddr);
>                  return -1;
>              }
>              break;
> @@ -377,16 +382,19 @@ static int net_dgram_mcast_init(NetClientState *peer,
>          case SOCKET_ADDRESS_TYPE_FD:
>              fd = monitor_fd_param(monitor_cur(), local->u.fd.str, errp);
>              if (fd == -1) {
> +                g_free(saddr);
>                  return -1;
>              }
>              ret = qemu_socket_try_set_nonblock(fd);
>              if (ret < 0) {
> +                g_free(saddr);
>                  error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d",
>                                   name, fd);
>                  return -1;
>              }
>              break;
>          default:
> +            g_free(saddr);
>              error_setg(errp, "only support inet or fd type for local");
>              return -1;
>          }
> @@ -396,13 +404,16 @@ static int net_dgram_mcast_init(NetClientState *peer,
>                                   local->type == SOCKET_ADDRESS_TYPE_FD,
>                                   remote, errp);
>      if (!s) {
> +        g_free(saddr);
>          return -1;
>      }
>  
> -    s->dgram_dst = saddr;
> +    g_assert(s->dgram_dst == NULL);
> +    s->dgram_dst = (struct sockaddr *)saddr;
>  
>      snprintf(s->nc.info_str, sizeof(s->nc.info_str), "mcast=%s:%d",
> -             inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
> +             inet_ntoa(saddr->sin_addr), ntohs(saddr->sin_port));
> +
>      return 0;
>  
>  }
> @@ -416,8 +427,8 @@ static int net_dgram_init(NetClientState *peer,
>  {
>      NetDgramState *s;
>      int fd, ret;
> -    struct sockaddr_in raddr_in;
>      gchar *info_str;
> +    struct sockaddr *dgram_dst;
>  
>      /* detect multicast address */
>      if (remote && remote->type == SOCKET_ADDRESS_TYPE_INET) {
> @@ -458,7 +469,7 @@ static int net_dgram_init(NetClientState *peer,
>  
>      switch (local->type) {
>      case SOCKET_ADDRESS_TYPE_INET: {
> -        struct sockaddr_in laddr_in;
> +        struct sockaddr_in laddr_in, raddr_in;
>  
>          if (convert_host_port(&laddr_in, local->u.inet.host, local->u.inet.port,
>                                errp) < 0) {
> @@ -492,9 +503,12 @@ static int net_dgram_init(NetClientState *peer,
>          }
>          qemu_socket_set_nonblock(fd);
>  
> +        dgram_dst = g_malloc(sizeof(raddr_in));
> +        memcpy(dgram_dst, &raddr_in, sizeof(raddr_in));
> +
>          info_str = g_strdup_printf("udp=%s:%d/%s:%d",
> -                 inet_ntoa(laddr_in.sin_addr), ntohs(laddr_in.sin_port),
> -                 inet_ntoa(raddr_in.sin_addr), ntohs(raddr_in.sin_port));
> +                        inet_ntoa(laddr_in.sin_addr), ntohs(laddr_in.sin_port),
> +                        inet_ntoa(raddr_in.sin_addr), ntohs(raddr_in.sin_port));
>  
>          break;
>      }
> @@ -521,7 +535,8 @@ static int net_dgram_init(NetClientState *peer,
>      }
>  
>      if (remote) {
> -        s->dgram_dst = raddr_in;
> +        g_assert(s->dgram_dst == NULL);
> +        s->dgram_dst = dgram_dst;
>  
>          pstrcpy(s->nc.info_str, sizeof(s->nc.info_str), info_str);
>          g_free(info_str);
diff mbox series

Patch

diff --git a/net/dgram.c b/net/dgram.c
index 45d869efc844..57933bfbd1c1 100644
--- a/net/dgram.c
+++ b/net/dgram.c
@@ -39,7 +39,7 @@  typedef struct NetDgramState {
     NetClientState nc;
     int fd;
     SocketReadState rs;
-    struct sockaddr_in dgram_dst; /* contains destination iff connectionless */
+    struct sockaddr *dgram_dst; /* contains destination iff connectionless */
     bool read_poll;               /* waiting to receive data? */
     bool write_poll;              /* waiting to transmit data? */
 } NetDgramState;
@@ -83,10 +83,9 @@  static ssize_t net_dgram_receive_dgram(NetClientState *nc,
     ssize_t ret;
 
     do {
-        if (s->dgram_dst.sin_family != AF_UNIX) {
-            ret = sendto(s->fd, buf, size, 0,
-                         (struct sockaddr *)&s->dgram_dst,
-                         sizeof(s->dgram_dst));
+        if (s->dgram_dst) {
+            ret = sendto(s->fd, buf, size, 0, s->dgram_dst,
+                         sizeof(struct sockaddr_in));
         } else {
             ret = send(s->fd, buf, size, 0);
         }
@@ -243,6 +242,8 @@  static void net_dgram_cleanup(NetClientState *nc)
         close(s->fd);
         s->fd = -1;
     }
+    g_free(s->dgram_dst);
+    s->dgram_dst = NULL;
 }
 
 static NetClientInfo net_dgram_socket_info = {
@@ -259,7 +260,7 @@  static NetDgramState *net_dgram_fd_init_dgram(NetClientState *peer,
                                               SocketAddress *mcast,
                                               Error **errp)
 {
-    struct sockaddr_in saddr;
+    struct sockaddr_in *saddr = NULL;
     int newfd;
     NetClientState *nc;
     NetDgramState *s;
@@ -281,24 +282,25 @@  static NetDgramState *net_dgram_fd_init_dgram(NetClientState *peer,
      */
 
     if (is_fd && mcast != NULL) {
-            if (convert_host_port(&saddr, mcast->u.inet.host,
-                                  mcast->u.inet.port, errp) < 0) {
+            saddr = g_new(struct sockaddr_in, 1);
+
+            if (convert_host_port(saddr, mcast->u.inet.host, mcast->u.inet.port,
+                                  errp) < 0) {
                 goto err;
             }
             /* must be bound */
-            if (saddr.sin_addr.s_addr == 0) {
+            if (saddr->sin_addr.s_addr == 0) {
                 error_setg(errp, "can't setup multicast destination address");
                 goto err;
             }
             /* clone dgram socket */
-            newfd = net_dgram_mcast_create(&saddr, NULL, errp);
+            newfd = net_dgram_mcast_create(saddr, NULL, errp);
             if (newfd < 0) {
                 goto err;
             }
             /* clone newfd to fd, close newfd */
             dup2(newfd, fd);
             close(newfd);
-
     }
 
     nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name);
@@ -310,16 +312,13 @@  static NetDgramState *net_dgram_fd_init_dgram(NetClientState *peer,
     net_dgram_read_poll(s, true);
 
     /* mcast: save bound address as dst */
-    if (is_fd && mcast != NULL) {
-        s->dgram_dst = saddr;
+    if (saddr) {
+        g_assert(s->dgram_dst == NULL);
+        s->dgram_dst = (struct sockaddr *)saddr;
         snprintf(nc->info_str, sizeof(nc->info_str),
                  "fd=%d (cloned mcast=%s:%d)",
-                 fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+                 fd, inet_ntoa(saddr->sin_addr), ntohs(saddr->sin_port));
     } else {
-        if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
-            s->dgram_dst.sin_family = AF_UNIX;
-        }
-
         snprintf(nc->info_str, sizeof(nc->info_str), "fd=%d %s", fd,
                  SocketAddressType_str(sa_type));
     }
@@ -327,6 +326,7 @@  static NetDgramState *net_dgram_fd_init_dgram(NetClientState *peer,
     return s;
 
 err:
+    g_free(saddr);
     closesocket(fd);
     return NULL;
 }
@@ -340,21 +340,24 @@  static int net_dgram_mcast_init(NetClientState *peer,
 {
     NetDgramState *s;
     int fd, ret;
-    struct sockaddr_in saddr;
+    struct sockaddr_in *saddr;
 
     if (remote->type != SOCKET_ADDRESS_TYPE_INET) {
         error_setg(errp, "multicast only support inet type");
         return -1;
     }
 
-    if (convert_host_port(&saddr, remote->u.inet.host, remote->u.inet.port,
+    saddr = g_new(struct sockaddr_in, 1);
+    if (convert_host_port(saddr, remote->u.inet.host, remote->u.inet.port,
                           errp) < 0) {
+        g_free(saddr);
         return -1;
     }
 
     if (!local) {
-        fd = net_dgram_mcast_create(&saddr, NULL, errp);
+        fd = net_dgram_mcast_create(saddr, NULL, errp);
         if (fd < 0) {
+            g_free(saddr);
             return -1;
         }
     } else {
@@ -363,13 +366,15 @@  static int net_dgram_mcast_init(NetClientState *peer,
             struct in_addr localaddr;
 
             if (inet_aton(local->u.inet.host, &localaddr) == 0) {
+                g_free(saddr);
                 error_setg(errp, "localaddr '%s' is not a valid IPv4 address",
                            local->u.inet.host);
                 return -1;
             }
 
-            fd = net_dgram_mcast_create(&saddr, &localaddr, errp);
+            fd = net_dgram_mcast_create(saddr, &localaddr, errp);
             if (fd < 0) {
+                g_free(saddr);
                 return -1;
             }
             break;
@@ -377,16 +382,19 @@  static int net_dgram_mcast_init(NetClientState *peer,
         case SOCKET_ADDRESS_TYPE_FD:
             fd = monitor_fd_param(monitor_cur(), local->u.fd.str, errp);
             if (fd == -1) {
+                g_free(saddr);
                 return -1;
             }
             ret = qemu_socket_try_set_nonblock(fd);
             if (ret < 0) {
+                g_free(saddr);
                 error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d",
                                  name, fd);
                 return -1;
             }
             break;
         default:
+            g_free(saddr);
             error_setg(errp, "only support inet or fd type for local");
             return -1;
         }
@@ -396,13 +404,16 @@  static int net_dgram_mcast_init(NetClientState *peer,
                                  local->type == SOCKET_ADDRESS_TYPE_FD,
                                  remote, errp);
     if (!s) {
+        g_free(saddr);
         return -1;
     }
 
-    s->dgram_dst = saddr;
+    g_assert(s->dgram_dst == NULL);
+    s->dgram_dst = (struct sockaddr *)saddr;
 
     snprintf(s->nc.info_str, sizeof(s->nc.info_str), "mcast=%s:%d",
-             inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+             inet_ntoa(saddr->sin_addr), ntohs(saddr->sin_port));
+
     return 0;
 
 }
@@ -416,8 +427,8 @@  static int net_dgram_init(NetClientState *peer,
 {
     NetDgramState *s;
     int fd, ret;
-    struct sockaddr_in raddr_in;
     gchar *info_str;
+    struct sockaddr *dgram_dst;
 
     /* detect multicast address */
     if (remote && remote->type == SOCKET_ADDRESS_TYPE_INET) {
@@ -458,7 +469,7 @@  static int net_dgram_init(NetClientState *peer,
 
     switch (local->type) {
     case SOCKET_ADDRESS_TYPE_INET: {
-        struct sockaddr_in laddr_in;
+        struct sockaddr_in laddr_in, raddr_in;
 
         if (convert_host_port(&laddr_in, local->u.inet.host, local->u.inet.port,
                               errp) < 0) {
@@ -492,9 +503,12 @@  static int net_dgram_init(NetClientState *peer,
         }
         qemu_socket_set_nonblock(fd);
 
+        dgram_dst = g_malloc(sizeof(raddr_in));
+        memcpy(dgram_dst, &raddr_in, sizeof(raddr_in));
+
         info_str = g_strdup_printf("udp=%s:%d/%s:%d",
-                 inet_ntoa(laddr_in.sin_addr), ntohs(laddr_in.sin_port),
-                 inet_ntoa(raddr_in.sin_addr), ntohs(raddr_in.sin_port));
+                        inet_ntoa(laddr_in.sin_addr), ntohs(laddr_in.sin_port),
+                        inet_ntoa(raddr_in.sin_addr), ntohs(raddr_in.sin_port));
 
         break;
     }
@@ -521,7 +535,8 @@  static int net_dgram_init(NetClientState *peer,
     }
 
     if (remote) {
-        s->dgram_dst = raddr_in;
+        g_assert(s->dgram_dst == NULL);
+        s->dgram_dst = dgram_dst;
 
         pstrcpy(s->nc.info_str, sizeof(s->nc.info_str), info_str);
         g_free(info_str);