diff mbox

[PATCHv7,3/9] slirp: Adding IPv6 UDP support

Message ID e4a16e2d4a3d7cd589639222c0414d265ed41226.1454927009.git.samuel.thibault@ens-lyon.org
State New
Headers show

Commit Message

Samuel Thibault Feb. 8, 2016, 10:28 a.m. UTC 
From: Guillaume Subiron <maethor@subiron.org>

This adds the sin6 case in the fhost and lhost unions and related macros.
It adds udp6_input() and udp6_output().
It adds the IPv6 case in sorecvfrom().
Finally, udp_input() is called by ip6_input().

Signed-off-by: Guillaume Subiron <maethor@subiron.org>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/Makefile.objs |   2 +-
 slirp/ip6_input.c   |   2 +-
 slirp/socket.c      |   5 ++
 slirp/socket.h      |   6 +++
 slirp/udp.h         |   5 ++
 slirp/udp6.c        | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 168 insertions(+), 2 deletions(-)
 create mode 100644 slirp/udp6.c

Comments

Thomas Huth Feb. 9, 2016, 8:44 p.m. UTC | #1 
On 08.02.2016 11:28, Samuel Thibault wrote:
> From: Guillaume Subiron <maethor@subiron.org>
> 
> This adds the sin6 case in the fhost and lhost unions and related macros.
> It adds udp6_input() and udp6_output().
> It adds the IPv6 case in sorecvfrom().
> Finally, udp_input() is called by ip6_input().
> 
> 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 32b1ba3..b79ddec 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -541,7 +541,12 @@ sorecvfrom(struct socket *so)
>  	                   (struct sockaddr_in *) &daddr,
>  	                   so->so_iptos);
>  	        break;
> +	    case AF_INET6:
> +	        udp6_output(so, m, (struct sockaddr_in6 *) &saddr,
> +	                    (struct sockaddr_in6 *) &daddr);
> +	        break;
>  	    default:
> +	        g_assert_not_reached();

Could this be triggered by the guest? If so, I'd like to suggest to use
qemu_log_mask(LOG_GUEST_ERROR, ...) instead, since a guest should not be
able to terminate QEMU like this.

>  	        break;
>  	    }
>  	  } /* rx error */
...
> diff --git a/slirp/udp6.c b/slirp/udp6.c
> new file mode 100644
> index 0000000..63d6a8c
> --- /dev/null
> +++ b/slirp/udp6.c
> @@ -0,0 +1,150 @@
> +/*
> + * Copyright (c) 2013
> + * Guillaume Subiron
> + *
> + * Please read the file COPYRIGHT for the
> + * terms and conditions of the copyright.
> + */
> +
> +#include "slirp.h"
> +#include "udp.h"
> +
> +void udp6_input(struct mbuf *m)
> +{
> +    Slirp *slirp = m->slirp;
> +    struct ip6 *ip, save_ip;
> +    struct udphdr *uh;
> +    int hlen = sizeof(struct ip6);
> +    int len;
> +    struct socket *so;
> +    struct sockaddr_in6 lhost;
> +
> +    DEBUG_CALL("udp6_input");
> +    DEBUG_ARG("m = %lx", (long)m);
> +
> +    if (slirp->restricted) {
> +        goto bad;
> +    }
> +
> +    ip = mtod(m, struct ip6 *);
> +    m->m_len -= hlen;
> +    m->m_data += hlen;
> +    uh = mtod(m, struct udphdr *);
> +    m->m_len += hlen;
> +    m->m_data -= hlen;
> +
> +    if (ip6_cksum(m)) {
> +        goto bad;
> +    }
> +
> +    len = ntohs((uint16_t)uh->uh_ulen);
> +
> +    /*
> +     * Make mbuf data length reflect UDP length.
> +     * If not enough data to reflect UDP length, drop.
> +     */
> +    if (ntohs(ip->ip_pl) != len) {
> +        if (len > ntohs(ip->ip_pl)) {
> +            goto bad;
> +        }
> +        m_adj(m, len - ntohs(ip->ip_pl));
> +        ip->ip_pl = htons(len);
> +    }
> +
> +    /* TODO handle DHCP/BOOTP */
> +    /* TODO handle TFTP */
> +
> +    /* Locate pcb for datagram. */
> +    lhost.sin6_family = AF_INET6;
> +    lhost.sin6_addr = ip->ip_src;
> +    lhost.sin6_port = uh->uh_sport;
> +
> +    so = solookup(&slirp->udp_last_so, &slirp->udb,
> +                  (struct sockaddr_storage *) &lhost, NULL);
> +
> +    if (so == NULL) {
> +        /* If there's no socket for this packet, create one. */
> +        so = socreate(slirp);
> +        if (!so) {
> +            goto bad;
> +        }
> +        if (udp_attach(so, AF_INET6) == -1) {
> +            DEBUG_MISC((dfd, " udp6_attach errno = %d-%s\n",
> +                        errno, strerror(errno)));
> +            sofree(so);
> +            goto bad;
> +        }
> +
> +        /* Setup fields */
> +        so->so_lfamily = AF_INET6;
> +        so->so_laddr6 = ip->ip_src;
> +        so->so_lport6 = uh->uh_sport;
> +    }
> +
> +    so->so_ffamily = AF_INET6;
> +    so->so_faddr6 = ip->ip_dst; /* XXX */
> +    so->so_fport6 = uh->uh_dport; /* XXX */

Why use the XXXs here? Some additional words in the comments would be
nice...

> +    hlen += sizeof(struct udphdr);
> +    m->m_len -= hlen;
> +    m->m_data += hlen;
> +
> +    /*
> +     * Now we sendto() the packet.
> +     */
> +    if (sosendto(so, m) == -1) {
> +        m->m_len += hlen;
> +        m->m_data -= hlen;
> +        *ip = save_ip;

It's getting late already and maybe I should stop reviewing ... but ...
using save_ip here looks bogus to me. Is this right, or just a
copy-n-paste error from the udpv4 code? Where is save_ip initialized?

> +        DEBUG_MISC((dfd, "udp tx errno = %d-%s\n", errno, strerror(errno)));
> +        /* TODO: ICMPv6 error */
> +        /*icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errno));*/
> +        goto bad;
> +    }
> +
> +    m_free(so->so_m);   /* used for ICMP if error on sorecvfrom */
> +
> +    /* restore the orig mbuf packet */
> +    m->m_len += hlen;
> +    m->m_data -= hlen;
> +    *ip = save_ip;

dito.

> +    so->so_m = m;
> +
> +    return;
> +bad:
> +    m_free(m);
> +}
> +
> +int udp6_output(struct socket *so, struct mbuf *m,
> +        struct sockaddr_in6 *saddr, struct sockaddr_in6 *daddr)
> +{
> +    struct ip6 *ip;
> +    struct udphdr *uh;
> +
> +    DEBUG_CALL("udp6_output");
> +    DEBUG_ARG("so = %lx", (long)so);
> +    DEBUG_ARG("m = %lx", (long)m);
> +
> +    /* adjust for header */
> +    m->m_data -= sizeof(struct udphdr);
> +    m->m_len += sizeof(struct udphdr);
> +    uh = mtod(m, struct udphdr *);
> +    m->m_data -= sizeof(struct ip6);
> +    m->m_len += sizeof(struct ip6);
> +    ip = mtod(m, struct ip6 *);
> +
> +    /* Build IP header */
> +    ip->ip_pl = htons(m->m_len - sizeof(struct ip6));
> +    ip->ip_nh = IPPROTO_UDP;
> +    ip->ip_src = saddr->sin6_addr;
> +    ip->ip_dst = daddr->sin6_addr;
> +
> +    /* Build UDP header */
> +    uh->uh_sport = saddr->sin6_port;
> +    uh->uh_dport = daddr->sin6_port;
> +    uh->uh_ulen = ip->ip_pl;
> +    uh->uh_sum = 0;
> +    uh->uh_sum = ip6_cksum(m);

I think you're missing the check for uh_sum = 0.

According to RFC768:

"If the computed  checksum  is zero,  it is transmitted  as all ones"

And according to RFC2460:

"whenever originating a UDP packet, an IPv6 node must compute a UDP
 checksum over the packet and the pseudo-header, and, if that
 computation yields a result of zero, it must be changed to hex
 FFFF for placement in the UDP header."

This is already done in udp.c, so you should also do this in udp6.c, I
think.

> +    return ip6_output(so, m, 0);
> +}
> 

 Thomas
Samuel Thibault Feb. 9, 2016, 9:13 p.m. UTC | #2 
Thomas Huth, on Tue 09 Feb 2016 21:44:18 +0100, wrote:
> > +	    case AF_INET6:
> > +	        udp6_output(so, m, (struct sockaddr_in6 *) &saddr,
> > +	                    (struct sockaddr_in6 *) &daddr);
> > +	        break;
> >  	    default:
> > +	        g_assert_not_reached();
> 
> Could this be triggered by the guest?

No, here we are in sorecvfrom, which only reads ipv4 or ipv6 packets
from the udp socket.

> > +    so->so_ffamily = AF_INET6;
> > +    so->so_faddr6 = ip->ip_dst; /* XXX */
> > +    so->so_fport6 = uh->uh_dport; /* XXX */
> 
> Why use the XXXs here? Some additional words in the comments would be
> nice...

That's a copy/paste from the UDPv4 code.  I don't know why they are there.

Samuel
Samuel Thibault Feb. 9, 2016, 9:19 p.m. UTC | #3 
Thanks for your reviews so far!  I have integrated the rest of comments,
the only remaining question for patches 1-3 is about srand() and rand().

Samuel
Thomas Huth Feb. 10, 2016, 7:18 a.m. UTC | #4 
On 09.02.2016 22:19, Samuel Thibault wrote:
> Thanks for your reviews so far!  I have integrated the rest of comments,
> the only remaining question for patches 1-3 is about srand() and rand().

I personally don't mind whether you use rand(), g_random_int_range() or
g_rand_int_range() here, but of course, the ..._range() functions seem
to fit more naturally here.
I just think that if you use rand() or g_random_int_range(), the rng
should be seeded from the main() function, not from the slirp code.

 Thomas
Samuel Thibault Feb. 10, 2016, 7:37 a.m. UTC | #5 
Thomas Huth, on Wed 10 Feb 2016 08:18:45 +0100, wrote:
> On 09.02.2016 22:19, Samuel Thibault wrote:
> > Thanks for your reviews so far!  I have integrated the rest of comments,
> > the only remaining question for patches 1-3 is about srand() and rand().
> 
> I personally don't mind whether you use rand(), g_random_int_range() or
> g_rand_int_range() here, but of course, the ..._range() functions seem
> to fit more naturally here.
> I just think that if you use rand() or g_random_int_range(), the rng
> should be seeded from the main() function, not from the slirp code.

Understood, but what do maintainers prefer?  I don't mind either way, I
just want to write what will be preferred.

Samuel
diff mbox

Patch

diff --git a/slirp/Makefile.objs b/slirp/Makefile.objs
index 2dfe8e0..faa32b6 100644
--- a/slirp/Makefile.objs
+++ b/slirp/Makefile.objs
@@ -1,3 +1,3 @@ 
 common-obj-y = cksum.o if.o ip_icmp.o ip6_icmp.o ip6_input.o ip6_output.o ip_input.o ip_output.o dnssearch.o
 common-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o
-common-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o ndp_table.o
+common-obj-y += tcp_subr.o tcp_timer.o udp.o udp6.o bootp.o tftp.o arp_table.o ndp_table.o
diff --git a/slirp/ip6_input.c b/slirp/ip6_input.c
index 828f47c..d7c612e 100644
--- a/slirp/ip6_input.c
+++ b/slirp/ip6_input.c
@@ -61,7 +61,7 @@  void ip6_input(struct mbuf *m)
         icmp6_send_error(m, ICMP6_UNREACH, ICMP6_UNREACH_NO_ROUTE);
         break;
     case IPPROTO_UDP:
-        icmp6_send_error(m, ICMP6_UNREACH, ICMP6_UNREACH_NO_ROUTE);
+        udp6_input(m);
         break;
     case IPPROTO_ICMPV6:
         icmp6_input(m);
diff --git a/slirp/socket.c b/slirp/socket.c
index 32b1ba3..b79ddec 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -541,7 +541,12 @@  sorecvfrom(struct socket *so)
 	                   (struct sockaddr_in *) &daddr,
 	                   so->so_iptos);
 	        break;
+	    case AF_INET6:
+	        udp6_output(so, m, (struct sockaddr_in6 *) &saddr,
+	                    (struct sockaddr_in6 *) &daddr);
+	        break;
 	    default:
+	        g_assert_not_reached();
 	        break;
 	    }
 	  } /* rx error */
diff --git a/slirp/socket.h b/slirp/socket.h
index 9dae491..39ef592 100644
--- a/slirp/socket.h
+++ b/slirp/socket.h
@@ -34,17 +34,23 @@  struct socket {
   union {   /* foreign host */
       struct sockaddr_storage ss;
       struct sockaddr_in sin;
+      struct sockaddr_in6 sin6;
   } fhost;
 #define so_faddr fhost.sin.sin_addr
 #define so_fport fhost.sin.sin_port
+#define so_faddr6 fhost.sin6.sin6_addr
+#define so_fport6 fhost.sin6.sin6_port
 #define so_ffamily fhost.ss.ss_family
 
   union {   /* local host */
       struct sockaddr_storage ss;
       struct sockaddr_in sin;
+      struct sockaddr_in6 sin6;
   } lhost;
 #define so_laddr lhost.sin.sin_addr
 #define so_lport lhost.sin.sin_port
+#define so_laddr6 lhost.sin6.sin6_addr
+#define so_lport6 lhost.sin6.sin6_port
 #define so_lfamily lhost.ss.ss_family
 
   uint8_t	so_iptos;	/* Type of service */
diff --git a/slirp/udp.h b/slirp/udp.h
index 2f9de38..10cc780 100644
--- a/slirp/udp.h
+++ b/slirp/udp.h
@@ -83,4 +83,9 @@  struct socket * udp_listen(Slirp *, uint32_t, u_int, uint32_t, u_int,
 int udp_output(struct socket *so, struct mbuf *m,
                 struct sockaddr_in *saddr, struct sockaddr_in *daddr,
                 int iptos);
+
+void udp6_input(register struct mbuf *);
+int udp6_output(struct socket *so, struct mbuf *m,
+                struct sockaddr_in6 *saddr, struct sockaddr_in6 *daddr);
+
 #endif
diff --git a/slirp/udp6.c b/slirp/udp6.c
new file mode 100644
index 0000000..63d6a8c
--- /dev/null
+++ b/slirp/udp6.c
@@ -0,0 +1,150 @@ 
+/*
+ * Copyright (c) 2013
+ * Guillaume Subiron
+ *
+ * Please read the file COPYRIGHT for the
+ * terms and conditions of the copyright.
+ */
+
+#include "slirp.h"
+#include "udp.h"
+
+void udp6_input(struct mbuf *m)
+{
+    Slirp *slirp = m->slirp;
+    struct ip6 *ip, save_ip;
+    struct udphdr *uh;
+    int hlen = sizeof(struct ip6);
+    int len;
+    struct socket *so;
+    struct sockaddr_in6 lhost;
+
+    DEBUG_CALL("udp6_input");
+    DEBUG_ARG("m = %lx", (long)m);
+
+    if (slirp->restricted) {
+        goto bad;
+    }
+
+    ip = mtod(m, struct ip6 *);
+    m->m_len -= hlen;
+    m->m_data += hlen;
+    uh = mtod(m, struct udphdr *);
+    m->m_len += hlen;
+    m->m_data -= hlen;
+
+    if (ip6_cksum(m)) {
+        goto bad;
+    }
+
+    len = ntohs((uint16_t)uh->uh_ulen);
+
+    /*
+     * Make mbuf data length reflect UDP length.
+     * If not enough data to reflect UDP length, drop.
+     */
+    if (ntohs(ip->ip_pl) != len) {
+        if (len > ntohs(ip->ip_pl)) {
+            goto bad;
+        }
+        m_adj(m, len - ntohs(ip->ip_pl));
+        ip->ip_pl = htons(len);
+    }
+
+    /* TODO handle DHCP/BOOTP */
+    /* TODO handle TFTP */
+
+    /* Locate pcb for datagram. */
+    lhost.sin6_family = AF_INET6;
+    lhost.sin6_addr = ip->ip_src;
+    lhost.sin6_port = uh->uh_sport;
+
+    so = solookup(&slirp->udp_last_so, &slirp->udb,
+                  (struct sockaddr_storage *) &lhost, NULL);
+
+    if (so == NULL) {
+        /* If there's no socket for this packet, create one. */
+        so = socreate(slirp);
+        if (!so) {
+            goto bad;
+        }
+        if (udp_attach(so, AF_INET6) == -1) {
+            DEBUG_MISC((dfd, " udp6_attach errno = %d-%s\n",
+                        errno, strerror(errno)));
+            sofree(so);
+            goto bad;
+        }
+
+        /* Setup fields */
+        so->so_lfamily = AF_INET6;
+        so->so_laddr6 = ip->ip_src;
+        so->so_lport6 = uh->uh_sport;
+    }
+
+    so->so_ffamily = AF_INET6;
+    so->so_faddr6 = ip->ip_dst; /* XXX */
+    so->so_fport6 = uh->uh_dport; /* XXX */
+
+    hlen += sizeof(struct udphdr);
+    m->m_len -= hlen;
+    m->m_data += hlen;
+
+    /*
+     * Now we sendto() the packet.
+     */
+    if (sosendto(so, m) == -1) {
+        m->m_len += hlen;
+        m->m_data -= hlen;
+        *ip = save_ip;
+        DEBUG_MISC((dfd, "udp tx errno = %d-%s\n", errno, strerror(errno)));
+        /* TODO: ICMPv6 error */
+        /*icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errno));*/
+        goto bad;
+    }
+
+    m_free(so->so_m);   /* used for ICMP if error on sorecvfrom */
+
+    /* restore the orig mbuf packet */
+    m->m_len += hlen;
+    m->m_data -= hlen;
+    *ip = save_ip;
+    so->so_m = m;
+
+    return;
+bad:
+    m_free(m);
+}
+
+int udp6_output(struct socket *so, struct mbuf *m,
+        struct sockaddr_in6 *saddr, struct sockaddr_in6 *daddr)
+{
+    struct ip6 *ip;
+    struct udphdr *uh;
+
+    DEBUG_CALL("udp6_output");
+    DEBUG_ARG("so = %lx", (long)so);
+    DEBUG_ARG("m = %lx", (long)m);
+
+    /* adjust for header */
+    m->m_data -= sizeof(struct udphdr);
+    m->m_len += sizeof(struct udphdr);
+    uh = mtod(m, struct udphdr *);
+    m->m_data -= sizeof(struct ip6);
+    m->m_len += sizeof(struct ip6);
+    ip = mtod(m, struct ip6 *);
+
+    /* Build IP header */
+    ip->ip_pl = htons(m->m_len - sizeof(struct ip6));
+    ip->ip_nh = IPPROTO_UDP;
+    ip->ip_src = saddr->sin6_addr;
+    ip->ip_dst = daddr->sin6_addr;
+
+    /* Build UDP header */
+    uh->uh_sport = saddr->sin6_port;
+    uh->uh_dport = daddr->sin6_port;
+    uh->uh_ulen = ip->ip_pl;
+    uh->uh_sum = 0;
+    uh->uh_sum = ip6_cksum(m);
+
+    return ip6_output(so, m, 0);
+}