[PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one
@ 2022-05-12  9:39 ` lixue liang
  0 siblings, 0 replies; 16+ messages in thread
From: lixue liang @ 2022-05-12  9:39 UTC (permalink / raw)
  To: jesse.brandeburg, anthony.l.nguyen, kuba, intel-wired-lan, netdev
  Cc: lixue liang, kernel test robot

In some cases, when the user uses igb_set_eeprom to modify the MAC
address to be invalid, the igb driver will fail to load. If there is no
network card device, the user must modify it to a valid MAC address by
other means.

Since the MAC address can be modified ,then add a random valid MAC address
to replace the invalid MAC address in the driver can be workable, it can
continue to finish the loading ,and output the relevant log reminder.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: lixue liang <lianglixue@greatwall.com.cn>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index a513570c2ad6..746233befade 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3359,10 +3359,10 @@ 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)) {
-		eth_random_addr(netdev->dev_addr);
-		memcpy(hw->mac.addr, netdev->dev_addr, netdev->addr_len);
-		dev_info(&pdev->dev,
-			 "Invalid Mac Address, already got random Mac Address\n");
+		eth_hw_addr_random(netdev);
+		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
+		dev_err(&pdev->dev,
+			"Invalid MAC Address, already assigned random MAC Address\n");
 	}
 
 	igb_set_default_mac_filter(adapter);
-- 
2.27.0


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

* [Intel-wired-lan] [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one
@ 2022-05-12  9:39 ` lixue liang
  0 siblings, 0 replies; 16+ messages in thread
From: lixue liang @ 2022-05-12  9:39 UTC (permalink / raw)
  To: intel-wired-lan

In some cases, when the user uses igb_set_eeprom to modify the MAC
address to be invalid, the igb driver will fail to load. If there is no
network card device, the user must modify it to a valid MAC address by
other means.

Since the MAC address can be modified ,then add a random valid MAC address
to replace the invalid MAC address in the driver can be workable, it can
continue to finish the loading ,and output the relevant log reminder.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: lixue liang <lianglixue@greatwall.com.cn>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index a513570c2ad6..746233befade 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3359,10 +3359,10 @@ 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)) {
-		eth_random_addr(netdev->dev_addr);
-		memcpy(hw->mac.addr, netdev->dev_addr, netdev->addr_len);
-		dev_info(&pdev->dev,
-			 "Invalid Mac Address, already got random Mac Address\n");
+		eth_hw_addr_random(netdev);
+		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
+		dev_err(&pdev->dev,
+			"Invalid MAC Address, already assigned random MAC Address\n");
 	}
 
 	igb_set_default_mac_filter(adapter);
-- 
2.27.0


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

* Re: [Intel-wired-lan] [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one
  2022-05-12  9:39 ` [Intel-wired-lan] " lixue liang
@ 2022-05-12 13:55   ` Paul Menzel
  -1 siblings, 0 replies; 16+ messages in thread
From: Paul Menzel @ 2022-05-12 13:55 UTC (permalink / raw)
  To: lixue liang
  Cc: jesse.brandeburg, anthony.l.nguyen, kuba, intel-wired-lan, netdev

Dear Lixue,


Thank you for sending version 2. Some more minor nits.

Am 12.05.22 um 11:39 schrieb lixue liang:
> In some cases, when the user uses igb_set_eeprom to modify the MAC
> address to be invalid, the igb driver will fail to load. If there is no
> network card device, the user must modify it to a valid MAC address by
> other means.
> 
> Since the MAC address can be modified ,then add a random valid MAC address
> to replace the invalid MAC address in the driver can be workable, it can
> continue to finish the loading ,and output the relevant log reminder.

Please add the space after the comma.

> Reported-by: kernel test robot <lkp@intel.com>

This line is confusing. Maybe add that to the version change-log below 
the `---`.

> Signed-off-by: lixue liang <lianglixue@greatwall.com.cn>
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index a513570c2ad6..746233befade 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3359,10 +3359,10 @@ 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)) {
> -		eth_random_addr(netdev->dev_addr);
> -		memcpy(hw->mac.addr, netdev->dev_addr, netdev->addr_len);
> -		dev_info(&pdev->dev,
> -			 "Invalid Mac Address, already got random Mac Address\n");
> +		eth_hw_addr_random(netdev);
> +		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
> +		dev_err(&pdev->dev,
> +			"Invalid MAC Address, already assigned random MAC Address\n");

Please spell it MAC address.

>   	}
>   
>   	igb_set_default_mac_filter(adapter);


Kind regards,

Paul

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

* [Intel-wired-lan] [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one
@ 2022-05-12 13:55   ` Paul Menzel
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Menzel @ 2022-05-12 13:55 UTC (permalink / raw)
  To: intel-wired-lan

Dear Lixue,


Thank you for sending version 2. Some more minor nits.

Am 12.05.22 um 11:39 schrieb lixue liang:
> In some cases, when the user uses igb_set_eeprom to modify the MAC
> address to be invalid, the igb driver will fail to load. If there is no
> network card device, the user must modify it to a valid MAC address by
> other means.
> 
> Since the MAC address can be modified ,then add a random valid MAC address
> to replace the invalid MAC address in the driver can be workable, it can
> continue to finish the loading ,and output the relevant log reminder.

Please add the space after the comma.

> Reported-by: kernel test robot <lkp@intel.com>

This line is confusing. Maybe add that to the version change-log below 
the `---`.

> Signed-off-by: lixue liang <lianglixue@greatwall.com.cn>
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index a513570c2ad6..746233befade 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3359,10 +3359,10 @@ 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)) {
> -		eth_random_addr(netdev->dev_addr);
> -		memcpy(hw->mac.addr, netdev->dev_addr, netdev->addr_len);
> -		dev_info(&pdev->dev,
> -			 "Invalid Mac Address, already got random Mac Address\n");
> +		eth_hw_addr_random(netdev);
> +		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
> +		dev_err(&pdev->dev,
> +			"Invalid MAC Address, already assigned random MAC Address\n");

Please spell it MAC address.

>   	}
>   
>   	igb_set_default_mac_filter(adapter);


Kind regards,

Paul

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

* Re: [Intel-wired-lan] [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one
  2022-05-12 13:55   ` Paul Menzel
  (?)
@ 2022-05-13  3:25   ` 梁礼学
  -1 siblings, 0 replies; 16+ messages in thread
From: 梁礼学 @ 2022-05-13  3:25 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Jesse Brandeburg, anthony.l.nguyen, Jakub Kicinski,
	intel-wired-lan, netdev

Dear Paul,

Thank you so much for taking so much time to provide guidance,
I've fixed the other two issues, but the "Reported by" tag issue I don’t quite understand.

> 2022年5月12日 21:55，Paul Menzel <pmenzel@molgen.mpg.de> 写道：
> 
>> Reported-by: kernel test robot <lkp@intel.com>
> 
> This line is confusing. Maybe add that to the version change-log below the `—`

I add it in the form of cmmit message. I understand that you mean to add 
"Reported by" under '---‘, but I don't know how to add it. Please give me 
further guidance. I'm so sorry to trouble you.Sincerely waiting for your response.

Thank you!

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

* Re: [Intel-wired-lan] [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one
  2022-05-12 13:55   ` Paul Menzel
  (?)
  (?)
@ 2022-05-13  3:47   ` 梁礼学
  -1 siblings, 0 replies; 16+ messages in thread
From: 梁礼学 @ 2022-05-13  3:47 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Jesse Brandeburg, anthony.l.nguyen, Jakub Kicinski,
	intel-wired-lan, Netdev

Dear Paul,

Thank you so much for taking so much time to provide guidance,
I've fixed the other two issues, but the "Reported by" tag issue I don’t quite understand.

> 2022年5月12日 21:55，Paul Menzel <pmenzel@molgen.mpg.de> 写道：
> 
> Dear Lixue,
> 
> 
> Thank you for sending version 2. Some more minor nits.
> 
> Am 12.05.22 um 11:39 schrieb lixue liang:
>> In some cases, when the user uses igb_set_eeprom to modify the MAC
>> address to be invalid, the igb driver will fail to load. If there is no
>> network card device, the user must modify it to a valid MAC address by
>> other means.
>> Since the MAC address can be modified ,then add a random valid MAC address
>> to replace the invalid MAC address in the driver can be workable, it can
>> continue to finish the loading ,and output the relevant log reminder.
> 
> Please add the space after the comma.
> 
>> Reported-by: kernel test robot <lkp@intel.com>
> 
> This line is confusing. Maybe add that to the version change-log below the `---`.
> 

I add it in the form of cmmit message. I understand that you mean to add 
"Reported by" under '---‘, but I don't know how to add it. Please give me 
further guidance. I'm so sorry to trouble you.Sincerely waiting for your response.

Thank you!


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

* Re: [Intel-wired-lan] [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one
  2022-05-12 13:55   ` Paul Menzel
                     ` (2 preceding siblings ...)
  (?)
@ 2022-05-13  5:00   ` lixue liang
  -1 siblings, 0 replies; 16+ messages in thread
From: lixue liang @ 2022-05-13  5:00 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Jesse Brandeburg, anthony.l.nguyen, kuba, intel-wired-lan, netdev

Dear Paul,

Thank you so much for taking so much time to provide guidance,
I've fixed the other two issues, but the "Reported by" tag issue I don’t quite understand.

> 2022年5月12日 21:55，Paul Menzel <pmenzel@molgen.mpg.de> 写道：
> 
> Dear Lixue,
> 
> 
> Thank you for sending version 2. Some more minor nits.
> 
> Am 12.05.22 um 11:39 schrieb lixue liang:
>> In some cases, when the user uses igb_set_eeprom to modify the MAC
>> address to be invalid, the igb driver will fail to load. If there is no
>> network card device, the user must modify it to a valid MAC address by
>> other means.
>> Since the MAC address can be modified ,then add a random valid MAC address
>> to replace the invalid MAC address in the driver can be workable, it can
>> continue to finish the loading ,and output the relevant log reminder.
> 
> Please add the space after the comma.
> 
>> Reported-by: kernel test robot <lkp@intel.com>
> 
> This line is confusing. Maybe add that to the version change-log below the `---`.

I add it in the form of cmmit message. I understand that you mean to add 
"Reported by" under '---‘, but I don't know how to add it. Please give me 
further guidance. I'm so sorry to trouble you.Sincerely waiting for your response.

Thank you!

PS,
When I use lianglixue@greatwall.com.cn to reply, I get this error: 
<intel-wired-lan@lists.osuosl.org>: host lists.osuosl.org[140.211.166.34] said:
554 5.7.1 <spamfw.greatwall.com.cn[111.48.58.95]>: Client host rejected:
Access denied (in reply to RCPT TO command)

So I temporarily use lixue.liang5086@gmail.com to send, sorry about that.

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

[parent not found: <DM6PR11MB46576A8E6EBCB81E29FA49DC9BD69@DM6PR11MB4657.namprd11.prod.outlook.com>]

* [Intel-wired-lan] [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one
       [not found]   ` <DM6PR11MB46576A8E6EBCB81E29FA49DC9BD69@DM6PR11MB4657.namprd11.prod.outlook.com>
@ 2022-05-25  8:30     ` Palczewski, Mateusz
  0 siblings, 0 replies; 16+ messages in thread
From: Palczewski, Mateusz @ 2022-05-25  8:30 UTC (permalink / raw)
  To: intel-wired-lan

> Dear Lixue,
> 
> 
> Thank you for sending version 2. Some more minor nits.
> 
> Am 12.05.22 um 11:39 schrieb lixue liang:
> > In some cases, when the user uses igb_set_eeprom to modify the MAC 
> > address to be invalid, the igb driver will fail to load. If there is 
> > no network card device, the user must modify it to a valid MAC address 
> > by other means.
> > 
> > Since the MAC address can be modified ,then add a random valid MAC 
> > address to replace the invalid MAC address in the driver can be 
> > workable, it can continue to finish the loading ,and output the relevant log reminder.
> 
> Please add the space after the comma.
> 
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> This line is confusing. Maybe add that to the version change-log below the `---`.
> 
> > Signed-off-by: lixue liang <lianglixue@greatwall.com.cn>
> > ---
> >   drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > b/drivers/net/ethernet/intel/igb/igb_main.c
> > index a513570c2ad6..746233befade 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -3359,10 +3359,10 @@ 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)) {
> > -		eth_random_addr(netdev->dev_addr);
> > -		memcpy(hw->mac.addr, netdev->dev_addr, netdev->addr_len);
> > -		dev_info(&pdev->dev,
> > -			 "Invalid Mac Address, already got random Mac Address\n");
> > +		eth_hw_addr_random(netdev);
> > +		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
> > +		dev_err(&pdev->dev,
> > +			"Invalid MAC Address, already assigned random MAC Address\n");
> 
> Please spell it MAC address.
> 
> >   	}
> >   
> >   	igb_set_default_mac_filter(adapter);
> 
> 
> Kind regards,
> 
> Paul
> 
Hi, 
Please apply the changes suggested by Paul, other than that this looks good. 

