diff mbox series

[RFC] managing PHY carrier from user space

Message ID 2fcf4c20a9adf410db98706f6f2508e46aade2c1.camel@infinera.com
State RFC, archived
Delegated to: David Miller
Headers show
Series [RFC] managing PHY carrier from user space | expand

Commit Message

Joakim Tjernlund Sept. 11, 2018, 4:41 p.m. UTC
I am looking for a way to induce carrier state from user space, primarily
for Fixed PHYs as these are always up. ifplugd/dhcp etc. does not behave properly
if the link is up when it really isn't.

I came up with a new 'phy_carrier' attribute in /sys/class/net/eth0/phydev
where I can induce carrier state:


I would like to know if this acceptable for linux proper or if there is
a better way to do this?

 Jocke

Comments

Florian Fainelli Sept. 11, 2018, 4:56 p.m. UTC | #1
On 09/11/2018 09:41 AM, Joakim Tjernlund wrote:
> I am looking for a way to induce carrier state from user space, primarily
> for Fixed PHYs as these are always up. ifplugd/dhcp etc. does not behave properly
> if the link is up when it really isn't.

Was my suggestion in my email to you somehow not working? This is
obviously not acceptable for upstream, there is no reason, even for a
fixed PHY, to attempt to mangle with the carrier state for any
reasonable production purposes.

> 
> I came up with a new 'phy_carrier' attribute in /sys/class/net/eth0/phydev
> where I can induce carrier state:
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index a1e7ea4d4b16..f82beeabdd75 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -612,10 +612,39 @@ phy_has_fixups_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(phy_has_fixups);
>  
> +static ssize_t
> +phy_carrier_show(struct device *dev, struct device_attribute *attr,
> +                char *buf)
> +{
> +       struct phy_device *phydev = to_phy_device(dev);
> +       struct net_device *netdev = phydev->attached_dev;
> +
> +       return sprintf(buf, "%d\n", netif_carrier_ok(netdev));
> +}
> +
> +static ssize_t phy_carrier_store(struct device *dev, struct device_attribute *attr,
> +                                const char *buf, size_t len)
> +{
> +       struct phy_device *phydev = to_phy_device(dev);
> +       struct net_device *netdev = phydev->attached_dev;
> +       bool enable;
> +
> +       if (strtobool(buf, &enable))
> +               return -EINVAL;
> +
> +       if (enable)
> +               netif_carrier_on(netdev);
> +       else
> +               netif_carrier_off(netdev);
> +       return len;
> +}
> +static DEVICE_ATTR_RW(phy_carrier);
> +
>  static struct attribute *phy_dev_attrs[] = {
>         &dev_attr_phy_id.attr,
>         &dev_attr_phy_interface.attr,
>         &dev_attr_phy_has_fixups.attr,
> +       &dev_attr_phy_carrier.attr,
>         NULL,
>  };
>  ATTRIBUTE_GROUPS(phy_dev);
> 
> I would like to know if this acceptable for linux proper or if there is
> a better way to do this?



> 
>  Jocke
>
Joakim Tjernlund Sept. 11, 2018, 7:21 p.m. UTC | #2
On Tue, 2018-09-11 at 09:56 -0700, Florian Fainelli wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> On 09/11/2018 09:41 AM, Joakim Tjernlund wrote:
> > I am looking for a way to induce carrier state from user space, primarily
> > for Fixed PHYs as these are always up. ifplugd/dhcp etc. does not behave properly
> > if the link is up when it really isn't.
> 
> Was my suggestion in my email to you somehow not working? This is
> obviously not acceptable for upstream, there is no reason, even for a
> fixed PHY, to attempt to mangle with the carrier state for any
> reasonable production purposes.

Ohh, I never got that mail. Scanning the netdev archives I found it though, thanks.
I will go down the ndo_change_carrier() way and see whether I can work out what to
do w.r.t fixed link status callback.

 Thanks
         Jocke
diff mbox series

Patch

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a1e7ea4d4b16..f82beeabdd75 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -612,10 +612,39 @@  phy_has_fixups_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(phy_has_fixups);
 
+static ssize_t
+phy_carrier_show(struct device *dev, struct device_attribute *attr,
+                char *buf)
+{
+       struct phy_device *phydev = to_phy_device(dev);
+       struct net_device *netdev = phydev->attached_dev;
+
+       return sprintf(buf, "%d\n", netif_carrier_ok(netdev));
+}
+
+static ssize_t phy_carrier_store(struct device *dev, struct device_attribute *attr,
+                                const char *buf, size_t len)
+{
+       struct phy_device *phydev = to_phy_device(dev);
+       struct net_device *netdev = phydev->attached_dev;
+       bool enable;
+
+       if (strtobool(buf, &enable))
+               return -EINVAL;
+
+       if (enable)
+               netif_carrier_on(netdev);
+       else
+               netif_carrier_off(netdev);
+       return len;
+}
+static DEVICE_ATTR_RW(phy_carrier);
+
 static struct attribute *phy_dev_attrs[] = {
        &dev_attr_phy_id.attr,
        &dev_attr_phy_interface.attr,
        &dev_attr_phy_has_fixups.attr,
+       &dev_attr_phy_carrier.attr,
        NULL,
 };
 ATTRIBUTE_GROUPS(phy_dev);