[Qemu-devel] net: small fixes

[Qemu-devel] net: small fixes

[Vincent Palatin]

Dear Qemu developers,

While debugging a machine emulation using SLIRP based user networking, I ran into a couple of issues.
Please find attached the patches for them :
1) fix the SLIRP compilation when the debug traces are activated.
2) avoid packet loss with several receivers on the same vlan.

Regards,
-- 
Vincent

[Qemu-devel] [PATCH 1/2] net: fix trace when debug is activated in slirp

[Vincent Palatin]

make the code compile correctly when DEBUG is activated.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
---
 slirp/bootp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/slirp/bootp.c b/slirp/bootp.c
index 0905c6d..1eb2ed1 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -284,7 +284,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
     } else {
         static const char nak_msg[] = "requested address not available";
 
-        DPRINTF("nak'ed addr=%08x\n", ntohl(preq_addr->s_addr));
+        DPRINTF("nak'ed addr=%08x\n", ntohl(preq_addr.s_addr));
 
         *q++ = RFC2132_MSG_TYPE;
         *q++ = 1;
-- 
1.7.3.1

[Qemu-devel] [PATCH 2/2] net: fix qemu_can_send_packet logic

[Vincent Palatin]

If any of the clients is not ready to receive (ie it has a can_receive
callback and can_receive() returns false), we don't want to start
sending, else this client may miss/discard the packet.

I got this behaviour with the following setup :
the emulated machine is using an USB-ethernet adapter, it is connected
to the network using SLIRP and I'm dumping the traffic in a .pcap file.
As per the following command line :
-net nic,model=usb,vlan=1 -net user,vlan=1 -net dump,vlan=1,file=/tmp/pkt.pcap
Every time that two packets are coming in a row from the host, the
usb-net code will receive the first one, then returns 0 to can_receive
call since it has a 1 packet long queue. But as the dump code is always
ready to receive, qemu_can_send_packet will return true and the next
packet will discard the previous one in the usb-net code.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
---
 net.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net.c b/net.c
index ec4745d..72ac4cf 100644
--- a/net.c
+++ b/net.c
@@ -411,11 +411,11 @@ int qemu_can_send_packet(VLANClientState *sender)
         }
 
         /* no can_receive() handler, they can always receive */
-        if (!vc->info->can_receive || vc->info->can_receive(vc)) {
-            return 1;
+        if (vc->info->can_receive && !vc->info->can_receive(vc)) {
+            return 0;
         }
     }
-    return 0;
+    return 1;
 }
 
 static ssize_t qemu_deliver_packet(VLANClientState *sender,
-- 
1.7.3.1

Re: [Qemu-devel] [PATCH 1/2] net: fix trace when debug is activated in slirp

[Stefan Hajnoczi]

On Wed, Mar 2, 2011 at 10:25 PM, Vincent Palatin <vpalatin@chromium.org> wrote:
> make the code compile correctly when DEBUG is activated.
>
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> ---
>  slirp/bootp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 0905c6d..1eb2ed1 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -284,7 +284,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>     } else {
>         static const char nak_msg[] = "requested address not available";
>
> -        DPRINTF("nak'ed addr=%08x\n", ntohl(preq_addr->s_addr));
> +        DPRINTF("nak'ed addr=%08x\n", ntohl(preq_addr.s_addr));

Looks good.

By the way, this is why it's nicer to rely on compiler dead code
elimination than #ifdefing out code:

#define DEBUG_ENABLED 0

static inline void dprintf(const char *fmt, ...)
{
    if (DEBUG_ENABLED) {
        va_list args;

        va_start(args, fmt);
        vfprintf(stderr, fmt, args);
        va_end(args);
    }
}

dprintf() callers will still have their arguments parsed and checked
by the compiler, even though it all compiles out when DEBUG_ENABLED is
0.

Also, QEMU tracing is often a better choice for new debug
instrumentation although for existing code there are lots of DPRINTF()
users today.

Stefan

Re: [Qemu-devel] [PATCH 1/2] net: fix trace when debug is activated in slirp

[Blue Swirl]

Thanks, applied both.

On Thu, Mar 3, 2011 at 12:25 AM, Vincent Palatin <vpalatin@chromium.org> wrote:
> make the code compile correctly when DEBUG is activated.
>
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> ---
>  slirp/bootp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 0905c6d..1eb2ed1 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -284,7 +284,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>     } else {
>         static const char nak_msg[] = "requested address not available";
>
> -        DPRINTF("nak'ed addr=%08x\n", ntohl(preq_addr->s_addr));
> +        DPRINTF("nak'ed addr=%08x\n", ntohl(preq_addr.s_addr));
>
>         *q++ = RFC2132_MSG_TYPE;
>         *q++ = 1;
> --
> 1.7.3.1
>
>
>