Regards,
Mateusz 

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

* [PATCH v3 3/3] igb_main: Assign random MAC address instead of fail in case of invalid one
  2022-05-12 13:55   ` Paul Menzel
@ 2022-05-30  3:19     ` Lixue Liang
  -1 siblings, 0 replies; 16+ messages in thread
From: Lixue Liang @ 2022-05-30  3:19 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, the igb driver will fail to load. If there is no
network card device, the user must modify it to a valid MAC address by
other means.

Since the MAC address can be modified, then add a random valid MAC address
to replace the invalid MAC address in the driver can be workable, it can
continue to finish the loading, and output the relevant log reminder.

Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 746233befade..40f43534a3af 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3362,7 +3362,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		eth_hw_addr_random(netdev);
 		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
 		dev_err(&pdev->dev,
-			"Invalid MAC Address, already assigned random MAC Address\n");
+			"Invalid MAC address, already assigned random MAC address\n");
 	}
 
 	igb_set_default_mac_filter(adapter);
-- 
2.27.0


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

* [Intel-wired-lan] [PATCH v3 3/3] igb_main: Assign random MAC address instead of fail in case of invalid one
@ 2022-05-30  3:19     ` Lixue Liang
  0 siblings, 0 replies; 16+ messages in thread
From: Lixue Liang @ 2022-05-30  3:19 UTC (permalink / raw)
  To: intel-wired-lan

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, the igb driver will fail to load. If there is no
network card device, the user must modify it to a valid MAC address by
other means.

Since the MAC address can be modified, then add a random valid MAC address
to replace the invalid MAC address in the driver can be workable, it can
continue to finish the loading, and output the relevant log reminder.

Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 746233befade..40f43534a3af 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3362,7 +3362,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		eth_hw_addr_random(netdev);
 		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
 		dev_err(&pdev->dev,
-			"Invalid MAC Address, already assigned random MAC Address\n");
+			"Invalid MAC address, already assigned random MAC address\n");
 	}
 
 	igb_set_default_mac_filter(adapter);
-- 
2.27.0


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

* Re: [PATCH v3 3/3] igb_main: Assign random MAC address instead of fail in case of invalid one
  2022-05-30  3:19     ` [Intel-wired-lan] " Lixue Liang
