DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: Konstantin Ananyev <konstantin.ananyev@intel.com>
Cc: dev@dpdk.org, shahafs@mellanox.com
Subject: Re: [dpdk-dev] [RFC PATCH 0/3] ethdev: few changes in rte_ethdev layer
Date: Thu, 14 Dec 2017 10:24:09 +0530	[thread overview]
Message-ID: <20171214045408.GA843@jerin> (raw)
In-Reply-To: <1512140112-13067-1-git-send-email-konstantin.ananyev@intel.com>

-----Original Message-----
> Date: Fri, 1 Dec 2017 14:55:12 +0000
> From: Konstantin Ananyev <konstantin.ananyev@intel.com>
> To: dev@dpdk.org, dev@dpdk.org
> CC: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Subject: [dpdk-dev] [RFC PATCH 0/3] ethdev: few changes in rte_ethdev layer
> X-Mailer: git-send-email 1.7.0.7
> 

Hi Konstantin,

> The series introduces 2 main changes:
> 
> 1.Introduce a separate data structure (rte_eth_queue_local)
> to store local to given process (i.e. no-shareable) information
> for each configured rx/tx queue.
> Memory for that structure is allocated/freed dynamically during
> rte_eth_dev_configure().
> Reserve a space for queue specific (rx|tx)_pkt_burst(),
> tx_pkt_prepare() function pointers inside that structure.
> Move rx/tx callback related information inside that structure.
> That introduces a change in current behavior: all callbacks for
> un-configured queues will be automatically removed.
> Also as size of struct rte_eth_dev changes that patch is an ABI breakage,
> so deprecation notice for 18.05 is filled.
> Further suggestions how to introduce the same functionality
> without ABI breakage are welcome.

Are we doing this to enable, Tx/Rx queue specific burst functions to
invoke based on new TX/RX queue specific offloads framework.
Meaning, if a offload configured for the given queue,
driver can select a different function pointer for Rx/Tx burst functions.

> 
> 2. Make it safe to remove rx/tx callback at runtime.
> Right now it is not possible for the application to figure out
> when it is safe to free removed callback handle and
> associated with it resources(unless the queue is stopped).
> That's probably not a big problem if all callbacks are static
> hange through whole application lifetime)
> and/or application doesn't allocate any resources for the callback handler.
> Though if callbacks have to be added/removed dynamically and
> callback handler would require more resources to operate properly -
> then it might become an issue.
> So patch #2 fixes that problem - now as soon as
> rte_eth_remove_(rx|tx)_callback() completes successfully, application
> can safely free all associated with the removed callback resources.
> 
> Performance impact:
> If application doesn't use RX/TX callbacks, then the tests I run didn't
> reveal any performance degradation.
> Though if application do use RX/TX callbacks - patch #2 does introduce
> some slowdown.
>  
> To be more specific here, on BDW (E5-2699 v4) 2.2GHz, 4x10Gb (X520-4)
> with http://dpdk.org/dev/patchwork/patch/31864/ patch installed I got:
> 1) testpmd ... --latencystats=1 - slowdown < 1%
> 2) examples//l3fwd ... --parse-ptype - - slowdown < 1%
> 3) examples/rxtx_callbacks - slowdown ~8%
> All that in terms of packet throughput (Mpps).

We tried on an arm64 machine; We got following result on host and guest
using l3fwd.


Note:
+ve number means "Performance regression with patches"
-ve number means "Performance improvement with patches"

Relative performance difference in percentage:

% difference on host (2 x 40G)	
pkt_sz  1c      2c      4c      8c
64	1.41	0.18	0.51	0.29
72	0.50	0.53	0.09	0.19
128	0.31	0.31	0.42	0.00
256	-0.44	-0.44	0.00	0.00
512	1.94	1.94	0.01	0.01
1024	0.00	0.01	0.00	0.01
1518	0.02	0.01	0.02	0.02

% difference on guest (2 x 40G)	
pkt_sz  1c      2c      4c      8c
64      5.78	2.30	-2.45	-0.01
72	1.13	1.13	1.31	-4.29
128	1.36	1.36	1.54	-0.82
256	2.02	2.03	-0.01	-0.35
512	4.05	4.04	0.00	-0.46
1024	0.39	0.38	0.00	-0.74
1518	0.00	0.00	-0.05	-4.20

I think, it is because we added more code under 
the RTE_ETHDEV_RXTX_CALLBACKS and it is enabled by
default.

I think, the impact will vary based on 
architecture and underneath i-cache, d-cache and IPC.

I am just thinking, How we can avoid such impact?
How about,
# Remove RTE_ETHDEV_RXTX_CALLBACKS option
# Hook Rx/TX callbacks under TX/RX offload flags
# ethdev layer gives helper function to invoke
callbacks to driver. But, don't invoke from common code
# When application needs the callback support, it
configured the given RX/TX queue offloads
# If the Rx/TX callback configure by the application
then driver calls the helper functions exposed by
the common code to invoke RX callbacks.

Jerin

> 
> Ability to safely remove callbacks at runtime implies
> some sort of synchronization.
> Even I tried to make it as light as possible,
> probably some slowdown is unavoidable.
> Of course instead of introducing these changes at rte_ethdev layer
> similar technique could be applied on individual callback basis.
> In that case it would be up to callback writer/installer to decide
> does he/she need a removable at runtime callback or not.
> Though in that case, each installed callback might introduce extra
> synchronization overhead and slowdown.
> 
> Konstantin Ananyev (3):
>   ethdev: introduce eth_queue_local
>   ethdev: make it safe to remove rx/tx callback at runtime
>   doc: ethdev ABI change deprecation notice
> 
>  doc/guides/rel_notes/deprecation.rst |   5 +
>  lib/librte_ether/rte_ethdev.c        | 390 ++++++++++++++++++++++-------------
>  lib/librte_ether/rte_ethdev.h        | 174 ++++++++++++----
>  3 files changed, 387 insertions(+), 182 deletions(-)
> 
> -- 
> 2.13.5
> 

  reply	other threads:[~2017-12-14  4:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-01 14:55 Konstantin Ananyev
2017-12-14  4:54 ` Jerin Jacob [this message]
2017-12-14 13:57   ` Ananyev, Konstantin
2017-12-18 16:38     ` Jerin Jacob
2017-12-19 15:27       ` Ananyev, Konstantin
2017-12-20  7:28         ` Jerin Jacob
2017-12-20 18:23           ` Ananyev, Konstantin
2018-01-04  9:22             ` Jerin Jacob
2018-01-15 16:29               ` Ananyev, Konstantin

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=20171214045408.GA843@jerin \
    --to=jerin.jacob@caviumnetworks.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=shahafs@mellanox.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).