From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v1 03/48] net/mlx4: check max number of ports dynamically Date: Wed, 2 Aug 2017 09:52:23 +0200 Message-ID: <20170802075223.GW19852@6wind.com> References: <04ea7bf54d1ddba834fb187ae169b3de79b5aaa0.1501598384.git.adrien.mazarguil@6wind.com> <70A7408C6E1BFB41B192A929744D8523969F2112@ALA-MBD.corp.ad.wrs.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "dev@dpdk.org" , =?utf-8?Q?Ga=C3=ABtan?= Rivet To: "Legacy, Allain" Return-path: Received: from mail-wr0-f171.google.com (mail-wr0-f171.google.com [209.85.128.171]) by dpdk.org (Postfix) with ESMTP id 4C0DC913D for ; Wed, 2 Aug 2017 09:52:33 +0200 (CEST) Received: by mail-wr0-f171.google.com with SMTP id y43so15374740wrd.3 for ; Wed, 02 Aug 2017 00:52:33 -0700 (PDT) Content-Disposition: inline In-Reply-To: <70A7408C6E1BFB41B192A929744D8523969F2112@ALA-MBD.corp.ad.wrs.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" On Tue, Aug 01, 2017 at 05:35:30PM +0000, Legacy, Allain wrote: > > -----Original Message----- > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > > Sent: Tuesday, August 01, 2017 12:54 PM > <...> > > @@ -5946,12 +5949,11 @@ mlx4_arg_parse(const char *key, const char *val, > > void *out) > > return -errno; > > } > > if (strcmp(MLX4_PMD_PORT_KVARG, key) == 0) { > > - if (tmp >= MLX4_PMD_MAX_PHYS_PORTS) { > > - ERROR("invalid port index %lu (max: %u)", > > - tmp, MLX4_PMD_MAX_PHYS_PORTS - 1); > > + if (!(conf->ports.present & (1 << tmp))) { > > + ERROR("invalid port index %lu", tmp); > > The original error included the max value. Wouldn't it be useful to report this to the > user to help them understand their mistake? Makes sense, I'll add it back. > > @@ -6085,16 +6092,16 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, > > struct rte_pci_device *pci_dev) > > } > > INFO("%u port(s) detected", device_attr.phys_port_cnt); > > > > + for (i = 0; i < device_attr.phys_port_cnt; ++i) > > + conf.ports.present |= 1 << i; > > The loop could be avoided with: > > conf.ports.present = (1 << device_attr.phys_port_cnt) - 1; I will also make that change in the next iteration, thanks. -- Adrien Mazarguil 6WIND