diff mbox

[05/18] slirp: Factorizing address translation

Message ID 1449792930-27293-5-git-send-email-samuel.thibault@ens-lyon.org
State New
Headers show

Commit Message

Samuel Thibault Dec. 11, 2015, 12:15 a.m. UTC
From: Guillaume Subiron <maethor@subiron.org>

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 <maethor@subiron.org>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 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(-)

Comments

Samuel Thibault Dec. 11, 2015, 11:14 p.m. UTC | #1
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 <maethor@subiron.org>
> 
> 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 <maethor@subiron.org>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
>  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
>
Thomas Huth Dec. 14, 2015, 11:41 a.m. UTC | #2
On 12/12/15 00:14, Samuel Thibault wrote:
> Hello,
> 
> Thomas, this is the last refactoring patch which doesn't have a review
> yet, right?

Right. It somehow showed up out of order in my e-mail program, so I
missed it on Friday, sorry. So here's a review...

> 
> Samuel Thibault, on Fri 11 Dec 2015 01:15:17 +0100, wrote:
>> From: Guillaume Subiron <maethor@subiron.org>
>>
>> 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

s/paquet/packet/ and s/destinated/destined/ (I think)

>> 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

s/existant/existent/

>> 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 <maethor@subiron.org>
>> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
>> ---
[...]
>> 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

In case you rework this patch, I think you could remove the "TODO" ...
that looks rather like an interim developer's comment that should not be
in the final patch, I think.

>> + */
>> +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 */

dito

>> +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;
>> +    }
>> +}
[...]

I'd maybe also split this up into several patches, one for
sotranslate_out(), and one or two for the remaining changes, but it also
looks fine to me as it currently is already.

Reviewed-by: Thomas Huth <thuth@redhat.com>
diff mbox

Patch

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