From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f196.google.com (mail-wr0-f196.google.com [209.85.128.196]) by dpdk.org (Postfix) with ESMTP id 79AC17CEB for ; Fri, 20 Apr 2018 10:10:41 +0200 (CEST) Received: by mail-wr0-f196.google.com with SMTP id z73-v6so20565994wrb.0 for ; Fri, 20 Apr 2018 01:10:41 -0700 (PDT) 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:in-reply-to; bh=BIg85JRkwUU7yV35sAXAWLB9B597+yltOuTPoPjdmes=; b=TsWxJNfCABhBcSGr6B1jm2VNf2ZnP1NMym/vgmPXZrqoxP3LsRaEQsqROt2M9Oo6a6 3klzxebYcqW03MIKZ+wIKsSwmxXQ+y/U8HtiS9PoIr2g75LvzTp2YciaacbZNtkN2S04 XGyPMIWPxbIkNq8otYouRxoZdF6U/lLiSi9DRAi1anSlrMIotA8mJae868f5vhR4Rabd 32OLyZjg82CqOZdaVNPnfVkj+tfs2KuatJFiukBI+XOwnLc+S8THXPGF+7hSgiY8TOgw yMoQHL9hQSrq/kCY0qD19UnQjMVtYMrKg4WNG8p+q3El56j7JlceKPB5GQaMU3UdJfpo UHGA== 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:in-reply-to; bh=BIg85JRkwUU7yV35sAXAWLB9B597+yltOuTPoPjdmes=; b=N9vSa0b+15EbIOrnh3/r1UpFTXSfJr9RkE7qPon5BRYmY3EPuoRMuYRol5LhtWpAJ4 OL2kxRXN+Pcu1lpibQ4DifwG/4dqdLPaUwaNO+8FEbT5cVcDm3VLFFt/Qo0/377sSlXM ZsRc+A0lwRfW0cfQWyOPy+OH5LVc/ZJmtQzFQovDyO0vwdxLfzgrERkgZkL0S/bJWSba 0xixNhT2XHlY1sf0VCzDjGwDvwO80fyu+VA898lkEbBBXdqjHZrQFgBVO7wS/biDvgsx eSRzLWqkexPAKhgLu1Jy0UeT381WPHY8R+xOkfv9umBWYsTPk2cThils5RTDNwq+z9Qj NMvQ== X-Gm-Message-State: ALQs6tBDGmhdv7S/K7c7Vfo4GcqyRfRfK5yBLlCap0oF5wHtYjG5l4Ju 03wikvpH/hyeXBQmWYnZlilgDQ== X-Google-Smtp-Source: AIpwx4/dQiTdw8QWyVILdMQJ6x7UFVGkuUfm+6nuH0yd05OhF/cYEdkcF4Up9hKGHXqarIDzHffdYg== X-Received: by 2002:adf:9045:: with SMTP id h63-v6mr7244452wrh.218.1524211841279; Fri, 20 Apr 2018 01:10:41 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id a205sm708167wmf.18.2018.04.20.01.10.40 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 Apr 2018 01:10:40 -0700 (PDT) Date: Fri, 20 Apr 2018 10:10:26 +0200 From: Adrien Mazarguil To: Xueming Li Cc: Shahaf Shuler , Nelio Laranjeiro , Wenzhuo Lu , Jingjing Wu , Thomas Monjalon , dev@dpdk.org Message-ID: <20180420081026.GI4957@6wind.com> References: <20180409121035.148813-1-xuemingl@mellanox.com> <20180419154840.80006-1-xuemingl@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180419154840.80006-1-xuemingl@mellanox.com> Subject: Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add supported hash function check 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, 20 Apr 2018 08:10:41 -0000 On Thu, Apr 19, 2018 at 11:48:39PM +0800, Xueming Li wrote: > Add supported RSS hash function check in device configuration to > have better error verbosity for application developers. > > Signed-off-by: Xueming Li In general, I do not like API/wrappers generating log messages on their own, as those can't be caught and interpreted by applications. I believe error codes should be explicit enough. However in this specific instance, rte_eth_dev_configure() already does it a lot, so I guess it's fine: Acked-by: Adrien Mazarguil One nit below. > --- > lib/librte_ether/rte_ethdev.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 3c049ef43..4ac0fadae 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -1179,6 +1179,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, > ETHER_MAX_LEN; > } > > + /* Check that device supports requested rss hash functions. */ > + if ((dev_info.flow_type_rss_offloads | > + dev_conf->rx_adv_conf.rss_conf.rss_hf) != > + dev_info.flow_type_rss_offloads) { > + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: " > + "0x%"PRIx64", valid value: 0x%"PRIx64"\n", > + port_id, > + dev_conf->rx_adv_conf.rss_conf.rss_hf, > + dev_info.flow_type_rss_offloads); > + return -EINVAL; > + } > + > /* > * Setup new number of RX/TX queues and reconfigure device. > */ > @@ -2835,9 +2847,20 @@ rte_eth_dev_rss_hash_update(uint16_t port_id, > struct rte_eth_rss_conf *rss_conf) > { > struct rte_eth_dev *dev; > + struct rte_eth_dev_info dev_info = {0}; A "{0}" initializer for structures is nonstandard and may trigger compiler warnings. I encourage you to initialize one field C99-style instead: struct rte_eth_dev_info dev_info = { .device = NULL }; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > + rte_eth_dev_info_get(port_id, &dev_info); > + if ((dev_info.flow_type_rss_offloads | rss_conf->rss_hf) != > + dev_info.flow_type_rss_offloads) { > + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: " > + "0x%"PRIx64", valid value: 0x%"PRIx64"\n", > + port_id, > + rss_conf->rss_hf, > + dev_info.flow_type_rss_offloads); > + return -EINVAL; > + } > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_update, -ENOTSUP); > return eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev, > rss_conf)); > -- > 2.13.3 > -- Adrien Mazarguil 6WIND