On Fri, 28 Jul 2017 07:58:15 +0200, Jiri Pirko wrote: > Fri, Jul 28, 2017 at 07:35:07AM CEST, j...@resnulli.us wrote: > >Fri, Jul 28, 2017 at 04:31:22AM CEST, jakub.kicin...@netronome.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
Hm.. Since we would return the type and IDs via netlink, I was actually leaning towards ndo_get_port_info. "Name" somehow implies a string in my mind. Perhaps the "info" part is where I went wrong, could we go with netdev_port_label? Or netdev_port_ident? I would also prefer to avoid the phys in name, since it doesn't sit well with PFs vs VFs. Ah, naming things...
