Re: [PATCH net-next V2 7/7] devlink: Add eswitch mode boot defaults

[Mark Bloch <mbloch@nvidia.com>]

On 05/06/2026 7:02, Randy Dunlap wrote:
> 
> 
> On 6/4/26 2:49 AM, Mark Bloch wrote:
>>
>>
>> On 04/06/2026 6:53, Randy Dunlap wrote:
>>>
>>>
>>> On 6/3/26 6:16 PM, Mark Bloch wrote:
>>>>
>>>>
>>>> On 03/06/2026 23:06, Randy Dunlap wrote:
>>>>> Hi.
>>>>>
>>>>> On 6/3/26 12:32 PM, Mark Bloch wrote:
>>>>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>>>>> index 063c11ca33e5..7af9f2898d92 100644
>>>>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>>>>> @@ -1264,6 +1264,31 @@ Kernel parameters
>>>>>>  	dell_smm_hwmon.fan_max=
>>>>>>  			[HW] Maximum configurable fan speed.
>>>>>>  
>>>>>> +	devlink_eswitch_mode=
>>>>>> +			[NET]
>>>>>> +			Format:
>>>>>> +			[<selector>]:<mode>
>>>>>
>>>>> It appears (please correct me if I am mistaken) that the '[' and ']'
>>>>> above don't mean "optional" but instead they are required characters...
>>>>>
>>>>>> +
>>>>>> +			<selector>:
>>>>>> +			* | <handle>[,<handle>...]
>>>>>
>>>>> while here they mean "optional".
>>>>>
>>>>> That is confusing (inconsistent). Also, if the square brackets are
>>>>> always required around the <selector>, what purpose do they serve?
>>>>
>>>> Yes, you are right, this is confusing. The outer square brackets are part of
>>>> the syntax and are required, while the brackets in "[,<handle>...]" mean that
>>>> additional handles are optional.
>>>>
>>>> I couldn't find a better way to describe this. What I want to say is that the
>>>> selector is always wrapped in square brackets. Inside the brackets it can either
>>>> be "*" to match all devices, or a comma separated list of handles. If "*" is
>>>> not used, then at least one handle has to be provided.
>>>>
>>>> Maybe it would be clearer to spell it out explicitly, something like:
>>>>
>>>> Format:
>>>>   [<selector>]:<mode>
>>>>
>>>> The '[' and ']' characters are literal and required.
>>>>
>>>> <selector>:
>>>>   * | <handle>[,<handle>...]
>>>>
>>>> If '*' is not used, <selector> must contain at least one <handle>.
>>>>
>>>> Does that sound like a reasonable way to document it?
>>>
>>> Yes, that helps a little bit. Better than nothing.
>>>
>>> But why are they required at all?
>>
>> Jiri suggested using the square brackets, and I liked that they made the
>> selector look like a grouped argument. But if that is too confusing, I can
>> also drop them and use a simpler separator, for example:
>>
>> 	devlink_eswitch_mode=
>> 			[NET]
>> 			Format:
>> 			<selector>=<mode>
>>
>> 			<selector>:
>> 			* | <handle>[,<handle>...]
>>
>> 			<handle>:
>> 			<bus-name>/<dev-name>
>>
>> 			Configure default devlink eswitch mode for matching
>> 			devlink instances during device initialization.
>>
>> 			<mode>:
>> 			legacy | switchdev | switchdev_inactive
>>
>> 			Examples:
>> 			devlink_eswitch_mode=*=switchdev
>> 			devlink_eswitch_mode=pci/0000:08:00.0=switchdev
>> 			devlink_eswitch_mode=pci/0000:08:00.0,pci/0000:09:00.1=switchdev_inactive
>>
>> Does this look better to you?
> 
> Yes, that looks much better to me.
> But you should do whatever you think is right.

Good syntax should not be confusing. If it was not clear to you, then
it will probably not be clear to others either, so I am fine with
changing it.

I will post v3 with this change.

Thanks for the input!

Mark

> 
>>>>>> +
>>>>>> +			<handle>:
>>>>>> +			<bus-name>/<dev-name>
>>>>>> +
>>>>>> +			Configure default devlink eswitch mode for matching
>>>>>> +			devlink instances during device initialization.
>>>>>> +
>>>>>> +			<mode>:
>>>>>> +			legacy | switchdev | switchdev_inactive
>>>>>> +
>>>>>> +			Examples:
>>>>>> +			devlink_eswitch_mode=[*]:switchdev
>>>>>> +			devlink_eswitch_mode=[pci/0000:08:00.0]:switchdev
>>>>>> +			devlink_eswitch_mode=[pci/0000:08:00.0,pci/0000:09:00.1]:legacy
>>>>>> +
>>>>>> +			See Documentation/networking/devlink/devlink-defaults.rst
>>>>>> +			for the full syntax.
>>>>>> +
>>>>>>  	dfltcc=		[HW,S390]
>>>>>>  			Format: { on | off | def_only | inf_only | always }
>>>>>>  			on:       s390 zlib hardware support for compression on
>>>>>
>>>>>
>>>>
>>>
>>
>