Re: [Qemu-devel] [PATCH RFC 1/4] slirp: add helper for tcp6 socket creation
Maxim Samoylov, le mar. 30 oct. 2018 16:58:17 +0300, a ecrit: > On 27.10.2018 14:11, Samuel Thibault wrote: > > 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 Which shows why it's useful to share the code :) Samuel
Re: [Qemu-devel] [PATCH RFC 1/4] slirp: add helper for tcp6 socket creation
On 27.10.2018 14:11, Samuel Thibault wrote: Hello, Thanks for working on this! Hi! 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 *) , , 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 == loopback_addr.s_addr) so->so_faddr = slirp->vhost_addr; else 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 *) , , flags); so->so_lport6 = lport; /* Kept in network format */ so->so_laddr6 = laddr; /* Ditto */ so->fhost.sin6 = addr; if (!memcmp(_addr, _any, sizeof(in6addr_any)) || !memcmp(_addr, _loopback, sizeof(in6addr_loopback))) { memcpy(>so_faddr6, >vhost_addr6, sizeof(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, , 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. Samuel --- Maxim Samoylov, max7...@yandex-team.ru
Re: [Qemu-devel] [PATCH RFC 1/4] slirp: add helper for tcp6 socket creation
Samuel Thibault, le sam. 27 oct. 2018 13:11:41 +0200, a ecrit: > 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); Also memset(, 0, sizeof(addr); > addr.sin_family = AF_INET; > addr.sin_addr.s_addr = haddr; > addr.sin_port = hport;
Re: [Qemu-devel] [PATCH RFC 1/4] slirp: add helper for tcp6 socket creation
Hello, Thanks for working on this! 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 *) , , 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 == loopback_addr.s_addr) so->so_faddr = slirp->vhost_addr; else 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 *) , , flags); so->so_lport6 = lport; /* Kept in network format */ so->so_laddr6 = laddr; /* Ditto */ so->fhost.sin6 = addr; if (!memcmp(_addr, _any, sizeof(in6addr_any)) || !memcmp(_addr, _loopback, sizeof(in6addr_loopback))) { memcpy(>so_faddr6, >vhost_addr6, sizeof(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, , 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 :) Samuel
[Qemu-devel] [PATCH RFC 1/4] slirp: add helper for tcp6 socket creation
Signed-off-by: Maxim Samoylov --- slirp/socket.c | 73 ++ slirp/socket.h | 2 ++ 2 files changed, 75 insertions(+) diff --git a/slirp/socket.c b/slirp/socket.c index 322383a..e16e6c1 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -776,6 +776,79 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, return so; } +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; +int s, opt = 1; +socklen_t addrlen = sizeof(addr); +memset(, 0, addrlen); + +/* The same flow as in tcp_listen above */ + +so = socreate(slirp); +if (!so) { +return NULL; +} + +so->so_tcpcb = tcp_newtcpcb(so); +if (so->so_tcpcb == NULL) { +free(so); +return NULL; +} + +insque(so, >tcb); + +if (flags & SS_FACCEPTONCE) { +so->so_tcpcb->t_timer[TCPT_KEEP] = TCPTV_KEEP_INIT * 2; +} + +so->so_state &= SS_PERSISTENT_MASK; +so->so_state |= (SS_FACCEPTCONN | flags); +so->so_lfamily = AF_INET6; +so->so_lport6 = lport; /* Kept in network format */ +so->so_laddr6 = laddr; + +addr.sin6_family = AF_INET6; +addr.sin6_addr = haddr; +addr.sin6_port = hport; + +s = qemu_socket(AF_INET6, SOCK_STREAM, 0); +if ((s < 0) || +(socket_set_fast_reuse(s) < 0) || +(bind(s, (struct sockaddr *), sizeof(addr)) < 0) || +(listen(s, 1) < 0)) { +int tmperrno = errno; /* Don't clobber the real reason we failed */ +if (s >= 0) { +closesocket(s); +} +sofree(so); +/* Restore the real errno */ +#ifdef _WIN32 +WSASetLastError(tmperrno); +#else +errno = tmperrno; +#endif +return NULL; +} +qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, , sizeof(int)); + +getsockname(s, (struct sockaddr *), ); +so->fhost.sin6 = addr; + +if (!memcmp(_addr, _any, sizeof(in6addr_any)) || +!memcmp(_addr, _loopback, +sizeof(in6addr_loopback))) { + +memcpy(>so_faddr6, >vhost_addr6, sizeof(slirp->vhost_addr6)); +} + +so->s = s; +return so; +} + /* * Various session state calls * XXX Should be #define's diff --git a/slirp/socket.h b/slirp/socket.h index 2f224bc..b200bb6 100644 --- a/slirp/socket.h +++ b/slirp/socket.h @@ -144,6 +144,8 @@ void sorecvfrom(struct socket *); int sosendto(struct socket *, struct mbuf *); struct socket * tcp_listen(Slirp *, uint32_t, u_int, uint32_t, u_int, int); +struct socket *tcp6_listen(Slirp *, struct in6_addr, u_int, + struct in6_addr, u_int, int); void soisfconnecting(register struct socket *); void soisfconnected(register struct socket *); void sofwdrain(struct socket *); -- 2.7.4