From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 64F112C8 for ; Fri, 7 Jul 2017 17:39:00 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Jul 2017 08:38:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,323,1496127600"; d="scan'208";a="990423118" Received: from dwdohert-mobl1.ger.corp.intel.com (HELO [163.33.228.167]) ([163.33.228.167]) by orsmga003.jf.intel.com with ESMTP; 07 Jul 2017 08:38:57 -0700 To: Gowrishankar References: <266cd54d289bfd6e9535a173c9607f0234f8b1b7.1499167396.git.gowrishankar.m@linux.vnet.ibm.com> Cc: dev@dpdk.org, Thomas Monjalon , Ferruh Yigit From: Declan Doherty Message-ID: <2115e096-6c7f-08a1-bec2-1860b5ff11ae@intel.com> Date: Fri, 7 Jul 2017 16:38:56 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <266cd54d289bfd6e9535a173c9607f0234f8b1b7.1499167396.git.gowrishankar.m@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] net/bonding: support bifurcated driver in eal cli using --vdev 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, 07 Jul 2017 15:39:01 -0000 On 04/07/2017 12:57 PM, Gowrishankar wrote: > From: Gowrishankar Muthukrishnan > > At present, creating bonding devices using --vdev is broken for PMD like > mlx5 as it is neither UIO nor VFIO based and hence PMD driver is unknown > to find_port_id_by_pci_addr(), as below. > > testpmd --vdev 'net_bonding0,mode=1,slave=,socket_id=0' > > PMD: bond_ethdev_parse_slave_port_kvarg(150) - Invalid slave port value > () specified > EAL: Failed to parse slave ports for bonded device net_bonding0 > > This patch fixes parsing PCI ID from bonding device params by verifying > it in RTE PCI bus, rather than checking dev->kdrv. > > Changes: > v2 - revisit fix by iterating rte_pci_bus > > Signed-off-by: Gowrishankar Muthukrishnan > --- ... > Hey Gowrishankar, I was having a look at this patch and there is the following checkpatch error. _coding style issues_ WARNING:AVOID_EXTERNS: externs should be avoided in .c files #48: FILE: drivers/net/bonding/rte_eth_bond_args.c:43: +extern struct rte_pci_bus rte_pci_bus; Looking at bit closer at the issue I think there is a simpler solution, the bonding driver really shouldn't be parsing the PCI bus directly, and since PCI devices use the PCI DBF as their name we can simply replace the all the scanning code with a simple call to rte_eth_dev_get_port_by_name API. Can you try the following changes which removes any dependency on parsing the PCI bus tree directly, which works fine for me with UIO devices. If this works I'll send this fix to the list. Thanks Declan --- drivers/net/bonding/rte_eth_bond_args.c | 109 ++++---------------------------- 1 file changed, 14 insertions(+), 95 deletions(-) diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c index ffab6e0..9fb98c6 100644 --- a/drivers/net/bonding/rte_eth_bond_args.c +++ b/drivers/net/bonding/rte_eth_bond_args.c @@ -40,7 +40,6 @@ #include "rte_eth_bond.h" #include "rte_eth_bond_private.h" -extern struct rte_pci_bus rte_pci_bus; const char *pmd_bond_init_valid_arguments[] = { PMD_BOND_SLAVE_PORT_KVARG, @@ -53,104 +52,22 @@ const char *pmd_bond_init_valid_arguments[] = { NULL }; -static inline int -find_port_id_by_pci_addr(const struct rte_pci_addr *pci_addr) -{ - struct rte_pci_device *pci_dev; - struct rte_pci_addr *eth_pci_addr; - unsigned i; - - for (i = 0; i < rte_eth_dev_count(); i++) { - pci_dev = RTE_ETH_DEV_TO_PCI(&rte_eth_devices[i]); - eth_pci_addr = &pci_dev->addr; - - if (pci_addr->bus == eth_pci_addr->bus && - pci_addr->devid == eth_pci_addr->devid && - pci_addr->domain == eth_pci_addr->domain && - pci_addr->function == eth_pci_addr->function) - return i; - } - return -1; -} - -static inline int -find_port_id_by_dev_name(const char *name) -{ - unsigned i; - - for (i = 0; i < rte_eth_dev_count(); i++) { - if (rte_eth_devices[i].data == NULL) - continue; - - if (strcmp(rte_eth_devices[i].device->name, name) == 0) - return i; - } - return -1; -} - -/** - * Parses a port identifier string to a port id by pci address, then by name, - * and finally port id. - */ -static inline int -parse_port_id(const char *port_str) -{ - struct rte_pci_device *dev; - struct rte_pci_addr dev_addr; - int port_id = -1; - - /* try parsing as pci address, physical devices */ - if (eal_parse_pci_DomBDF(port_str, &dev_addr) == 0) { - FOREACH_DEVICE_ON_PCIBUS(dev) { - if (rte_eal_compare_pci_addr(&dev->addr, &dev_addr)) - continue; - - port_id = find_port_id_by_pci_addr(&dev_addr); - } - if (port_id < 0) - return -1; - } else { - /* try parsing as device name, virtual devices */ - port_id = find_port_id_by_dev_name(port_str); - if (port_id < 0) { - char *end; - errno = 0; - - /* try parsing as port id */ - port_id = strtol(port_str, &end, 10); - if (*end != 0 || errno != 0) - return -1; - } - } - - if (port_id < 0 || port_id > RTE_MAX_ETHPORTS) { - RTE_BOND_LOG(ERR, "Slave port specified (%s) outside expected range", - port_str); - return -1; - } - return port_id; -} - int -bond_ethdev_parse_slave_port_kvarg(const char *key, +bond_ethdev_parse_slave_port_kvarg(const char *key __rte_unused, const char *value, void *extra_args) { - struct bond_ethdev_slave_ports *slave_ports; + struct bond_ethdev_slave_ports *slaves; + uint8_t pid; if (value == NULL || extra_args == NULL) return -1; - slave_ports = extra_args; + if (rte_eth_dev_get_port_by_name(value, &pid) < 0) + return -1; + + slaves = extra_args; + slaves->slaves[slaves->slave_count++] = pid; - if (strcmp(key, PMD_BOND_SLAVE_PORT_KVARG) == 0) { - int port_id = parse_port_id(value); - if (port_id < 0) { - RTE_BOND_LOG(ERR, "Invalid slave port value (%s) specified", value); - return -1; - } else - slave_ports->slaves[slave_ports->slave_count++] = - (uint8_t)port_id; - } return 0; } @@ -214,20 +131,22 @@ int bond_ethdev_parse_primary_slave_port_id_kvarg(const char *key __rte_unused, const char *value, void *extra_args) { - int primary_slave_port_id; + uint8_t pid, *primary_pid; if (value == NULL || extra_args == NULL) return -1; - primary_slave_port_id = parse_port_id(value); - if (primary_slave_port_id < 0) + if (rte_eth_dev_get_port_by_name(value, &pid) < 0) return -1; - *(uint8_t *)extra_args = (uint8_t)primary_slave_port_id; + primary_pid = (uint8_t *)extra_args; + *primary_pid = pid; return 0; } + + int bond_ethdev_parse_balance_xmit_policy_kvarg(const char *key __rte_unused, const char *value, void *extra_args) -- 2.9.4