Hi Raslan,
I had submitted newer version and waiting for ack/merge.
dpdk.org/dev/patchwork/patch/29039/
Thanks,
Gowrishankar
On Monday 02 October 2017 02:11 PM, Raslan Darawsheh wrote:
Hi Guys,
This is gentle remainder of this patch,
Do we have any updates about it?
Kindest regards
Raslan Darawsheh
-----Original Message-----
From: gowrishankar muthukrishnan [mailto:gowrishanka...@linux.vnet.ibm.com]
Sent: Wednesday, September 6, 2017 11:59 AM
To: Thomas Monjalon <tho...@monjalon.net>
Cc: dev@dpdk.org; Gaëtan Rivet <gaetan.ri...@6wind.com>; Declan Doherty
<declan.dohe...@intel.com>; Ferruh Yigit <ferruh.yi...@intel.com>; Raslan Darawsheh
<rasl...@mellanox.com>
Subject: [Suspected-Phishing]Re: [dpdk-dev] [PATCH v2] net/bonding: support
bifurcated driver in eal cli using --vdev
Hi Thomas,
I will rework on my patch with these suggestions and send new version.
Thanks Declan and Gaëtan. Thank you Thomas too reminding me.
Regards,
Gowrishankar
On Tuesday 05 September 2017 02:43 PM, Thomas Monjalon wrote:
Ping - any news?
31/07/2017 16:34, Gaëtan Rivet:
Hi Gowrishankar, Declan,
On Mon, Jul 10, 2017 at 12:02:24PM +0530, gowrishankar muthukrishnan wrote:
On Friday 07 July 2017 09:08 PM, Declan Doherty wrote:
On 04/07/2017 12:57 PM, Gowrishankar wrote:
From: Gowrishankar Muthukrishnan
<gowrishanka...@linux.vnet.ibm.com>
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 <EAL args> --vdev 'net_bonding0,mode=1,slave=<PCI>,socket_id=0'
PMD: bond_ethdev_parse_slave_port_kvarg(150) - Invalid slave port
value (<PCI ID>) 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
<gowrishanka...@linux.vnet.ibm.com>
---
...
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;
Hi Declan,
Thank you for your review.
Yes, but I also saw some references like above in older code.
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.
I agree that it would be better to be able to use the ether API for
this.
The issue is that PCI devices are inconsistent regarding their names.
The possibility is given to the user to employ the simplified BDF
format for PCI device name, instead of the DomBDF format.
Unfortunately, the default device name for a PCI device is in the
DomBDF format. This means that the name won't match if the device was
probed by using the PCI blacklist mode (the default PCI mode).
The matching must be refined.
But you are removing an option to mention ports by PCI addresses
right (as I see parse_port_id() completely removed in your patch) ?.
IMO, we just need to check if given eth pci id (incase we mention
ports ib PCI ID) is one of what EAL scanned in PCI. Also, slaves
should not be from any blacklisted PCI ids (as we test with -b or -w).
Declan is right about the iteration of PCI devices. The device list
for the PCI bus is private, the extern declaration to the rte_pci_bus
is the telltale sign that there is something wrong in the approach here.
In order to respect the new rte_bus logic, I think what you want to
achieve can be done by using the rte_bus->find_device with the
correct device comparison function.
static int
pci_addr_cmp(const struct rte_device *dev, const void *_pci_addr) {
struct rte_pci_device *pdev;
char *addr = _pci_addr;
struct rte_pci_addr paddr;
static struct rte_bus *pci_bus = NULL;
if (pci_bus == NULL)
pci_bus = rte_bus_find_by_name("pci");
if (pci_bus->parse(addr, &paddr) != 0) {
/* Invalid PCI addr given as input. */
return -1;
}
pdev = RTE_DEV_TO_PCI(dev);
return rte_eal_compare_pci_addr(&pdev->addr, &paddr); }
Then verify that you are able to get a device by using it as follows:
{
struct rte_bus *pci_bus;
struct rte_device *dev;
pci_bus = rte_bus_find_by_name("pci");
if (pci_bus == NULL) {
RTE_LOG(ERR, PMD, "Unable to find PCI bus\n");
return -1;
}
dev = pci_bus->find_device(NULL, pci_addr_cmp, devname);
if (dev == NULL) {
RTE_LOG(ERR, PMD, "Unable to find the device %s to enslave.\n",
devname);
return -EINVAL;
}
}
I hope it's clear enough. You can find examples of use for this API
in lib/librte_eal/common/eal_common_dev.c
It's a quick implementation to outline the possible direction, I
haven't compiled it. It should be refined.
For example, the PCI address validation should not be happening in
the comparison function, the pci_bus could be matched once instead of
twice, etc...
But the logic should work.
Best regards,
--
Regards,
Gowrishankar M
Linux Networking
