DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [dpdk-stable] [PATCH] net/ixgbe: fix API parameter checking
From: Iremonger, Bernard @ 2017-01-11 17:05 UTC (permalink / raw)
  To: Yigit, Ferruh, dev@dpdk.org, Lu, Wenzhuo; +Cc: stable@dpdk.org
In-Reply-To: <daac85e4-5687-920c-2047-5d0467c2723f@intel.com>

Hi Ferruh,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 3:15 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org;
> Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: stable@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH] net/ixgbe: fix API parameter checking
> 
> On 1/11/2017 2:25 PM, Bernard Iremonger wrote:
> > Add checks to rte_pmd_ixgbe_* API's to ensure that the port is an
> > ixgbe port.
> >
> > Fixes: 49e248223e9f ("net/ixgbe: add API for VF management")
> >
> > CC: stable@dpdk.org
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 71
> > ++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 69 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index b7ddd4f..ca14104 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -1,7 +1,7 @@
> >  /*-
> >   *   BSD LICENSE
> >   *
> > - *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
> > + *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
> >   *   All rights reserved.
> >   *
> >   *   Redistribution and use in source and binary forms, with or without
> > @@ -4066,6 +4066,12 @@ rte_pmd_ixgbe_set_vf_mac_addr(uint8_t port,
> uint16_t vf,
> >  	dev = &rte_eth_devices[port];
> >  	rte_eth_dev_info_get(port, &dev_info);
> >
> > +	if (!strstr(dev_info.driver_name, "ixgbe"))
> > +		return -ENOTSUP;
> > +
> > +	if (strstr(dev_info.driver_name, "ixgbe_vf"))
> > +		return -ENOTSUP;
> > +
> 
> This part seems common for all functions, what do you think exporting this
> into a static function?
> 
> Also in the feature if you need to update the method to decide if this port_id
> is supported or not, only that function will be effected.
> 
> <...>
Ok, I will put the checks into a static function and send a v2.

Regards,

Bernard.

^ permalink raw reply

* [PATCH v3 1/3] pci: If a driver's probe function fails, unmap resources.
From: Ben Walker @ 2017-01-11 17:10 UTC (permalink / raw)
  To: dev; +Cc: Ben Walker
In-Reply-To: <1479931644-78960-1-git-send-email-benjamin.walker@intel.com>

If resources were mapped prior to probe, unmap them
if probe fails.

This does not handle the case where the kernel driver was
forcibly unbound prior to probe.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 lib/librte_eal/common/eal_common_pci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 33485bc..72547bd 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -210,8 +210,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 
 		/* call the driver probe() function */
 		ret = dr->probe(dr, dev);
-		if (ret)
+		if (ret) {
 			dev->driver = NULL;
+			if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+				rte_eal_pci_unmap_device(dev);
+		}
 
 		return ret;
 	}
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 2/3] pci: Separate detaching ethernet ports from PCI devices
From: Ben Walker @ 2017-01-11 17:10 UTC (permalink / raw)
  To: dev; +Cc: Ben Walker
In-Reply-To: <20170111171012.126251-1-benjamin.walker@intel.com>

Attaching and detaching ethernet ports from an application
is not the same thing as physically removing a PCI device,
so clarify the flags indicating support. All PCI devices
are assumed to be physically removable, so no flag is
necessary in the PCI layer.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 doc/guides/prog_guide/port_hotplug_framework.rst | 2 +-
 drivers/net/bnxt/bnxt_ethdev.c                   | 3 ++-
 drivers/net/e1000/em_ethdev.c                    | 4 ++--
 drivers/net/e1000/igb_ethdev.c                   | 7 ++++---
 drivers/net/fm10k/fm10k_ethdev.c                 | 4 ++--
 drivers/net/i40e/i40e_ethdev.c                   | 4 ++--
 drivers/net/i40e/i40e_ethdev_vf.c                | 3 ++-
 drivers/net/ixgbe/ixgbe_ethdev.c                 | 7 ++++---
 drivers/net/nfp/nfp_net.c                        | 4 ++--
 drivers/net/virtio/virtio_ethdev.c               | 6 ++++--
 drivers/net/vmxnet3/vmxnet3_ethdev.c             | 3 ++-
 drivers/net/xenvirt/rte_eth_xenvirt.c            | 2 +-
 lib/librte_eal/common/include/rte_pci.h          | 2 --
 lib/librte_ether/rte_ethdev.c                    | 2 --
 14 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/doc/guides/prog_guide/port_hotplug_framework.rst b/doc/guides/prog_guide/port_hotplug_framework.rst
index 6e4436e..8c49319 100644
--- a/doc/guides/prog_guide/port_hotplug_framework.rst
+++ b/doc/guides/prog_guide/port_hotplug_framework.rst
@@ -106,5 +106,5 @@ Limitations
 
 *       Not all PMDs support detaching feature.
         To know whether a PMD can support detaching, search for the
-        "RTE_PCI_DRV_DETACHABLE" flag in PMD implementation. If the flag is
+        "RTE_ETH_DEV_DETACHABLE" flag in rte_eth_dev::data::dev_flags. If the flag is
         defined in the PMD, detaching is supported.
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7518b6b..eead73b 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1057,6 +1057,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
 		RTE_LOG(INFO, PMD, "%s", bnxt_version);
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	bp = eth_dev->data->dev_private;
 
 	if (bnxt_vf_pciid(pci_dev->id.device_id))
@@ -1168,7 +1169,7 @@ static struct eth_driver bnxt_rte_pmd = {
 	.pci_drv = {
 		    .id_table = bnxt_pci_id_map,
 		    .drv_flags = RTE_PCI_DRV_NEED_MAPPING |
-			    RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_INTR_LSC,
+			    RTE_PCI_DRV_INTR_LSC,
 		    .probe = rte_eth_dev_pci_probe,
 		    .remove = rte_eth_dev_pci_remove
 		    },
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 5f6e66d..17e30db 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -313,6 +313,7 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
 	}
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
 	hw->device_id = pci_dev->id.device_id;
@@ -392,8 +393,7 @@ eth_em_dev_uninit(struct rte_eth_dev *eth_dev)
 static struct eth_driver rte_em_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_em_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2bb57f5..2301235 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -774,6 +774,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
 	}
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	hw->hw_addr= (void *)pci_dev->mem_resource[0].addr;
 
@@ -982,6 +983,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
 
 	pci_dev = E1000_DEV_TO_PCI(eth_dev);
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	hw->device_id = pci_dev->id.device_id;
 	hw->vendor_id = pci_dev->id.vendor_id;
@@ -1085,8 +1087,7 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
 static struct eth_driver rte_igb_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_igb_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
@@ -1101,7 +1102,7 @@ static struct eth_driver rte_igb_pmd = {
 static struct eth_driver rte_igbvf_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_igbvf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index d8353e9..dd021e4 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -2858,6 +2858,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 		return 0;
 
 	rte_eth_copy_pci_info(dev, pdev);
+	dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
 	memset(macvlan, 0, sizeof(*macvlan));
@@ -3080,8 +3081,7 @@ static const struct rte_pci_id pci_id_fm10k_map[] = {
 static struct eth_driver rte_pmd_fm10k = {
 	.pci_drv = {
 		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 0eb4c99..1687144 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -671,8 +671,7 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
 static struct eth_driver rte_i40e_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_i40e_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
@@ -959,6 +958,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	intr_handle = &pci_dev->intr_handle;
 
 	rte_eth_copy_pci_info(dev, pci_dev);
+	dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	pf->adapter = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	pf->adapter->eth_dev = dev;
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 0dc0af5..b32f9e7 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1460,6 +1460,7 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev)
 	}
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	hw->vendor_id = pci_dev->id.vendor_id;
 	hw->device_id = pci_dev->id.device_id;
@@ -1529,7 +1530,7 @@ i40evf_dev_uninit(struct rte_eth_dev *eth_dev)
 static struct eth_driver rte_i40evf_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_i40evf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 060772d..b7415e8 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1134,6 +1134,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
 	}
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	/* Vendor and Device ID need to be set before init of shared code */
 	hw->device_id = pci_dev->id.device_id;
@@ -1424,6 +1425,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	}
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	hw->device_id = pci_dev->id.device_id;
 	hw->vendor_id = pci_dev->id.vendor_id;
@@ -1566,8 +1568,7 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 static struct eth_driver rte_ixgbe_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_ixgbe_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
@@ -1582,7 +1583,7 @@ static struct eth_driver rte_ixgbe_pmd = {
 static struct eth_driver rte_ixgbevf_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_ixgbevf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index e85315f..88169a7 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2338,6 +2338,7 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 
 	pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	hw->device_id = pci_dev->id.device_id;
 	hw->vendor_id = pci_dev->id.vendor_id;
@@ -2475,8 +2476,7 @@ static struct rte_pci_id pci_id_nfp_net_map[] = {
 static struct eth_driver rte_nfp_net_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_nfp_net_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			     RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 54ea7d7..d1747f6 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1212,8 +1212,10 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 	else
 		eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
 
-	if (pci_dev)
+	if (pci_dev) {
 		rte_eth_copy_pci_info(eth_dev, pci_dev);
+		eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	}
 
 	rx_func_get(eth_dev);
 
@@ -1383,7 +1385,7 @@ static struct eth_driver rte_virtio_pmd = {
 			.name = "net_virtio",
 		},
 		.id_table = pci_id_virtio_map,
-		.drv_flags = RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = 0,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 54533ca..52e3644 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -250,6 +250,7 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 
 	/* Vendor and Device ID need to be set before init of shared code */
 	hw->device_id = pci_dev->id.device_id;
@@ -340,7 +341,7 @@ eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev)
 static struct eth_driver rte_vmxnet3_pmd = {
 	.pci_drv = {
 		.id_table = pci_id_vmxnet3_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
diff --git a/drivers/net/xenvirt/rte_eth_xenvirt.c b/drivers/net/xenvirt/rte_eth_xenvirt.c
index 609824b..e7640ab 100644
--- a/drivers/net/xenvirt/rte_eth_xenvirt.c
+++ b/drivers/net/xenvirt/rte_eth_xenvirt.c
@@ -672,7 +672,7 @@ eth_dev_xenvirt_create(const char *name, const char *params,
 	eth_dev->data = data;
 	eth_dev->dev_ops = &ops;
 
-	eth_dev->data->dev_flags = RTE_PCI_DRV_DETACHABLE;
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	eth_dev->data->kdrv = RTE_KDRV_NONE;
 	eth_dev->data->drv_name = drivername;
 	eth_dev->driver = NULL;
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 87cad59..8557e47 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -216,8 +216,6 @@ struct rte_pci_driver {
 #define RTE_PCI_DRV_NEED_MAPPING 0x0001
 /** Device driver supports link state interrupt */
 #define RTE_PCI_DRV_INTR_LSC	0x0008
-/** Device driver supports detaching capability */
-#define RTE_PCI_DRV_DETACHABLE	0x0010
 
 /**
  * A structure describing a PCI mapping.
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 9dea1f1..222e267 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3272,8 +3272,6 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev, struct rte_pci_device *pci_de
 	eth_dev->data->dev_flags = 0;
 	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
 		eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
-	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_DETACHABLE)
-		eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
 
 	eth_dev->data->kdrv = pci_dev->kdrv;
 	eth_dev->data->numa_node = pci_dev->device.numa_node;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 3/3] pci: Pass rte_pci_addr to functions instead of separate args
From: Ben Walker @ 2017-01-11 17:10 UTC (permalink / raw)
  To: dev; +Cc: Ben Walker
In-Reply-To: <20170111171012.126251-1-benjamin.walker@intel.com>

Instead of passing domain, bus, devid, func, just pass
an rte_pci_addr.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 4350134..2933f00 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -228,8 +228,7 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
 
 /* Scan one pci sysfs entry, and fill the devices list from it. */
 static int
-pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
-	     uint8_t devid, uint8_t function)
+pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 {
 	char filename[PATH_MAX];
 	unsigned long tmp;
@@ -242,10 +241,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 		return -1;
 
 	memset(dev, 0, sizeof(*dev));
-	dev->addr.domain = domain;
-	dev->addr.bus = bus;
-	dev->addr.devid = devid;
-	dev->addr.function = function;
+	dev->addr = *addr;
 
 	/* get vendor id */
 	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
@@ -390,16 +386,14 @@ pci_update_device(const struct rte_pci_addr *addr)
 		 pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
 		 addr->function);
 
-	return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
-				addr->function);
+	return pci_scan_one(filename, addr);
 }
 
 /*
  * split up a pci address into its constituent parts.
  */
 static int
-parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
-		uint8_t *bus, uint8_t *devid, uint8_t *function)
+parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
 {
 	/* first split on ':' */
 	union splitaddr {
@@ -427,10 +421,10 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
 
 	/* now convert to int values */
 	errno = 0;
-	*domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
-	*bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
-	*devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
-	*function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
+	addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
+	addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
+	addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
+	addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
 	if (errno != 0)
 		goto error;
 
@@ -451,8 +445,7 @@ rte_eal_pci_scan(void)
 	struct dirent *e;
 	DIR *dir;
 	char dirname[PATH_MAX];
-	uint16_t domain;
-	uint8_t bus, devid, function;
+	struct rte_pci_addr addr;
 
 	dir = opendir(pci_get_sysfs_path());
 	if (dir == NULL) {
@@ -461,20 +454,21 @@ rte_eal_pci_scan(void)
 		return -1;
 	}
 
+
 	while ((e = readdir(dir)) != NULL) {
 		if (e->d_name[0] == '.')
 			continue;
 
-		if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
-				&bus, &devid, &function) != 0)
+		if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
 			continue;
 
 		snprintf(dirname, sizeof(dirname), "%s/%s",
 				pci_get_sysfs_path(), e->d_name);
-		if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
+		if (pci_scan_one(dirname, &addr) < 0)
 			goto error;
 	}
 	closedir(dir);
+
 	return 0;
 
 error:
-- 
2.9.3

^ permalink raw reply related

* [PATCH 0/2] crypto: add user defined name initializing parameter
From: Fan Zhang @ 2017-01-11 17:13 UTC (permalink / raw)
  To: dev; +Cc: pablo.de.lara.guarch

This patchset adds a user defined name initializing parameter to all
software cryptodevs.

Fan Zhang (2):
  cryptodev: add user defined name initializing parameter to software
    PMD
  crypto: add user defined name initializing parameter parsing to
    software PMDs

 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c   | 58 ++++++++++------------------
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 61 ++++++++++--------------------
 drivers/crypto/kasumi/rte_kasumi_pmd.c     | 58 +++++++++++-----------------
 drivers/crypto/null/null_crypto_pmd.c      | 55 ++++++++++-----------------
 drivers/crypto/openssl/rte_openssl_pmd.c   | 55 ++++++++++-----------------
 drivers/crypto/snow3g/rte_snow3g_pmd.c     | 59 +++++++++++------------------
 drivers/crypto/zuc/rte_zuc_pmd.c           | 59 +++++++++++------------------
 lib/librte_cryptodev/rte_cryptodev.c       | 45 ++++++++++++++++++++++
 lib/librte_cryptodev/rte_cryptodev.h       |  7 ++--
 lib/librte_cryptodev/rte_cryptodev_pmd.h   |  7 ++++
 10 files changed, 203 insertions(+), 261 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH 1/2] cryptodev: add user defined name initializing parameter to software PMD
From: Fan Zhang @ 2017-01-11 17:13 UTC (permalink / raw)
  To: dev; +Cc: pablo.de.lara.guarch
In-Reply-To: <1484154786-91531-1-git-send-email-roy.fan.zhang@intel.com>

This patch adds a user defined name initializing parameter to cryptodev
library.

Originally, for software cryptodev PMD, the vdev name parameter is
treated as the driver identifier, and will create an unique name for each
device automatically, which is not necessarily as same as the vdev
parameter.

This patch allows the user to either create a unique name for his software
cryptodev, or by default, let the system creates a unique one. This should
help the user managing the created cryptodevs easily.

Examples:
CLI command fragment 1: --vdev "crypto_aesni_gcm_pmd"
The above command will result in creating a AESNI-GCM PMD with name of
"crypto_aesni_gcm_X", where postfix X is the number assigned by the system,
starting from 0. This fragment can be placed in the same CLI command
multiple times, resulting the postfixs incremented by one for each new
device.

CLI command fragment 2: --vdev "crypto_aesni_gcm_pmd,name=gcm1"
The above command will result in creating a AESNI-GCM PMD with name of
"gcm1". This fragment can be placed in the same CLI command multiple
times, as long as each having a unique name value.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 lib/librte_cryptodev/rte_cryptodev.c     | 45 ++++++++++++++++++++++++++++++++
 lib/librte_cryptodev/rte_cryptodev.h     |  7 +++--
 lib/librte_cryptodev/rte_cryptodev_pmd.h |  7 +++++
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 127e8d0..3b6da8b 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -101,11 +101,13 @@ struct rte_cryptodev_callback {
 	uint32_t active;			/**< Callback is executing */
 };
 
+#define RTE_CRYPTODEV_VDEV_NAME				("name")
 #define RTE_CRYPTODEV_VDEV_MAX_NB_QP_ARG		("max_nb_queue_pairs")
 #define RTE_CRYPTODEV_VDEV_MAX_NB_SESS_ARG		("max_nb_sessions")
 #define RTE_CRYPTODEV_VDEV_SOCKET_ID			("socket_id")
 
 static const char *cryptodev_vdev_valid_params[] = {
+	RTE_CRYPTODEV_VDEV_NAME,
 	RTE_CRYPTODEV_VDEV_MAX_NB_QP_ARG,
 	RTE_CRYPTODEV_VDEV_MAX_NB_SESS_ARG,
 	RTE_CRYPTODEV_VDEV_SOCKET_ID
@@ -143,6 +145,18 @@ parse_integer_arg(const char *key __rte_unused,
 	return 0;
 }
 
+/** Parse name */
+static int
+parse_name_arg(const char *key __rte_unused,
+		const char *value, void *extra_args)
+{
+	struct rte_crypto_vdev_init_params *params = extra_args;
+
+	strncpy(params->name, value, RTE_CRYPTODEV_NAME_MAX_LEN);
+
+	return 0;
+}
+
 int
 rte_cryptodev_parse_vdev_init_params(struct rte_crypto_vdev_init_params *params,
 		const char *input_args)
@@ -179,6 +193,12 @@ rte_cryptodev_parse_vdev_init_params(struct rte_crypto_vdev_init_params *params,
 		if (ret < 0)
 			goto free_kvlist;
 
+		ret = rte_kvargs_process(kvlist, RTE_CRYPTODEV_VDEV_NAME,
+					&parse_name_arg,
+					params);
+		if (ret < 0)
+			goto free_kvlist;
+
 		if (params->socket_id >= number_of_sockets()) {
 			CDEV_LOG_ERR("Invalid socket id specified to create "
 				"the virtual crypto device on");
@@ -1205,3 +1225,28 @@ rte_crypto_op_pool_create(const char *name, enum rte_crypto_op_type type,
 
 	return mp;
 }
+
+int
+rte_cryptodev_pmd_create_dev_name(char *name, const char *dev_name_prefix)
+{
+	struct rte_cryptodev *dev = NULL;
+	uint32_t i = 0;
+
+	if (name == NULL)
+		return -EINVAL;
+
+	for (i = 0; i < RTE_CRYPTO_MAX_DEVS; i++) {
+		int ret = snprintf(name, RTE_CRYPTODEV_NAME_MAX_LEN,
+				"%s_%u", dev_name_prefix, i);
+
+		if (ret < 0)
+			return ret;
+
+		dev = rte_cryptodev_pmd_get_named_dev(name);
+		if (!dev)
+			return 0;
+	}
+
+	return -1;
+}
+
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index 8f63e8f..b5399af 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -300,6 +300,8 @@ struct rte_cryptodev_stats {
 	/**< Total error count on operations dequeued */
 };
 
+#define RTE_CRYPTODEV_NAME_MAX_LEN	(64)
+/**< Max length of name of crypto PMD */
 #define RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS	8
 #define RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS	2048
 
@@ -311,6 +313,7 @@ struct rte_crypto_vdev_init_params {
 	unsigned max_nb_queue_pairs;
 	unsigned max_nb_sessions;
 	uint8_t socket_id;
+	char name[RTE_CRYPTODEV_NAME_MAX_LEN];
 };
 
 /**
@@ -635,10 +638,6 @@ struct rte_cryptodev {
 	/**< Flag indicating the device is attached */
 } __rte_cache_aligned;
 
-
-#define RTE_CRYPTODEV_NAME_MAX_LEN	(64)
-/**< Max length of name of crypto PMD */
-
 /**
  *
  * The data part, with no function pointers, associated with each device.
diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index abfe2dc..dc57bfa 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -519,6 +519,13 @@ int rte_cryptodev_pci_probe(struct rte_pci_driver *pci_drv,
  */
 int rte_cryptodev_pci_remove(struct rte_pci_device *pci_dev);
 
+/**
+ * @internal
+ * Create unique device name
+ */
+int
+rte_cryptodev_pmd_create_dev_name(char *name, const char *dev_name_prefix);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/2] crypto: add user defined name initializing parameter parsing to software PMDs
From: Fan Zhang @ 2017-01-11 17:13 UTC (permalink / raw)
  To: dev; +Cc: pablo.de.lara.guarch
In-Reply-To: <1484154786-91531-1-git-send-email-roy.fan.zhang@intel.com>

This patch adds user defined name parsing feature to software PMDs.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c   | 58 ++++++++++------------------
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 61 ++++++++++--------------------
 drivers/crypto/kasumi/rte_kasumi_pmd.c     | 58 +++++++++++-----------------
 drivers/crypto/null/null_crypto_pmd.c      | 55 ++++++++++-----------------
 drivers/crypto/openssl/rte_openssl_pmd.c   | 55 ++++++++++-----------------
 drivers/crypto/snow3g/rte_snow3g_pmd.c     | 59 +++++++++++------------------
 drivers/crypto/zuc/rte_zuc_pmd.c           | 59 +++++++++++------------------
 7 files changed, 148 insertions(+), 257 deletions(-)

diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
index dba5e15..ddb2e0e 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
@@ -43,27 +43,6 @@
 
 #include "aesni_gcm_pmd_private.h"
 
-/**
- * Global static parameter used to create a unique name for each AES-NI multi
- * buffer crypto device.
- */
-static unsigned unique_name_id;
-
-static inline int
-create_unique_device_name(char *name, size_t size)
-{
-	int ret;
-
-	if (name == NULL)
-		return -EINVAL;
-
-	ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD),
-			unique_name_id++);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
 static int
 aesni_gcm_calculate_hash_sub_key(uint8_t *hsubkey, unsigned hsubkey_length,
 		uint8_t *aeskey, unsigned aeskey_length)
@@ -407,14 +386,23 @@ aesni_gcm_pmd_dequeue_burst(void *queue_pair,
 static int aesni_gcm_remove(const char *name);
 
 static int
-aesni_gcm_create(const char *name,
-		struct rte_crypto_vdev_init_params *init_params)
+aesni_gcm_create(struct rte_crypto_vdev_init_params *init_params)
 {
 	struct rte_cryptodev *dev;
-	char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
 	struct aesni_gcm_private *internals;
 	enum aesni_gcm_vector_mode vector_mode;
 
+	if (init_params->name[0] == '\0') {
+		int ret = rte_cryptodev_pmd_create_dev_name(
+				init_params->name,
+				RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD));
+
+		if (ret < 0) {
+			GCM_LOG_ERR("failed to create unique name");
+			return ret;
+		}
+	}
+
 	/* Check CPU for support for AES instruction set */
 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
 		GCM_LOG_ERR("AES instructions not supported by CPU");
@@ -433,15 +421,7 @@ aesni_gcm_create(const char *name,
 		return -EFAULT;
 	}
 
-	/* create a unique device name */
-	if (create_unique_device_name(crypto_dev_name,
-			RTE_CRYPTODEV_NAME_MAX_LEN) != 0) {
-		GCM_LOG_ERR("failed to create unique cryptodev name");
-		return -EINVAL;
-	}
-
-
-	dev = rte_cryptodev_pmd_virtual_dev_init(crypto_dev_name,
+	dev = rte_cryptodev_pmd_virtual_dev_init(init_params->name,
 			sizeof(struct aesni_gcm_private), init_params->socket_id);
 	if (dev == NULL) {
 		GCM_LOG_ERR("failed to create cryptodev vdev");
@@ -484,9 +464,9 @@ aesni_gcm_create(const char *name,
 	return 0;
 
 init_error:
-	GCM_LOG_ERR("driver %s: create failed", name);
+	GCM_LOG_ERR("driver %s: create failed", init_params->name);
 
-	aesni_gcm_remove(crypto_dev_name);
+	aesni_gcm_remove(init_params->name);
 	return -EFAULT;
 }
 
@@ -496,19 +476,23 @@ aesni_gcm_probe(const char *name, const char *input_args)
 	struct rte_crypto_vdev_init_params init_params = {
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS,
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS,
-		rte_socket_id()
+		rte_socket_id(),
+		{0}
 	};
 
 	rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
 
 	RTE_LOG(INFO, PMD, "Initialising %s on NUMA node %d\n", name,
 			init_params.socket_id);
+	if (init_params.name[0] != '\0')
+		RTE_LOG(INFO, PMD, "  User defined name = %s\n",
+			init_params.name);
 	RTE_LOG(INFO, PMD, "  Max number of queue pairs = %d\n",
 			init_params.max_nb_queue_pairs);
 	RTE_LOG(INFO, PMD, "  Max number of sessions = %d\n",
 			init_params.max_nb_sessions);
 
-	return aesni_gcm_create(name, &init_params);
+	return aesni_gcm_create(&init_params);
 }
 
 static int
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index f07cd07..26dc05e 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -40,27 +40,6 @@
 
 #include "rte_aesni_mb_pmd_private.h"
 
-/**
- * Global static parameter used to create a unique name for each AES-NI multi
- * buffer crypto device.
- */
-static unsigned unique_name_id;
-
-static inline int
-create_unique_device_name(char *name, size_t size)
-{
-	int ret;
-
-	if (name == NULL)
-		return -EINVAL;
-
-	ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD),
-			unique_name_id++);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
 typedef void (*hash_one_block_t)(void *data, void *digest);
 typedef void (*aes_keyexp_t)(void *key, void *enc_exp_keys, void *dec_exp_keys);
 
@@ -598,18 +577,21 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 static int cryptodev_aesni_mb_remove(const char *name);
 
 static int
-cryptodev_aesni_mb_create(const char *name,
-		struct rte_crypto_vdev_init_params *init_params)
+cryptodev_aesni_mb_create(struct rte_crypto_vdev_init_params *init_params)
 {
 	struct rte_cryptodev *dev;
-	char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
 	struct aesni_mb_private *internals;
 	enum aesni_mb_vector_mode vector_mode;
 
-	/* Check CPU for support for AES instruction set */
-	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
-		MB_LOG_ERR("AES instructions not supported by CPU");
-		return -EFAULT;
+	if (init_params->name[0] == '\0') {
+		int ret = rte_cryptodev_pmd_create_dev_name(
+				init_params->name,
+				RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD));
+
+		if (ret < 0) {
+			MB_LOG_ERR("failed to create unique name");
+			return ret;
+		}
 	}
 
 	/* Check CPU for supported vector instruction set */
@@ -624,15 +606,7 @@ cryptodev_aesni_mb_create(const char *name,
 		return -EFAULT;
 	}
 
-	/* create a unique device name */
-	if (create_unique_device_name(crypto_dev_name,
-			RTE_CRYPTODEV_NAME_MAX_LEN) != 0) {
-		MB_LOG_ERR("failed to create unique cryptodev name");
-		return -EINVAL;
-	}
-
-
-	dev = rte_cryptodev_pmd_virtual_dev_init(crypto_dev_name,
+	dev = rte_cryptodev_pmd_virtual_dev_init(init_params->name,
 			sizeof(struct aesni_mb_private), init_params->socket_id);
 	if (dev == NULL) {
 		MB_LOG_ERR("failed to create cryptodev vdev");
@@ -673,9 +647,10 @@ cryptodev_aesni_mb_create(const char *name,
 
 	return 0;
 init_error:
-	MB_LOG_ERR("driver %s: cryptodev_aesni_create failed", name);
+	MB_LOG_ERR("driver %s: cryptodev_aesni_create failed",
+			init_params->name);
 
-	cryptodev_aesni_mb_remove(crypto_dev_name);
+	cryptodev_aesni_mb_remove(init_params->name);
 	return -EFAULT;
 }
 
@@ -687,19 +662,23 @@ cryptodev_aesni_mb_probe(const char *name,
 	struct rte_crypto_vdev_init_params init_params = {
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS,
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS,
-		rte_socket_id()
+		rte_socket_id(),
+		""
 	};
 
 	rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
 
 	RTE_LOG(INFO, PMD, "Initialising %s on NUMA node %d\n", name,
 			init_params.socket_id);
+	if (init_params.name[0] != '\0')
+		RTE_LOG(INFO, PMD, "  User defined name = %s\n",
+			init_params.name);
 	RTE_LOG(INFO, PMD, "  Max number of queue pairs = %d\n",
 			init_params.max_nb_queue_pairs);
 	RTE_LOG(INFO, PMD, "  Max number of sessions = %d\n",
 			init_params.max_nb_sessions);
 
-	return cryptodev_aesni_mb_create(name, &init_params);
+	return cryptodev_aesni_mb_create(&init_params);
 }
 
 static int
diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c b/drivers/crypto/kasumi/rte_kasumi_pmd.c
index b119da2..c38c851 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd.c
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c
@@ -47,27 +47,6 @@
 #define KASUMI_MAX_BURST 4
 #define BYTE_LEN 8
 
-/**
- * Global static parameter used to create a unique name for each KASUMI
- * crypto device.
- */
-static unsigned unique_name_id;
-
-static inline int
-create_unique_device_name(char *name, size_t size)
-{
-	int ret;
-
-	if (name == NULL)
-		return -EINVAL;
-
-	ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_KASUMI_PMD),
-			unique_name_id++);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
 /** Get xform chain order. */
 static enum kasumi_operation
 kasumi_get_mode(const struct rte_crypto_sym_xform *xform)
@@ -559,14 +538,23 @@ kasumi_pmd_dequeue_burst(void *queue_pair,
 static int cryptodev_kasumi_remove(const char *name);
 
 static int
-cryptodev_kasumi_create(const char *name,
-		struct rte_crypto_vdev_init_params *init_params)
+cryptodev_kasumi_create(struct rte_crypto_vdev_init_params *init_params)
 {
 	struct rte_cryptodev *dev;
-	char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
 	struct kasumi_private *internals;
 	uint64_t cpu_flags = 0;
 
+	if (init_params->name[0] == '\0') {
+		int ret = rte_cryptodev_pmd_create_dev_name(
+				init_params->name,
+				RTE_STR(CRYPTODEV_NAME_KASUMI_PMD));
+
+		if (ret < 0) {
+			KASUMI_LOG_ERR("failed to create unique name");
+			return ret;
+		}
+	}
+
 	/* Check CPU for supported vector instruction set */
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX))
 		cpu_flags |= RTE_CRYPTODEV_FF_CPU_AVX;
@@ -577,14 +565,7 @@ cryptodev_kasumi_create(const char *name,
 		return -EFAULT;
 	}
 
-	/* Create a unique device name. */
-	if (create_unique_device_name(crypto_dev_name,
-			RTE_CRYPTODEV_NAME_MAX_LEN) != 0) {
-		KASUMI_LOG_ERR("failed to create unique cryptodev name");
-		return -EINVAL;
-	}
-
-	dev = rte_cryptodev_pmd_virtual_dev_init(crypto_dev_name,
+	dev = rte_cryptodev_pmd_virtual_dev_init(init_params->name,
 			sizeof(struct kasumi_private), init_params->socket_id);
 	if (dev == NULL) {
 		KASUMI_LOG_ERR("failed to create cryptodev vdev");
@@ -609,9 +590,10 @@ cryptodev_kasumi_create(const char *name,
 
 	return 0;
 init_error:
-	KASUMI_LOG_ERR("driver %s: cryptodev_kasumi_create failed", name);
+	KASUMI_LOG_ERR("driver %s: cryptodev_kasumi_create failed",
+			init_params->name);
 
-	cryptodev_kasumi_remove(crypto_dev_name);
+	cryptodev_kasumi_remove(init_params->name);
 	return -EFAULT;
 }
 
@@ -622,19 +604,23 @@ cryptodev_kasumi_probe(const char *name,
 	struct rte_crypto_vdev_init_params init_params = {
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS,
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS,
-		rte_socket_id()
+		rte_socket_id(),
+		{0}
 	};
 
 	rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
 
 	RTE_LOG(INFO, PMD, "Initialising %s on NUMA node %d\n", name,
 			init_params.socket_id);
+	if (init_params.name[0] != '\0')
+		RTE_LOG(INFO, PMD, "  User defined name = %s\n",
+			init_params.name);
 	RTE_LOG(INFO, PMD, "  Max number of queue pairs = %d\n",
 			init_params.max_nb_queue_pairs);
 	RTE_LOG(INFO, PMD, "  Max number of sessions = %d\n",
 			init_params.max_nb_sessions);
 
-	return cryptodev_kasumi_create(name, &init_params);
+	return cryptodev_kasumi_create(&init_params);
 }
 
 static int
diff --git a/drivers/crypto/null/null_crypto_pmd.c b/drivers/crypto/null/null_crypto_pmd.c
index c69606b..ce60470 100644
--- a/drivers/crypto/null/null_crypto_pmd.c
+++ b/drivers/crypto/null/null_crypto_pmd.c
@@ -38,27 +38,6 @@
 
 #include "null_crypto_pmd_private.h"
 
-/**
- * Global static parameter used to create a unique name for each crypto device.
- */
-static unsigned unique_name_id;
-
-static inline int
-create_unique_device_name(char *name, size_t size)
-{
-	int ret;
-
-	if (name == NULL)
-		return -EINVAL;
-
-	ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_NULL_PMD),
-			unique_name_id++);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
-
 /** verify and set session parameters */
 int
 null_crypto_set_session_parameters(
@@ -186,21 +165,24 @@ static int cryptodev_null_remove(const char *name);
 
 /** Create crypto device */
 static int
-cryptodev_null_create(const char *name,
-		struct rte_crypto_vdev_init_params *init_params)
+cryptodev_null_create(struct rte_crypto_vdev_init_params *init_params)
 {
 	struct rte_cryptodev *dev;
-	char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
 	struct null_crypto_private *internals;
 
-	/* create a unique device name */
-	if (create_unique_device_name(crypto_dev_name,
-			RTE_CRYPTODEV_NAME_MAX_LEN) != 0) {
-		NULL_CRYPTO_LOG_ERR("failed to create unique cryptodev name");
-		return -EINVAL;
+	if (init_params->name[0] == '\0') {
+		int ret = rte_cryptodev_pmd_create_dev_name(
+				init_params->name,
+				RTE_STR(CRYPTODEV_NAME_NULL_PMD));
+
+		if (ret < 0) {
+			NULL_CRYPTO_LOG_ERR("failed to create unique "
+					"name");
+			return ret;
+		}
 	}
 
-	dev = rte_cryptodev_pmd_virtual_dev_init(crypto_dev_name,
+	dev = rte_cryptodev_pmd_virtual_dev_init(init_params->name,
 			sizeof(struct null_crypto_private),
 			init_params->socket_id);
 	if (dev == NULL) {
@@ -226,8 +208,9 @@ cryptodev_null_create(const char *name,
 	return 0;
 
 init_error:
-	NULL_CRYPTO_LOG_ERR("driver %s: cryptodev_null_create failed", name);
-	cryptodev_null_remove(crypto_dev_name);
+	NULL_CRYPTO_LOG_ERR("driver %s: cryptodev_null_create failed",
+			init_params->name);
+	cryptodev_null_remove(init_params->name);
 
 	return -EFAULT;
 }
@@ -240,19 +223,23 @@ cryptodev_null_probe(const char *name,
 	struct rte_crypto_vdev_init_params init_params = {
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS,
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS,
-		rte_socket_id()
+		rte_socket_id(),
+		{0}
 	};
 
 	rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
 
 	RTE_LOG(INFO, PMD, "Initialising %s on NUMA node %d\n", name,
 			init_params.socket_id);
+	if (init_params.name[0] != '\0')
+		RTE_LOG(INFO, PMD, "  User defined name = %s\n",
+			init_params.name);
 	RTE_LOG(INFO, PMD, "  Max number of queue pairs = %d\n",
 			init_params.max_nb_queue_pairs);
 	RTE_LOG(INFO, PMD, "  Max number of sessions = %d\n",
 			init_params.max_nb_sessions);
 
-	return cryptodev_null_create(name, &init_params);
+	return cryptodev_null_create(&init_params);
 }
 
 /** Uninitialise null crypto device */
diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 5f8fa33..c556a9e 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -47,28 +47,6 @@ static int cryptodev_openssl_remove(const char *name);
 /*----------------------------------------------------------------------------*/
 
 /**
- * Global static parameter used to create a unique name for each
- * OPENSSL crypto device.
- */
-static unsigned int unique_name_id;
-
-static inline int
-create_unique_device_name(char *name, size_t size)
-{
-	int ret;
-
-	if (name == NULL)
-		return -EINVAL;
-
-	ret = snprintf(name, size, "%s_%u",
-			RTE_STR(CRYPTODEV_NAME_OPENSSL_PMD),
-			unique_name_id++);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
-/**
  * Increment counter by 1
  * Counter is 64 bit array, big-endian
  */
@@ -964,21 +942,23 @@ openssl_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 
 /** Create OPENSSL crypto device */
 static int
-cryptodev_openssl_create(const char *name,
-		struct rte_crypto_vdev_init_params *init_params)
+cryptodev_openssl_create(struct rte_crypto_vdev_init_params *init_params)
 {
 	struct rte_cryptodev *dev;
-	char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
 	struct openssl_private *internals;
 
-	/* create a unique device name */
-	if (create_unique_device_name(crypto_dev_name,
-			RTE_CRYPTODEV_NAME_MAX_LEN) != 0) {
-		OPENSSL_LOG_ERR("failed to create unique cryptodev name");
-		return -EINVAL;
+	if (init_params->name[0] == '\0') {
+		int ret = rte_cryptodev_pmd_create_dev_name(
+				init_params->name,
+				RTE_STR(CRYPTODEV_NAME_OPENSSL_PMD));
+
+		if (ret < 0) {
+			OPENSSL_LOG_ERR("failed to create unique name");
+			return ret;
+		}
 	}
 
-	dev = rte_cryptodev_pmd_virtual_dev_init(crypto_dev_name,
+	dev = rte_cryptodev_pmd_virtual_dev_init(init_params->name,
 			sizeof(struct openssl_private),
 			init_params->socket_id);
 	if (dev == NULL) {
@@ -1006,9 +986,10 @@ cryptodev_openssl_create(const char *name,
 	return 0;
 
 init_error:
-	OPENSSL_LOG_ERR("driver %s: cryptodev_openssl_create failed", name);
+	OPENSSL_LOG_ERR("driver %s: cryptodev_openssl_create failed",
+			init_params->name);
 
-	cryptodev_openssl_remove(crypto_dev_name);
+	cryptodev_openssl_remove(init_params->name);
 	return -EFAULT;
 }
 
@@ -1020,19 +1001,23 @@ cryptodev_openssl_probe(const char *name,
 	struct rte_crypto_vdev_init_params init_params = {
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS,
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS,
-		rte_socket_id()
+		rte_socket_id(),
+		{0}
 	};
 
 	rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
 
 	RTE_LOG(INFO, PMD, "Initialising %s on NUMA node %d\n", name,
 			init_params.socket_id);
+	if (init_params.name[0] != '\0')
+		RTE_LOG(INFO, PMD, "  User defined name = %s\n",
+			init_params.name);
 	RTE_LOG(INFO, PMD, "  Max number of queue pairs = %d\n",
 			init_params.max_nb_queue_pairs);
 	RTE_LOG(INFO, PMD, "  Max number of sessions = %d\n",
 			init_params.max_nb_sessions);
 
-	return cryptodev_openssl_create(name, &init_params);
+	return cryptodev_openssl_create(&init_params);
 }
 
 /** Uninitialise OPENSSL crypto device */
diff --git a/drivers/crypto/snow3g/rte_snow3g_pmd.c b/drivers/crypto/snow3g/rte_snow3g_pmd.c
index 3b4292a..4a70f06 100644
--- a/drivers/crypto/snow3g/rte_snow3g_pmd.c
+++ b/drivers/crypto/snow3g/rte_snow3g_pmd.c
@@ -46,27 +46,6 @@
 #define SNOW3G_MAX_BURST 8
 #define BYTE_LEN 8
 
-/**
- * Global static parameter used to create a unique name for each SNOW 3G
- * crypto device.
- */
-static unsigned unique_name_id;
-
-static inline int
-create_unique_device_name(char *name, size_t size)
-{
-	int ret;
-
-	if (name == NULL)
-		return -EINVAL;
-
-	ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD),
-			unique_name_id++);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
 /** Get xform chain order. */
 static enum snow3g_operation
 snow3g_get_mode(const struct rte_crypto_sym_xform *xform)
@@ -548,14 +527,23 @@ snow3g_pmd_dequeue_burst(void *queue_pair,
 static int cryptodev_snow3g_remove(const char *name);
 
 static int
-cryptodev_snow3g_create(const char *name,
-		struct rte_crypto_vdev_init_params *init_params)
+cryptodev_snow3g_create(struct rte_crypto_vdev_init_params *init_params)
 {
 	struct rte_cryptodev *dev;
-	char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
 	struct snow3g_private *internals;
 	uint64_t cpu_flags = 0;
 
+	if (init_params->name[0] == '\0') {
+		int ret = rte_cryptodev_pmd_create_dev_name(
+				init_params->name,
+				RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD));
+
+		if (ret < 0) {
+			SNOW3G_LOG_ERR("failed to create unique name");
+			return ret;
+		}
+	}
+
 	/* Check CPU for supported vector instruction set */
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
 		cpu_flags |= RTE_CRYPTODEV_FF_CPU_SSE;
@@ -564,15 +552,7 @@ cryptodev_snow3g_create(const char *name,
 		return -EFAULT;
 	}
 
-
-	/* Create a unique device name. */
-	if (create_unique_device_name(crypto_dev_name,
-			RTE_CRYPTODEV_NAME_MAX_LEN) != 0) {
-		SNOW3G_LOG_ERR("failed to create unique cryptodev name");
-		return -EINVAL;
-	}
-
-	dev = rte_cryptodev_pmd_virtual_dev_init(crypto_dev_name,
+	dev = rte_cryptodev_pmd_virtual_dev_init(init_params->name,
 			sizeof(struct snow3g_private), init_params->socket_id);
 	if (dev == NULL) {
 		SNOW3G_LOG_ERR("failed to create cryptodev vdev");
@@ -597,9 +577,10 @@ cryptodev_snow3g_create(const char *name,
 
 	return 0;
 init_error:
-	SNOW3G_LOG_ERR("driver %s: cryptodev_snow3g_create failed", name);
+	SNOW3G_LOG_ERR("driver %s: cryptodev_snow3g_create failed",
+			init_params->name);
 
-	cryptodev_snow3g_remove(crypto_dev_name);
+	cryptodev_snow3g_remove(init_params->name);
 	return -EFAULT;
 }
 
@@ -610,19 +591,23 @@ cryptodev_snow3g_probe(const char *name,
 	struct rte_crypto_vdev_init_params init_params = {
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS,
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS,
-		rte_socket_id()
+		rte_socket_id(),
+		{0}
 	};
 
 	rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
 
 	RTE_LOG(INFO, PMD, "Initialising %s on NUMA node %d\n", name,
 			init_params.socket_id);
+	if (init_params.name[0] != '\0')
+		RTE_LOG(INFO, PMD, "  User defined name = %s\n",
+			init_params.name);
 	RTE_LOG(INFO, PMD, "  Max number of queue pairs = %d\n",
 			init_params.max_nb_queue_pairs);
 	RTE_LOG(INFO, PMD, "  Max number of sessions = %d\n",
 			init_params.max_nb_sessions);
 
-	return cryptodev_snow3g_create(name, &init_params);
+	return cryptodev_snow3g_create(&init_params);
 }
 
 static int
diff --git a/drivers/crypto/zuc/rte_zuc_pmd.c b/drivers/crypto/zuc/rte_zuc_pmd.c
index 3849119..062cfff 100644
--- a/drivers/crypto/zuc/rte_zuc_pmd.c
+++ b/drivers/crypto/zuc/rte_zuc_pmd.c
@@ -45,27 +45,6 @@
 #define ZUC_MAX_BURST 8
 #define BYTE_LEN 8
 
-/**
- * Global static parameter used to create a unique name for each ZUC
- * crypto device.
- */
-static unsigned unique_name_id;
-
-static inline int
-create_unique_device_name(char *name, size_t size)
-{
-	int ret;
-
-	if (name == NULL)
-		return -EINVAL;
-
-	ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_ZUC_PMD),
-			unique_name_id++);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
 /** Get xform chain order. */
 static enum zuc_operation
 zuc_get_mode(const struct rte_crypto_sym_xform *xform)
@@ -448,14 +427,23 @@ zuc_pmd_dequeue_burst(void *queue_pair,
 static int cryptodev_zuc_remove(const char *name);
 
 static int
-cryptodev_zuc_create(const char *name,
-		struct rte_crypto_vdev_init_params *init_params)
+cryptodev_zuc_create(struct rte_crypto_vdev_init_params *init_params)
 {
 	struct rte_cryptodev *dev;
-	char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
 	struct zuc_private *internals;
 	uint64_t cpu_flags = 0;
 
+	if (init_params->name[0] == '\0') {
+		int ret = rte_cryptodev_pmd_create_dev_name(
+				init_params->name,
+				RTE_STR(CRYPTODEV_NAME_ZUC_PMD));
+
+		if (ret < 0) {
+			ZUC_LOG_ERR("failed to create unique name");
+			return ret;
+		}
+	}
+
 	/* Check CPU for supported vector instruction set */
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
 		cpu_flags |= RTE_CRYPTODEV_FF_CPU_SSE;
@@ -464,15 +452,7 @@ cryptodev_zuc_create(const char *name,
 		return -EFAULT;
 	}
 
-
-	/* Create a unique device name. */
-	if (create_unique_device_name(crypto_dev_name,
-			RTE_CRYPTODEV_NAME_MAX_LEN) != 0) {
-		ZUC_LOG_ERR("failed to create unique cryptodev name");
-		return -EINVAL;
-	}
-
-	dev = rte_cryptodev_pmd_virtual_dev_init(crypto_dev_name,
+	dev = rte_cryptodev_pmd_virtual_dev_init(init_params->name,
 			sizeof(struct zuc_private), init_params->socket_id);
 	if (dev == NULL) {
 		ZUC_LOG_ERR("failed to create cryptodev vdev");
@@ -497,9 +477,10 @@ cryptodev_zuc_create(const char *name,
 
 	return 0;
 init_error:
-	ZUC_LOG_ERR("driver %s: cryptodev_zuc_create failed", name);
+	ZUC_LOG_ERR("driver %s: cryptodev_zuc_create failed",
+			init_params->name);
 
-	cryptodev_zuc_remove(crypto_dev_name);
+	cryptodev_zuc_remove(init_params->name);
 	return -EFAULT;
 }
 
@@ -510,19 +491,23 @@ cryptodev_zuc_probe(const char *name,
 	struct rte_crypto_vdev_init_params init_params = {
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS,
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS,
-		rte_socket_id()
+		rte_socket_id(),
+		{0}
 	};
 
 	rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
 
 	RTE_LOG(INFO, PMD, "Initialising %s on NUMA node %d\n", name,
 			init_params.socket_id);
+	if (init_params.name[0] != '\0')
+		RTE_LOG(INFO, PMD, "  User defined name = %s\n",
+			init_params.name);
 	RTE_LOG(INFO, PMD, "  Max number of queue pairs = %d\n",
 			init_params.max_nb_queue_pairs);
 	RTE_LOG(INFO, PMD, "  Max number of sessions = %d\n",
 			init_params.max_nb_sessions);
 
-	return cryptodev_zuc_create(name, &init_params);
+	return cryptodev_zuc_create(&init_params);
 }
 
 static int
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 1/1] i40e: improve message grepability
From: Michał Mirosław @ 2017-01-11 17:13 UTC (permalink / raw)
  To: dev
In-Reply-To: <1c4d6b329cb00fbed3d443800ed7eeb5b4bf57c7.1484127857.git.mirq-linux@rere.qmqm.pl>

From: Michał Mirosław <michal.miroslaw@atendesoftware.pl>

Join message lines for easier grepping.
PRIxXX are left glued to strings as they are in other parts of the file.

Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
---
v2: move long strings on a line by themselves
v3: indent modified lines with TAB only

Change-Id: Idbaa3392d8112f96a9f15818f6ee1b5174a50c87
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/i40e/i40e_ethdev.c | 312 +++++++++++++++++++++--------------------
 1 file changed, 161 insertions(+), 151 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 24a3ac806..95be29df0 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -719,8 +719,8 @@ i40e_add_tx_flow_control_drop_filter(struct i40e_pf *pf)
 				pf->main_vsi_seid, 0,
 				TRUE, NULL, NULL);
 	if (ret)
-		PMD_INIT_LOG(ERR, "Failed to add filter to drop flow control "
-				  " frames from VSIs.");
+		PMD_INIT_LOG(ERR,
+			"Failed to add filter to drop flow control frames from VSIs.");
 }
 
 static int
@@ -1050,8 +1050,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	hw->back = I40E_PF_TO_ADAPTER(pf);
 	hw->hw_addr = (uint8_t *)(pci_dev->mem_resource[0].addr);
 	if (!hw->hw_addr) {
-		PMD_INIT_LOG(ERR, "Hardware is not available, "
-			     "as address is NULL");
+		PMD_INIT_LOG(ERR,
+			"Hardware is not available, as address is NULL");
 		return -ENODEV;
 	}
 
@@ -1187,8 +1187,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	/* Set the global registers with default ether type value */
 	ret = i40e_vlan_tpid_set(dev, ETH_VLAN_TYPE_OUTER, ETHER_TYPE_VLAN);
 	if (ret != I40E_SUCCESS) {
-		PMD_INIT_LOG(ERR, "Failed to set the default outer "
-			     "VLAN ether type");
+		PMD_INIT_LOG(ERR,
+			"Failed to set the default outer VLAN ether type");
 		goto err_setup_pf_switch;
 	}
 
@@ -1224,8 +1224,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	/* Should be after VSI initialized */
 	dev->data->mac_addrs = rte_zmalloc("i40e", len, 0);
 	if (!dev->data->mac_addrs) {
-		PMD_INIT_LOG(ERR, "Failed to allocated memory "
-					"for storing mac address");
+		PMD_INIT_LOG(ERR,
+			"Failed to allocated memory for storing mac address");
 		goto err_mac_alloc;
 	}
 	ether_addr_copy((struct ether_addr *)hw->mac.perm_addr,
@@ -1889,8 +1889,9 @@ i40e_dev_start(struct rte_eth_dev *dev)
 				    dev->data->nb_rx_queues * sizeof(int),
 				    0);
 		if (!intr_handle->intr_vec) {
-			PMD_INIT_LOG(ERR, "Failed to allocate %d rx_queues"
-				     " intr_vec\n", dev->data->nb_rx_queues);
+			PMD_INIT_LOG(ERR,
+				"Failed to allocate %d rx_queues intr_vec\n",
+				dev->data->nb_rx_queues);
 			return -ENOMEM;
 		}
 	}
@@ -1963,8 +1964,8 @@ i40e_dev_start(struct rte_eth_dev *dev)
 		i40e_pf_enable_irq0(hw);
 
 		if (dev->data->dev_conf.intr_conf.lsc != 0)
-			PMD_INIT_LOG(INFO, "lsc won't enable because of"
-				     " no intr multiplex\n");
+			PMD_INIT_LOG(INFO,
+				"lsc won't enable because of no intr multiplex\n");
 	} else if (dev->data->dev_conf.intr_conf.lsc != 0) {
 		ret = i40e_aq_set_phy_int_mask(hw,
 					       ~(I40E_AQ_EVENT_LINK_UPDOWN |
@@ -2916,13 +2917,15 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
 	ret = i40e_aq_debug_read_register(hw, I40E_GL_SWT_L2TAGCTRL(reg_id),
 					  &reg_r, NULL);
 	if (ret != I40E_SUCCESS) {
-		PMD_DRV_LOG(ERR, "Fail to debug read from "
-			    "I40E_GL_SWT_L2TAGCTRL[%d]", reg_id);
+		PMD_DRV_LOG(ERR,
+			   "Fail to debug read from I40E_GL_SWT_L2TAGCTRL[%d]",
+			   reg_id);
 		ret = -EIO;
 		return ret;
 	}
-	PMD_DRV_LOG(DEBUG, "Debug read from I40E_GL_SWT_L2TAGCTRL[%d]: "
-		    "0x%08"PRIx64"", reg_id, reg_r);
+	PMD_DRV_LOG(DEBUG,
+		"Debug read from I40E_GL_SWT_L2TAGCTRL[%d]: 0x%08"PRIx64,
+		reg_id, reg_r);
 
 	reg_w = reg_r & (~(I40E_GL_SWT_L2TAGCTRL_ETHERTYPE_MASK));
 	reg_w |= ((uint64_t)tpid << I40E_GL_SWT_L2TAGCTRL_ETHERTYPE_SHIFT);
@@ -2936,12 +2939,14 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
 					   reg_w, NULL);
 	if (ret != I40E_SUCCESS) {
 		ret = -EIO;
-		PMD_DRV_LOG(ERR, "Fail to debug write to "
-			    "I40E_GL_SWT_L2TAGCTRL[%d]", reg_id);
+		PMD_DRV_LOG(ERR,
+			"Fail to debug write to I40E_GL_SWT_L2TAGCTRL[%d]",
+			reg_id);
 		return ret;
 	}
-	PMD_DRV_LOG(DEBUG, "Debug write 0x%08"PRIx64" to "
-		    "I40E_GL_SWT_L2TAGCTRL[%d]", reg_w, reg_id);
+	PMD_DRV_LOG(DEBUG,
+		"Debug write 0x%08"PRIx64" to I40E_GL_SWT_L2TAGCTRL[%d]",
+		reg_w, reg_id);
 
 	return ret;
 }
@@ -3085,8 +3090,9 @@ i40e_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	max_high_water = I40E_RXPBSIZE >> I40E_KILOSHIFT;
 	if ((fc_conf->high_water > max_high_water) ||
 			(fc_conf->high_water < fc_conf->low_water)) {
-		PMD_INIT_LOG(ERR, "Invalid high/low water setup value in KB, "
-			"High_water must <= %d.", max_high_water);
+		PMD_INIT_LOG(ERR,
+			"Invalid high/low water setup value in KB, High_water must be <= %d.",
+			max_high_water);
 		return -EINVAL;
 	}
 
@@ -3258,8 +3264,8 @@ i40e_macaddr_remove(struct rte_eth_dev *dev, uint32_t index)
 				/* No VMDQ pool enabled or configured */
 				if (!(pf->flags & I40E_FLAG_VMDQ) ||
 					(i > pf->nb_cfg_vmdq_vsi)) {
-					PMD_DRV_LOG(ERR, "No VMDQ pool enabled"
-							"/configured");
+					PMD_DRV_LOG(ERR,
+						"No VMDQ pool enabled/configured");
 					return;
 				}
 				vsi = pf->vmdq[i - 1].vsi;
@@ -3460,9 +3466,9 @@ i40e_dev_rss_reta_update(struct rte_eth_dev *dev,
 
 	if (reta_size != lut_size ||
 		reta_size > ETH_RSS_RETA_SIZE_512) {
-		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
-			"(%d) doesn't match the number hardware can supported "
-					"(%d)\n", reta_size, lut_size);
+		PMD_DRV_LOG(ERR,
+			"The size of hash lookup table configured (%d) doesn't match the number hardware can supported (%d)\n",
+			reta_size, lut_size);
 		return -EINVAL;
 	}
 
@@ -3501,9 +3507,9 @@ i40e_dev_rss_reta_query(struct rte_eth_dev *dev,
 
 	if (reta_size != lut_size ||
 		reta_size > ETH_RSS_RETA_SIZE_512) {
-		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
-			"(%d) doesn't match the number hardware can supported "
-					"(%d)\n", reta_size, lut_size);
+		PMD_DRV_LOG(ERR,
+			"The size of hash lookup table configured (%d) doesn't match the number hardware can supported (%d)\n",
+			reta_size, lut_size);
 		return -EINVAL;
 	}
 
@@ -3558,8 +3564,9 @@ i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
 	mem->va = mz->addr;
 	mem->pa = rte_mem_phy2mch(mz->memseg_id, mz->phys_addr);
 	mem->zone = (const void *)mz;
-	PMD_DRV_LOG(DEBUG, "memzone %s allocated with physical address: "
-		    "%"PRIu64, mz->name, mem->pa);
+	PMD_DRV_LOG(DEBUG,
+		"memzone %s allocated with physical address: %"PRIu64,
+		mz->name, mem->pa);
 
 	return I40E_SUCCESS;
 }
@@ -3576,9 +3583,9 @@ i40e_free_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
 	if (!mem)
 		return I40E_ERR_PARAM;
 
-	PMD_DRV_LOG(DEBUG, "memzone %s to be freed with physical address: "
-		    "%"PRIu64, ((const struct rte_memzone *)mem->zone)->name,
-		    mem->pa);
+	PMD_DRV_LOG(DEBUG,
+		"memzone %s to be freed with physical address: %"PRIu64,
+		((const struct rte_memzone *)mem->zone)->name, mem->pa);
 	rte_memzone_free((const struct rte_memzone *)mem->zone);
 	mem->zone = NULL;
 	mem->va = NULL;
@@ -3737,9 +3744,9 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		pf->flags |= I40E_FLAG_SRIOV;
 		pf->vf_nb_qps = RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF;
 		pf->vf_num = pci_dev->max_vfs;
-		PMD_DRV_LOG(DEBUG, "%u VF VSIs, %u queues per VF VSI, "
-			    "in total %u queues", pf->vf_num, pf->vf_nb_qps,
-			    pf->vf_nb_qps * pf->vf_num);
+		PMD_DRV_LOG(DEBUG,
+			"%u VF VSIs, %u queues per VF VSI, in total %u queues",
+			pf->vf_num, pf->vf_nb_qps, pf->vf_nb_qps * pf->vf_num);
 	} else {
 		pf->vf_nb_qps = 0;
 		pf->vf_num = 0;
@@ -3767,14 +3774,13 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 			if (pf->max_nb_vmdq_vsi) {
 				pf->flags |= I40E_FLAG_VMDQ;
 				pf->vmdq_nb_qps = pf->vmdq_nb_qp_max;
-				PMD_DRV_LOG(DEBUG, "%u VMDQ VSIs, %u queues "
-					    "per VMDQ VSI, in total %u queues",
-					    pf->max_nb_vmdq_vsi,
-					    pf->vmdq_nb_qps, pf->vmdq_nb_qps *
-					    pf->max_nb_vmdq_vsi);
+				PMD_DRV_LOG(DEBUG,
+					"%u VMDQ VSIs, %u queues per VMDQ VSI, in total %u queues",
+					pf->max_nb_vmdq_vsi, pf->vmdq_nb_qps,
+					pf->vmdq_nb_qps * pf->max_nb_vmdq_vsi);
 			} else {
-				PMD_DRV_LOG(INFO, "No enough queues left for "
-					    "VMDq");
+				PMD_DRV_LOG(INFO,
+					"No enough queues left for VMDq");
 			}
 		} else {
 			PMD_DRV_LOG(INFO, "No queue or VSI left for VMDq");
@@ -3787,15 +3793,15 @@ i40e_pf_parameter_init(struct rte_eth_dev *dev)
 		pf->flags |= I40E_FLAG_DCB;
 
 	if (qp_count > hw->func_caps.num_tx_qp) {
-		PMD_DRV_LOG(ERR, "Failed to allocate %u queues, which exceeds "
-			    "the hardware maximum %u", qp_count,
-			    hw->func_caps.num_tx_qp);
+		PMD_DRV_LOG(ERR,
+			"Failed to allocate %u queues, which exceeds the hardware maximum %u",
+			qp_count, hw->func_caps.num_tx_qp);
 		return -EINVAL;
 	}
 	if (vsi_count > hw->func_caps.num_vsis) {
-		PMD_DRV_LOG(ERR, "Failed to allocate %u VSIs, which exceeds "
-			    "the hardware maximum %u", vsi_count,
-			    hw->func_caps.num_vsis);
+		PMD_DRV_LOG(ERR,
+			"Failed to allocate %u VSIs, which exceeds the hardware maximum %u",
+			vsi_count, hw->func_caps.num_vsis);
 		return -EINVAL;
 	}
 
@@ -4041,8 +4047,8 @@ i40e_res_pool_alloc(struct i40e_res_pool_info *pool,
 		 */
 		entry = rte_zmalloc("res_pool", sizeof(*entry), 0);
 		if (entry == NULL) {
-			PMD_DRV_LOG(ERR, "Failed to allocate memory for "
-				    "resource pool");
+			PMD_DRV_LOG(ERR,
+				"Failed to allocate memory for resource pool");
 			return -ENOMEM;
 		}
 		entry->base = valid_entry->base;
@@ -4082,9 +4088,9 @@ validate_tcmap_parameter(struct i40e_vsi *vsi, uint8_t enabled_tcmap)
 	}
 
 	if (!bitmap_is_subset(hw->func_caps.enabled_tcmap, enabled_tcmap)) {
-		PMD_DRV_LOG(ERR, "Enabled TC map 0x%x not applicable to "
-			    "HW support 0x%x", hw->func_caps.enabled_tcmap,
-			    enabled_tcmap);
+		PMD_DRV_LOG(ERR,
+			"Enabled TC map 0x%x not applicable to HW support 0x%x",
+			hw->func_caps.enabled_tcmap, enabled_tcmap);
 		return I40E_NOT_SUPPORTED;
 	}
 	return I40E_SUCCESS;
@@ -4426,8 +4432,8 @@ i40e_update_default_filter_setting(struct i40e_vsi *vsi)
 		struct i40e_mac_filter *f;
 		struct ether_addr *mac;
 
-		PMD_DRV_LOG(WARNING, "Cannot remove the default "
-			    "macvlan filter");
+		PMD_DRV_LOG(WARNING,
+			"Cannot remove the default macvlan filter");
 		/* It needs to add the permanent mac into mac list */
 		f = rte_zmalloc("macv_filter", sizeof(*f), 0);
 		if (f == NULL) {
@@ -4477,8 +4483,9 @@ i40e_vsi_get_bw_config(struct i40e_vsi *vsi)
 	ret = i40e_aq_query_vsi_ets_sla_config(hw, vsi->seid,
 					&ets_sla_config, NULL);
 	if (ret != I40E_SUCCESS) {
-		PMD_DRV_LOG(ERR, "VSI failed to get TC bandwdith "
-			    "configuration %u", hw->aq.asq_last_status);
+		PMD_DRV_LOG(ERR,
+			"VSI failed to get TC bandwdith configuration %u",
+			hw->aq.asq_last_status);
 		return ret;
 	}
 
@@ -4566,14 +4573,14 @@ i40e_vsi_setup(struct i40e_pf *pf,
 
 	if (type != I40E_VSI_MAIN && type != I40E_VSI_SRIOV &&
 	    uplink_vsi == NULL) {
-		PMD_DRV_LOG(ERR, "VSI setup failed, "
-			    "VSI link shouldn't be NULL");
+		PMD_DRV_LOG(ERR,
+			"VSI setup failed, VSI link shouldn't be NULL");
 		return NULL;
 	}
 
 	if (type == I40E_VSI_MAIN && uplink_vsi != NULL) {
-		PMD_DRV_LOG(ERR, "VSI setup failed, MAIN VSI "
-			    "uplink VSI should be NULL");
+		PMD_DRV_LOG(ERR,
+			"VSI setup failed, MAIN VSI uplink VSI should be NULL");
 		return NULL;
 	}
 
@@ -4724,8 +4731,8 @@ i40e_vsi_setup(struct i40e_pf *pf,
 		ret = i40e_vsi_config_tc_queue_mapping(vsi, &ctxt.info,
 						I40E_DEFAULT_TCMAP);
 		if (ret != I40E_SUCCESS) {
-			PMD_DRV_LOG(ERR, "Failed to configure "
-				    "TC queue mapping");
+			PMD_DRV_LOG(ERR,
+				"Failed to configure TC queue mapping");
 			goto fail_msix_alloc;
 		}
 		ctxt.seid = vsi->seid;
@@ -4795,8 +4802,8 @@ i40e_vsi_setup(struct i40e_pf *pf,
 		ret = i40e_vsi_config_tc_queue_mapping(vsi, &ctxt.info,
 						I40E_DEFAULT_TCMAP);
 		if (ret != I40E_SUCCESS) {
-			PMD_DRV_LOG(ERR, "Failed to configure "
-				    "TC queue mapping");
+			PMD_DRV_LOG(ERR,
+				"Failed to configure TC queue mapping");
 			goto fail_msix_alloc;
 		}
 		ctxt.info.up_enable_bits = I40E_DEFAULT_TCMAP;
@@ -4838,8 +4845,8 @@ i40e_vsi_setup(struct i40e_pf *pf,
 		ret = i40e_vsi_config_tc_queue_mapping(vsi, &ctxt.info,
 						I40E_DEFAULT_TCMAP);
 		if (ret != I40E_SUCCESS) {
-			PMD_DRV_LOG(ERR, "Failed to configure "
-					"TC queue mapping");
+			PMD_DRV_LOG(ERR,
+				"Failed to configure TC queue mapping");
 			goto fail_msix_alloc;
 		}
 		ctxt.info.up_enable_bits = I40E_DEFAULT_TCMAP;
@@ -4856,8 +4863,8 @@ i40e_vsi_setup(struct i40e_pf *pf,
 		ret = i40e_vsi_config_tc_queue_mapping(vsi, &ctxt.info,
 						I40E_DEFAULT_TCMAP);
 		if (ret != I40E_SUCCESS) {
-			PMD_DRV_LOG(ERR, "Failed to configure "
-					"TC queue mapping.");
+			PMD_DRV_LOG(ERR,
+				"Failed to configure TC queue mapping.");
 			goto fail_msix_alloc;
 		}
 		ctxt.info.up_enable_bits = I40E_DEFAULT_TCMAP;
@@ -5120,8 +5127,9 @@ i40e_pf_setup(struct i40e_pf *pf)
 		/* make queue allocated first, let FDIR use queue pair 0*/
 		ret = i40e_res_pool_alloc(&pf->qp_pool, I40E_DEFAULT_QP_NUM_FDIR);
 		if (ret != I40E_FDIR_QUEUE_ID) {
-			PMD_DRV_LOG(ERR, "queue allocation fails for FDIR :"
-				    " ret =%d", ret);
+			PMD_DRV_LOG(ERR,
+				"queue allocation fails for FDIR: ret =%d",
+				ret);
 			pf->flags &= ~I40E_FLAG_FDIR;
 		}
 	}
@@ -5144,8 +5152,8 @@ i40e_pf_setup(struct i40e_pf *pf)
 						hw->func_caps.rss_table_size);
 		return I40E_ERR_PARAM;
 	}
-	PMD_DRV_LOG(INFO, "Hardware capability of hash lookup table "
-			"size: %u\n", hw->func_caps.rss_table_size);
+	PMD_DRV_LOG(INFO, "Hardware capability of hash lookup table size: %u\n",
+		hw->func_caps.rss_table_size);
 	pf->hash_lut_size = hw->func_caps.rss_table_size;
 
 	/* Enable ethtype and macvlan filters */
@@ -5395,8 +5403,8 @@ i40e_dev_rx_init(struct i40e_pf *pf)
 
 		ret = i40e_rx_queue_init(rxq);
 		if (ret != I40E_SUCCESS) {
-			PMD_DRV_LOG(ERR, "Failed to do RX queue "
-				    "initialization");
+			PMD_DRV_LOG(ERR,
+				"Failed to do RX queue initialization");
 			break;
 		}
 	}
@@ -5680,8 +5688,9 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
 		ret = i40e_clean_arq_element(hw, &info, &pending);
 
 		if (ret != I40E_SUCCESS) {
-			PMD_DRV_LOG(INFO, "Failed to read msg from AdminQ, "
-				    "aq_err: %u", hw->aq.asq_last_status);
+			PMD_DRV_LOG(INFO,
+				"Failed to read msg from AdminQ, aq_err: %u",
+				hw->aq.asq_last_status);
 			break;
 		}
 		opcode = rte_le_to_cpu_16(info.desc.opcode);
@@ -5998,8 +6007,8 @@ i40e_find_all_vlan_for_mac(struct i40e_vsi *vsi,
 			for (k = 0; k < I40E_UINT32_BIT_SIZE; k++) {
 				if (vsi->vfta[j] & (1 << k)) {
 					if (i > num - 1) {
-						PMD_DRV_LOG(ERR, "vlan number "
-							    "not match");
+						PMD_DRV_LOG(ERR,
+							"vlan number doesn't match");
 						return I40E_ERR_PARAM;
 					}
 					(void)rte_memcpy(&mv_f[i].macaddr,
@@ -6472,8 +6481,7 @@ i40e_set_rss_key(struct i40e_vsi *vsi, uint8_t *key, uint8_t key_len)
 
 		ret = i40e_aq_set_rss_key(hw, vsi->vsi_id, key_dw);
 		if (ret)
-			PMD_INIT_LOG(ERR, "Failed to configure RSS key "
-				     "via AQ");
+			PMD_INIT_LOG(ERR, "Failed to configure RSS key via AQ");
 	} else {
 		uint32_t *hash_key = (uint32_t *)key;
 		uint16_t i;
@@ -6846,8 +6854,9 @@ i40e_add_vxlan_port(struct i40e_pf *pf, uint16_t port)
 	/* Now check if there is space to add the new port */
 	idx = i40e_get_vxlan_port_idx(pf, 0);
 	if (idx < 0) {
-		PMD_DRV_LOG(ERR, "Maximum number of UDP ports reached,"
-			"not adding port %d", port);
+		PMD_DRV_LOG(ERR,
+			"Maximum number of UDP ports reached, not adding port %d",
+			port);
 		return -ENOSPC;
 	}
 
@@ -7218,15 +7227,15 @@ i40e_set_symmetric_hash_enable_per_port(struct i40e_hw *hw, uint8_t enable)
 
 	if (enable > 0) {
 		if (reg & I40E_PRTQF_CTL_0_HSYM_ENA_MASK) {
-			PMD_DRV_LOG(INFO, "Symmetric hash has already "
-							"been enabled");
+			PMD_DRV_LOG(INFO,
+				"Symmetric hash has already been enabled");
 			return;
 		}
 		reg |= I40E_PRTQF_CTL_0_HSYM_ENA_MASK;
 	} else {
 		if (!(reg & I40E_PRTQF_CTL_0_HSYM_ENA_MASK)) {
-			PMD_DRV_LOG(INFO, "Symmetric hash has already "
-							"been disabled");
+			PMD_DRV_LOG(INFO,
+				"Symmetric hash has already been disabled");
 			return;
 		}
 		reg &= ~I40E_PRTQF_CTL_0_HSYM_ENA_MASK;
@@ -7350,16 +7359,16 @@ i40e_set_hash_filter_global_config(struct i40e_hw *hw,
 	if (g_cfg->hash_func == RTE_ETH_HASH_FUNCTION_TOEPLITZ) {
 		/* Toeplitz */
 		if (reg & I40E_GLQF_CTL_HTOEP_MASK) {
-			PMD_DRV_LOG(DEBUG, "Hash function already set to "
-								"Toeplitz");
+			PMD_DRV_LOG(DEBUG,
+				"Hash function already set to Toeplitz");
 			goto out;
 		}
 		reg |= I40E_GLQF_CTL_HTOEP_MASK;
 	} else if (g_cfg->hash_func == RTE_ETH_HASH_FUNCTION_SIMPLE_XOR) {
 		/* Simple XOR */
 		if (!(reg & I40E_GLQF_CTL_HTOEP_MASK)) {
-			PMD_DRV_LOG(DEBUG, "Hash function already set to "
-							"Simple XOR");
+			PMD_DRV_LOG(DEBUG,
+				"Hash function already set to Simple XOR");
 			goto out;
 		}
 		reg &= ~I40E_GLQF_CTL_HTOEP_MASK;
@@ -8362,13 +8371,14 @@ i40e_ethertype_filter_set(struct i40e_pf *pf,
 	}
 	if (filter->ether_type == ETHER_TYPE_IPv4 ||
 		filter->ether_type == ETHER_TYPE_IPv6) {
-		PMD_DRV_LOG(ERR, "unsupported ether_type(0x%04x) in"
-			" control packet filter.", filter->ether_type);
+		PMD_DRV_LOG(ERR,
+			"unsupported ether_type(0x%04x) in control packet filter.",
+			filter->ether_type);
 		return -EINVAL;
 	}
 	if (filter->ether_type == ETHER_TYPE_VLAN)
-		PMD_DRV_LOG(WARNING, "filter vlan ether_type in first tag is"
-			" not supported.");
+		PMD_DRV_LOG(WARNING,
+			"filter vlan ether_type in first tag is not supported.");
 
 	/* Check if there is the filter in SW list */
 	memset(&check_filter, 0, sizeof(check_filter));
@@ -8398,11 +8408,10 @@ i40e_ethertype_filter_set(struct i40e_pf *pf,
 			pf->main_vsi->seid,
 			filter->queue, add, &stats, NULL);
 
-	PMD_DRV_LOG(INFO, "add/rem control packet filter, return %d,"
-			 " mac_etype_used = %u, etype_used = %u,"
-			 " mac_etype_free = %u, etype_free = %u\n",
-			 ret, stats.mac_etype_used, stats.etype_used,
-			 stats.mac_etype_free, stats.etype_free);
+	PMD_DRV_LOG(INFO,
+		"add/rem control packet filter, return %d, mac_etype_used = %u, etype_used = %u, mac_etype_free = %u, etype_free = %u\n",
+		ret, stats.mac_etype_used, stats.etype_used,
+		stats.mac_etype_free, stats.etype_free);
 	if (ret < 0)
 		return -ENOSYS;
 
@@ -8720,9 +8729,9 @@ i40e_configure_registers(struct i40e_hw *hw)
 		ret = i40e_aq_debug_write_register(hw, reg_table[i].addr,
 						reg_table[i].val, NULL);
 		if (ret < 0) {
-			PMD_DRV_LOG(ERR, "Failed to write 0x%"PRIx64" to the "
-				"address of 0x%"PRIx32, reg_table[i].val,
-							reg_table[i].addr);
+			PMD_DRV_LOG(ERR,
+				"Failed to write 0x%"PRIx64" to the address of 0x%"PRIx32,
+				reg_table[i].val, reg_table[i].addr);
 			break;
 		}
 		PMD_DRV_LOG(DEBUG, "Write 0x%"PRIx64" to the address of "
@@ -8767,8 +8776,9 @@ i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi)
 						   I40E_VSI_L2TAGSTXVALID(
 						   vsi->vsi_id), reg, NULL);
 		if (ret < 0) {
-			PMD_DRV_LOG(ERR, "Failed to update "
-				"VSI_L2TAGSTXVALID[%d]", vsi->vsi_id);
+			PMD_DRV_LOG(ERR,
+				"Failed to update VSI_L2TAGSTXVALID[%d]",
+				vsi->vsi_id);
 			return I40E_ERR_CONFIG;
 		}
 	}
@@ -8819,11 +8829,10 @@ i40e_aq_add_mirror_rule(struct i40e_hw *hw,
 
 	rte_memcpy(&desc.params.raw, &cmd, sizeof(cmd));
 	status = i40e_asq_send_command(hw, &desc, entries, buff_len, NULL);
-	PMD_DRV_LOG(INFO, "i40e_aq_add_mirror_rule, aq_status %d,"
-			 "rule_id = %u"
-			 " mirror_rules_used = %u, mirror_rules_free = %u,",
-			 hw->aq.asq_last_status, resp->rule_id,
-			 resp->mirror_rules_used, resp->mirror_rules_free);
+	PMD_DRV_LOG(INFO,
+		"i40e_aq_add_mirror_rule, aq_status %d, rule_id = %u mirror_rules_used = %u, mirror_rules_free = %u,",
+		hw->aq.asq_last_status, resp->rule_id,
+		resp->mirror_rules_used, resp->mirror_rules_free);
 	*rule_id = rte_le_to_cpu_16(resp->rule_id);
 
 	return status;
@@ -8901,8 +8910,8 @@ i40e_mirror_rule_set(struct rte_eth_dev *dev,
 	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_set: sw_id = %d.", sw_id);
 
 	if (pf->main_vsi->veb == NULL || pf->vfs == NULL) {
-		PMD_DRV_LOG(ERR, "mirror rule can not be configured"
-			" without veb or vfs.");
+		PMD_DRV_LOG(ERR,
+			"mirror rule can not be configured without veb or vfs.");
 		return -ENOSYS;
 	}
 	if (pf->nb_mirror_rule > I40E_MAX_MIRROR_RULES) {
@@ -8934,9 +8943,9 @@ i40e_mirror_rule_set(struct rte_eth_dev *dev,
 					mirr_rule->entries,
 					mirr_rule->num_entries, mirr_rule->id);
 			if (ret < 0) {
-				PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
-						   " ret = %d, aq_err = %d.",
-						   ret, hw->aq.asq_last_status);
+				PMD_DRV_LOG(ERR,
+					"failed to remove mirror rule: ret = %d, aq_err = %d.",
+					ret, hw->aq.asq_last_status);
 				return -ENOSYS;
 			}
 			TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
@@ -9025,9 +9034,9 @@ i40e_mirror_rule_set(struct rte_eth_dev *dev,
 				      mirr_rule->rule_type, mirr_rule->entries,
 				      j, &rule_id);
 	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "failed to add mirror rule:"
-				   " ret = %d, aq_err = %d.",
-				   ret, hw->aq.asq_last_status);
+		PMD_DRV_LOG(ERR,
+			"failed to add mirror rule: ret = %d, aq_err = %d.",
+			ret, hw->aq.asq_last_status);
 		rte_free(mirr_rule);
 		return -ENOSYS;
 	}
@@ -9079,9 +9088,9 @@ i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id)
 				mirr_rule->entries,
 				mirr_rule->num_entries, mirr_rule->id);
 		if (ret < 0) {
-			PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
-					   " status = %d, aq_err = %d.",
-					   ret, hw->aq.asq_last_status);
+			PMD_DRV_LOG(ERR,
+				"failed to remove mirror rule: status = %d, aq_err = %d.",
+				ret, hw->aq.asq_last_status);
 			return -ENOSYS;
 		}
 		TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
@@ -9513,9 +9522,9 @@ i40e_config_switch_comp_tc(struct i40e_veb *veb, uint8_t tc_map)
 	ret = i40e_aq_config_switch_comp_bw_config(hw, veb->seid,
 						   &veb_bw, NULL);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "AQ command Config switch_comp BW allocation"
-				  " per TC failed = %d",
-				  hw->aq.asq_last_status);
+		PMD_INIT_LOG(ERR,
+			"AQ command Config switch_comp BW allocation per TC failed = %d",
+			hw->aq.asq_last_status);
 		return ret;
 	}
 
@@ -9523,16 +9532,18 @@ i40e_config_switch_comp_tc(struct i40e_veb *veb, uint8_t tc_map)
 	ret = i40e_aq_query_switch_comp_ets_config(hw, veb->seid,
 						   &ets_query, NULL);
 	if (ret != I40E_SUCCESS) {
-		PMD_DRV_LOG(ERR, "Failed to get switch_comp ETS"
-				 " configuration %u", hw->aq.asq_last_status);
+		PMD_DRV_LOG(ERR,
+			"Failed to get switch_comp ETS configuration %u",
+			hw->aq.asq_last_status);
 		return ret;
 	}
 	memset(&bw_query, 0, sizeof(bw_query));
 	ret = i40e_aq_query_switch_comp_bw_config(hw, veb->seid,
 						  &bw_query, NULL);
 	if (ret != I40E_SUCCESS) {
-		PMD_DRV_LOG(ERR, "Failed to get switch_comp bandwidth"
-				 " configuration %u", hw->aq.asq_last_status);
+		PMD_DRV_LOG(ERR,
+			"Failed to get switch_comp bandwidth configuration %u",
+			hw->aq.asq_last_status);
 		return ret;
 	}
 
@@ -9597,8 +9608,8 @@ i40e_vsi_config_tc(struct i40e_vsi *vsi, uint8_t tc_map)
 	}
 	ret = i40e_aq_config_vsi_tc_bw(hw, vsi->seid, &bw_data, NULL);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "AQ command Config VSI BW allocation"
-			" per TC failed = %d",
+		PMD_INIT_LOG(ERR,
+			"AQ command Config VSI BW allocation per TC failed = %d",
 			hw->aq.asq_last_status);
 		goto out;
 	}
@@ -9619,9 +9630,8 @@ i40e_vsi_config_tc(struct i40e_vsi *vsi, uint8_t tc_map)
 	/* Update the VSI after updating the VSI queue-mapping information */
 	ret = i40e_aq_update_vsi_params(hw, &ctxt, NULL);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Failed to configure "
-			    "TC queue mapping = %d",
-			    hw->aq.asq_last_status);
+		PMD_INIT_LOG(ERR, "Failed to configure TC queue mapping = %d",
+			hw->aq.asq_last_status);
 		goto out;
 	}
 	/* update the local VSI info with updated queue map */
@@ -9673,8 +9683,8 @@ i40e_dcb_hw_configure(struct i40e_pf *pf,
 	/* Use the FW API if FW > v4.4*/
 	if (!(((hw->aq.fw_maj_ver == 4) && (hw->aq.fw_min_ver >= 4)) ||
 	      (hw->aq.fw_maj_ver >= 5))) {
-		PMD_INIT_LOG(ERR, "FW < v4.4, can not use FW LLDP API"
-				  " to configure DCB");
+		PMD_INIT_LOG(ERR,
+			"FW < v4.4, can not use FW LLDP API to configure DCB");
 		return I40E_ERR_FIRMWARE_API_VERSION;
 	}
 
@@ -9740,8 +9750,8 @@ i40e_dcb_hw_configure(struct i40e_pf *pf,
 							 I40E_DEFAULT_TCMAP);
 			if (ret)
 				PMD_INIT_LOG(WARNING,
-					 "Failed configuring TC for VSI seid=%d\n",
-					 vsi_list->vsi->seid);
+					"Failed configuring TC for VSI seid=%d\n",
+					vsi_list->vsi->seid);
 			/* continue */
 		}
 	}
@@ -9801,15 +9811,15 @@ i40e_dcb_init_configure(struct rte_eth_dev *dev, bool sw_dcb)
 						I40E_APP_PROTOID_FCOE;
 			ret = i40e_set_dcb_config(hw);
 			if (ret) {
-				PMD_INIT_LOG(ERR, "default dcb config fails."
-					" err = %d, aq_err = %d.", ret,
-					  hw->aq.asq_last_status);
+				PMD_INIT_LOG(ERR,
+					"default dcb config fails. err = %d, aq_err = %d.",
+					ret, hw->aq.asq_last_status);
 				return -ENOSYS;
 			}
 		} else {
-			PMD_INIT_LOG(ERR, "DCB initialization in FW fails,"
-					  " err = %d, aq_err = %d.", ret,
-					  hw->aq.asq_last_status);
+			PMD_INIT_LOG(ERR,
+				"DCB initialization in FW fails, err = %d, aq_err = %d.",
+				ret, hw->aq.asq_last_status);
 			return -ENOTSUP;
 		}
 	} else {
@@ -9820,14 +9830,14 @@ i40e_dcb_init_configure(struct rte_eth_dev *dev, bool sw_dcb)
 		ret = i40e_init_dcb(hw);
 		if (!ret) {
 			if (hw->dcbx_status == I40E_DCBX_STATUS_DISABLED) {
-				PMD_INIT_LOG(ERR, "HW doesn't support"
-						  " DCBX offload.");
+				PMD_INIT_LOG(ERR,
+					"HW doesn't support DCBX offload.");
 				return -ENOTSUP;
 			}
 		} else {
-			PMD_INIT_LOG(ERR, "DCBX configuration failed, err = %d,"
-					  " aq_err = %d.", ret,
-					  hw->aq.asq_last_status);
+			PMD_INIT_LOG(ERR,
+				"DCBX configuration failed, err = %d, aq_err = %d.",
+				ret, hw->aq.asq_last_status);
 			return -ENOTSUP;
 		}
 	}
-- 
2.11.0

^ permalink raw reply related

* [PATCH v2] net/ixgbe: fix API parameter checking
From: Bernard Iremonger @ 2017-01-11 17:24 UTC (permalink / raw)
  To: dev, wenzhuo.lu, ferruh.yigit; +Cc: Bernard Iremonger, stable
In-Reply-To: <1484144751-10412-1-git-send-email-bernard.iremonger@intel.com>

Add checks to rte_pmd_ixgbe_* API's to ensure that the port
is an ixgbe port.

Fixes: 49e248223e9f ("net/ixgbe: add API for VF management")

CC: stable@dpdk.org
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
Changes in v2:
Moved pmd checks into new function is_ixgbe_pmd.

 drivers/net/ixgbe/ixgbe_ethdev.c | 55 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index b7ddd4f..cc8f7d6 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -244,6 +244,7 @@ static void ixgbe_remove_rar(struct rte_eth_dev *dev, uint32_t index);
 static void ixgbe_set_default_mac_addr(struct rte_eth_dev *dev,
 					   struct ether_addr *mac_addr);
 static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct ixgbe_dcb_config *dcb_config);
+static int is_ixgbe_pmd(const char *driver_name);
 
 /* For Virtual Function support */
 static int eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev);
@@ -4050,6 +4051,18 @@ ixgbe_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr)
 	ixgbe_add_rar(dev, addr, 0, 0);
 }
 
+static int
+is_ixgbe_pmd(const char *driver_name)
+{
+	if (!strstr(driver_name, "ixgbe"))
+		return -ENOTSUP;
+
+	if (strstr(driver_name, "ixgbe_vf"))
+		return -ENOTSUP;
+
+	return 0;
+}
+
 int
 rte_pmd_ixgbe_set_vf_mac_addr(uint8_t port, uint16_t vf,
 		struct ether_addr *mac_addr)
@@ -4066,6 +4079,9 @@ rte_pmd_ixgbe_set_vf_mac_addr(uint8_t port, uint16_t vf,
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+		return -ENOTSUP;
+
 	if (vf >= dev_info.max_vfs)
 		return -EINVAL;
 
@@ -4564,6 +4580,9 @@ rte_pmd_ixgbe_set_vf_vlan_anti_spoof(uint8_t port, uint16_t vf, uint8_t on)
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+		return -ENOTSUP;
+
 	if (vf >= dev_info.max_vfs)
 		return -EINVAL;
 
@@ -4591,6 +4610,9 @@ rte_pmd_ixgbe_set_vf_mac_anti_spoof(uint8_t port, uint16_t vf, uint8_t on)
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+		return -ENOTSUP;
+
 	if (vf >= dev_info.max_vfs)
 		return -EINVAL;
 
@@ -4617,10 +4639,13 @@ rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf, uint16_t vlan_id)
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+		return -ENOTSUP;
+
 	if (vf >= dev_info.max_vfs)
 		return -EINVAL;
 
-	if (vlan_id > 4095)
+	if (vlan_id > ETHER_MAX_VLAN_ID)
 		return -EINVAL;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -4643,10 +4668,15 @@ rte_pmd_ixgbe_set_tx_loopback(uint8_t port, uint8_t on)
 	struct ixgbe_hw *hw;
 	uint32_t ctrl;
 	struct rte_eth_dev *dev;
+	struct rte_eth_dev_info dev_info;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
+	rte_eth_dev_info_get(port, &dev_info);
+
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+		return -ENOTSUP;
 
 	if (on > 1)
 		return -EINVAL;
@@ -4672,10 +4702,15 @@ rte_pmd_ixgbe_set_all_queues_drop_en(uint8_t port, uint8_t on)
 	int i;
 	int num_queues = (int)(IXGBE_QDE_IDX_MASK >> IXGBE_QDE_IDX_SHIFT);
 	struct rte_eth_dev *dev;
+	struct rte_eth_dev_info dev_info;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
+	rte_eth_dev_info_get(port, &dev_info);
+
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+		return -ENOTSUP;
 
 	if (on > 1)
 		return -EINVAL;
@@ -4704,6 +4739,9 @@ rte_pmd_ixgbe_set_vf_split_drop_en(uint8_t port, uint16_t vf, uint8_t on)
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+		return -ENOTSUP;
+
 	/* only support VF's 0 to 63 */
 	if ((vf >= dev_info.max_vfs) || (vf > 63))
 		return -EINVAL;
@@ -4736,6 +4774,9 @@ rte_pmd_ixgbe_set_vf_vlan_stripq(uint8_t port, uint16_t vf, uint8_t on)
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+		return -ENOTSUP;
+
 	if (vf >= dev_info.max_vfs)
 		return -EINVAL;
 
@@ -4775,7 +4816,7 @@ rte_pmd_ixgbe_set_vf_rxmode(uint8_t port, uint16_t vf, uint16_t rx_mask, uint8_t
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
-	if (strstr(dev_info.driver_name, "ixgbe_vf"))
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
 		return -ENOTSUP;
 
 	if (vf >= dev_info.max_vfs)
@@ -4822,7 +4863,7 @@ rte_pmd_ixgbe_set_vf_rx(uint8_t port, uint16_t vf, uint8_t on)
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
-	if (strstr(dev_info.driver_name, "ixgbe_vf"))
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
 		return -ENOTSUP;
 
 	if (vf >= dev_info.max_vfs)
@@ -4873,7 +4914,7 @@ rte_pmd_ixgbe_set_vf_tx(uint8_t port, uint16_t vf, uint8_t on)
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
-	if (strstr(dev_info.driver_name, "ixgbe_vf"))
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
 		return -ENOTSUP;
 
 	if (vf >= dev_info.max_vfs)
@@ -4922,7 +4963,7 @@ rte_pmd_ixgbe_set_vf_vlan_filter(uint8_t port, uint16_t vlan,
 	dev = &rte_eth_devices[port];
 	rte_eth_dev_info_get(port, &dev_info);
 
-	if (strstr(dev_info.driver_name, "ixgbe_vf"))
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
 		return -ENOTSUP;
 
 	if ((vlan > ETHER_MAX_VLAN_ID) || (vf_mask == 0))
@@ -4965,7 +5006,7 @@ int rte_pmd_ixgbe_set_vf_rate_limit(uint8_t port, uint16_t vf,
 	rte_eth_dev_info_get(port, &dev_info);
 	rte_eth_link_get_nowait(port, &link);
 
-	if (strstr(dev_info.driver_name, "ixgbe_vf"))
+	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
 		return -ENOTSUP;
 
 	if (vf >= dev_info.max_vfs)
-- 
2.10.1

^ permalink raw reply related

* Re: [PATCH] kni: use bulk functions to allocate and free mbufs
From: Ananyev, Konstantin @ 2017-01-11 17:28 UTC (permalink / raw)
  To: Stephen Hemminger, Sergey Vyazmitinov
  Cc: olivier.matz@6wind.com, Yigit, Ferruh, dev@dpdk.org
In-Reply-To: <20170111081759.7b1ee146@xeon-e3>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Wednesday, January 11, 2017 4:18 PM
> To: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>
> Cc: olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> 
> On Fri, 30 Dec 2016 04:50:16 +0700
> Sergey Vyazmitinov <s.vyazmitinov@brain4net.com> wrote:
> 
> >  /**
> > + * Free n packets mbuf back into its original mempool.
> > + *
> > + * Free each mbuf, and all its segments in case of chained buffers. Each
> > + * segment is added back into its original mempool.
> > + *
> > + * @param mp
> > + *   The packets mempool.
> > + * @param mbufs
> > + *   The packets mbufs array to be freed.
> > + * @param n
> > + *   Number of packets.
> > + */
> > +static inline void rte_pktmbuf_free_bulk(struct rte_mempool *mp,
> > +		struct rte_mbuf **mbufs, unsigned n)
> > +{
> > +	struct rte_mbuf *mbuf, *m_next;
> > +	unsigned i;
> > +	for (i = 0; i < n; ++i) {
> > +		mbuf = mbufs[i];
> > +		__rte_mbuf_sanity_check(mbuf, 1);
> > +
> > +		mbuf = mbuf->next;
> > +		while (mbuf != NULL) {
> > +			m_next = mbuf->next;
> > +			rte_pktmbuf_free_seg(mbuf);
> > +			mbuf = m_next;
> > +		}
> > +	}
> > +	rte_mempool_put_bulk(mp, (void * const *)mbufs, n);
> > +}
> 
> The mbufs may come from different pools. You need to handle that.

I suppose both stituations are possible:
1) user knows off-hand that all mbufs in the group are from the same mempool
2) user can't guarantee that all mbufs in the group are from same mempool.

As I understand that patch is for case 1) only.
For 2) it could be a separate function and separate patch.

Konstantin

^ permalink raw reply

* Re: [PATCH] kni: fast data availability check in thread_single loop
From: Ferruh Yigit @ 2017-01-11 17:29 UTC (permalink / raw)
  To: Sergey Vyazmitinov; +Cc: dev
In-Reply-To: <1483053795-8489-1-git-send-email-s.vyazmitinov@brain4net.com>

On 12/29/2016 11:23 PM, Sergey Vyazmitinov wrote:
> This allow to significant reduces packets processing latency.
> 
> Signed-off-by: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>
> ---
>  .../linuxapp/eal/include/exec-env/rte_kni_common.h |  6 ++++
>  lib/librte_eal/linuxapp/kni/kni_misc.c             | 33 ++++++++++++++++------
>  2 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> index 09713b0..8183a8e 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> @@ -109,6 +109,12 @@ struct rte_kni_fifo {
>  	void *volatile buffer[];     /**< The buffer contains mbuf pointers */
>  };
>  
> +static inline int
> +kni_fifo_empty(struct rte_kni_fifo *fifo)
> +{
> +	return fifo->write == fifo->read;
> +}
> +
>  /*
>   * The kernel image of the rte_mbuf struct, with only the relevant fields.
>   * Padding is necessary to assure the offsets of these fields
> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
> index 497db9b..4bf9bfa 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> @@ -45,6 +45,7 @@ MODULE_AUTHOR("Intel Corporation");
>  MODULE_DESCRIPTION("Kernel Module for managing kni devices");
>  
>  #define KNI_RX_LOOP_NUM 1000
> +#define KNI_RX_DATA_LOOP_NUM 2500
>  
>  #define KNI_MAX_DEVICES 32
>  
> @@ -129,25 +130,39 @@ static struct pernet_operations kni_net_ops = {
>  #endif
>  };
>  
> -static int
> -kni_thread_single(void *data)
> +static inline void
> +kni_thread_single_rx_data_loop(struct kni_net *knet)
>  {
> -	struct kni_net *knet = data;
> -	int j;
>  	struct kni_dev *dev;
> +	int i;
>  
> -	while (!kthread_should_stop()) {
> -		down_read(&knet->kni_list_lock);
> -		for (j = 0; j < KNI_RX_LOOP_NUM; j++) {
> -			list_for_each_entry(dev, &knet->kni_list_head, list) {
> +	for (i = 0; i < KNI_RX_DATA_LOOP_NUM; ++i) {

When there are multiple KNI interfaces, and lets assume there is traffic
too, this will behave like:

KNI1x2500 data_packets + KNI2x2500 data_packets .... KNI10x2500

After data packets, KNI1 resp_packet + KNI2 resp_packets ...

Won't this scenario also may cause latency? And perhaps jitter according
KNI interface traffic loads?

This may be good for some use cases, but not sure if this is good for all.

> +		list_for_each_entry(dev, &knet->kni_list_head, list) {
> +			/* Burst dequeue from rx_q */
> +			if (!kni_fifo_empty((struct rte_kni_fifo *)dev->rx_q)) {

Do we need this check, since first thing in kni_net_rx_normal() is
checking if there is item in the queue?

>  #ifdef RTE_KNI_VHOST
>  				kni_chk_vhost_rx(dev);
>  #else
>  				kni_net_rx(dev);
>  #endif
> -				kni_net_poll_resp(dev);
>  			}
>  		}
> +	}
> +	list_for_each_entry(dev, &knet->kni_list_head, list) {
> +		kni_net_poll_resp(dev);
> +	}
> +}
> +
> +static int
> +kni_thread_single(void *data)
> +{
> +	struct kni_net *knet = data;
> +	int j;
> +
> +	while (!kthread_should_stop()) {
> +		down_read(&knet->kni_list_lock);
> +		for (j = 0; j < KNI_RX_LOOP_NUM; j++)
> +			kni_thread_single_rx_data_loop(knet);
>  		up_read(&knet->kni_list_lock);
>  #ifdef RTE_KNI_PREEMPT_DEFAULT
>  		/* reschedule out for a while */
> 

^ permalink raw reply

* Re: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
From: Olivier MATZ @ 2017-01-11 17:32 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Tiwei Bie, Ananyev, Konstantin, Adrien Mazarguil, dev,
	Lu, Wenzhuo, Mcnamara, John, olivier.matz, Zhang, Helin, Dai, Wei,
	Wang, Xiao W
In-Reply-To: <1542539.LCBRG7nZDl@xps13>

Hi Tiwei, Hi Thomas,

On Mon, 09 Jan 2017 12:26:53 +0100, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2017-01-09 11:57, Tiwei Bie:
> > On Sun, Jan 08, 2017 at 08:39:55PM +0800, Ananyev, Konstantin
> > wrote:  
> > > > Well my first reply to this thread was asking why isn't the
> > > > whole API global from the start then?  
> > > 
> > > That's good question, and my preference would always be to have
> > > the API to configure this feature as generic one.
> > > I guess the main reason why it is not right now we don't reach an
> > > agreement how this API should look like: 
> > > http://dpdk.org/ml/archives/dev/2016-September/047810.html
> > > But I'll leave it to the author to provide the real reason
> > > here.   
> > 
> > Yes, currently this work just provided a thin layer over 82599's
> > hardware MACsec offload support to allow users configure 82599's
> > MACsec offload engine. The current API may be too specific and may
> > need a rework to be used with other NICs.  
> 
> I think it is a really good approach to start such API privately in a
> driver. It will give us more time and experience to design a proper
> generic API.
> 
> Regarding the mbuf flag, it looks straight-forward, and as it is IEEE
> standardized, I do not see any objection to add it now.
> However, I will wait for the approval of Olivier - as maintainer of
> mbuf.
> 

Generally speaking, we have to be careful when introducing new mbuf
flags, since we don't have so much of them (~25 remaining out of 64,
which mean we may run out of them in 3-4 years).

In this particular case, despite the flag is added for an ixgbe-specific
API, when MACsec will be implemented on another PMD, the exact same
flag would also be needed. That's the same for the ethdev capability
flag. Moreover, as Thomas stated, it's a standardized protocol so it's
legitimate to have it included in rte_mbuf.h.

For me, having PMD-specific APIs for a new feature is not a problem,
but I think we should try to have a generic API as soon as the feature
is supported by several PMDs.

Regards,
Olivier

^ permalink raw reply

* Re: [PATCH] kni: use bulk functions to allocate and free mbufs
From: Stephen Hemminger @ 2017-01-11 17:35 UTC (permalink / raw)
  To: Ananyev, Konstantin
  Cc: Sergey Vyazmitinov, olivier.matz@6wind.com, Yigit, Ferruh,
	dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F103F8F@irsmsx105.ger.corp.intel.com>

On Wed, 11 Jan 2017 17:28:21 +0000
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:

> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > Sent: Wednesday, January 11, 2017 4:18 PM
> > To: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>
> > Cc: olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> > 
> > On Fri, 30 Dec 2016 04:50:16 +0700
> > Sergey Vyazmitinov <s.vyazmitinov@brain4net.com> wrote:
> >   
> > >  /**
> > > + * Free n packets mbuf back into its original mempool.
> > > + *
> > > + * Free each mbuf, and all its segments in case of chained buffers. Each
> > > + * segment is added back into its original mempool.
> > > + *
> > > + * @param mp
> > > + *   The packets mempool.
> > > + * @param mbufs
> > > + *   The packets mbufs array to be freed.
> > > + * @param n
> > > + *   Number of packets.
> > > + */
> > > +static inline void rte_pktmbuf_free_bulk(struct rte_mempool *mp,
> > > +		struct rte_mbuf **mbufs, unsigned n)
> > > +{
> > > +	struct rte_mbuf *mbuf, *m_next;
> > > +	unsigned i;
> > > +	for (i = 0; i < n; ++i) {
> > > +		mbuf = mbufs[i];
> > > +		__rte_mbuf_sanity_check(mbuf, 1);
> > > +
> > > +		mbuf = mbuf->next;
> > > +		while (mbuf != NULL) {
> > > +			m_next = mbuf->next;
> > > +			rte_pktmbuf_free_seg(mbuf);
> > > +			mbuf = m_next;
> > > +		}
> > > +	}
> > > +	rte_mempool_put_bulk(mp, (void * const *)mbufs, n);
> > > +}  
> > 
> > The mbufs may come from different pools. You need to handle that.  
> 
> I suppose both stituations are possible:
> 1) user knows off-hand that all mbufs in the group are from the same mempool
> 2) user can't guarantee that all mbufs in the group are from same mempool.
> 
> As I understand that patch is for case 1) only.
> For 2) it could be a separate function and separate patch.
> 
> Konstantin
> 
> 

Please don't make unnecessary assumptions in pursuit of minor optimizations.
It is trivial to write a correct free bulk that handles pool changing.
Also the free_seg could be bulked as well.

^ permalink raw reply

* Re: [PATCH 4/7] kni: remove useless return statements
From: Ferruh Yigit @ 2017-01-11 17:36 UTC (permalink / raw)
  To: Stephen Hemminger, dev
In-Reply-To: <20170109233022.31154-5-stephen@networkplumber.org>

On 1/9/2017 11:30 PM, Stephen Hemminger wrote:
> A return statement at end of void function is unnecessary.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_82575.c   |  4 ----
>  lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_i210.c    |  2 --
>  lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_mac.c     |  4 ----
>  lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c     |  1 -
>  lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c   |  3 +--
>  lib/librte_eal/linuxapp/kni/ethtool/igb/igb_main.c      |  2 --
>  lib/librte_eal/linuxapp/kni/ethtool/igb/igb_vmdq.c      | 11 ++---------
>  lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82598.c |  2 --
>  8 files changed, 3 insertions(+), 26 deletions(-)
> 

I am not sure about benefit of refactoring drivers in KNI, since we have
no intention to do development on that code, at best we can get updated
version of the driver and replace, which I don't expect to happen soon.

But if you have strong opinion on this:
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply

* Re: [PATCH v6 1/6] mbuf: add flag for MACsec
From: Olivier MATZ @ 2017-01-11 17:41 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: dev, adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1484109098-8936-2-git-send-email-tiwei.bie@intel.com>

On Wed, 11 Jan 2017 12:31:33 +0800, Tiwei Bie <tiwei.bie@intel.com>
wrote:
> Add a new Tx flag in mbuf, that can be set by applications to
> enable the MACsec offload for a packet to be transmitted.
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

^ permalink raw reply

* Re: [PATCH] kni: use bulk functions to allocate and free mbufs
From: Ananyev, Konstantin @ 2017-01-11 17:43 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Sergey Vyazmitinov, olivier.matz@6wind.com, Yigit, Ferruh,
	dev@dpdk.org
In-Reply-To: <20170111093559.753a0fc9@xeon-e3>



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, January 11, 2017 5:36 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>; olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> 
> On Wed, 11 Jan 2017 17:28:21 +0000
> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > > Sent: Wednesday, January 11, 2017 4:18 PM
> > > To: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>
> > > Cc: olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> > >
> > > On Fri, 30 Dec 2016 04:50:16 +0700
> > > Sergey Vyazmitinov <s.vyazmitinov@brain4net.com> wrote:
> > >
> > > >  /**
> > > > + * Free n packets mbuf back into its original mempool.
> > > > + *
> > > > + * Free each mbuf, and all its segments in case of chained buffers. Each
> > > > + * segment is added back into its original mempool.
> > > > + *
> > > > + * @param mp
> > > > + *   The packets mempool.
> > > > + * @param mbufs
> > > > + *   The packets mbufs array to be freed.
> > > > + * @param n
> > > > + *   Number of packets.
> > > > + */
> > > > +static inline void rte_pktmbuf_free_bulk(struct rte_mempool *mp,
> > > > +		struct rte_mbuf **mbufs, unsigned n)
> > > > +{
> > > > +	struct rte_mbuf *mbuf, *m_next;
> > > > +	unsigned i;
> > > > +	for (i = 0; i < n; ++i) {
> > > > +		mbuf = mbufs[i];
> > > > +		__rte_mbuf_sanity_check(mbuf, 1);
> > > > +
> > > > +		mbuf = mbuf->next;
> > > > +		while (mbuf != NULL) {
> > > > +			m_next = mbuf->next;
> > > > +			rte_pktmbuf_free_seg(mbuf);
> > > > +			mbuf = m_next;
> > > > +		}
> > > > +	}
> > > > +	rte_mempool_put_bulk(mp, (void * const *)mbufs, n);
> > > > +}
> > >
> > > The mbufs may come from different pools. You need to handle that.
> >
> > I suppose both stituations are possible:
> > 1) user knows off-hand that all mbufs in the group are from the same mempool
> > 2) user can't guarantee that all mbufs in the group are from same mempool.
> >
> > As I understand that patch is for case 1) only.
> > For 2) it could be a separate function and separate patch.
> >
> > Konstantin
> >
> >
> 
> Please don't make unnecessary assumptions in pursuit of minor optimizations.

I don't suggest to make *any* assumptions.
What I am saying we  can have 2 functions for two different cases.
Obviously we'll have to document it properly.
Konstantin

> It is trivial to write a correct free bulk that handles pool changing.
> Also the free_seg could be bulked as well.

^ permalink raw reply

* Re: [PATCH] kni: use bulk functions to allocate and free mbufs
From: Ferruh Yigit @ 2017-01-11 17:47 UTC (permalink / raw)
  To: Ananyev, Konstantin, Stephen Hemminger
  Cc: Sergey Vyazmitinov, olivier.matz@6wind.com, dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F103FCA@irsmsx105.ger.corp.intel.com>

On 1/11/2017 5:43 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>> Sent: Wednesday, January 11, 2017 5:36 PM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>> Cc: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>; olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
>>
>> On Wed, 11 Jan 2017 17:28:21 +0000
>> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
>>
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
>>>> Sent: Wednesday, January 11, 2017 4:18 PM
>>>> To: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>
>>>> Cc: olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
>>>>
>>>> On Fri, 30 Dec 2016 04:50:16 +0700
>>>> Sergey Vyazmitinov <s.vyazmitinov@brain4net.com> wrote:
>>>>
>>>>>  /**
>>>>> + * Free n packets mbuf back into its original mempool.
>>>>> + *
>>>>> + * Free each mbuf, and all its segments in case of chained buffers. Each
>>>>> + * segment is added back into its original mempool.
>>>>> + *
>>>>> + * @param mp
>>>>> + *   The packets mempool.
>>>>> + * @param mbufs
>>>>> + *   The packets mbufs array to be freed.
>>>>> + * @param n
>>>>> + *   Number of packets.
>>>>> + */
>>>>> +static inline void rte_pktmbuf_free_bulk(struct rte_mempool *mp,
>>>>> +		struct rte_mbuf **mbufs, unsigned n)
>>>>> +{
>>>>> +	struct rte_mbuf *mbuf, *m_next;
>>>>> +	unsigned i;
>>>>> +	for (i = 0; i < n; ++i) {
>>>>> +		mbuf = mbufs[i];
>>>>> +		__rte_mbuf_sanity_check(mbuf, 1);
>>>>> +
>>>>> +		mbuf = mbuf->next;
>>>>> +		while (mbuf != NULL) {
>>>>> +			m_next = mbuf->next;
>>>>> +			rte_pktmbuf_free_seg(mbuf);
>>>>> +			mbuf = m_next;
>>>>> +		}
>>>>> +	}
>>>>> +	rte_mempool_put_bulk(mp, (void * const *)mbufs, n);
>>>>> +}
>>>>
>>>> The mbufs may come from different pools. You need to handle that.
>>>
>>> I suppose both stituations are possible:
>>> 1) user knows off-hand that all mbufs in the group are from the same mempool
>>> 2) user can't guarantee that all mbufs in the group are from same mempool.
>>>
>>> As I understand that patch is for case 1) only.
>>> For 2) it could be a separate function and separate patch.
>>>
>>> Konstantin
>>>
>>>
>>
>> Please don't make unnecessary assumptions in pursuit of minor optimizations.
> 
> I don't suggest to make *any* assumptions.
> What I am saying we  can have 2 functions for two different cases.

kni_free_mbufs() is static function. Even user knows if all mbufs are
some same pool or not, can't pass this information to the free function.

Of course this information can be passed via new API, or as an update to
exiting API, but I think it is better to update free function to cover
both cases instead of getting this information from user.

> Obviously we'll have to document it properly.
> Konstantin
> 
>> It is trivial to write a correct free bulk that handles pool changing.
>> Also the free_seg could be bulked as well.

^ permalink raw reply

* Re: [PATCH v3 1/1] i40e: improve message grepability
From: Ferruh Yigit @ 2017-01-11 17:50 UTC (permalink / raw)
  To: Michał Mirosław, dev
In-Reply-To: <e2c6a6379c3e31292436988cdd8d7d15675a22bb.1484154780.git.mirq-linux@rere.qmqm.pl>

On 1/11/2017 5:13 PM, Michał Mirosław wrote:
> From: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> 
> Join message lines for easier grepping.
> PRIxXX are left glued to strings as they are in other parts of the file.
> 
> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply

* Re: [PATCH v3 1/1] i40e: improve message grepability
From: Ferruh Yigit @ 2017-01-11 17:52 UTC (permalink / raw)
  To: Michał Mirosław, dev
In-Reply-To: <6f17c4c8-5cea-e5b7-2c53-aaa4545d0787@intel.com>

On 1/11/2017 5:50 PM, Ferruh Yigit wrote:
> On 1/11/2017 5:13 PM, Michał Mirosław wrote:
>> From: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
>>
>> Join message lines for easier grepping.
>> PRIxXX are left glued to strings as they are in other parts of the file.
>>
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [PATCH] net/enic: use new Rx checksum flags
From: Ferruh Yigit @ 2017-01-11 18:00 UTC (permalink / raw)
  To: John Daley; +Cc: dev
In-Reply-To: <20170111034212.5294-1-johndale@cisco.com>

On 1/11/2017 3:42 AM, John Daley wrote:
> Use the new L3 and L4 ..CKSUM_GOOD  and ..CKSUM_UNKNOWN flags to
> distinguish good checksums from unknown ones.
> 
> Signed-off-by: John Daley <johndale@cisco.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [PATCH] net/enic: remove unnecessary function parameter attributes
From: Ferruh Yigit @ 2017-01-11 18:00 UTC (permalink / raw)
  To: John Daley; +Cc: dev, stable
In-Reply-To: <20170111034212.5294-2-johndale@cisco.com>

On 1/11/2017 3:42 AM, John Daley wrote:
> Remove __rte_unused attributes in function declaration when
> the parameters really are used.
> 
> Fixes: dfbd6a9cb504 ("net/enic: extend flow director support for 1300 series")
> 
> CC: stable@dpdk.org
> Signed-off-by: John Daley <johndale@cisco.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [dpdk-stable] [PATCH] app/testpmd: fix ixgbe private API calling
From: Ferruh Yigit @ 2017-01-11 18:07 UTC (permalink / raw)
  To: Iremonger, Bernard, Lu, Wenzhuo, dev@dpdk.org
  Cc: Wu, Jingjing, stable@dpdk.org
In-Reply-To: <8CEF83825BEC744B83065625E567D7C224D1CDBD@IRSMSX108.ger.corp.intel.com>

On 1/11/2017 4:29 PM, Iremonger, Bernard wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, January 11, 2017 4:19 PM
>> To: Iremonger, Bernard <bernard.iremonger@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>; dev@dpdk.org
>> Cc: Wu, Jingjing <jingjing.wu@intel.com>; stable@dpdk.org
>> Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix ixgbe
>> private API calling
>>
>> On 1/11/2017 3:47 PM, Iremonger, Bernard wrote:
>>> Hi Ferruh,
>>>
>>>> -----Original Message-----
>>>> From: Yigit, Ferruh
>>>> Sent: Wednesday, January 11, 2017 3:27 PM
>>>> To: Iremonger, Bernard <bernard.iremonger@intel.com>; Lu, Wenzhuo
>>>> <wenzhuo.lu@intel.com>; dev@dpdk.org
>>>> Cc: Wu, Jingjing <jingjing.wu@intel.com>; stable@dpdk.org
>>>> Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix ixgbe
>>>> private API calling
>>>>
>>>> On 1/11/2017 3:20 PM, Iremonger, Bernard wrote:
>>>>> Hi Wenzhuo,
>>>>>
>>>>> <snip>
>>>>>>> Subject: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API
>>>>>>> calling
>>>>>>>
>>>>>>> Some ixgbe private APIs are added to expose ixgbe specific functions.
>>>>>>> When they're used by testpmd, there's no check for if the NICs are
>>>> ixgbe.
>>>>>>> Other NICs also have chance to  call these APIs.
>>>>>>> This patch add the check and the feedback print.
>>>>>>
>>>>>> I am not sure that testpmd is the right place to do this.
>>>>>> The rte_pmd_ixgbe_* functions are public API's which can be called
>>>>>> by other applications.
>>>>>> The checks should be in the rte_pmd_ixgbe_* API's
>>>>>
>>>>> It is useful to handle the return code -ENOTSUP in testpmd.
>>>>>
>>>>
>>>> Makes sense, and I think it is good idea to add them in your patch,
>>>> since it introduces returning -ENOTSUP, would you mind sending a new
>>>> version of your patch with this update?
>>>> So we can drop this patch completely.
>>>>
>>>> Thanks,
>>>> ferruh
>>>>
>>> I don't think this patch should be dropped.
>>> Testpmd is already handling -EINVAL and -ENODEV.
>>> It makes sense for it to handle -ENOTSUP for the cases in the patch.
>>
>> This patch adds driver check [1] before ixgbe APIs, since now that check is
>> done within ixgbe APIs by your patch. Why do we need this patch at all?
>>
>> [1]
>> if (strstr(dev_info.driver_name, "ixgbe") != NULL)
>  
> I think our lines have got slightly crossed.
> This patch is doing two things, the check [1] above which is not needed now (after my ixgbe patch).
> Secondly it is handling the ret code of the rte_pmd_ixgbe_* API's, I think this is useful.

I was thinking returning -ENOTSUP introduced for your patch, but it
seems this is not true for all functions, some is already returning
-ENOTSUP, so testpmd should cover them.

I believe intention of this patch is the first item you have mentioned,
but can be converted to second one too.

Wenzhuo,

When do you have time, can you convert this patch to one that covers
"-ENOTSUP" error messages for ixgbe APIs please? If you don't have time,
please let me know I can do the patch?

Thanks,
ferruh

> 
> Regards,
> 
> Bernard.
> 
>   
> 

^ permalink raw reply

* Pktgen DPDK net bonding mode 4 802.3AD (LACP) not working with BIGIP trunk with LACP enabled
From: Vincent Li @ 2017-01-11 18:08 UTC (permalink / raw)
  To: users; +Cc: dev

Hi,

I have direct two cable connections between Pktgen DPDK box and BIGIP

this is a problem I am not sure if it Pktgen, or the DPDK net bonding mode
4 LACP, or the BIGIP trunk with LACP enabled issue, I am hoping someone
here have more clue and give me some direction on how to trouble shoot
this. I am suspecting there might some issue between DPDK net bonding
8023ad mode 4 and BIGIP trunk LACP mode. I turned
on CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG and
defined RTE_LIBRTE_BOND_DEBUG_8023AD
in drivers/net/bonding/rte_eth_bond_8023ad.c hoping getting more debug
output in Pktgen, I could not see any debug output for 8023ad protocol.


packet tx/rx works :

1 Pktgen with DPDK net bonding mode 1, 2, 3, 5
2 BIGIP trunk with LACP disabled (trunk port status UP)

packet tx/rx failed:

1, Pktgen with DPDK net bonding mode 4 802.3AD
2, BIGIP trunk with LACP enabled (trunk status is DOWN)

or
1, Pktgen with DPDK net bonding mode 4 802.3AD
2, BIGIP trunk with LACP disabled (trunk status is UP)

my end goal is to achieve 10G < througput < 20G with Pktgen DPDK net
bonding with Intel 82599 to load test BIGIP, so far with bonding mode 1,
2,3, 5, I am unable to achieve throughput > 10G.

^ permalink raw reply

* Re: [PATCH v2] net/ixgbe: fix API parameter checking
From: Ferruh Yigit @ 2017-01-11 18:21 UTC (permalink / raw)
  To: Bernard Iremonger, dev, wenzhuo.lu; +Cc: stable
In-Reply-To: <1484155499-13349-1-git-send-email-bernard.iremonger@intel.com>

On 1/11/2017 5:24 PM, Bernard Iremonger wrote:
> Add checks to rte_pmd_ixgbe_* API's to ensure that the port
> is an ixgbe port.
> 
> Fixes: 49e248223e9f ("net/ixgbe: add API for VF management")
> 
> CC: stable@dpdk.org
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [PATCH] kni: use bulk functions to allocate and free mbufs
From: Ananyev, Konstantin @ 2017-01-11 18:25 UTC (permalink / raw)
  To: Yigit, Ferruh, Stephen Hemminger
  Cc: Sergey Vyazmitinov, olivier.matz@6wind.com, dev@dpdk.org
In-Reply-To: <63819ae1-f056-0ad7-b7dd-041fe1fe08fa@intel.com>



> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 5:48 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Stephen Hemminger <stephen@networkplumber.org>
> Cc: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>; olivier.matz@6wind.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> 
> On 1/11/2017 5:43 PM, Ananyev, Konstantin wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >> Sent: Wednesday, January 11, 2017 5:36 PM
> >> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> >> Cc: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>; olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>;
> dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> >>
> >> On Wed, 11 Jan 2017 17:28:21 +0000
> >> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> >>
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> >>>> Sent: Wednesday, January 11, 2017 4:18 PM
> >>>> To: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>
> >>>> Cc: olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> >>>>
> >>>> On Fri, 30 Dec 2016 04:50:16 +0700
> >>>> Sergey Vyazmitinov <s.vyazmitinov@brain4net.com> wrote:
> >>>>
> >>>>>  /**
> >>>>> + * Free n packets mbuf back into its original mempool.
> >>>>> + *
> >>>>> + * Free each mbuf, and all its segments in case of chained buffers. Each
> >>>>> + * segment is added back into its original mempool.
> >>>>> + *
> >>>>> + * @param mp
> >>>>> + *   The packets mempool.
> >>>>> + * @param mbufs
> >>>>> + *   The packets mbufs array to be freed.
> >>>>> + * @param n
> >>>>> + *   Number of packets.
> >>>>> + */
> >>>>> +static inline void rte_pktmbuf_free_bulk(struct rte_mempool *mp,
> >>>>> +		struct rte_mbuf **mbufs, unsigned n)
> >>>>> +{
> >>>>> +	struct rte_mbuf *mbuf, *m_next;
> >>>>> +	unsigned i;
> >>>>> +	for (i = 0; i < n; ++i) {
> >>>>> +		mbuf = mbufs[i];
> >>>>> +		__rte_mbuf_sanity_check(mbuf, 1);
> >>>>> +
> >>>>> +		mbuf = mbuf->next;
> >>>>> +		while (mbuf != NULL) {
> >>>>> +			m_next = mbuf->next;
> >>>>> +			rte_pktmbuf_free_seg(mbuf);
> >>>>> +			mbuf = m_next;
> >>>>> +		}
> >>>>> +	}
> >>>>> +	rte_mempool_put_bulk(mp, (void * const *)mbufs, n);
> >>>>> +}
> >>>>
> >>>> The mbufs may come from different pools. You need to handle that.
> >>>
> >>> I suppose both stituations are possible:
> >>> 1) user knows off-hand that all mbufs in the group are from the same mempool
> >>> 2) user can't guarantee that all mbufs in the group are from same mempool.
> >>>
> >>> As I understand that patch is for case 1) only.
> >>> For 2) it could be a separate function and separate patch.
> >>>
> >>> Konstantin
> >>>
> >>>
> >>
> >> Please don't make unnecessary assumptions in pursuit of minor optimizations.
> >
> > I don't suggest to make *any* assumptions.
> > What I am saying we  can have 2 functions for two different cases.
> 
> kni_free_mbufs() is static function. Even user knows if all mbufs are
> some same pool or not, can't pass this information to the free function.
> 
> Of course this information can be passed via new API, or as an update to
> exiting API, but I think it is better to update free function to cover
> both cases instead of getting this information from user.

I suppose misunderstanding came from the fact that kni_free_mbufs()
is modified to use rte_pktmbuf_free_bulk(mp, mbufs, n).
I am not talking about kni part of the patch
(to be honest I didn't pay much attention to it).
What I am saying there are many situations when user knows off-hand
that all  mbufs in that group are from the same mempool and such
function will be useful too.
BTW, for my own curiosity, how it could happen with KNI that 
kni_fifo_get() would return mbufs not from kni->pktmbuf_pool
(I am not really familiar with KNI and its use-cases)?
Konstantin

> 
> > Obviously we'll have to document it properly.
> > Konstantin
> >
> >> It is trivial to write a correct free bulk that handles pool changing.
> >> Also the free_seg could be bulked as well.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox