[PATCH net 0/2][pull request] ixgbe: stop interface name changes

[PATCH net 0/2][pull request] ixgbe: stop interface name changes

[Tony Nguyen]

Jedrzej adds the option to not rename devlink ports and opts ixgbe into
this as some configurations rely on pre-devlink naming which could end up
broken as a result of the renaming.

The following are changes since commit 01051012887329ea78eaca19b1d2eac4c9f601b5:
  netlink: specs: ethtool: fix module EEPROM input/output arguments
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue 10GbE

Jedrzej Jagielski (2):
  devlink: allow driver to freely name interfaces
  ixgbe: prevent from unwanted interface name changes

 drivers/net/ethernet/intel/ixgbe/devlink/devlink.c | 1 +
 include/net/devlink.h                              | 7 ++++++-
 net/devlink/port.c                                 | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.47.1

[PATCH net 1/2] devlink: allow driver to freely name interfaces

[Tony Nguyen]

From: Jedrzej Jagielski <jedrzej.jagielski@intel.com>

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.

Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 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 93640a29427c..bfa795bf9998 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)
-- 
2.47.1

[PATCH net 2/2] ixgbe: prevent from unwanted interface name changes

[Tony Nguyen]

From: Jedrzej Jagielski <jedrzej.jagielski@intel.com>

Users of the ixgbe driver report that after adding devlink support by
the commit a0285236ab93 ("ixgbe: add initial devlink support") their
configs got broken due to unwanted changes of interfaces names. It's
caused by changing names by devlink port initialization flow.

To prevent from that set skip_phys_port_name_get flag for ixgbe devlink
ports.

Reported-by: David Howells <dhowells@redhat.com>
Closes: https://lore.kernel.org/netdev/3452224.1745518016@warthog.procyon.org.uk/
Reported-by: David Kaplan <David.Kaplan@amd.com>
Closes: https://lore.kernel.org/netdev/LV3PR12MB92658474624CCF60220157199470A@LV3PR12MB9265.namprd12.prod.outlook.com/
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Fixes: a0285236ab93 ("ixgbe: add initial devlink support")
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/devlink/devlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
index 54f1b83dfe42..47fae5035b9f 100644
--- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
+++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
@@ -543,6 +543,7 @@ int ixgbe_devlink_register_port(struct ixgbe_adapter *adapter)
 
 	attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
 	attrs.phys.port_number = adapter->hw.bus.func;
+	attrs.skip_phys_port_name_get = 1;
 	ixgbe_devlink_set_switch_id(adapter, &attrs.switch_id);
 
 	devlink_port_attrs_set(devlink_port, &attrs);
-- 
2.47.1

Re: [PATCH net 1/2] devlink: allow driver to freely name interfaces

[Jakub Kicinski]

On Fri,  1 Aug 2025 10:22:37 -0700 Tony Nguyen wrote:
> Subject: [PATCH net 1/2] devlink: allow driver to freely name interfaces

The subject is a bit misleading.. Maybe something like:

  let driver opt out of automatic phys_port_name generation

> Date: Fri,  1 Aug 2025 10:22:37 -0700
> X-Mailer: git-send-email 2.47.1
> 
> From: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
> 
> 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.
> 
> Suggested-by: Jiri Pirko <jiri@resnulli.us>

Link to the suggestion could be useful?

> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  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 93640a29427c..bfa795bf9998 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

Again, we're not actually doing anything the the interface name.
We're exposing a sysfs attribute which systemd/udev then uses
to rename the interface. From kernel PoV this is about attributes.

>   * @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.
> +				       */

Thanks for noting that this is compat-only. I think it'd be better
to consolidate the comments, since we have kdoc..
Move this note up to kdoc; or document the member inline:

	splittable:1,
	/**
	 * @skip_phys_port_..: bok bok ba-gok!
	 */
	skip_phys_port_name_get:1;

BTW the name is a bit long, "no_phys_port_name" please?

RE: [PATCH net 1/2] devlink: allow driver to freely name interfaces

[Jagielski, Jedrzej]

From: Jakub Kicinski <kuba@kernel.org> 
Sent: Friday, August 1, 2025 10:15 PM

>On Fri,  1 Aug 2025 10:22:37 -0700 Tony Nguyen wrote:
>> Subject: [PATCH net 1/2] devlink: allow driver to freely name interfaces
>
>The subject is a bit misleading.. Maybe something like:
>
>  let driver opt out of automatic phys_port_name generation

Sure, title will be changed

>
>> Date: Fri,  1 Aug 2025 10:22:37 -0700
>> X-Mailer: git-send-email 2.47.1
>> 
>> From: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
>> 
>> 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.
>> 
>> Suggested-by: Jiri Pirko <jiri@resnulli.us>
>
>Link to the suggestion could be useful?

yeah, why not, it provides some context

>
>> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
>> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
>> ---
>>  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 93640a29427c..bfa795bf9998 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
>
>Again, we're not actually doing anything the the interface name.
>We're exposing a sysfs attribute which systemd/udev then uses
>to rename the interface. From kernel PoV this is about attributes.
>
>>   * @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.
>> +				       */
>
>Thanks for noting that this is compat-only. I think it'd be better

It was Jiri who instantly suggested that note :P

>to consolidate the comments, since we have kdoc..
>Move this note up to kdoc; or document the member inline:
>
>	splittable:1,
>	/**
>	 * @skip_phys_port_..: bok bok ba-gok!
>	 */
>	skip_phys_port_name_get:1;

oh right, it will be moved to kdoc

>
>BTW the name is a bit long, "no_phys_port_name" please?

sure, i like it, will be changed