[uml-devel] regression: pcap broken in 2.6.20.x

[uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Using the exact same kernel config with 2.6.19.x works but not with 2.6.20.x

# ifconfig eth1 0.0.0.0 promisc up
SIOCSIFADDR: No such device
eth1: unknown interface: No such device
eth1: unknown interface: No such device
eth1: unknown interface: No such device

Antoine
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGK6zIGK2zHPGK1rsRCvlWAJ4mPnP1dQQgol4Bpx4+CFt/bnEHOgCfQfDC
7KG75RhnxZiS98gsVA+nURE=
=wsN+
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Jeff Dike]

On Sun, Apr 22, 2007 at 07:43:21PM +0100, Antoine Martin wrote:
> Using the exact same kernel config with 2.6.19.x works but not with 2.6.20.x
> 
> # ifconfig eth1 0.0.0.0 promisc up
> SIOCSIFADDR: No such device
> eth1: unknown interface: No such device
> eth1: unknown interface: No such device
> eth1: unknown interface: No such device

Works here - what do ifconfig -a and dmesg say?

				Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Jeff Dike wrote:
> On Sun, Apr 22, 2007 at 07:43:21PM +0100, Antoine Martin wrote:
>> Using the exact same kernel config with 2.6.19.x works but not with 2.6.20.x
>>
>> # ifconfig eth1 0.0.0.0 promisc up
>> SIOCSIFADDR: No such device
>> eth1: unknown interface: No such device
>> eth1: unknown interface: No such device
>> eth1: unknown interface: No such device
> 
> Works here - what do ifconfig -a and dmesg say?
Ah, DOH!
Could it be that the randomization of the mac address made it appear as
eth4 and not eth1... (why eth4 and not eth3 or eth2, beats me)
Until now all kernels made it appear as eth1.

Is there any chance of adding the ability to set the mac address of the
interface as is currently the case for tuntap?
ie: eth1=pcap,00:00:00:01:02:03,eth0[,filters]

Thanks
Antoine
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGL8FCGK2zHPGK1rsRCgNaAJ4gS5hW4Fg3+cd9L17TkooKKD9N4QCfS+Fs
P0uU130bSLX81MjbNAL/RdI=
=jOcH
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Blaisorblade]

On mercoledì 25 aprile 2007, Antoine Martin wrote:
> Jeff Dike wrote:
> > On Sun, Apr 22, 2007 at 07:43:21PM +0100, Antoine Martin wrote:
> >> Using the exact same kernel config with 2.6.19.x works but not with
> >> 2.6.20.x
> >>
> >> # ifconfig eth1 0.0.0.0 promisc up
> >> SIOCSIFADDR: No such device
> >> eth1: unknown interface: No such device
> >> eth1: unknown interface: No such device
> >> eth1: unknown interface: No such device
> >
> > Works here - what do ifconfig -a and dmesg say?
>
> Ah, DOH!
> Could it be that the randomization of the mac address made it appear as
> eth4 and not eth1... (why eth4 and not eth3 or eth2, beats me)
> Until now all kernels made it appear as eth1.

Hmm... Ubuntu renames interfaces based on their MAC address, which are listed 
in /etc/iftab. Dunno about other distributions.

If there are so many interfaces and they were just reordered, this could be 
caused by a (non-existant) change in link order. Or (I don't know how) it 
could derive some way from the fact that interfaces are initialized later 
(instead of __initcall, late_initcall is now used).
-- 
Inform me of my mistakes, so I can add them to my list!
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Jeff Dike]

On Thu, Apr 26, 2007 at 04:12:40PM +0200, Blaisorblade wrote:
> If there are so many interfaces and they were just reordered, this could be 
> caused by a (non-existant) change in link order. Or (I don't know how) it 
> could derive some way from the fact that interfaces are initialized later 
> (instead of __initcall, late_initcall is now used).

No, there's a database of interface names and MAC addresses somewhere.
Distros have started making sure that ethx is always the same device
by making sure that it always has the same MAC.  So when a device
shows up with a different MAC, it gets a new name.

The easiest way to work around this is to provide a MAC on the UML
command line.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Jeff Dike wrote:
> On Thu, Apr 26, 2007 at 04:12:40PM +0200, Blaisorblade wrote:
>> If there are so many interfaces and they were just reordered, this could be 
>> caused by a (non-existant) change in link order. Or (I don't know how) it 
>> could derive some way from the fact that interfaces are initialized later 
>> (instead of __initcall, late_initcall is now used).
> 
> No, there's a database of interface names and MAC addresses somewhere.
> Distros have started making sure that ethx is always the same device
> by making sure that it always has the same MAC.  So when a device
> shows up with a different MAC, it gets a new name.
Question remains as to why the mac address was the same until 2.6.20 and
is now different.

> 
> The easiest way to work around this is to provide a MAC on the UML
> command line.
Except, pcap doesn't have the option.
I am looking at pcap_user.c and pcap_kern.c trying to figure out where I
can add parsing for that.

Antoine
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGMhTmGK2zHPGK1rsRCkwAAJ0TPF5I088YhGv1QHQWhe5wmnsWwACfSLk2
e1tMjWEtWYMJDQ33vGqQUIg=
=ljCV
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

>> The easiest way to work around this is to provide a MAC on the UML
>> command line.
> Except, pcap doesn't have the option.
> I am looking at pcap_user.c and pcap_kern.c trying to figure out where I
> can add parsing for that.
in: check_transport(struct transport *transport, char *eth, int n,
                           void **init_out, char **mac_out)

There is a:
	if(!transport->setup(eth, mac_out, *init_out)){


So I guess that I should be looking in pcap_setup(char *str, char
**mac_out, void *data) in pcap_kern.c?
To populate mac_out.
Right?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGMhaVGK2zHPGK1rsRCmiTAJ96Pqxr/ioaDz5ngEfHU4EVHGCgYACfRS4M
iT62rIGq6yJ3LiOZOz6WtKM=
=Oxzw
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Jeff Dike]

On Fri, Apr 27, 2007 at 04:21:11PM +0100, Antoine Martin wrote:
> Question remains as to why the mac address was the same until 2.6.20 and
> is now different.

We started providing randomized MACs at bootup instead of deriving
them from the IP when the interface was brought up.  The old behavior
broke when distros started bringing interfaces up without assigning
IPs to them.

> Except, pcap doesn't have the option.

Hmmm that would be a problem.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Jeff Dike wrote:
> On Fri, Apr 27, 2007 at 04:21:11PM +0100, Antoine Martin wrote:
>> Question remains as to why the mac address was the same until 2.6.20 and
>> is now different.
> 
> We started providing randomized MACs at bootup instead of deriving
> them from the IP when the interface was brought up.  The old behavior
> broke when distros started bringing interfaces up without assigning
> IPs to them.
> 
>> Except, pcap doesn't have the option.
> 
> Hmmm that would be a problem.
:)
I've got a patch - coming soon (tm)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGMiT4GK2zHPGK1rsRCuDUAJ46JsqrHWxmo2FfxRsmasKpi0VIZACfRtzP
dsp1RvyxpqNX8qK2wANFI4g=
=vSOe
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Antoine Martin wrote:
> Jeff Dike wrote:
>> On Fri, Apr 27, 2007 at 04:21:11PM +0100, Antoine Martin wrote:
>>> Question remains as to why the mac address was the same until 2.6.20 and
>>> is now different.
>> We started providing randomized MACs at bootup instead of deriving
>> them from the IP when the interface was brought up.  The old behavior
>> broke when distros started bringing interfaces up without assigning
>> IPs to them.
> 
>>> Except, pcap doesn't have the option.
>> Hmmm that would be a problem.
> :)
> I've got a patch - coming soon (tm)
Attached.