@ 2022-05-30  7:47       ` Paul Menzel
  -1 siblings, 0 replies; 16+ messages in thread
From: Paul Menzel @ 2022-05-30  7:47 UTC (permalink / raw)
  To: Lixue Liang
  Cc: anthony.l.nguyen, intel-wired-lan, jesse.brandeburg, kuba,
	lianglixue, netdev

Dear Lixue,

Am 30.05.22 um 05:19 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, the igb driver will fail to load. If there is no
> network card device, the user must modify it to a valid MAC address by
> other means.
> 
> Since the MAC address can be modified, then add a random valid MAC address
> to replace the invalid MAC address in the driver can be workable, it can
> continue to finish the loading, and output the relevant log reminder.
> 
> Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 746233befade..40f43534a3af 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3362,7 +3362,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		eth_hw_addr_random(netdev);
>   		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
>   		dev_err(&pdev->dev,
> -			"Invalid MAC Address, already assigned random MAC Address\n");
> +			"Invalid MAC address, already assigned random MAC address\n");
>   	}
>   
>   	igb_set_default_mac_filter(adapter);

The diff does not do what is described in the commit message. I also 
just noticed, that it was spelled “MAC Address” before, so it would be 
fine for me if you send in this commit separately (but with the correct 
commit message).


Kind regards,

Paul

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

* [Intel-wired-lan] [PATCH v3 3/3] igb_main: Assign random MAC address instead of fail in case of invalid one
@ 2022-05-30  7:47       ` Paul Menzel
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Menzel @ 2022-05-30  7:47 UTC (permalink / raw)
  To: intel-wired-lan

Dear Lixue,

Am 30.05.22 um 05:19 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, the igb driver will fail to load. If there is no
> network card device, the user must modify it to a valid MAC address by
> other means.
> 
> Since the MAC address can be modified, then add a random valid MAC address
> to replace the invalid MAC address in the driver can be workable, it can
> continue to finish the loading, and output the relevant log reminder.
> 
> Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 746233befade..40f43534a3af 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3362,7 +3362,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		eth_hw_addr_random(netdev);
>   		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
>   		dev_err(&pdev->dev,
> -			"Invalid MAC Address, already assigned random MAC Address\n");
> +			"Invalid MAC address, already assigned random MAC address\n");
>   	}
>   
>   	igb_set_default_mac_filter(adapter);

The diff does not do what is described in the commit message. I also 
just noticed, that it was spelled ?MAC Address? before, so it would be 
fine for me if you send in this commit separately (but with the correct 
commit message).


Kind regards,

Paul

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

* [PATCH v3] igb_main: Assign random MAC address instead of fail in case of invalid one
  2022-05-12 13:55   ` Paul Menzel
@ 2022-05-30 10:58     ` Lixue Liang
  -1 siblings, 0 replies; 16+ messages in thread
From: Lixue Liang @ 2022-05-30 10:58 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, the igb driver will fail to load. If there is no
network card device, the user must modify it to a valid MAC address by
other means.

Since the MAC address can be modified, then add a random valid MAC address
to replace the invalid MAC address in the driver can be workable, it can
continue to finish the loading, and output the relevant log reminder.

Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++---
 1 file changed, 4 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..40f43534a3af 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3359,9 +3359,10 @@ 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;
+		eth_hw_addr_random(netdev);
+		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
+		dev_err(&pdev->dev,
+			"Invalid MAC address, already assigned random MAC address\n");
 	}
 
 	igb_set_default_mac_filter(adapter);
-- 
2.27.0


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

* [Intel-wired-lan] [PATCH v3] igb_main: Assign random MAC address instead of fail in case of invalid one
@ 2022-05-30 10:58     ` Lixue Liang
  0 siblings, 0 replies; 16+ messages in thread
From: Lixue Liang @ 2022-05-30 10:58 UTC (permalink / raw)
  To: intel-wired-lan

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, the igb driver will fail to load. If there is no
network card device, the user must modify it to a valid MAC address by
other means.

Since the MAC address can be modified, then add a random valid MAC address
to replace the invalid MAC address in the driver can be workable, it can
continue to finish the loading, and output the relevant log reminder.

Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++---
 1 file changed, 4 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..40f43534a3af 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3359,9 +3359,10 @@ 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;
+		eth_hw_addr_random(netdev);
+		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
+		dev_err(&pdev->dev,
+			"Invalid MAC address, already assigned random MAC address\n");
 	}
 
 	igb_set_default_mac_filter(adapter);
-- 
2.27.0


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

* Re: [PATCH v3] igb_main: Assign random MAC address instead of fail in case of invalid one
  2022-05-30 10:58     ` [Intel-wired-lan] " Lixue Liang
