From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id 64DFEA56E for ; Mon, 15 Jan 2018 16:57:50 +0100 (CET) Received: by mail-wm0-f49.google.com with SMTP id i186so2766162wmi.4 for ; Mon, 15 Jan 2018 07:57:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=x6dOFfO4S6AwERpuoCbG4GyVLrwUE/M7U09QhAJq+AY=; b=GZoPx9GFIw2Mi462XY/RR/16k5qDaazwZKpX7U0ynLjZDx67OymKHuS3KlSj90mEw+ I2Kn9ulQ8WB+L1nHO8sOvuYWLQxYwJLpLf+DPKnE8vMKW4bv3UqU79UjlcrsxNXuLqMm z/rTPZ+8lbtjHyzGj5Yoe7wVlLA1Z4S4HozF5PFTKRqplj91WnesttbZTJpHCYZIu3u/ OqJ80rOJLX78OujRFo9OUSP0IbiYWBa9lWbPEtjsndiGYP7sYB8Z9ZtoZMQvB1PPbfsd Gul9DHLrAauCYyzVdBjtqrm5IIPm+SIDlD47nJ3yOZudsULNwBB09dDlCxipRqv4KHav ZRkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=x6dOFfO4S6AwERpuoCbG4GyVLrwUE/M7U09QhAJq+AY=; b=AmfA4/aSvnHh4QifFdf16pxKETwmvw+bzF7MVXFBWBHiRMufOAXegVgq8InFJ+gMyf Ws3i9Kx0FHo0WESwMId+eeTRzuGMpJcOS0R4Jt5cmEd9KEg9XPIDp8B/AOpkfpT9xgy4 lLSLgBbsK7altrqn5mbReS/UGjQjYnU/o2O2iR0nyzmQBYPiwQtxZaNVvC0ovVTfZB5j m4YvMofwEu8grJ5bOqXKrsNcrveJz8EAnHPdkb07uVJWWcXrJg65wIhGjHKQKhrKfHcl 3TqGLIhNCkpVZSfPscG/rcGvZuh5bjlD/Yq/WWZvBaCyjKsG/DBkDOzfE8o2M1Tn0iJr tSEQ== X-Gm-Message-State: AKwxytfrvOXoZfecxPMWwlJVdmP7oXlkSebyatQfs5bba8xMZKmFieud QBHffmbHEoVlU6RuVgsnnch2kg== X-Google-Smtp-Source: ACJfBovwwg1JKRdgd50bLLrrsvzkpUmA5d/gPj2qWLsVW0OCs58sXn5/xN1ccsJCcX+J6Gc0X4yUHw== X-Received: by 10.28.167.5 with SMTP id q5mr8090146wme.90.1516031869765; Mon, 15 Jan 2018 07:57:49 -0800 (PST) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 44sm6034541wrv.0.2018.01.15.07.57.48 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 15 Jan 2018 07:57:48 -0800 (PST) Date: Mon, 15 Jan 2018 16:57:36 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Moti Haimovsky Cc: ferruh.yigit@intel.com, stephen@networkplumber.org, dev@dpdk.org Message-ID: <20180115155736.jk6uzuu5bqj4mcff@bidouze.vm.6wind.com> References: <1515095458-186363-2-git-send-email-motih@mellanox.com> <1515595223-36144-1-git-send-email-motih@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1515595223-36144-1-git-send-email-motih@mellanox.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH V3 1/2] net/failsafe: convert to new Tx 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, 15 Jan 2018 15:57:50 -0000 Hi Moti, Thanks for the patches. I have some nitpicks, but overall it is good. Sorry for doing the review so late, the changes should not take long, it's mostly syntax. For the commit title, I'd prefer "use" instead of "convert to" [PATCH V3 1/2] net/failsafe: use new Tx offloads API As the old API is still supported and used. On Wed, Jan 10, 2018 at 04:40:22PM +0200, Moti Haimovsky wrote: .. > @@ -84,9 +85,18 @@ > fs_dev_configure(struct rte_eth_dev *dev) > { > struct sub_device *sdev; > + uint64_t supp_tx_offloads = PRIV(dev)->infos.tx_offload_capa; > + uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads; I'm not fond of compound variable definition and initialization. Initialization should be done after the definition list. > uint8_t i; > int ret; > > + if ((tx_offloads & supp_tx_offloads) != tx_offloads) { > + rte_errno = ENOTSUP; > + ERROR("Some Tx offloads are not supported, " > + "requested 0x%lx supported 0x%lx\n", Please use PRIx64 instead of %lx for displaying uint64_t. > + tx_offloads, supp_tx_offloads); > + return -rte_errno; > + } > FOREACH_SUBDEV(sdev, i, dev) { > int rmv_interrupt = 0; > int lsc_interrupt = 0; > @@ -311,6 +321,22 @@ > return ret; > } > > +static bool > +fs_txq_are_offloads_valid(struct rte_eth_dev *dev, uint64_t offloads) _are seems unnecessary here: if (fs_txq_offloads_valid() == false) { ... } reads well enough and is shorter. > +{ > + uint64_t port_offloads = dev->data->dev_conf.txmode.offloads; > + uint64_t queue_supp_offloads = PRIV(dev)->infos.tx_queue_offload_capa; > + uint64_t port_supp_offloads = PRIV(dev)->infos.tx_offload_capa; > + > + if ((offloads & (queue_supp_offloads | port_supp_offloads)) != > + offloads) > + return false; > + /* Verify we have no conflict with port offloads */ > + if ((port_offloads ^ offloads) & port_supp_offloads) > + return false; > + return true; > +} > + > static void > fs_tx_queue_release(void *queue) > { > @@ -347,6 +373,22 @@ > fs_tx_queue_release(txq); > dev->data->tx_queues[tx_queue_id] = NULL; > } > + /* > + * Don't verify queue offloads for applications which > + * use the old API. > + */ > + if (tx_conf != NULL && > + !!(tx_conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) && No need for !! > + !fs_txq_are_offloads_valid(dev, tx_conf->offloads)) { Please check against explicit "false" instead of using '!'. -- Gaëtan Rivet 6WIND