DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Declan Doherty <declan.doherty@intel.com>
Cc: dev@dpdk.org, Fan Zhang <roy.fan.zhang@intel.com>,
	pablo.de.lara.guarch@intel.com
Subject: Re: [dpdk-dev] [PATCH v5] crypto/scheduler: add driver for scheduler crypto pmd
Date: Tue, 17 Jan 2017 21:21:18 +0100	[thread overview]
Message-ID: <4849565.cTZ7J3d9yX@xps13> (raw)
In-Reply-To: <a4de9055-ea21-adcb-78ce-8bd031c0cf97@intel.com>

2017-01-17 14:09, Declan Doherty:
> On 17/01/17 13:19, Fan Zhang wrote:
> > This patch provides the initial implementation of the scheduler poll mode
> > driver using DPDK cryptodev framework.
> >
> > Scheduler PMD is used to schedule and enqueue the crypto ops to the
> > hardware and/or software crypto devices attached to it (slaves). The
> > dequeue operation from the slave(s), and the possible dequeued crypto op
> > reordering, are then carried out by the scheduler.
> >
> > As the initial version, the scheduler PMD currently supports only the
> > Round-robin mode, which distributes the enqueued burst of crypto ops
> > among its slaves in a round-robin manner. This mode may help to fill
> > the throughput gap between the physical core and the existing cryptodevs
> > to increase the overall performance. Moreover, the scheduler PMD is
> > provided the APIs for user to create his/her own scheduler.
> >
> > Build instructions:
> > To build DPDK with CRYTPO_SCHEDULER_PMD the user is required to set
> > CONFIG_RTE_LIBRTE_PMD_CRYPTO_SCHEDULER=y in config/common_base
> >
> > Notice:
> > - Scheduler PMD shares same EAL commandline options as other cryptodevs.
> >   However, apart from socket_id, the rest of cryptodev options are
> >   ignored. The scheduler PMD's max_nb_queue_pairs and max_nb_sessions
> >   options are set as the minimum values of the attached slaves'. For
> >   example, a scheduler cryptodev is attached 2 cryptodevs with
> >   max_nb_queue_pairs of 2 and 8, respectively. The scheduler cryptodev's
> >   max_nb_queue_pairs will be automatically updated as 2.
> >
> > - In addition, an extra option "slave" is added. The user can attach one
> >   or more slave cryptodevs initially by passing their names with this
> >   option. Here is an example:
> >
> >   ... --vdev "crypto_aesni_mb_pmd,name=aesni_mb_1" --vdev "crypto_aesni_
> >   mb_pmd,name=aesni_mb_2" --vdev "crypto_scheduler_pmd,slave=aesni_mb_1,
> >   slave=aesni_mb_2" ...
> >
> >   Remember the software cryptodevs to be attached shall be declared before
> >   the scheduler PMD, otherwise the scheduler will fail to locate the
> >   slave(s) and report error.
> >
> > - The scheduler cryptodev cannot be started unless the scheduling mode
> >   is set and at least one slave is attached. Also, to configure the
> >   scheduler in the run-time, like attach/detach slave(s), change
> >   scheduling mode, or enable/disable crypto op ordering, one should stop
> >   the scheduler first, otherwise an error will be returned.
> >
> > Changes in v5:
> > Fixed EOF whitespace warning.
> > Updated Copyright.
> >
> > Changes in v4:
> > Fixed a few bugs.
> > Added slave EAL commandline option support.
> >
> > Changes in v3:
> > Fixed config/common_base.
> >
> > Changes in v2:
> > New approaches in API to suit future scheduling modes.
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> > ---
> >  config/common_base                                 |   6 +
> >  drivers/crypto/Makefile                            |   1 +
> >  drivers/crypto/scheduler/Makefile                  |  66 +++
> >  drivers/crypto/scheduler/rte_cryptodev_scheduler.c | 460 +++++++++++++++++++
> >  drivers/crypto/scheduler/rte_cryptodev_scheduler.h | 167 +++++++
> >  .../scheduler/rte_cryptodev_scheduler_operations.h |  71 +++
> >  .../scheduler/rte_pmd_crypto_scheduler_version.map |  12 +
> >  drivers/crypto/scheduler/scheduler_pmd.c           | 360 +++++++++++++++
> >  drivers/crypto/scheduler/scheduler_pmd_ops.c       | 489 +++++++++++++++++++++
> >  drivers/crypto/scheduler/scheduler_pmd_private.h   | 115 +++++
> >  drivers/crypto/scheduler/scheduler_roundrobin.c    | 417 ++++++++++++++++++
> >  lib/librte_cryptodev/rte_cryptodev.h               |   4 +
> >  mk/rte.app.mk                                      |   3 +-
> >  13 files changed, 2170 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/crypto/scheduler/Makefile
> >  create mode 100644 drivers/crypto/scheduler/rte_cryptodev_scheduler.c
> >  create mode 100644 drivers/crypto/scheduler/rte_cryptodev_scheduler.h
> >  create mode 100644 drivers/crypto/scheduler/rte_cryptodev_scheduler_operations.h
> >  create mode 100644 drivers/crypto/scheduler/rte_pmd_crypto_scheduler_version.map
> >  create mode 100644 drivers/crypto/scheduler/scheduler_pmd.c
> >  create mode 100644 drivers/crypto/scheduler/scheduler_pmd_ops.c
> >  create mode 100644 drivers/crypto/scheduler/scheduler_pmd_private.h
> >  create mode 100644 drivers/crypto/scheduler/scheduler_roundrobin.c
[...]
> 
> Acked-by: Declan Doherty <declan.doherty@intel.com>

NACK
I could argue it is too big for an unique patch,
but it's even worst when you ack without stripping the long patch.
My mouse is out of order after this long scroll looking for a comment.

  reply	other threads:[~2017-01-17 20:21 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-02 14:15 [dpdk-dev] [PATCH] Scheduler: " Fan Zhang
2016-12-02 14:31 ` Thomas Monjalon
2016-12-02 14:57   ` Bruce Richardson
2016-12-02 16:22     ` Declan Doherty
2016-12-05 15:12       ` Neil Horman
2016-12-07 12:42         ` Declan Doherty
2016-12-07 14:16           ` Neil Horman
2016-12-07 14:46             ` Richardson, Bruce
2016-12-07 16:04               ` Declan Doherty
2016-12-08 14:57                 ` Neil Horman
2017-01-03 17:08 ` [dpdk-dev] [PATCH v2] " Fan Zhang
2017-01-03 17:16 ` [dpdk-dev] [PATCH v3] " Fan Zhang
2017-01-17 10:57   ` [dpdk-dev] [PATCH v4] " Fan Zhang
2017-01-17 13:19     ` [dpdk-dev] [PATCH v5] crypto/scheduler: " Fan Zhang
2017-01-17 14:09       ` Declan Doherty
2017-01-17 20:21         ` Thomas Monjalon [this message]
2017-01-24 16:06       ` [dpdk-dev] [PATCH v6 00/11] " Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 01/11] cryptodev: add scheduler PMD name and type Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 02/11] crypto/scheduler: add APIs for scheduler Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 03/11] crypto/scheduler: add internal structure declarations Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 04/11] crypto/scheduler: add scheduler API implementations Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 05/11] crypto/scheduler: add round-robin scheduling mode Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 06/11] crypto/scheduler: register scheduler vdev driver Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 07/11] crypto/scheduler: register operation function pointer table Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 08/11] crypto/scheduler: add scheduler PMD to DPDK compile system Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 09/11] crypto/scheduler: add scheduler PMD config options Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 10/11] app/test: add unit test for cryptodev scheduler PMD Fan Zhang
2017-01-24 16:06         ` [dpdk-dev] [PATCH v6 11/11] crypto/scheduler: add documentation Fan Zhang
2017-01-24 16:23         ` [dpdk-dev] [PATCH v7 00/11] crypto/scheduler: add driver for scheduler crypto pmd Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 01/11] cryptodev: add scheduler PMD name and type Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 02/11] crypto/scheduler: add APIs for scheduler Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 03/11] crypto/scheduler: add internal structure declarations Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 04/11] crypto/scheduler: add scheduler API implementations Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 05/11] crypto/scheduler: add round-robin scheduling mode Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 06/11] crypto/scheduler: register scheduler vdev driver Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 07/11] crypto/scheduler: register operation function pointer table Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 08/11] crypto/scheduler: add scheduler PMD to DPDK compile system Fan Zhang
2017-01-24 16:23           ` [dpdk-dev] [PATCH v7 09/11] crypto/scheduler: add scheduler PMD config options Fan Zhang
2017-01-24 16:24           ` [dpdk-dev] [PATCH v7 10/11] app/test: add unit test for cryptodev scheduler PMD Fan Zhang
2017-01-24 16:24           ` [dpdk-dev] [PATCH v7 11/11] crypto/scheduler: add documentation Fan Zhang
2017-01-24 16:29           ` [dpdk-dev] [PATCH v7 00/11] crypto/scheduler: add driver for scheduler crypto pmd De Lara Guarch, Pablo
2017-01-25 14:03             ` De Lara Guarch, Pablo

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=4849565.cTZ7J3d9yX@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=pablo.de.lara.guarch@intel.com \
    --cc=roy.fan.zhang@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).