netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] devlink: Add devlink port documentation
@ 2020-11-30 16:41 Parav Pandit
  2020-11-30 19:29 ` Jacob Keller
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Parav Pandit @ 2020-11-30 16:41 UTC (permalink / raw)
  To: netdev, davem, kuba; +Cc: Parav Pandit, Jiri Pirko

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>
---
 .../networking/devlink/devlink-port.rst       | 102 ++++++++++++++++++
 Documentation/networking/devlink/index.rst    |   1 +
 2 files changed, 103 insertions(+)
 create mode 100644 Documentation/networking/devlink/devlink-port.rst

diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
new file mode 100644
index 000000000000..966d2ee328a6
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-port.rst
@@ -0,0 +1,102 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+Devlink Port
+============
+
+``devlink-port`` provides capability for a driver to expose various
+flavours of ports which exist on device. A devlink port can of an
+embedded switch (eswitch) present on the device.
+
+A devlink port can be of 3 diffferent types.
+
+.. list-table:: List of devlink port types
+   :widths: 23 90
+
+   * - Type
+     - Description
+   * - ``DEVLINK_PORT_TYPE_ETH``
+     - This type is set for a devlink port when a physical link layer of the port
+       is Ethernet.
+   * - ``DEVLINK_PORT_TYPE_IB``
+     - This type is set for a devlink port when a physical link layer of the port
+       is InfiniBand.
+   * - ``DEVLINK_PORT_TYPE_AUTO``
+     - This type is indicated by the user when user prefers to set the port type
+       to be automatically detected by the device driver.
+
+Devlink port can be of few different flavours described below.
+
+.. list-table:: List of devlink port flavours
+   :widths: 33 90
+
+   * - Flavour
+     - Description
+   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
+     - Any kind of port which is physically facing the user. This can be
+       a eswitch physical port or any other physical port on the device.
+   * - ``DEVLINK_PORT_FLAVOUR_CPU``
+     - This indicates a CPU port.
+   * - ``DEVLINK_PORT_FLAVOUR_DSA``
+     - This indicates a interconnect port in a distributed switch architecture.
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
+     - This indicates an eswitch port representing PCI physical function(PF).
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
+     - This indicates an eswitch port representing PCI virtual function(VF).
+   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
+     - This indicates a virtual port facing the user.
+   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
+     - This indicates an virtual port facing the user.
+
+A devlink port may be for a controller consist of one or more PCI device(s).
+A devlink instance holds ports of two types of controllers.
+
+(1) controller discovered on same system where eswitch resides
+This is the case where PCI PF/VF of a controller and devlink eswitch
+instance both are located on a single system.
+
+(2) controller located on external host system.
+This is the case where a controller is located in one system and its
+devlink eswitch ports are located in a different system.
+
+An example view of two controller systems::
+
+                 ---------------------------------------------------------
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
+    | connect |  | -------                       -------                 |
+    -----------  |     | controller_num=1 (no eswitch)                   |
+                 ------|--------------------------------------------------
+                 (internal wire)
+                       |
+                 ---------------------------------------------------------
+                 | devlink eswitch ports and reps                        |
+                 | ----------------------------------------------------- |
+                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 |                                                       |
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+                 | | pf0 |______/________/       | pf1 |___/_______/     |
+                 | -------                       -------                 |
+                 |                                                       |
+                 |  local controller_num=0 (eswitch)                     |
+                 ---------------------------------------------------------
+
+Port function configuration
+===========================
+
+When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
+``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the port of a PCI function.
+A user can configure the port function attributes before enumerating the
+function. For example user may set the hardware address of the function
+represented by the devlink port.
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index d82874760ae2..aab79667f97b 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -18,6 +18,7 @@ general.
    devlink-info
    devlink-flash
    devlink-params
+   devlink-port
    devlink-region
    devlink-resource
    devlink-reload
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next] devlink: Add devlink port documentation
  2020-11-30 16:41 [PATCH net-next] devlink: Add devlink port documentation Parav Pandit
@ 2020-11-30 19:29 ` Jacob Keller
  2020-11-30 19:51   ` Parav Pandit
  2020-11-30 20:00 ` [PATCH net-next v2] " Parav Pandit
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Jacob Keller @ 2020-11-30 19:29 UTC (permalink / raw)
  To: Parav Pandit, netdev, davem, kuba; +Cc: Jiri Pirko



On 11/30/2020 8:41 AM, 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>

Good to see this! I saw a couple of minor nits.

- Jake

> ---
>  .../networking/devlink/devlink-port.rst       | 102 ++++++++++++++++++
>  Documentation/networking/devlink/index.rst    |   1 +
>  2 files changed, 103 insertions(+)
>  create mode 100644 Documentation/networking/devlink/devlink-port.rst
> 
> diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
> new file mode 100644
> index 000000000000..966d2ee328a6
> --- /dev/null
> +++ b/Documentation/networking/devlink/devlink-port.rst
> @@ -0,0 +1,102 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============
> +Devlink Port
> +============
> +
> +``devlink-port`` provides capability for a driver to expose various
> +flavours of ports which exist on device. A devlink port can of an
> +embedded switch (eswitch) present on the device.
> +

Seems like a word is missing here in the "A devlink port can of an
embedded switch". Perhaps "Can be of"?

> +A devlink port can be of 3 diffferent types.
> +
> +.. list-table:: List of devlink port types
> +   :widths: 23 90
> +
> +   * - Type
> +     - Description
> +   * - ``DEVLINK_PORT_TYPE_ETH``
> +     - This type is set for a devlink port when a physical link layer of the port
> +       is Ethernet.
> +   * - ``DEVLINK_PORT_TYPE_IB``
> +     - This type is set for a devlink port when a physical link layer of the port
> +       is InfiniBand.
> +   * - ``DEVLINK_PORT_TYPE_AUTO``
> +     - This type is indicated by the user when user prefers to set the port type
> +       to be automatically detected by the device driver.
> +
> +Devlink port can be of few different flavours described below.
> +
> +.. list-table:: List of devlink port flavours
> +   :widths: 33 90
> +
> +   * - Flavour
> +     - Description
> +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> +     - Any kind of port which is physically facing the user. This can be
> +       a eswitch physical port or any other physical port on the device.
> +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> +     - This indicates a CPU port.
> +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> +     - This indicates a interconnect port in a distributed switch architecture.
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> +     - This indicates an eswitch port representing PCI physical function(PF).
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> +     - This indicates an eswitch port representing PCI virtual function(VF).
> +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> +     - This indicates a virtual port facing the user.
> +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> +     - This indicates an virtual port facing the user.

DEVLINK_PORT_FLAVOUR_VIRTUAL is repeated.

> +
> +A devlink port may be for a controller consist of one or more PCI device(s).
> +A devlink instance holds ports of two types of controllers.
> +

s/consist/consisting/ ?

> +(1) controller discovered on same system where eswitch resides
> +This is the case where PCI PF/VF of a controller and devlink eswitch
> +instance both are located on a single system.
> +
> +(2) controller located on external host system.
> +This is the case where a controller is located in one system and its
> +devlink eswitch ports are located in a different system.
> +
> +An example view of two controller systems::
> +
> +                 ---------------------------------------------------------
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
> +    | connect |  | -------                       -------                 |
> +    -----------  |     | controller_num=1 (no eswitch)                   |
> +                 ------|--------------------------------------------------
> +                 (internal wire)
> +                       |
> +                 ---------------------------------------------------------
> +                 | devlink eswitch ports and reps                        |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 |                                                       |
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +                 | | pf0 |______/________/       | pf1 |___/_______/     |
> +                 | -------                       -------                 |
> +                 |                                                       |
> +                 |  local controller_num=0 (eswitch)                     |
> +                 ---------------------------------------------------------
> +
> +Port function configuration
> +===========================
> +
> +When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
> +``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the port of a PCI function.
> +A user can configure the port function attributes before enumerating the
> +function. For example user may set the hardware address of the function
> +represented by the devlink port.
> diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
> index d82874760ae2..aab79667f97b 100644
> --- a/Documentation/networking/devlink/index.rst
> +++ b/Documentation/networking/devlink/index.rst
> @@ -18,6 +18,7 @@ general.
>     devlink-info
>     devlink-flash
>     devlink-params
> +   devlink-port
>     devlink-region
>     devlink-resource
>     devlink-reload
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next] devlink: Add devlink port documentation
  2020-11-30 19:29 ` Jacob Keller
@ 2020-11-30 19:51   ` Parav Pandit
  0 siblings, 0 replies; 22+ messages in thread
From: Parav Pandit @ 2020-11-30 19:51 UTC (permalink / raw)
  To: Jacob Keller, netdev, davem, kuba; +Cc: Jiri Pirko



> From: Jacob Keller <jacob.e.keller@intel.com>
> Sent: Tuesday, December 1, 2020 12:59 AM
> 
> 
> 
> On 11/30/2020 8:41 AM, 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>
> 
> Good to see this! I saw a couple of minor nits.
>
Thanks, fixing them.
 
> - Jake
> 
> > ---
> >  .../networking/devlink/devlink-port.rst       | 102 ++++++++++++++++++
> >  Documentation/networking/devlink/index.rst    |   1 +
> >  2 files changed, 103 insertions(+)
> >  create mode 100644 Documentation/networking/devlink/devlink-port.rst
> >
> > diff --git a/Documentation/networking/devlink/devlink-port.rst
> > b/Documentation/networking/devlink/devlink-port.rst
> > new file mode 100644
> > index 000000000000..966d2ee328a6
> > --- /dev/null
> > +++ b/Documentation/networking/devlink/devlink-port.rst
> > @@ -0,0 +1,102 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +============
> > +Devlink Port
> > +============
> > +
> > +``devlink-port`` provides capability for a driver to expose various
> > +flavours of ports which exist on device. A devlink port can of an
> > +embedded switch (eswitch) present on the device.
> > +
> 
> Seems like a word is missing here in the "A devlink port can of an embedded
> switch". Perhaps "Can be of"?
> 
Yes. 
> > +A devlink port can be of 3 diffferent types.
> > +
> > +.. list-table:: List of devlink port types
> > +   :widths: 23 90
> > +
> > +   * - Type
> > +     - Description
> > +   * - ``DEVLINK_PORT_TYPE_ETH``
> > +     - This type is set for a devlink port when a physical link layer of the port
> > +       is Ethernet.
> > +   * - ``DEVLINK_PORT_TYPE_IB``
> > +     - This type is set for a devlink port when a physical link layer of the port
> > +       is InfiniBand.
> > +   * - ``DEVLINK_PORT_TYPE_AUTO``
> > +     - This type is indicated by the user when user prefers to set the port type
> > +       to be automatically detected by the device driver.
> > +
> > +Devlink port can be of few different flavours described below.
> > +
> > +.. list-table:: List of devlink port flavours
> > +   :widths: 33 90
> > +
> > +   * - Flavour
> > +     - Description
> > +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> > +     - Any kind of port which is physically facing the user. This can be
> > +       a eswitch physical port or any other physical port on the device.
> > +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> > +     - This indicates a CPU port.
> > +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> > +     - This indicates a interconnect port in a distributed switch architecture.
> > +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> > +     - This indicates an eswitch port representing PCI physical function(PF).
> > +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> > +     - This indicates an eswitch port representing PCI virtual function(VF).
> > +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> > +     - This indicates a virtual port facing the user.
> > +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> > +     - This indicates an virtual port facing the user.
> 
> DEVLINK_PORT_FLAVOUR_VIRTUAL is repeated.
> 
Removing it.

> > +
> > +A devlink port may be for a controller consist of one or more PCI device(s).
> > +A devlink instance holds ports of two types of controllers.
> > +
> 
> s/consist/consisting/ ?
>
Yes.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH net-next v2] devlink: Add devlink port documentation
  2020-11-30 16:41 [PATCH net-next] devlink: Add devlink port documentation Parav Pandit
  2020-11-30 19:29 ` Jacob Keller
@ 2020-11-30 20:00 ` Parav Pandit
  2020-11-30 20:27   ` Keller, Jacob E
  2020-12-02  1:34   ` Jakub Kicinski
  2020-12-02 13:53 ` [PATCH net-next v3] " Parav Pandit
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 22+ messages in thread
From: Parav Pandit @ 2020-11-30 20:00 UTC (permalink / raw)
  To: netdev, davem, kuba; +Cc: jacob.e.keller, Parav Pandit, Jiri Pirko

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>
---
Changelog:
v1->v2:
 - Removed duplicate table entries for DEVLINK_PORT_FLAVOUR_VIRTUAL.
 - replaced 'consist of' to 'consisting'
 - changed 'can be' to 'can be of'
---
 .../networking/devlink/devlink-port.rst       | 100 ++++++++++++++++++
 Documentation/networking/devlink/index.rst    |   1 +
 2 files changed, 101 insertions(+)
 create mode 100644 Documentation/networking/devlink/devlink-port.rst

diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
new file mode 100644
index 000000000000..f3ed65acbd52
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-port.rst
@@ -0,0 +1,100 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+Devlink Port
+============
+
+``devlink-port`` provides capability for a driver to expose various
+flavours of ports which exist on device. A devlink port can be of an
+embedded switch (eswitch) present on the device.
+
+A devlink port can be of 3 diffferent types.
+
+.. list-table:: List of devlink port types
+   :widths: 23 90
+
+   * - Type
+     - Description
+   * - ``DEVLINK_PORT_TYPE_ETH``
+     - This type is set for a devlink port when a physical link layer of the port
+       is Ethernet.
+   * - ``DEVLINK_PORT_TYPE_IB``
+     - This type is set for a devlink port when a physical link layer of the port
+       is InfiniBand.
+   * - ``DEVLINK_PORT_TYPE_AUTO``
+     - This type is indicated by the user when user prefers to set the port type
+       to be automatically detected by the device driver.
+
+Devlink port can be of few different flavours described below.
+
+.. list-table:: List of devlink port flavours
+   :widths: 33 90
+
+   * - Flavour
+     - Description
+   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
+     - Any kind of port which is physically facing the user. This can be
+       a eswitch physical port or any other physical port on the device.
+   * - ``DEVLINK_PORT_FLAVOUR_CPU``
+     - This indicates a CPU port.
+   * - ``DEVLINK_PORT_FLAVOUR_DSA``
+     - This indicates a interconnect port in a distributed switch architecture.
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
+     - This indicates an eswitch port representing PCI physical function(PF).
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
+     - This indicates an eswitch port representing PCI virtual function(VF).
+   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
+     - This indicates a virtual port facing the user.
+
+A devlink port may be for a controller consisting one or more PCI device(s).
+A devlink instance holds ports of two types of controllers.
+
+(1) controller discovered on same system where eswitch resides
+This is the case where PCI PF/VF of a controller and devlink eswitch
+instance both are located on a single system.
+
+(2) controller located on external host system.
+This is the case where a controller is located in one system and its
+devlink eswitch ports are located in a different system.
+
+An example view of two controller systems::
+
+                 ---------------------------------------------------------
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
+    | connect |  | -------                       -------                 |
+    -----------  |     | controller_num=1 (no eswitch)                   |
+                 ------|--------------------------------------------------
+                 (internal wire)
+                       |
+                 ---------------------------------------------------------
+                 | devlink eswitch ports and reps                        |
+                 | ----------------------------------------------------- |
+                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 |                                                       |
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+                 | | pf0 |______/________/       | pf1 |___/_______/     |
+                 | -------                       -------                 |
+                 |                                                       |
+                 |  local controller_num=0 (eswitch)                     |
+                 ---------------------------------------------------------
+
+Port function configuration
+===========================
+
+When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
+``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the port of a PCI function.
+A user can configure the port function attributes before enumerating the
+function. For example user may set the hardware address of the function
+represented by the devlink port.
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index d82874760ae2..aab79667f97b 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -18,6 +18,7 @@ general.
    devlink-info
    devlink-flash
    devlink-params
+   devlink-port
    devlink-region
    devlink-resource
    devlink-reload
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next v2] devlink: Add devlink port documentation
  2020-11-30 20:00 ` [PATCH net-next v2] " Parav Pandit
@ 2020-11-30 20:27   ` Keller, Jacob E
  2020-12-02  1:34   ` Jakub Kicinski
  1 sibling, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2020-11-30 20:27 UTC (permalink / raw)
  To: Parav Pandit, netdev, davem, kuba; +Cc: Jiri Pirko



> -----Original Message-----
> From: Parav Pandit <parav@nvidia.com>
> Sent: Monday, November 30, 2020 12:00 PM
> To: netdev@vger.kernel.org; davem@davemloft.net; kuba@kernel.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Parav Pandit
> <parav@nvidia.com>; Jiri Pirko <jiri@nvidia.com>
> Subject: [PATCH net-next v2] devlink: Add devlink port documentation
> 
> 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>

> ---
> Changelog:
> v1->v2:
>  - Removed duplicate table entries for DEVLINK_PORT_FLAVOUR_VIRTUAL.
>  - replaced 'consist of' to 'consisting'
>  - changed 'can be' to 'can be of'
> ---
>  .../networking/devlink/devlink-port.rst       | 100 ++++++++++++++++++
>  Documentation/networking/devlink/index.rst    |   1 +
>  2 files changed, 101 insertions(+)
>  create mode 100644 Documentation/networking/devlink/devlink-port.rst
> 
> diff --git a/Documentation/networking/devlink/devlink-port.rst
> b/Documentation/networking/devlink/devlink-port.rst
> new file mode 100644
> index 000000000000..f3ed65acbd52
> --- /dev/null
> +++ b/Documentation/networking/devlink/devlink-port.rst
> @@ -0,0 +1,100 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============
> +Devlink Port
> +============
> +
> +``devlink-port`` provides capability for a driver to expose various
> +flavours of ports which exist on device. A devlink port can be of an
> +embedded switch (eswitch) present on the device.
> +
> +A devlink port can be of 3 diffferent types.
> +
> +.. list-table:: List of devlink port types
> +   :widths: 23 90
> +
> +   * - Type
> +     - Description
> +   * - ``DEVLINK_PORT_TYPE_ETH``
> +     - This type is set for a devlink port when a physical link layer of the port
> +       is Ethernet.
> +   * - ``DEVLINK_PORT_TYPE_IB``
> +     - This type is set for a devlink port when a physical link layer of the port
> +       is InfiniBand.
> +   * - ``DEVLINK_PORT_TYPE_AUTO``
> +     - This type is indicated by the user when user prefers to set the port type
> +       to be automatically detected by the device driver.
> +
> +Devlink port can be of few different flavours described below.
> +
> +.. list-table:: List of devlink port flavours
> +   :widths: 33 90
> +
> +   * - Flavour
> +     - Description
> +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> +     - Any kind of port which is physically facing the user. This can be
> +       a eswitch physical port or any other physical port on the device.
> +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> +     - This indicates a CPU port.
> +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> +     - This indicates a interconnect port in a distributed switch architecture.
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> +     - This indicates an eswitch port representing PCI physical function(PF).
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> +     - This indicates an eswitch port representing PCI virtual function(VF).
> +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> +     - This indicates a virtual port facing the user.
> +
> +A devlink port may be for a controller consisting one or more PCI device(s).
> +A devlink instance holds ports of two types of controllers.
> +
> +(1) controller discovered on same system where eswitch resides
> +This is the case where PCI PF/VF of a controller and devlink eswitch
> +instance both are located on a single system.
> +
> +(2) controller located on external host system.
> +This is the case where a controller is located in one system and its
> +devlink eswitch ports are located in a different system.
> +
> +An example view of two controller systems::
> +
> +                 ---------------------------------------------------------
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
> +    | connect |  | -------                       -------                 |
> +    -----------  |     | controller_num=1 (no eswitch)                   |
> +                 ------|--------------------------------------------------
> +                 (internal wire)
> +                       |
> +                 ---------------------------------------------------------
> +                 | devlink eswitch ports and reps                        |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 |                                                       |
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +                 | | pf0 |______/________/       | pf1 |___/_______/     |
> +                 | -------                       -------                 |
> +                 |                                                       |
> +                 |  local controller_num=0 (eswitch)                     |
> +                 ---------------------------------------------------------
> +
> +Port function configuration
> +===========================
> +
> +When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
> +``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the port of a PCI function.
> +A user can configure the port function attributes before enumerating the
> +function. For example user may set the hardware address of the function
> +represented by the devlink port.
> diff --git a/Documentation/networking/devlink/index.rst
> b/Documentation/networking/devlink/index.rst
> index d82874760ae2..aab79667f97b 100644
> --- a/Documentation/networking/devlink/index.rst
> +++ b/Documentation/networking/devlink/index.rst
> @@ -18,6 +18,7 @@ general.
>     devlink-info
>     devlink-flash
>     devlink-params
> +   devlink-port
>     devlink-region
>     devlink-resource
>     devlink-reload
> --
> 2.26.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next v2] devlink: Add devlink port documentation
  2020-11-30 20:00 ` [PATCH net-next v2] " Parav Pandit
  2020-11-30 20:27   ` Keller, Jacob E
@ 2020-12-02  1:34   ` Jakub Kicinski
  2020-12-02  4:27     ` Parav Pandit
  1 sibling, 1 reply; 22+ messages in thread
From: Jakub Kicinski @ 2020-12-02  1:34 UTC (permalink / raw)
  To: Parav Pandit; +Cc: netdev, davem, jacob.e.keller, Jiri Pirko

On Mon, 30 Nov 2020 22:00:25 +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>

> +============
> +Devlink Port
> +============
> +
> +``devlink-port`` provides capability for a driver to expose various
> +flavours of ports which exist on device. A devlink port can be of an
> +embedded switch (eswitch) present on the device.

The wording is a little awkward here.

The first paragraph should clarify what object represents.

This just says it exposes ports that exist.

A better phrasing would be to say that these are ports of an eswitch,
in trivial case the only ports will be the physical ports of the card.

> +A devlink port can be of 3 diffferent types.

"can be of" repeated from the previous line

> +.. list-table:: List of devlink port types
> +   :widths: 23 90
> +
> +   * - Type
> +     - Description
> +   * - ``DEVLINK_PORT_TYPE_ETH``
> +     - This type is set for a devlink port when a physical link layer of the port

Is "physical link layer" a thing? I the common names are physical layer
and a (data) link layer. I don't think I've seen physical link layer,
or would know what it is...

> +       is Ethernet.
> +   * - ``DEVLINK_PORT_TYPE_IB``
> +     - This type is set for a devlink port when a physical link layer of the port
> +       is InfiniBand.
> +   * - ``DEVLINK_PORT_TYPE_AUTO``
> +     - This type is indicated by the user when user prefers to set the port type
> +       to be automatically detected by the device driver.

IMO type should be after flavor. Flavor is a higher level attribute,
only physical ports have a type.

> +Devlink port can be of few different flavours described below.
> +
> +.. list-table:: List of devlink port flavours
> +   :widths: 33 90
> +
> +   * - Flavour
> +     - Description
> +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> +     - Any kind of port which is physically facing the user. This can be

Hm. Not a great phrasing :(

It faces a physical networking layer. To me PCIe faces the user.

> +       a eswitch physical port or any other physical port on the device.
> +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> +     - This indicates a CPU port.

You need to mention this is a DSA-only thing.

> +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> +     - This indicates a interconnect port in a distributed switch architecture.

(DSA)

> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> +     - This indicates an eswitch port representing PCI physical function(PF).
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> +     - This indicates an eswitch port representing PCI virtual function(VF).
> +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> +     - This indicates a virtual port facing the user.

No idea what that means from the description. 

> +A devlink port may be for a controller consisting one or more PCI device(s).

Port can have multiple PCI devices?

> +A devlink instance holds ports of two types of controllers.
> +
> +(1) controller discovered on same system where eswitch resides
> +This is the case where PCI PF/VF of a controller and devlink eswitch
> +instance both are located on a single system.
> +
> +(2) controller located on external host system.
> +This is the case where a controller is located in one system and its
> +devlink eswitch ports are located in a different system.
> +
> +An example view of two controller systems::
> +
> +                 ---------------------------------------------------------
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
> +    | connect |  | -------                       -------                 |
> +    -----------  |     | controller_num=1 (no eswitch)                   |
> +                 ------|--------------------------------------------------
> +                 (internal wire)
> +                       |
> +                 ---------------------------------------------------------
> +                 | devlink eswitch ports and reps                        |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 |                                                       |
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +                 | | pf0 |______/________/       | pf1 |___/_______/     |
> +                 | -------                       -------                 |
> +                 |                                                       |
> +                 |  local controller_num=0 (eswitch)                     |
> +                 ---------------------------------------------------------
> +
> +Port function configuration
> +===========================
> +
> +When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
> +``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the port of a PCI function.
> +A user can configure the port function attributes before enumerating the
> +function. For example user may set the hardware address of the function
> +represented by the devlink port.
> diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
> index d82874760ae2..aab79667f97b 100644
> --- a/Documentation/networking/devlink/index.rst
> +++ b/Documentation/networking/devlink/index.rst
> @@ -18,6 +18,7 @@ general.
>     devlink-info
>     devlink-flash
>     devlink-params
> +   devlink-port
>     devlink-region
>     devlink-resource
>     devlink-reload


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next v2] devlink: Add devlink port documentation
  2020-12-02  1:34   ` Jakub Kicinski
@ 2020-12-02  4:27     ` Parav Pandit
  0 siblings, 0 replies; 22+ messages in thread
From: Parav Pandit @ 2020-12-02  4:27 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, davem, jacob.e.keller, Jiri Pirko



> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, December 2, 2020 7:05 AM
> 
> On Mon, 30 Nov 2020 22:00:25 +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>
> 
> > +============
> > +Devlink Port
> > +============
> > +
> > +``devlink-port`` provides capability for a driver to expose various
> > +flavours of ports which exist on device. A devlink port can be of an
> > +embedded switch (eswitch) present on the device.
> 
> The wording is a little awkward here.
> 
> The first paragraph should clarify what object represents.
> 
> This just says it exposes ports that exist.
> 
> A better phrasing would be to say that these are ports of an eswitch, in trivial
> case the only ports will be the physical ports of the card.
>
Sometimes it is eswitch port, sometimes its physical port on card, sometimes virtual port.
Will rephase.
 
> > +A devlink port can be of 3 diffferent types.
> 
> "can be of" repeated from the previous line
> 
Will rephase.

> > +.. list-table:: List of devlink port types
> > +   :widths: 23 90
> > +
> > +   * - Type
> > +     - Description
> > +   * - ``DEVLINK_PORT_TYPE_ETH``
> > +     - This type is set for a devlink port when a physical link layer
> > + of the port
> 
> Is "physical link layer" a thing? I the common names are physical layer and a
> (data) link layer. I don't think I've seen physical link layer, or would know what it
> is...
I will drop 'physical'. And just say link layer.

> 
> > +       is Ethernet.
> > +   * - ``DEVLINK_PORT_TYPE_IB``
> > +     - This type is set for a devlink port when a physical link layer of the port
> > +       is InfiniBand.
> > +   * - ``DEVLINK_PORT_TYPE_AUTO``
> > +     - This type is indicated by the user when user prefers to set the port type
> > +       to be automatically detected by the device driver.
> 
> IMO type should be after flavor. Flavor is a higher level attribute, only physical
> ports have a type.
> 
I will shift flavour up.
virtual port flavour also has type as eth/ib.

> > +Devlink port can be of few different flavours described below.
> > +
> > +.. list-table:: List of devlink port flavours
> > +   :widths: 33 90
> > +
> > +   * - Flavour
> > +     - Description
> > +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> > +     - Any kind of port which is physically facing the user. This can
> > + be
> 
> Hm. Not a great phrasing :(
> 
> It faces a physical networking layer. To me PCIe faces the user.
>
PCIe and physical networking ports, both are visible to users.
Former is usually inside the server, later is more frequently visible outside where networking cable is connected.
 
So, how about wording it as,

Any kind of physical networking port.

> > +       a eswitch physical port or any other physical port on the device.
> > +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> > +     - This indicates a CPU port.
> 
> You need to mention this is a DSA-only thing.
> 
Ok. will add.

> > +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> > +     - This indicates a interconnect port in a distributed switch architecture.
> 
> (DSA)
> 
> > +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> > +     - This indicates an eswitch port representing PCI physical function(PF).
> > +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> > +     - This indicates an eswitch port representing PCI virtual function(VF).
> > +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> > +     - This indicates a virtual port facing the user.
> 
> No idea what that means from the description.
>
Let me rephase it as below.

This indicates a virtual port for the virtual PCI device such as PCI VF.
 
> > +A devlink port may be for a controller consisting one or more PCI device(s).
> 
> Port can have multiple PCI devices?
For each PCI device there is a port as depicted in the diagram from commit 3a2d9588c4f7.

> 
> > +A devlink instance holds ports of two types of controllers.
> > +
> > +(1) controller discovered on same system where eswitch resides This
> > +is the case where PCI PF/VF of a controller and devlink eswitch
> > +instance both are located on a single system.
> > +
> > +(2) controller located on external host system.
> > +This is the case where a controller is located in one system and its
> > +devlink eswitch ports are located in a different system.
> > +
> > +An example view of two controller systems::
> > +
> > +                 ---------------------------------------------------------
> > +                 |                                                       |
> > +                 |           --------- ---------         ------- ------- |
> > +    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> > +    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> > +    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
> > +    | connect |  | -------                       -------                 |
> > +    -----------  |     | controller_num=1 (no eswitch)                   |
> > +                 ------|--------------------------------------------------
> > +                 (internal wire)
> > +                       |
> > +                 ---------------------------------------------------------
> > +                 | devlink eswitch ports and reps                        |
> > +                 | ----------------------------------------------------- |
> > +                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
> > +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> > +                 | ----------------------------------------------------- |
> > +                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
> > +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> > +                 | ----------------------------------------------------- |
> > +                 |                                                       |
> > +                 |                                                       |
> > +                 |           --------- ---------         ------- ------- |
> > +                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> > +                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> > +                 | | pf0 |______/________/       | pf1 |___/_______/     |
> > +                 | -------                       -------                 |
> > +                 |                                                       |
> > +                 |  local controller_num=0 (eswitch)                     |
> > +
> > + ---------------------------------------------------------
> > +
> > +Port function configuration
> > +===========================
> > +
> > +When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
> > +``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the port of a PCI function.
> > +A user can configure the port function attributes before enumerating
> > +the function. For example user may set the hardware address of the
> > +function represented by the devlink port.
> > diff --git a/Documentation/networking/devlink/index.rst
> > b/Documentation/networking/devlink/index.rst
> > index d82874760ae2..aab79667f97b 100644
> > --- a/Documentation/networking/devlink/index.rst
> > +++ b/Documentation/networking/devlink/index.rst
> > @@ -18,6 +18,7 @@ general.
> >     devlink-info
> >     devlink-flash
> >     devlink-params
> > +   devlink-port
> >     devlink-region
> >     devlink-resource
> >     devlink-reload


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH net-next v3] devlink: Add devlink port documentation
  2020-11-30 16:41 [PATCH net-next] devlink: Add devlink port documentation Parav Pandit
  2020-11-30 19:29 ` Jacob Keller
  2020-11-30 20:00 ` [PATCH net-next v2] " Parav Pandit
@ 2020-12-02 13:53 ` Parav Pandit
  2020-12-03  2:27   ` Randy Dunlap
  2020-12-03 18:02 ` [PATCH net-next v4] " Parav Pandit
  2020-12-07 22:13 ` [PATCH net-next v5] " Parav Pandit
  4 siblings, 1 reply; 22+ messages in thread
From: Parav Pandit @ 2020-12-02 13:53 UTC (permalink / raw)
  To: netdev, davem, kuba; +Cc: jacob.e.keller, Parav Pandit, Jiri Pirko

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>
---
Changelog:
v2->v3:
 - rephased many lines
 - first paragraph now describe devlink port
 - instead of saying PCI device/function, using PCI function every
   where
 - changed 'physical link layer' to 'link layer'
 - made devlink port type description more clear
 - made devlink port flavour description more clear
 - moved devlink port type table after port flavour
 - added description for the example diagram
 - describe CPU port that its linked to DSA
 - made devlink port description for eswitch port more clear
v1->v2:
 - Removed duplicate table entries for DEVLINK_PORT_FLAVOUR_VIRTUAL.
 - replaced 'consist of' to 'consisting'
 - changed 'can be' to 'can be of'
---
 .../networking/devlink/devlink-port.rst       | 111 ++++++++++++++++++
 Documentation/networking/devlink/index.rst    |   1 +
 2 files changed, 112 insertions(+)
 create mode 100644 Documentation/networking/devlink/devlink-port.rst

diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
new file mode 100644
index 000000000000..8407bbe9ce88
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-port.rst
@@ -0,0 +1,111 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+Devlink Port
+============
+
+``devlink-port`` is a port that exist on the device. A devlink port can
+be of one among many flavours. A devlink port flavour along with port
+attributes describe what a port represents.
+
+A device driver who intents to publish a devlink port, sets the
+devlink port attributes and registers the devlink port.
+
+Devlink port flavours are described below.
+
+.. list-table:: List of devlink port flavours
+   :widths: 33 90
+
+   * - Flavour
+     - Description
+   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
+     - Any kind of physical networking port. This can be a eswitch physical
+       port or any other physical port on the device.
+   * - ``DEVLINK_PORT_FLAVOUR_DSA``
+     - This indicates a DSA interconnect port.
+   * - ``DEVLINK_PORT_FLAVOUR_CPU``
+     - This indicates a CPU port applicable only to DSA.
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
+     - This indicates an eswitch port representing a networking port of
+       PCI physical function (PF).
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
+     - This indicates an eswitch port representing a networking port of
+       PCI virtual function (VF).
+   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
+     - This indicates a virtual port for the virtual PCI device such as PCI VF.
+
+A devlink port types are described below.
+
+.. list-table:: List of devlink port types
+   :widths: 23 90
+
+   * - Type
+     - Description
+   * - ``DEVLINK_PORT_TYPE_ETH``
+     - Driver should set this port type when a link layer of the port is Ethernet.
+   * - ``DEVLINK_PORT_TYPE_IB``
+     - Driver should set this port type when a link layer of the port is InfiniBand.
+   * - ``DEVLINK_PORT_TYPE_AUTO``
+     - This type is indicated by the user when user prefers to set the port type
+       to be automatically detected by the device driver.
+
+A controller consist of one or more PCI functions. Such PCI function can have one
+or more networking ports. A networking port of such PCI function is represented
+by the eswitch devlink port. A devlink instance holds ports of two types of
+controllers.
+
+(1) controller discovered on same system where eswitch resides:
+This is the case where PCI PF/VF of a controller and devlink eswitch
+instance both are located on a single system.
+
+(2) controller located on external host system.
+This is the case where a controller is located in one system and its
+devlink eswitch ports are located in a different system. Such controller
+is called external controller.
+
+An example view of two controller systems::
+
+In this example a controller which contains the eswitch is local controller
+with controller number = 0. The second is a external controller having
+controller number = 1. Eswitch devlink instance has representor devlink
+ports for the PCI functions of both the controllers.
+
+                 ---------------------------------------------------------
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
+    | connect |  | -------                       -------                 |
+    -----------  |     | controller_num=1 (no eswitch)                   |
+                 ------|--------------------------------------------------
+                 (internal wire)
+                       |
+                 ---------------------------------------------------------
+                 | devlink eswitch ports and reps                        |
+                 | ----------------------------------------------------- |
+                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 |                                                       |
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+                 | | pf0 |______/________/       | pf1 |___/_______/     |
+                 | -------                       -------                 |
+                 |                                                       |
+                 |  local controller_num=0 (eswitch)                     |
+                 ---------------------------------------------------------
+
+Port function configuration
+===========================
+
+When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
+``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the networking port of a
+PCI function. A user can configure the port function attributes before
+enumerating the function. For example user may set the hardware address of
+the function represented by the devlink port function.
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index d82874760ae2..aab79667f97b 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -18,6 +18,7 @@ general.
    devlink-info
    devlink-flash
    devlink-params
+   devlink-port
    devlink-region
    devlink-resource
    devlink-reload
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next v3] devlink: Add devlink port documentation
  2020-12-02 13:53 ` [PATCH net-next v3] " Parav Pandit
@ 2020-12-03  2:27   ` Randy Dunlap
  2020-12-03  5:06     ` Parav Pandit
  0 siblings, 1 reply; 22+ messages in thread
From: Randy Dunlap @ 2020-12-03  2:27 UTC (permalink / raw)
  To: Parav Pandit, netdev, davem, kuba; +Cc: jacob.e.keller, Jiri Pirko

Hi--

On 12/2/20 5:53 AM, 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>
> ---
> Changelog:
> v2->v3:
>  - rephased many lines

     rephrased

>  - first paragraph now describe devlink port
>  - instead of saying PCI device/function, using PCI function every
>    where
>  - changed 'physical link layer' to 'link layer'
>  - made devlink port type description more clear
>  - made devlink port flavour description more clear
>  - moved devlink port type table after port flavour
>  - added description for the example diagram
>  - describe CPU port that its linked to DSA
>  - made devlink port description for eswitch port more clear
> v1->v2:
>  - Removed duplicate table entries for DEVLINK_PORT_FLAVOUR_VIRTUAL.
>  - replaced 'consist of' to 'consisting'
>  - changed 'can be' to 'can be of'
> ---
>  .../networking/devlink/devlink-port.rst       | 111 ++++++++++++++++++
>  Documentation/networking/devlink/index.rst    |   1 +
>  2 files changed, 112 insertions(+)
>  create mode 100644 Documentation/networking/devlink/devlink-port.rst
> 
> diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
> new file mode 100644
> index 000000000000..8407bbe9ce88
> --- /dev/null
> +++ b/Documentation/networking/devlink/devlink-port.rst
> @@ -0,0 +1,111 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============
> +Devlink Port
> +============
> +
> +``devlink-port`` is a port that exist on the device. A devlink port can

                                   exists

> +be of one among many flavours. A devlink port flavour along with port
> +attributes describe what a port represents.
> +
> +A device driver who intents to publish a devlink port, sets the

                   that intends                         ^no comma

> +devlink port attributes and registers the devlink port.
> +
> +Devlink port flavours are described below.
> +
> +.. list-table:: List of devlink port flavours
> +   :widths: 33 90
> +
> +   * - Flavour
> +     - Description
> +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> +     - Any kind of physical networking port. This can be a eswitch physical

                                                            an

> +       port or any other physical port on the device.
> +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> +     - This indicates a DSA interconnect port.
> +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> +     - This indicates a CPU port applicable only to DSA.
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> +     - This indicates an eswitch port representing a networking port of
> +       PCI physical function (PF).
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> +     - This indicates an eswitch port representing a networking port of
> +       PCI virtual function (VF).
> +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> +     - This indicates a virtual port for the virtual PCI device such as PCI VF.
> +
> +A devlink port types are described below.

  The devlink port types

> +
> +.. list-table:: List of devlink port types
> +   :widths: 23 90
> +
> +   * - Type
> +     - Description
> +   * - ``DEVLINK_PORT_TYPE_ETH``
> +     - Driver should set this port type when a link layer of the port is Ethernet.
> +   * - ``DEVLINK_PORT_TYPE_IB``
> +     - Driver should set this port type when a link layer of the port is InfiniBand.
> +   * - ``DEVLINK_PORT_TYPE_AUTO``
> +     - This type is indicated by the user when user prefers to set the port type
> +       to be automatically detected by the device driver.
> +
> +A controller consist of one or more PCI functions. Such PCI function can have one

                consists

> +or more networking ports. A networking port of such PCI function is represented
> +by the eswitch devlink port. A devlink instance holds ports of two types of
> +controllers.
> +
> +(1) controller discovered on same system where eswitch resides:
> +This is the case where PCI PF/VF of a controller and devlink eswitch
> +instance both are located on a single system.
> +
> +(2) controller located on external host system.
> +This is the case where a controller is located in one system and its
> +devlink eswitch ports are located in a different system. Such controller
> +is called external controller.
> +
> +An example view of two controller systems::
> +
> +In this example a controller which contains the eswitch is local controller
> +with controller number = 0. The second is a external controller having
> +controller number = 1. Eswitch devlink instance has representor devlink
> +ports for the PCI functions of both the controllers.

I find that sentence confusing but I don't know how to fix it.

> +
> +                 ---------------------------------------------------------
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
> +    | connect |  | -------                       -------                 |
> +    -----------  |     | controller_num=1 (no eswitch)                   |
> +                 ------|--------------------------------------------------
> +                 (internal wire)
> +                       |
> +                 ---------------------------------------------------------
> +                 | devlink eswitch ports and reps                        |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 |                                                       |
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +                 | | pf0 |______/________/       | pf1 |___/_______/     |
> +                 | -------                       -------                 |
> +                 |                                                       |
> +                 |  local controller_num=0 (eswitch)                     |
> +                 ---------------------------------------------------------
> +
> +Port function configuration
> +===========================
> +
> +When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
> +``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the networking port of a
> +PCI function. A user can configure the port function attributes before
> +enumerating the function. For example user may set the hardware address of
> +the function represented by the devlink port function.

thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next v3] devlink: Add devlink port documentation
  2020-12-03  2:27   ` Randy Dunlap
