All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Neukum <oneukum@suse.com>
To: Hayes Wang <hayeswang@realtek.com>
Cc: USB list <linux-usb@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: handling MAC set by user space in reset_resume() of r8152
Date: Wed, 27 Jul 2022 13:39:43 +0200	[thread overview]
Message-ID: <2397d98d-e373-1740-eb5f-8fe795a0352a@suse.com> (raw)

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

Hi,

looking at the driver it looks to me like the address
provided to ndo_set_mac_address() is thrown away after use.
That looks problematic to me, because reset_resume()
should redo the operation.
What do you think?

	Regards
		Oliver

[-- Attachment #2: 0001-r8152-restore-external-MAC-in-reset_resume.patch --]
[-- Type: text/x-patch, Size: 3349 bytes --]

From 19fc972a5cc98197bc81a7c56dd5d68e3fdfc36b Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Wed, 27 Jul 2022 13:29:42 +0200
Subject: [PATCH] r8152: restore external MAC in reset_resume

If user space has set the MAC of the interface,
reset_resume() must restore that setting rather
than redetermine the MAC like if te interface
is probed regularly.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/r8152.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0f6efaabaa32..5cf74b984655 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -923,6 +923,7 @@ struct r8152 {
 	atomic_t rx_count;
 
 	bool eee_en;
+	bool external_mac;
 	int intr_interval;
 	u32 saved_wolopts;
 	u32 msg_enable;
@@ -933,6 +934,8 @@ struct r8152 {
 	u32 rx_copybreak;
 	u32 rx_pending;
 	u32 fc_pause_on, fc_pause_off;
+	/* for reset_resume */
+	struct sockaddr saved_addr;
 
 	unsigned int pipe_in, pipe_out, pipe_intr, pipe_ctrl_in, pipe_ctrl_out;
 
@@ -1574,6 +1577,7 @@ static int __rtl8152_set_mac_address(struct net_device *netdev, void *p,
 	mutex_lock(&tp->control);
 
 	eth_hw_addr_set(netdev, addr->sa_data);
+	memcpy(&tp->saved_addr, addr, sizeof(tp->saved_addr));
 
 	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
 	pla_ocp_write(tp, PLA_IDR, BYTE_EN_SIX_BYTES, 8, addr->sa_data);
@@ -1589,7 +1593,13 @@ static int __rtl8152_set_mac_address(struct net_device *netdev, void *p,
 
 static int rtl8152_set_mac_address(struct net_device *netdev, void *p)
 {
-	return __rtl8152_set_mac_address(netdev, p, false);
+	int rv;
+	struct r8152 *tp = netdev_priv(netdev);
+
+	rv =  __rtl8152_set_mac_address(netdev, p, false);
+	if (!rv)
+		tp->external_mac = true;
+	return rv;
 }
 
 /* Devices containing proper chips can support a persistent
@@ -1676,10 +1686,14 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
 static int determine_ethernet_addr(struct r8152 *tp, struct sockaddr *sa)
 {
 	struct net_device *dev = tp->netdev;
-	int ret;
+	int ret = 0;
 
 	sa->sa_family = dev->type;
 
+	if (tp->external_mac) {
+		ether_addr_copy(sa->sa_data, dev->dev_addr);
+		return ret;
+	}
 	ret = eth_platform_get_mac_address(&tp->udev->dev, sa->sa_data);
 	if (ret < 0) {
 		if (tp->version == RTL_VER_01) {
@@ -1713,17 +1727,18 @@ static int determine_ethernet_addr(struct r8152 *tp, struct sockaddr *sa)
 static int set_ethernet_addr(struct r8152 *tp, bool in_resume)
 {
 	struct net_device *dev = tp->netdev;
-	struct sockaddr sa;
-	int ret;
+	int ret = 0;
 
-	ret = determine_ethernet_addr(tp, &sa);
-	if (ret < 0)
-		return ret;
+	if (!tp->external_mac) {
+		ret = determine_ethernet_addr(tp, &tp->saved_addr);
+		if (ret < 0)
+			return ret;
+	}
 
 	if (tp->version == RTL_VER_01)
-		eth_hw_addr_set(dev, sa.sa_data);
+		eth_hw_addr_set(dev, tp->saved_addr.sa_data);
 	else
-		ret = __rtl8152_set_mac_address(dev, &sa, in_resume);
+		ret = __rtl8152_set_mac_address(dev, &tp->saved_addr, in_resume);
 
 	return ret;
 }
@@ -6225,6 +6240,7 @@ static void rtl8152_down(struct r8152 *tp)
 	r8152_aldps_en(tp, false);
 	r8152b_enter_oob(tp);
 	r8152_aldps_en(tp, true);
+	tp->external_mac = false;
 }
 
 static void rtl8153_up(struct r8152 *tp)
-- 
2.35.3


             reply	other threads:[~2022-07-27 11:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27 11:39 Oliver Neukum [this message]
2022-07-27 18:34 ` handling MAC set by user space in reset_resume() of r8152 Andrew Lunn
2022-07-28  8:40   ` Hayes Wang
2022-07-28  8:54     ` Oliver Neukum
2022-07-28  9:41       ` Hayes Wang

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=2397d98d-e373-1740-eb5f-8fe795a0352a@suse.com \
    --to=oneukum@suse.com \
    --cc=hayeswang@realtek.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.