Not sure which approach is best:

1) this one requires a command line like:
"vmlinux eth0=pcap,eth0,,mac=00:01:02:03:04:05"

- --- a/arch/um/drivers/pcap_kern.c   2007-02-04 18:44:54.000000000 +0000
+++ b/arch/um/drivers/pcap_kern.c   2007-04-27 17:40:15.000000000 +0100
@@ -15,6 +15,7 @@
        int promisc;
        int optimize;
        char *filter;
+       char *mac;
 };

 void pcap_init(struct net_device *dev, void *data)
@@ -56,17 +57,18 @@
 int pcap_setup(char *str, char **mac_out, void *data)
 {
        struct pcap_init *init = data;
- -       char *remain, *host_if = NULL, *options[2] = { NULL, NULL };
+       char *remain, *host_if = NULL, *options[3] = { NULL, NULL, NULL };
        int i;

        *init = ((struct pcap_init)
                { .host_if      = "eth0",
                  .promisc      = 1,
                  .optimize     = 0,
- -                 .filter       = NULL });
+                 .filter       = NULL,
+                 .mac          = "00:00:00:00:00:00" });

        remain = split_if_spec(str, &host_if, &init->filter,
- -                              &options[0], &options[1], NULL);
+                              &options[0], &options[1], &options[2], NULL);
        if(remain != NULL){
                printk(KERN_ERR "pcap_setup - Extra garbage on "
                       "specification : '%s'\n", remain);
@@ -87,6 +89,10 @@
                        init->optimize = 1;
                else if(!strcmp(options[i], "nooptimize"))
                        init->optimize = 0;
+               else if(!strncmp(options[i], "mac=", 4) &&
strlen(options[i])==21) {
+                       strncpy(init->mac, options[i]+4, 17);
+                       *mac_out = init->mac;
+               }
                else printk("pcap_setup : bad option - '%s'\n", options[i]);
        }



2) or this one: which just assumes that the mac address is any option
that isn't [no]promisc or [no]optimize and is 17 characters long (I
don't see much point in doing extra checks since this is check in
setup_ether_xx later on)

- --- /uml/kernels/src/linux-2.6.20.9-amd64/arch/um/drivers/pcap_kern.c
2007-02-04 18:44:54.000000000 +0000
+++ pcap_kern.c 2007-04-27 17:45:31.000000000 +0100
@@ -15,6 +15,7 @@
        int promisc;
        int optimize;
        char *filter;
+       char *mac;
 };

 void pcap_init(struct net_device *dev, void *data)
