netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Julian Wiedmann <jwi@linux.ibm.com>
To: David Miller <davem@davemloft.net>
Cc: <netdev@vger.kernel.org>, <linux-s390@vger.kernel.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Stefan Raspl <raspl@linux.ibm.com>,
	Ursula Braun <ubraun@linux.ibm.com>,
	Julian Wiedmann <jwi@linux.ibm.com>
Subject: [PATCH net-next 4/8] s390/qeth: register MAC address earlier
Date: Fri, 25 Jan 2019 15:44:19 +0100	[thread overview]
Message-ID: <20190125144423.81443-5-jwi@linux.ibm.com> (raw)
In-Reply-To: <20190125144423.81443-1-jwi@linux.ibm.com>

commit 4789a2188048 ("s390/qeth: fix race when setting MAC address")
resolved a race where our initial programming of dev_addr into the HW
and a call to ndo_set_mac_address() could run concurrently. In this
case, we could end up getting confused about which address was actually
set in the HW.

The quick fix was to introduce additional locking that blocks any
ndo_set_mac_address() while the device is being set online. But the race
primarily originated from the fact that we first register the netdevice,
and only then program its dev_addr. By re-ordering this sequence,
userspace will only be able to change the MAC address _after_ we have
finished with setting the initial dev_addr.

Still, the same MAC address race can also occur during a subsequent call
to qeth_l2_set_online(). So keep around the locking for now, until a
follow-up patch fully resolves this.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_l2_main.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 8b9181c8bc7e..3aa11f32dcd5 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -97,8 +97,7 @@ static int qeth_l2_send_setmac(struct qeth_card *card, __u8 *mac)
 	rc = qeth_l2_send_setdelmac(card, mac, IPA_CMD_SETVMAC);
 	if (rc == 0) {
 		dev_info(&card->gdev->dev,
-			 "MAC address %pM successfully registered on device %s\n",
-			 mac, card->dev->name);
+			 "MAC address %pM successfully registered\n", mac);
 	} else {
 		switch (rc) {
 		case -EEXIST:
@@ -458,6 +457,15 @@ static int qeth_l2_request_initial_mac(struct qeth_card *card)
 	return 0;
 }
 
+static void qeth_l2_register_dev_addr(struct qeth_card *card)
+{
+	if (!is_valid_ether_addr(card->dev->dev_addr))
+		qeth_l2_request_initial_mac(card);
+
+	if (!IS_OSN(card) && !qeth_l2_send_setmac(card, card->dev->dev_addr))
+		card->info.mac_bits |= QETH_LAYER2_MAC_REGISTERED;
+}
+
 static int qeth_l2_validate_addr(struct net_device *dev)
 {
 	struct qeth_card *card = dev->ml_priv;
@@ -845,8 +853,6 @@ static int qeth_l2_setup_netdev(struct qeth_card *card, bool carrier_ok)
 				       PAGE_SIZE * (QDIO_MAX_ELEMENTS_PER_BUFFER - 1));
 	}
 
-	if (!is_valid_ether_addr(card->dev->dev_addr))
-		qeth_l2_request_initial_mac(card);
 	netif_napi_add(card->dev, &card->napi, qeth_poll, QETH_NAPI_WEIGHT);
 	rc = register_netdev(card->dev);
 	if (!rc && carrier_ok)
@@ -901,14 +907,12 @@ static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
 		dev_info(&card->gdev->dev,
 		"The device represents a Bridge Capable Port\n");
 
+	qeth_l2_register_dev_addr(card);
+
 	rc = qeth_l2_setup_netdev(card, carrier_ok);
 	if (rc)
 		goto out_remove;
 
-	if (card->info.type != QETH_CARD_TYPE_OSN &&
-	    !qeth_l2_send_setmac(card, card->dev->dev_addr))
-		card->info.mac_bits |= QETH_LAYER2_MAC_REGISTERED;
-
 	if (qeth_is_diagass_supported(card, QETH_DIAGS_CMD_TRAP)) {
 		if (card->info.hwtrap &&
 		    qeth_hw_trap(card, QETH_DIAGS_TRAP_ARM))
-- 
2.16.4


  parent reply	other threads:[~2019-01-25 14:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-25 14:44 [PATCH net-next 0/8] s390/qeth: updates 2019-01-25 Julian Wiedmann
2019-01-25 14:44 ` [PATCH net-next 1/8] s390/qeth: streamline TX buffer management Julian Wiedmann
2019-01-25 14:44 ` [PATCH net-next 2/8] s390/qeth: remove bogus netif_wake_queue() Julian Wiedmann
2019-01-25 14:44 ` [PATCH net-next 3/8] s390/qeth: consolidate open/stop netdev ops Julian Wiedmann
2019-01-25 14:44 ` Julian Wiedmann [this message]
2019-01-25 14:44 ` [PATCH net-next 5/8] s390/qeth: remove TX disable from online path Julian Wiedmann
2019-01-25 14:44 ` [PATCH net-next 6/8] s390/qeth: delay netdevice registration Julian Wiedmann
2019-01-25 14:44 ` [PATCH net-next 7/8] s390/qeth: detach netdevice while card is offline Julian Wiedmann
2019-01-25 14:44 ` [PATCH net-next 8/8] s390/qeth: remove VLAN tracking for L2 devices Julian Wiedmann
2019-01-26  5:24 ` [PATCH net-next 0/8] s390/qeth: updates 2019-01-25 David Miller

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=20190125144423.81443-5-jwi@linux.ibm.com \
    --to=jwi@linux.ibm.com \
    --cc=davem@davemloft.net \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=raspl@linux.ibm.com \
    --cc=schwidefsky@de.ibm.com \
    --cc=ubraun@linux.ibm.com \
    /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;
as well as URLs for NNTP newsgroup(s).