| Message ID | e4a16e2d4a3d7cd589639222c0414d265ed41226.1454927009.git.samuel.thibault@ens-lyon.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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 --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); +}