From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45390) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a7NuR-0000AS-9M for qemu-devel@nongnu.org; Fri, 11 Dec 2015 08:38:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a7NuO-0008NH-1p for qemu-devel@nongnu.org; Fri, 11 Dec 2015 08:38:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36871) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a7NuN-0008N0-Qd for qemu-devel@nongnu.org; Fri, 11 Dec 2015 08:38:51 -0500 References: <20151211001505.GV2905@var.home> <1449792930-27293-1-git-send-email-samuel.thibault@ens-lyon.org> <1449792930-27293-2-git-send-email-samuel.thibault@ens-lyon.org> From: Thomas Huth Message-ID: <566AD1E4.1050403@redhat.com> Date: Fri, 11 Dec 2015 14:38:44 +0100 MIME-Version: 1.0 In-Reply-To: <1449792930-27293-2-git-send-email-samuel.thibault@ens-lyon.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff 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 11/12/15 01:15, Samuel Thibault wrote: > From: Guillaume Subiron >=20 > Basically, this patch replaces "arp" by "resolution" every time "arp" > means "mac resolution" and not specifically ARP. >=20 > Some indentation problems are solved in functions that will be modified > in the next patches (ip_input=E2=80=A6). >=20 > In if_encap, a switch is added to prepare for the IPv6 case. Some code > is factorized. >=20 > Signed-off-by: Guillaume Subiron > Signed-off-by: Samuel Thibault > --- > slirp/mbuf.c | 2 +- > slirp/mbuf.h | 2 +- > slirp/slirp.c | 24 ++++++++++++++++++++---- > 3 files changed, 22 insertions(+), 6 deletions(-) >=20 > diff --git a/slirp/mbuf.c b/slirp/mbuf.c > index 795fc29..bc942b6 100644 > --- a/slirp/mbuf.c > +++ b/slirp/mbuf.c > @@ -91,7 +91,7 @@ m_get(Slirp *slirp) > m->m_len =3D 0; > m->m_nextpkt =3D NULL; > m->m_prevpkt =3D NULL; > - m->arp_requested =3D false; > + m->resolution_requested =3D false; > m->expiration_date =3D (uint64_t)-1; > end_error: > DEBUG_ARG("m =3D %p", m); > diff --git a/slirp/mbuf.h b/slirp/mbuf.h > index b144f1c..38fedf4 100644 > --- a/slirp/mbuf.h > +++ b/slirp/mbuf.h > @@ -79,7 +79,7 @@ struct mbuf { > int m_len; /* Amount of data in this mbuf */ > =20 > Slirp *slirp; > - bool arp_requested; > + bool resolution_requested; > uint64_t expiration_date; > /* start of dynamic buffer area, must be last element */ > union { > diff --git a/slirp/slirp.c b/slirp/slirp.c > index 35f819a..380ddc3 100644 > --- a/slirp/slirp.c > +++ b/slirp/slirp.c > @@ -776,6 +776,8 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) > return 1; > } > =20 > + switch (iph->ip_v) { > + case IPVERSION: > if (iph->ip_dst.s_addr =3D=3D 0) { > /* 0.0.0.0 can not be a destination address, something went wr= ong, > * avoid making it worse */ That indentation looks now broken - shouldn't the if-statement now be indented with four more spaces now? > @@ -786,7 +788,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) > struct ethhdr *reh =3D (struct ethhdr *)arp_req; > struct arphdr *rah =3D (struct arphdr *)(arp_req + ETH_HLEN); > =20 > - if (!ifm->arp_requested) { > + if (!ifm->resolution_requested) { > /* If the client addr is not known, send an ARP request */ > memset(reh->h_dest, 0xff, ETH_ALEN); > memcpy(reh->h_source, special_ethaddr, ETH_ALEN - 4); > @@ -812,22 +814,36 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) > rah->ar_tip =3D iph->ip_dst.s_addr; > slirp->client_ipaddr =3D iph->ip_dst; > slirp_output(slirp->opaque, arp_req, sizeof(arp_req)); > - ifm->arp_requested =3D true; > + ifm->resolution_requested =3D true; > =20 > /* Expire request and drop outgoing packet after 1 second = */ > ifm->expiration_date =3D qemu_clock_get_ns(QEMU_CLOCK_REAL= TIME) + 1000000000ULL; > } > 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 =3D htons(ETH_P_IP); > + break; > + } > + > + default: > + /* Do not assert while we don't manage IP6VERSION */ > + /* assert(0); */ > + break; > + } > + > + memcpy(eh->h_dest, ethaddr, ETH_ALEN); > + DEBUG_ARGS((dfd, " src =3D %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 =3D %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; > - } > } The lines after the switch statement should also only be indented with 4 spaces, not with 8. Also, I'd prefer if you could split this patch into two: One for renaming the "arp_requested" field, and one for adding the additional logic with the switch statement (since there are two different kind of changes). Thomas