From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-f66.google.com (mail-ot1-f66.google.com [209.85.210.66]) by dpdk.org (Postfix) with ESMTP id 15DDD1B416 for ; Mon, 3 Dec 2018 16:44:09 +0100 (CET) Received: by mail-ot1-f66.google.com with SMTP id s5so11977454oth.7 for ; Mon, 03 Dec 2018 07:44:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=PPYw0VG+lxO3S4ftCAuj3GhTL455QPjCKcBbHCQqGsE=; b=KjLVJ2Z4cCX54t6Q1rfiwyuIiZ/Vq7i27GdMzOxmp9FTKqV1B0rXJkSo9SdyZ6dSg6 /+Xv2ecBcKv2l5jbgX4ax+HEWW1p+Mt0/Y3PzL2X1ENbBmJUsXPcGHKbGHIY1ABtrsOS 8EQikOHB9GPJ0SY/mILHHM/dkaVKZXTbBO4Mnx6UMqAmnn0VPwebm2iuU0m0km08ryCl FSAnG+JmvWHEQmEc+XGqWK0zaIExpaggFFeAjiqkInM+jg4m2Extg2tTMHE329NNmUrO CXRacdcb36CNGE2tLBSEc1huTdZK1whS/tdD8k69vO8pQPSmDaIHZ4ahc0Dt4uD8Z4dn /7kg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=PPYw0VG+lxO3S4ftCAuj3GhTL455QPjCKcBbHCQqGsE=; b=JECXDgLWC/2i3byj7nECHvZ4EDvj7JDNS4gQpxXx5vklHy5gpVc6EAka+oUBRxK8Dx YyzXci+HLMd88b8C0yL/6FKPibErp6w5yfxxmeUEYDcB9xH/C8dZBp8Fs1Brm3XAUhFw e8Tr/a/aHR/YDIXjvrgEBY4GznD5qvSjMfbvbIiPf3qrPh/9Uo4tNm9U8HncAmk5ZPa9 dSa/+cYRPwxsAhh+WajCsdnfHL0cS+gvO5mGg/pGtNuIgA3P8yfeMHXCCAY01g1VmKIm mw8DhaICEuGcf5ErLRZM5VDFZSJevStVIgBBsIyUX04ycPA1kbO8bwqq7PRX8/j5nIcD zeww== X-Gm-Message-State: AA+aEWbPkN2zILTHg+vPfpAxlIoh1z/aRlxmOKkv+CFjQFPe8ar1hWKY SwDUPhJejWhm2mKug+wJM8YAKxZtDRYVgSOumng= X-Google-Smtp-Source: AFSGD/WXFckzdKAp5Ehar5N1SovKOGCRGz+ZD6VAggqed+03uuXSeHXyBpI3iB00T6ItWe3T+gN7S4gaIzazyxpcgsw= X-Received: by 2002:a9d:7ac6:: with SMTP id m6mr10732788otn.62.1543851848336; Mon, 03 Dec 2018 07:44:08 -0800 (PST) MIME-Version: 1.0 References: <1542956179-80951-1-git-send-email-wenzhuo.lu@intel.com> <1542956179-80951-4-git-send-email-wenzhuo.lu@intel.com> In-Reply-To: From: Rami Rosen Date: Mon, 3 Dec 2018 17:43:57 +0200 Message-ID: To: wenzhuo.lu@intel.com Cc: dev@dpdk.org, qiming.yang@intel.com, xiaoyun.li@intel.com, jingjing.wu@intel.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH 03/19] net/ice: support device and queue ops 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, 03 Dec 2018 15:44:09 -0000 Hi, The same comment refers also to V2 of the patch, [PATCH v2 03/20] net/ice: support device and queue ops Regards, Rami Rosen On Mon, 3 Dec 2018 at 17:24, Rami Rosen wrote: > > Hi, Wenzhuo, > > > +static int > > +ice_dev_start(struct rte_eth_dev *dev) > > +{ > > + struct rte_eth_dev_data *data = dev->data; > > + struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > > + struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private); > > + uint16_t nb_rxq = 0; > > + uint16_t nb_txq, i; > > + int ret; > > + > > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > + return -E_RTE_SECONDARY; > > + > [Rami Rosen] Suppose start of a TX queue failes in the loop below. You > go to **tx_err** label, where you stop > all **RX** queues (which actually were not started at all, since they > are started only later in this method; and then you > return -EIO and the ice_dev_start() method is terminated, without > actually stopping any TX queues which were already started; > So maybe it is better to call ice_tx_queue_stop() in tx_err and > ice_rx_queue_stop() in rx_err. > Apart from it, there is a typo: "Tx queues' contex" should be =>Tx > queues' context" > > > + /* program Tx queues' contex in hardware */ > > + for (nb_txq = 0; nb_txq < data->nb_tx_queues; nb_txq++) { > > + ret = ice_tx_queue_start(dev, nb_txq); > > + if (ret) { > > + PMD_DRV_LOG(ERR, "fail to start Tx queue %u", nb_txq); > > + goto tx_err; > > + } > > + } > > + > > > + /* program Rx queues' context in hardware*/ > > + for (nb_rxq = 0; nb_rxq < data->nb_rx_queues; nb_rxq++) { > > + ret = ice_rx_queue_start(dev, nb_rxq); > > + if (ret) { > > + PMD_DRV_LOG(ERR, "fail to start Rx queue %u", nb_rxq); > > + goto rx_err; > > + } > > + } > ,,, > > > + /* stop the started queues if failed to start all queues */ > > +rx_err: > > + for (i = 0; i < nb_txq; i++) > > + ice_tx_queue_stop(dev, i); > > +tx_err: > > + for (i = 0; i < nb_rxq; i++) > > + ice_rx_queue_stop(dev, i); > > + > > + return -EIO; > > +} > > + > > Regards, > Rami Rosen