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 BE2AC1B19A for ; Mon, 18 Sep 2017 16:44:37 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Sep 2017 07:44:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,413,1500966000"; d="scan'208";a="129835891" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.24]) by orsmga004.jf.intel.com with SMTP; 18 Sep 2017 07:44:34 -0700 Received: by (sSMTP sendmail emulation); Mon, 18 Sep 2017 15:44:33 +0100 Date: Mon, 18 Sep 2017 15:44:33 +0100 From: Bruce Richardson To: Shahaf Shuler Cc: Thomas Monjalon , "Ananyev, Konstantin" , "stephen@networkplumber.org" , "dev@dpdk.org" Message-ID: <20170918144432.GA17244@bricha3-MOBL3.ger.corp.intel.com> References: <20170918110456.GB15516@bricha3-MOBL3.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772584F24BFE7@irsmsx105.ger.corp.intel.com> <32037584.9VqYmpDC01@xps> <20170918113733.GA14460@bricha3-MOBL3.ger.corp.intel.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.8.3 (2017-05-23) Subject: Re: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to the new offloads API 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: Mon, 18 Sep 2017 14:44:38 -0000 On Mon, Sep 18, 2017 at 02:27:25PM +0000, Shahaf Shuler wrote: > Monday, September 18, 2017 2:38 PM, Bruce Richardson > > On Mon, Sep 18, 2017 at 01:32:29PM +0200, Thomas Monjalon wrote: > > > 18/09/2017 13:11, Ananyev, Konstantin: > > > > From: Richardson, Bruce > > > > > > > > > > > > I think we all in favor to have a separate API here. > > > > > > Though from the discussion we had at latest TB, I am not sure it > > > > > > is doable in 17.11 timeframe. > > > > > > > > > > Ok, so does that imply no change in this release, and that the > > > > > existing set is to be ignored? > > > > > > > > No, my understanding the current plan is to go forward with Shahaf > > > > patches, and then apply another one (new set/get API) on top of them. > > > > > > Yes, it is what we agreed (hope to see it in minutes). > > > If someone can do these new patches in 17.11 timeframe, it's great! > > > Bruce, do you want to make it a try? > > > > If I have the chance, I can try, but given how short time is and that userspace > > is on next week, I very much doubt I'll even get it started. > > I wasn't aware to the techboard decision on the extra patchset needed. > I think it will be wrong to introduce an API on 17.11 and change it again on 18.02. > I will do my best to make everything ready for 17.11 so we can have one solid API on top of which all PMDs and application will be converted. Considering some Holidays and the DPDK summit I won't have much time to work on it. > > The plan is as follows: > 1. complete the last comment on the current series and integrate it. > 2. send a new patchset to convert to the API suggested above. > > Aggregating the different suggestions I come up with the below. if this is agreed, then I will move with the implementation. > (I thought it is good to return error values for the get function). I'd rather you didn't. :-) The only realistic error I would consider is an invalid port id, and I think returning 0 - no offloads - is fine in those cases. The user will pretty quickly discover it's an invalid port id anyway, so I prefer a get function to just return the value as a return value and be done with it! Otherwise, these will do fine. I would prefer some way to only change one offload at a time without having to call "get" and do bit twiddling before a call to "set", but this will be ok, if others are happy with it. If we at least get the return value as the mask of enabled offloads we can at least shorten some cases as e.g. rte_eth_set_port_tx_offloads(port_id, rte_eth_get_port_tx_offloads(port_id) | OFFLOAD_X); /Bruce > > ** > * Get Tx offloads set on a specific port. > * > * @param port_id > * The port identifier of the Ethernet device. > * @param offloads > * A pointer to uint64_t where the offloads flags > * will be filled using DEV_TX_OFFLOAD_* flags. > * @return > * - (0) if successful. > * - (-ENOTSUP or -ENODEV) on failure. > */ > int rte_eth_get_port_tx_offloads(uint8_t port_id, uint64_t *offloads); > > ** > * Get Tx offloads set on a specific queue. > * > * @param port_id > * The port identifier of the Ethernet device. > * @param queue_id > * The queue identifier. > * @param offloads > * A pointer to uint64_t where the offloads flags > * will be filled using DEV_TX_OFFLOAD_* flags. > * @return > * - (0) if successful. > * - (-ENOTSUP or -ENODEV) on failure. > */ > int rte_eth_get_queue_tx_offloads(uint8_t port_id, uint16_t queue_id, > uint64_t *offloads); > ** > * Set Tx offloads on a specific port. > * > * @param port_id > * The port identifier of the Ethernet device. > * @param offloads_mask > * Indicates which offloads to be set using DEV_TX_OFFLOAD_* flags. > * @return > * (0) if all offloads set successfully, otherwise offloads > * flags which were not set. > * > */ > uint64_t rte_eth_set_port_tx_offloads(uint8_t port_id, uint64_t offloads_mask); > > /** > * Set Tx offloads on a specific queue. > * > * @param port_id > * The port identifier of the Ethernet device. > * @param queue_id > * The queue identifier. > * @param offloads_mask > * Indicates which offloads to be set using DEV_TX_OFFLOAD_* flags. > * @return > * (0) if all offloads set successfully, otherwise offloads > * flags which were not set. > * > */ > uint64_t rte_eth_set_queue_tx_offloads(uint8_t port_id, uint16_t queue_id, > uint64_t offloads_mask); > /** > * Get Rx offloads set on a specific port. > * > * @param port_id > * The port identifier of the Ethernet device. > * @param offloads > * A pointer to uint64_t where the offloads flags > * will be filled using DEV_RX_OFFLOAD_* flags. > * @return > * - (0) if successful. > * - (-ENOTSUP or -ENODEV) on failure. > */ > int rte_eth_get_port_rx_offloads(uint8_t port_id, uint64_t *offloads); > > /** > * Get Rx offloads set on a specific queue. > * > * @param port_id > * The port identifier of the Ethernet device. > * @param queue_id > * The queue identifier. > * @param offloads > * A pointer to uint64_t where the offloads flags > * will be filled using DEV_RX_OFFLOAD_* flags. > * @return > * - (0) if successful. > * - (-ENOTSUP or -ENODEV) on failure. > */ > int rte_eth_get_queue_rx_offlaods(uint8_t port_id, uint16_t queue_id, > uint64_t *offloads); > > /** > * Set Rx offloads on a specific port. > * > * @param port_id > * The port identifier of the Ethernet device. > * @param offloads_mask > * Indicates which offloads to be set using DEV_RX_OFFLOAD_* flags. > * @return > * (0) if all offloads set successfully, otherwise offloads > * flags which were not set. > * > */ > uint64_t rte_eth_set_port_rx_offloads(uint8_t port_id, uint64_t offloads_mask); > > /** > * Set Rx offloads on a specific port. > * > * @param port_id > * The port identifier of the Ethernet device. > * @param queue_id > * The queue identifier. > * @param offloads_mask > * Indicates which offloads to be set using DEV_RX_OFFLOAD_* flags. > * @return > * (0) if all offloads set successfully, otherwise offloads > * flags which were not set. > * > */ > uint64_t rte_eth_set_queue_rx_offloads(uint8_t port_id, uint16_t queue_id, > uint64_t offloads_mask); > > > > > /Bruce