From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 2F410107A for ; Thu, 14 Dec 2017 14:57:06 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Dec 2017 05:57:05 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,400,1508828400"; d="scan'208";a="13444234" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by fmsmga001.fm.intel.com with ESMTP; 14 Dec 2017 05:57:04 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.67]) by IRSMSX108.ger.corp.intel.com ([169.254.11.167]) with mapi id 14.03.0319.002; Thu, 14 Dec 2017 13:57:03 +0000 From: "Ananyev, Konstantin" To: Jerin Jacob CC: "dev@dpdk.org" , "shahafs@mellanox.com" Thread-Topic: [dpdk-dev] [RFC PATCH 0/3] ethdev: few changes in rte_ethdev layer Thread-Index: AQHTarRqpnKgXxM3E062tX4SPFnQpqNCWb6AgAB88LA= Date: Thu, 14 Dec 2017 13:57:02 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772585FAC98BC@irsmsx105.ger.corp.intel.com> References: <1512140112-13067-1-git-send-email-konstantin.ananyev@intel.com> <20171214045408.GA843@jerin> In-Reply-To: <20171214045408.GA843@jerin> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTRiOTBmMGMtNTIyNy00YThmLWE2ZGItZjM2ZDJjZWM2MDhhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6Ing3NkhwQVJvVkpUOWVka1BBYW9yNlJDbWpoM2V1UzlaYTJCOVFDbUFLMmc9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC PATCH 0/3] ethdev: few changes in rte_ethdev layer X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Dec 2017 13:57:07 -0000 Hi Jerin, >=20 > Hi Konstantin, >=20 > > 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.=09 > > 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 breakag= e, > > so deprecation notice for 18.05 is filled. > > Further suggestions how to introduce the same functionality > > without ABI breakage are welcome. >=20 > 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. Yes that's the stretch goal. We do need some extra space for rx/tx callbacks (use field) that needs to be cache aligned and we plan to make rx/tx function per queue. So I thought it would be a good opportunity to do these 2 things in one go = - to minimize number of ABI breakages.=20 >=20 > > > > 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 hand= ler. > > 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=3D1 - slowdown < 1% > > 2) examples//l3fwd ... --parse-ptype - - slowdown < 1% > > 3) examples/rxtx_callbacks - slowdown ~8% > > All that in terms of packet throughput (Mpps). >=20 > We tried on an arm64 machine; We got following result on host and guest > using l3fwd. Thanks for testing it. So these numbers below - are for l3fwd running with or without rx callback = installed? i.e. with or without '--parse-ptype' option (and on a NIC with/without ptyp= e HW support)? >=20 >=20 > Note: > +ve number means "Performance regression with patches" > -ve number means "Performance improvement with patches" >=20 > Relative performance difference in percentage: >=20 > % 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 >=20 > % 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 >=20 > I think, it is because we added more code under > the RTE_ETHDEV_RXTX_CALLBACKS and it is enabled by > default. >=20 > I think, the impact will vary based on > architecture and underneath i-cache, d-cache and IPC. >=20 > 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.=20 I suppose you suggest that user would have to decide at queue_setup() stage does he wants a callback(s) for RX/TX or no? As I can see, the main drawback with that approach - it would introduce new= limitation. Right now it is possible to add/remove RX/TX callback at runtime (without stopping and reconfiguring the queue), and I suppose we do want to= keep that ability. Again in some cases user can't decide does he wants a callback installed or= not, before he actually calls dev_start(). l3fwd ptype-parse callback is an example of such situation: right now user has to start device first to figure out which ptypes are sup= ported by this PMD with desired configuration, then based on this information he will (or will= not) setup the callback. What I thought in same direction - introduce a notion of 'static' and 'dyna= mic' callbacks. Static ones can't be removed while queue is not stopped while dynamic can. But that also requires user to select at queue_setup() stage does he wants = dynamic callbacks for that queue or not (new offload flag or extra field in rxconf/= txconf structure).=20 So again - same limitation for the user. Another possibility here - have a flag 'dynamic' or 'static' not per queue,= but per individual callback. Yes, that would mean API change for add/remove callback functions. Konstantin > 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. >=20 > Jerin >=20 > > > > 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 > >