On 27.10.2018 14:11, Samuel Thibault wrote:

Thanks for working on this!


I preferred to make this RFC simple and straightforward
with dumb code duplication because I wasn't sure if the feature is
useful for upstream at all :)

I will then unify v4 and v6 implementations as you suggested
(for other patches in the series too) and post the re-spin.

There is a lot of overlap with tcp_listen. It'd be much better to
refactor it this way:

struct socket *
tcpx_listen(Slirp *slirp, struct sockaddr *addr, socklen_t *addrlen, int flags)
        ... The current content of tcp_listen, with all heading and
        trailing addr manipulations removed...
        ... so->so_lfamily = addr->sa_family;...
        ... s = qemu_socket(addr->sa_family, SOCK_STREAM, 0);...
         ... (bind(s, addr, *addrlen) < 0) ||...
        ... getsockname(s, addr, addrlen);

struct socket *
tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
            u_int lport, int flags)
        struct sockaddr_in addr;
        struct socket *so;
        socklen_t addrsize = sizeof(addr);

        addr.sin_family = AF_INET;
        addr.sin_addr.s_addr = haddr;
        addr.sin_port = hport;

        so = tcpx_listen(slirp, (struct sockaddr *) &addr, &addrsize, flags);

        so->so_lport = lport; /* Kept in network format */
        so->so_laddr.s_addr = laddr; /* Ditto */

        so->so_ffamily = AF_INET;
        so->so_fport = addr.sin_port;
        if (addr.sin_addr.s_addr == 0 || addr.sin_addr.s_addr == 
           so->so_faddr = slirp->vhost_addr;
           so->so_faddr = addr.sin_addr;

The v6 version then boils down to

struct socket *
tcp6_listen(Slirp *slirp, struct in6_addr haddr, u_int hport, struct
            in6_addr laddr, u_int lport, int flags)
        struct sockaddr_in6 addr;
        struct socket *so;
        socklen_t addrsize = sizeof(addr);

        addr.sin6_family = AF_INET6;
        addr.sin6_addr = haddr;
        addr.sin6_port = hport;

        so = tcpx_listen(slirp, (struct sockaddr *) &addr, &addrsize, flags);

        so->so_lport6 = lport; /* Kept in network format */
        so->so_laddr6 = laddr; /* Ditto */

        so->fhost.sin6 = addr;
        if (!memcmp(&addr.sin6_addr, &in6addr_any, sizeof(in6addr_any)) ||
            !memcmp(&addr.sin6_addr, &in6addr_loopback,
                    sizeof(in6addr_loopback))) {
            memcpy(&so->so_faddr6, &slirp->vhost_addr6, 

modulo all typos etc. I may have done.

Maxim Samoylov, le ven. 26 oct. 2018 03:03:40 +0300, a ecrit:
+    qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int));

Is there a reason why you set SO_OOBINLINE, but not TCP_NODELAY? That's
the kind of discrepancy we don't want to let unseen, thus the reason for
a shared implementation :)

Actually my code was initially based on the last year master state, so I
missed your changes on TCP_NODELAY while rebasing, will fix.


Maxim Samoylov, max7...@yandex-team.ru

Reply via email to