From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37307) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTF9D-0003zj-Uc for qemu-devel@nongnu.org; Tue, 09 Feb 2016 15:44:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTF99-0008CS-D2 for qemu-devel@nongnu.org; Tue, 09 Feb 2016 15:44:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51844) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTF99-0008C9-5f for qemu-devel@nongnu.org; Tue, 09 Feb 2016 15:44:27 -0500 References: From: Thomas Huth Message-ID: <56BA4FA2.1070809@redhat.com> Date: Tue, 9 Feb 2016 21:44:18 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCHv7 3/9] slirp: Adding IPv6 UDP support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Thibault , qemu-devel@nongnu.org Cc: zhanghailiang , Li Zhijian , Stefan Hajnoczi , Jason Wang , Dave Gilbert , Vasiliy Tolstov , Huangpeng , Gonglei , Jan Kiszka , Yang Hongyang , Guillaume Subiron On 08.02.2016 11:28, Samuel Thibault wrote: > From: Guillaume Subiron >=20 > This adds the sin6 case in the fhost and lhost unions and related macro= s. > It adds udp6_input() and udp6_output(). > It adds the IPv6 case in sorecvfrom(). > Finally, udp_input() is called by ip6_input(). >=20 > Signed-off-by: Guillaume Subiron > Signed-off-by: Samuel Thibault > --- ... > 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 =3D m->slirp; > + struct ip6 *ip, save_ip; > + struct udphdr *uh; > + int hlen =3D sizeof(struct ip6); > + int len; > + struct socket *so; > + struct sockaddr_in6 lhost; > + > + DEBUG_CALL("udp6_input"); > + DEBUG_ARG("m =3D %lx", (long)m); > + > + if (slirp->restricted) { > + goto bad; > + } > + > + ip =3D mtod(m, struct ip6 *); > + m->m_len -=3D hlen; > + m->m_data +=3D hlen; > + uh =3D mtod(m, struct udphdr *); > + m->m_len +=3D hlen; > + m->m_data -=3D hlen; > + > + if (ip6_cksum(m)) { > + goto bad; > + } > + > + len =3D 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) !=3D len) { > + if (len > ntohs(ip->ip_pl)) { > + goto bad; > + } > + m_adj(m, len - ntohs(ip->ip_pl)); > + ip->ip_pl =3D htons(len); > + } > + > + /* TODO handle DHCP/BOOTP */ > + /* TODO handle TFTP */ > + > + /* Locate pcb for datagram. */ > + lhost.sin6_family =3D AF_INET6; > + lhost.sin6_addr =3D ip->ip_src; > + lhost.sin6_port =3D uh->uh_sport; > + > + so =3D solookup(&slirp->udp_last_so, &slirp->udb, > + (struct sockaddr_storage *) &lhost, NULL); > + > + if (so =3D=3D NULL) { > + /* If there's no socket for this packet, create one. */ > + so =3D socreate(slirp); > + if (!so) { > + goto bad; > + } > + if (udp_attach(so, AF_INET6) =3D=3D -1) { > + DEBUG_MISC((dfd, " udp6_attach errno =3D %d-%s\n", > + errno, strerror(errno))); > + sofree(so); > + goto bad; > + } > + > + /* Setup fields */ > + so->so_lfamily =3D AF_INET6; > + so->so_laddr6 =3D ip->ip_src; > + so->so_lport6 =3D uh->uh_sport; > + } > + > + so->so_ffamily =3D AF_INET6; > + so->so_faddr6 =3D ip->ip_dst; /* XXX */ > + so->so_fport6 =3D uh->uh_dport; /* XXX */ Why use the XXXs here? Some additional words in the comments would be nice... > + hlen +=3D sizeof(struct udphdr); > + m->m_len -=3D hlen; > + m->m_data +=3D hlen; > + > + /* > + * Now we sendto() the packet. > + */ > + if (sosendto(so, m) =3D=3D -1) { > + m->m_len +=3D hlen; > + m->m_data -=3D hlen; > + *ip =3D 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 =3D %d-%s\n", errno, strerror(e= rrno))); > + /* TODO: ICMPv6 error */ > + /*icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errn= o));*/ > + goto bad; > + } > + > + m_free(so->so_m); /* used for ICMP if error on sorecvfrom */ > + > + /* restore the orig mbuf packet */ > + m->m_len +=3D hlen; > + m->m_data -=3D hlen; > + *ip =3D save_ip; dito. > + so->so_m =3D 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 =3D %lx", (long)so); > + DEBUG_ARG("m =3D %lx", (long)m); > + > + /* adjust for header */ > + m->m_data -=3D sizeof(struct udphdr); > + m->m_len +=3D sizeof(struct udphdr); > + uh =3D mtod(m, struct udphdr *); > + m->m_data -=3D sizeof(struct ip6); > + m->m_len +=3D sizeof(struct ip6); > + ip =3D mtod(m, struct ip6 *); > + > + /* Build IP header */ > + ip->ip_pl =3D htons(m->m_len - sizeof(struct ip6)); > + ip->ip_nh =3D IPPROTO_UDP; > + ip->ip_src =3D saddr->sin6_addr; > + ip->ip_dst =3D daddr->sin6_addr; > + > + /* Build UDP header */ > + uh->uh_sport =3D saddr->sin6_port; > + uh->uh_dport =3D daddr->sin6_port; > + uh->uh_ulen =3D ip->ip_pl; > + uh->uh_sum =3D 0; > + uh->uh_sum =3D ip6_cksum(m); I think you're missing the check for uh_sum =3D 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); > +} >=20 Thomas