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

2025-07-04 Thread Jiri Pirko
Fri, Jul 04, 2025 at 11:17:23AM +0200, [email protected] wrote:
>Thu, Jul 03, 2025 at 01:30:21PM +0200, [email protected] 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.
>>
>>Remove triggering the warning when ndo_get_phys_port_name() is
>>implemented for driver which interface is about to get a devlink
>>port on.
>
>What's the reason for this? If you are missing some formatting, you
>should add it to devlink.
>
>Please don't to this.

I read the thread with the reported regression. Instead of this, could
you please perhaps rather add a flag to attrs:

struct devlink_port_attrs {
u8 split:1,
   splittable:1,
   skip_phys_port_name_get:1; /* This is for compatibility only,
   * newly added driver/port
   * instance should never
   * set this. */
Or something like that and check-return0 in
__devlink_port_phys_port_name_get()
?


>
>>
>>Suggested-by: Przemek Kitszel 
>>Signed-off-by: Jedrzej Jagielski 
>>---
>> net/devlink/port.c | 17 -
>> 1 file changed, 17 deletions(-)
>>
>>diff --git a/net/devlink/port.c b/net/devlink/port.c
>>index 939081a0e615..f885c8e73307 100644
>>--- a/net/devlink/port.c
>>+++ b/net/devlink/port.c
>>@@ -1161,23 +1161,6 @@ static void devlink_port_type_netdev_checks(struct 
>>devlink_port *devlink_port,
>> {
>>  const struct net_device_ops *ops = netdev->netdev_ops;
>> 
>>- /* If driver registers devlink port, it should set devlink port
>>-  * attributes accordingly so the compat functions are called
>>-  * and the original ops are not used.
>>-  */
>>- if (ops->ndo_get_phys_port_name) {
>>- /* Some drivers use the same set of ndos for netdevs
>>-  * that have devlink_port registered and also for
>>-  * those who don't. Make sure that ndo_get_phys_port_name
>>-  * returns -EOPNOTSUPP here in case it is defined.
>>-  * Warn if not.
>>-  */
>>- char name[IFNAMSIZ];
>>- int err;
>>-
>>- err = ops->ndo_get_phys_port_name(netdev, name, sizeof(name));
>>- WARN_ON(err != -EOPNOTSUPP);
>>- }
>>  if (ops->ndo_get_port_parent_id) {
>>  /* Some drivers use the same set of ndos for netdevs
>>   * that have devlink_port registered and also for
>>-- 
>>2.31.1
>>


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

2025-07-04 Thread Jiri Pirko
Thu, Jul 03, 2025 at 01:30:21PM +0200, [email protected] 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.
>
>Remove triggering the warning when ndo_get_phys_port_name() is
>implemented for driver which interface is about to get a devlink
>port on.

What's the reason for this? If you are missing some formatting, you
should add it to devlink.

Please don't to this.

>
>Suggested-by: Przemek Kitszel 
>Signed-off-by: Jedrzej Jagielski 
>---
> net/devlink/port.c | 17 -
> 1 file changed, 17 deletions(-)
>
>diff --git a/net/devlink/port.c b/net/devlink/port.c
>index 939081a0e615..f885c8e73307 100644
>--- a/net/devlink/port.c
>+++ b/net/devlink/port.c
>@@ -1161,23 +1161,6 @@ static void devlink_port_type_netdev_checks(struct 
>devlink_port *devlink_port,
> {
>   const struct net_device_ops *ops = netdev->netdev_ops;
> 
>-  /* If driver registers devlink port, it should set devlink port
>-   * attributes accordingly so the compat functions are called
>-   * and the original ops are not used.
>-   */
>-  if (ops->ndo_get_phys_port_name) {
>-  /* Some drivers use the same set of ndos for netdevs
>-   * that have devlink_port registered and also for
>-   * those who don't. Make sure that ndo_get_phys_port_name
>-   * returns -EOPNOTSUPP here in case it is defined.
>-   * Warn if not.
>-   */
>-  char name[IFNAMSIZ];
>-  int err;
>-
>-  err = ops->ndo_get_phys_port_name(netdev, name, sizeof(name));
>-  WARN_ON(err != -EOPNOTSUPP);
>-  }
>   if (ops->ndo_get_port_parent_id) {
>   /* Some drivers use the same set of ndos for netdevs
>* that have devlink_port registered and also for
>-- 
>2.31.1
>