From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 7B3D77D14 for ; Fri, 25 Aug 2017 15:26:38 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 3293A20DCC; Fri, 25 Aug 2017 09:26:37 -0400 (EDT) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Fri, 25 Aug 2017 09:26:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=dyj3ipoi3j5CjFH uxWbl9xhNIzv9w3+bAdCQg56SpF4=; b=OlqtDHK7g5M2vFS0AbcmWSg8JfFaXV9 4GmqXJXD+zW6EkzJXJFUjalSHyf6n4KKq8jgyE/D8doDIWchtZJPk4qaHN3cQ77n kBFVXI2ZmCh87PhdIvA8hZrMOvWgUuKqq9bcKJLL5zP4zqr3QuDepPeFg8kmtBD4 a2fN2G52ffZY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc:x-sasl-enc; s= fm1; bh=dyj3ipoi3j5CjFHuxWbl9xhNIzv9w3+bAdCQg56SpF4=; b=k+Tld2Dn 7en+LVZPY/GQgNH+mfKYoPN0REiWF7RE6Q4r1xFXbORh2edd/49NZfhMgofdFk/f src6ybtlKz8qQUDj4yFVbOgph41t24r6Xmc2HG/iULb/vBvnBLIYQ7aeid6D2RO0 g3iWMbbt2ZAQo9BYwArcEQTuMBmOiqkBIP7y7PMbwH060NqJdaCsc1jDXz2I0FS1 M/tvEATWgIwdy90X/rAPLmMSK4JVeZ3jGc+zDjsXRSeparUNz7K/OedDnqmoEko2 mQ9Vi1Iq7sHNI6V5vg5LrO5NmoLVi/Ey131jL6iwkklct6iJ09gPamkCZMs7Kp3j utVN8DvQRpG/Ow== X-ME-Sender: X-Sasl-enc: 5mj37p5NZFiPXSBieHgjIo37EZMY400pQK8JP9uG+vYZ 1503667596 Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id CE59D240B1; Fri, 25 Aug 2017 09:26:36 -0400 (EDT) From: Thomas Monjalon To: Shahaf Shuler , "Ananyev, Konstantin" Cc: dev@dpdk.org Date: Fri, 25 Aug 2017 15:26:36 +0200 Message-ID: <2468395.3cLPV6SE8H@xps> In-Reply-To: References: <5969739.W1oj9hpjs5@xps> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [RFC 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: Fri, 25 Aug 2017 13:26:38 -0000 24/08/2017 09:12, Shahaf Shuler: > Thursday, August 24, 2017 1:06 AM, Thomas Monjalon: > > 23/08/2017 15:13, Shahaf Shuler: > > > Wednesday, August 23, 2017 3:29 PM, Ananyev, Konstantin: > > > > From: Shahaf Shuler > > > > > In order to enable PMDs to support only one of the APIs, and > > > > > applications to avoid branching according to the underlying device > > > > > a copy functions to/from the old/new APIs were added. > > > > Looks a good intent. > > I would prefer the word "convert" instead of "copy". > > > > > > > int > > > > > rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id, > > [...] > > > > > + } else if ((!(dev->data->dev_flags & RTE_ETH_DEV_RXQ_OFFLOAD)) && > > > > > + (dev->data->dev_conf.rxmode.ignore == 1)) { > > > > > + int ret; > > > > > + struct rte_eth_rxmode rxmode; > > > > > + > > > > > + rte_eth_copy_rxq_offloads(&rxmode, rx_conf); > > > > > + if (memcmp(&rxmode, &dev->data->dev_conf.rxmode, > > > > > + sizeof(rxmode))) { > > > > > + /* > > > > > + * device which work with rxmode offloads API requires > > > > > + * a re-configuration in order to apply the new offloads > > > > > + * configuration. > > > > > + */ > > > > > + dev->data->dev_conf.rxmode = rxmode; > > > > > + ret = rte_eth_dev_configure(port_id, > > > > > + dev->data->nb_rx_queues, > > > > > + dev->data->nb_tx_queues, > > > > > + &dev->data->dev_conf); > > > > > > > > Hmm, and why we would need to reconfigure our device in the middle > > > > of rx queue setup? > > > > > > The reason is the old Rx offloads API is configured on device configure. > > > This if section is for applications which already moved to the new > > > offload API however the underlying PMD still uses the old one. > > > > Isn't it risky to re-run configure here? > > We could also declare this case as an error. > > > > I think applications which have migrated to the new API, could use the > > convert functions themselves before calling configure to support not > > migrated PMDs. > > The cons of my solution are: > > - discourage apps to migrate before all PMDs have migrated > > - expose a temporary function to convert API I propose it anyway because > > there is always someone to like bad ideas ;) > > Yes. I tried to make it as simple as possible for application to move to the new API. > Defining it as error flow, will enforce the application to check the PMD offload mode and branch accordingly. The conversion functions are a good helpers, yet the code remains complex due to the different cases with the different PMDs. > > Considering the re-configuration is risky, and without other ideas I will need to fall back to the error flow case. > Are we OK with that? I think we can take the risk of keeping this call to rte_eth_dev_configure() in the middle of rte_eth_rx_queue_setup(). In theory it should be acceptable. If we merge it soon, it can be better tested with every drivers.