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 <chout...@adacore.com>
>>>>>> ---
>>>>>> 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
