From: Boaz Harrosh <bharrosh@panasas.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
"David S. Miller" <davem@davemloft.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Julia Lawall <julia@diku.dk>,
uml-devel <user-mode-linux-devel@lists.sourceforge.net>,
linux-kernel <linux-kernel@vger.kernel.org>,
Stephen Hemminger <shemminger@vyatta.com>,
Al Viro <viro@ZenIV.linux.org.uk>
Subject: [PATCH] um: Proper Fix for f25c80a4: remove duplicate structure field initialization
Date: Wed, 29 Sep 2010 10:34:27 +0200 [thread overview]
Message-ID: <4CA2FA13.8050405@panasas.com> (raw)
In-Reply-To: <AANLkTi=E7+CpJZE3XC7O1LvJu6LWfmvSxonskH4CYg25@mail.gmail.com>
uml_net_set_mac() was broken and luckily it was never used, before.
What it was trying to do is spin_lock before memcopy the mac address.
Linus attempted to fix it in assumption that someone decided the
lock was needed. But since it was never ever used at all, and was
just dead code, I think we can assume that it is not needed, after
all.
On the other hand patch [f25c80a4] was trying to use eth_mac_addr()
in eth_configure(), *which was the real fallout*. Because of state
checks done inside eth_mac_addr() the address was never set. I have
not reintroduced the memcpy wrapper, but I've put a comment for future
cats.
The code now is back to exactly as it was before [f25c80a4]. With
the cleanup applied. If the spin_lock is indeed needed then a contender
should supply a test case that fails, then fix it with the proper
locking, as a separate unrelated patch.
CC: Julia Lawall <julia@diku.dk>
CC: David S. Miller <davem@davemloft.net>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Al Viro <viro@ZenIV.linux.org.uk>
Tested-by: Boaz Harrosh <bharrosh@panasas.com>
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
arch/um/drivers/net_kern.c | 17 +++--------------
1 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/arch/um/drivers/net_kern.c b/arch/um/drivers/net_kern.c
index 2ab233b..47d0c37 100644
--- a/arch/um/drivers/net_kern.c
+++ b/arch/um/drivers/net_kern.c
@@ -255,18 +255,6 @@ static void uml_net_tx_timeout(struct net_device *dev)
netif_wake_queue(dev);
}
-static int uml_net_set_mac(struct net_device *dev, void *addr)
-{
- struct uml_net_private *lp = netdev_priv(dev);
- struct sockaddr *hwaddr = addr;
-
- spin_lock_irq(&lp->lock);
- eth_mac_addr(dev, hwaddr->sa_data);
- spin_unlock_irq(&lp->lock);
-
- return 0;
-}
-
static int uml_net_change_mtu(struct net_device *dev, int new_mtu)
{
dev->mtu = new_mtu;
@@ -373,7 +361,7 @@ static const struct net_device_ops uml_netdev_ops = {
.ndo_start_xmit = uml_net_start_xmit,
.ndo_set_multicast_list = uml_net_set_multicast_list,
.ndo_tx_timeout = uml_net_tx_timeout,
- .ndo_set_mac_address = uml_net_set_mac,
+ .ndo_set_mac_address = eth_mac_addr,
.ndo_change_mtu = uml_net_change_mtu,
.ndo_validate_addr = eth_validate_addr,
};
@@ -472,7 +460,8 @@ static void eth_configure(int n, void *init, char *mac,
((*transport->user->init)(&lp->user, dev) != 0))
goto out_unregister;
- eth_mac_addr(dev, device->mac);
+ /* don't use eth_mac_addr, it will not work here */
+ memcpy(dev->dev_addr, device->mac, ETH_ALEN);
dev->mtu = transport->user->mtu;
dev->netdev_ops = ¨_netdev_ops;
dev->ethtool_ops = ¨_net_ethtool_ops;
--
1.7.2.3
next prev parent reply other threads:[~2010-09-29 8:34 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-27 13:17 {painfully BISECTED} Please revert f25c80a4b2: arch/um/drivers: remove duplicate structure field initialization Boaz Harrosh
2010-09-27 14:06 ` Phil Turmel
2010-09-28 20:24 ` Linus Torvalds
2010-09-28 20:47 ` Andrew Morton
2010-09-28 20:51 ` David Miller
2010-09-28 20:57 ` Linus Torvalds
2010-09-28 21:00 ` David Miller
2010-09-28 21:08 ` Linus Torvalds
2010-09-29 8:34 ` Boaz Harrosh [this message]
2010-09-29 15:05 ` [PATCH] um: Proper Fix for f25c80a4: " Linus Torvalds
2010-09-30 2:28 ` David Miller
2010-09-29 8:41 ` {painfully BISECTED} Please revert f25c80a4b2: arch/um/drivers: " Boaz Harrosh
2010-09-29 15:01 ` Linus Torvalds
2010-09-30 2:27 ` David Miller
2010-09-28 21:11 ` Al Viro
2010-09-28 21:24 ` Al Viro
2010-09-28 21:42 ` David Miller
2010-09-28 21:51 ` Al Viro
2010-09-29 17:19 ` [uml-devel] " Renzo Davoli
2011-01-26 16:32 ` {painfullyBISECTED} " Emil Langrock
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4CA2FA13.8050405@panasas.com \
--to=bharrosh@panasas.com \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=julia@diku.dk \
--cc=linux-kernel@vger.kernel.org \
--cc=shemminger@vyatta.com \
--cc=torvalds@linux-foundation.org \
--cc=user-mode-linux-devel@lists.sourceforge.net \
--cc=viro@ZenIV.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox