| Message ID | 1311956703-24788-1-git-send-email-chouteau@adacore.com |
|---|---|
| State | New |
| Headers | show |
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 <chouteau@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
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 <chouteau@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. > Unfortunately, this patch also has a more sever issues: it breaks DHCP e.g. Slirp's bootp sends out all replies, also acks and offers, as broadcasts. Normal servers already use the clients IP address as destination here. Even if bootp is fixed, you still lack logic to deal with special addresses in your arp table lookup. At least broadcasts need to be handled, I think other multicasts aren't supported by slirp anyway. Jan
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 <chouteau@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. >> > > Unfortunately, this patch also has a more sever issues: it breaks DHCP e.g. > > Slirp's bootp sends out all replies, also acks and offers, as > broadcasts. Normal servers already use the clients IP address as > destination here. Obviously, using a broadcast address on return is valid as well. So this is no slirp bug, it's purely an ARP table lookup issue introduced by this patch. > > Even if bootp is fixed, you still lack logic to deal with special > addresses in your arp table lookup. At least broadcasts need to be > handled, I think other multicasts aren't supported by slirp anyway. > Jan
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 <chouteau@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... >>> >>>> +{ >>>> + 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. >>>> diff --git a/slirp/bootp.c b/slirp/bootp.c >>>> index 1eb2ed1..07cbb80 100644 >>>> --- a/slirp/bootp.c >>>> +++ b/slirp/bootp.c >>>> @@ -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. I will handle this case directly in arp_table_add to get a consistent behavior. >> >> Unfortunately, this patch also has a more sever issues: it breaks DHCP e.g. >> >> Slirp's bootp sends out all replies, also acks and offers, as >> broadcasts. Normal servers already use the clients IP address as >> destination here. > > Obviously, using a broadcast address on return is valid as well. So this > is no slirp bug, it's purely an ARP table lookup issue introduced by > this patch. > >> >> Even if bootp is fixed, you still lack logic to deal with special >> addresses in your arp table lookup. At least broadcasts need to be >> handled, I think other multicasts aren't supported by slirp anyway. >> > That's right, I will add broadcast handling in the next version. Thanks for your review.
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 <chouteau@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. :) > >>>> >>>>> +{ >>>>> + 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. Jan
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 <chouteau@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,
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]) +{ + 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++) { + 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); + } + saddr.sin_addr = slirp->vhost_addr; saddr.sin_port = htons(BOOTP_SERVER); @@ -218,7 +225,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) rbp->bp_xid = bp->bp_xid; rbp->bp_htype = 1; rbp->bp_hlen = 6; - memcpy(rbp->bp_hwaddr, bp->bp_hwaddr, 6); + memcpy(rbp->bp_hwaddr, bp->bp_hwaddr, ETH_ALEN); rbp->bp_yiaddr = daddr.sin_addr; /* Client IP address */ rbp->bp_siaddr = saddr.sin_addr; /* Server IP address */ diff --git a/slirp/slirp.c b/slirp/slirp.c index df787ea..ba76757 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -31,11 +31,11 @@ struct in_addr loopback_addr; /* emulated hosts use the MAC addr 52:55:IP:IP:IP:IP */ -static const uint8_t special_ethaddr[6] = { +static const uint8_t special_ethaddr[ETH_ALEN] = { 0x52, 0x55, 0x00, 0x00, 0x00, 0x00 }; -static const uint8_t zero_ethaddr[6] = { 0, 0, 0, 0, 0, 0 }; +static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 }; /* XXX: suppress those select globals */ fd_set *global_readfds, *global_writefds, *global_xfds; @@ -599,42 +599,8 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, global_xfds = NULL; } -#define ETH_ALEN 6 -#define ETH_HLEN 14 - -#define ETH_P_IP 0x0800 /* Internet Protocol packet */ -#define ETH_P_ARP 0x0806 /* Address Resolution packet */ - -#define ARPOP_REQUEST 1 /* ARP request */ -#define ARPOP_REPLY 2 /* ARP reply */ - -struct ethhdr -{ - unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ - unsigned char h_source[ETH_ALEN]; /* source ether addr */ - unsigned short h_proto; /* packet type ID field */ -}; - -struct arphdr -{ - unsigned short ar_hrd; /* format of hardware address */ - unsigned short ar_pro; /* format of protocol address */ - unsigned char ar_hln; /* length of hardware address */ - unsigned char ar_pln; /* length of protocol address */ - unsigned short ar_op; /* ARP opcode (command) */ - - /* - * Ethernet looks like this : This bit is variable sized however... - */ - unsigned char ar_sha[ETH_ALEN]; /* sender hardware address */ - uint32_t ar_sip; /* sender IP address */ - unsigned char ar_tha[ETH_ALEN]; /* target hardware address */ - uint32_t ar_tip ; /* target IP address */ -} __attribute__((packed)); - static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) { - struct ethhdr *eh = (struct ethhdr *)pkt; struct arphdr *ah = (struct arphdr *)(pkt + ETH_HLEN); uint8_t arp_reply[max(ETH_HLEN + sizeof(struct arphdr), 64)]; struct ethhdr *reh = (struct ethhdr *)arp_reply; @@ -645,6 +611,12 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) ar_op = ntohs(ah->ar_op); switch(ar_op) { case ARPOP_REQUEST: + if (ah->ar_tip == ah->ar_sip) { + /* Gratuitous ARP */ + arp_table_add(&slirp->arp_table, ah->ar_sip, ah->ar_sha); + return; + } + if ((ah->ar_tip & slirp->vnetwork_mask.s_addr) == slirp->vnetwork_addr.s_addr) { if (ah->ar_tip == slirp->vnameserver_addr.s_addr || @@ -657,8 +629,8 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) return; arp_ok: memset(arp_reply, 0, sizeof(arp_reply)); - /* XXX: make an ARP request to have the client address */ - memcpy(slirp->client_ethaddr, eh->h_source, ETH_ALEN); + + arp_table_add(&slirp->arp_table, ah->ar_sip, ah->ar_sha); /* ARP request for alias/dns mac address */ memcpy(reh->h_dest, pkt + ETH_ALEN, ETH_ALEN); @@ -679,11 +651,7 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) } break; case ARPOP_REPLY: - /* reply to request of client mac address ? */ - if (!memcmp(slirp->client_ethaddr, zero_ethaddr, ETH_ALEN) && - ah->ar_sip == slirp->client_ipaddr.s_addr) { - memcpy(slirp->client_ethaddr, ah->ar_sha, ETH_ALEN); - } + arp_table_add(&slirp->arp_table, ah->ar_sip, ah->ar_sha); break; default: break; @@ -729,15 +697,16 @@ void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len) { uint8_t buf[1600]; struct ethhdr *eh = (struct ethhdr *)buf; + uint8_t ethaddr[ETH_ALEN]; + const struct ip *iph = (const struct ip *)ip_data; if (ip_data_len + ETH_HLEN > sizeof(buf)) return; - - if (!memcmp(slirp->client_ethaddr, zero_ethaddr, ETH_ALEN)) { + + if (!arp_table_search(&slirp->arp_table, iph->ip_dst.s_addr, ethaddr)) { uint8_t arp_req[ETH_HLEN + sizeof(struct arphdr)]; struct ethhdr *reh = (struct ethhdr *)arp_req; struct arphdr *rah = (struct arphdr *)(arp_req + ETH_HLEN); - const struct ip *iph = (const struct ip *)ip_data; /* If the client addr is not known, there is no point in sending the packet to it. Normally the sender should have @@ -765,7 +734,7 @@ void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len) slirp->client_ipaddr = iph->ip_dst; slirp_output(slirp->opaque, arp_req, sizeof(arp_req)); } else { - memcpy(eh->h_dest, slirp->client_ethaddr, ETH_ALEN); + memcpy(eh->h_dest, ethaddr, ETH_ALEN); memcpy(eh->h_source, special_ethaddr, ETH_ALEN - 4); /* XXX: not correct */ memcpy(&eh->h_source[2], &slirp->vhost_addr, 4); diff --git a/slirp/slirp.h b/slirp/slirp.h index 16bb6ba..103b39d 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -170,6 +170,51 @@ int inet_aton(const char *cp, struct in_addr *ia); /* osdep.c */ int qemu_socket(int domain, int type, int protocol); +#define ETH_ALEN 6 +#define ETH_HLEN 14 + +#define ETH_P_IP 0x0800 /* Internet Protocol packet */ +#define ETH_P_ARP 0x0806 /* Address Resolution packet */ + +#define ARPOP_REQUEST 1 /* ARP request */ +#define ARPOP_REPLY 2 /* ARP reply */ + +struct ethhdr { + unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ + unsigned char h_source[ETH_ALEN]; /* source ether addr */ + unsigned short h_proto; /* packet type ID field */ +}; + +struct arphdr { + unsigned short ar_hrd; /* format of hardware address */ + unsigned short ar_pro; /* format of protocol address */ + unsigned char ar_hln; /* length of hardware address */ + unsigned char ar_pln; /* length of protocol address */ + unsigned short ar_op; /* ARP opcode (command) */ + + /* + * Ethernet looks like this : This bit is variable sized however... + */ + unsigned char ar_sha[ETH_ALEN]; /* sender hardware address */ + uint32_t ar_sip; /* sender IP address */ + unsigned char ar_tha[ETH_ALEN]; /* target hardware address */ + uint32_t ar_tip; /* target IP address */ +} __attribute__((packed)); + +#define ARP_TABLE_SIZE 16 + +typedef struct ArpTable { + struct arphdr table[ARP_TABLE_SIZE]; + int next_victim; +} ArpTable; + +void arp_table_add(ArpTable *arptbl, + int ip_addr, + uint8_t ethaddr[ETH_ALEN]); + +bool arp_table_search(ArpTable *arptbl, + int in_ip_addr, + uint8_t out_ethaddr[ETH_ALEN]); struct Slirp { QTAILQ_ENTRY(Slirp) entry; @@ -181,9 +226,6 @@ struct Slirp { struct in_addr vdhcp_startaddr; struct in_addr vnameserver_addr; - /* ARP cache for the guest IP addresses (XXX: allow many entries) */ - uint8_t client_ethaddr[6]; - struct in_addr client_ipaddr; char client_hostname[33]; @@ -227,6 +269,8 @@ struct Slirp { char *tftp_prefix; struct tftp_session tftp_sessions[TFTP_SESSIONS_MAX]; + ArpTable arp_table; + void *opaque; };
This patch adds a simple ARP table in Slirp and also adds handling of gratuitous ARP requests. Signed-off-by: Fabien Chouteau <chouteau@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