From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35191) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a7WtZ-0002tU-OW for qemu-devel@nongnu.org; Fri, 11 Dec 2015 18:14:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a7WtW-00076t-Fp for qemu-devel@nongnu.org; Fri, 11 Dec 2015 18:14:37 -0500 Received: from domu-toccata.ens-lyon.fr ([140.77.166.138]:40231 helo=sonata.ens-lyon.org) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a7WtW-00076D-5u for qemu-devel@nongnu.org; Fri, 11 Dec 2015 18:14:34 -0500 Date: Sat, 12 Dec 2015 00:14:31 +0100 From: Samuel Thibault Message-ID: <20151211231431.GD2764@var.home> References: <20151211001505.GV2905@var.home> <1449792930-27293-1-git-send-email-samuel.thibault@ens-lyon.org> <1449792930-27293-5-git-send-email-samuel.thibault@ens-lyon.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449792930-27293-5-git-send-email-samuel.thibault@ens-lyon.org> Subject: Re: [Qemu-devel] [PATCH 05/18] slirp: Factorizing address translation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Thomas Huth , zhanghailiang , Li Zhijian , Stefan Hajnoczi , Jason Wang , Dave Gilbert , Vasiliy Tolstov , Huangpeng , Gonglei , Jan Kiszka , Yang Hongyang , Guillaume Subiron Hello, Thomas, this is the last refactoring patch which doesn't have a review yet, right? Samuel Samuel Thibault, on Fri 11 Dec 2015 01:15:17 +0100, wrote: > From: Guillaume Subiron > > This patch factorizes some duplicate code into a new function, > sotranslate_out(). This function perform the address translation when a > packet is transmitted to the host network. If the paquet is destinated > to the host, the loopback address is used, and if the paquet is > destinated to the virtual DNS, the real DNS address is used. This code > is just a copy of the existant, but factorized and ready to manage the > IPv6 case. > > On the same model, the major part of udp_output() code is moved into a > new sotranslate_in(). This function is directly used in sorecvfrom(), > like sotranslate_out() in sosendto(). > udp_output() becoming useless, it is removed and udp_output2() is > renamed into udp_output(). This adds consistency with the udp6_output() > function introduced by further patches. > > Lastly, this factorizes some duplicate code into sotranslate_accept(), which > performs the address translation when a connection is established on the host > for port forwarding: if it comes from localhost, the host virtual address is > used instead. > > Signed-off-by: Guillaume Subiron > Signed-off-by: Samuel Thibault > --- > slirp/bootp.c | 2 +- > slirp/ip_icmp.c | 19 ++------- > slirp/socket.c | 115 +++++++++++++++++++++++++++++++++++++++++++++---------- > slirp/socket.h | 5 +++ > slirp/tcp_subr.c | 35 ++++------------- > slirp/tftp.c | 6 +-- > slirp/udp.c | 37 ++---------------- > slirp/udp.h | 3 +- > 8 files changed, 119 insertions(+), 103 deletions(-) > > diff --git a/slirp/bootp.c b/slirp/bootp.c > index 1baaab1..0027279 100644 > --- a/slirp/bootp.c > +++ b/slirp/bootp.c > @@ -325,7 +325,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) > > m->m_len = sizeof(struct bootp_t) - > sizeof(struct ip) - sizeof(struct udphdr); > - udp_output2(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY); > + udp_output(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY); > } > > void bootp_input(struct mbuf *m) > diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c > index 58b7ceb..3a29847 100644 > --- a/slirp/ip_icmp.c > +++ b/slirp/ip_icmp.c > @@ -157,7 +157,7 @@ icmp_input(struct mbuf *m, int hlen) > goto freeit; > } else { > struct socket *so; > - struct sockaddr_in addr; > + struct sockaddr_storage addr; > if ((so = socreate(slirp)) == NULL) goto freeit; > if (icmp_send(so, m, hlen) == 0) { > return; > @@ -181,20 +181,9 @@ icmp_input(struct mbuf *m, int hlen) > so->so_state = SS_ISFCONNECTED; > > /* Send the packet */ > - addr.sin_family = AF_INET; > - if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == > - slirp->vnetwork_addr.s_addr) { > - /* It's an alias */ > - if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { > - if (get_dns_addr(&addr.sin_addr) < 0) > - addr.sin_addr = loopback_addr; > - } else { > - addr.sin_addr = loopback_addr; > - } > - } else { > - addr.sin_addr = so->so_faddr; > - } > - addr.sin_port = so->so_fport; > + addr = so->fhost.ss; > + sotranslate_out(so, &addr); > + > if(sendto(so->s, icmp_ping_msg, strlen(icmp_ping_msg), 0, > (struct sockaddr *)&addr, sizeof(addr)) == -1) { > DEBUG_MISC((dfd,"icmp_input udp sendto tx errno = %d-%s\n", > diff --git a/slirp/socket.c b/slirp/socket.c > index bf603c9..97948e8 100644 > --- a/slirp/socket.c > +++ b/slirp/socket.c > @@ -438,6 +438,7 @@ void > sorecvfrom(struct socket *so) > { > struct sockaddr_storage addr; > + struct sockaddr_storage saddr, daddr; > socklen_t addrlen = sizeof(struct sockaddr_storage); > > DEBUG_CALL("sorecvfrom"); > @@ -525,11 +526,17 @@ sorecvfrom(struct socket *so) > > /* > * If this packet was destined for CTL_ADDR, > - * make it look like that's where it came from, done by udp_output > + * make it look like that's where it came from > */ > + saddr = addr; > + sotranslate_in(so, &saddr); > + daddr = so->lhost.ss; > + > switch (so->so_ffamily) { > case AF_INET: > - udp_output(so, m, (struct sockaddr_in *) &addr); > + udp_output(so, m, (struct sockaddr_in *) &saddr, > + (struct sockaddr_in *) &daddr, > + so->so_iptos); > break; > default: > break; > @@ -544,33 +551,20 @@ sorecvfrom(struct socket *so) > int > sosendto(struct socket *so, struct mbuf *m) > { > - Slirp *slirp = so->slirp; > int ret; > - struct sockaddr_in addr; > + struct sockaddr_storage addr; > > DEBUG_CALL("sosendto"); > DEBUG_ARG("so = %p", so); > DEBUG_ARG("m = %p", m); > > - addr.sin_family = AF_INET; > - if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == > - slirp->vnetwork_addr.s_addr) { > - /* It's an alias */ > - if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { > - if (get_dns_addr(&addr.sin_addr) < 0) > - addr.sin_addr = loopback_addr; > - } else { > - addr.sin_addr = loopback_addr; > - } > - } else > - addr.sin_addr = so->so_faddr; > - addr.sin_port = so->so_fport; > - > - DEBUG_MISC((dfd, " sendto()ing, addr.sin_port=%d, addr.sin_addr.s_addr=%.16s\n", ntohs(addr.sin_port), inet_ntoa(addr.sin_addr))); > + addr = so->fhost.ss; > + DEBUG_CALL(" sendto()ing)"); > + sotranslate_out(so, &addr); > > /* Don't care what port we get */ > ret = sendto(so->s, m->m_data, m->m_len, 0, > - (struct sockaddr *)&addr, sizeof (struct sockaddr)); > + (struct sockaddr *)&addr, sizeof(addr)); > if (ret < 0) > return -1; > > @@ -726,3 +720,84 @@ sofwdrain(struct socket *so) > else > sofcantsendmore(so); > } > + > +/* > + * Translate addr in host addr when it is a virtual address > + * :TODO:maethor:130314: Manage IPv6 > + */ > +void sotranslate_out(struct socket *so, struct sockaddr_storage *addr) > +{ > + Slirp *slirp = so->slirp; > + struct sockaddr_in *sin = (struct sockaddr_in *)addr; > + > + switch (addr->ss_family) { > + case AF_INET: > + if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == > + slirp->vnetwork_addr.s_addr) { > + /* It's an alias */ > + if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { > + if (get_dns_addr(&sin->sin_addr) < 0) { > + sin->sin_addr = loopback_addr; > + } > + } else { > + sin->sin_addr = loopback_addr; > + } > + } > + > + DEBUG_MISC((dfd, " addr.sin_port=%d, " > + "addr.sin_addr.s_addr=%.16s\n", > + ntohs(sin->sin_port), inet_ntoa(sin->sin_addr))); > + break; > + > + default: > + break; > + } > +} > + > +/* :TODO:maethor:130314: IPv6 */ > +void sotranslate_in(struct socket *so, struct sockaddr_storage *addr) > +{ > + Slirp *slirp = so->slirp; > + struct sockaddr_in *sin = (struct sockaddr_in *)addr; > + > + switch (addr->ss_family) { > + case AF_INET: > + if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == > + slirp->vnetwork_addr.s_addr) { > + uint32_t inv_mask = ~slirp->vnetwork_mask.s_addr; > + > + if ((so->so_faddr.s_addr & inv_mask) == inv_mask) { > + sin->sin_addr = slirp->vhost_addr; > + } else if (sin->sin_addr.s_addr == loopback_addr.s_addr || > + so->so_faddr.s_addr != slirp->vhost_addr.s_addr) { > + sin->sin_addr = so->so_faddr; > + } > + } > + break; > + > + default: > + break; > + } > +} > + > +/* > + * Translate connections from localhost to the real hostname > + * :TODO: IPv6 > + */ > +void sotranslate_accept(struct socket *so) > +{ > + Slirp *slirp = so->slirp; > + > + switch (so->so_ffamily) { > + case AF_INET: > + if (so->so_faddr.s_addr == INADDR_ANY || > + (so->so_faddr.s_addr & loopback_mask) == > + (loopback_addr.s_addr & loopback_mask)) { > + so->so_faddr = slirp->vhost_addr; > + } > + break; > + > + default: > + break; > + } > +} > diff --git a/slirp/socket.h b/slirp/socket.h > index e854903..b27bbb2 100644 > --- a/slirp/socket.h > +++ b/slirp/socket.h > @@ -105,4 +105,9 @@ struct iovec; /* For win32 */ > size_t sopreprbuf(struct socket *so, struct iovec *iov, int *np); > int soreadbuf(struct socket *so, const char *buf, int size); > > +void sotranslate_out(struct socket *, struct sockaddr_storage *); > +void sotranslate_in(struct socket *, struct sockaddr_storage *); > +void sotranslate_accept(struct socket *); > + > + > #endif /* _SOCKET_H_ */ > diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c > index 47262db..76c716f 100644 > --- a/slirp/tcp_subr.c > +++ b/slirp/tcp_subr.c > @@ -326,7 +326,6 @@ tcp_sockclosed(struct tcpcb *tp) > */ > int tcp_fconnect(struct socket *so) > { > - Slirp *slirp = so->slirp; > int ret=0; > > DEBUG_CALL("tcp_fconnect"); > @@ -334,30 +333,17 @@ int tcp_fconnect(struct socket *so) > > if( (ret = so->s = qemu_socket(AF_INET,SOCK_STREAM,0)) >= 0) { > int opt, s=so->s; > - struct sockaddr_in addr; > + struct sockaddr_storage addr; > > qemu_set_nonblock(s); > socket_set_fast_reuse(s); > opt = 1; > qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(opt)); > > - addr.sin_family = AF_INET; > - if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == > - slirp->vnetwork_addr.s_addr) { > - /* It's an alias */ > - if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { > - if (get_dns_addr(&addr.sin_addr) < 0) > - addr.sin_addr = loopback_addr; > - } else { > - addr.sin_addr = loopback_addr; > - } > - } else > - addr.sin_addr = so->so_faddr; > - addr.sin_port = so->so_fport; > - > - DEBUG_MISC((dfd, " connect()ing, addr.sin_port=%d, " > - "addr.sin_addr.s_addr=%.16s\n", > - ntohs(addr.sin_port), inet_ntoa(addr.sin_addr))); > + addr = so->fhost.ss; > + DEBUG_CALL(" connect()ing") > + sotranslate_out(so, &addr); > + > /* We don't care what port we get */ > ret = connect(s,(struct sockaddr *)&addr,sizeof (addr)); > > @@ -431,15 +417,8 @@ void tcp_connect(struct socket *inso) > qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int)); > socket_set_nodelay(s); > > - so->so_ffamily = AF_INET; > - so->so_fport = addr.sin_port; > - so->so_faddr = addr.sin_addr; > - /* Translate connections from localhost to the real hostname */ > - if (so->so_faddr.s_addr == 0 || > - (so->so_faddr.s_addr & loopback_mask) == > - (loopback_addr.s_addr & loopback_mask)) { > - so->so_faddr = slirp->vhost_addr; > - } > + so->fhost.sin = addr; > + sotranslate_accept(so); > > /* Close the accept() socket, set right state */ > if (inso->so_state & SS_FACCEPTONCE) { > diff --git a/slirp/tftp.c b/slirp/tftp.c > index a329fb2..ccb6130 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -155,7 +155,7 @@ static int tftp_send_oack(struct tftp_session *spt, > > m->m_len = sizeof(struct tftp_t) - 514 + n - > sizeof(struct ip) - sizeof(struct udphdr); > - udp_output2(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY); > + udp_output(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY); > > return 0; > } > @@ -193,7 +193,7 @@ static void tftp_send_error(struct tftp_session *spt, > m->m_len = sizeof(struct tftp_t) - 514 + 3 + strlen(msg) - > sizeof(struct ip) - sizeof(struct udphdr); > > - udp_output2(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY); > + udp_output(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY); > > out: > tftp_session_terminate(spt); > @@ -243,7 +243,7 @@ static void tftp_send_next_block(struct tftp_session *spt, > m->m_len = sizeof(struct tftp_t) - (512 - nobytes) - > sizeof(struct ip) - sizeof(struct udphdr); > > - udp_output2(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY); > + udp_output(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY); > > if (nobytes == 512) { > tftp_session_update(spt); > diff --git a/slirp/udp.c b/slirp/udp.c > index 7a5c95b..8203eb1 100644 > --- a/slirp/udp.c > +++ b/slirp/udp.c > @@ -236,7 +236,7 @@ bad: > m_free(m); > } > > -int udp_output2(struct socket *so, struct mbuf *m, > +int udp_output(struct socket *so, struct mbuf *m, > struct sockaddr_in *saddr, struct sockaddr_in *daddr, > int iptos) > { > @@ -287,31 +287,6 @@ int udp_output2(struct socket *so, struct mbuf *m, > return (error); > } > > -int udp_output(struct socket *so, struct mbuf *m, > - struct sockaddr_in *addr) > - > -{ > - Slirp *slirp = so->slirp; > - struct sockaddr_in saddr, daddr; > - > - saddr = *addr; > - if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == > - slirp->vnetwork_addr.s_addr) { > - uint32_t inv_mask = ~slirp->vnetwork_mask.s_addr; > - > - if ((so->so_faddr.s_addr & inv_mask) == inv_mask) { > - saddr.sin_addr = slirp->vhost_addr; > - } else if (addr->sin_addr.s_addr == loopback_addr.s_addr || > - so->so_faddr.s_addr != slirp->vhost_addr.s_addr) { > - saddr.sin_addr = so->so_faddr; > - } > - } > - daddr.sin_addr = so->so_laddr; > - daddr.sin_port = so->so_lport; > - > - return udp_output2(so, m, &saddr, &daddr, so->so_iptos); > -} > - > int > udp_attach(struct socket *so) > { > @@ -378,14 +353,8 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, > socket_set_fast_reuse(so->s); > > getsockname(so->s,(struct sockaddr *)&addr,&addrlen); > - 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; > - } > + so->fhost.sin = addr; > + sotranslate_accept(so); > so->so_lfamily = AF_INET; > so->so_lport = lport; > so->so_laddr.s_addr = laddr; > diff --git a/slirp/udp.h b/slirp/udp.h > index 9bf31fe..a04b8ce 100644 > --- a/slirp/udp.h > +++ b/slirp/udp.h > @@ -76,12 +76,11 @@ struct mbuf; > void udp_init(Slirp *); > void udp_cleanup(Slirp *); > void udp_input(register struct mbuf *, int); > -int udp_output(struct socket *, struct mbuf *, struct sockaddr_in *); > int udp_attach(struct socket *); > void udp_detach(struct socket *); > struct socket * udp_listen(Slirp *, uint32_t, u_int, uint32_t, u_int, > int); > -int udp_output2(struct socket *so, struct mbuf *m, > +int udp_output(struct socket *so, struct mbuf *m, > struct sockaddr_in *saddr, struct sockaddr_in *daddr, > int iptos); > #endif > -- > 2.6.2 > -- Samuel hiri, le cri ici, c des marrants j'ai un rep ".uglyhackdirectorywithoutacls" ds mon home -+- #ens-mim en stage -+-