linux-kernel - RE: [PATCH] Revert "net: ethernet: ixgbe: check the return value of ixgbe_mii_bus

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR11MB2600094C066D51C15B81EA70E5360@BYAPR11MB2600.namprd11.prod.outlook.com>
Date:   Fri, 25 Sep 2020 08:51:51 +0000
From:   "Liu, Yongxin" <Yongxin.Liu@...driver.com>
To:     Bartosz Golaszewski <bgolaszewski@...libre.com>
CC:     "David S . Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] Revert "net: ethernet: ixgbe: check the return value of
 ixgbe_mii_bus_init()"

> -----Original Message-----
> From: Bartosz Golaszewski <bgolaszewski@...libre.com>
> Sent: Friday, September 25, 2020 16:15
> To: Liu, Yongxin <Yongxin.Liu@...driver.com>
> Cc: David S . Miller <davem@...emloft.net>; netdev
> <netdev@...r.kernel.org>; LKML <linux-kernel@...r.kernel.org>
> Subject: Re: [PATCH] Revert "net: ethernet: ixgbe: check the return value
> of ixgbe_mii_bus_init()"
> 
> On Fri, Sep 25, 2020 at 4:45 AM Yongxin Liu <yongxin.liu@...driver.com>
> wrote:
> >
> > This reverts commit 09ef193fef7efb0175a04634853862d717adbb95.
> >
> > For C3000 family of SoCs, they have four ixgbe devices sharing a single
> MDIO bus.
> > ixgbe_mii_bus_init() returns -ENODEV for other three devices. The
> > propagation of the error code makes other three ixgbe devices
> unregistered.
> >
> > Signed-off-by: Yongxin Liu <yongxin.liu@...driver.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index 2f8a4cfc5fa1..5e5223becf86 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -11031,14 +11031,10 @@ static int ixgbe_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
> >                         IXGBE_LINK_SPEED_10GB_FULL |
> IXGBE_LINK_SPEED_1GB_FULL,
> >                         true);
> >
> > -       err = ixgbe_mii_bus_init(hw);
> > -       if (err)
> > -               goto err_netdev;
> > +       ixgbe_mii_bus_init(hw);
> >
> >         return 0;
> >
> > -err_netdev:
> > -       unregister_netdev(netdev);
> >  err_register:
> >         ixgbe_release_hw_control(adapter);
> >         ixgbe_clear_interrupt_scheme(adapter);
> > --
> > 2.14.4
> >
> 
> Then we should check if err == -ENODEV, not outright ignore all potential
> errors, right?
> 

Hm, it is weird to take -ENODEV as a no error.
How about just return 0 instead of -ENODEV in the following function?

<drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c>
 902 s32 ixgbe_mii_bus_init(struct ixgbe_hw *hw)
 903 {
 .
 .
 .
 913   switch (hw->device_id) {
 914   /* C3000 SoCs */
 915   case IXGBE_DEV_ID_X550EM_A_KR:
 916   case IXGBE_DEV_ID_X550EM_A_KR_L:
 917   case IXGBE_DEV_ID_X550EM_A_SFP_N:
 918   case IXGBE_DEV_ID_X550EM_A_SGMII:
 919   case IXGBE_DEV_ID_X550EM_A_SGMII_L:
 920   case IXGBE_DEV_ID_X550EM_A_10G_T:
 921   case IXGBE_DEV_ID_X550EM_A_SFP:
 922   case IXGBE_DEV_ID_X550EM_A_1G_T:
 923   case IXGBE_DEV_ID_X550EM_A_1G_T_L:
 924     if (!ixgbe_x550em_a_has_mii(hw))
 925       return -ENODEV;


Thanks,
Yongxin


> Bartosz