[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv7 3/9] slirp: Adding IPv6 UDP support
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCHv7 3/9] slirp: Adding IPv6 UDP support |
Date: |
Tue, 9 Feb 2016 21:44:18 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 08.02.2016 11:28, Samuel Thibault wrote:
> From: Guillaume Subiron <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Samuel Thibault <address@hidden>
> ---
...
> 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
- [Qemu-devel] [PATCHv7 0/9] slirp: Adding IPv6 support to Qemu -net user mode, Samuel Thibault, 2016/02/08
- [Qemu-devel] [PATCHv7 6/9] slirp: Reindent after refactoring, Samuel Thibault, 2016/02/08
- [Qemu-devel] [PATCHv7 4/9] slirp: Factorizing tcpiphdr structure with an union, Samuel Thibault, 2016/02/08
- [Qemu-devel] [PATCHv7 8/9] slirp: Adding IPv6 address for DNS relay, Samuel Thibault, 2016/02/08
- [Qemu-devel] [PATCHv7 7/9] slirp: Handle IPv6 in TCP functions, Samuel Thibault, 2016/02/08
- [Qemu-devel] [PATCHv7 5/9] slirp: Generalizing and neutralizing various TCP functions before adding IPv6 stuff, Samuel Thibault, 2016/02/08
- [Qemu-devel] [PATCHv7 9/9] qapi-schema, qemu-options & slirp: Adding Qemu options for IPv6 addresses, Samuel Thibault, 2016/02/08
- [Qemu-devel] [PATCHv7 3/9] slirp: Adding IPv6 UDP support, Samuel Thibault, 2016/02/08
- Re: [Qemu-devel] [PATCHv7 3/9] slirp: Adding IPv6 UDP support,
Thomas Huth <=
[Qemu-devel] [PATCHv7 1/9] slirp: Adding IPv6, ICMPv6 Echo and NDP autoconfiguration, Samuel Thibault, 2016/02/08
Re: [Qemu-devel] [PATCHv7 1/9] slirp: Adding IPv6, ICMPv6 Echo and NDP autoconfiguration, Eric Blake, 2016/02/09