RE: [PATCH net-next v4] devlink: Add devlink port documentation

[Parav Pandit <parav@nvidia.com>]

> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, December 7, 2020 11:10 PM
> 
> On Mon, 7 Dec 2020 04:46:14 +0000 Parav Pandit wrote:
> > > From: Jakub Kicinski <kuba@kernel.org>
> > > Sent: Sunday, December 6, 2020 1:57 AM
> > >
> > > On Thu, 3 Dec 2020 20:02:55 +0200 Parav Pandit wrote:
> > > > Added documentation for devlink port and port function related
> commands.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > > > Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> > > > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> > >
> > > > +============
> > > > +Devlink Port
> > > > +============
> > > > +
> > > > +``devlink-port`` is a port that exists on the device.
> > >
> > > Can we add something like:
> > >
> > > Each port is a logically separate ingress/egress point of the device.
> > >
> > > ?
> > This may not be true when both physical ports are under bond.
> 
> Bonding changes forwarding logic, not what points of egress ASIC has.
Ok. Do CPU and DSA port also follow same?

> 
> > > > Such PCI function consists
> > > > +of one or more networking ports.
> > >
> > > PCI function consists of networking ports? What do you mean by a
> > > networking port? All devlink ports are networking ports.
> > >
> > I am not sure this document should be a starting point to define such
> restriction.
> 
> Well it's the reality today. Adding "networking" everywhere in this document
> is pointless.
> 
Ok. so I drop networking and just say PCI function port.

> > > > A networking port of such PCI function is
> > > > +represented by the eswitch devlink port.
> > >
> > > What's eswitch devlink port? It was never defined.
> > Eswitch devlink port is the port which sets eswitch attributes (id and
> length).
> 
> You mean phys_port_id?
No. I mean phys_switch_id.

> 
> > > > before
> > > > +enumerating the function.
> > >
> > > What does this mean? What does enumerate mean in this context?
> > >
> > Enumerate means before creating the device of the function.
> > However today due to SR-IOV limitation, it is before probing the function
> device.
> 
> Can you rephrase to make the point clearer?
Sure. Will do.
> 
> > > > For example user may set the hardware address of
> > > > +the function represented by the devlink port function.
> > >
> > > What's a hardware address? You mean MAC address?
> > Yes, MAC address.
> > Port function attribute is named as hardware address to be generic enough
> similar to other iproute2 tools.
> 
> Right, but in iproute2 the context makes it clear. Here we could be talking
> about many things.
Kernel interface is not restricted to mac address, nor the iproute2.
So I will mention as hardware address to be precise what is does and additionally mention that it is mac address for Ethernet ports.

Will send v5 addressing these comments.