netdev - RE: [Intel-wired-lan] [PATCH iwl-net v2 1/2] devlink: allow driver to freely name interfaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <PH0PR11MB59025A3E1985F38E2F4EBAB7F023A@PH0PR11MB5902.namprd11.prod.outlook.com>
Date: Mon, 4 Aug 2025 11:41:06 +0000
From: "Jagielski, Jedrzej" <jedrzej.jagielski@...el.com>
To: "Keller, Jacob E" <jacob.e.keller@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC: "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "dhowells@...hat.com" <dhowells@...hat.com>,
	"David.Kaplan@....com" <David.Kaplan@....com>, "jiri@...nulli.us"
	<jiri@...nulli.us>, "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>
Subject: RE: [Intel-wired-lan] [PATCH iwl-net v2 1/2] devlink: allow driver to
 freely name interfaces

From: Keller, Jacob E <jacob.e.keller@...el.com> 
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 <jacob.e.keller@...el.com>

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

>
>
>> Suggested-by: Jiri Pirko <jiri@...nulli.us>
>> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
>> ---
>> 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)