LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] net: tun/tap: fixed hw address handling
@ 2007-03-25  8:29 Brian Braunstein
  2007-03-26 20:55 ` Ahmed S. Darwish
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Brian Braunstein @ 2007-03-25  8:29 UTC (permalink / raw)
  To: Max Krasnyansky; +Cc: vtun, akpm, jgarzik, netdev, linux-kernel


From: Brian Braunstein <linuxkernel@bristyle.com>

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 <linuxkernel@bristyle.com>

---

Kernel Version: 2.6.20.4

--- linux-2.6.20.4-ORIG/drivers/net/tun.c	2007-03-23 12:52:51.000000000 -0700
+++ linux-2.6.20.4/drivers/net/tun.c	2007-03-25 00:44:20.000000000 -0700
@@ -18,6 +18,10 @@
 /*
  *  Changes:
  *
+ *  Brian Braunstein <linuxkernel@bristyle.com> 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 <dragorn@kismetwireless.net> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: tun/tap: fixed hw address handling
  2007-03-25  8:29 [PATCH] net: tun/tap: fixed hw address handling Brian Braunstein
@ 2007-03-26 20:55 ` Ahmed S. Darwish
  2007-03-26 21:05   ` Ahmed S. Darwish
  2007-03-26 21:22 ` Chuck Ebbert
  2007-03-29 18:07 ` Chuck Ebbert
  2 siblings, 1 reply; 5+ messages in thread
From: Ahmed S. Darwish @ 2007-03-26 20:55 UTC (permalink / raw)
  To: Brian Braunstein
  Cc: Max Krasnyansky, vtun, akpm, jgarzik, netdev, linux-kernel

On Sun, Mar 25, 2007 at 01:29:29AM -0700, Brian Braunstein wrote:
> 
> From: Brian Braunstein <linuxkernel@bristyle.com>
> 

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 <linuxkernel@bristyle.com>
> 
> ---
> 
> 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.000000000 -0700
> +++ linux-2.6.20.4/drivers/net/tun.c	2007-03-25 00:44:20.000000000 -0700
> @@ -18,6 +18,10 @@
> /*
>  *  Changes:
>  *
> + *  Brian Braunstein <linuxkernel@bristyle.com> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: tun/tap: fixed hw address handling
  2007-03-26 20:55 ` Ahmed S. Darwish
@ 2007-03-26 21:05   ` Ahmed S. Darwish
  0 siblings, 0 replies; 5+ messages in thread
From: Ahmed S. Darwish @ 2007-03-26 21:05 UTC (permalink / raw)
  To: Brian Braunstein
  Cc: Max Krasnyansky, vtun, akpm, jgarzik, netdev, linux-kernel

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.000000000 -0700
> > +++ linux-2.6.20.4/drivers/net/tun.c	2007-03-25 00:44:20.000000000 -0700
> > @@ -18,6 +18,10 @@
> > /*
> >  *  Changes:
> >  *
> > + *  Brian Braunstein <linuxkernel@bristyle.com> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: tun/tap: fixed hw address handling
  2007-03-25  8:29 [PATCH] net: tun/tap: fixed hw address handling Brian Braunstein
  2007-03-26 20:55 ` Ahmed S. Darwish
@ 2007-03-26 21:22 ` Chuck Ebbert
  2007-03-29 18:07 ` Chuck Ebbert
  2 siblings, 0 replies; 5+ messages in thread
From: Chuck Ebbert @ 2007-03-26 21:22 UTC (permalink / raw)
  To: Brian Braunstein
  Cc: Max Krasnyansky, vtun, akpm, jgarzik, netdev, linux-kernel

Brian Braunstein wrote:
> 
> From: Brian Braunstein <linuxkernel@bristyle.com>
> 
> 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 <linuxkernel@bristyle.com>
> 

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

> 
> --- linux-2.6.20.4-ORIG/drivers/net/tun.c    2007-03-23
> 12:52:51.000000000 -0700
> +++ linux-2.6.20.4/drivers/net/tun.c    2007-03-25 00:44:20.000000000 -0700
> @@ -18,6 +18,10 @@
> /*
>  *  Changes:
>  *
> + *  Brian Braunstein <linuxkernel@bristyle.com> 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 <dragorn@kismetwireless.net> 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
> 
 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: tun/tap: fixed hw address handling
  2007-03-25  8:29 [PATCH] net: tun/tap: fixed hw address handling Brian Braunstein
  2007-03-26 20:55 ` Ahmed S. Darwish
  2007-03-26 21:22 ` Chuck Ebbert
@ 2007-03-29 18:07 ` Chuck Ebbert
  2 siblings, 0 replies; 5+ messages in thread
From: Chuck Ebbert @ 2007-03-29 18:07 UTC (permalink / raw)
  To: Brian Braunstein
  Cc: Max Krasnyansky, vtun, akpm, jgarzik, netdev, linux-kernel

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

Brian Braunstein wrote:
> 
> From: Brian Braunstein <linuxkernel@bristyle.com>
> 
> 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 <linuxkernel@bristyle.com>

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 <linuxkernel@bristyle.com>

--- 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



[-- Attachment #2: linux-2.6-20.5t-tuntap_fix_set_hw_address.patch --]
[-- Type: text/plain, Size: 2895 bytes --]

From: Brian Braunstein <linuxkernel@bristyle.com>

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 <linuxkernel@bristyle.com>

--- 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-03-29 18:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-25  8:29 [PATCH] net: tun/tap: fixed hw address handling Brian Braunstein
2007-03-26 20:55 ` Ahmed S. Darwish
2007-03-26 21:05   ` Ahmed S. Darwish
2007-03-26 21:22 ` Chuck Ebbert
2007-03-29 18:07 ` Chuck Ebbert

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox