[Qemu-devel] [PATCH 0/3] [PULL] slirp: ARP table related fixes

[Qemu-devel] [PATCH 0/3] [PULL] slirp: ARP table related fixes

[Jan Kiszka]

The following changes since commit 81e34a2401f7ffd519bb7f093e833cb48734169f:

  Merge remote-tracking branch 'mst/for_anthony' into staging (2011-08-04 17:15:22 -0500)

are available in the git repository at:

  git://git.kiszka.org/qemu.git queues/slirp

Fixes some fall-outs from the latest merge, ie. a build breakage on
mingw and a regression that broke at least ping.


CC: Gerd Hoffmann <kraxel@redhat.com>

Jan Kiszka (3):
  slirp: Fix types of IP address parameters
  slirp: Read current time only once per if_start call
  slirp: Only start packet expiration for delayed ones

 slirp/arp_table.c |   16 ++++++++--------
 slirp/if.c        |    8 +-------
 slirp/slirp.c     |    3 +++
 slirp/slirp.h     |    4 ++--
 4 files changed, 14 insertions(+), 17 deletions(-)

-- 
1.7.3.4

[Qemu-devel] [PATCH 1/3] slirp: Fix types of IP address parameters

[Jan Kiszka]

Should be uint32_t for IPv4, not int. Also avoid in_addr_t without
proper includes. Fixes build regression on mingw32.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 slirp/arp_table.c |   16 ++++++++--------
 slirp/slirp.h     |    4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index 820dee2..5d7b8ac 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -24,9 +24,9 @@
 
 #include "slirp.h"
 
-void arp_table_add(Slirp *slirp, int ip_addr, uint8_t ethaddr[ETH_ALEN])
+void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN])
 {
-    const in_addr_t broadcast_addr =
+    const uint32_t broadcast_addr =
         ~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr;
     ArpTable *arptbl = &slirp->arp_table;
     int i;
@@ -60,29 +60,29 @@ void arp_table_add(Slirp *slirp, int ip_addr, uint8_t ethaddr[ETH_ALEN])
     arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;
 }
 
-bool arp_table_search(Slirp *slirp, int in_ip_addr,
+bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
                       uint8_t out_ethaddr[ETH_ALEN])
 {
-    const in_addr_t broadcast_addr =
+    const uint32_t broadcast_addr =
         ~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr;
     ArpTable *arptbl = &slirp->arp_table;
     int i;
 
     DEBUG_CALL("arp_table_search");
-    DEBUG_ARG("ip = 0x%x", in_ip_addr);
+    DEBUG_ARG("ip = 0x%x", ip_addr);
 
     /* Check 0.0.0.0/8 invalid source-only addresses */
-    assert((in_ip_addr & htonl(~(0xf << 28))) != 0);
+    assert((ip_addr & htonl(~(0xf << 28))) != 0);
 
     /* If broadcast address */
-    if (in_ip_addr == 0xffffffff || in_ip_addr == broadcast_addr) {
+    if (ip_addr == 0xffffffff || ip_addr == broadcast_addr) {
         /* return Ethernet broadcast address */
         memset(out_ethaddr, 0xff, ETH_ALEN);
         return 1;
     }
 
     for (i = 0; i < ARP_TABLE_SIZE; i++) {
-        if (arptbl->table[i].ar_sip == in_ip_addr) {
+        if (arptbl->table[i].ar_sip == 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],
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 2a070e6..dcf99d5 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -208,9 +208,9 @@ typedef struct ArpTable {
     int next_victim;
 } ArpTable;
 
-void arp_table_add(Slirp *slirp, int ip_addr, uint8_t ethaddr[ETH_ALEN]);
+void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN]);
 
-bool arp_table_search(Slirp *slirp, int in_ip_addr,
+bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
                       uint8_t out_ethaddr[ETH_ALEN]);
 
 struct Slirp {
-- 
1.7.3.4

[Qemu-devel] [PATCH 2/3] slirp: Read current time only once per if_start call

[Jan Kiszka]

No need to update the current time for each packet we send from the
queue. Processing time is comparably short.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 slirp/if.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/slirp/if.c b/slirp/if.c
index 2d79e45..47bebe4 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -157,9 +157,8 @@ diddit:
 void
 if_start(Slirp *slirp)
 {
+    uint64_t now = qemu_get_clock_ns(rt_clock);
     int requeued = 0;
-    uint64_t now;
-
 	struct mbuf *ifm, *ifqt;
 
 	DEBUG_CALL("if_start");
@@ -172,8 +171,6 @@ if_start(Slirp *slirp)
         if (!slirp_can_output(slirp->opaque))
             return;
 
-        now = qemu_get_clock_ns(rt_clock);
-
 	/*
 	 * See which queue to get next packet from
 	 * If there's something in the fastq, select it immediately
-- 
1.7.3.4

[Qemu-devel] [PATCH 3/3] slirp: Only start packet expiration for delayed ones

[Jan Kiszka]

The expiration timeout must only affect packets that are queued due to
pending ARP resolutions. The old version broke ping e.g.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 slirp/if.c    |    3 ---
 slirp/slirp.c |    3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/slirp/if.c b/slirp/if.c
index 47bebe4..2852396 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -106,9 +106,6 @@ if_output(struct socket *so, struct mbuf *ifm)
 	ifs_init(ifm);
 	insque(ifm, ifq);
 
-        /* Expiration date = Now + 1 second */
-        ifm->expiration_date = qemu_get_clock_ns(rt_clock) + 1000000000ULL;
-
 diddit:
 	slirp->if_queued++;
 
diff --git a/slirp/slirp.c b/slirp/slirp.c
index a86cc6e..2c242ef 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -738,6 +738,9 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
             slirp->client_ipaddr = iph->ip_dst;
             slirp_output(slirp->opaque, arp_req, sizeof(arp_req));
             ifm->arp_requested = true;
+
+            /* Expire request and drop outgoing packet after 1 second */
+            ifm->expiration_date = qemu_get_clock_ns(rt_clock) + 1000000000ULL;
         }
         return 0;
     } else {
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH 0/3] [PULL] slirp: ARP table related fixes

[Anthony Liguori]

On 08/05/2011 08:32 AM, Jan Kiszka wrote:
> The following changes since commit 81e34a2401f7ffd519bb7f093e833cb48734169f:
>
>    Merge remote-tracking branch 'mst/for_anthony' into staging (2011-08-04 17:15:22 -0500)
>
> are available in the git repository at:
>
>    git://git.kiszka.org/qemu.git queues/slirp
>
> Fixes some fall-outs from the latest merge, ie. a build breakage on
> mingw and a regression that broke at least ping.
>
>
> CC: Gerd Hoffmann<kraxel@redhat.com>

Pulled.  Thanks.

Regards,

Anthony Liguori

>
> Jan Kiszka (3):
>    slirp: Fix types of IP address parameters
>    slirp: Read current time only once per if_start call
>    slirp: Only start packet expiration for delayed ones
>
>   slirp/arp_table.c |   16 ++++++++--------
>   slirp/if.c        |    8 +-------
>   slirp/slirp.c     |    3 +++
>   slirp/slirp.h     |    4 ++--
>   4 files changed, 14 insertions(+), 17 deletions(-)
>