netdev - Re: [PATCH net-next 1/3] net: phy: Create sysfs reciprocal links for attached

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] [day] [month] [year] [list]

Message-ID: <6ccadb96-bdbb-a789-234c-374bf18d5162@gmail.com>
Date:   Fri, 26 May 2017 20:22:16 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Woojung.Huh@...rochip.com, netdev@...r.kernel.org
Cc:     davem@...emloft.net, andrew@...n.ch
Subject: Re: [PATCH net-next 1/3] net: phy: Create sysfs reciprocal links for
 attached_dev/phydev



On 05/26/2017 06:31 PM, Woojung.Huh@...rochip.com wrote:
>>
>> Yes, that's a very valid point, how about we even do this:
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index f84414b8f2ee..dc666ec13651 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -960,15 +960,21 @@ int phy_attach_direct(struct net_device *dev,
>> struct phy_device *phydev,
>>
>>         phydev->attached_dev = dev;
>>         dev->phydev = phydev;
>> +
>> +       /* Some Ethernet drivers try to connect to a PHY device before
>> +        * calling register_netdevice() -> netdev_register_kobject() and
>> +        * does the dev->dev.kobj initialization. Here we only check for
>> +        * success which indicates that the network device kobject is
>> +        * ready.
>> +        */
>>         err = sysfs_create_link(&phydev->mdio.dev.kobj, &dev->dev.kobj,
>>                                 "attached_dev");
>> -       if (err)
>> -               goto error;
>> -
>> -       err = sysfs_create_link(&dev->dev.kobj, &phydev->mdio.dev.kobj,
>> -                               "phydev");
>> -       if (err)
>> -               goto error;
>> +       if (!err) {
>> +               err = sysfs_create_link(&dev->dev.kobj,
>> &phydev->mdio.dev.kobj,
>> +                                       "phydev");
>> +               if (err)
>> +                       goto error;
>> +       }
>>
>>         phydev->dev_flags = flags;
>>
> Looks better and clean.
> 
> How about sysfs_remove_link() in phy_detach()?

Good catch, I was able to reproduce the situation in which macb calls
phy_connect_direct(), will submit a patch shortly. Thanks!
-- 
Florian