@ 2020-12-03  5:06     ` Parav Pandit
  0 siblings, 0 replies; 22+ messages in thread
From: Parav Pandit @ 2020-12-03  5:06 UTC (permalink / raw)
  To: Randy Dunlap, netdev, davem, kuba; +Cc: jacob.e.keller, Jiri Pirko



> From: Randy Dunlap <rdunlap@infradead.org>
> Sent: Thursday, December 3, 2020 7:57 AM
> 
> Hi--
> 
> On 12/2/20 5:53 AM, 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>
> > ---
> > Changelog:
> > v2->v3:
> >  - rephased many lines
> 
>      rephrased
> 
> >  - first paragraph now describe devlink port
> >  - instead of saying PCI device/function, using PCI function every
> >    where
> >  - changed 'physical link layer' to 'link layer'
> >  - made devlink port type description more clear
> >  - made devlink port flavour description more clear
> >  - moved devlink port type table after port flavour
> >  - added description for the example diagram
> >  - describe CPU port that its linked to DSA
> >  - made devlink port description for eswitch port more clear
> > v1->v2:
> >  - Removed duplicate table entries for DEVLINK_PORT_FLAVOUR_VIRTUAL.
> >  - replaced 'consist of' to 'consisting'
> >  - changed 'can be' to 'can be of'
> > ---
> >  .../networking/devlink/devlink-port.rst       | 111 ++++++++++++++++++
> >  Documentation/networking/devlink/index.rst    |   1 +
> >  2 files changed, 112 insertions(+)
> >  create mode 100644 Documentation/networking/devlink/devlink-port.rst
> >
> > diff --git a/Documentation/networking/devlink/devlink-port.rst
> > b/Documentation/networking/devlink/devlink-port.rst
> > new file mode 100644
> > index 000000000000..8407bbe9ce88
> > --- /dev/null
> > +++ b/Documentation/networking/devlink/devlink-port.rst
> > @@ -0,0 +1,111 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +============
> > +Devlink Port
> > +============
> > +
> > +``devlink-port`` is a port that exist on the device. A devlink port
> > +can
> 
>                                    exists
> 
> > +be of one among many flavours. A devlink port flavour along with port
> > +attributes describe what a port represents.
> > +
> > +A device driver who intents to publish a devlink port, sets the
> 
>                    that intends                         ^no comma
> 
> > +devlink port attributes and registers the devlink port.
> > +
> > +Devlink port flavours are described below.
> > +
> > +.. list-table:: List of devlink port flavours
> > +   :widths: 33 90
> > +
> > +   * - Flavour
> > +     - Description
> > +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> > +     - Any kind of physical networking port. This can be a eswitch
> > + physical
> 
>                                                             an
> 
> > +       port or any other physical port on the device.
> > +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> > +     - This indicates a DSA interconnect port.
> > +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> > +     - This indicates a CPU port applicable only to DSA.
> > +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> > +     - This indicates an eswitch port representing a networking port of
> > +       PCI physical function (PF).
> > +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> > +     - This indicates an eswitch port representing a networking port of
> > +       PCI virtual function (VF).
> > +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> > +     - This indicates a virtual port for the virtual PCI device such as PCI VF.
> > +
> > +A devlink port types are described below.
> 
>   The devlink port types
> 
> > +
> > +.. list-table:: List of devlink port types
> > +   :widths: 23 90
> > +
> > +   * - Type
> > +     - Description
> > +   * - ``DEVLINK_PORT_TYPE_ETH``
> > +     - Driver should set this port type when a link layer of the port is Ethernet.
> > +   * - ``DEVLINK_PORT_TYPE_IB``
> > +     - Driver should set this port type when a link layer of the port is InfiniBand.
> > +   * - ``DEVLINK_PORT_TYPE_AUTO``
> > +     - This type is indicated by the user when user prefers to set the port type
> > +       to be automatically detected by the device driver.
> > +
> > +A controller consist of one or more PCI functions. Such PCI function
> > +can have one
> 
>                 consists
> 
> > +or more networking ports. A networking port of such PCI function is
> > +represented by the eswitch devlink port. A devlink instance holds
> > +ports of two types of controllers.
> > +
> > +(1) controller discovered on same system where eswitch resides:
> > +This is the case where PCI PF/VF of a controller and devlink eswitch
> > +instance both are located on a single system.
> > +
> > +(2) controller located on external host system.
> > +This is the case where a controller is located in one system and its
> > +devlink eswitch ports are located in a different system. Such
> > +controller is called external controller.
> > +
> > +An example view of two controller systems::
> > +
> > +In this example a controller which contains the eswitch is local
> > +controller with controller number = 0. The second is a external
> > +controller having controller number = 1. Eswitch devlink instance has
> > +representor devlink ports for the PCI functions of both the controllers.
> 
> I find that sentence confusing but I don't know how to fix it.
> 
Will rewrite it as below and also address above comments.

In this example, external controller (identified by controller number = 1) doesn't have eswitch.
Local controller (identified by controller number = 0) has the eswitch. Devlink instance on local
Controller has eswitch devlink ports representing networking ports for both the controllers.

> > +
> > +                 ---------------------------------------------------------
> > +                 |                                                       |
> > +                 |           --------- ---------         ------- ------- |
> > +    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> > +    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> > +    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
> > +    | connect |  | -------                       -------                 |
> > +    -----------  |     | controller_num=1 (no eswitch)                   |
> > +                 ------|--------------------------------------------------
> > +                 (internal wire)
> > +                       |
> > +                 ---------------------------------------------------------
> > +                 | devlink eswitch ports and reps                        |
> > +                 | ----------------------------------------------------- |
> > +                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
> > +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> > +                 | ----------------------------------------------------- |
> > +                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
> > +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> > +                 | ----------------------------------------------------- |
> > +                 |                                                       |
> > +                 |                                                       |
> > +                 |           --------- ---------         ------- ------- |
> > +                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> > +                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> > +                 | | pf0 |______/________/       | pf1 |___/_______/     |
> > +                 | -------                       -------                 |
> > +                 |                                                       |
> > +                 |  local controller_num=0 (eswitch)                     |
> > +
> > + ---------------------------------------------------------
> > +
> > +Port function configuration
> > +===========================
> > +
> > +When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
> > +``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the networking port of
> > +a PCI function. A user can configure the port function attributes
> > +before enumerating the function. For example user may set the
> > +hardware address of the function represented by the devlink port function.
> 
> thanks.
Thanks for the review Randy.

> --
> ~Randy


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH net-next v4] devlink: Add devlink port documentation
  2020-11-30 16:41 [PATCH net-next] devlink: Add devlink port documentation Parav Pandit
                   ` (2 preceding siblings ...)
  2020-12-02 13:53 ` [PATCH net-next v3] " Parav Pandit
@ 2020-12-03 18:02 ` Parav Pandit
  2020-12-03 20:31   ` Randy Dunlap
  2020-12-05 20:27   ` Jakub Kicinski
  2020-12-07 22:13 ` [PATCH net-next v5] " Parav Pandit
  4 siblings, 2 replies; 22+ messages in thread
From: Parav Pandit @ 2020-12-03 18:02 UTC (permalink / raw)
  To: netdev, davem, kuba; +Cc: jacob.e.keller, Parav Pandit, Jiri Pirko

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>
---
Changelog:
v3->v4:
 - changed 'exist' to 'exists'
 - added 'an' eswitch
 - changed 'can have one' to 'consists of'
 - changed 'who intents' to 'that intends'
 - removed unnecessary comma
 - rewrote description for the example diagram
 - changed 'controller consist of' to 'controller consists of'
v2->v3:
 - rephrased many lines
 - first paragraph now describe devlink port
 - instead of saying PCI device/function, using PCI function every
   where
 - changed 'physical link layer' to 'link layer'
 - made devlink port type description more clear
 - made devlink port flavour description more clear
 - moved devlink port type table after port flavour
 - added description for the example diagram
 - describe CPU port that its linked to DSA
 - made devlink port description for eswitch port more clear
v1->v2:
 - Removed duplicate table entries for DEVLINK_PORT_FLAVOUR_VIRTUAL.
 - replaced 'consist of' to 'consisting'
 - changed 'can be' to 'can be of'
---
 .../networking/devlink/devlink-port.rst       | 111 ++++++++++++++++++
 Documentation/networking/devlink/index.rst    |   1 +
 2 files changed, 112 insertions(+)
 create mode 100644 Documentation/networking/devlink/devlink-port.rst

diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
new file mode 100644
index 000000000000..ac18cb8041dc
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-port.rst
@@ -0,0 +1,111 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+Devlink Port
+============
+
+``devlink-port`` is a port that exists on the device. A devlink port can
+be of one among many flavours. A devlink port flavour along with port
+attributes describe what a port represents.
+
+A device driver that intends to publish a devlink port sets the
+devlink port attributes and registers the devlink port.
+
+Devlink port flavours are described below.
+
+.. list-table:: List of devlink port flavours
+   :widths: 33 90
+
+   * - Flavour
+     - Description
+   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
+     - Any kind of physical networking port. This can be an eswitch physical
+       port or any other physical port on the device.
+   * - ``DEVLINK_PORT_FLAVOUR_DSA``
+     - This indicates a DSA interconnect port.
+   * - ``DEVLINK_PORT_FLAVOUR_CPU``
+     - This indicates a CPU port applicable only to DSA.
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
+     - This indicates an eswitch port representing a networking port of
+       PCI physical function (PF).
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
+     - This indicates an eswitch port representing a networking port of
+       PCI virtual function (VF).
+   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
+     - This indicates a virtual port for the virtual PCI device such as PCI VF.
+
+Devlink port types are described below.
+
+.. list-table:: List of devlink port types
+   :widths: 23 90
+
+   * - Type
+     - Description
+   * - ``DEVLINK_PORT_TYPE_ETH``
+     - Driver should set this port type when a link layer of the port is Ethernet.
+   * - ``DEVLINK_PORT_TYPE_IB``
+     - Driver should set this port type when a link layer of the port is InfiniBand.
+   * - ``DEVLINK_PORT_TYPE_AUTO``
+     - This type is indicated by the user when user prefers to set the port type
+       to be automatically detected by the device driver.
+
+A controller consists of one or more PCI functions. Such PCI function consists
+of one or more networking ports. A networking port of such PCI function is
+represented by the eswitch devlink port. A devlink instance holds ports of two
+types of controllers.
+
+(1) controller discovered on same system where eswitch resides:
+This is the case where PCI PF/VF of a controller and devlink eswitch
+instance both are located on a single system.
+
+(2) controller located on external host system.
+This is the case where a controller is in one system and its devlink
+eswitch ports are in a different system. Such controller is called
+external controller.
+
+An example view of two controller systems::
+
+In this example, external controller (identified by controller number = 1)
+doesn't have eswitch. Local controller (identified by controller number = 0)
+has the eswitch. Devlink instance on local Controller has eswitch devlink
+ports representing networking ports for both the controllers.
+
+                 ---------------------------------------------------------
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
+    | connect |  | -------                       -------                 |
+    -----------  |     | controller_num=1 (no eswitch)                   |
+                 ------|--------------------------------------------------
+                 (internal wire)
+                       |
+                 ---------------------------------------------------------
+                 | devlink eswitch ports and reps                        |
+                 | ----------------------------------------------------- |
+                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 |                                                       |
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+                 | | pf0 |______/________/       | pf1 |___/_______/     |
+                 | -------                       -------                 |
+                 |                                                       |
+                 |  local controller_num=0 (eswitch)                     |
+                 ---------------------------------------------------------
+
+Port function configuration
+===========================
+
+When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
+``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the networking port of a
+PCI function. A user can configure the port function attributes before
+enumerating the function. For example user may set the hardware address of
+the function represented by the devlink port function.
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index d82874760ae2..aab79667f97b 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -18,6 +18,7 @@ general.
    devlink-info
    devlink-flash
    devlink-params
+   devlink-port
    devlink-region
    devlink-resource
    devlink-reload
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next v4] devlink: Add devlink port documentation
  2020-12-03 18:02 ` [PATCH net-next v4] " Parav Pandit
