All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [PATCH v5] igb: Assign random MAC address instead of fail in case of invalid one
@ 2022-06-09  8:39 ` Lixue Liang
  0 siblings, 0 replies; 4+ messages in thread
From: Lixue Liang @ 2022-06-09  8:39 UTC (permalink / raw)
  To: pmenzel; +Cc: lianglixue, netdev, intel-wired-lan, kuba

From: Lixue Liang <lianglixue@greatwall.com.cn>

In some cases, when the user uses igb_set_eeprom to modify the MAC address
to be invalid, or an invalid MAC address appears when with uninitialized
samples, the igb driver will fail to load. If there is no network card
device, the user could not conveniently modify it to a valid MAC address,
for example using ethtool to modify.

Through module parameter to set，when the MAC address is invalid, a random
valid MAC address can be used to continue loading and output relevant log
reminders. In this way, users can conveniently correct invalid MAC address.

Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
---
Changelog:
* v5:
  - Through the setting of module parameters, it is allowed to complete
    the loading of the igb network card driver with an invalid MAC address.
  Suggested-by <alexander.duyck@gmail.com>
* v4:
  - Change the igb_mian in the title to igb
  - Fix dev_err message: replace "already assigned random MAC address"
    with "Invalid MAC address. Assigned random MAC address"
  Suggested-by Tony <anthony.l.nguyen@intel.com>

* v3:
  - Add space after comma in commit message
  - Correct spelling of MAC address
  Suggested-by Paul <pmenzel@molgen.mpg.de>

* v2:
  - Change memcpy to ether_addr_copy
  - Change dev_info to dev_err
  - Fix the description of the commit message
  - Change eth_random_addr to eth_hw_addr_random
  Reported-by: kernel test robot <lkp@intel.com>

 drivers/net/ethernet/intel/igb/igb_main.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 34b33b21e0dc..8162e8999ccb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -238,8 +238,11 @@ MODULE_LICENSE("GPL v2");
 
 #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
 static int debug = -1;
+static unsigned int invalid_mac_address_allow;
 module_param(debug, int, 0);
+module_param(invalid_mac_address_allow, uint, 0);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
+MODULE_PARM_DESC(invalid_mac_address_allow, "Allow NIC driver to be loaded with invalid MAC address");
 
 struct igb_reg_info {
 	u32 ofs;
@@ -3359,9 +3362,16 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	eth_hw_addr_set(netdev, hw->mac.addr);
 
 	if (!is_valid_ether_addr(netdev->dev_addr)) {
-		dev_err(&pdev->dev, "Invalid MAC Address\n");
-		err = -EIO;
-		goto err_eeprom;
+		if (!invalid_mac_address_allow) {
+			dev_err(&pdev->dev, "Invalid MAC Address\n");
+			err = -EIO;
+			goto err_eeprom;
+		} else {
+			eth_hw_addr_random(netdev);
+			ether_addr_copy(hw->mac.addr, netdev->dev_addr);
+			dev_err(&pdev->dev,
+				"Invalid MAC address. Assigned random MAC address\n");
+		}
 	}
 
 	igb_set_default_mac_filter(adapter);
-- 
2.27.0

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH v5] igb: Assign random MAC address instead of fail in case of invalid one
@ 2022-06-09  8:39 ` Lixue Liang
  0 siblings, 0 replies; 4+ messages in thread
From: Lixue Liang @ 2022-06-09  8:39 UTC (permalink / raw)
  To: pmenzel
  Cc: anthony.l.nguyen, intel-wired-lan, jesse.brandeburg, kuba,
	lianglixue, netdev

From: Lixue Liang <lianglixue@greatwall.com.cn>

In some cases, when the user uses igb_set_eeprom to modify the MAC address
to be invalid, or an invalid MAC address appears when with uninitialized
samples, the igb driver will fail to load. If there is no network card
device, the user could not conveniently modify it to a valid MAC address,
for example using ethtool to modify.

Through module parameter to set，when the MAC address is invalid, a random
valid MAC address can be used to continue loading and output relevant log
reminders. In this way, users can conveniently correct invalid MAC address.

Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
---
Changelog:
* v5:
  - Through the setting of module parameters, it is allowed to complete
    the loading of the igb network card driver with an invalid MAC address.
  Suggested-by <alexander.duyck@gmail.com>
