Re: [PATCH] net: tun/tap: fixed hw address handling

2007-03-29 Thread Chuck Ebbert 
Brian Braunstein wrote:
 
 From: Brian Braunstein [EMAIL PROTECTED]
 
 Fixed tun/tap driver's handling of hw addresses.

Okay, the attached patch applies. Can someone comment on whether
it makes sense? (pasted inline for comments)

From: Brian Braunstein [EMAIL PROTECTED]

Fixed tun/tap driver's handling of hw addresses. The hw address is stored in
both the net_device.dev_addr and tun.dev_addr fields.  These fields were not
kept synchronized, and in fact weren't even initialized to the same value.
Now during both init and when performing SIOCSIFHWADDR on the tun device
these values are both updated.  However, if SIOCSIFHWADDR is performed on the
net device directly (for instance, setting the hw address using ifconfig),
the tun device does not get updated.  Perhaps the tun.dev_addr field should
be removed completely at some point, as it is redundant and
net_device.dev_addr can be used anywhere it is used.

Signed-off-by: Brian Braunstein [EMAIL PROTECTED]

--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -196,7 +196,10 @@ static void tun_net_init(struct net_devi
dev-set_multicast_list = tun_net_mclist;

ether_setup(dev);
-   random_ether_addr(dev-dev_addr);
+
+   /* random address already created for us by tun_set_iff, use it 
*/
+   memcpy(dev-dev_addr, tun-dev_addr, min(sizeof(tun-dev_addr), 
sizeof(dev-dev_addr)) );
+
dev-tx_queue_len = TUN_READQ_SIZE;  /* We prefer our own queue 
length */
break;
}
@@ -636,6 +639,7 @@ static int tun_chr_ioctl(struct inode *i
return 0;

case SIOCGIFHWADDR:
+   /* Note: the actual net device's address may be different */
memcpy(ifr.ifr_hwaddr.sa_data, tun-dev_addr,
min(sizeof ifr.ifr_hwaddr.sa_data, sizeof 
tun-dev_addr));
if (copy_to_user( argp, ifr, sizeof ifr))
@@ -643,16 +647,22 @@ static int tun_chr_ioctl(struct inode *i
return 0;

case SIOCSIFHWADDR:
-   /** Set the character device's hardware address. This is used 
when
-* filtering packets being sent from the network device to the 
character
-* device. */
-   memcpy(tun-dev_addr, ifr.ifr_hwaddr.sa_data,
-   min(sizeof ifr.ifr_hwaddr.sa_data, sizeof 
tun-dev_addr));
-   DBG(KERN_DEBUG %s: set hardware address: %x:%x:%x:%x:%x:%x\n,
-   tun-dev-name,
-   tun-dev_addr[0], tun-dev_addr[1], 
tun-dev_addr[2],
-   tun-dev_addr[3], tun-dev_addr[4], 
tun-dev_addr[5]);
-   return 0;
+   /* try to set the actual net device's hw address */
+   int ret = dev_set_mac_address(tun-dev, ifr.ifr_hwaddr);
+
+   if (ret == 0) {
+   /** Set the character device's hardware address. This 
is used when
+* filtering packets being sent from the network device 
to the character
+* device. */
+   memcpy(tun-dev_addr, ifr.ifr_hwaddr.sa_data,
+   min(sizeof ifr.ifr_hwaddr.sa_data, 
sizeof tun-dev_addr));
+   DBG(KERN_DEBUG %s: set hardware address: 
%x:%x:%x:%x:%x:%x\n,
+   tun-dev-name,
+   tun-dev_addr[0], tun-dev_addr[1], 
tun-dev_addr[2],
+   tun-dev_addr[3], tun-dev_addr[4], 
tun-dev_addr[5]);
+   }
+
+   return  ret;

case SIOCADDMULTI:
/** Add the specified group to the character device's multicast 
filter


From: Brian Braunstein [EMAIL PROTECTED]

Fixed tun/tap driver's handling of hw addresses. The hw address is stored in
both the net_device.dev_addr and tun.dev_addr fields.  These fields were not
kept synchronized, and in fact weren't even initialized to the same value.
Now during both init and when performing SIOCSIFHWADDR on the tun device
these values are both updated.  However, if SIOCSIFHWADDR is performed on the
net device directly (for instance, setting the hw address using ifconfig),
the tun device does not get updated.  Perhaps the tun.dev_addr field should
be removed completely at some point, as it is redundant and
net_device.dev_addr can be used anywhere it is used.

Signed-off-by: Brian Braunstein [EMAIL PROTECTED]

--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -196,7 +196,10 @@ static void tun_net_init(struct net_devi
dev-set_multicast_list = tun_net_mclist;
 
ether_setup(dev);
-   random_ether_addr(dev-dev_addr);
+
+   /* random address already created for us by tun_set_iff, use it 
*/
+   memcpy(dev-dev_addr, tun-dev_addr, min(sizeof(tun-dev_addr), 
sizeof(dev-dev_addr)) );
+

Re: [PATCH] net: tun/tap: fixed hw address handling

2007-03-26 Thread Ahmed S. Darwish 
On Sun, Mar 25, 2007 at 01:29:29AM -0700, Brian Braunstein wrote:
 
 From: Brian Braunstein [EMAIL PROTECTED]
 

No need for this line. This line is used when you _forward_ another patch
from others. Signed-off-by is enough

 
 Signed-off-by: Brian Braunstein [EMAIL PROTECTED]
 
 ---
 
 Kernel Version: 2.6.20.4
 

It's always better to generate the patch against the latest -rc kernel.

 --- linux-2.6.20.4-ORIG/drivers/net/tun.c 2007-03-23 
 12:52:51.0 -0700
 +++ linux-2.6.20.4/drivers/net/tun.c  2007-03-25 00:44:20.0 -0700
 @@ -18,6 +18,10 @@
 /*
  *  Changes:
  *
 + *  Brian Braunstein [EMAIL PROTECTED] 2007/03/23
 + *Fixed hw address handling.  Now net_device.dev_addr is kept 
 consistent

Your mailer still wrap the long lines mistakenly as it did in the first
version of the patch.

Regards,

-- 
Ahmed S. Darwish
http://darwish.07.googlepages.com

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: tun/tap: fixed hw address handling

2007-03-26 Thread Ahmed S. Darwish 
On Mon, Mar 26, 2007 at 10:55:11PM +0200, ahmed wrote:
 On Sun, Mar 25, 2007 at 01:29:29AM -0700, Brian Braunstein wrote:
  --- linux-2.6.20.4-ORIG/drivers/net/tun.c   2007-03-23 
  12:52:51.0 -0700
  +++ linux-2.6.20.4/drivers/net/tun.c2007-03-25 00:44:20.0 
  -0700
  @@ -18,6 +18,10 @@
  /*
   *  Changes:
   *
  + *  Brian Braunstein [EMAIL PROTECTED] 2007/03/23
  + *Fixed hw address handling.  Now net_device.dev_addr is kept 
  consistent
 
 Your mailer still wrap the long lines mistakenly as it did in the first
 version of the patch.
 

It seems a bug in my mutt mail reader (not a feature, it displays other 
mails long lines without wrapping), sorry.

-- 
Ahmed S. Darwish
http://darwish.07.googlepages.com

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: tun/tap: fixed hw address handling

2007-03-26 Thread Chuck Ebbert 
Brian Braunstein wrote:
 
 From: Brian Braunstein [EMAIL PROTECTED]
 
 Fixed tun/tap driver's handling of hw addresses. The hw address is
 stored in
 both the net_device.dev_addr and tun.dev_addr fields.  These fields were
 not
 kept synchronized, and in fact weren't even initialized to the same value.
 Now during both init and when performing SIOCSIFHWADDR on the tun device
 these values are both updated.  However, if SIOCSIFHWADDR is performed
 on the
 net device directly (for instance, setting the hw address using ifconfig),
 the tun device does not get updated.  Perhaps the tun.dev_addr field should
 be removed completely at some point, as it is redundant and
 net_device.dev_addr can be used anywhere it is used.
 Signed-off-by: Brian Braunstein [EMAIL PROTECTED]
 

Patch is hopelessly corrupted:
  missing leading space in context lines
  tab converted to four spaces

 
 --- linux-2.6.20.4-ORIG/drivers/net/tun.c2007-03-23
 12:52:51.0 -0700
 +++ linux-2.6.20.4/drivers/net/tun.c2007-03-25 00:44:20.0 -0700
 @@ -18,6 +18,10 @@
 /*
  *  Changes:
  *
 + *  Brian Braunstein [EMAIL PROTECTED] 2007/03/23
 + *Fixed hw address handling.  Now net_device.dev_addr is kept
 consistent
 + *with tun.dev_addr when the address is set by this module.
 + *
  *  Mike Kershaw [EMAIL PROTECTED] 2005/08/14
  *Add TUNSETLINK ioctl to set the link encapsulation
  *
 @@ -196,7 +200,10 @@ static void tun_net_init(struct net_devi
 dev-set_multicast_list = tun_net_mclist;
 
 ether_setup(dev);
 -random_ether_addr(dev-dev_addr);
 +
 +/* random address already created for us by tun_set_iff, use it */
 +memcpy(dev-dev_addr, tun-dev_addr, min(sizeof(tun-dev_addr),
 sizeof(dev-dev_addr)) );
 +
 dev-tx_queue_len = TUN_READQ_SIZE;  /* We prefer our own queue
 length */
 break;
 }
 @@ -636,6 +643,7 @@ static int tun_chr_ioctl(struct inode *i
 return 0;
 
 case SIOCGIFHWADDR:
 +/* Note: the actual net device's address may be different */
 memcpy(ifr.ifr_hwaddr.sa_data, tun-dev_addr,
 min(sizeof ifr.ifr_hwaddr.sa_data, sizeof tun-dev_addr));
 if (copy_to_user( argp, ifr, sizeof ifr))
 @@ -643,16 +651,24 @@ static int tun_chr_ioctl(struct inode *i
 return 0;
 
 case SIOCSIFHWADDR:
 -/** Set the character device's hardware address. This is used when
 - * filtering packets being sent from the network device to the
 character
 - * device. */
 -memcpy(tun-dev_addr, ifr.ifr_hwaddr.sa_data,
 -min(sizeof ifr.ifr_hwaddr.sa_data, sizeof tun-dev_addr));
 -DBG(KERN_DEBUG %s: set hardware address: %x:%x:%x:%x:%x:%x\n,
 -tun-dev-name,
 -tun-dev_addr[0], tun-dev_addr[1], tun-dev_addr[2],
 -tun-dev_addr[3], tun-dev_addr[4], tun-dev_addr[5]);
 -return 0;
 +{
 +/* try to set the actual net device's hw address */
 +int ret = dev_set_mac_address(tun-dev, ifr.ifr_hwaddr);
 +
 +if (ret == 0) {
 +/** Set the character device's hardware address. This is
 used when
 + * filtering packets being sent from the network device to
 the character
 + * device. */
 +memcpy(tun-dev_addr, ifr.ifr_hwaddr.sa_data,
 +min(sizeof ifr.ifr_hwaddr.sa_data, sizeof
 tun-dev_addr));
 +DBG(KERN_DEBUG %s: set hardware address:
 %x:%x:%x:%x:%x:%x\n,
 +tun-dev-name,
 +tun-dev_addr[0], tun-dev_addr[1], tun-dev_addr[2],
 +tun-dev_addr[3], tun-dev_addr[4], tun-dev_addr[5]);
 +}
 +
 +return  ret;
 +}
 
 case SIOCADDMULTI:
 /** Add the specified group to the character device's multicast
 filter
 
 

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] net: tun/tap: fixed hw address handling

2007-03-25 Thread Brian Braunstein 


From: Brian Braunstein [EMAIL PROTECTED]

Fixed tun/tap driver's handling of hw addresses. The hw address is stored in
both the net_device.dev_addr and tun.dev_addr fields.  These fields were not
kept synchronized, and in fact weren't even initialized to the same value.
Now during both init and when performing SIOCSIFHWADDR on the tun device
these values are both updated.  However, if SIOCSIFHWADDR is performed on the
net device directly (for instance, setting the hw address using ifconfig),
the tun device does not get updated.  Perhaps the tun.dev_addr field should
be removed completely at some point, as it is redundant and
net_device.dev_addr can be used anywhere it is used. 


Signed-off-by: Brian Braunstein [EMAIL PROTECTED]

---

Kernel Version: 2.6.20.4

--- linux-2.6.20.4-ORIG/drivers/net/tun.c   2007-03-23 12:52:51.0 
-0700
+++ linux-2.6.20.4/drivers/net/tun.c2007-03-25 00:44:20.0 -0700
@@ -18,6 +18,10 @@
/*
 *  Changes:
 *
+ *  Brian Braunstein [EMAIL PROTECTED] 2007/03/23
+ *Fixed hw address handling.  Now net_device.dev_addr is kept consistent
+ *with tun.dev_addr when the address is set by this module.
+ *
 *  Mike Kershaw [EMAIL PROTECTED] 2005/08/14
 *Add TUNSETLINK ioctl to set the link encapsulation
 *
@@ -196,7 +200,10 @@ static void tun_net_init(struct net_devi
dev-set_multicast_list = tun_net_mclist;

ether_setup(dev);
-   random_ether_addr(dev-dev_addr);
+
+   /* random address already created for us by tun_set_iff, use it 
*/
+   memcpy(dev-dev_addr, tun-dev_addr, min(sizeof(tun-dev_addr), 
sizeof(dev-dev_addr)) );
+
dev-tx_queue_len = TUN_READQ_SIZE;  /* We prefer our own queue 
length */
break;
}
@@ -636,6 +643,7 @@ static int tun_chr_ioctl(struct inode *i
return 0;

case SIOCGIFHWADDR:
+   /* Note: the actual net device's address may be different */
memcpy(ifr.ifr_hwaddr.sa_data, tun-dev_addr,
min(sizeof ifr.ifr_hwaddr.sa_data, sizeof 
tun-dev_addr));
if (copy_to_user( argp, ifr, sizeof ifr))
@@ -643,16 +651,24 @@ static int tun_chr_ioctl(struct inode *i
return 0;

case SIOCSIFHWADDR:
-   /** Set the character device's hardware address. This is used 
when
-* filtering packets being sent from the network device to the 
character
-* device. */
-   memcpy(tun-dev_addr, ifr.ifr_hwaddr.sa_data,
-   min(sizeof ifr.ifr_hwaddr.sa_data, sizeof 
tun-dev_addr));
-   DBG(KERN_DEBUG %s: set hardware address: %x:%x:%x:%x:%x:%x\n,
-   tun-dev-name,
-   tun-dev_addr[0], tun-dev_addr[1], 
tun-dev_addr[2],
-   tun-dev_addr[3], tun-dev_addr[4], 
tun-dev_addr[5]);
-   return 0;
+   {
+   /* try to set the actual net device's hw address */
+   int ret = dev_set_mac_address(tun-dev, ifr.ifr_hwaddr);
+
+   if (ret == 0) {
+   /** Set the character device's hardware address. This 
is used when
+* filtering packets being sent from the network device 
to the character
+* device. */
+   memcpy(tun-dev_addr, ifr.ifr_hwaddr.sa_data,
+   min(sizeof ifr.ifr_hwaddr.sa_data, sizeof 
tun-dev_addr));
+   DBG(KERN_DEBUG %s: set hardware address: 
%x:%x:%x:%x:%x:%x\n,
+   tun-dev-name,
+   tun-dev_addr[0], tun-dev_addr[1], 
tun-dev_addr[2],
+   tun-dev_addr[3], tun-dev_addr[4], 
tun-dev_addr[5]);
+   }
+
+   return  ret;
+   }

case SIOCADDMULTI:
/** Add the specified group to the character device's multicast 
filter

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html