From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 358AE2C52 for ; Mon, 7 Mar 2016 15:34:58 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP; 07 Mar 2016 06:34:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,551,1449561600"; d="scan'208";a="61363279" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by fmsmga004.fm.intel.com with ESMTP; 07 Mar 2016 06:34:38 -0800 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.19]) by IRSMSX153.ger.corp.intel.com ([169.254.9.60]) with mapi id 14.03.0248.002; Mon, 7 Mar 2016 14:34:37 +0000 From: "Van Haaren, Harry" To: "Andralojc, WojciechX" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2] examples/skeleton-cat: PQoS CAT and CDP, example of libpqos usage Thread-Index: AQHRcKd/h0rkndjIOkO/VIEDKhG6VJ9OFI6Q Date: Mon, 7 Mar 2016 14:34:36 +0000 Message-ID: References: <1456320402-108962-1-git-send-email-wojciechx.andralojc@intel.com> <1456499233-66251-1-git-send-email-wojciechx.andralojc@intel.com> In-Reply-To: <1456499233-66251-1-git-send-email-wojciechx.andralojc@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjA3MDgxZDgtYzliYi00NjcxLTg0NDMtNDc2MmUzMDczODEyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IjNxaEszRmxjQW5uOEFrV3ltMElRWGs3WGx4aWM1TXhPemxDRnJmNm02V2c9In0= x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "Kantecki, Tomasz" , "Cornu, Marcel D" Subject: Re: [dpdk-dev] [PATCH v2] examples/skeleton-cat: PQoS CAT and CDP, example of libpqos usage X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Mar 2016 14:34:58 -0000 Hi All, From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wojciech Andralojc > Subject: [dpdk-dev] [PATCH v2] examples/skeleton-cat: PQoS CAT and CDP, e= xample of libpqos > usage Some minor comments/suggested-fixes in-line. I've snipped away lots of code, below only that relevant to my comments. > diff --git a/doc/guides/sample_app_ug/skeleton-cat.rst > b/doc/guides/sample_app_ug/skeleton-cat.rst > +CAT and CDP features allow management of the CPU's last level cache. > +CAT introduces classes of service (COS) that are essentially bitmasks. CDP is not explained anywhere - add (Cache Data Partitioning) after it here= , or write it and abbreviate as COS is. > +Running the Application > +----------------------- > + > +To run the example in a ``linuxapp`` environment and enable CAT on cpus = 0-2: > + > +.. code-block:: console > + > + ./build/basicfwd-cat -c 2 -n 4 -- --l3ca=3D0x3@(0-2) This doesn't work on my machine: - Bash complains about syntax error near ( Quotes around it fixes --l3ca=3D0x3@"(0-2)" - the -c 2 will enable core represented by bitmask 0x2, so only the 2nd co= re. Is this intended? > +or to enable CAT and CDP on cpus 1,3: > + > +.. code-block:: console > + > + ./build/basicfwd-cat -c 2 -n 4 -- --l3ca=3D(0x00C00,0x00300)@(1,3) Add a note here that when running on a maschine that doesn't support CDP, t= he "CDP requested but not supported" and "PQOS: L3CA init failed!" errors=20 will show up. > +The ``main()`` also allocates a mempool to hold the mbufs (Message Buffe= rs) > +used by the application: >>From here until the end of the RST doc is duplicated from the basicfwd skel= eton app: remove all the duplicated content. > +/* > + * Initializes a given port using global settings and with the RX buffer= s > + * coming from the mbuf_pool passed as a parameter. > + */ > +static inline int > +port_init(uint8_t port, struct rte_mempool *mbuf_pool) > +{ Multi-line comment duplicated from basicfwd: can be removed. > +/* > + * The lcore main. This is the main thread that does the work, reading f= rom > + * an input port and writing to an output port. > + */ > +static __attribute__((noreturn)) void > +lcore_main(void) > +{ Same. > +/* > + * The main function, which does initialization and calls the per-lcore > + * functions. > + */ > +int > +main(int argc, char *argv[]) > +{ and same again :) > + /* Initialize the Environment Abstraction Layer (EAL). */ > + int ret =3D rte_eal_init(argc, argv); > + if (ret < 0) > + rte_exit(EXIT_FAILURE, "Error with EAL initialization\n"); > + > + argc -=3D ret; > + argv +=3D ret; > + > + ret =3D cat_init(argc, argv); > + if (ret < 0) > + rte_exit(EXIT_FAILURE, "PQOS: L3CA init failed!\n"); Add a short comment above cat_init() stating it initializes PQOS/CAT, that the API is similar to rte_eal_init() and to refer to the sample-app documentation for details. Two checkpatch issues: WARNING: Block comments use a trailing */ on a separate line #1231: FILE: examples/skeleton-cat/cat.c:421: + * then default one (#0) */ WARNING: Block comments use a trailing */ on a separate line #1360: FILE: examples/skeleton-cat/cat.c:550: + * to be used for that group on that socket */ Apart from these minor issues; Acked-by: Harry van Haaren