handling MAC set by user space in reset_resume() of r8152

* handling MAC set by user space in reset_resume() of r8152
@ 2022-07-27 11:39 Oliver Neukum
  2022-07-27 18:34 ` Andrew Lunn
  0 siblings, 1 reply; 5+ messages in thread
From: Oliver Neukum @ 2022-07-27 11:39 UTC (permalink / raw)
  To: Hayes Wang; +Cc: USB list, netdev

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


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