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

[Randy Dunlap]

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.

>>>>> +
>>>>> +			<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
>>>>
>>>>
>>>
>>
> 

-- 
~Randy