From: Keller, Jacob E <[email protected]> 
Sent: Saturday, August 2, 2025 1:28 AM

>On 7/7/2025 1:58 AM, Jedrzej Jagielski wrote:
>> Currently when adding devlink port it is prohibited to let
>> a driver name an interface on its own. In some scenarios
>> it would not be preferable to provide such limitation,
>> eg some compatibility purposes.
>> 
>> Add flag skip_phys_port_name_get to devlink_port_attrs struct
>> which indicates if devlink should not alter name of interface.
>> 
>
>I feel like this somewhat lacks context in the commit message. The
>renaming is really the result of a policy provided by most distributions
>which uses the physical port name to determine its interface name, right?
>
>The fact that without the devlink support, this would be skipped, and
>adding devlink support made it change is not *exactly* the kernel's
>fault, but.. I think it does make sense to have the option for older
>drivers so they can add devlink support without triggering this
>behavioral change.
>
>While I might prefer more detail in the commit message, with or without
>that:
>Reviewed-by: Jacob Keller <[email protected]>

Thanks for suggestion Jake! I will try to be more precise ;)

>
>
>> Suggested-by: Jiri Pirko <[email protected]>
>> Signed-off-by: Jedrzej Jagielski <[email protected]>
>> ---
>> v2: add skip_phys_port_name_get flag to skip changing if name
>> ---
>>  include/net/devlink.h | 7 ++++++-
>>  net/devlink/port.c    | 3 +++
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index 0091f23a40f7..414ae25de897 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -78,6 +78,7 @@ struct devlink_port_pci_sf_attrs {
>>   * @flavour: flavour of the port
>>   * @split: indicates if this is split port
>>   * @splittable: indicates if the port can be split.
>> + * @skip_phys_port_name_get: if set devlink doesn't alter interface name
>>   * @lanes: maximum number of lanes the port supports. 0 value is not passed 
>> to netlink.
>>   * @switch_id: if the port is part of switch, this is buffer with ID, 
>> otherwise this is NULL
>>   * @phys: physical port attributes
>> @@ -87,7 +88,11 @@ struct devlink_port_pci_sf_attrs {
>>   */
>>  struct devlink_port_attrs {
>>      u8 split:1,
>> -       splittable:1;
>> +       splittable:1,
>> +       skip_phys_port_name_get:1; /* This is for compatibility only,
>> +                                   * newly added driver/port instance
>> +                                   * should never set this.
>> +                                   */
>>      u32 lanes;
>>      enum devlink_port_flavour flavour;
>>      struct netdev_phys_item_id switch_id;
>> diff --git a/net/devlink/port.c b/net/devlink/port.c
>> index 939081a0e615..bf52c8a57992 100644
>> --- a/net/devlink/port.c
>> +++ b/net/devlink/port.c
>> @@ -1522,6 +1522,9 @@ static int __devlink_port_phys_port_name_get(struct 
>> devlink_port *devlink_port,
>>      if (!devlink_port->attrs_set)
>>              return -EOPNOTSUPP;
>>  
>> +    if (devlink_port->attrs.skip_phys_port_name_get)
>> +            return 0;
>> +
>>      switch (attrs->flavour) {
>>      case DEVLINK_PORT_FLAVOUR_PHYSICAL:
>>              if (devlink_port->linecard)

Reply via email to