From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH V3 2/2] net/failsafe: convert to new Rx offloads API Date: Mon, 15 Jan 2018 17:02:52 +0100 Message-ID: <20180115160252.734ts4actt6wqrav@bidouze.vm.6wind.com> References: <1515095458-186363-2-git-send-email-motih@mellanox.com> <1515595223-36144-1-git-send-email-motih@mellanox.com> <1515595223-36144-2-git-send-email-motih@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: ferruh.yigit@intel.com, stephen@networkplumber.org, dev@dpdk.org To: Moti Haimovsky Return-path: Received: from mail-wr0-f169.google.com (mail-wr0-f169.google.com [209.85.128.169]) by dpdk.org (Postfix) with ESMTP id 37D11A496 for ; Mon, 15 Jan 2018 17:03:06 +0100 (CET) Received: by mail-wr0-f169.google.com with SMTP id g38so9023918wrd.2 for ; Mon, 15 Jan 2018 08:03:06 -0800 (PST) Content-Disposition: inline In-Reply-To: <1515595223-36144-2-git-send-email-motih@mellanox.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" My remarks for this one are mostly the same, I will try to repeat them faithfully... "convert to" -> "use" On Wed, Jan 10, 2018 at 04:40:23PM +0200, Moti Haimovsky wrote: ... > +static bool > +fs_rxq_are_offloads_valid(struct rte_eth_dev *dev, uint64_t offloads) function name can be shortened (s/_are//). > +{ > + uint64_t port_offloads = dev->data->dev_conf.rxmode.offloads; > + uint64_t queue_supp_offloads = PRIV(dev)->infos.rx_queue_offload_capa; > + uint64_t port_supp_offloads = PRIV(dev)->infos.rx_offload_capa; > + Please separate variable definition and initialization. > + 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_rx_queue_release(void *queue) > { > @@ -290,6 +313,17 @@ > fs_rx_queue_release(rxq); > dev->data->rx_queues[rx_queue_id] = NULL; > } > + /* Verify application offloads are valid for our port and queue. */ > + if (!fs_rxq_are_offloads_valid(dev, rx_conf->offloads)) { > + rte_errno = ENOTSUP; > + ERROR("%p: Rx queue offloads 0x%lx don't match port " Here, the device pointer should not be displayed. It may be a larger issue with the fail-safe, that should be fixed (when using multiple fail-safe instances), but for now, all outputs should follow the same format. Otherwise, use PRIx64 for displaying uint64_t. > + "offloads 0x%lx or supported offloads 0x%lx", > + (void *)dev, rx_conf->offloads, > + dev->data->dev_conf.rxmode.offloads, > + PRIV(dev)->infos.rx_offload_capa | > + PRIV(dev)->infos.rx_queue_offload_capa); > + return -rte_errno; > + } > rxq = rte_zmalloc(NULL, > sizeof(*rxq) + > sizeof(rte_atomic64_t) * PRIV(dev)->subs_tail, > @@ -589,12 +623,16 @@ > sizeof(default_infos)); > } else { > uint32_t rx_offload_capa; > + uint32_t rxq_offload_capa; > > rx_offload_capa = default_infos.rx_offload_capa; > + rxq_offload_capa = default_infos.rx_queue_offload_capa; > FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) { > rte_eth_dev_info_get(PORT_ID(sdev), > &PRIV(dev)->infos); > rx_offload_capa &= PRIV(dev)->infos.rx_offload_capa; > + rxq_offload_capa &= > + PRIV(dev)->infos.rx_queue_offload_capa; > } > sdev = TX_SUBDEV(dev); > rte_eth_dev_info_get(PORT_ID(sdev), &PRIV(dev)->infos); > -- > 1.8.3.1 > Regards, -- Gaëtan Rivet 6WIND