DPDK patches and discussions
 help / color / mirror / Atom feed
From: Alex Rosenbaum <rosenbaumalex@gmail.com>
To: Yuanhan Liu <yliu@fridaylinux.org>
Cc: Remy Horton <remy.horton@intel.com>, DPDK <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 10:37:42 +0200	[thread overview]
Message-ID: <CAFgAxU8uYHCWDDDmVirem31+xJNq6N3MHa5mY4irsDPWknQbKg@mail.gmail.com> (raw)
In-Reply-To: <20180111031820.GR29540@yliu-mob>

Declan,

In all previous series I explained my objection for using the broker
framework for holding the VF rep port.
You guys just waved it away (and sent a new series) instead of really
giving a clear reason why they are required.

I think Yuanhan's suggestion gets ride of the broker model in a clean way.
It exposes the VF rep's and keeps much similar to port model to allow
both control and second phase data path. The flow steering actions
suggestion match much nicer with this as well.

PS: me and Yuanhan did exchange ideas regarding this previously, so
it's not surprising I agree with his suggestion.

Alex


On Thu, Jan 11, 2018 at 5:18 AM, Yuanhan Liu <yliu@fridaylinux.org> wrote:
> 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

  reply	other threads:[~2018-01-11  8:37 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     ` [dpdk-dev] [PATCH v4 0/5] lib: add Port Representors Yuanhan Liu
2018-01-11  8:37       ` Alex Rosenbaum [this message]
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=CAFgAxU8uYHCWDDDmVirem31+xJNq6N3MHa5mY4irsDPWknQbKg@mail.gmail.com \
    --to=rosenbaumalex@gmail.com \
    --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 \
    --cc=yliu@fridaylinux.org \
    /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).