Re: [Qemu-devel] [PATCH RFC 1/4] slirp: add helper for tcp6 socket creation

2018-10-30 Thread Samuel Thibault
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

2018-10-30 Thread Maxim Samoylov




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

2018-10-27 Thread Samuel Thibault
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

2018-10-27 Thread Samuel Thibault
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

2018-10-25 Thread Maxim Samoylov
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