netdev - Re: [RFC] switchdev: generate phys_port

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170728055815.GB1857@nanopsycho>
Date:   Fri, 28 Jul 2017 07:58:15 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Or Gerlitz <gerlitz.or@...il.com>, netdev@...r.kernel.org,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Michael Chan <michael.chan@...adcom.com>,
        Sathya Perla <sathya.perla@...adcom.com>,
        David Miller <davem@...emloft.net>, simon.horman@...ronome.com
Subject: Re: [RFC] switchdev: generate phys_port_name in the core

Fri, Jul 28, 2017 at 07:35:07AM CEST, jiri@...nulli.us wrote:
>Fri, Jul 28, 2017 at 04:31:22AM CEST, jakub.kicinski@...ronome.com wrote:
>>On Thu, 27 Jul 2017 13:30:44 +0300, Or Gerlitz wrote:
>>> > want to add port splitting support, for example, reporting the name on
>>> > physical ports will become more of a necessity.  
>>> 
>>> > If we adopt Jiri's suggestion of returning structured data it will be
>>> > very easy to give user space type and indexes separately, but we should
>>> > probably still return the string for backwards compatibility.  
>>> 
>>> I am not still clear how the structured data would look like
>>
>>I decided to just quickly write the code, that should be easier to 
>>understand.  We can probably leave out the netlink part of the API
>>if there is no need for it right now, but that's what I ment by
>>returning the information in a more structured way.
>>
>>Tested-by: nobody :)
>>Suggested-by: Jiri (if I understood correctly)
>
>Yes, you did :) Couple of nits inlined.
>
>
>>---
>> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c |  8 ++-
>> drivers/net/ethernet/mellanox/mlxsw/switchx2.c   | 10 ++--
>> drivers/net/ethernet/netronome/nfp/nfp_port.c    | 26 ++++-----
>> drivers/net/ethernet/netronome/nfp/nfp_port.h    |  4 +-
>> include/linux/netdevice.h                        | 18 ++++++-
>> include/uapi/linux/if_link.h                     | 16 ++++++
>> net/core/dev.c                                   | 31 +++++++++--
>> net/core/rtnetlink.c                             | 69 ++++++++++++++++++++++++
>> 8 files changed, 153 insertions(+), 29 deletions(-)
>>
>>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>>index 45e60be9c277..7a71291b8ec3 100644
>>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>>@@ -637,16 +637,14 @@ static int mlx5e_rep_close(struct net_device *dev)
>> }
>> 
>> static int mlx5e_rep_get_phys_port_name(struct net_device *dev,
>>-					char *buf, size_t len)
>>+					struct netdev_port_info *info)
>
>Either we rename ndo to something like ndo_get_port_info or you rename
>the struct to netdev_port_name_info. These 2 should be in sync

As this is strictly related to port name, I think it should be:
struct netdev_phys_port_name_info

And the rest named in-sync with this