| Message ID | 20220510012159.8924-1-lianglixue@greatwall.com.cn |
|---|---|
| State | Superseded |
| Headers | show |
| Series | igb_ethtool: fix efficiency issues in igb_set_eeprom | expand |
Thank you very much for replying to my question and for suggesting corrections. Since the interface (igb_set_eeprom) for arbitrarily modifying the mac address is provided, it is equivalent to providing a way to modify the mac address arbitrarily and cause the igb driver to fail to load. Therefore, a way to restore a valid mac address should also be reserved, so as to prevent most users from being helpless in the case of an invalid mac address. Except by using tools to flash the firmware or modify the igb driver to continue loading under an invalid mac address, however most users do not have this ability. In the case of invalidity, the invalid mac address must be changed to a legal address, so it is always better to use a valid mac address to continue pretending to be a network card driver when the mac address is invalid, which is always better than not being able to load the driver, such as the microchip network card (lan743x) driver . I think it is worthwhile to replace the invalid mac address with a random valid mac address to complete the igb driver loading. In response to the above problem, I will provide a patch on igb_main.c again.. Thank you for taking the time. > 2022年5月11日 03:43，Jesse Brandeburg <jesse.brandeburg@intel.com> 写道： > > Please include netdev mailing list when mailing the maintainers of netdev.
Dear lianglixue, Thank you for your patch. Am 10.05.22 um 03:21 schrieb lianglixue: It’d be great if you spelled your name with spaces (if possible). Also, currently your Google Mail address would be used for the Author field. If you want to use your company(?) address, please add a From: line at the top. > Modify the value of eeprom in igb_set_eeprom. If the mac address > of i210 is changed to illegal, then in igp_probe in the > igb_main file, is_valid_eth_addr (netdev->dev_addr) exits > with an error, causing the igb driver to fail to load, > such as ethtool -E eth0 magic 0x15338086 offset 0 value 0x01. > > In this way, the igb driver can no longer be loaded, > and the legal mac address cannot be recovered through ethtool; > add is_valid_eth_addr to igb_set_eeprom to determine > whether it is legal to rewrite, so as to avoid driver > errors due to illegal mac addresses. Please reflow the text for 75 characters per line. > Signed-off-by: lianglixue <lianglixue@greatwall.com.cn> > --- > drivers/net/ethernet/intel/igb/igb_ethtool.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c > index 2a5782063f4c..30554fd684db 100644 > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c > @@ -798,6 +798,13 @@ static int igb_set_eeprom(struct net_device *netdev, > if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16))) > return -EFAULT; > > + if (hw->mac.type == e1000_i210 && eeprom->offset == 0) { > + if (!is_valid_ether_addr(bytes)) { > + dev_err(&adapter->pdev->dev, "Invalid MAC Address for i210\n"); > + return -EINVAL; > + } > + } > + > max_len = hw->nvm.word_size * 2; > > first_word = eeprom->offset >> 1; Kind regards, Paul
Hi Paul, Thank you very much for your reply and suggestions.I have made the corresponding changes according to your suggestion. In addition, for the problem that the invalid mac address cannot load the igb driver, I personally think there is a better way to modify it, and I will resubmit the patch about igb_main.c. It's the same question, but I may not know how to continue submitting new patches on this email, sorry about that. > 2022年5月11日 14:41，Paul Menzel <pmenzel@molgen.mpg.de> 写道： > > Dear lianglixue, > > > Thank you for your patch. > > Am 10.05.22 um 03:21 schrieb lianglixue: > > It’d be great if you spelled your name with spaces (if possible). > > Also, currently your Google Mail address would be used for the Author field. If you want to use your company(?) address, please add a From: line at the top. > >> Modify the value of eeprom in igb_set_eeprom. If the mac address >> of i210 is changed to illegal, then in igp_probe in the >> igb_main file, is_valid_eth_addr (netdev->dev_addr) exits >> with an error, causing the igb driver to fail to load, >> such as ethtool -E eth0 magic 0x15338086 offset 0 value 0x01. >> In this way, the igb driver can no longer be loaded, >> and the legal mac address cannot be recovered through ethtool; >> add is_valid_eth_addr to igb_set_eeprom to determine >> whether it is legal to rewrite, so as to avoid driver >> errors due to illegal mac addresses. > > Please reflow the text for 75 characters per line. > >> Signed-off-by: lianglixue <lianglixue@greatwall.com.cn> >> --- >> drivers/net/ethernet/intel/igb/igb_ethtool.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c >> index 2a5782063f4c..30554fd684db 100644 >> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c >> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c >> @@ -798,6 +798,13 @@ static int igb_set_eeprom(struct net_device *netdev, >> if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16))) >> return -EFAULT; >> + if (hw->mac.type == e1000_i210 && eeprom->offset == 0) { >> + if (!is_valid_ether_addr(bytes)) { >> + dev_err(&adapter->pdev->dev, "Invalid MAC Address for i210\n"); >> + return -EINVAL; >> + } >> + } >> + >> max_len = hw->nvm.word_size * 2; >> first_word = eeprom->offset >> 1; > > > Kind regards, > > Paul
Dear Lixue, Am 11.05.22 um 09:59 schrieb lixue liang: > Thank you very much for your reply and suggestions.I have made the > corresponding changes according to your suggestion. Thank you. > In addition, for the problem that the invalid mac address cannot load > the igb driver, I personally think there is a better way to modify > it, and I will resubmit the patch about igb_main.c. Awesome. > It's the same question, but I may not know how to continue submitting > new patches on this email, sorry about that. As you use `git send-email`, what does `git log` show as the patch author? It might be as easy as to do git config --global user.name "Lixue Liang" git config --global user.email lianglixue@greatwall.com.cn git commit --amend --author "Lixue Liang <lianglixue@greatwall.com.cn>" Kind regards, Paul PS: When replying, it’d be great if you used interleaved style [1]. [1]: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c index 2a5782063f4c..30554fd684db 100644 --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c @@ -798,6 +798,13 @@ static int igb_set_eeprom(struct net_device *netdev, if (eeprom->magic != (hw->vendor_id | (hw->device_id << 16))) return -EFAULT; + if (hw->mac.type == e1000_i210 && eeprom->offset == 0) { + if (!is_valid_ether_addr(bytes)) { + dev_err(&adapter->pdev->dev, "Invalid MAC Address for i210\n"); + return -EINVAL; + } + } + max_len = hw->nvm.word_size * 2; first_word = eeprom->offset >> 1;
Modify the value of eeprom in igb_set_eeprom. If the mac address of i210 is changed to illegal, then in igp_probe in the igb_main file, is_valid_eth_addr (netdev->dev_addr) exits with an error, causing the igb driver to fail to load, such as ethtool -E eth0 magic 0x15338086 offset 0 value 0x01. In this way, the igb driver can no longer be loaded, and the legal mac address cannot be recovered through ethtool; add is_valid_eth_addr to igb_set_eeprom to determine whether it is legal to rewrite, so as to avoid driver errors due to illegal mac addresses. Signed-off-by: lianglixue <lianglixue@greatwall.com.cn> --- drivers/net/ethernet/intel/igb/igb_ethtool.c | 7 +++++++ 1 file changed, 7 insertions(+)