Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On 05/18/2018 06:45 AM, Andrew Lunn wrote: >> What benefit does it have to register unused ports? What is a usecase >> for them. Like Florian, I also think they should not be registered. > > Hi Jiri > > They physically exist, so we are accurately describing the hardware by > registering them. You are right that the driver is advertising a number of ports that does not match what is being expected. We unfortunately do not have a good API for specifying e.g: a sparse port allocation. -- Florian
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
> What benefit does it have to register unused ports? What is a usecase > for them. Like Florian, I also think they should not be registered. Hi Jiri They physically exist, so we are accurately describing the hardware by registering them. Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Fri, May 18, 2018 at 12:40:17AM CEST, and...@lunn.ch wrote:
>On Thu, May 17, 2018 at 03:06:36PM -0700, Florian Fainelli wrote:
>> On 05/17/2018 02:08 PM, Andrew Lunn wrote:
>> > On Thu, May 17, 2018 at 10:48:55PM +0200, Jiri Pirko wrote:
>> >> Thu, May 17, 2018 at 09:14:32PM CEST, f.faine...@gmail.com wrote:
>> >>> On 05/17/2018 10:39 AM, Jiri Pirko wrote:
>> >> That is compiled inside "fixed_phy", isn't it?
>> >
>> > It matches what CONFIG_FIXED_PHY is, so if it's built-in it also
>> > becomes
>> > built-in, if is modular, it is also modular, this was fixed with
>> > 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
>> > dsa-loop dependency on FIXED_PHY")
>>
>> Now I have it compiled as module, and after modprobe dsa_loop I see:
>> [ 1168.129202] libphy: Fixed MDIO Bus: probed
>> [ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
>>
>> This messages I did not see when I had fixed_phy compiled as buildin.
>>
>> But I still see no netdevs :/
>> >>>
>> >>> The platform data assumes there is a network device named "eth0" as the
>> >>
>> >> Oups, I missed, I created dummy device and modprobed again. Now I see:
>> >>
>> >> $ sudo devlink port
>> >> mdio_bus/fixed-0:1f/0: type eth netdev lan1
>> >> mdio_bus/fixed-0:1f/1: type eth netdev lan2
>> >> mdio_bus/fixed-0:1f/2: type eth netdev lan3
>> >> mdio_bus/fixed-0:1f/3: type eth netdev lan4
>> >> mdio_bus/fixed-0:1f/4: type notset
>> >> mdio_bus/fixed-0:1f/5: type notset
>> >> mdio_bus/fixed-0:1f/6: type notset
>> >> mdio_bus/fixed-0:1f/7: type notset
>> >> mdio_bus/fixed-0:1f/8: type notset
>> >> mdio_bus/fixed-0:1f/9: type notset
>> >> mdio_bus/fixed-0:1f/10: type notset
>> >> mdio_bus/fixed-0:1f/11: type notset
>> >>
>> >> I wonder why there are ports 4-11
>> >
>> > Hi Jiri
>> >
>> > ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
>> >
>> > It is allocating a switch with 12 ports. However only 4 of them have
>> > names. So the core only creates slave devices for those 4.
>> >
>> > This is a useful test. Real hardware often has unused ports. A WiFi AP
>> > with a 7 port switch which only uses 6 ports is often seen.
>>
>> The following patch should fix this:
>>
>>
>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>> index adf50fbc4c13..a06c29ec91f0 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -262,13 +262,14 @@ static int dsa_port_setup(struct dsa_port *dp)
>>
>> memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
>>
>> + if (dp->type == DSA_PORT_TYPE_UNUSED)
>> + return 0;
>> +
>> err = devlink_port_register(ds->devlink, &dp->devlink_port,
>> dp->index);
>
>Hi Florian, Jiri
>
>Maybe it is better to add a devlink port type unused?
The output with my patchset looks like this now:
salt:~/iproute2$ sudo devlink/devlink port
mdio_bus/fixed-0:1f/0: type eth netdev lan1 flavour physical number 0
mdio_bus/fixed-0:1f/1: type eth netdev lan2 flavour physical number 1
mdio_bus/fixed-0:1f/2: type eth netdev lan3 flavour physical number 2
mdio_bus/fixed-0:1f/3: type eth netdev lan4 flavour physical number 3
mdio_bus/fixed-0:1f/4: type notset
mdio_bus/fixed-0:1f/5: type notset flavour cpu number 5
mdio_bus/fixed-0:1f/6: type notset
mdio_bus/fixed-0:1f/7: type notset
mdio_bus/fixed-0:1f/8: type notset
mdio_bus/fixed-0:1f/9: type notset
mdio_bus/fixed-0:1f/10: type notset
mdio_bus/fixed-0:1f/11: type notset
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Fri, May 18, 2018 at 03:41:37AM CEST, and...@lunn.ch wrote: >> >>> ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS); >> >>> >> >>> It is allocating a switch with 12 ports. However only 4 of them have >> >>> names. So the core only creates slave devices for those 4. >> >>> >> >>> This is a useful test. Real hardware often has unused ports. A WiFi AP >> >>> with a 7 port switch which only uses 6 ports is often seen. >> >> >> >> The following patch should fix this: >> >> >> >> >> >> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c >> >> index adf50fbc4c13..a06c29ec91f0 100644 >> >> --- a/net/dsa/dsa2.c >> >> +++ b/net/dsa/dsa2.c >> >> @@ -262,13 +262,14 @@ static int dsa_port_setup(struct dsa_port *dp) >> >> >> >> memset(&dp->devlink_port, 0, sizeof(dp->devlink_port)); >> >> >> >> + if (dp->type == DSA_PORT_TYPE_UNUSED) >> >> + return 0; >> >> + >> >> err = devlink_port_register(ds->devlink, &dp->devlink_port, >> >> dp->index); >> > >> > Hi Florian, Jiri >> > >> > Maybe it is better to add a devlink port type unused? >> >> The port does not exist on the switch, so it should not even be >> registered IMHO. > >Hi Florian > >The ports do exist, when you called dsa_switch_alloc() you said the >switch has 12 ports. What benefit does it have to register unused ports? What is a usecase for them. Like Florian, I also think they should not be registered. > > Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
> >>> ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS); > >>> > >>> It is allocating a switch with 12 ports. However only 4 of them have > >>> names. So the core only creates slave devices for those 4. > >>> > >>> This is a useful test. Real hardware often has unused ports. A WiFi AP > >>> with a 7 port switch which only uses 6 ports is often seen. > >> > >> The following patch should fix this: > >> > >> > >> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c > >> index adf50fbc4c13..a06c29ec91f0 100644 > >> --- a/net/dsa/dsa2.c > >> +++ b/net/dsa/dsa2.c > >> @@ -262,13 +262,14 @@ static int dsa_port_setup(struct dsa_port *dp) > >> > >> memset(&dp->devlink_port, 0, sizeof(dp->devlink_port)); > >> > >> + if (dp->type == DSA_PORT_TYPE_UNUSED) > >> + return 0; > >> + > >> err = devlink_port_register(ds->devlink, &dp->devlink_port, > >> dp->index); > > > > Hi Florian, Jiri > > > > Maybe it is better to add a devlink port type unused? > > The port does not exist on the switch, so it should not even be > registered IMHO. Hi Florian The ports do exist, when you called dsa_switch_alloc() you said the switch has 12 ports. Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On 05/17/2018 03:40 PM, Andrew Lunn wrote:
> On Thu, May 17, 2018 at 03:06:36PM -0700, Florian Fainelli wrote:
>> On 05/17/2018 02:08 PM, Andrew Lunn wrote:
>>> On Thu, May 17, 2018 at 10:48:55PM +0200, Jiri Pirko wrote:
Thu, May 17, 2018 at 09:14:32PM CEST, f.faine...@gmail.com wrote:
> On 05/17/2018 10:39 AM, Jiri Pirko wrote:
That is compiled inside "fixed_phy", isn't it?
>>>
>>> It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
>>> built-in, if is modular, it is also modular, this was fixed with
>>> 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
>>> dsa-loop dependency on FIXED_PHY")
>>
>> Now I have it compiled as module, and after modprobe dsa_loop I see:
>> [ 1168.129202] libphy: Fixed MDIO Bus: probed
>> [ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
>>
>> This messages I did not see when I had fixed_phy compiled as buildin.
>>
>> But I still see no netdevs :/
>
> The platform data assumes there is a network device named "eth0" as the
Oups, I missed, I created dummy device and modprobed again. Now I see:
$ sudo devlink port
mdio_bus/fixed-0:1f/0: type eth netdev lan1
mdio_bus/fixed-0:1f/1: type eth netdev lan2
mdio_bus/fixed-0:1f/2: type eth netdev lan3
mdio_bus/fixed-0:1f/3: type eth netdev lan4
mdio_bus/fixed-0:1f/4: type notset
mdio_bus/fixed-0:1f/5: type notset
mdio_bus/fixed-0:1f/6: type notset
mdio_bus/fixed-0:1f/7: type notset
mdio_bus/fixed-0:1f/8: type notset
mdio_bus/fixed-0:1f/9: type notset
mdio_bus/fixed-0:1f/10: type notset
mdio_bus/fixed-0:1f/11: type notset
I wonder why there are ports 4-11
>>>
>>> Hi Jiri
>>>
>>> ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
>>>
>>> It is allocating a switch with 12 ports. However only 4 of them have
>>> names. So the core only creates slave devices for those 4.
>>>
>>> This is a useful test. Real hardware often has unused ports. A WiFi AP
>>> with a 7 port switch which only uses 6 ports is often seen.
>>
>> The following patch should fix this:
>>
>>
>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>> index adf50fbc4c13..a06c29ec91f0 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -262,13 +262,14 @@ static int dsa_port_setup(struct dsa_port *dp)
>>
>> memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
>>
>> + if (dp->type == DSA_PORT_TYPE_UNUSED)
>> + return 0;
>> +
>> err = devlink_port_register(ds->devlink, &dp->devlink_port,
>> dp->index);
>
> Hi Florian, Jiri
>
> Maybe it is better to add a devlink port type unused?
The port does not exist on the switch, so it should not even be
registered IMHO.
--
Florian
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On Thu, May 17, 2018 at 03:06:36PM -0700, Florian Fainelli wrote:
> On 05/17/2018 02:08 PM, Andrew Lunn wrote:
> > On Thu, May 17, 2018 at 10:48:55PM +0200, Jiri Pirko wrote:
> >> Thu, May 17, 2018 at 09:14:32PM CEST, f.faine...@gmail.com wrote:
> >>> On 05/17/2018 10:39 AM, Jiri Pirko wrote:
> >> That is compiled inside "fixed_phy", isn't it?
> >
> > It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
> > built-in, if is modular, it is also modular, this was fixed with
> > 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
> > dsa-loop dependency on FIXED_PHY")
>
> Now I have it compiled as module, and after modprobe dsa_loop I see:
> [ 1168.129202] libphy: Fixed MDIO Bus: probed
> [ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
>
> This messages I did not see when I had fixed_phy compiled as buildin.
>
> But I still see no netdevs :/
> >>>
> >>> The platform data assumes there is a network device named "eth0" as the
> >>
> >> Oups, I missed, I created dummy device and modprobed again. Now I see:
> >>
> >> $ sudo devlink port
> >> mdio_bus/fixed-0:1f/0: type eth netdev lan1
> >> mdio_bus/fixed-0:1f/1: type eth netdev lan2
> >> mdio_bus/fixed-0:1f/2: type eth netdev lan3
> >> mdio_bus/fixed-0:1f/3: type eth netdev lan4
> >> mdio_bus/fixed-0:1f/4: type notset
> >> mdio_bus/fixed-0:1f/5: type notset
> >> mdio_bus/fixed-0:1f/6: type notset
> >> mdio_bus/fixed-0:1f/7: type notset
> >> mdio_bus/fixed-0:1f/8: type notset
> >> mdio_bus/fixed-0:1f/9: type notset
> >> mdio_bus/fixed-0:1f/10: type notset
> >> mdio_bus/fixed-0:1f/11: type notset
> >>
> >> I wonder why there are ports 4-11
> >
> > Hi Jiri
> >
> > ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
> >
> > It is allocating a switch with 12 ports. However only 4 of them have
> > names. So the core only creates slave devices for those 4.
> >
> > This is a useful test. Real hardware often has unused ports. A WiFi AP
> > with a 7 port switch which only uses 6 ports is often seen.
>
> The following patch should fix this:
>
>
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index adf50fbc4c13..a06c29ec91f0 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -262,13 +262,14 @@ static int dsa_port_setup(struct dsa_port *dp)
>
> memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
>
> + if (dp->type == DSA_PORT_TYPE_UNUSED)
> + return 0;
> +
> err = devlink_port_register(ds->devlink, &dp->devlink_port,
> dp->index);
Hi Florian, Jiri
Maybe it is better to add a devlink port type unused?
Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On 05/17/2018 02:08 PM, Andrew Lunn wrote:
> On Thu, May 17, 2018 at 10:48:55PM +0200, Jiri Pirko wrote:
>> Thu, May 17, 2018 at 09:14:32PM CEST, f.faine...@gmail.com wrote:
>>> On 05/17/2018 10:39 AM, Jiri Pirko wrote:
>> That is compiled inside "fixed_phy", isn't it?
>
> It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
> built-in, if is modular, it is also modular, this was fixed with
> 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
> dsa-loop dependency on FIXED_PHY")
Now I have it compiled as module, and after modprobe dsa_loop I see:
[ 1168.129202] libphy: Fixed MDIO Bus: probed
[ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
This messages I did not see when I had fixed_phy compiled as buildin.
But I still see no netdevs :/
>>>
>>> The platform data assumes there is a network device named "eth0" as the
>>
>> Oups, I missed, I created dummy device and modprobed again. Now I see:
>>
>> $ sudo devlink port
>> mdio_bus/fixed-0:1f/0: type eth netdev lan1
>> mdio_bus/fixed-0:1f/1: type eth netdev lan2
>> mdio_bus/fixed-0:1f/2: type eth netdev lan3
>> mdio_bus/fixed-0:1f/3: type eth netdev lan4
>> mdio_bus/fixed-0:1f/4: type notset
>> mdio_bus/fixed-0:1f/5: type notset
>> mdio_bus/fixed-0:1f/6: type notset
>> mdio_bus/fixed-0:1f/7: type notset
>> mdio_bus/fixed-0:1f/8: type notset
>> mdio_bus/fixed-0:1f/9: type notset
>> mdio_bus/fixed-0:1f/10: type notset
>> mdio_bus/fixed-0:1f/11: type notset
>>
>> I wonder why there are ports 4-11
>
> Hi Jiri
>
> ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
>
> It is allocating a switch with 12 ports. However only 4 of them have
> names. So the core only creates slave devices for those 4.
>
> This is a useful test. Real hardware often has unused ports. A WiFi AP
> with a 7 port switch which only uses 6 ports is often seen.
The following patch should fix this:
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index adf50fbc4c13..a06c29ec91f0 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -262,13 +262,14 @@ static int dsa_port_setup(struct dsa_port *dp)
memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
+ if (dp->type == DSA_PORT_TYPE_UNUSED)
+ return 0;
+
err = devlink_port_register(ds->devlink, &dp->devlink_port,
dp->index);
if (err)
return err;
switch (dp->type) {
- case DSA_PORT_TYPE_UNUSED:
- break;
case DSA_PORT_TYPE_CPU:
case DSA_PORT_TYPE_DSA:
err = dsa_port_link_register_of(dp);
@@ -286,6 +287,8 @@ static int dsa_port_setup(struct dsa_port *dp)
else
devlink_port_type_eth_set(&dp->devlink_port,
dp->slave);
break;
+ default:
+ break;
}
return 0;
@@ -293,11 +296,12 @@ static int dsa_port_setup(struct dsa_port *dp)
static void dsa_port_teardown(struct dsa_port *dp)
{
+ if (dp->type == DSA_PORT_TYPE_UNUSED)
+ return;
+
devlink_port_unregister(&dp->devlink_port);
switch (dp->type) {
- case DSA_PORT_TYPE_UNUSED:
- break;
case DSA_PORT_TYPE_CPU:
case DSA_PORT_TYPE_DSA:
dsa_port_link_unregister_of(dp);
@@ -308,6 +312,8 @@ static void dsa_port_teardown(struct dsa_port *dp)
dp->slave = NULL;
}
break;
+ default:
+ break;
}
}
--
Florian
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On Thu, May 17, 2018 at 10:48:55PM +0200, Jiri Pirko wrote:
> Thu, May 17, 2018 at 09:14:32PM CEST, f.faine...@gmail.com wrote:
> >On 05/17/2018 10:39 AM, Jiri Pirko wrote:
> That is compiled inside "fixed_phy", isn't it?
> >>>
> >>> It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
> >>> built-in, if is modular, it is also modular, this was fixed with
> >>> 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
> >>> dsa-loop dependency on FIXED_PHY")
> >>
> >> Now I have it compiled as module, and after modprobe dsa_loop I see:
> >> [ 1168.129202] libphy: Fixed MDIO Bus: probed
> >> [ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
> >>
> >> This messages I did not see when I had fixed_phy compiled as buildin.
> >>
> >> But I still see no netdevs :/
> >
> >The platform data assumes there is a network device named "eth0" as the
>
> Oups, I missed, I created dummy device and modprobed again. Now I see:
>
> $ sudo devlink port
> mdio_bus/fixed-0:1f/0: type eth netdev lan1
> mdio_bus/fixed-0:1f/1: type eth netdev lan2
> mdio_bus/fixed-0:1f/2: type eth netdev lan3
> mdio_bus/fixed-0:1f/3: type eth netdev lan4
> mdio_bus/fixed-0:1f/4: type notset
> mdio_bus/fixed-0:1f/5: type notset
> mdio_bus/fixed-0:1f/6: type notset
> mdio_bus/fixed-0:1f/7: type notset
> mdio_bus/fixed-0:1f/8: type notset
> mdio_bus/fixed-0:1f/9: type notset
> mdio_bus/fixed-0:1f/10: type notset
> mdio_bus/fixed-0:1f/11: type notset
>
> I wonder why there are ports 4-11
Hi Jiri
ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
It is allocating a switch with 12 ports. However only 4 of them have
names. So the core only creates slave devices for those 4.
This is a useful test. Real hardware often has unused ports. A WiFi AP
with a 7 port switch which only uses 6 ports is often seen.
Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Thu, May 17, 2018 at 09:14:32PM CEST, f.faine...@gmail.com wrote:
>On 05/17/2018 10:39 AM, Jiri Pirko wrote:
That is compiled inside "fixed_phy", isn't it?
>>>
>>> It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
>>> built-in, if is modular, it is also modular, this was fixed with
>>> 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
>>> dsa-loop dependency on FIXED_PHY")
>>
>> Now I have it compiled as module, and after modprobe dsa_loop I see:
>> [ 1168.129202] libphy: Fixed MDIO Bus: probed
>> [ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
>>
>> This messages I did not see when I had fixed_phy compiled as buildin.
>>
>> But I still see no netdevs :/
>
>The platform data assumes there is a network device named "eth0" as the
Oups, I missed, I created dummy device and modprobed again. Now I see:
$ sudo devlink port
mdio_bus/fixed-0:1f/0: type eth netdev lan1
mdio_bus/fixed-0:1f/1: type eth netdev lan2
mdio_bus/fixed-0:1f/2: type eth netdev lan3
mdio_bus/fixed-0:1f/3: type eth netdev lan4
mdio_bus/fixed-0:1f/4: type notset
mdio_bus/fixed-0:1f/5: type notset
mdio_bus/fixed-0:1f/6: type notset
mdio_bus/fixed-0:1f/7: type notset
mdio_bus/fixed-0:1f/8: type notset
mdio_bus/fixed-0:1f/9: type notset
mdio_bus/fixed-0:1f/10: type notset
mdio_bus/fixed-0:1f/11: type notset
I wonder why there are ports 4-11
>parent device, yes I know this is terrible, but unfortunately we don't
>have anything better at this point, though that could certainly change
>that to take a proper struct device reference in the future.
>
>I am assuming that you don't have such a network device named "eth0" in
>your system? You can also look at the less than 360 LOCs of the driver
>and find out where your problem is, this is not mlxsw :)
Yeah, I'm a bit lost in the embedded/dsa world :)
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
> The platform data assumes there is a network device named "eth0" as the > parent device, yes I know this is terrible, but unfortunately we don't > have anything better at this point That is something we need to solve. With DT, it is easy, we have a phandle to a device, and the name does not matter. systemd can rename eth0 to enp0s25 and we will still find it, because we don't look for the name, we look for the OF node associated to the device. For platform data driven devices, we don't have this luxury. I have an intel board with a Marvell switch. I have Debian running on it, so systemd renames the interface. Builtin vs modules then becomes an issue. Does the switch driver probe before or after systemd renames the interface? For embedded systems like this, we probably need a way to find an interface based on its bus location, not its name. Not as good, but maybe sufficient, a function which gives us the first physical interface the machine has. Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On 05/17/2018 10:39 AM, Jiri Pirko wrote:
>>> That is compiled inside "fixed_phy", isn't it?
>>
>> It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
>> built-in, if is modular, it is also modular, this was fixed with
>> 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
>> dsa-loop dependency on FIXED_PHY")
>
> Now I have it compiled as module, and after modprobe dsa_loop I see:
> [ 1168.129202] libphy: Fixed MDIO Bus: probed
> [ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
>
> This messages I did not see when I had fixed_phy compiled as buildin.
>
> But I still see no netdevs :/
The platform data assumes there is a network device named "eth0" as the
parent device, yes I know this is terrible, but unfortunately we don't
have anything better at this point, though that could certainly change
that to take a proper struct device reference in the future.
I am assuming that you don't have such a network device named "eth0" in
your system? You can also look at the less than 360 LOCs of the driver
and find out where your problem is, this is not mlxsw :)
--
Florian
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Thu, May 17, 2018 at 04:51:17PM CEST, f.faine...@gmail.com wrote:
>
>
>On 05/17/2018 07:30 AM, Jiri Pirko wrote:
>> Thu, May 17, 2018 at 04:08:10PM CEST, f.faine...@gmail.com wrote:
>>>
>>>
>>> On 05/17/2018 07:02 AM, Jiri Pirko wrote:
Fri, Mar 23, 2018 at 06:09:29PM CET, f.faine...@gmail.com wrote:
> On 03/23/2018 07:49 AM, Jiri Pirko wrote:
>> Fri, Mar 23, 2018 at 02:30:02PM CET, and...@lunn.ch wrote:
>>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
From: Jiri Pirko
Set the attrs and allow to expose port flavour to user via devlink.
Signed-off-by: Jiri Pirko
---
net/dsa/dsa2.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index adf50fbc4c13..49453690696d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
case DSA_PORT_TYPE_UNUSED:
break;
case DSA_PORT_TYPE_CPU:
+ /* dp->index is used now as port_number. However
+ * CPU ports should have separate numbering
+ * independent from front panel port numbers.
+ */
+ devlink_port_attrs_set(&dp->devlink_port,
+ DEVLINK_PORT_FLAVOUR_CPU,
+ dp->index, false, 0);
+ err = dsa_port_link_register_of(dp);
+ if (err) {
+ dev_err(ds->dev, "failed to setup link for port
%d.%d\n",
+ ds->index, dp->index);
+ return err;
+ }
>>>
>>> Ah, i get it. These used to be two case statements with one code
>>> block. But you split them apart, so needed to duplicate the
>>> dsa_port_link_register.
>>>
>>> Unfortunately, you forgot to add a 'break;', so it still falls
>>> through, and overwrites the port flavour to DSA.
>>
>> ah, crap. Don't have hw to test this :/
>> Will fix. Thanks!
>
> You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
> emulate a DSA switch. It won't create interconnect ports, since only one
Hmm, trying to use dsa_loop. Doing:
modprobe dsa_loop
modprobe fixed_phy
I don't see the netdevs. Any idea what am I doing wrong? Thanks!
>>>
>>> Yes, modprobe dsa-loop-bdinfo first, which will create the
>>
>> That is compiled inside "fixed_phy", isn't it?
>
>It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
>built-in, if is modular, it is also modular, this was fixed with
>40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
>dsa-loop dependency on FIXED_PHY")
Now I have it compiled as module, and after modprobe dsa_loop I see:
[ 1168.129202] libphy: Fixed MDIO Bus: probed
[ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
This messages I did not see when I had fixed_phy compiled as buildin.
But I still see no netdevs :/
>
>> In my case, "Module fixed_phy is builtin"
>> So it should be enough just to "modprobe dsa_loop", right? That does not
>> work :/
>
>There is a caveat that if you have everything modular, if you loaded
>fixed_phy first, and forgot to load dsa-loop-bdinfo, then nothing will
>happen because the MDIO bus was created and there were no matching devices.
>
>Send me your .config and I will take a look. Attached is mine (64Bit x86
>VM), start with:
>--
>Florian
>#
># Automatically generated file; DO NOT EDIT.
># Linux/x86 4.15.0-rc2 Kernel Configuration
>#
>CONFIG_64BIT=y
>CONFIG_X86_64=y
>CONFIG_X86=y
>CONFIG_INSTRUCTION_DECODER=y
>CONFIG_OUTPUT_FORMAT="elf64-x86-64"
>CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
>CONFIG_LOCKDEP_SUPPORT=y
>CONFIG_STACKTRACE_SUPPORT=y
>CONFIG_MMU=y
>CONFIG_ARCH_MMAP_RND_BITS_MIN=28
>CONFIG_ARCH_MMAP_RND_BITS_MAX=32
>CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
>CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
>CONFIG_NEED_DMA_MAP_STATE=y
>CONFIG_NEED_SG_DMA_LENGTH=y
>CONFIG_GENERIC_ISA_DMA=y
>CONFIG_GENERIC_BUG=y
>CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
>CONFIG_GENERIC_HWEIGHT=y
>CONFIG_ARCH_MAY_HAVE_PC_FDC=y
>CONFIG_RWSEM_XCHGADD_ALGORITHM=y
>CONFIG_GENERIC_CALIBRATE_DELAY=y
>CONFIG_ARCH_HAS_CPU_RELAX=y
>CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
>CONFIG_HAVE_SETUP_PER_CPU_AREA=y
>CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
>CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
>CONFIG_ARCH_HIBERNATION_POSSIBLE=y
>CONFIG_ARCH_SUSPEND_POSSIBLE=y
>CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
>CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
>CONFIG_ZONE_DMA32=y
>CONFIG_AUDIT_ARCH=y
>CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
>CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
>CONFIG_X86_64_SMP=y
>CONFIG_ARCH_SUPPORTS_UPROBES=y
>CONFIG_FI
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Thu, May 17, 2018 at 04:48:28PM CEST, and...@lunn.ch wrote: >> >Yes, modprobe dsa-loop-bdinfo first, which will create the >> >> That is compiled inside "fixed_phy", isn't it? > >Nope. > >It follows a pattern seen with I2C and SPI subsystem. A bus driver >provides a bus to Linux. But i2c and SPI, unlike PCI or USB, you >cannot enumerate the devices on the bus, you need to know what devices >are there. So the board file registers an info structure, listing what >devices are on the bus. When the bus pops into existence, the core >links the bus to the info structure about devices on the bus and then >probes the devices. > >The same is happening here. The fixed_phy driver provides an MDIO bus. > >The info structure in dsa-loop-bdinfo says there is an dsa-loop device >at address 31 on that bus. > >Combine the two causes the dsa-loop device to actually probe. I understand. Yet I have no dsa_loop_bdinfo.ko module. In my .config I have: CONFIG_FIXED_PHY=y CONFIG_NET_DSA_LOOP=m I had to do this: diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile index 15c2a831edf1..2d773d3a7d49 100644 --- a/drivers/net/dsa/Makefile +++ b/drivers/net/dsa/Makefile @@ -2,9 +2,7 @@ obj-$(CONFIG_NET_DSA_BCM_SF2) += bcm-sf2.o bcm-sf2-objs := bcm_sf2.o bcm_sf2_cfp.o obj-$(CONFIG_NET_DSA_LOOP) += dsa_loop.o -ifdef CONFIG_NET_DSA_LOOP -obj-$(CONFIG_FIXED_PHY)+= dsa_loop_bdinfo.o -endif +obj-$(CONFIG_NET_DSA_LOOP) += dsa_loop_bdinfo.o obj-$(CONFIG_NET_DSA_MT7530) += mt7530.o obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o obj-$(CONFIG_NET_DSA_QCA8K)+= qca8k.o Now dsa_loop_bdinfo.ko gets compiled. I have no clue why it does not work without the patch :O "obj-$(CONFIG_FIXED_PHY)" doesn't work.
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On 05/17/2018 07:30 AM, Jiri Pirko wrote:
> Thu, May 17, 2018 at 04:08:10PM CEST, f.faine...@gmail.com wrote:
>>
>>
>> On 05/17/2018 07:02 AM, Jiri Pirko wrote:
>>> Fri, Mar 23, 2018 at 06:09:29PM CET, f.faine...@gmail.com wrote:
On 03/23/2018 07:49 AM, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 02:30:02PM CET, and...@lunn.ch wrote:
>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
>>> From: Jiri Pirko
>>>
>>> Set the attrs and allow to expose port flavour to user via devlink.
>>>
>>> Signed-off-by: Jiri Pirko
>>> ---
>>> net/dsa/dsa2.c | 23 +++
>>> 1 file changed, 23 insertions(+)
>>>
>>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>>> index adf50fbc4c13..49453690696d 100644
>>> --- a/net/dsa/dsa2.c
>>> +++ b/net/dsa/dsa2.c
>>> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
>>> case DSA_PORT_TYPE_UNUSED:
>>> break;
>>> case DSA_PORT_TYPE_CPU:
>>> + /* dp->index is used now as port_number. However
>>> +* CPU ports should have separate numbering
>>> +* independent from front panel port numbers.
>>> +*/
>>> + devlink_port_attrs_set(&dp->devlink_port,
>>> + DEVLINK_PORT_FLAVOUR_CPU,
>>> + dp->index, false, 0);
>>> + err = dsa_port_link_register_of(dp);
>>> + if (err) {
>>> + dev_err(ds->dev, "failed to setup link for port
>>> %d.%d\n",
>>> + ds->index, dp->index);
>>> + return err;
>>> + }
>>
>> Ah, i get it. These used to be two case statements with one code
>> block. But you split them apart, so needed to duplicate the
>> dsa_port_link_register.
>>
>> Unfortunately, you forgot to add a 'break;', so it still falls
>> through, and overwrites the port flavour to DSA.
>
> ah, crap. Don't have hw to test this :/
> Will fix. Thanks!
You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
emulate a DSA switch. It won't create interconnect ports, since only one
>>>
>>> Hmm, trying to use dsa_loop. Doing:
>>> modprobe dsa_loop
>>> modprobe fixed_phy
>>>
>>> I don't see the netdevs. Any idea what am I doing wrong? Thanks!
>>
>> Yes, modprobe dsa-loop-bdinfo first, which will create the
>
> That is compiled inside "fixed_phy", isn't it?
It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
built-in, if is modular, it is also modular, this was fixed with
40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
dsa-loop dependency on FIXED_PHY")
> In my case, "Module fixed_phy is builtin"
> So it should be enough just to "modprobe dsa_loop", right? That does not
> work :/
There is a caveat that if you have everything modular, if you loaded
fixed_phy first, and forgot to load dsa-loop-bdinfo, then nothing will
happen because the MDIO bus was created and there were no matching devices.
Send me your .config and I will take a look. Attached is mine (64Bit x86
VM), start with:
--
Florian
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.15.0-rc2 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=28
CONFIG_ARCH_MMAP_RND_BITS_MAX=32
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_X86_64_SMP=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_THREAD_INFO_IN_TASK=y
#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERN
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
> >Yes, modprobe dsa-loop-bdinfo first, which will create the > > That is compiled inside "fixed_phy", isn't it? Nope. It follows a pattern seen with I2C and SPI subsystem. A bus driver provides a bus to Linux. But i2c and SPI, unlike PCI or USB, you cannot enumerate the devices on the bus, you need to know what devices are there. So the board file registers an info structure, listing what devices are on the bus. When the bus pops into existence, the core links the bus to the info structure about devices on the bus and then probes the devices. The same is happening here. The fixed_phy driver provides an MDIO bus. The info structure in dsa-loop-bdinfo says there is an dsa-loop device at address 31 on that bus. Combine the two causes the dsa-loop device to actually probe. Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Thu, May 17, 2018 at 04:08:10PM CEST, f.faine...@gmail.com wrote:
>
>
>On 05/17/2018 07:02 AM, Jiri Pirko wrote:
>> Fri, Mar 23, 2018 at 06:09:29PM CET, f.faine...@gmail.com wrote:
>>> On 03/23/2018 07:49 AM, Jiri Pirko wrote:
Fri, Mar 23, 2018 at 02:30:02PM CET, and...@lunn.ch wrote:
> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko
>>
>> Set the attrs and allow to expose port flavour to user via devlink.
>>
>> Signed-off-by: Jiri Pirko
>> ---
>> net/dsa/dsa2.c | 23 +++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>> index adf50fbc4c13..49453690696d 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
>> case DSA_PORT_TYPE_UNUSED:
>> break;
>> case DSA_PORT_TYPE_CPU:
>> +/* dp->index is used now as port_number. However
>> + * CPU ports should have separate numbering
>> + * independent from front panel port numbers.
>> + */
>> +devlink_port_attrs_set(&dp->devlink_port,
>> + DEVLINK_PORT_FLAVOUR_CPU,
>> + dp->index, false, 0);
>> +err = dsa_port_link_register_of(dp);
>> +if (err) {
>> +dev_err(ds->dev, "failed to setup link for port
>> %d.%d\n",
>> +ds->index, dp->index);
>> +return err;
>> +}
>
> Ah, i get it. These used to be two case statements with one code
> block. But you split them apart, so needed to duplicate the
> dsa_port_link_register.
>
> Unfortunately, you forgot to add a 'break;', so it still falls
> through, and overwrites the port flavour to DSA.
ah, crap. Don't have hw to test this :/
Will fix. Thanks!
>>>
>>> You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
>>> emulate a DSA switch. It won't create interconnect ports, since only one
>>
>> Hmm, trying to use dsa_loop. Doing:
>> modprobe dsa_loop
>> modprobe fixed_phy
>>
>> I don't see the netdevs. Any idea what am I doing wrong? Thanks!
>
>Yes, modprobe dsa-loop-bdinfo first, which will create the
That is compiled inside "fixed_phy", isn't it?
In my case, "Module fixed_phy is builtin"
So it should be enough just to "modprobe dsa_loop", right? That does not
work :/
>mdio_board_info and then modprobe dsa-loop.
>
>>
>>
>>> switch can be created with the method chosen, but this would have helped
>>> you catch the missing break since the "CPU" port would have been
>>> displayed as "DSA" anyway.
>>>
>>> If you need hardware, I am sure this can be somehow arranged. By that, I
>>> mean something on which you can run upstream Linux on without out of
>>> tree patches.
>>> --
>>> Florian
>
>--
>Florian
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On 05/17/2018 07:02 AM, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 06:09:29PM CET, f.faine...@gmail.com wrote:
>> On 03/23/2018 07:49 AM, Jiri Pirko wrote:
>>> Fri, Mar 23, 2018 at 02:30:02PM CET, and...@lunn.ch wrote:
On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
> From: Jiri Pirko
>
> Set the attrs and allow to expose port flavour to user via devlink.
>
> Signed-off-by: Jiri Pirko
> ---
> net/dsa/dsa2.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index adf50fbc4c13..49453690696d 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
> case DSA_PORT_TYPE_UNUSED:
> break;
> case DSA_PORT_TYPE_CPU:
> + /* dp->index is used now as port_number. However
> + * CPU ports should have separate numbering
> + * independent from front panel port numbers.
> + */
> + devlink_port_attrs_set(&dp->devlink_port,
> +DEVLINK_PORT_FLAVOUR_CPU,
> +dp->index, false, 0);
> + err = dsa_port_link_register_of(dp);
> + if (err) {
> + dev_err(ds->dev, "failed to setup link for port
> %d.%d\n",
> + ds->index, dp->index);
> + return err;
> + }
Ah, i get it. These used to be two case statements with one code
block. But you split them apart, so needed to duplicate the
dsa_port_link_register.
Unfortunately, you forgot to add a 'break;', so it still falls
through, and overwrites the port flavour to DSA.
>>>
>>> ah, crap. Don't have hw to test this :/
>>> Will fix. Thanks!
>>
>> You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
>> emulate a DSA switch. It won't create interconnect ports, since only one
>
> Hmm, trying to use dsa_loop. Doing:
> modprobe dsa_loop
> modprobe fixed_phy
>
> I don't see the netdevs. Any idea what am I doing wrong? Thanks!
Yes, modprobe dsa-loop-bdinfo first, which will create the
mdio_board_info and then modprobe dsa-loop.
>
>
>> switch can be created with the method chosen, but this would have helped
>> you catch the missing break since the "CPU" port would have been
>> displayed as "DSA" anyway.
>>
>> If you need hardware, I am sure this can be somehow arranged. By that, I
>> mean something on which you can run upstream Linux on without out of
>> tree patches.
>> --
>> Florian
--
Florian
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Fri, Mar 23, 2018 at 06:09:29PM CET, f.faine...@gmail.com wrote:
>On 03/23/2018 07:49 AM, Jiri Pirko wrote:
>> Fri, Mar 23, 2018 at 02:30:02PM CET, and...@lunn.ch wrote:
>>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
From: Jiri Pirko
Set the attrs and allow to expose port flavour to user via devlink.
Signed-off-by: Jiri Pirko
---
net/dsa/dsa2.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index adf50fbc4c13..49453690696d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
case DSA_PORT_TYPE_UNUSED:
break;
case DSA_PORT_TYPE_CPU:
+ /* dp->index is used now as port_number. However
+ * CPU ports should have separate numbering
+ * independent from front panel port numbers.
+ */
+ devlink_port_attrs_set(&dp->devlink_port,
+ DEVLINK_PORT_FLAVOUR_CPU,
+ dp->index, false, 0);
+ err = dsa_port_link_register_of(dp);
+ if (err) {
+ dev_err(ds->dev, "failed to setup link for port
%d.%d\n",
+ ds->index, dp->index);
+ return err;
+ }
>>>
>>> Ah, i get it. These used to be two case statements with one code
>>> block. But you split them apart, so needed to duplicate the
>>> dsa_port_link_register.
>>>
>>> Unfortunately, you forgot to add a 'break;', so it still falls
>>> through, and overwrites the port flavour to DSA.
>>
>> ah, crap. Don't have hw to test this :/
>> Will fix. Thanks!
>
>You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
>emulate a DSA switch. It won't create interconnect ports, since only one
Hmm, trying to use dsa_loop. Doing:
modprobe dsa_loop
modprobe fixed_phy
I don't see the netdevs. Any idea what am I doing wrong? Thanks!
>switch can be created with the method chosen, but this would have helped
>you catch the missing break since the "CPU" port would have been
>displayed as "DSA" anyway.
>
>If you need hardware, I am sure this can be somehow arranged. By that, I
>mean something on which you can run upstream Linux on without out of
>tree patches.
>--
>Florian
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Fri, Mar 23, 2018 at 06:09:29PM CET, f.faine...@gmail.com wrote:
>On 03/23/2018 07:49 AM, Jiri Pirko wrote:
>> Fri, Mar 23, 2018 at 02:30:02PM CET, and...@lunn.ch wrote:
>>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
From: Jiri Pirko
Set the attrs and allow to expose port flavour to user via devlink.
Signed-off-by: Jiri Pirko
---
net/dsa/dsa2.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index adf50fbc4c13..49453690696d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
case DSA_PORT_TYPE_UNUSED:
break;
case DSA_PORT_TYPE_CPU:
+ /* dp->index is used now as port_number. However
+ * CPU ports should have separate numbering
+ * independent from front panel port numbers.
+ */
+ devlink_port_attrs_set(&dp->devlink_port,
+ DEVLINK_PORT_FLAVOUR_CPU,
+ dp->index, false, 0);
+ err = dsa_port_link_register_of(dp);
+ if (err) {
+ dev_err(ds->dev, "failed to setup link for port
%d.%d\n",
+ ds->index, dp->index);
+ return err;
+ }
>>>
>>> Ah, i get it. These used to be two case statements with one code
>>> block. But you split them apart, so needed to duplicate the
>>> dsa_port_link_register.
>>>
>>> Unfortunately, you forgot to add a 'break;', so it still falls
>>> through, and overwrites the port flavour to DSA.
>>
>> ah, crap. Don't have hw to test this :/
>> Will fix. Thanks!
>
>You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
>emulate a DSA switch. It won't create interconnect ports, since only one
>switch can be created with the method chosen, but this would have helped
>you catch the missing break since the "CPU" port would have been
>displayed as "DSA" anyway.
Okay.
>
>If you need hardware, I am sure this can be somehow arranged. By that, I
>mean something on which you can run upstream Linux on without out of
>tree patches.
Andrew is already taking care of that. Thanks.
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On 03/23/2018 07:49 AM, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 02:30:02PM CET, and...@lunn.ch wrote:
>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
>>> From: Jiri Pirko
>>>
>>> Set the attrs and allow to expose port flavour to user via devlink.
>>>
>>> Signed-off-by: Jiri Pirko
>>> ---
>>> net/dsa/dsa2.c | 23 +++
>>> 1 file changed, 23 insertions(+)
>>>
>>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>>> index adf50fbc4c13..49453690696d 100644
>>> --- a/net/dsa/dsa2.c
>>> +++ b/net/dsa/dsa2.c
>>> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
>>> case DSA_PORT_TYPE_UNUSED:
>>> break;
>>> case DSA_PORT_TYPE_CPU:
>>> + /* dp->index is used now as port_number. However
>>> +* CPU ports should have separate numbering
>>> +* independent from front panel port numbers.
>>> +*/
>>> + devlink_port_attrs_set(&dp->devlink_port,
>>> + DEVLINK_PORT_FLAVOUR_CPU,
>>> + dp->index, false, 0);
>>> + err = dsa_port_link_register_of(dp);
>>> + if (err) {
>>> + dev_err(ds->dev, "failed to setup link for port
>>> %d.%d\n",
>>> + ds->index, dp->index);
>>> + return err;
>>> + }
>>
>> Ah, i get it. These used to be two case statements with one code
>> block. But you split them apart, so needed to duplicate the
>> dsa_port_link_register.
>>
>> Unfortunately, you forgot to add a 'break;', so it still falls
>> through, and overwrites the port flavour to DSA.
>
> ah, crap. Don't have hw to test this :/
> Will fix. Thanks!
You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
emulate a DSA switch. It won't create interconnect ports, since only one
switch can be created with the method chosen, but this would have helped
you catch the missing break since the "CPU" port would have been
displayed as "DSA" anyway.
If you need hardware, I am sure this can be somehow arranged. By that, I
mean something on which you can run upstream Linux on without out of
tree patches.
--
Florian
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
Fri, Mar 23, 2018 at 02:30:02PM CET, and...@lunn.ch wrote:
>On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko
>>
>> Set the attrs and allow to expose port flavour to user via devlink.
>>
>> Signed-off-by: Jiri Pirko
>> ---
>> net/dsa/dsa2.c | 23 +++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>> index adf50fbc4c13..49453690696d 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
>> case DSA_PORT_TYPE_UNUSED:
>> break;
>> case DSA_PORT_TYPE_CPU:
>> +/* dp->index is used now as port_number. However
>> + * CPU ports should have separate numbering
>> + * independent from front panel port numbers.
>> + */
>> +devlink_port_attrs_set(&dp->devlink_port,
>> + DEVLINK_PORT_FLAVOUR_CPU,
>> + dp->index, false, 0);
>> +err = dsa_port_link_register_of(dp);
>> +if (err) {
>> +dev_err(ds->dev, "failed to setup link for port
>> %d.%d\n",
>> +ds->index, dp->index);
>> +return err;
>> +}
>
>Ah, i get it. These used to be two case statements with one code
>block. But you split them apart, so needed to duplicate the
>dsa_port_link_register.
>
>Unfortunately, you forgot to add a 'break;', so it still falls
>through, and overwrites the port flavour to DSA.
ah, crap. Don't have hw to test this :/
Will fix. Thanks!
>
>> case DSA_PORT_TYPE_DSA:
>> +/* dp->index is used now as port_number. However
>> + * DSA ports should have separate numbering
>> + * independent from front panel port numbers.
>> + */
>> +devlink_port_attrs_set(&dp->devlink_port,
>> + DEVLINK_PORT_FLAVOUR_DSA,
>> + dp->index, false, 0);
>
> Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
> From: Jiri Pirko
>
> Set the attrs and allow to expose port flavour to user via devlink.
>
> Signed-off-by: Jiri Pirko
> ---
> net/dsa/dsa2.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index adf50fbc4c13..49453690696d 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
> case DSA_PORT_TYPE_UNUSED:
> break;
> case DSA_PORT_TYPE_CPU:
> + /* dp->index is used now as port_number. However
> + * CPU ports should have separate numbering
> + * independent from front panel port numbers.
> + */
> + devlink_port_attrs_set(&dp->devlink_port,
> +DEVLINK_PORT_FLAVOUR_CPU,
> +dp->index, false, 0);
> + err = dsa_port_link_register_of(dp);
> + if (err) {
> + dev_err(ds->dev, "failed to setup link for port
> %d.%d\n",
> + ds->index, dp->index);
> + return err;
> + }
Ah, i get it. These used to be two case statements with one code
block. But you split them apart, so needed to duplicate the
dsa_port_link_register.
Unfortunately, you forgot to add a 'break;', so it still falls
through, and overwrites the port flavour to DSA.
> case DSA_PORT_TYPE_DSA:
> + /* dp->index is used now as port_number. However
> + * DSA ports should have separate numbering
> + * independent from front panel port numbers.
> + */
> + devlink_port_attrs_set(&dp->devlink_port,
> +DEVLINK_PORT_FLAVOUR_DSA,
> +dp->index, false, 0);
Andrew
Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
> From: Jiri Pirko
>
> Set the attrs and allow to expose port flavour to user via devlink.
>
> Signed-off-by: Jiri Pirko
> ---
> net/dsa/dsa2.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index adf50fbc4c13..49453690696d 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
> case DSA_PORT_TYPE_UNUSED:
> break;
> case DSA_PORT_TYPE_CPU:
> + /* dp->index is used now as port_number. However
> + * CPU ports should have separate numbering
> + * independent from front panel port numbers.
> + */
> + devlink_port_attrs_set(&dp->devlink_port,
> +DEVLINK_PORT_FLAVOUR_CPU,
> +dp->index, false, 0);
> + err = dsa_port_link_register_of(dp);
> + if (err) {
> + dev_err(ds->dev, "failed to setup link for port
> %d.%d\n",
> + ds->index, dp->index);
> + return err;
> + }
Hi Jiri
Please could you explain what you are trying to achieve with this call to
dsa_port_link_register_of(dp);
Thanks
Andrew
