From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47462) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnu9U-00034l-BQ for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:11:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qnu9S-00007Z-Mr for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:11:32 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:49008) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnu9S-00007M-9U for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:11:30 -0400 Message-ID: <4E36C21C.6030305@web.de> Date: Mon, 01 Aug 2011 17:11:24 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1311956703-24788-1-git-send-email-chouteau@adacore.com> <4E32EF27.2060406@siemens.com> <4E33CA59.7070607@web.de> <4E33CC89.502@web.de> <4E36C055.2070407@adacore.com> In-Reply-To: <4E36C055.2070407@adacore.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigCB0ED2954F02F9BDE3F628E9" Sender: jan.kiszka@web.de Subject: Re: [Qemu-devel] [PATCH V2 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) --------------enigCB0ED2954F02F9BDE3F628E9 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2011-08-01 17:03, Fabien Chouteau wrote: > On 30/07/2011 11:19, Jan Kiszka wrote: >> On 2011-07-30 11:09, Jan Kiszka wrote: >>> On 2011-07-29 19:34, Jan Kiszka wrote: >>>> On 2011-07-29 18:25, Fabien Chouteau wrote: >>>>> This patch adds a simple ARP table in Slirp and also adds handling = of >>>>> gratuitous ARP requests. >>>>> >>>>> Signed-off-by: Fabien Chouteau >>>>> --- >>>>> Makefile.objs | 2 +- >>>>> slirp/arp_table.c | 50 +++++++++++++++++++++++++++++++++++++++++= + >>>>> slirp/bootp.c | 23 ++++++++++++------ >>>>> slirp/slirp.c | 63 +++++++++++++----------------------------= ----------- >>>>> slirp/slirp.h | 50 +++++++++++++++++++++++++++++++++++++++--= >>>>> 5 files changed, 129 insertions(+), 59 deletions(-) >>>>> create mode 100644 slirp/arp_table.c >>>>> >>>>> 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-commo= n.o >>>>> >>>>> 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_t= able.o >>>>> common-obj-$(CONFIG_SLIRP) +=3D $(addprefix slirp/, $(slirp-obj-y)= ) >>>>> >>>>> # xen backend driver support >>>>> diff --git a/slirp/arp_table.c b/slirp/arp_table.c >>>>> new file mode 100644 >>>>> index 0000000..5d7404b >>>>> --- /dev/null >>>>> +++ b/slirp/arp_table.c >>>>> @@ -0,0 +1,50 @@ >>>>> +#include "slirp.h" >>>>> + >>>>> +void arp_table_add(ArpTable *arptbl, >>>>> + int ip_addr, >>>>> + uint8_t ethaddr[ETH_ALEN]) >>>> >>>> I still prefer the condensed formatting, but that's a minor nit. >=20 > OK I'll change it to keep consistent style. >=20 > Unfortunately, there's nothing on this subject in the CODING_STYLE... We should add a section on consistency - but I guess that will always remain a subjective matter. :) >=20 >>>> >>>>> +{ >>>>> + 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])); >>>>> + >>>>> + /* Search for an entry */ >>>>> + for (i =3D 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip !=3D= 0; i++) { >>>> >>>> Missed that on round #1: Why treating 0.0.0.0 special? If ip_addr ca= n be >>>> zero, the current logic will append every "update" of that address a= s a >>>> new entry. >=20 > Isn't 0.0.0.0 a reserved address? I think it's safe to use it here. Actually, the whole 0.0.0.0/8 is source-only, ie. it should never show up in the ARP table. Jan --------------enigCB0ED2954F02F9BDE3F628E9 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/ iEYEARECAAYFAk42wh8ACgkQitSsb3rl5xRrAwCgk5+ZUtZ5gYRtm6vgLdu0QFXV a5UAoI5nEiXkEWALEctA81VRV9Clz0yj =31/t -----END PGP SIGNATURE----- --------------enigCB0ED2954F02F9BDE3F628E9--