lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 24 Jun 2009 14:45:42 +0900 From: Naohiro Ooiwa <nooiwa@...aclelinux.com> To: Michael Chan <mchan@...adcom.com> CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org> Subject: Re: [PATCH] bnx2: Fix the behavior of ethtool when ONBOOT=no Hi Michael Thank you for your reply. I tested this patch now. The result was good for me. Thanks you. Naohiro Ooiwa Michael Chan wrote: > Naohiro Ooiwa wrote: > >> Hi Michael >> >> Thank you for quick reply and checking my patch. >> >> You're right. >> So, can we change the get_link method in ethtool ops. >> The following is image. >> >> How do you feel about this idea. >> >> >> Best Regards, >> Naohiro Ooiwa >> >> >> Signed-off-by: Ooiwa Naohiro <nooiwa@...aclelinux.com> > > This patch is fine. Thanks. > > Acked-by: Michael Chan <mchan@...adcom.com> > >> --- >> bnx2.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c >> index 9eee986..89cd235 100644 >> --- a/drivers/net/bnx2.c >> +++ b/drivers/net/bnx2.c >> @@ -6824,6 +6824,14 @@ bnx2_nway_reset(struct net_device *dev) >> } >> >> static int >> +bnx2_get_link(struct net_device *dev) >> +{ >> + struct bnx2 *bp = netdev_priv(dev); >> + >> + return bp->link_up; >> +} >> + >> +static int >> bnx2_get_eeprom_len(struct net_device *dev) >> { >> struct bnx2 *bp = netdev_priv(dev); >> @@ -7390,7 +7398,7 @@ static const struct ethtool_ops >> bnx2_ethtool_ops = { >> .get_wol = bnx2_get_wol, >> .set_wol = bnx2_set_wol, >> .nway_reset = bnx2_nway_reset, >> - .get_link = ethtool_op_get_link, >> + .get_link = bnx2_get_link, >> .get_eeprom_len = bnx2_get_eeprom_len, >> .get_eeprom = bnx2_get_eeprom, >> .set_eeprom = bnx2_set_eeprom, >> >> >> >> >> Michael Chan wrote: >>> Naohiro Ooiwa wrote: >>> >>>> Hi Michael >>>> >>>> I found a little bug. >>>> >>>> When configure in ifcfg-eth* is ONBOOT=no, >>>> the behavior of ethtool command is wrong. >>>> >>>> # grep ONBOOT /etc/sysconfig/network-scripts/ifcfg-eth2 >>>> ONBOOT=no >>>> # ethtool eth2 | tail -n1 >>>> Link detected: yes >>>> >>>> I think "Link detected" should be "no". >>>> >>>> The bnx2 driver should run netif_carrier_off() in initialization >>>> until bnx2_open() is called. >>>> >>>> The following is my patch. >>>> It move netif_carrier_off() to bnx2_init_one(). >>>> I had already tested this patch. The result is good for me. >>>> >>>> Could you please check the my patch ? >>>> >>>> >>>> Best Regards, >>>> Naohiro Ooiwa >>>> >>>> >>>> Signed-off-by: Naohiro Ooiwa <nooiwa@...iraclelinux.com> >>> This patch will not work. Calling netif_carrier_off() will schedule >>> a link_watch event. If register_netdev() fails for any reason, the >>> netdev will be freed but the link_watch event can still be >> scheduled. >>> Calling netif_carrier_off() after register_netdev() returns also >>> will not work because it can then race with ->open() and >> the interrupt >>> that indicates link up. >>> >>> Thanks. >>> >>>> --- >>>> drivers/net/bnx2.c | 5 +++-- >>>> 1 files changed, 3 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c >>>> index 38f1c33..9eee986 100644 >>>> --- a/drivers/net/bnx2.c >>>> +++ b/drivers/net/bnx2.c >>>> @@ -6151,8 +6151,6 @@ bnx2_open(struct net_device *dev) >>>> struct bnx2 *bp = netdev_priv(dev); >>>> int rc; >>>> >>>> - netif_carrier_off(dev); >>>> - >>>> bnx2_set_power_state(bp, PCI_D0); >>>> bnx2_disable_int(bp); >>>> >>>> @@ -8066,6 +8064,9 @@ bnx2_init_one(struct pci_dev *pdev, >>>> const struct pci_device_id *ent) >>>> if (CHIP_NUM(bp) == CHIP_NUM_5709) >>>> dev->features |= NETIF_F_TSO6; >>>> >>>> + /* Should set nocarrier until bnx2_open() is called */ >>>> + netif_carrier_off(dev); >>>> + >>>> if ((rc = register_netdev(dev))) { >>>> dev_err(&pdev->dev, "Cannot register net device\n"); >>>> goto error; >>>> -- >>>> 1.5.4.1 >>>> >>>> >>>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>> the body of a message to majordomo@...r.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists