PATCH: tun/tap driver hw address handling

* PATCH: tun/tap driver hw address handling
       [not found] ` <45E73715.9030909@qualcomm.com>
@ 2007-03-24  8:56   ` Brian Braunstein
  2007-03-24 17:04     ` Ahmed S. Darwish
  0 siblings, 1 reply; 2+ messages in thread
From: Brian Braunstein @ 2007-03-24  8:56 UTC (permalink / raw)
  To: Max Krasnyansky; +Cc: torvalds, linux-kernel, rajesh_mish


Hi Max,

  Here's the patch we discussed at the beginning of the month.

Linus,

  According to Documentation/SubmittingPatches "bug fixes" or "obvious" 
changes
  should CCed to you, so this is why I have done this.

Note: This entire email can be found at
http://bristyle.com/share/patch-tuntap-hw_addr_handling.txt

-----------------------------------------------------------------------------
Patch Description:

  Kernel Version: 2.6.20.4

  Summary:
    Fix tun/tap driver's handling of hw addresses.  Specifically, ensure 
that
    when the tun.dev_addr field is set, the net_device.dev_addr field gets
    set to the same value.

  Background:
    The device hw address is stored in 2 places, in the tun.dev_addr field,
    and of course the net_device struct's dev_addr field.  It really 
seems to
    me that the tun.dev_addr field is redundant, and that anywhere it is 
used
    it would be better to use the net_device.dev_addr field.  However, I do
    not want to start ripping things out of structs that other people might
    be using, so I've left it.

  Fixed Problem:
    The problem was that when one did an IOCTL on the tun/tap device, the
    device address would only get updated in the tun.dev_addr, and not the
    net_device.dev_addr field.  In addition, the initial setting of the
    tun.dev_addr and net_device.addr fields were different.  This meant that
    if you asked the tun/tap device for it's hw address, it would report a
    different value than ifconfig.

  Still Remaining Problem:
    There is a problem with not fixing the redundant tun.dev_addr field
    around.  The problem is that when the net_device.dev_addr gets updated,
    we get no notification of this update.  So if someone changes the 
address
    using "ifconfig hw ether xxxxx", then the net_device.dev_addr and
    tun.dev_addr fields different in value.  Not good.  If you think I 
should
    strip out the tun.dev_addr field and replace it's usage with
    net_device.dev_addr, then I will do that later.  However, I would 
need to
    do a survey to see if anyone else's code outside of tun.c depends on 
this
    field.  If so...I guess I'll just leave it.

--- 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-24 01:36:59.000000000 -0700
@@ -18,6 +18,11 @@
 /*
  *  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.  However,
+ *    changes made to the net_device.dev_addr are still not tracked.
+ *
  *  Mike Kershaw <dragorn@kismetwireless.net> 2005/08/14
  *    Add TUNSETLINK ioctl to set the link encapsulation
  *
@@ -196,7 +201,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 +644,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))
@@ -652,7 +661,8 @@ static int tun_chr_ioctl(struct inode *i
                                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;
+               /* Now set the actual net device's address */
+               return dev_set_mac_address(tun->dev, &ifr.ifr_hwaddr);

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


-----------------------------------------------------------------------------


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