From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34466) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnyux-00044O-MT for qemu-devel@nongnu.org; Mon, 01 Aug 2011 16:16:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qnyuw-0001Fa-Ay for qemu-devel@nongnu.org; Mon, 01 Aug 2011 16:16:51 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:52088) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnyuv-0001FQ-Rz for qemu-devel@nongnu.org; Mon, 01 Aug 2011 16:16:50 -0400 Message-ID: <4E3709AC.5060206@web.de> Date: Mon, 01 Aug 2011 22:16:44 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1312215517-9271-1-git-send-email-chouteau@adacore.com> In-Reply-To: <1312215517-9271-1-git-send-email-chouteau@adacore.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig268D661F2E374077B1429073" Sender: jan.kiszka@web.de Subject: Re: [Qemu-devel] [PATCH V3 1/2] [SLIRP] Simple ARP table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fabien Chouteau Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig268D661F2E374077B1429073 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2011-08-01 18:18, Fabien Chouteau wrote: > This patch adds a simple ARP table in Slirp and also adds handling of > gratuitous ARP requests. >=20 > Signed-off-by: Fabien Chouteau > --- > Makefile.objs | 2 +- > slirp/arp_table.c | 72 +++++++++++++++++++++++++++++++++++++++++++++= ++++++++ > slirp/bootp.c | 21 +++++++++------ > slirp/slirp.c | 63 +++++++++++----------------------------------= > slirp/slirp.h | 47 ++++++++++++++++++++++++++++++++-- > 5 files changed, 146 insertions(+), 59 deletions(-) > create mode 100644 slirp/arp_table.c >=20 > diff --git a/Makefile.objs b/Makefile.objs > index 6991a9f..0c10557 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -151,7 +151,7 @@ common-obj-y +=3D qemu-timer.o qemu-timer-common.o > =20 > slirp-obj-y =3D cksum.o if.o ip_icmp.o ip_input.o ip_output.o > slirp-obj-y +=3D slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp= _output.o > -slirp-obj-y +=3D tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o > +slirp-obj-y +=3D tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table= =2Eo > common-obj-$(CONFIG_SLIRP) +=3D $(addprefix slirp/, $(slirp-obj-y)) > =20 > # xen backend driver support > diff --git a/slirp/arp_table.c b/slirp/arp_table.c > new file mode 100644 > index 0000000..c7034ee > --- /dev/null > +++ b/slirp/arp_table.c > @@ -0,0 +1,72 @@ > +#include "slirp.h" > + > +void arp_table_add(Slirp *slirp, int ip_addr, uint8_t ethaddr[ETH_ALEN= ]) > +{ > + const in_addr_t broadcast_addr =3D > + ~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr; That's only part of the picture. 255.255.255.255 is a valid broadcast address as well. > + ArpTable *arptbl =3D &slirp->arp_table; > + int i; > + > + DEBUG_CALL("arp_table_add"); > + DEBUG_ARG("ip =3D 0x%x", ip_addr); > + DEBUG_ARGS((dfd, " hw addr =3D %02x:%02x:%02x:%02x:%02x:%02x\n", > + ethaddr[0], ethaddr[1], ethaddr[2], > + ethaddr[3], ethaddr[4], ethaddr[5])); > + > + if ((ip_addr & ~(0xf << 28)) =3D=3D 0 || > + ip_addr =3D=3D broadcast_addr) { > + /* Do not register 0.0.0.0/8 or broadcast addresses */ > + return; > + } > + > + /* Search for an entry */ > + for (i =3D 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip !=3D 0= ; i++) { Forgot to remove the test for ar_sip !=3D 0. > + if (arptbl->table[i].ar_sip =3D=3D ip_addr) { > + /* Update the entry */ > + memcpy(arptbl->table[i].ar_sha, ethaddr, ETH_ALEN); > + return; > + } > + } > + > + /* No entry found, create a new one */ > + arptbl->table[arptbl->next_victim].ar_sip =3D ip_addr; > + memcpy(arptbl->table[arptbl->next_victim].ar_sha, ethaddr, ETH_AL= EN); > + arptbl->next_victim =3D (arptbl->next_victim + 1) % ARP_TABLE_SIZE= ; > +} > + > +bool arp_table_search(Slirp *slirp, int in_ip_addr, > + uint8_t out_ethaddr[ETH_ALEN]) > +{ > + const in_addr_t broadcast_addr =3D > + ~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr; Same as above. That means DCHP is still broken. Please include that scenario in your tests before sending the next round. > + ArpTable *arptbl =3D &slirp->arp_table; > + int i; > + > + DEBUG_CALL("arp_table_search"); > + DEBUG_ARG("ip =3D 0x%x", in_ip_addr); > + > + /* If address in 0.0.0.0/8 */ > + if ((in_ip_addr & ~(0xf << 28)) =3D=3D 0) { Should rather be an assert() as it means the caller is about to send a frame to that source-only address. Jan --------------enig268D661F2E374077B1429073 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk43CbAACgkQitSsb3rl5xRlEgCgxZHK7F3KYst+n1zJzY4a1yXm 0ukAn0Aezku6o8kSOiNDdYO3sAUabVCM =nnJb -----END PGP SIGNATURE----- --------------enig268D661F2E374077B1429073--