From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35206) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8XH3-0005c6-F8 for qemu-devel@nongnu.org; Mon, 14 Dec 2015 12:51:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a8XH0-0000ff-87 for qemu-devel@nongnu.org; Mon, 14 Dec 2015 12:51:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52348) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8XH0-0000fW-1B for qemu-devel@nongnu.org; Mon, 14 Dec 2015 12:50:58 -0500 References: <20151214134939.GA3142@var.bordeaux.inria.fr> <1450101088-14575-1-git-send-email-samuel.thibault@ens-lyon.org> <1450101088-14575-3-git-send-email-samuel.thibault@ens-lyon.org> From: Thomas Huth Message-ID: <566F0179.50209@redhat.com> Date: Mon, 14 Dec 2015 18:50:49 +0100 MIME-Version: 1.0 In-Reply-To: <1450101088-14575-3-git-send-email-samuel.thibault@ens-lyon.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/9] slirp: Adding address family switch for incoming frames 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 14/12/15 14:51, Samuel Thibault wrote: > From: Guillaume Subiron > > In if_encap, a switch is added to prepare for the IPv6 case. Some code > is factorized. > > This prepares for IPv6 support. > > Signed-off-by: Guillaume Subiron > Signed-off-by: Samuel Thibault > --- > slirp/slirp.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 48 insertions(+), 13 deletions(-) > > diff --git a/slirp/slirp.c b/slirp/slirp.c > index 1d5d172..f8dc505 100644 > --- a/slirp/slirp.c > +++ b/slirp/slirp.c > @@ -762,20 +762,15 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) > } > } > > -/* Output the IP packet to the ethernet device. Returns 0 if the packet must be > - * re-queued. > +/* Prepare the IPv4 packet to be sent to the ethernet device. Returns 1 if no > + * packet should be sent, 0 if the packet must be re-queued, 2 if the packet > + * is ready to go. > */ > -int if_encap(Slirp *slirp, struct mbuf *ifm) > +static int if_encap4(Slirp *slirp, struct mbuf *ifm, struct ethhdr *eh, > + uint8_t ethaddr[ETH_ALEN]) > { > - uint8_t buf[1600]; > - struct ethhdr *eh = (struct ethhdr *)buf; > - uint8_t ethaddr[ETH_ALEN]; > const struct ip *iph = (const struct ip *)ifm->m_data; > > - if (ifm->m_len + ETH_HLEN > sizeof(buf)) { > - return 1; > - } > - > if (iph->ip_dst.s_addr == 0) { > /* 0.0.0.0 can not be a destination address, something went wrong, > * avoid making it worse */ > @@ -819,15 +814,55 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) > } > return 0; > } else { > - memcpy(eh->h_dest, ethaddr, ETH_ALEN); > memcpy(eh->h_source, special_ethaddr, ETH_ALEN - 4); > /* XXX: not correct */ > memcpy(&eh->h_source[2], &slirp->vhost_addr, 4); > eh->h_proto = htons(ETH_P_IP); > - memcpy(buf + sizeof(struct ethhdr), ifm->m_data, ifm->m_len); > - slirp_output(slirp->opaque, buf, ifm->m_len + ETH_HLEN); > + > + /* Send this */ > + return 2; > + } > +} > + > +/* Output the IP packet to the ethernet device. Returns 0 if the packet must be > + * re-queued. > + */ > +int if_encap(Slirp *slirp, struct mbuf *ifm) > +{ > + uint8_t buf[1600]; > + struct ethhdr *eh = (struct ethhdr *)buf; > + uint8_t ethaddr[ETH_ALEN]; > + const struct ip *iph = (const struct ip *)ifm->m_data; > + int ret; > + > + if (ifm->m_len + ETH_HLEN > sizeof(buf)) { > return 1; > } > + > + switch (iph->ip_v) { > + case IPVERSION: > + ret = if_encap4(slirp, ifm, eh, ethaddr); > + if (ret < 2) { > + return ret; > + } > + break; > + > + default: > + /* Do not assert while we don't manage IP6VERSION */ > + /* assert(0); */ Not sure if we ever want to have an assert() here - since I assume this could be triggered by the guest? In that case, it would be better to use a qemu_log_mask(LOG_UNIMP, ...) or something similar in a later patch instead and simply "return 1" afterwards. > + break; > + } > + > + memcpy(eh->h_dest, ethaddr, ETH_ALEN); > + DEBUG_ARGS((dfd, " src = %02x:%02x:%02x:%02x:%02x:%02x\n", > + eh->h_source[0], eh->h_source[1], eh->h_source[2], > + eh->h_source[3], eh->h_source[4], eh->h_source[5])); > + DEBUG_ARGS((dfd, " dst = %02x:%02x:%02x:%02x:%02x:%02x\n", > + eh->h_dest[0], eh->h_dest[1], eh->h_dest[2], > + eh->h_dest[3], eh->h_dest[4], eh->h_dest[5])); > + memcpy(buf + sizeof(struct ethhdr), ifm->m_data, ifm->m_len); > + slirp_output(slirp->opaque, buf, ifm->m_len + ETH_HLEN); > + return 1; > } > > /* Drop host forwarding rule, return 0 if found. */ The patch now looks IMHO much nicer than the last version, thanks for reworking it! Reviewed-by: Thomas Huth