[PATCH] fix tuntap oversight

[PATCH] fix tuntap oversight

[Jeremy Martin]

Hi everyone,

This 2.6.5 patch for the universal tuntap driver allows setting the MAC
address via ifconfig(8).  

I'm not subscribed to netdev so if you could CC me that'd be awesome.

-Jeremy

Patch contents:

-- CUT HERE --

===== drivers/net/tun.c 1.33 vs edited =====
--- 1.33/drivers/net/tun.c	Mon Mar 15 12:45:05 2004
+++ edited/drivers/net/tun.c	Sun Apr 11 22:39:06 2004
@@ -117,6 +117,15 @@
 	return &tun->stats;
 }
 
+static int tun_mac_addr(struct net_device *dev, void *p)
+{
+	struct sockaddr *addr=p;
+	if (netif_running(dev))
+		return -EBUSY;
+	memcpy(dev->dev_addr, addr->sa_data,dev->addr_len);
+	return 0;
+}
+
 /* Initialize net device. */
 static void tun_net_init(struct net_device *dev)
 {
@@ -138,6 +147,7 @@
 	case TUN_TAP_DEV:
 		/* Ethernet TAP Device */
 		dev->set_multicast_list = tun_net_mclist;
+		dev->set_mac_address = tun_mac_addr;
 
 		/* Generate random Ethernet address.  */
 		*(u16 *)dev->dev_addr = htons(0x00FF);

Re: [PATCH] fix tuntap oversight

[David S. Miller]

On Sun, 11 Apr 2004 23:59:47 -0700
Jeremy Martin <martinjd@csc.uvic.ca> wrote:

> +static int tun_mac_addr(struct net_device *dev, void *p)
> +{
> +	struct sockaddr *addr=p;
> +	if (netif_running(dev))
> +		return -EBUSY;
> +	memcpy(dev->dev_addr, addr->sa_data,dev->addr_len);
> +	return 0;
> +}

This netif_running() check is not necessary, and in fact
wrong.

In fact, if ethernet drivers erroneously do this, this causes
them to fail to support the ALB bonding driver modes which
require on-the-fly MAC address changes while the interface is
up.

Re: [PATCH] fix tuntap oversight

[Jeremy Martin]

On Mon, Apr 12, 2004 at 12:15:51AM -0700, David S. Miller wrote:
> 
> This netif_running() check is not necessary, and in fact
> wrong.
> 
> In fact, if ethernet drivers erroneously do this, this causes
> them to fail to support the ALB bonding driver modes which
> require on-the-fly MAC address changes while the interface is
> up.
> 

I just took a look in drivers/net/
and 
	acenic.c
	atarilance.c
	b44.c
	cs89x0.c
	net_init.c
	typhoon.c

all use that netif_running() check when setting the MAC.  I actually just pulled
the function from net_init.c for the tun change.  Are these broken?
(I'm asking in total ignorance so be gentle :).

-Jeremy

Re: [PATCH] fix tuntap oversight

[Jeff Garzik]

Jeremy Martin wrote:
> On Mon, Apr 12, 2004 at 12:15:51AM -0700, David S. Miller wrote:
> 
>>This netif_running() check is not necessary, and in fact
>>wrong.
>>
>>In fact, if ethernet drivers erroneously do this, this causes
>>them to fail to support the ALB bonding driver modes which
>>require on-the-fly MAC address changes while the interface is
>>up.
>>
> 
> 
> I just took a look in drivers/net/
> and 
> 	acenic.c
> 	atarilance.c
> 	b44.c
> 	cs89x0.c
> 	net_init.c
> 	typhoon.c
> 
> all use that netif_running() check when setting the MAC.  I actually just pulled
> the function from net_init.c for the tun change.  Are these broken?
> (I'm asking in total ignorance so be gentle :).

It's different for a driver that drives real hardware.

struct net_device::set_mac_address() is called inside rtnl_lock().  The 
safe thing to do is
1) read MAC address from eeprom on probe
2) write MAC address to hardware upon each dev->open()
3) use default eth_mac_addr() from net_init.c

And the netif_running() check in eth_mac_addr() is correct, because it 
does not update the hardware MAC address (which in this API would be 
impossible).

Normally the netif_running() check is for hardware that cannot update 
its MAC address safely during operation.

	Jeff

Re: [PATCH] fix tuntap oversight

[David S. Miller]

On Mon, 12 Apr 2004 09:29:16 -0700
Jeremy Martin <martinjd@csc.uvic.ca> wrote:

> all use that netif_running() check when setting the MAC.  I actually just pulled
> the function from net_init.c for the tun change.  Are these broken?

Yes.