On 01/08/2011 22:16, Jan Kiszka wrote:
> 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.
>>
>> Signed-off-by: Fabien Chouteau <chout...@adacore.com>
>> ---
>> 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
>>
>> 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..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 =
>> + ~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.
Of course, how can I have missed this...
>> + ArpTable *arptbl = &slirp->arp_table;
>> + 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]));
>> +
>> + if ((ip_addr & ~(0xf << 28)) == 0 ||
>> + ip_addr == broadcast_addr) {
>> + /* Do not register 0.0.0.0/8 or broadcast addresses */
>> + return;
>> + }
>> +
>> + /* Search for an entry */
>> + for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
>
> Forgot to remove the test for ar_sip != 0.
>
>> + if (arptbl->table[i].ar_sip == 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 = ip_addr;
>> + memcpy(arptbl->table[arptbl->next_victim].ar_sha, ethaddr, ETH_ALEN);
>> + arptbl->next_victim = (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 =
>> + ~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.
OK I will test with DHCP.
Thanks again,
--
Fabien Chouteau
