[PATCH net v2 0/2][pull request] ixgbe: bypass devlink phys_port_name generation

[PATCH net v2 0/2][pull request] ixgbe: bypass devlink phys_port_name generation

[Tony Nguyen]

Jedrzej adds option to skip phys_port_name generation and opts
ixgbe into it as some configurations rely on pre-devlink naming
which could end up broken as a result.
---
v2:
- use shorter flag name (no_phys_port_name)
- move compatibility comment to kdoc
- adjust commit messages and titles

v1: https://lore.kernel.org/netdev/20250801172240.3105730-1-anthony.l.nguyen@intel.com/

The following are changes since commit 4eabe4cc0958e28ceaf592bbb62c234339642e41:
  dpll: zl3073x: ZL3073X_I2C and ZL3073X_SPI should depend on NET
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: let driver opt out of automatic phys_port_name generation
  ixgbe: prevent from unwanted interface name changes

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

-- 
2.47.1

[PATCH net v2 1/2] devlink: let driver opt out of automatic phys_port_name generation

[Tony Nguyen]

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

Currently when adding devlink port, phys_port_name is automatically
generated within devlink port initialization flow. As a result adding
devlink port support to driver may result in forced changes of interface
names, which breaks already existing network configs.

This is an expected behavior but in some scenarios it would not be
preferable to provide such limitation for legacy driver not being able to
keep 'pre-devlink' interface name.

Add flag no_phys_port_name to devlink_port_attrs struct which indicates
if devlink should not alter name of interface.

Suggested-by: Jiri Pirko <jiri@resnulli.us>
Link: https://lore.kernel.org/all/nbwrfnjhvrcduqzjl4a2jafnvvud6qsbxlvxaxilnryglf4j7r@btuqrimnfuly/
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 include/net/devlink.h | 5 ++++-
 net/devlink/port.c    | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 93640a29427c..4458b46407ee 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -78,6 +78,8 @@ 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.
+ * @no_phys_port_name: skip automatic phys_port_name generation; for compatibility only,
+ * 		       newly added driver/port instance should never set this.
  * @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 +89,8 @@ struct devlink_port_pci_sf_attrs {
  */
 struct devlink_port_attrs {
 	u8 split:1,
-	   splittable:1;
+	   splittable:1,
+	   no_phys_port_name:1;
 	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..44f808e65006 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.no_phys_port_name)
+		return 0;
+
 	switch (attrs->flavour) {
 	case DEVLINK_PORT_FLAVOUR_PHYSICAL:
 		if (devlink_port->linecard)
-- 
2.47.1

[PATCH net v2 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 interface names. It's
caused by automatic phys_port_name generation during devlink port
initialization flow.

To prevent from that set no_phys_port_name 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/
Fixes: a0285236ab93 ("ixgbe: add initial devlink support")
Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@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..d227f4d2a2d1 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.no_phys_port_name = 1;
 	ixgbe_devlink_set_switch_id(adapter, &attrs.switch_id);
 
 	devlink_port_attrs_set(devlink_port, &attrs);
-- 
2.47.1

Re: [PATCH net v2 1/2] devlink: let driver opt out of automatic phys_port_name generation

[Jakub Kicinski]

On Tue,  5 Aug 2025 15:33:41 -0700 Tony Nguyen wrote:
> + * @no_phys_port_name: skip automatic phys_port_name generation; for compatibility only,

line over 80 chars

Re: [PATCH net v2 1/2] devlink: let driver opt out of automatic phys_port_name generation

[Jakub Kicinski]

On Tue,  5 Aug 2025 15:33:41 -0700 Tony Nguyen wrote:
> +	if (devlink_port->attrs.no_phys_port_name)
> +		return 0;

Why are you returning 0 rather than -EOPNOTSUPP?
Driver which doesn't implement phys_port_name would normally return
-EOPNOTSUPP when user tries to read the sysfs file.

Re: [PATCH net v2 1/2] devlink: let driver opt out of automatic phys_port_name generation

[anthony.l.nguyen]

On 8/8/25 11:52 AM, Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue,  5 Aug 2025 15:33:41 -0700 Tony Nguyen wrote:
> > + * @no_phys_port_name: skip automatic phys_port_name generation; for compatibility only,
> 
> line over 80 chars

Will fix this.

Thanks,
Tony

Re: [PATCH net v2 1/2] devlink: let driver opt out of automatic phys_port_name generation

[Tony Nguyen]

On 8/8/2025 11:53 AM, Jakub Kicinski wrote:
> On Tue,  5 Aug 2025 15:33:41 -0700 Tony Nguyen wrote:
>> +	if (devlink_port->attrs.no_phys_port_name)
>> +		return 0;
> 
> Why are you returning 0 rather than -EOPNOTSUPP?
> Driver which doesn't implement phys_port_name would normally return
> -EOPNOTSUPP when user tries to read the sysfs file.

Jedrek is out so I'm not sure the reason, but it does seem
-EOPNOTSUPP would be more appropriate so I'll make that change.

Thanks,
Tony

Re: [PATCH net v2 1/2] devlink: let driver opt out of automatic phys_port_name generation

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 706 bytes --]



On 8/11/2025 2:27 PM, Tony Nguyen wrote:
> 
> 
> On 8/8/2025 11:53 AM, Jakub Kicinski wrote:
>> On Tue,  5 Aug 2025 15:33:41 -0700 Tony Nguyen wrote:
>>> +	if (devlink_port->attrs.no_phys_port_name)
>>> +		return 0;
>>
>> Why are you returning 0 rather than -EOPNOTSUPP?
>> Driver which doesn't implement phys_port_name would normally return
>> -EOPNOTSUPP when user tries to read the sysfs file.
> 
> Jedrek is out so I'm not sure the reason, but it does seem
> -EOPNOTSUPP would be more appropriate so I'll make that change.
> 
> Thanks,
> Tony

I agree, -EOPNOTSUPP makes the most sense here. It aligns with the way
we return -EOPNOTSUPP when attributes for the port aren't set.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net v2 1/2] devlink: let driver opt out of automatic phys_port_name generation

[Przemek Kitszel]

On 8/11/25 23:27, Tony Nguyen wrote:
> 
> 
> On 8/8/2025 11:53 AM, Jakub Kicinski wrote:
>> On Tue,  5 Aug 2025 15:33:41 -0700 Tony Nguyen wrote:
>>> +    if (devlink_port->attrs.no_phys_port_name)
>>> +        return 0;
>>
>> Why are you returning 0 rather than -EOPNOTSUPP?
>> Driver which doesn't implement phys_port_name would normally return
>> -EOPNOTSUPP when user tries to read the sysfs file.
> 
> Jedrek is out so I'm not sure the reason, but it does seem
> -EOPNOTSUPP would be more appropriate so I'll make that change.

when it was a driver callback, returning a 0 was short-circuiting
further (devlink) name computations

but in current form (v2), it is indeed better (at least wrt outcomes)
to return -EOPNOTSUPP, as that aligns with both sysfs and devlink output

so, good to update, thank you!

> 
> Thanks,
> Tony