Edgar E. Iglesias, le Thu 08 May 2014 06:59:22 +0000, a écrit :
> On Thu, May 08, 2014 at 08:50:33AM +0200, Samuel Thibault wrote:
> > Edgar E. Iglesias, le Thu 08 May 2014 06:10:18 +0000, a écrit :
> > > The search part looks OK to me but when adding to the arp table, don't
> > > you at least want to avoid adding mappings for 0.0.0.0/32?
> >
> > I don't see the gain, actually. It would mean burning some CPU all the
> > time just to save a small potential memory loss and CPU burning in the
> > rare case when the guest behaves oddly.
> >
> > > to avoid for ex garps to pollute the cache with invalid entries?
> >
> > Only one entry will be created and updated by garps. The guest already
> > has a lot of ways to pollute the cache :)
>
> I was under the impression that entries for 0.0.0.0 are strictly
> invalid (not about performance). I might be wrong though.
I'd tend to think that, but what should be done? I don't think we want
an assert failure :)
At best I could think of using the patch below, which avoids registering
anything for 0.0.0.0, and use a broadcast to answer a guest which
would have used 0.0.0.0 as a source for whatever reason. I don't find
anything else reasonable. What would be preferred?
Samuel
diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index ecdb0ba..d160cfc 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -37,12 +37,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t
ethaddr[ETH_ALEN])
ethaddr[0], ethaddr[1], ethaddr[2],
ethaddr[3], ethaddr[4], ethaddr[5]));
- /* Check 0.0.0.0/8 invalid source-only addresses */
- if ((ip_addr & htonl(~(0xfU << 28))) == 0) {
- return;
- }
-
- if (ip_addr == 0xffffffff || ip_addr == broadcast_addr) {
+ if (ip_addr == 0 || ip_addr == 0xffffffff || ip_addr == broadcast_addr) {
/* Do not register broadcast addresses */
return;
}
@@ -73,11 +68,8 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
DEBUG_CALL("arp_table_search");
DEBUG_ARG("ip = 0x%x", ip_addr);
- /* Check 0.0.0.0/8 invalid source-only addresses */
- assert((ip_addr & htonl(~(0xfU << 28))) != 0);
-
/* If broadcast address */
- if (ip_addr == 0xffffffff || ip_addr == broadcast_addr) {
+ if (ip_addr == 0 || ip_addr == 0xffffffff || ip_addr == broadcast_addr) {
/* return Ethernet broadcast address */
memset(out_ethaddr, 0xff, ETH_ALEN);
return 1;
