DPDK patches and discussions
 help / color / mirror / Atom feed
From: Yuanhan Liu <yliu@fridaylinux.org>
To: Remy Horton <remy.horton@intel.com>
Cc: dev@dpdk.org, John McNamara <john.mcnamara@intel.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Jingjing Wu <jingjing.wu@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Shahaf Shuler <shahafs@mellanox.com>,
	Olga Shern <olgas@mellanox.com>,
	Alex Rosenbaum <alexr@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH v4 0/5] lib: add Port Representors
Date: Thu, 11 Jan 2018 11:18:20 +0800	[thread overview]
Message-ID: <20180111031820.GR29540@yliu-mob> (raw)
In-Reply-To: <20180108143720.7994-1-remy.horton@intel.com>

On Mon, Jan 08, 2018 at 02:37:15PM +0000, Remy Horton wrote:
> Port Representors provide a logical presentation in DPDK of VF (virtual 
> function) ports for the purposes of control and monitoring. Each port 
> representor device represents a single VF and is associated with it's 
> parent physical function (PF) PMD which provides the back-end hooks for 
> the representor device ops and defines the control domain to which that 
> port belongs. This allows to use existing DPDK APIs to monitor and control 
> the port without the need to create and maintain VF specific APIs.

Firstly, I don't object this model. More precisely, I don't object for
introducing a port to control the VFs. I even like this idea a bit, for
at least it could get rid of some PMD specific APIs, say
rte_pmd_i40e_set_vf_mac_addr, etc.

However, I don't quite like this design, for a simple reason, it makes
things way more complex:

- new APIs are introduced.
- new virtual PMD is required
- broker concept


I was thinking below should work:

- Use the devargs for enabling the port (or VF) representor model.
  For example: -w 04:00.0,vf_ports=0-3

  The vf_ports is for telling the driver we are going to create VF
  representors for VF 0 to VF 3. It could be a port mask like what
  this patchset does.

- Then inside the driver (say, i40e)

  * allocate 1 DPDK ethdev port
  * allocate 4 DPDK ethdev port, one for each VF specified by
    the vf_ports option. And these are VF representors.
  * link the VF representors to the right VF port
    For example, for i40e, it should be (from patch 3):
        vf = &pf->vfs[representor->vport_id];
  * set the proper ethdev callbacks for the VF representors.

As you can see, none of above complexity is introduced.

Probably you might find something are missing:

- to identify the vf rep port for a specific VF.

  Which could be addressed by the new devargs syntax we are proposing:
  http://dpdk.org/ml/archives/dev/2017-December/084234.html

  For example, the right VF rep port could be identified by below devarg:
      bus=pci,id=04:00.0/class=eth,vf_id=0

- the ability of hotplug

  I think it could also be addressed by the new devargs syntax, also by
  re-using the rte_eth_dev_attach() function:

      rte_eth_dev_attach("bus=pci,id=04:00.0/class=eth,vf_id=4", &port_id);
  

Thoughts?

	--yliu