@ 2020-12-03 20:31   ` Randy Dunlap
  2020-12-05 20:27   ` Jakub Kicinski
  1 sibling, 0 replies; 22+ messages in thread
From: Randy Dunlap @ 2020-12-03 20:31 UTC (permalink / raw)
  To: Parav Pandit, netdev, davem, kuba; +Cc: jacob.e.keller, Jiri Pirko

Hi--

On 12/3/20 10:02 AM, 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>
> ---
> Changelog:
> v3->v4:
>  - changed 'exist' to 'exists'
>  - added 'an' eswitch
>  - changed 'can have one' to 'consists of'
>  - changed 'who intents' to 'that intends'
>  - removed unnecessary comma
>  - rewrote description for the example diagram
>  - changed 'controller consist of' to 'controller consists of'
> v2->v3:
>  - rephrased many lines
>  - first paragraph now describe devlink port
>  - instead of saying PCI device/function, using PCI function every
>    where
>  - changed 'physical link layer' to 'link layer'
>  - made devlink port type description more clear
>  - made devlink port flavour description more clear
>  - moved devlink port type table after port flavour
>  - added description for the example diagram
>  - describe CPU port that its linked to DSA
>  - made devlink port description for eswitch port more clear
> v1->v2:
>  - Removed duplicate table entries for DEVLINK_PORT_FLAVOUR_VIRTUAL.
>  - replaced 'consist of' to 'consisting'
>  - changed 'can be' to 'can be of'
> ---
>  .../networking/devlink/devlink-port.rst       | 111 ++++++++++++++++++
>  Documentation/networking/devlink/index.rst    |   1 +
>  2 files changed, 112 insertions(+)
>  create mode 100644 Documentation/networking/devlink/devlink-port.rst
> 
> diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
> new file mode 100644
> index 000000000000..ac18cb8041dc
> --- /dev/null
> +++ b/Documentation/networking/devlink/devlink-port.rst
> @@ -0,0 +1,111 @@

I find that this doc is now readable.  :)

Other people can comment on the technical details.

thanks.

-- 
~Randy
Acked-by: Randy Dunlap <rdunlap@infradead.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next v4] devlink: Add devlink port documentation
  2020-12-03 18:02 ` [PATCH net-next v4] " Parav Pandit
  2020-12-03 20:31   ` Randy Dunlap
@ 2020-12-05 20:27   ` Jakub Kicinski
  2020-12-07  4:46     ` Parav Pandit
  1 sibling, 1 reply; 22+ messages in thread
From: Jakub Kicinski @ 2020-12-05 20:27 UTC (permalink / raw)
  To: Parav Pandit; +Cc: netdev, davem, jacob.e.keller, Jiri Pirko

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.

?

> A devlink port can
> +be of one among many flavours. A devlink port flavour along with port
> +attributes describe what a port represents.
> +
> +A device driver that intends to publish a devlink port sets the
> +devlink port attributes and registers the devlink port.
> +
> +Devlink port flavours are described below.
> +
> +.. list-table:: List of devlink port flavours
> +   :widths: 33 90
> +
> +   * - Flavour
> +     - Description
> +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> +     - Any kind of physical networking port. This can be an eswitch physical
> +       port or any other physical port on the device.
> +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> +     - This indicates a DSA interconnect port.
> +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> +     - This indicates a CPU port applicable only to DSA.
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> +     - This indicates an eswitch port representing a networking port of
> +       PCI physical function (PF).
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> +     - This indicates an eswitch port representing a networking port of
> +       PCI virtual function (VF).
> +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> +     - This indicates a virtual port for the virtual PCI device such as PCI VF.
> +
> +Devlink port types are described below.

How about:

Physical ports can have different types based on the link layer:

> +.. list-table:: List of devlink port types
> +   :widths: 23 90
> +
> +   * - Type
> +     - Description
> +   * - ``DEVLINK_PORT_TYPE_ETH``
> +     - Driver should set this port type when a link layer of the port is Ethernet.
> +   * - ``DEVLINK_PORT_TYPE_IB``
> +     - Driver should set this port type when a link layer of the port is InfiniBand.

Please wrap at 80 chars.

> +   * - ``DEVLINK_PORT_TYPE_AUTO``
> +     - This type is indicated by the user when user prefers to set the port type
> +       to be automatically detected by the device driver.

How about:

This type is indicated by the user when driver should detect the port 
type automatically.

> +A controller consists of one or more PCI functions. 

This need some intro. Like:

PCI controllers
---------------

In most cases PCI devices will have only one controller, with
potentially multiple physical and virtual functions. Devices connected
to multiple CPUs and SmartNICs, however, may have multiple controllers.

> 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.

> A networking port of such PCI function is
> +represented by the eswitch devlink port.

What's eswitch devlink port? It was never defined.

> A devlink instance holds ports of two
> +types of controllers.

For devices with multiple controllers we can distinguish...

> +(1) controller discovered on same system where eswitch resides:
> +This is the case where PCI PF/VF of a controller and devlink eswitch
> +instance both are located on a single system.

How is eswitch located on a system? Eswitch is in the NIC

I think you should say refer to eswitch being controlled by a system.

> +(2) controller located on external host system.
> +This is the case where a controller is in one system and its devlink
> +eswitch ports are in a different system. Such controller is called
> +external controller.

> +An example view of two controller systems::
> +
> +In this example, external controller (identified by controller number = 1)
> +doesn't have eswitch. Local controller (identified by controller number = 0)
> +has the eswitch. Devlink instance on local Controller has eswitch devlink
> +ports representing networking ports for both the controllers.
> +
> +                 ---------------------------------------------------------
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
> +    | connect |  | -------                       -------                 |
> +    -----------  |     | controller_num=1 (no eswitch)                   |
> +                 ------|--------------------------------------------------
> +                 (internal wire)
> +                       |
> +                 ---------------------------------------------------------
> +                 | devlink eswitch ports and reps                        |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 |                                                       |
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +                 |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +                 | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +                 | | pf0 |______/________/       | pf1 |___/_______/     |
> +                 | -------                       -------                 |
> +                 |                                                       |
> +                 |  local controller_num=0 (eswitch)                     |
> +                 ---------------------------------------------------------
> +
> +Port function configuration
> +===========================
> +
> +When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
> +``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the networking port of a
> +PCI function. 

Networking port of a PCI function?

> A user can configure the port function attributes 

port function attributes?

> before
> +enumerating the function.

What does this mean? What does enumerate mean in this context?

> 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?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next v4] devlink: Add devlink port documentation
  2020-12-05 20:27   ` Jakub Kicinski
@ 2020-12-07  4:46     ` Parav Pandit
  2020-12-07 17:40       ` Jakub Kicinski
  0 siblings, 1 reply; 22+ messages in thread
From: Parav Pandit @ 2020-12-07  4:46 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, davem, jacob.e.keller, Jiri Pirko



> 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.

> 
> > A devlink port can
> > +be of one among many flavours. A devlink port flavour along with port
> > +attributes describe what a port represents.
> > +
> > +A device driver that intends to publish a devlink port sets the
> > +devlink port attributes and registers the devlink port.
> > +
> > +Devlink port types are described below.
> 
> How about:
> 
> Physical ports can have different types based on the link layer:
>
Ok. will add.
 
> > +.. list-table:: List of devlink port types
> > +   :widths: 23 90
> > +
> > +   * - Type
> > +     - Description
> > +   * - ``DEVLINK_PORT_TYPE_ETH``
> > +     - Driver should set this port type when a link layer of the port is Ethernet.
> > +   * - ``DEVLINK_PORT_TYPE_IB``
> > +     - Driver should set this port type when a link layer of the port is InfiniBand.
> 
> Please wrap at 80 chars.
> 
Ack.
> > +   * - ``DEVLINK_PORT_TYPE_AUTO``
> > +     - This type is indicated by the user when user prefers to set the port type
> > +       to be automatically detected by the device driver.
> 
> How about:
> 
> This type is indicated by the user when driver should detect the port type
> automatically.
> 
Will change.

> > +A controller consists of one or more PCI functions.
> 
> This need some intro. Like:
> 
> PCI controllers
> ---------------
> 
> In most cases PCI devices will have only one controller, with potentially multiple
> physical and virtual functions. Devices connected to multiple CPUs and
> SmartNICs, however, may have multiple controllers.
> 
> > 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.
 
> > 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).

> 
> > A devlink instance holds ports of two
> > +types of controllers.
> 
> For devices with multiple controllers we can distinguish...
> 
Yes, will change.

> > +(1) controller discovered on same system where eswitch resides:
> > +This is the case where PCI PF/VF of a controller and devlink eswitch
> > +instance both are located on a single system.
> 
> How is eswitch located on a system? Eswitch is in the NIC
>
Yes, I meant eswitch devlink instance and controller devlink instance are same.
 Will rephase.

> I think you should say refer to eswitch being controlled by a system.
> 
> > +(2) controller located on external host system.
> > +This is the case where a controller is in one system and its devlink
> > +eswitch ports are in a different system. Such controller is called
> > +external controller.
> 
> > +An example view of two controller systems::
> > +
> > +Port function configuration
> > +===========================
> > +
> > +When a port flavor is ``DEVLINK_PORT_FLAVOUR_PCI_PF`` or
> > +``DEVLINK_PORT_FLAVOUR_PCI_VF``, it represents the networking port of
> > +a PCI function.
> 
> Networking port of a PCI function?
> 
> > A user can configure the port function attributes
> 
> port function attributes?
> 
> > 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.

> > 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.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next v4] devlink: Add devlink port documentation
  2020-12-07  4:46     ` Parav Pandit
@ 2020-12-07 17:40       ` Jakub Kicinski
  2020-12-07 20:00         ` Parav Pandit
  0 siblings, 1 reply; 22+ messages in thread
From: Jakub Kicinski @ 2020-12-07 17:40 UTC (permalink / raw)
  To: Parav Pandit; +Cc: netdev, davem, jacob.e.keller, Jiri Pirko

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.

> > > 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.

> > > 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?

> > > 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?

> > > 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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next v4] devlink: Add devlink port documentation
  2020-12-07 17:40       ` Jakub Kicinski
@ 2020-12-07 20:00         ` Parav Pandit
  2020-12-07 20:12           ` Jakub Kicinski
  0 siblings, 1 reply; 22+ messages in thread
From: Parav Pandit @ 2020-12-07 20:00 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, davem, jacob.e.keller, Jiri Pirko



> 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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next v4] devlink: Add devlink port documentation
  2020-12-07 20:00         ` Parav Pandit
@ 2020-12-07 20:12           ` Jakub Kicinski
  2020-12-07 20:16             ` Parav Pandit
  0 siblings, 1 reply; 22+ messages in thread
From: Jakub Kicinski @ 2020-12-07 20:12 UTC (permalink / raw)
  To: Parav Pandit; +Cc: netdev, davem, jacob.e.keller, Jiri Pirko

On Mon, 7 Dec 2020 20:00:53 +0000 Parav Pandit wrote:
> > > > 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?

Yes, DSA/CPU port types are points of egress to the devlink instance.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next v4] devlink: Add devlink port documentation
  2020-12-07 20:12           ` Jakub Kicinski
@ 2020-12-07 20:16             ` Parav Pandit
  0 siblings, 0 replies; 22+ messages in thread
From: Parav Pandit @ 2020-12-07 20:16 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, davem, jacob.e.keller, Jiri Pirko



> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Tuesday, December 8, 2020 1:43 AM
> 
> On Mon, 7 Dec 2020 20:00:53 +0000 Parav Pandit wrote:
> > > > > 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?
> 
> Yes, DSA/CPU port types are points of egress to the devlink instance.
Ok. got it. Thanks.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH net-next v5] devlink: Add devlink port documentation
  2020-11-30 16:41 [PATCH net-next] devlink: Add devlink port documentation Parav Pandit
                   ` (3 preceding siblings ...)
  2020-12-03 18:02 ` [PATCH net-next v4] " Parav Pandit
@ 2020-12-07 22:13 ` Parav Pandit
  2020-12-08  1:14   ` Randy Dunlap
  4 siblings, 1 reply; 22+ messages in thread
From: Parav Pandit @ 2020-12-07 22:13 UTC (permalink / raw)
  To: netdev, davem, kuba; +Cc: jacob.e.keller, Parav Pandit, Jiri Pirko

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>
---
Changelog:
v4->v5:
 - described logically ingress and egress point of devlink port
 - removed networking from devlink port description
 - rephrased port type description
 - introdue PCI controller section and description
 - rephrased controller, device, function description
 - removed confusing eswitch to system wording
 - rephrased port function description
 - added example of mac address in port function attribute description
v3->v4:
 - changed 'exist' to 'exists'
 - added 'an' eswitch
 - changed 'can have one' to 'consists of'
 - changed 'who intents' to 'that intends'
 - removed unnecessary comma
 - rewrote description for the example diagram
 - changed 'controller consist of' to 'controller consists of'
v2->v3:
 - rephrased many lines
 - first paragraph now describe devlink port
 - instead of saying PCI device/function, using PCI function every
   where
 - changed 'physical link layer' to 'link layer'
 - made devlink port type description more clear
 - made devlink port flavour description more clear
 - moved devlink port type table after port flavour
 - added description for the example diagram
 - describe CPU port that its linked to DSA
 - made devlink port description for eswitch port more clear
v1->v2:
 - Removed duplicate table entries for DEVLINK_PORT_FLAVOUR_VIRTUAL.
 - replaced 'consist of' to 'consisting'
 - changed 'can be' to 'can be of'
---
 .../networking/devlink/devlink-port.rst       | 116 ++++++++++++++++++
 Documentation/networking/devlink/index.rst    |   1 +
 2 files changed, 117 insertions(+)
 create mode 100644 Documentation/networking/devlink/devlink-port.rst

diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
new file mode 100644
index 000000000000..dce87d2c07ac
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-port.rst
@@ -0,0 +1,116 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+Devlink Port
+============
+
+``devlink-port`` is a port that exists on the device. It has a logically
+separate ingress/egress point of the device. A devlink port can be of one
+among many flavours. A devlink port flavour along with port attributes
+describe what a port represents.
+
+A device driver that intends to publish a devlink port sets the
+devlink port attributes and registers the devlink port.
+
+Devlink port flavours are described below.
+
+.. list-table:: List of devlink port flavours
+   :widths: 33 90
+
+   * - Flavour
+     - Description
+   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
+     - Any kind of physical networking port. This can be an eswitch physical
+       port or any other physical port on the device.
+   * - ``DEVLINK_PORT_FLAVOUR_DSA``
+     - This indicates a DSA interconnect port.
+   * - ``DEVLINK_PORT_FLAVOUR_CPU``
+     - This indicates a CPU port applicable only to DSA.
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
+     - This indicates an eswitch port representing a networking port of
+       PCI physical function (PF).
+   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
+     - This indicates an eswitch port representing a networking port of
+       PCI virtual function (VF).
+   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
+     - This indicates a virtual port for the virtual PCI device such as PCI VF.
+
+Devlink port can have a different type based on the link layer described below.
+
+.. list-table:: List of devlink port types
+   :widths: 23 90
+
+   * - Type
+     - Description
+   * - ``DEVLINK_PORT_TYPE_ETH``
+     - Driver should set this port type when a link layer of the port is
+       Ethernet.
+   * - ``DEVLINK_PORT_TYPE_IB``
+     - Driver should set this port type when a link layer of the port is
+       InfiniBand.
+   * - ``DEVLINK_PORT_TYPE_AUTO``
+     - This type is indicated by the user when driver should detect the port
+       type automatically.
+
+PCI controllers
+---------------
+In most cases PCI device has only one controller. A controller consists of
+potentially multiple physical and virtual functions. Such PCI function consists
+of one or more ports. This port of the function is represented by the devlink
+eswitch port.
+
+A PCI Device connected to multiple CPUs or multiple PCI root complex or
+SmartNIC, however, may have multiple controllers. For a device with multiple
+controllers, each controller is distinguished by a unique controller number.
+An eswitch on the PCI device may suppport ports of multiple controllers. 
+
+An example view of two controller systems::
+
+In this example, external controller (identified by controller number = 1)
+doesn't have eswitch. Local controller (identified by controller number = 0)
+has the eswitch. Devlink instance on local controller has eswitch devlink
+ports representing ports for both the controllers.
+
+                 ---------------------------------------------------------
+                 |                                                       |
+                 |           --------- ---------         ------- ------- |
+    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
+    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
+    | connect |  | -------                       -------                 |
+    -----------  |     | controller_num=1 (no eswitch)                   |
+                 ------|--------------------------------------------------
+                 (internal wire)
+                       |
+                 ---------------------------------------------------------
+                 | devlink eswitch ports and reps                        |
+                 | ----------------------------------------------------- |
+                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
+                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
+                 | ----------------------------------------------------- |
+                 |                                                       |
+                 |                                                       |
+    -----------  |           --------- ---------         ------- ------- |
+    | smartNIC|  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
+    | pci rc  |==| -------   ----/---- ---/----- ------- ---/--- ---/--- |
+    | connect |  | | pf0 |______/________/       | pf1 |___/_______/     |
+    -----------  | -------                       -------                 |
+                 |                                                       |
+                 |  local controller_num=0 (eswitch)                     |
+                 ---------------------------------------------------------
+
+Port function configuration
+===========================
+
+A user can configure the port function attribute before enumerating the
+PCI function. Usually it means, user should configure port function attribute
+before a bus specific device for the function is created. However, when
+SRIOV is enabled, virtual function devices are created on the PCI bus.
+Hence, function attribute should be configured before binding virtual
+function device to the driver.
+
+User may set the hardware address of the function represented by the devlink
+port function. For Ethernet port function this means a MAC address.
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index d82874760ae2..aab79667f97b 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -18,6 +18,7 @@ general.
    devlink-info
    devlink-flash
    devlink-params
+   devlink-port
    devlink-region
    devlink-resource
    devlink-reload
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next v5] devlink: Add devlink port documentation
  2020-12-07 22:13 ` [PATCH net-next v5] " Parav Pandit
@ 2020-12-08  1:14   ` Randy Dunlap
  2020-12-08  5:59     ` Parav Pandit
  2020-12-08  6:51     ` Parav Pandit
  0 siblings, 2 replies; 22+ messages in thread
From: Randy Dunlap @ 2020-12-08  1:14 UTC (permalink / raw)
  To: Parav Pandit, netdev, davem, kuba; +Cc: jacob.e.keller, Jiri Pirko

Hi--

On 12/7/20 2:13 PM, 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>
> ---
> Changelog:
> v4->v5:
>  - described logically ingress and egress point of devlink port
>  - removed networking from devlink port description
>  - rephrased port type description
>  - introdue PCI controller section and description

     introduce

>  - rephrased controller, device, function description
>  - removed confusing eswitch to system wording
>  - rephrased port function description
>  - added example of mac address in port function attribute description

