[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] [SLIRP] Simple ARP table
From: |
Fabien Chouteau |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] [SLIRP] Simple ARP table |
Date: |
Wed, 27 Jul 2011 14:55:04 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.18) Gecko/20110617 Lightning/1.0b2 Mnenhy/0.8.3 Thunderbird/3.1.11 |
On 27/07/2011 12:49, Jan Kiszka wrote:
> On 2011-07-26 18:21, 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 <address@hidden>
>> ---
>> Makefile.objs | 2 +-
>> slirp/arp_table.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>> slirp/bootp.c | 15 ++++++-----
>> slirp/slirp.c | 68
>> ++++++++++++++++------------------------------------
>> slirp/slirp.h | 51 +++++++++++++++++++++++++++++++++++++--
>> 5 files changed, 139 insertions(+), 58 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..e3251ed
>> --- /dev/null
>> +++ b/slirp/arp_table.c
>> @@ -0,0 +1,61 @@
>> +#include "slirp.h"
>> +
>> +void arp_table_flush(ArpTable *arptbl)
>> +{
>> + int i;
>> +
>> + assert(arptbl != NULL);
>> +
>> + for (i = 0; i < ARP_TABLE_SIZE; i++) {
>> + arptbl->table[i].ar_sip = 0;
>> + }
>> + arptbl->next_victim = 0;
>> +}
>
> arp_table_flush is only used for initialization, but the memory it
> touches is already zero-initialized (on alloc in slirp_init). So you can
> save this service.
OK, it was just in case slirp can be re-initialized.
>> +{
>> + int i;
>> +
>> + DEBUG_CALL("arp_table_add");
>> + DEBUG_ARG("ip = 0x%x", ahdr->ar_sip);
>> + DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
>> + ahdr->ar_sha[0], ahdr->ar_sha[1], ahdr->ar_sha[2],
>> + ahdr->ar_sha[3], ahdr->ar_sha[4], ahdr->ar_sha[5]));
>> +
>> + /* Search for an entry */
>> + for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
>> + if (arptbl->table[i].ar_sip == ahdr->ar_sip) {
>> + /* Update the entry */
>> + memcpy(arptbl->table[i].ar_sha, ahdr->ar_sha, ETH_ALEN);
>> + return;
>> + }
>> + }
>> +
>> + /* No entry found, create a new one */
>> + arptbl->table[arptbl->next_victim].ar_sip = ahdr->ar_sip;
>> + memcpy(arptbl->table[arptbl->next_victim].ar_sha, ahdr->ar_sha,
>> ETH_ALEN);
>> + arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;
>
> Pragmatic aging. But I guess that's OK, we shouldn't need anything
> smarter here.
Yes, simple round-robin.
>> diff --git a/slirp/bootp.c b/slirp/bootp.c
>> index 1eb2ed1..ccca93b 100644
>> --- a/slirp/bootp.c
>> +++ b/slirp/bootp.c
>> @@ -149,6 +149,7 @@ static void bootp_reply(Slirp *slirp, const struct
>> bootp_t *bp)
>> struct in_addr preq_addr;
>> int dhcp_msg_type, val;
>> uint8_t *q;
>> + uint8_t client_ethaddr[ETH_ALEN];
>>
>> /* extract exact DHCP msg type */
>> dhcp_decode(bp, &dhcp_msg_type, &preq_addr);
>> @@ -165,7 +166,7 @@ static void bootp_reply(Slirp *slirp, const struct
>> bootp_t *bp)
>> dhcp_msg_type != DHCPREQUEST)
>> return;
>> /* XXX: this is a hack to get the client mac address */
>> - memcpy(slirp->client_ethaddr, bp->bp_hwaddr, 6);
>> + memcpy(client_ethaddr, bp->bp_hwaddr, ETH_ALEN);
>
> Makes me wonder if the comment above still applies.
Actually I don't think it is a hack at all.
Makes me think I should update the ARP table here, with the IP address assigned
to this client.
Thanks for your review, I will fix the others style problems.
Regards,
--
Fabien Chouteau