Re: [RFC PATCH 1/2] net: dsa: return early if there is no master
I think you can also be more aggressive and remove the checks: if (!master) return -EINVAL; from dsa_port_send and dsa_port_recv. At least it sounds broken to me that this could ever happen. The following comment got me curious: /* * stop master only if it's active, don't probe it otherwise. * Under normal usage it would be active because we're using it, but * during tear-down it may have been removed ahead of us. */ if (master && device_active(master)) eth_get_ops(master)->stop(master); Do we actually care about device removal? I don't think it will work right now. If you do "unbind eth 0" and then using a DSA port you'll get a panic. The check for master doesn't really help here because it will return "priv->master_dev" which is just set in .pre_probe(). Thus in the error case, it will contain a dangling pointer. -michael
Re: [RFC PATCH 1/2] net: dsa: return early if there is no master
Am 2021-02-23 17:32, schrieb Vladimir Oltean:
On Tue, Feb 23, 2021 at 05:19:05PM +0100, Michael Walle wrote:
It doesn't make sense to have DSA without a master port. Error out
early
if there is no master port.
Fixes: fc054d563bfb ("net: Introduce DSA class for Ethernet switches")
Signed-off-by: Michael Walle
---
Reviewed-by: Vladimir Oltean
I think you can also be more aggressive and remove the checks:
if (!master)
return -EINVAL;
from dsa_port_send and dsa_port_recv. At least it sounds broken to me
that this could ever happen.
Yep, I've seen these, too. I'll send a non-RFC version and remove these.
-michael
Re: [RFC PATCH 1/2] net: dsa: return early if there is no master
On Tue, Feb 23, 2021 at 05:19:05PM +0100, Michael Walle wrote:
> It doesn't make sense to have DSA without a master port. Error out early
> if there is no master port.
>
> Fixes: fc054d563bfb ("net: Introduce DSA class for Ethernet switches")
> Signed-off-by: Michael Walle
> ---
Reviewed-by: Vladimir Oltean
I think you can also be more aggressive and remove the checks:
if (!master)
return -EINVAL;
from dsa_port_send and dsa_port_recv. At least it sounds broken to me
that this could ever happen.
[RFC PATCH 1/2] net: dsa: return early if there is no master
It doesn't make sense to have DSA without a master port. Error out early
if there is no master port.
Fixes: fc054d563bfb ("net: Introduce DSA class for Ethernet switches")
Signed-off-by: Michael Walle
---
net/dsa-uclass.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index 2ce9ddb90d..88a8ea9352 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -280,6 +280,10 @@ static int dsa_port_probe(struct udevice *pdev)
if (!port_pdata->phy)
return -ENODEV;
+ master = dsa_get_master(dev);
+ if (!master)
+ return -ENODEV;
+
/*
* Inherit port's hwaddr from the DSA master, unless the port already
* has a unique MAC address specified in the environment.
@@ -288,10 +292,6 @@ static int dsa_port_probe(struct udevice *pdev)
if (!is_zero_ethaddr(env_enetaddr))
return 0;
- master = dsa_get_master(dev);
- if (!master)
- return 0;
-
master_pdata = dev_get_plat(master);
eth_pdata = dev_get_plat(pdev);
memcpy(eth_pdata->enetaddr, master_pdata->enetaddr, ARP_HLEN);
--
2.20.1
