diff mbox series

[v12,10/17] net: dgram: make dgram_dst generic

Message ID 20221020091624.48368-11-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 Oct. 20, 2022, 9:16 a.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.

Rename it dest_addr, and store socket length in dest_len.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 net/dgram.c | 82 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 29 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 20, 2022, 11:17 a.m. UTC | #1 
On 20/10/22 11:16, 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.
> 
> Rename it dest_addr, and store socket length in dest_len.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   net/dgram.c | 82 ++++++++++++++++++++++++++++++++++-------------------
>   1 file changed, 53 insertions(+), 29 deletions(-)

>   static NetClientInfo net_dgram_socket_info = {
> @@ -260,7 +263,7 @@ static NetDgramState *net_dgram_fd_init(NetClientState *peer,
>                                           SocketAddress *mcast,
>                                           Error **errp)
>   {
> -    struct sockaddr_in saddr;
> +    struct sockaddr_in *saddr = NULL;

Preferrably g_autofree.

>       int newfd;
>       NetClientState *nc;
>       NetDgramState *s;
> @@ -275,31 +278,32 @@ static NetDgramState *net_dgram_fd_init(NetClientState *peer,
>       qapi_free_SocketAddress(sa);
>   
>       /*
> -     * fd passed: multicast: "learn" dgram_dst address from bound address and
> +     * fd passed: multicast: "learn" dest_addr address from bound address and
>        * save it. Because this may be "shared" socket from a "master" process,
>        * datagrams would be recv() by ONLY ONE process: we must "clone" this
>        * dgram socket --jjo
>        */
>   
>       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);
> @@ -311,21 +315,20 @@ static NetDgramState *net_dgram_fd_init(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->dest_addr == NULL);
> +        s->dest_addr = (struct sockaddr *)saddr;
> +        s->dest_len = sizeof(*saddr);
>           qemu_set_info_str(nc, "fd=%d (cloned mcast=%s:%d)", fd,
> -                          inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
> +                          inet_ntoa(saddr->sin_addr), ntohs(saddr->sin_port));
>       } else {
> -        if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
> -            s->dgram_dst.sin_family = AF_UNIX;
> -        }
> -
>           qemu_set_info_str(nc, "fd=%d %s", fd, SocketAddressType_str(sa_type));
>       }
>   
>       return s;
>   
>   err:
> +    g_free(saddr);
>       closesocket(fd);
>       return NULL;
>   }
> @@ -339,21 +342,24 @@ static int net_dgram_mcast_init(NetClientState *peer,
>   {
>       NetDgramState *s;
>       int fd, ret;
> -    struct sockaddr_in saddr;
> +    struct sockaddr_in *saddr;

Preferrably:

   g_autofree struct sockaddr_in *saddr = NULL.

>   
>       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 {
> @@ -362,13 +368,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;
> @@ -376,16 +384,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;
>           }
> @@ -395,13 +406,17 @@ 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->dest_addr == NULL);
> +    s->dest_addr = (struct sockaddr *)saddr;
> +    s->dest_len = sizeof(*saddr);
> +
> +    qemu_set_info_str(&s->nc, "mcast=%s:%d", inet_ntoa(saddr->sin_addr),
> +                      ntohs(saddr->sin_port));
>   
> -    qemu_set_info_str(&s->nc, "mcast=%s:%d", inet_ntoa(saddr.sin_addr),
> -                      ntohs(saddr.sin_port));
>       return 0;
>   
>   }
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Laurent Vivier Oct. 20, 2022, 3:08 p.m. UTC | #2 
On 10/20/22 13:17, Philippe Mathieu-Daudé wrote:
> On 20/10/22 11:16, 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.
>>
>> Rename it dest_addr, and store socket length in dest_len.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>   net/dgram.c | 82 ++++++++++++++++++++++++++++++++++-------------------
>>   1 file changed, 53 insertions(+), 29 deletions(-)
> 
>>   static NetClientInfo net_dgram_socket_info = {
>> @@ -260,7 +263,7 @@ static NetDgramState *net_dgram_fd_init(NetClientState *peer,
>>                                           SocketAddress *mcast,
>>                                           Error **errp)
>>   {
>> -    struct sockaddr_in saddr;
>> +    struct sockaddr_in *saddr = NULL;
> 
> Preferrably g_autofree.

No, because saddr pointer is copied to s->dest_addr.

...
>> @@ -339,21 +342,24 @@ static int net_dgram_mcast_init(NetClientState *peer,
>>   {
>>       NetDgramState *s;
>>       int fd, ret;
>> -    struct sockaddr_in saddr;
>> +    struct sockaddr_in *saddr;
> 
> Preferrably:
> 
>    g_autofree struct sockaddr_in *saddr = NULL.

The same here.

Thanks,
Laurent
diff mbox series

Patch

diff --git a/net/dgram.c b/net/dgram.c
index 5339585b8202..e20be9ca79d3 100644
--- a/net/dgram.c
+++ b/net/dgram.c
@@ -40,9 +40,11 @@  typedef struct NetDgramState {
     NetClientState nc;
     int fd;
     SocketReadState rs;
-    struct sockaddr_in dgram_dst; /* contains destination iff connectionless */
     bool read_poll;               /* waiting to receive data? */
     bool write_poll;              /* waiting to transmit data? */
+    /* contains destination iff connectionless */
+    struct sockaddr *dest_addr;
+    socklen_t dest_len;
 } NetDgramState;
 
 static void net_dgram_send(void *opaque);
@@ -84,10 +86,8 @@  static ssize_t net_dgram_receive(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->dest_addr) {
+            ret = sendto(s->fd, buf, size, 0, s->dest_addr, s->dest_len);
         } else {
             ret = send(s->fd, buf, size, 0);
         }
@@ -244,6 +244,9 @@  static void net_dgram_cleanup(NetClientState *nc)
         close(s->fd);
         s->fd = -1;
     }
+    g_free(s->dest_addr);
+    s->dest_addr = NULL;
+    s->dest_len = 0;
 }
 
 static NetClientInfo net_dgram_socket_info = {
@@ -260,7 +263,7 @@  static NetDgramState *net_dgram_fd_init(NetClientState *peer,
                                         SocketAddress *mcast,
                                         Error **errp)
 {
-    struct sockaddr_in saddr;
+    struct sockaddr_in *saddr = NULL;
     int newfd;
     NetClientState *nc;
     NetDgramState *s;
@@ -275,31 +278,32 @@  static NetDgramState *net_dgram_fd_init(NetClientState *peer,
     qapi_free_SocketAddress(sa);
 
     /*
-     * fd passed: multicast: "learn" dgram_dst address from bound address and
+     * fd passed: multicast: "learn" dest_addr address from bound address and
      * save it. Because this may be "shared" socket from a "master" process,
      * datagrams would be recv() by ONLY ONE process: we must "clone" this
      * dgram socket --jjo
      */
 
     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);
@@ -311,21 +315,20 @@  static NetDgramState *net_dgram_fd_init(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->dest_addr == NULL);
+        s->dest_addr = (struct sockaddr *)saddr;
+        s->dest_len = sizeof(*saddr);
         qemu_set_info_str(nc, "fd=%d (cloned mcast=%s:%d)", fd,
-                          inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+                          inet_ntoa(saddr->sin_addr), ntohs(saddr->sin_port));
     } else {
-        if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
-            s->dgram_dst.sin_family = AF_UNIX;
-        }
-
         qemu_set_info_str(nc, "fd=%d %s", fd, SocketAddressType_str(sa_type));
     }
 
     return s;
 
 err:
+    g_free(saddr);
     closesocket(fd);
     return NULL;
 }
@@ -339,21 +342,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 {
@@ -362,13 +368,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;
@@ -376,16 +384,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;
         }
@@ -395,13 +406,17 @@  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->dest_addr == NULL);
+    s->dest_addr = (struct sockaddr *)saddr;
+    s->dest_len = sizeof(*saddr);
+
+    qemu_set_info_str(&s->nc, "mcast=%s:%d", inet_ntoa(saddr->sin_addr),
+                      ntohs(saddr->sin_port));
 
-    qemu_set_info_str(&s->nc, "mcast=%s:%d", inet_ntoa(saddr.sin_addr),
-                      ntohs(saddr.sin_port));
     return 0;
 
 }
@@ -412,9 +427,10 @@  int net_init_dgram(const Netdev *netdev, const char *name,
 {
     NetDgramState *s;
     int fd, ret;
-    struct sockaddr_in raddr_in;
-    struct sockaddr_in laddr_in;
     SocketAddress *remote, *local;
+    struct sockaddr *dest_addr;
+    struct sockaddr_in laddr_in, raddr_in;
+    socklen_t dest_len;
 
     assert(netdev->type == NET_CLIENT_DRIVER_DGRAM);
 
@@ -491,6 +507,10 @@  int net_init_dgram(const Netdev *netdev, const char *name,
             return -1;
         }
         qemu_socket_set_nonblock(fd);
+
+        dest_len = sizeof(raddr_in);
+        dest_addr = g_malloc(dest_len);
+        memcpy(dest_addr, &raddr_in, dest_len);
         break;
     case SOCKET_ADDRESS_TYPE_FD:
         fd = monitor_fd_param(monitor_cur(), local->u.fd.str, errp);
@@ -503,6 +523,8 @@  int net_init_dgram(const Netdev *netdev, const char *name,
                              name, fd);
             return -1;
         }
+        dest_addr = NULL;
+        dest_len = 0;
         break;
     default:
         error_setg(errp, "only support inet or fd type for local");
@@ -515,7 +537,9 @@  int net_init_dgram(const Netdev *netdev, const char *name,
     }
 
     if (remote) {
-        s->dgram_dst = raddr_in;
+        g_assert(s->dest_addr == NULL);
+        s->dest_addr = dest_addr;
+        s->dest_len = dest_len;
     }
 
     switch (local->type) {