@@ -56,17 +57,18 @@
 int pcap_setup(char *str, char **mac_out, void *data)
 {
        struct pcap_init *init = data;
- -       char *remain, *host_if = NULL, *options[2] = { NULL, NULL };
+       char *remain, *host_if = NULL, *options[3] = { NULL, NULL, NULL };
        int i;

        *init = ((struct pcap_init)
                { .host_if      = "eth0",
                  .promisc      = 1,
                  .optimize     = 0,
- -                 .filter       = NULL });
+                 .filter       = NULL,
+                 .mac          = "00:00:00:00:00:00" });

        remain = split_if_spec(str, &host_if, &init->filter,
- -                              &options[0], &options[1], NULL);
+                              &options[0], &options[1], &options[2], NULL);
        if(remain != NULL){
                printk(KERN_ERR "pcap_setup - Extra garbage on "
                       "specification : '%s'\n", remain);
@@ -87,6 +89,11 @@
                        init->optimize = 1;
                else if(!strcmp(options[i], "nooptimize"))
                        init->optimize = 0;
+               else if(strlen(options[i])==17) {
+                       //a mac address is always 17 characters long
+                       strncpy(init->mac, options[i], 17);
+                       *mac_out = init->mac;
+               }
                else printk("pcap_setup : bad option - '%s'\n", options[i]);
        }
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGMijlGK2zHPGK1rsRCp1LAJ4wKs6o+Wv8dd2VCUO1Zd9jKKhoywCdF9FK
P6meCx9gwAFlI2+JelLDB7U=
=Yzfw
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Antoine Martin wrote:
> Antoine Martin wrote:
>> Jeff Dike wrote:
>>> On Fri, Apr 27, 2007 at 04:21:11PM +0100, Antoine Martin wrote:
>>>> Question remains as to why the mac address was the same until 2.6.20 and
>>>> is now different.
>>> We started providing randomized MACs at bootup instead of deriving
>>> them from the IP when the interface was brought up.  The old behavior
>>> broke when distros started bringing interfaces up without assigning
>>> IPs to them.
>>>> Except, pcap doesn't have the option.
>>> Hmmm that would be a problem.
>> :)
>> I've got a patch - coming soon (tm)
> Attached.
> 
Also here (after some thought, I prefer the second one):
http://uml.nagafix.co.uk/pcap-macaddr.patch
Any objections to merging?

Antoine
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGMir+GK2zHPGK1rsRCsAiAJ9NHPOvA2ZAiSWiMrjQCZtJy1Q58ACdHmxN
3se61pCBjtdJy6FAbasHkTY=
=WltA
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Jeff Dike]

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

On Fri, Apr 27, 2007 at 05:46:29PM +0100, Antoine Martin wrote:
> Attached.
> 
> Not sure which approach is best:
> 
> 1) this one requires a command line like:
> "vmlinux eth0=pcap,eth0,,mac=00:01:02:03:04:05"

> 2) or this one: which just assumes that the mac address is any option
> that isn't [no]promisc or [no]optimize and is 17 characters long (I
> don't see much point in doing extra checks since this is check in
> setup_ether_xx later on)

Or, how about this:

Index: linux-2.6.21-mm/arch/um/drivers/pcap_kern.c
===================================================================
--- linux-2.6.21-mm.orig/arch/um/drivers/pcap_kern.c	2007-04-27 16:15:41.000000000 -0400
+++ linux-2.6.21-mm/arch/um/drivers/pcap_kern.c	2007-04-27 16:15:57.000000000 -0400
@@ -70,7 +70,7 @@ int pcap_setup(char *str, char **mac_out
 		  .filter 	= NULL });
 
 	remain = split_if_spec(str, &host_if, &init->filter,
-			       &options[0], &options[1], NULL);
+			       &options[0], &options[1], mac_out, NULL);
 	if(remain != NULL){
 		printk(KERN_ERR "pcap_setup - Extra garbage on "
 		       "specification : '%s'\n", remain);

You end up using it like so:

eth0=pcap,eth0,tcp,,,2:1:2:3:4:5

You need to toss in some commas so the MAC doesn't get mistaken for a
pcap option.  If you have violent objections to this, we can make this
work with your 'mac=blah' thing.  I like it because it's more
consistent with how everything else specifies MACs.

Attached are a couple other patches I did in order to fix all the bugs
I encountered in making this work.  You may need them in order for the
above to apply cleanly.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

[-- Attachment #2: use-free --]
[-- Type: text/plain, Size: 4019 bytes --]

Index: linux-2.6.21-mm/arch/um/drivers/pcap_kern.c
===================================================================
--- linux-2.6.21-mm.orig/arch/um/drivers/pcap_kern.c	2007-04-27 13:16:49.000000000 -0400
+++ linux-2.6.21-mm/arch/um/drivers/pcap_kern.c	2007-04-27 16:20:57.000000000 -0400
@@ -31,19 +31,21 @@ void pcap_init(struct net_device *dev, v
 	ppri->filter = init->filter;
 }
 
-static int pcap_read(int fd, struct sk_buff **skb, 
+static int pcap_read(int fd, struct sk_buff **skb,
 		       struct uml_net_private *lp)
 {
 	*skb = ether_adjust_skb(*skb, ETH_HEADER_OTHER);
-	if(*skb == NULL) return(-ENOMEM);
-	return(pcap_user_read(fd, skb_mac_header(*skb),
+	if(*skb == NULL)
+		return -ENOMEM;
+
+	return pcap_user_read(fd, skb_mac_header(*skb),
 			      (*skb)->dev->mtu + ETH_HEADER_OTHER,
-			      (struct pcap_data *) &lp->user));
+			      (struct pcap_data *) &lp->user);
 }
 
 static int pcap_write(int fd, struct sk_buff **skb, struct uml_net_private *lp)
 {
-	return(-EPERM);
+	return -EPERM;
 }
 
 static const struct net_kern_info pcap_kern_info = {
@@ -65,12 +67,12 @@ int pcap_setup(char *str, char **mac_out
 		  .optimize 	= 0,
 		  .filter 	= NULL });
 
-	remain = split_if_spec(str, &host_if, &init->filter, 
+	remain = split_if_spec(str, &host_if, &init->filter,
 			       &options[0], &options[1], NULL);
 	if(remain != NULL){
 		printk(KERN_ERR "pcap_setup - Extra garbage on "
 		       "specification : '%s'\n", remain);
-		return(0);
+		return 0;
 	}
 
 	if(host_if != NULL)
@@ -87,10 +89,13 @@ int pcap_setup(char *str, char **mac_out
 			init->optimize = 1;
 		else if(!strcmp(options[i], "nooptimize"))
 			init->optimize = 0;
-		else printk("pcap_setup : bad option - '%s'\n", options[i]);
+		else {
+			printk("pcap_setup : bad option - '%s'\n", options[i]);
+			return 0;
+		}
 	}
 
-	return(1);
+	return 1;
 }
 
 static struct transport pcap_transport = {
Index: linux-2.6.21-mm/arch/um/include/user.h
===================================================================
--- linux-2.6.21-mm.orig/arch/um/include/user.h	2007-04-26 17:33:01.000000000 -0400
+++ linux-2.6.21-mm/arch/um/include/user.h	2007-04-27 14:21:35.000000000 -0400
@@ -27,5 +27,6 @@ extern int in_aton(char *str);
 extern int open_gdb_chan(void);
 extern size_t strlcpy(char *, const char *, size_t);
 extern size_t strlcat(char *, const char *, size_t);
+extern int reserved_address(void *addr);
 
 #endif
Index: linux-2.6.21-mm/arch/um/kernel/um_arch.c
===================================================================
--- linux-2.6.21-mm.orig/arch/um/kernel/um_arch.c	2007-04-26 17:41:21.000000000 -0400
+++ linux-2.6.21-mm/arch/um/kernel/um_arch.c	2007-04-27 14:30:36.000000000 -0400
@@ -500,6 +500,13 @@ void __init check_bugs(void)
 	os_check_bugs();
 }
 
+int reserved_address(void *addr)
+{
+	struct page *page = virt_to_page(addr);
+
+	return(PageReserved(page));
+}
+
 void apply_alternatives(struct alt_instr *start, struct alt_instr *end)
 {
 }
Index: linux-2.6.21-mm/arch/um/os-Linux/main.c
===================================================================
--- linux-2.6.21-mm.orig/arch/um/os-Linux/main.c	2007-04-26 17:41:10.000000000 -0400
+++ linux-2.6.21-mm/arch/um/os-Linux/main.c	2007-04-27 14:30:31.000000000 -0400
@@ -266,6 +266,8 @@ void __wrap_free(void *ptr)
 	/* We need to know how the allocation happened, so it can be correctly
 	 * freed.  This is done by seeing what region of memory the pointer is
 	 * in -
+	 *	in a reserved page - free, assume the pointer was
+	 *	    acquired with malloc, since it couldn't have been kmalloced.
 	 * 	physical memory - kmalloc/kfree
 	 *	kernel virtual memory - vmalloc/vfree
 	 * 	anywhere else - malloc/free
@@ -281,7 +283,9 @@ void __wrap_free(void *ptr)
 	 * there is a possibility for memory leaks.
 	 */
 
-	if((addr >= uml_physmem) && (addr < high_physmem)){
+	if(kmalloc_ok && reserved_address(ptr))
+		__real_free(ptr);
+	else if((addr >= uml_physmem) && (addr < high_physmem)){
 		if(CAN_KMALLOC())
 			kfree(ptr);
 	}

[-- Attachment #3: pcap-tidying --]
[-- Type: text/plain, Size: 4525 bytes --]

Index: linux-2.6.21-mm/arch/um/drivers/net_kern.c
===================================================================
--- linux-2.6.21-mm.orig/arch/um/drivers/net_kern.c	2007-04-27 16:14:48.000000000 -0400
+++ linux-2.6.21-mm/arch/um/drivers/net_kern.c	2007-04-27 16:20:53.000000000 -0400
@@ -316,12 +316,14 @@ static void setup_etheraddr(char *str, u
 	}
 	if (!is_local_ether_addr(addr)) {
 		printk(KERN_WARNING
-		       "Warning: attempt to assign a globally valid ethernet address to a "
-		       "device\n");
-		printk(KERN_WARNING "You should better enable the 2nd rightmost bit "
-		      "in the first byte of the MAC, i.e. "
-		      "%02x:%02x:%02x:%02x:%02x:%02x\n",
-		      addr[0] | 0x02, addr[1], addr[2], addr[3], addr[4], addr[5]);
+		       "Warning: attempt to assign a globally valid ethernet "
+		       "address to a device\n");
+		printk(KERN_WARNING "You should better enable the 2nd "
+		       "rightmost bit in the first byte of the MAC,\n");
+		printk(KERN_WARNING "i.e. %02x:%02x:%02x:%02x:%02x:%02x\n",
+		       addr[0] | 0x02, addr[1], addr[2], addr[3], addr[4],
+		       addr[5]);
+		goto random;
 	}
 	return;
 
@@ -478,6 +480,7 @@ out_undo_user_init:
 		(*transport->user->remove)(&lp->user);
 out_unregister:
 	platform_device_unregister(&device->pdev);
+	return; /* platform_device_unregister frees dev and device */
 out_free_netdev:
 	free_netdev(dev);
 out_free_device:
Index: linux-2.6.21-mm/arch/um/drivers/pcap_kern.c
===================================================================
--- linux-2.6.21-mm.orig/arch/um/drivers/pcap_kern.c	2007-04-27 16:14:48.000000000 -0400
+++ linux-2.6.21-mm/arch/um/drivers/pcap_kern.c	2007-04-27 16:20:53.000000000 -0400
@@ -29,6 +29,8 @@ void pcap_init(struct net_device *dev, v
 	ppri->promisc = init->promisc;
 	ppri->optimize = init->optimize;
 	ppri->filter = init->filter;
+
+	printk("pcap backend, host interface %s\n", ppri->host_if);
 }
 
 static int pcap_read(int fd, struct sk_buff **skb,
Index: linux-2.6.21-mm/arch/um/drivers/pcap_user.c
===================================================================
--- linux-2.6.21-mm.orig/arch/um/drivers/pcap_user.c	2007-04-27 16:14:48.000000000 -0400
+++ linux-2.6.21-mm/arch/um/drivers/pcap_user.c	2007-04-27 16:20:53.000000000 -0400
@@ -13,6 +13,7 @@
 #include "pcap_user.h"
 #include "user.h"
 #include "um_malloc.h"
+#include "kern_constants.h"
 
 #define MAX_PACKET (ETH_MAX_PACKET + ETH_HEADER_OTHER)
 
@@ -26,8 +27,8 @@ static int pcap_user_init(void *data, vo
 
 	p = pcap_open_live(pri->host_if, MAX_PACKET, pri->promisc, 0, errors);
 	if(p == NULL){
-		printk("pcap_user_init : pcap_open_live failed - '%s'\n", 
-		       errors);
+		printk(UM_KERN_ERR "pcap_user_init : pcap_open_live failed - "
+		       "'%s'\n", errors);
 		return -EINVAL;
 	}
 
@@ -48,13 +49,13 @@ static int pcap_open(void *data)
 	if(pri->filter != NULL){
 		err = dev_netmask(pri->dev, &netmask);
 		if(err < 0){
-			printk("pcap_open : dev_netmask failed\n");
+			printk(UM_KERN_ERR "pcap_open : dev_netmask failed\n");
 			return -EIO;
 		}
 
 		pri->compiled = um_kmalloc(sizeof(struct bpf_program));
 		if(pri->compiled == NULL){
-			printk("pcap_open : kmalloc failed\n");
+			printk(UM_KERN_ERR "pcap_open : kmalloc failed\n");
 			return -ENOMEM;
 		}
 
@@ -62,15 +63,15 @@ static int pcap_open(void *data)
 				   (struct bpf_program *) pri->compiled, 
 				   pri->filter, pri->optimize, netmask);
 		if(err < 0){
-			printk("pcap_open : pcap_compile failed - '%s'\n", 
-			       pcap_geterr(pri->pcap));
+			printk(UM_KERN_ERR "pcap_open : pcap_compile failed - "
+			       "'%s'\n", pcap_geterr(pri->pcap));
 			return -EIO;
 		}
 
 		err = pcap_setfilter(pri->pcap, pri->compiled);
 		if(err < 0){
-			printk("pcap_open : pcap_setfilter failed - '%s'\n", 
-			       pcap_geterr(pri->pcap));
+			printk(UM_KERN_ERR "pcap_open : pcap_setfilter "
+			       "failed - '%s'\n", pcap_geterr(pri->pcap));
 			return -EIO;
 		}
 	}
@@ -85,7 +86,8 @@ static void pcap_remove(void *data)
 	if(pri->compiled != NULL)
 		pcap_freecode(pri->compiled);
 
-	pcap_close(pri->pcap);
+	if(pri->pcap != NULL)
+		pcap_close(pri->pcap);
 }
 
 struct pcap_handler_data {
@@ -114,7 +116,8 @@ int pcap_user_read(int fd, void *buffer,
 
 	n = pcap_dispatch(pri->pcap, 1, handler, (u_char *) &hdata);
 	if(n < 0){
-		printk("pcap_dispatch failed - %s\n", pcap_geterr(pri->pcap));
+		printk(UM_KERN_ERR "pcap_dispatch failed - %s\n",
+		       pcap_geterr(pri->pcap));
 		return -EIO;
 	}
 	else if(n == 0) 

[-- Attachment #4: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #5: Type: text/plain, Size: 194 bytes --]

_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Jeff Dike wrote:
> On Fri, Apr 27, 2007 at 05:46:29PM +0100, Antoine Martin wrote:
>> Attached.
>>
>> Not sure which approach is best:
>>
>> 1) this one requires a command line like:
>> "vmlinux eth0=pcap,eth0,,mac=00:01:02:03:04:05"
> 
>> 2) or this one: which just assumes that the mac address is any option
>> that isn't [no]promisc or [no]optimize and is 17 characters long (I
>> don't see much point in doing extra checks since this is check in
>> setup_ether_xx later on)
> 
> Or, how about this:
> 
> Index: linux-2.6.21-mm/arch/um/drivers/pcap_kern.c
> ===================================================================
> --- linux-2.6.21-mm.orig/arch/um/drivers/pcap_kern.c	2007-04-27 16:15:41.000000000 -0400
> +++ linux-2.6.21-mm/arch/um/drivers/pcap_kern.c	2007-04-27 16:15:57.000000000 -0400
> @@ -70,7 +70,7 @@ int pcap_setup(char *str, char **mac_out
>  		  .filter 	= NULL });
>  
>  	remain = split_if_spec(str, &host_if, &init->filter,
> -			       &options[0], &options[1], NULL);
> +			       &options[0], &options[1], mac_out, NULL);
>  	if(remain != NULL){
>  		printk(KERN_ERR "pcap_setup - Extra garbage on "
>  		       "specification : '%s'\n", remain);
> 
> You end up using it like so:
> 
> eth0=pcap,eth0,tcp,,,2:1:2:3:4:5
Full marks for number of lines of code changed! (1!)

> You need to toss in some commas so the MAC doesn't get mistaken for a
> pcap option.  If you have violent objections to this, we can make this
> work with your 'mac=blah' thing.  I like it because it's more
> consistent with how everything else specifies MACs.
Yep, my preferred patch was the one without the mac=

On the plus side, it only adds the parameter at the end, but it does
keep some code in there to deal with variable number/order for
[no]promisc and [no]optimized options, so I would prefer something like
this one - not just because I wrote it ;)
http://uml.nagafix.co.uk/pcap-macaddr.patch
Where the only required option is the pcap filter (which is harder to
move around since it doesn't have a fixed set of options - or does it?).

> Attached are a couple other patches I did in order to fix all the bugs
> I encountered in making this work.  You may need them in order for the
> above to apply cleanly.
Applied cleanly even without the cleanups.

Antoine

> 
> 				Jeff
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: linux-2.6.21-mm/arch/um/drivers/pcap_kern.c
> ===================================================================
> --- linux-2.6.21-mm.orig/arch/um/drivers/pcap_kern.c	2007-04-27 13:16:49.000000000 -0400
> +++ linux-2.6.21-mm/arch/um/drivers/pcap_kern.c	2007-04-27 16:20:57.000000000 -0400
> @@ -31,19 +31,21 @@ void pcap_init(struct net_device *dev, v
>  	ppri->filter = init->filter;
>  }
>  
> -static int pcap_read(int fd, struct sk_buff **skb, 
> +static int pcap_read(int fd, struct sk_buff **skb,
>  		       struct uml_net_private *lp)
>  {
>  	*skb = ether_adjust_skb(*skb, ETH_HEADER_OTHER);
> -	if(*skb == NULL) return(-ENOMEM);
> -	return(pcap_user_read(fd, skb_mac_header(*skb),
> +	if(*skb == NULL)
> +		return -ENOMEM;
> +
> +	return pcap_user_read(fd, skb_mac_header(*skb),
>  			      (*skb)->dev->mtu + ETH_HEADER_OTHER,
> -			      (struct pcap_data *) &lp->user));
> +			      (struct pcap_data *) &lp->user);
>  }
>  
>  static int pcap_write(int fd, struct sk_buff **skb, struct uml_net_private *lp)
>  {
> -	return(-EPERM);
> +	return -EPERM;
>  }
>  
>  static const struct net_kern_info pcap_kern_info = {
> @@ -65,12 +67,12 @@ int pcap_setup(char *str, char **mac_out
>  		  .optimize 	= 0,
>  		  .filter 	= NULL });
>  
> -	remain = split_if_spec(str, &host_if, &init->filter, 
> +	remain = split_if_spec(str, &host_if, &init->filter,
>  			       &options[0], &options[1], NULL);
>  	if(remain != NULL){
>  		printk(KERN_ERR "pcap_setup - Extra garbage on "
>  		       "specification : '%s'\n", remain);
> -		return(0);
> +		return 0;
>  	}
>  
>  	if(host_if != NULL)
> @@ -87,10 +89,13 @@ int pcap_setup(char *str, char **mac_out
>  			init->optimize = 1;
>  		else if(!strcmp(options[i], "nooptimize"))
>  			init->optimize = 0;
> -		else printk("pcap_setup : bad option - '%s'\n", options[i]);
> +		else {
> +			printk("pcap_setup : bad option - '%s'\n", options[i]);
> +			return 0;
> +		}
>  	}
>  
> -	return(1);
> +	return 1;
>  }
>  
>  static struct transport pcap_transport = {
> Index: linux-2.6.21-mm/arch/um/include/user.h
> ===================================================================
> --- linux-2.6.21-mm.orig/arch/um/include/user.h	2007-04-26 17:33:01.000000000 -0400
> +++ linux-2.6.21-mm/arch/um/include/user.h	2007-04-27 14:21:35.000000000 -0400
> @@ -27,5 +27,6 @@ extern int in_aton(char *str);
>  extern int open_gdb_chan(void);
>  extern size_t strlcpy(char *, const char *, size_t);
>  extern size_t strlcat(char *, const char *, size_t);
> +extern int reserved_address(void *addr);
>  
>  #endif
> Index: linux-2.6.21-mm/arch/um/kernel/um_arch.c
> ===================================================================
> --- linux-2.6.21-mm.orig/arch/um/kernel/um_arch.c	2007-04-26 17:41:21.000000000 -0400
> +++ linux-2.6.21-mm/arch/um/kernel/um_arch.c	2007-04-27 14:30:36.000000000 -0400
> @@ -500,6 +500,13 @@ void __init check_bugs(void)
>  	os_check_bugs();
>  }
>  
> +int reserved_address(void *addr)
> +{
> +	struct page *page = virt_to_page(addr);
> +
> +	return(PageReserved(page));
> +}
> +
>  void apply_alternatives(struct alt_instr *start, struct alt_instr *end)
>  {
>  }
> Index: linux-2.6.21-mm/arch/um/os-Linux/main.c
> ===================================================================
> --- linux-2.6.21-mm.orig/arch/um/os-Linux/main.c	2007-04-26 17:41:10.000000000 -0400
> +++ linux-2.6.21-mm/arch/um/os-Linux/main.c	2007-04-27 14:30:31.000000000 -0400
> @@ -266,6 +266,8 @@ void __wrap_free(void *ptr)
>  	/* We need to know how the allocation happened, so it can be correctly
>  	 * freed.  This is done by seeing what region of memory the pointer is
>  	 * in -
> +	 *	in a reserved page - free, assume the pointer was
> +	 *	    acquired with malloc, since it couldn't have been kmalloced.
>  	 * 	physical memory - kmalloc/kfree
>  	 *	kernel virtual memory - vmalloc/vfree
>  	 * 	anywhere else - malloc/free
> @@ -281,7 +283,9 @@ void __wrap_free(void *ptr)
>  	 * there is a possibility for memory leaks.
>  	 */
>  
> -	if((addr >= uml_physmem) && (addr < high_physmem)){
> +	if(kmalloc_ok && reserved_address(ptr))
> +		__real_free(ptr);
> +	else if((addr >= uml_physmem) && (addr < high_physmem)){
>  		if(CAN_KMALLOC())
>  			kfree(ptr);
>  	}
> 
> 
> ------------------------------------------------------------------------
> 
> Index: linux-2.6.21-mm/arch/um/drivers/net_kern.c
> ===================================================================
> --- linux-2.6.21-mm.orig/arch/um/drivers/net_kern.c	2007-04-27 16:14:48.000000000 -0400
> +++ linux-2.6.21-mm/arch/um/drivers/net_kern.c	2007-04-27 16:20:53.000000000 -0400
> @@ -316,12 +316,14 @@ static void setup_etheraddr(char *str, u
>  	}
>  	if (!is_local_ether_addr(addr)) {
>  		printk(KERN_WARNING
> -		       "Warning: attempt to assign a globally valid ethernet address to a "
> -		       "device\n");
> -		printk(KERN_WARNING "You should better enable the 2nd rightmost bit "
> -		      "in the first byte of the MAC, i.e. "
> -		      "%02x:%02x:%02x:%02x:%02x:%02x\n",
> -		      addr[0] | 0x02, addr[1], addr[2], addr[3], addr[4], addr[5]);
> +		       "Warning: attempt to assign a globally valid ethernet "
> +		       "address to a device\n");
> +		printk(KERN_WARNING "You should better enable the 2nd "
> +		       "rightmost bit in the first byte of the MAC,\n");
> +		printk(KERN_WARNING "i.e. %02x:%02x:%02x:%02x:%02x:%02x\n",
> +		       addr[0] | 0x02, addr[1], addr[2], addr[3], addr[4],
> +		       addr[5]);
> +		goto random;
>  	}
>  	return;
>  
> @@ -478,6 +480,7 @@ out_undo_user_init:
>  		(*transport->user->remove)(&lp->user);
>  out_unregister:
>  	platform_device_unregister(&device->pdev);
> +	return; /* platform_device_unregister frees dev and device */
>  out_free_netdev:
>  	free_netdev(dev);
>  out_free_device:
> Index: linux-2.6.21-mm/arch/um/drivers/pcap_kern.c
> ===================================================================
> --- linux-2.6.21-mm.orig/arch/um/drivers/pcap_kern.c	2007-04-27 16:14:48.000000000 -0400
> +++ linux-2.6.21-mm/arch/um/drivers/pcap_kern.c	2007-04-27 16:20:53.000000000 -0400
> @@ -29,6 +29,8 @@ void pcap_init(struct net_device *dev, v
>  	ppri->promisc = init->promisc;
>  	ppri->optimize = init->optimize;
>  	ppri->filter = init->filter;
> +
> +	printk("pcap backend, host interface %s\n", ppri->host_if);
>  }
>  
>  static int pcap_read(int fd, struct sk_buff **skb,
> Index: linux-2.6.21-mm/arch/um/drivers/pcap_user.c
> ===================================================================
> --- linux-2.6.21-mm.orig/arch/um/drivers/pcap_user.c	2007-04-27 16:14:48.000000000 -0400
> +++ linux-2.6.21-mm/arch/um/drivers/pcap_user.c	2007-04-27 16:20:53.000000000 -0400
> @@ -13,6 +13,7 @@
>  #include "pcap_user.h"
>  #include "user.h"
>  #include "um_malloc.h"
> +#include "kern_constants.h"
>  
>  #define MAX_PACKET (ETH_MAX_PACKET + ETH_HEADER_OTHER)
>  
> @@ -26,8 +27,8 @@ static int pcap_user_init(void *data, vo
>  
>  	p = pcap_open_live(pri->host_if, MAX_PACKET, pri->promisc, 0, errors);
>  	if(p == NULL){
> -		printk("pcap_user_init : pcap_open_live failed - '%s'\n", 
> -		       errors);
> +		printk(UM_KERN_ERR "pcap_user_init : pcap_open_live failed - "
> +		       "'%s'\n", errors);
>  		return -EINVAL;
>  	}
>  
> @@ -48,13 +49,13 @@ static int pcap_open(void *data)
>  	if(pri->filter != NULL){
>  		err = dev_netmask(pri->dev, &netmask);
>  		if(err < 0){
> -			printk("pcap_open : dev_netmask failed\n");
> +			printk(UM_KERN_ERR "pcap_open : dev_netmask failed\n");
>  			return -EIO;
>  		}
>  
>  		pri->compiled = um_kmalloc(sizeof(struct bpf_program));
>  		if(pri->compiled == NULL){
> -			printk("pcap_open : kmalloc failed\n");
> +			printk(UM_KERN_ERR "pcap_open : kmalloc failed\n");
>  			return -ENOMEM;
>  		}
>  
> @@ -62,15 +63,15 @@ static int pcap_open(void *data)
>  				   (struct bpf_program *) pri->compiled, 
>  				   pri->filter, pri->optimize, netmask);
>  		if(err < 0){
> -			printk("pcap_open : pcap_compile failed - '%s'\n", 
> -			       pcap_geterr(pri->pcap));
> +			printk(UM_KERN_ERR "pcap_open : pcap_compile failed - "
> +			       "'%s'\n", pcap_geterr(pri->pcap));
>  			return -EIO;
>  		}
>  
>  		err = pcap_setfilter(pri->pcap, pri->compiled);
>  		if(err < 0){
> -			printk("pcap_open : pcap_setfilter failed - '%s'\n", 
> -			       pcap_geterr(pri->pcap));
> +			printk(UM_KERN_ERR "pcap_open : pcap_setfilter "
> +			       "failed - '%s'\n", pcap_geterr(pri->pcap));
>  			return -EIO;
>  		}
>  	}
> @@ -85,7 +86,8 @@ static void pcap_remove(void *data)
>  	if(pri->compiled != NULL)
>  		pcap_freecode(pri->compiled);
>  
> -	pcap_close(pri->pcap);
> +	if(pri->pcap != NULL)
> +		pcap_close(pri->pcap);
>  }
>  
>  struct pcap_handler_data {
> @@ -114,7 +116,8 @@ int pcap_user_read(int fd, void *buffer,
>  
>  	n = pcap_dispatch(pri->pcap, 1, handler, (u_char *) &hdata);
>  	if(n < 0){
> -		printk("pcap_dispatch failed - %s\n", pcap_geterr(pri->pcap));
> +		printk(UM_KERN_ERR "pcap_dispatch failed - %s\n",
> +		       pcap_geterr(pri->pcap));
>  		return -EIO;
>  	}
>  	else if(n == 0) 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGNiUAGK2zHPGK1rsRCi4NAJ9AFZ4zDzKQgMmXiUjf6baYq+M2twCfab02
vPvPp55BRQxSOybYdCIlWXo=
=NS/F
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Jeff Dike]

On Mon, Apr 30, 2007 at 06:18:56PM +0100, Antoine Martin wrote:
> Full marks for number of lines of code changed! (1!)

Yeah, I thought that was a plus.

> On the plus side, it only adds the parameter at the end, but it does
> keep some code in there to deal with variable number/order for
> [no]promisc and [no]optimized options, so I would prefer something like
> this one - not just because I wrote it ;)
> http://uml.nagafix.co.uk/pcap-macaddr.patch
> Where the only required option is the pcap filter (which is harder to
> move around since it doesn't have a fixed set of options - or does it?).

On the negative side -
I dislike assumptions like this:
	+			//a mac address is always 17 characters long
Suppose someone uses 2:1:2:3:4:5 (11 chars), which I did for testing?
This is also conflating a MAC address with a pcap option, which it
isn't.

So what happens when someone adds more options to libpcap and UML
users start wanting to use them?  I would say that we add
'eth0=pcapv1,if,mac,filter,opt1,opt2,...' and futureproof this wrt
pcap options.  People will have to rearrange their command lines in
order to use this, but they already are in order to use the new
options.  Everyone else will stick with the current 'eth0=pcap,...'
format.

So, unless you have strenuous objections, I think I will stick with my
patch and wait for pcap to come up with more options.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Antoine Martin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Jeff Dike wrote:
> On Mon, Apr 30, 2007 at 06:18:56PM +0100, Antoine Martin wrote:
>> Full marks for number of lines of code changed! (1!)
> 
> Yeah, I thought that was a plus.
> 
>> On the plus side, it only adds the parameter at the end, but it does
>> keep some code in there to deal with variable number/order for
>> [no]promisc and [no]optimized options, so I would prefer something like
>> this one - not just because I wrote it ;)
>> http://uml.nagafix.co.uk/pcap-macaddr.patch
>> Where the only required option is the pcap filter (which is harder to
>> move around since it doesn't have a fixed set of options - or does it?).
> 
> On the negative side -
> I dislike assumptions like this:
Me too, but the mac address is later validated in setup_etheraddr the
validation is redundant.
> 	+			//a mac address is always 17 characters long
> Suppose someone uses 2:1:2:3:4:5 (11 chars), which I did for testing?
I thought that was an invalid mac address?(I've never seen mac addresses
specified that way on any software or hardware - yet)
> This is also conflating a MAC address with a pcap option, which it
> isn't.
I quite liked the idea of having the freedom to specify things in any
order and not having to have empty comas but no biggie.

> So what happens when someone adds more options to libpcap and UML
> users start wanting to use them?  I would say that we add
> 'eth0=pcapv1,if,mac,filter,opt1,opt2,...' and futureproof this wrt
> pcap options.  People will have to rearrange their command lines in
> order to use this, but they already are in order to use the new
> options.
Yeah, I preferred that too, it is also more consistent with the way the
other transports do it, but it needed to be backwards compatible. Which
can still be done, but only as long as:
pcap-options!='[no]promisc!'='[no]optimize!'=mac-address='pcap'
Isn't it the case already?

> Everyone else will stick with the current 'eth0=pcap,...'
> format.
> 
> So, unless you have strenuous objections, I think I will stick with my
> patch and wait for pcap to come up with more options.
Acked-by: Antoine Martin <antoine@nagafix.co.uk>
works for me (tm)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGNkPzGK2zHPGK1rsRCnCPAJ941sURMUK+XCY0DN3oC0e/I4qQowCfVoim
O4TJFIvpWyNYVumoPpcinek=
=AQjX
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] regression: pcap broken in 2.6.20.x

[Jeff Dike]

On Mon, Apr 30, 2007 at 08:30:59PM +0100, Antoine Martin wrote:
> > Suppose someone uses 2:1:2:3:4:5 (11 chars), which I did for testing?
> I thought that was an invalid mac address?(I've never seen mac addresses
> specified that way on any software or hardware - yet)

A valid MAC is 6 binary bytes - I haven't seen anywhere that "02" is
the only valid ASCII representation of one of those bytes.

> Yeah, I preferred that too, it is also more consistent with the way the
> other transports do it, but it needed to be backwards compatible. Which
> can still be done, but only as long as:
> pcap-options!='[no]promisc!'='[no]optimize!'=mac-address='pcap'
> Isn't it the case already?

The original pcap specification would remain, and that would be your
backward compatibility.

> Acked-by: Antoine Martin <antoine@nagafix.co.uk>
> works for me (tm)

Cool, I'll send it in.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel