From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 5DC6D239 for ; Wed, 13 Dec 2017 11:34:32 +0100 (CET) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Dec 2017 02:34:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,397,1508828400"; d="scan'208";a="2164508" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.106]) by fmsmga008.fm.intel.com with SMTP; 13 Dec 2017 02:34:29 -0800 Received: by (sSMTP sendmail emulation); Wed, 13 Dec 2017 10:34:28 +0000 Date: Wed, 13 Dec 2017 10:34:28 +0000 From: Bruce Richardson To: "Van Haaren, Harry" Cc: Pavan Nikhilesh , "jerin.jacob@caviumnetworks.com" , "Eads, Gage" , "hemant.agrawal@nxp.com" , "nipun.gupta@nxp.com" , "Ma, Liang J" , "dev@dpdk.org" Message-ID: <20171213103427.GA14464@bricha3-MOBL3.ger.corp.intel.com> References: <20171212192713.17620-1-pbhagavatula@caviumnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.9.1 (2017-09-22) Subject: Re: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test to driver 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: Wed, 13 Dec 2017 10:34:33 -0000 On Wed, Dec 13, 2017 at 10:19:51AM +0000, Van Haaren, Harry wrote: > > -----Original Message----- > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com] > > Sent: Tuesday, December 12, 2017 7:27 PM > > To: jerin.jacob@caviumnetworks.com; Richardson, Bruce > > ; Van Haaren, Harry > > ; Eads, Gage ; > > hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J > > > > Cc: dev@dpdk.org; Pavan Nikhilesh > > Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test > > to driver > > > > Move octeontx eventdev specific test (test_eventdev_octeontx.c) to > > driver/event/octeontx. > > > > Replying to 1st patch, as no cover letter; > > Summary of patchset: > - Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c > - Enable self tests to run when passed the vdev arg "self-test=1" > > > A few comments on this change; > > 1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component. > > From discussion on #IRC, it seems reasonable to call rte_eal_vdev_init() with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD. > > 2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication. > > > Specific comments will be sent as replies to the patches. Cheers, -Harry What I gather from a cursory glance at this set is that the self tests are designed to be triggered via devargs to the device driver, correct? I'm not sure I like this approach, though I do agree with having the tests inside the individual drivers. What I think I would prefer to see is the self-tests being called via an API rather than via devargs. I think we should add a "rte_event_dev_self_test()" API to the eventdev library, and have that then call into the driver-provided tests. This means that self-tests can only be called by applications which are set up to allow the tests to be called, e.g. the autotest binary, while also avoiding the issue of having lots of driver specifics clutter up test binaries. Regards, /Bruce