qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jung-uk Kim <jkim@FreeBSD.org>
To: Sebastian Herbszt <herbszt@gmx.de>
Cc: Juergen Lock <nox@jelal.kn-bremen.de>, qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH] networking using libpcap
Date: Fri, 18 Jul 2008 16:39:47 -0400	[thread overview]
Message-ID: <200807181639.49136.jkim@FreeBSD.org> (raw)
In-Reply-To: <043901c8e8f1$6150b260$0201a8c0@zeug>

On Friday 18 July 2008 12:12 pm, Sebastian Herbszt wrote:
> Jung-uk Kim wrote:
> > Since someone showed interest, I updated my patches against
> > trunk. :-)
> >
> > http://people.freebsd.org/~jkim/qemu-pcap-20080717.diff
> >
> > I turned it off by default for now.  If you want to enable it,
> > do:
> >
> > configure --enable-pcap
>
> To avoid confusion please change the pcap probe to error instead of
> silently setting pcap=no. Take a look at audio_drv_probe.

Done.

> >> Works perfect for me and allows access to the local Ethernet
> >> right out of the box, very much unlike tap and bridging. The
> >> attached version applies to trunk.
> >>
> >> I have modified (e.g. got rid of threads) the original patch
> >> from the forum and am using it here on Windows. It works fine
> >> but performance is pretty low.
> >
> > *After* applying the new patch:
> >
> > cp -p vl.c vl.c.orig
> > sed -e 's/#ifdef PCAP_SET_FILTER/#if 1/g' vl.c > vl.c.tmp
> > mv vl.c.tmp vl.c
> >
> > and try again?  BTW, I have no real experience with WinPcap, so
> > don't kill me if it does not work for you. ;-)
>
> Using pcap_setfilter helps and colinux (conet-bridged-daemon) does
> use one too. Currently your filter is
> "ether dst 52:54:00:12:34:56 or ((broadcast or multicast) and not
> ether src 52:54:00:12:34:56)". The filter used by colinux is
> "(ether dst 00:ff:81:24:00:00) or (ether broadcast or multicast) or
> (ip broadcast or multicast)".
>
> The "and not ether src 52:54:00:12:34:56" part in your filter
> prevents the VM from seeing own packets. It doesn't reply to own
> "ping broadcast" where it does in colinux and VMware Server.

I knew that I might have missed some edge cases. ;-P Corrected and 
enabled by default.

> >> +ifdef CONFIG_PCAP
> >> +LIBS+=-lpcap
> >> +endif
> >>
> >> On Windows it should be -lwpcap.
> >
> > Thanks for the tip!
> >
> >> +    if ((fd = pcap_get_selectable_fd(s->handle)) < 0) {
> >> + fprintf(stderr, "qemu: pcap_get_selectable_fd failed\n");
> >> + goto fail;
> >> +    }
> >> +    qemu_set_fd_handler(fd, pcap_send, NULL, s);
> >>
> >> pcap_get_selectable_fd() is not available on Windows. I just put
> >> pcap_send() in main_loop_wait().
> >
> > I added WinPcap API support from WinPcap manual pages but I have
> > no way of checking.  Can you try the patch and letting me know?
>
> Since you use the winpcap win32 only parts there is the following
> warning:
>
> vl.c: In function `net_pcap_init':
> vl.c:4247: warning: implicit declaration of function
> `pcap_getevent' vl.c:4247: warning: assignment makes pointer from
> integer without a cast
>
> You can avoid it by defining WPCAP in vl.c:
>
> #if defined(CONFIG_PCAP)
> #ifdef _WIN32
> #define WPCAP 1
> #endif
> #include <pcap.h>
> #endif

Thanks for the tip, again!

> I noticed that if no ifname is passed you try to get one with
> pcap_lookupdev. This is broken on winpcap, please see
> http://www.winpcap.org/pipermail/winpcap-bugs/2006-May/000220.html

I don't think it is "broken" on Windows.  The OP was just saying that 
it returns device name in wide characters on Windows.  In fact, both 
tcpdump (3.9.8) and WinDump (3.9.5) seem to do the same (trimmed 
formatting):

-----------------
	if (device == NULL) {
		device = pcap_lookupdev(ebuf);
			if (device == NULL)
				error("%s", ebuf);
	}
#ifdef WIN32
	//we assume that an ASCII string is always longer than 1 char
	if(strlen(device) == 1)
	{	//a Unicode string has a \0 as second byte (so strlen() is 1)
		fprintf(stderr, "%s: listening on %ws\n", program_name, device);
	}
	else
	{
		fprintf(stderr, "%s: listening on %s\n", program_name, device);
	}

	fflush(stderr);
#endif /* WIN32 */
	*ebuf = '\0';
	pd = pcap_open_live(device, snaplen, !pflag, 1000, ebuf);
	if (pd == NULL)
		error("%s", ebuf);
	else if (*ebuf)
		warning("%s", ebuf);
-----------------

The OP had to use %ws format, not just %s, it seems.  In fact, WinPcap 
seems to convert ASCII names to Unicode names unconditionally from 
pcap_lookupdev():

	/*
	 * Windows NT (NT 4.0, W2K, WXP). Convert the names to UNICODE for 
backward compatibility
	 */

> I am not sure if pcap_lookupdev() gets you the "right" interface on
> non-win32, so maybe just require ifname (tap does it too)?

At least, it works for me on FreeBSD. :-) Does WinDump work without 
specifying interface name when there is only one network device?  If 
it does, I am not going to change it.

> Otherwise it seems to compile and run fine.

Please try the updated patch and let me know:

http://people.freebsd.org/~jkim/qemu-pcap-20080718.diff

Note I had to re-shuffle the patch a bit to make it applicable to 
slightly older sources.

Juergen,

I simplified FreeBSD ports patch with the above patch:

http://people.freebsd.org/~jkim/qemu-devel-20080620_1-pcap.diff

Thanks!

Jung-uk Kim

  reply	other threads:[~2008-07-18 20:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-17 22:12 [Qemu-devel] Re: [PATCH] networking using libpcap Jung-uk Kim
2008-07-18 16:12 ` Sebastian Herbszt
2008-07-18 20:39   ` Jung-uk Kim [this message]
2008-07-18 23:07     ` Jung-uk Kim
2008-07-21 15:35     ` Sebastian Herbszt
2008-07-25 20:51     ` Anthony Liguori
2008-07-26 17:30       ` Sebastian Herbszt
2008-07-27  0:28         ` Anthony Liguori
2008-07-27 12:55       ` Paul Brook
2008-07-23  1:55 ` Anthony Liguori
  -- strict thread matches above, loose matches on Subject: below --
2008-07-02 15:02 [Qemu-devel] " Ulrich Hecht
2008-07-02 23:39 ` [Qemu-devel] " Sebastian Herbszt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200807181639.49136.jkim@FreeBSD.org \
    --to=jkim@freebsd.org \
    --cc=herbszt@gmx.de \
    --cc=nox@jelal.kn-bremen.de \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).