| Message ID | 20220610023922.74892-1-lianglixuehao@126.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v6] igb: Assign random MAC address instead of fail in case of invalid one | expand |
On 6/9/2022 7:39 PM, Lixue Liang wrote: > From: Lixue Liang <lianglixue@greatwall.com.cn> > > Add the module parameter "allow_invalid_mac_address" to control the netdev maintainers: I know the use of module parameters is extremely frowned upon. Is this a use/exception that would be allowed? > 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> > --- > Changelog: > * v6: > - Modify commit messages and naming of module parameters > Suggested-by Paul <pmenzel@molgen.mpg.de> > * 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..b61f216331da 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 bool allow_invalid_mac_address; > module_param(debug, int, 0); > +module_param(allow_invalid_mac_address, bool, 0); > MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); > +MODULE_PARM_DESC(allow_invalid_mac_address, "Allow NIC driver to be loaded with invalid MAC address"); Lixue: If the maintainers are willing to take it, I believe the convention is to group each parameter together vs mixing them. e.g. static int debug = -1; module_param(debug, int, 0); MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); +static bool allow_invalid_mac_address; +module_param(allow_invalid_mac_address, bool, 0); +MODULE_PARM_DESC(allow_invalid_mac_address, "Allow NIC driver to be loaded with invalid MAC address"); Thanks, Tony > struct igb_reg_info { > u32 ofs;
On Tue, 14 Jun 2022 12:09:02 -0700 Tony Nguyen wrote: > > Add the module parameter "allow_invalid_mac_address" to control the > > netdev maintainers: > > I know the use of module parameters is extremely frowned upon. Is this a > use/exception that would be allowed? I think so, I don't see a different way here.
As alexander.duyck@gmail.com said, "allow_unsupported_sfp" in ixgbe_main.c, and I also noticed "eeprom_bad_csum_allow" in e100.c, are all in the form of module parameters. If the invalid MAC address is automatically replaced with a random MAC address, other problems caused by the random MAC address may be difficult to debug. Using module parameters can make it easy for users to correct invalid MAC addresses without causing the above problems > 2022年6月15日 09:19，Jakub Kicinski <kuba@kernel.org> 写道： > > On Tue, 14 Jun 2022 12:09:02 -0700 Tony Nguyen wrote: >>> Add the module parameter "allow_invalid_mac_address" to control the >> >> netdev maintainers: >> >> I know the use of module parameters is extremely frowned upon. Is this a >> use/exception that would be allowed? > > I think so, I don't see a different way here.
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 34b33b21e0dc..b61f216331da 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 bool allow_invalid_mac_address; module_param(debug, int, 0); +module_param(allow_invalid_mac_address, bool, 0); MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); +MODULE_PARM_DESC(allow_invalid_mac_address, "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 (!allow_invalid_mac_address) { + 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);