> 
> +-----------------------------+   +---------------+  +---------------+
> |        Control Plane        |   |   Data Plane  |  |   Data Plane  |
> |         Application         |   |   Application |  |   Application |
> +-----------------------------+   +---------------+  +---------------+
> |         eth dev api         |   |  eth dev api  |  |  eth dev api  |
> +-----------------------------+   +---------------+  +---------------+
> +-------+  +-------+  +-------+   +---------------+  +---------------+
> |  PF0  |  | Port  |  | Port  |   |    VF0 PMD    |  |    VF0 PMD    |
> |  PMD  <--+ Rep 0 |  | Rep 1 |   +---------------+  +------+--------+
> |       |  | PMD   |  | PMD   |                             |
> +---+--^+  +-------+  +-+-----+                             |
>     |  |                |  |                                |
>     |  +----------------+  |                                |
>     |                      |                                |
>     |                      |                                |
> +--------------------------------+                          |
> |   |  HW (logical view)   |     |                          |
> | --+------+ +-------+ +---+---+ |                          |
> | |   PF   | |  VF0  | |  VF1  | |                          |
> | |        | |       | |       +----------------------------+
> | +--------+ +-------+ +-------+ |
> | +----------------------------+ |
> | |        VEB                 | |
> | +----------------------------+ |
> | +--------+                     |
> | |  Port  |                     |
> | |   0    |                     |
> | +--------+                     |
> +--------------------------------+
> 
> The figure above shows a deployment where the PF is bound to a DPDK control
> plane application which uses representor ports to manage the configuration and
> monitoring of it's VF ports. Each virtual function is represented in the
> application by a representor port PMD which enables control of the corresponding
> VF through eth dev APIs on the representor PMD such as:
> 
> - void rte_eth_promiscuous_enable(uint8_t port_id);
> - void rte_eth_promiscuous_disable(uint8_t port_id);
> - void rte_eth_allmulticast_enable(uint8_t port_id);
> - void rte_eth_allmulticast_disable(uint8_t port_id);
> - int rte_eth_dev_mac_addr_add(uint8_t port, struct ether_addr *mac_addr,
> 	uint32_t pool);
> - int rte_eth_dev_set_vlan_offload(uint8_t port_id, int offload_mask);
> 
> as well as monitoring through API's like
> 
> - void rte_eth_link_get(uint8_t port_id, struct rte_eth_link *link);
> - int rte_eth_stats_get(uint8_t port_id, struct rte_eth_stats *stats);
> 
> The port representor infrastructure is enabled through a single common, device
> independent, virtual PMD whos context is initialized and enabled through a
> broker instance running within the context of the physical function device
> driver.
> 
> +-------------------------+       +-------------------------+
> |        rte_ethdev       |       |       rte_ethdev        |
> +-------------------------+       +-------------------------+
> |  Physical Function PMD  |       |  Port Reperesentor PMD  |
> |         +-------------+ |       | +---------+ +---------+ |
> |         | Representor | |       | | dev_data| | dev_ops | |
> |         |    Broker   | |       | +----+----+ +----+----+ |
> |         | +---------+ | |       +------|-----------|------+
> |         | | VF Port | | |              |           |
> |         | | Context +------------------+           |
> |         | +---------+ | |                          |
> |         | +---------+ | |                          |
> |         | | Handler +------------------------------+
> |         | |   Ops   | | |
> |         | +---------+ | |
> |         +-------------+ |
> +-------------------------+
> 
> Creation of representor ports can be achieved either through the --vdev EAL
> option or through the rte_vdev_init() API. Each port representor requires the
> BDF of it's parent PF and the Virtual Function ID of the port which the
> representor will support. During initialization of the representor PMD, it calls
> the broker API to register itself with the PF PMD and to get it's context
> configured which includes the setting up of it's context and ops function
> handlers.
> 
> As the port representor model is based around the paradigm of using standard
> port based APIs, it will allow future expansion of functionality without the
> need to add new APIs. For example it should be possible to support configuration
> of egress QoS parameters using existing TM APIs by extending the port
> representor PMD/broker infrastructure.
> 
> Changes in v2:
> * Rebased to DPDK 17.11
> 
> Changes in v3:
> * Removed RTE_ETH_DEV_REPRESENTOR_PORT define
> * Removed switch_domain from struct rte_eth_dev_info
>   (to be reintroduced seperately when required).
> * Port Representor PMD functionality merged into main library
> * Removed functions from .map file that are not for use by applications
> * Some minor bugfixes (uninitalised variables & NULL terminators)
> * Use of SPDX licence headers in new files
> * Seperate headers for PMD and application
> * SPDX-License-Identifier in new files
> * Added test-pmd representor add/del commands
> 
> Changes in v4:
> * Added documentation
> * Rebased to a12f22678915
> 
> 
> Remy Horton (5):
>   lib: add Port Representor library
>   eal: add Port Representor command-line option
>   drivers/net/i40e: add Port Representor functionality
>   drivers/net/ixgbe: add Port Representor functionality
>   app/test-pmd: add Port Representor commands
> 
>  MAINTAINERS                                        |  12 +-
>  app/test-pmd/cmdline.c                             |  88 ++++
>  config/common_base                                 |   5 +
>  doc/api/doxy-api-index.md                          |   3 +-
>  doc/api/doxy-api.conf                              |   1 +
>  doc/guides/prog_guide/index.rst                    |   1 +
>  doc/guides/prog_guide/representor_lib.rst          |  62 +++
>  doc/guides/rel_notes/release_18_02.rst             |   9 +
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst        |  25 ++
>  drivers/net/i40e/Makefile                          |   1 +
>  drivers/net/i40e/i40e_ethdev.c                     |  16 +
>  drivers/net/i40e/i40e_ethdev.h                     |   1 +
>  drivers/net/i40e/i40e_prep_ops.c                   | 495 +++++++++++++++++++++
>  drivers/net/i40e/i40e_prep_ops.h                   |  15 +
>  drivers/net/i40e/rte_pmd_i40e.c                    |  47 ++
>  drivers/net/i40e/rte_pmd_i40e.h                    |  18 +
>  drivers/net/ixgbe/Makefile                         |   1 +
>  drivers/net/ixgbe/ixgbe_ethdev.c                   |  22 +-
>  drivers/net/ixgbe/ixgbe_ethdev.h                   |   5 +
>  drivers/net/ixgbe/ixgbe_prep_ops.c                 | 259 +++++++++++
>  drivers/net/ixgbe/ixgbe_prep_ops.h                 |  15 +
>  lib/Makefile                                       |   3 +
>  lib/librte_eal/bsdapp/eal/eal.c                    |   6 +
>  lib/librte_eal/common/eal_common_options.c         |   1 +
>  lib/librte_eal/common/eal_internal_cfg.h           |   2 +
>  lib/librte_eal/common/eal_options.h                |   2 +
>  lib/librte_eal/common/include/rte_eal.h            |   8 +
>  lib/librte_eal/linuxapp/eal/eal.c                  |   9 +
>  lib/librte_representor/Makefile                    |  26 ++
>  lib/librte_representor/rte_port_representor.c      | 326 ++++++++++++++
>  lib/librte_representor/rte_port_representor.h      |  60 +++
>  .../rte_port_representor_driver.h                  | 138 ++++++
>  .../rte_port_representor_version.map               |   8 +
>  mk/rte.app.mk                                      |   1 +
>  34 files changed, 1688 insertions(+), 3 deletions(-)
>  create mode 100644 doc/guides/prog_guide/representor_lib.rst
>  create mode 100644 drivers/net/i40e/i40e_prep_ops.c
>  create mode 100644 drivers/net/i40e/i40e_prep_ops.h
>  create mode 100644 drivers/net/ixgbe/ixgbe_prep_ops.c
>  create mode 100644 drivers/net/ixgbe/ixgbe_prep_ops.h
>  create mode 100644 lib/librte_representor/Makefile
>  create mode 100644 lib/librte_representor/rte_port_representor.c
>  create mode 100644 lib/librte_representor/rte_port_representor.h
>  create mode 100644 lib/librte_representor/rte_port_representor_driver.h
>  create mode 100644 lib/librte_representor/rte_port_representor_version.map
> 
> -- 
> 2.9.5

  parent reply	other threads:[~2018-01-11  3:18 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-17 14:42 [dpdk-dev] [PATCH v2 1/6] ethdev: added switch_domain and representor port flag Mohammad Abdul Awal
2017-11-17 14:42 ` [dpdk-dev] [PATCH v2 2/6] lib/representor: port representor library to manage broker infrastructure and representor PMDs Mohammad Abdul Awal
2017-11-17 23:03   ` Ferruh Yigit
2017-12-08 14:57     ` Mohammad Abdul Awal
2017-11-17 14:42 ` [dpdk-dev] [PATCH v2 3/6] eal: added --enable-representor command line argument in EAL to load representor broker infrastructure Mohammad Abdul Awal
2017-11-17 14:42 ` [dpdk-dev] [PATCH v2 4/6] net/representor: Implement port representor PMD Mohammad Abdul Awal
2017-11-20  7:46   ` Ferruh Yigit
2017-12-08 15:02     ` Remy Horton
2017-12-08 16:56       ` Mohammad Abdul Awal
2017-12-11 10:08         ` Remy Horton
2017-12-11 15:11           ` Mohammad Abdul Awal
2017-12-11 15:29             ` Remy Horton
2017-11-17 14:42 ` [dpdk-dev] [PATCH v2 5/6] net/i40e: Enable port representor PMD and broker for fortville PMD driver Mohammad Abdul Awal
2017-11-20  7:48   ` Ferruh Yigit
2017-11-17 14:42 ` [dpdk-dev] [PATCH v2 6/6] net/ixgbe: Enable port representor PMD and broker for ixgbe " Mohammad Abdul Awal
2017-11-20 19:57 ` [dpdk-dev] [PATCH v2 1/6] ethdev: added switch_domain and representor port flag Ferruh Yigit
2017-12-08 15:33   ` Mohammad Abdul Awal
2017-12-11 13:45     ` Alex Rosenbaum
2017-12-11 18:00       ` Mohammad Abdul Awal
2017-12-22 14:41 ` [dpdk-dev] [DPDK 0/5] lib: add Port Representors Remy Horton
2017-12-22 14:41   ` [dpdk-dev] [DPDK 1/5] lib: add Port Representor library Remy Horton
2017-12-22 14:41   ` [dpdk-dev] [DPDK 2/5] eal: add Port Representor command-line option Remy Horton
2017-12-22 14:41   ` [dpdk-dev] [DPDK 3/5] drivers/net/i40e: add Port Representor functionality Remy Horton
2017-12-22 14:41   ` [dpdk-dev] [DPDK 4/5] drivers/net/ixgbe: " Remy Horton
2017-12-22 14:41   ` [dpdk-dev] [DPDK 5/5] app/test-pmd: add Port Representor commands Remy Horton
2017-12-22 15:09   ` [dpdk-dev] [DPDK 0/5] lib: add Port Representors Remy Horton
2017-12-22 16:37   ` Neil Horman
2018-01-03 12:14     ` Mohammad Abdul Awal
2017-12-22 14:52 ` [dpdk-dev] [PATCH v3 " Remy Horton
2017-12-22 14:52   ` [dpdk-dev] [PATCH v3 1/5] lib: add Port Representor library Remy Horton
2017-12-22 14:52   ` [dpdk-dev] [PATCH v3 2/5] eal: add Port Representor command-line option Remy Horton
2017-12-22 14:52   ` [dpdk-dev] [PATCH v3 3/5] drivers/net/i40e: add Port Representor functionality Remy Horton
2017-12-22 14:52   ` [dpdk-dev] [PATCH v3 4/5] drivers/net/ixgbe: " Remy Horton
2017-12-22 14:52   ` [dpdk-dev] [PATCH v3 5/5] app/test-pmd: add Port Representor commands Remy Horton
2017-12-26 13:54   ` [dpdk-dev] [PATCH v3 0/5] lib: add Port Representors Neil Horman
2018-01-03 12:12     ` Mohammad Abdul Awal
2018-01-08 14:37   ` [dpdk-dev] [PATCH v4 " Remy Horton
2018-01-08 14:37     ` [dpdk-dev] [PATCH v4 1/5] lib: add Port Representor library Remy Horton
2018-01-09 22:06       ` Ferruh Yigit
2018-01-10 11:40         ` Remy Horton
2018-01-08 14:37     ` [dpdk-dev] [PATCH v4 2/5] eal: add Port Representor command-line option Remy Horton
2018-01-09 22:07       ` Ferruh Yigit
2018-01-08 14:37     ` [dpdk-dev] [PATCH v4 3/5] drivers/net/i40e: add Port Representor functionality Remy Horton
2018-01-09 22:09       ` Ferruh Yigit
2018-01-10  7:56       ` Xing, Beilei
2018-01-10 10:11         ` Mohammad Abdul Awal
2018-01-10 12:37           ` Xing, Beilei
2018-01-10 14:04             ` Mohammad Abdul Awal
2018-01-08 14:37     ` [dpdk-dev] [PATCH v4 4/5] drivers/net/ixgbe: " Remy Horton
2018-01-08 14:37     ` [dpdk-dev] [PATCH v4 5/5] app/test-pmd: add Port Representor commands Remy Horton
2018-01-09 22:10       ` Ferruh Yigit
2018-01-10  7:22         ` Remy Horton
2018-01-09 22:01     ` [dpdk-dev] [PATCH v4 0/5] lib: add Port Representors Ferruh Yigit
2018-01-10 14:17       ` Mohammad Abdul Awal
2018-01-09 23:22     ` Thomas Monjalon
2018-01-10 13:46       ` Doherty, Declan
2018-01-10 19:26         ` Thomas Monjalon
2018-01-15 12:12           ` Doherty, Declan
2018-01-15 16:09             ` Thomas Monjalon
2018-01-10 15:54     ` [dpdk-dev] [PATCH v5 " Remy Horton
2018-01-10 15:54       ` [dpdk-dev] [PATCH v5 1/5] lib: add Port Representor library Remy Horton
2018-01-30  9:23         ` Andrew Rybchenko
2018-01-10 15:54       ` [dpdk-dev] [PATCH v5 2/5] eal: add Port Representor command-line option Remy Horton
2018-01-10 15:54       ` [dpdk-dev] [PATCH v5 3/5] drivers/net/i40e: add Port Representor functionality Remy Horton
2018-01-10 15:54       ` [dpdk-dev] [PATCH v5 4/5] drivers/net/ixgbe: " Remy Horton
2018-01-10 15:54       ` [dpdk-dev] [PATCH v5 5/5] app/test-pmd: add Port Representor commands Remy Horton
2018-01-10 17:49       ` [dpdk-dev] [PATCH v5 0/5] lib: add Port Representors Remy Horton
2018-01-10 17:49         ` [dpdk-dev] [PATCH v5 1/5] lib: add Port Representor library Remy Horton
2018-01-10 17:49         ` [dpdk-dev] [PATCH v5 2/5] eal: add Port Representor command-line option Remy Horton
2018-01-10 17:49         ` [dpdk-dev] [PATCH v5 3/5] drivers/net/i40e: add Port Representor functionality Remy Horton
2018-01-10 17:49         ` [dpdk-dev] [PATCH v5 4/5] drivers/net/ixgbe: " Remy Horton
2018-01-10 17:49         ` [dpdk-dev] [PATCH v5 5/5] app/test-pmd: add Port Representor commands Remy Horton
2018-01-11  3:18     ` Yuanhan Liu [this message]
2018-01-11  8:37       ` [dpdk-dev] [PATCH v4 0/5] lib: add Port Representors Alex Rosenbaum
2018-01-11  9:38       ` Remy Horton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180111031820.GR29540@yliu-mob \
    --to=yliu@fridaylinux.org \
    --cc=alexr@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=jingjing.wu@intel.com \
    --cc=john.mcnamara@intel.com \
    --cc=olgas@mellanox.com \
    --cc=remy.horton@intel.com \
    --cc=shahafs@mellanox.com \
    --cc=thomas@monjalon.net \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).