From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v2 1/6] eventdev: introduce event driven programming model Date: Wed, 14 Dec 2016 12:10:52 +0530 Message-ID: <20161214064051.GB21135@localhost.localdomain> References: <1479447902-3700-2-git-send-email-jerin.jacob@caviumnetworks.com> <1480996340-29871-1-git-send-email-jerin.jacob@caviumnetworks.com> <1480996340-29871-2-git-send-email-jerin.jacob@caviumnetworks.com> <20161207111251.GA22932@bricha3-MOBL3.ger.corp.intel.com> <20161208014800.GB22031@svelivela-lt.caveonetworks.com> <20161208095752.GB55440@bricha3-MOBL3.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: , , , , To: Bruce Richardson Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0060.outbound.protection.outlook.com [104.47.34.60]) by dpdk.org (Postfix) with ESMTP id 448612A5D for ; Wed, 14 Dec 2016 07:41:19 +0100 (CET) Content-Disposition: inline In-Reply-To: <20161208095752.GB55440@bricha3-MOBL3.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Dec 08, 2016 at 09:57:52AM +0000, Bruce Richardson wrote: > On Thu, Dec 08, 2016 at 07:18:01AM +0530, Jerin Jacob wrote: > > On Wed, Dec 07, 2016 at 11:12:51AM +0000, Bruce Richardson wrote: > > > On Tue, Dec 06, 2016 at 09:22:15AM +0530, Jerin Jacob wrote: > > > > + */ > > > > +int > > > > +rte_event_port_link(uint8_t dev_id, uint8_t port_id, > > > > + const struct rte_event_queue_link link[], > > > > + uint16_t nb_links); > > > > + > > > > > > Hi again Jerin, > > > > > > another small suggestion here. I'm not a big fan of using small > > > structures to pass parameters into functions, especially when not all > > > fields are always going to be used. Rather than use the event queue link > > > structure, can we just pass in two array parameters here - the list of > > > QIDs, and the list of priorities. In cases where the eventdev > > > implementation does not support link prioritization, or where the app > > > does not want different priority mappings , then the second > > > array can be null [implying NORMAL priority for the don't care case]. > > > > > > int > > > rte_event_port_link(uint8_t dev_id, uint8_t port_id, > > > const uint8_t queues[], const uint8_t priorities[], > > > uint16_t nb_queues); > > > > > > This just makes mapping an array of queues easier, as we can just pass > > > an array of ints directly in, and it especially makes it easier to > > > create a single link via: > > > > > > rte_event_port_link(dev_id, port_id, &queue_id, NULL, 1); > > > > The reason why I thought of creating "struct rte_event_queue_link", > > - Its easy to add new parameter in link attributes if required > > Make the priority value be in a struct, perhaps. That would allow for > future expansion, while still making it easier for the case where people > just want the mappings without any prioritization. OK. I will change the API prototype to align with your proposal in v3 int rte_event_port_link(uint8_t dev_id, uint8_t port_id, const uint8_t queues[], const uint8_t priorities[], uint16_t nb_links); int rte_event_port_links_get(uint8_t dev_id, uint8_t port_id, uint8_t queues[], uint8_t priorities[]); > > > - Its _easy_ to implement PAUSE and RESUME in application > > > > PAUSE: > > nr_links = rte_event_port_links_get(,,link) > > rte_event_port_unlink_all > > > > RESUME: > > rte_event_port_link(,,link, nr_links); > > Ok, I had missed that implication. Since that is probably an important > operation we might want to do, perhaps links_get API should be updated > too to keep parameter matching. > > /Bruce