> ---
>  .../networking/devlink/devlink-port.rst       | 116 ++++++++++++++++++
>  Documentation/networking/devlink/index.rst    |   1 +
>  2 files changed, 117 insertions(+)
>  create mode 100644 Documentation/networking/devlink/devlink-port.rst
> 
> diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
> new file mode 100644
> index 000000000000..dce87d2c07ac
> --- /dev/null
> +++ b/Documentation/networking/devlink/devlink-port.rst
> @@ -0,0 +1,116 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============
> +Devlink Port
> +============
> +
> +``devlink-port`` is a port that exists on the device. It has a logically
> +separate ingress/egress point of the device. A devlink port can be of one

                                                          port can be any one
   of many flavours.

> +among many flavours. A devlink port flavour along with port attributes
> +describe what a port represents.
> +
> +A device driver that intends to publish a devlink port sets the
> +devlink port attributes and registers the devlink port.
> +
> +Devlink port flavours are described below.
> +
> +.. list-table:: List of devlink port flavours
> +   :widths: 33 90
> +
> +   * - Flavour
> +     - Description
> +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> +     - Any kind of physical networking port. This can be an eswitch physical
> +       port or any other physical port on the device.
> +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> +     - This indicates a DSA interconnect port.
> +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> +     - This indicates a CPU port applicable only to DSA.
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> +     - This indicates an eswitch port representing a networking port of
> +       PCI physical function (PF).
> +   * - ``DEVLINK_PORT_FLAVOUR_PCI_VF``
> +     - This indicates an eswitch port representing a networking port of
> +       PCI virtual function (VF).
> +   * - ``DEVLINK_PORT_FLAVOUR_VIRTUAL``
> +     - This indicates a virtual port for the virtual PCI device such as PCI VF.
> +
> +Devlink port can have a different type based on the link layer described below.
> +
> +.. list-table:: List of devlink port types
> +   :widths: 23 90
> +
> +   * - Type
> +     - Description
> +   * - ``DEVLINK_PORT_TYPE_ETH``
> +     - Driver should set this port type when a link layer of the port is
> +       Ethernet.
> +   * - ``DEVLINK_PORT_TYPE_IB``
> +     - Driver should set this port type when a link layer of the port is
> +       InfiniBand.
> +   * - ``DEVLINK_PORT_TYPE_AUTO``
> +     - This type is indicated by the user when driver should detect the port
> +       type automatically.
> +
> +PCI controllers
> +---------------
> +In most cases PCI device has only one controller. A controller consists of

either           PCI devices have
or               a PCI device has

> +potentially multiple physical and virtual functions. Such PCI function consists
> +of one or more ports. This port of the function is represented by the devlink
> +eswitch port.
> +
> +A PCI Device connected to multiple CPUs or multiple PCI root complex or

                                                                complexes

> +SmartNIC, however, may have multiple controllers. For a device with multiple
> +controllers, each controller is distinguished by a unique controller number.
> +An eswitch on the PCI device may suppport ports of multiple controllers. 

                                    support

> +
> +An example view of two controller systems::

I think that this is

  An example view of a two-controller system::

instead of 2 controller systems.  ?

> +
> +In this example, external controller (identified by controller number = 1)
> +doesn't have eswitch. Local controller (identified by controller number = 0)
> +has the eswitch. Devlink instance on local controller has eswitch devlink
> +ports representing ports for both the controllers.
> +
> +                 ---------------------------------------------------------
> +                 |                                                       |
> +                 |           --------- ---------         ------- ------- |
> +    -----------  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +    | server  |  | -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +    | pci rc  |=== | pf0 |______/________/       | pf1 |___/_______/     |
> +    | connect |  | -------                       -------                 |
> +    -----------  |     | controller_num=1 (no eswitch)                   |
> +                 ------|--------------------------------------------------
> +                 (internal wire)
> +                       |
> +                 ---------------------------------------------------------
> +                 | devlink eswitch ports and reps                        |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 | ctrl-0 |ctrl-0 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 | |ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 | ctrl-1 |ctrl-1 | |
> +                 | |pf0    | pf0vfN | pf0sfN | pf1    | pf1vfN |pf1sfN | |
> +                 | ----------------------------------------------------- |
> +                 |                                                       |
> +                 |                                                       |
> +    -----------  |           --------- ---------         ------- ------- |
> +    | smartNIC|  |           | vf(s) | | sf(s) |         |vf(s)| |sf(s)| |
> +    | pci rc  |==| -------   ----/---- ---/----- ------- ---/--- ---/--- |
> +    | connect |  | | pf0 |______/________/       | pf1 |___/_______/     |
> +    -----------  | -------                       -------                 |
> +                 |                                                       |
> +                 |  local controller_num=0 (eswitch)                     |
> +                 ---------------------------------------------------------
> +
> +Port function configuration
> +===========================
> +
> +A user can configure the port function attribute before enumerating the
> +PCI function. Usually it means, user should configure port function attribute
> +before a bus specific device for the function is created. However, when
> +SRIOV is enabled, virtual function devices are created on the PCI bus.
> +Hence, function attribute should be configured before binding virtual
> +function device to the driver.
> +
> +User may set the hardware address of the function represented by the devlink
> +port function. For Ethernet port function this means a MAC address.


-- 
~Randy


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next v5] devlink: Add devlink port documentation
  2020-12-08  1:14   ` Randy Dunlap
@ 2020-12-08  5:59     ` Parav Pandit
  2020-12-08  6:51     ` Parav Pandit
  1 sibling, 0 replies; 22+ messages in thread
From: Parav Pandit @ 2020-12-08  5:59 UTC (permalink / raw)
  To: Randy Dunlap, netdev, davem, kuba; +Cc: jacob.e.keller, Jiri Pirko

Hi Randy,

> From: Randy Dunlap <rdunlap@infradead.org>
> Sent: Tuesday, December 8, 2020 6:45 AM
> 
> > +An eswitch on the PCI device may suppport ports of multiple controllers.
> 
>                                     support
> 
> > +
> > +An example view of two controller systems::
> 
> I think that this is
> 
>   An example view of a two-controller system::
> 
> instead of 2 controller systems.  ?

Rephrasing it as "An example view of a system with two controllers" along with other comments.
Since Saeed is sending the sf patchset which also needs port documentation, I am sending v6 of this patch along with the change log in the subfunction patchset.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next v5] devlink: Add devlink port documentation
  2020-12-08  1:14   ` Randy Dunlap
  2020-12-08  5:59     ` Parav Pandit
@ 2020-12-08  6:51     ` Parav Pandit
  1 sibling, 0 replies; 22+ messages in thread
From: Parav Pandit @ 2020-12-08  6:51 UTC (permalink / raw)
  To: Randy Dunlap, netdev, davem, kuba; +Cc: jacob.e.keller, Jiri Pirko



> From: Randy Dunlap <rdunlap@infradead.org>
> Sent: Tuesday, December 8, 2020 6:45 AM
> 
> Hi--
> 
> On 12/7/20 2:13 PM, 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>
> > ---
> > Changelog:
> > v4->v5:
> >  - described logically ingress and egress point of devlink port
> >  - removed networking from devlink port description
> >  - rephrased port type description
> >  - introdue PCI controller section and description
> 
>      introduce
> 
> >  - rephrased controller, device, function description
> >  - removed confusing eswitch to system wording
> >  - rephrased port function description
> >  - added example of mac address in port function attribute description
> 
> > ---
> >  .../networking/devlink/devlink-port.rst       | 116 ++++++++++++++++++
> >  Documentation/networking/devlink/index.rst    |   1 +
> >  2 files changed, 117 insertions(+)
> >  create mode 100644 Documentation/networking/devlink/devlink-port.rst
> >
> > diff --git a/Documentation/networking/devlink/devlink-port.rst
> > b/Documentation/networking/devlink/devlink-port.rst
> > new file mode 100644
> > index 000000000000..dce87d2c07ac
> > --- /dev/null
> > +++ b/Documentation/networking/devlink/devlink-port.rst
> > @@ -0,0 +1,116 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +============
> > +Devlink Port
> > +============
> > +
> > +``devlink-port`` is a port that exists on the device. It has a
> > +logically separate ingress/egress point of the device. A devlink port
> > +can be of one
> 
>                                                           port can be any one
>    of many flavours.
> 
> > +among many flavours. A devlink port flavour along with port
> > +attributes describe what a port represents.
> > +
> > +A device driver that intends to publish a devlink port sets the
> > +devlink port attributes and registers the devlink port.
> > +
> > +Devlink port flavours are described below.
> > +
> > +.. list-table:: List of devlink port flavours
> > +   :widths: 33 90
> > +
> > +   * - Flavour
> > +     - Description
> > +   * - ``DEVLINK_PORT_FLAVOUR_PHYSICAL``
> > +     - Any kind of physical networking port. This can be an eswitch physical
> > +       port or any other physical port on the device.
> > +   * - ``DEVLINK_PORT_FLAVOUR_DSA``
> > +     - This indicates a DSA interconnect port.
> > +   * - ``DEVLINK_PORT_FLAVOUR_CPU``
> > +     - This indicates a CPU port applicable only to DSA.
> > +   * - ``DEVLINK_PORT_FLAVOUR_PCI_PF``
> > +     - This indicates an eswitch port representing a networking port of
> > +       PCI physical function (PF).
I forgot to remove 'networking' from the networking port here.
Will include this fix this in v6 in Saeed's subfunction patchset along with addressing Randy's comments.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-12-08  6:52 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-30 16:41 [PATCH net-next] devlink: Add devlink port documentation Parav Pandit
2020-11-30 19:29 ` Jacob Keller
2020-11-30 19:51   ` Parav Pandit
2020-11-30 20:00 ` [PATCH net-next v2] " Parav Pandit
2020-11-30 20:27   ` Keller, Jacob E
2020-12-02  1:34   ` Jakub Kicinski
2020-12-02  4:27     ` Parav Pandit
2020-12-02 13:53 ` [PATCH net-next v3] " Parav Pandit
2020-12-03  2:27   ` Randy Dunlap
2020-12-03  5:06     ` Parav Pandit
2020-12-03 18:02 ` [PATCH net-next v4] " Parav Pandit
2020-12-03 20:31   ` Randy Dunlap
2020-12-05 20:27   ` Jakub Kicinski
2020-12-07  4:46     ` Parav Pandit
2020-12-07 17:40       ` Jakub Kicinski
2020-12-07 20:00         ` Parav Pandit
2020-12-07 20:12           ` Jakub Kicinski
2020-12-07 20:16             ` Parav Pandit
2020-12-07 22:13 ` [PATCH net-next v5] " Parav Pandit
2020-12-08  1:14   ` Randy Dunlap
2020-12-08  5:59     ` Parav Pandit
2020-12-08  6:51     ` Parav Pandit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).