* v4:
  - Change the igb_mian in the title to igb
  - Fix dev_err message: replace "already assigned random MAC address"
    with "Invalid MAC address. Assigned random MAC address"
  Suggested-by Tony <anthony.l.nguyen@intel.com>

* v3:
  - Add space after comma in commit message
  - Correct spelling of MAC address
  Suggested-by Paul <pmenzel@molgen.mpg.de>

* v2:
  - Change memcpy to ether_addr_copy
  - Change dev_info to dev_err
  - Fix the description of the commit message
  - Change eth_random_addr to eth_hw_addr_random
  Reported-by: kernel test robot <lkp@intel.com>

 drivers/net/ethernet/intel/igb/igb_main.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 34b33b21e0dc..8162e8999ccb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -238,8 +238,11 @@ MODULE_LICENSE("GPL v2");
 
 #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
 static int debug = -1;
+static unsigned int invalid_mac_address_allow;
 module_param(debug, int, 0);
+module_param(invalid_mac_address_allow, uint, 0);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
+MODULE_PARM_DESC(invalid_mac_address_allow, "Allow NIC driver to be loaded with invalid MAC address");
 
 struct igb_reg_info {
 	u32 ofs;
@@ -3359,9 +3362,16 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	eth_hw_addr_set(netdev, hw->mac.addr);
 
 	if (!is_valid_ether_addr(netdev->dev_addr)) {
-		dev_err(&pdev->dev, "Invalid MAC Address\n");
-		err = -EIO;
-		goto err_eeprom;
+		if (!invalid_mac_address_allow) {
+			dev_err(&pdev->dev, "Invalid MAC Address\n");
+			err = -EIO;
+			goto err_eeprom;
+		} else {
+			eth_hw_addr_random(netdev);
+			ether_addr_copy(hw->mac.addr, netdev->dev_addr);
+			dev_err(&pdev->dev,
+				"Invalid MAC address. Assigned random MAC address\n");
+		}
 	}
 
 	igb_set_default_mac_filter(adapter);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v5] igb: Assign random MAC address instead of fail in case of invalid one
  2022-06-09  8:39 ` Lixue Liang
@ 2022-06-09 11:07   ` Paul Menzel
  -1 siblings, 0 replies; 4+ messages in thread
From: Paul Menzel @ 2022-06-09 11:07 UTC (permalink / raw)
  To: Lixue Liang
  Cc: anthony.l.nguyen, intel-wired-lan, jesse.brandeburg, kuba,
	lianglixue, netdev

Dear Lixue,


Thank you for all the iterations.

Am 09.06.22 um 10:39 schrieb Lixue Liang:
> From: Lixue Liang <lianglixue@greatwall.com.cn>
> 
> In some cases, when the user uses igb_set_eeprom to modify the MAC address
> to be invalid, or an invalid MAC address appears when with uninitialized
> samples, the igb driver will fail to load. If there is no network card
> device, the user could not conveniently modify it to a valid MAC address,
> for example using ethtool to modify.
> 
> Through module parameter to set，when the MAC address is invalid, a random
> valid MAC address can be used to continue loading and output relevant log
> reminders. In this way, users can conveniently correct invalid MAC address.

Maybe:

Add the module parameter `…` to control the behavior. When set to true, 
a random MAC address is assigned, and the driver can be loaded, allowing 
the user to correct the invalid MAC address.

> 
> Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
> ---

[…]

> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 34b33b21e0dc..8162e8999ccb 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -238,8 +238,11 @@ MODULE_LICENSE("GPL v2");
>   
>   #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
>   static int debug = -1;
> +static unsigned int invalid_mac_address_allow;

Make it a boolean?

>   module_param(debug, int, 0);
> +module_param(invalid_mac_address_allow, uint, 0);

Name it `allow_invalid_mac_address`?

>   MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
> +MODULE_PARM_DESC(invalid_mac_address_allow, "Allow NIC driver to be loaded with invalid MAC address");
>   
>   struct igb_reg_info {
>   	u32 ofs;
> @@ -3359,9 +3362,16 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	eth_hw_addr_set(netdev, hw->mac.addr);
>   
>   	if (!is_valid_ether_addr(netdev->dev_addr)) {
> -		dev_err(&pdev->dev, "Invalid MAC Address\n");
> -		err = -EIO;
> -		goto err_eeprom;
> +		if (!invalid_mac_address_allow) {
> +			dev_err(&pdev->dev, "Invalid MAC Address\n");

Correct the spelling in a patch in front of this patch?

> +			err = -EIO;
> +			goto err_eeprom;
> +		} else {
> +			eth_hw_addr_random(netdev);
> +			ether_addr_copy(hw->mac.addr, netdev->dev_addr);
> +			dev_err(&pdev->dev,
> +				"Invalid MAC address. Assigned random MAC address\n");
> +		}
>   	}
>   
>   	igb_set_default_mac_filter(adapter);


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Intel-wired-lan] [PATCH v5] igb: Assign random MAC address instead of fail in case of invalid one
@ 2022-06-09 11:07   ` Paul Menzel
  0 siblings, 0 replies; 4+ messages in thread
From: Paul Menzel @ 2022-06-09 11:07 UTC (permalink / raw)
  To: Lixue Liang; +Cc: lianglixue, netdev, intel-wired-lan, kuba

Dear Lixue,


Thank you for all the iterations.

Am 09.06.22 um 10:39 schrieb Lixue Liang:
> From: Lixue Liang <lianglixue@greatwall.com.cn>
> 
> In some cases, when the user uses igb_set_eeprom to modify the MAC address
> to be invalid, or an invalid MAC address appears when with uninitialized
> samples, the igb driver will fail to load. If there is no network card
> device, the user could not conveniently modify it to a valid MAC address,
> for example using ethtool to modify.
> 
> Through module parameter to set，when the MAC address is invalid, a random
> valid MAC address can be used to continue loading and output relevant log
> reminders. In this way, users can conveniently correct invalid MAC address.

Maybe:

Add the module parameter `…` to control the behavior. When set to true, 
a random MAC address is assigned, and the driver can be loaded, allowing 
the user to correct the invalid MAC address.

> 
> Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
> ---

[…]

> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 34b33b21e0dc..8162e8999ccb 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -238,8 +238,11 @@ MODULE_LICENSE("GPL v2");
>   
>   #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
>   static int debug = -1;
> +static unsigned int invalid_mac_address_allow;

Make it a boolean?

>   module_param(debug, int, 0);
> +module_param(invalid_mac_address_allow, uint, 0);

Name it `allow_invalid_mac_address`?

>   MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
> +MODULE_PARM_DESC(invalid_mac_address_allow, "Allow NIC driver to be loaded with invalid MAC address");
>   
>   struct igb_reg_info {
>   	u32 ofs;
> @@ -3359,9 +3362,16 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	eth_hw_addr_set(netdev, hw->mac.addr);
>   
>   	if (!is_valid_ether_addr(netdev->dev_addr)) {
> -		dev_err(&pdev->dev, "Invalid MAC Address\n");
> -		err = -EIO;
> -		goto err_eeprom;
> +		if (!invalid_mac_address_allow) {
> +			dev_err(&pdev->dev, "Invalid MAC Address\n");

Correct the spelling in a patch in front of this patch?

> +			err = -EIO;
> +			goto err_eeprom;
> +		} else {
> +			eth_hw_addr_random(netdev);
> +			ether_addr_copy(hw->mac.addr, netdev->dev_addr);
> +			dev_err(&pdev->dev,
> +				"Invalid MAC address. Assigned random MAC address\n");
> +		}
>   	}
>   
>   	igb_set_default_mac_filter(adapter);


Kind regards,

Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-09 11:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-09  8:39 [Intel-wired-lan] [PATCH v5] igb: Assign random MAC address instead of fail in case of invalid one Lixue Liang
2022-06-09  8:39 ` Lixue Liang
2022-06-09 11:07 ` Paul Menzel
2022-06-09 11:07   ` [Intel-wired-lan] " Paul Menzel

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.