From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:55651) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnuqh-0004NE-Kk for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:56:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qnuqg-0001Sz-8l for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:56:11 -0400 Received: from mel.act-europe.fr ([194.98.77.210]:49666) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnuqg-0001Sp-0O for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:56:10 -0400 Message-ID: <4E36CC8E.5000606@adacore.com> Date: Mon, 01 Aug 2011 17:55:58 +0200 From: Fabien Chouteau 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> <4E36C21C.6030305@web.de> In-Reply-To: <4E36C21C.6030305@web.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: Jan Kiszka Cc: "qemu-devel@nongnu.org" On 01/08/2011 17:11, Jan Kiszka wrote: > 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 += qemu-timer.o qemu-timer-common.o >>>>>> >>>>>> slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o >>>>>> slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o >>>>>> -slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o >>>>>> +slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o >>>>>> common-obj-$(CONFIG_SLIRP) += $(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. >> >> OK I'll change it to keep consistent style. >> >> 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. :) Indeed, I bet we can find in Qemu an example of every C coding style... > >> >>>>> >>>>>> +{ >>>>>> + int i; >>>>>> + >>>>>> + DEBUG_CALL("arp_table_add"); >>>>>> + DEBUG_ARG("ip = 0x%x", ip_addr); >>>>>> + DEBUG_ARGS((dfd, " hw addr = %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 = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) { >>>>> >>>>> Missed that on round #1: Why treating 0.0.0.0 special? If ip_addr can be >>>>> zero, the current logic will append every "update" of that address as a >>>>> new entry. >> >> 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. > Great, so I will just prevent insertion and search of 0.0.0.0/8 addresses. Regards, -- Fabien Chouteau