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. > +{ > + 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. > + 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(ArpTable *arptbl, > + int in_ip_addr, > + uint8_t out_ethaddr[ETH_ALEN]) > +{ > + int i; > + > + DEBUG_CALL("arp_table_search"); > + DEBUG_ARG("ip = 0x%x", in_ip_addr); > + > + for (i = 0; i < ARP_TABLE_SIZE; i++) { > + if (arptbl->table[i].ar_sip == in_ip_addr) { > + memcpy(out_ethaddr, arptbl->table[i].ar_sha, ETH_ALEN); > + DEBUG_ARGS((dfd, " found hw addr = > %02x:%02x:%02x:%02x:%02x:%02x\n", > + out_ethaddr[0], out_ethaddr[1], out_ethaddr[2], > + out_ethaddr[3], out_ethaddr[4], out_ethaddr[5])); > + return 1; > + } > + } > + > + return 0; > +} > diff --git a/slirp/bootp.c b/slirp/bootp.c > index 1eb2ed1..07cbb80 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); > @@ -164,8 +165,9 @@ static void bootp_reply(Slirp *slirp, const struct > bootp_t *bp) > if (dhcp_msg_type != DHCPDISCOVER && > dhcp_msg_type != DHCPREQUEST) > return; > - /* XXX: this is a hack to get the client mac address */ > - memcpy(slirp->client_ethaddr, bp->bp_hwaddr, 6); > + > + /* Get client's hardware address from bootp request */ > + memcpy(client_ethaddr, bp->bp_hwaddr, ETH_ALEN); > > m = m_get(slirp); > if (!m) { > @@ -178,25 +180,25 @@ static void bootp_reply(Slirp *slirp, const struct > bootp_t *bp) > > if (dhcp_msg_type == DHCPDISCOVER) { > if (preq_addr.s_addr != htonl(0L)) { > - bc = request_addr(slirp, &preq_addr, slirp->client_ethaddr); > + bc = request_addr(slirp, &preq_addr, client_ethaddr); > if (bc) { > daddr.sin_addr = preq_addr; > } > } > if (!bc) { > new_addr: > - bc = get_new_addr(slirp, &daddr.sin_addr, slirp->client_ethaddr); > + bc = get_new_addr(slirp, &daddr.sin_addr, client_ethaddr); > if (!bc) { > DPRINTF("no address left\n"); > return; > } > } > - memcpy(bc->macaddr, slirp->client_ethaddr, 6); > + memcpy(bc->macaddr, client_ethaddr, ETH_ALEN); > } else if (preq_addr.s_addr != htonl(0L)) { > - bc = request_addr(slirp, &preq_addr, slirp->client_ethaddr); > + bc = request_addr(slirp, &preq_addr, client_ethaddr); > if (bc) { > daddr.sin_addr = preq_addr; > - memcpy(bc->macaddr, slirp->client_ethaddr, 6); > + memcpy(bc->macaddr, client_ethaddr, ETH_ALEN); > } else { > daddr.sin_addr.s_addr = 0; > } > @@ -209,6 +211,11 @@ static void bootp_reply(Slirp *slirp, const struct > bootp_t *bp) > } > } > > + if (daddr.sin_addr.s_addr != 0) { > + /* Update ARP table for this IP address */ > + arp_table_add(&slirp->arp_table, daddr.sin_addr.s_addr, > client_ethaddr); Here you prevent that arp_table_add is called with a zero IP, but not in arp_input below. Likely harmless, but at least inconsistent. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux