Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports

[Florian Fainelli <f.fainelli@gmail.com>]

On 03/23/2018 07:49 AM, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 02:30:02PM CET, andrew@lunn.ch wrote:
>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@mellanox.com>
>>>
>>> Set the attrs and allow to expose port flavour to user via devlink.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>>> ---
>>>  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