@ 2022-05-31 20:36       ` Tony Nguyen
  -1 siblings, 0 replies; 16+ messages in thread
From: Tony Nguyen @ 2022-05-31 20:36 UTC (permalink / raw)
  To: Lixue Liang, pmenzel
  Cc: intel-wired-lan, jesse.brandeburg, kuba, lianglixue, netdev



On 5/30/2022 3:58 AM, Lixue Liang wrote:

nit: Please use just the driver name in the title, not the file being 
modified. i.e. igb: not igb_main:

Also, as Paul pointed out earlier, each time you do a revision, please 
increment the vX number and please include a changelog under the '---' 
to make review easier.

> 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, the igb driver will fail to load. If there is no
> network card device, the user must modify it to a valid MAC address by
> other means.
> 
> Since the MAC address can be modified, then add a random valid MAC address
> to replace the invalid MAC address in the driver can be workable, it can
> continue to finish the loading, and output the relevant log reminder.
> 
> Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++---
>   1 file changed, 4 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..40f43534a3af 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3359,9 +3359,10 @@ 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;
> +		eth_hw_addr_random(netdev);
> +		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
> +		dev_err(&pdev->dev,
> +			"Invalid MAC address, already assigned random MAC address\n");

I prefer the message as Alex suggested [1]: "Invalid MAC address. 
Assigned random MAC address". "already assigned random MAC address" 
seems a bit confusing to me.

Thanks,
Tony

>   	}
>   
>   	igb_set_default_mac_filter(adapter);

[1] 
https://lore.kernel.org/netdev/ad8cf673c8e9e21cb2e7afeb5c7e66cc76a36995.camel@gmail.com/

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

* [Intel-wired-lan] [PATCH v3] igb_main: Assign random MAC address instead of fail in case of invalid one
@ 2022-05-31 20:36       ` Tony Nguyen
  0 siblings, 0 replies; 16+ messages in thread
From: Tony Nguyen @ 2022-05-31 20:36 UTC (permalink / raw)
  To: intel-wired-lan



On 5/30/2022 3:58 AM, Lixue Liang wrote:

nit: Please use just the driver name in the title, not the file being 
modified. i.e. igb: not igb_main:

Also, as Paul pointed out earlier, each time you do a revision, please 
increment the vX number and please include a changelog under the '---' 
to make review easier.

> 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, the igb driver will fail to load. If there is no
> network card device, the user must modify it to a valid MAC address by
> other means.
> 
> Since the MAC address can be modified, then add a random valid MAC address
> to replace the invalid MAC address in the driver can be workable, it can
> continue to finish the loading, and output the relevant log reminder.
> 
> Signed-off-by: Lixue Liang <lianglixue@greatwall.com.cn>
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++---
>   1 file changed, 4 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..40f43534a3af 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3359,9 +3359,10 @@ 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;
> +		eth_hw_addr_random(netdev);
> +		ether_addr_copy(hw->mac.addr, netdev->dev_addr);
> +		dev_err(&pdev->dev,
> +			"Invalid MAC address, already assigned random MAC address\n");

I prefer the message as Alex suggested [1]: "Invalid MAC address. 
Assigned random MAC address". "already assigned random MAC address" 
seems a bit confusing to me.

Thanks,
Tony

>   	}
>   
>   	igb_set_default_mac_filter(adapter);

[1] 
https://lore.kernel.org/netdev/ad8cf673c8e9e21cb2e7afeb5c7e66cc76a36995.camel at gmail.com/

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

end of thread, other threads:[~2022-05-31 20:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-12  9:39 [PATCH v2 2/2] igb_main: Assign random MAC address instead of fail in case of invalid one lixue liang
2022-05-12  9:39 ` [Intel-wired-lan] " lixue liang
2022-05-12 13:55 ` Paul Menzel
2022-05-12 13:55   ` Paul Menzel
2022-05-13  3:25   ` 梁礼学
2022-05-13  3:47   ` 梁礼学
2022-05-13  5:00   ` lixue liang
     [not found]   ` <DM6PR11MB46576A8E6EBCB81E29FA49DC9BD69@DM6PR11MB4657.namprd11.prod.outlook.com>
2022-05-25  8:30     ` Palczewski, Mateusz
2022-05-30  3:19   ` [PATCH v3 3/3] " Lixue Liang
2022-05-30  3:19     ` [Intel-wired-lan] " Lixue Liang
2022-05-30  7:47     ` Paul Menzel
2022-05-30  7:47       ` [Intel-wired-lan] " Paul Menzel
2022-05-30 10:58   ` [PATCH v3] " Lixue Liang
2022-05-30 10:58     ` [Intel-wired-lan] " Lixue Liang
2022-05-31 20:36     ` Tony Nguyen
2022-05-31 20:36       ` [Intel-wired-lan] " Tony Nguyen

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.