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
