[dpdk-dev] [PATCH v7 03/21] eal/linux: generalize PCI kernel unbinding driver to EAL

[Jianbo Liu]

On 28 October 2016 at 20:26, Shreyansh Jain  wrote:
> From: Jan Viktorin 
>
> Generalize the PCI-specific pci_unbind_kernel_driver. It is now divided
> into two parts. First, determination of the path and string identification
> of the device to be unbound. Second, the actual unbind operation which is
> generic.
>
> BSD implementation updated as ENOTSUP
>
> Signed-off-by: Jan Viktorin 
> Signed-off-by: Shreyansh Jain 
> --
> Changes since v2:
>  - update BSD support for unbind kernel driver
> ---
>  lib/librte_eal/bsdapp/eal/eal.c   |  7 +++
>  lib/librte_eal/bsdapp/eal/eal_pci.c   |  4 ++--
>  lib/librte_eal/common/eal_private.h   | 13 +
>  lib/librte_eal/linuxapp/eal/eal.c | 26 ++
>  lib/librte_eal/linuxapp/eal/eal_pci.c | 33 +
>  5 files changed, 57 insertions(+), 26 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 35e3117..5271fc2 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -633,3 +633,10 @@ rte_eal_process_type(void)
>  {
> return rte_config.process_type;
>  }
> +
> +int
> +rte_eal_unbind_kernel_driver(const char *devpath __rte_unused,
> +const char *devid __rte_unused)
> +{
> +   return -ENOTSUP;
> +}
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c 
> b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index 7ed0115..703f034 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -89,11 +89,11 @@
>
>  /* unbind kernel driver for this device */
>  int
> -pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)
> +pci_unbind_kernel_driver(struct rte_pci_device *dev)
>  {
> RTE_LOG(ERR, EAL, "RTE_PCI_DRV_FORCE_UNBIND flag is not implemented "
> "for BSD\n");
> -   return -ENOTSUP;
> +   return rte_eal_unbind_kernel_driver(dev);

Missing the second parameter for devid.

>  }
>
>  /* Map pci device */
> diff --git a/lib/librte_eal/common/eal_private.h 
> b/lib/librte_eal/common/eal_private.h
> index 9e7d8f6..b0c208a 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -256,6 +256,19 @@ int rte_eal_alarm_init(void);
>  int rte_eal_check_module(const char *module_name);
>
>  /**
> + * Unbind kernel driver bound to the device specified by the given devpath,
> + * and its string identification.
> + *
> + * @param devpath  path to the device directory ("/sys/.../devices/")
> + * @param devididentification of the device ()
> + *
> + * @return
> + *  -1  unbind has failed
> + *   0  module has been unbound
> + */
> +int rte_eal_unbind_kernel_driver(const char *devpath, const char *devid);
> +
> +/**
>   * Get cpu core_id.
>   *
>   * This function is private to the EAL.
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
> b/lib/librte_eal/linuxapp/eal/eal.c
> index 2075282..5f6676d 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -943,3 +943,29 @@ rte_eal_check_module(const char *module_name)
> /* Module has been found */
> return 1;
>  }
> +
> +int
> +rte_eal_unbind_kernel_driver(const char *devpath, const char *devid)
> +{
> +   char filename[PATH_MAX];
> +   FILE *f;
> +
> +   snprintf(filename, sizeof(filename),
> +"%s/driver/unbind", devpath);
> +
> +   f = fopen(filename, "w");
> +   if (f == NULL) /* device was not bound */
> +   return 0;
> +
> +   if (fwrite(devid, strlen(devid), 1, f) == 0) {
> +   RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__,
> +   filename);
> +   goto error;
> +   }
> +
> +   fclose(f);
> +   return 0;
> +error:
> +   fclose(f);
> +   return -1;
> +}
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c 
> b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 876ba38..a03553f 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -59,38 +59,23 @@ int
>  pci_unbind_kernel_driver(struct rte_pci_device *dev)
>  {
> int n;
> -   FILE *f;
> -   char filename[PATH_MAX];
> -   char buf[BUFSIZ];
> +   char devpath[PATH_MAX];
> +   char devid[BUFSIZ];
> struct rte_pci_addr *loc = &dev->addr;
>
> -   /* open /sys/bus/pci/devices/:BB:CC.D/driver */
> -   snprintf(filename, sizeof(filename),
> -   "%s/" PCI_PRI_FMT "/driver/unbind", pci_get_sysfs_path(),
> +   /* devpath /sys/bus/pci/devices/:BB:CC.D */
> +   snprintf(devpath, sizeof(devpath),
> +   "%s/" PCI_PRI_FMT, pci_get_sysfs_path(),
> loc->domain, loc->bus, loc->devid, loc->function);
>
> -   f = fopen(filename, "w");
> -   if (f == NULL) /* device was not bound */
> -   return 0;
> -
> -   n = snprintf(buf, sizeof(buf), PCI_P

[dpdk-dev] [PATCH v3] net/qede: fix advertising link speed capability

[Harish Patil]

>
>2016-10-31 11:35, Rasesh Mody:
>> From: Harish Patil 
>> 
>> Fix to advertise device's link speed capability based on NVM
>> port configuration instead of returning driver supported speeds.
>> 
>> Fixes: 95e67b479506 ("net/qede: add 100G link speed capability")
>> 
>> Signed-off-by: Harish Patil 
>[...]
>> +/* Fill up the native advertised speed */
>> +switch (params.speed.advertised_speeds) {
>> +case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_10G:
>> +adv_speed = 1;
>> +break;
>> +case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_25G:
>> +adv_speed = 25000;
>> +break;
>> +case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_40G:
>> +adv_speed = 4;
>> +break;
>> +case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_50G:
>> +adv_speed = 5;
>> +break;
>> +case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_BB_100G:
>> +adv_speed = 10;
>> +break;
>> +default:
>> +DP_NOTICE(hwfn, false, "Unknown speed\n");
>> +adv_speed = 0;
>> +}
>> +if_link->adv_speed = adv_speed;
>
>The qede devices support only one speed?
>I guess it is wrong but it is a step in right direction so it
>will be enough for 16.11.
>
>Applied
>
qede device is capable of 10, 25, 40, 50, 100Gb speeds.
It's configured at factory to have 100Gb, 50Gb, 40Gb or 25Gb speeds.
A unique PCI ID gets assigned to the device based on the speed configured.
25G device can auto-negotiate down to 10G speeds when connected to a 10G
switch.

So only for 25G case the above logic does not work correctly, for which I
have a submitted a minor fix today:
("[PATCH] net/qede: fix unknown speed errmsg for 25G link?). Pls include
it in 16.11.

Thanks,
Harish

[dpdk-dev] [PATCH] app/test: fix crash of lpm test

[Dai, Wei]

Hi, Oliver

Thanks for your catching this bug.
After reviewing related codes, I can acknowledge it.

Thanks
-Wei
> -Original Message-
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> Sent: Wednesday, November 9, 2016 9:08 PM
> To: dev at dpdk.org; Dai, Wei 
> Cc: Richardson, Bruce 
> Subject: [PATCH] app/test: fix crash of lpm test
> 
> The test recently added accesses to lpm->tbl8[ip >> 8] with is much larger 
> than
> the size of the table, causing a crash of the test application.
> 
> Fix this typo by replacing tbl8 by tbl24.
> 
> Fixes: 231fa88ed522 ("app/test: verify LPM tbl8 recycle")
> 
> Signed-off-by: Olivier Matz 
Acked-by: Wei Dai 

> ---
> 
> Hi Wei,
> 
> I don't know lpm very well and I did not spend much time to understand the
> test case. I guess that's the proper fix, but please check carefully that I'm 
> not
> doing something wrong :)
> 
> Thanks,
> Olivier
> 
> 
>  app/test/test_lpm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test/test_lpm.c b/app/test/test_lpm.c index 80e0efc..41ae80f
> 100644
> --- a/app/test/test_lpm.c
> +++ b/app/test/test_lpm.c
> @@ -1256,7 +1256,7 @@ test18(void)
>   rte_lpm_add(lpm, ip, depth, next_hop);
> 
>   TEST_LPM_ASSERT(lpm->tbl24[ip>>8].valid_group);
> - tbl8_group_index = lpm->tbl8[ip>>8].group_idx;
> + tbl8_group_index = lpm->tbl24[ip>>8].group_idx;
> 
>   depth = 23;
>   next_hop = 2;
> @@ -1272,7 +1272,7 @@ test18(void)
>   rte_lpm_add(lpm, ip, depth, next_hop);
> 
>   TEST_LPM_ASSERT(lpm->tbl24[ip>>8].valid_group);
> - TEST_LPM_ASSERT(tbl8_group_index == lpm->tbl8[ip>>8].group_idx);
> + TEST_LPM_ASSERT(tbl8_group_index == lpm->tbl24[ip>>8].group_idx);
> 
>   depth = 24;
>   next_hop = 4;
> @@ -1288,7 +1288,7 @@ test18(void)
>   rte_lpm_add(lpm, ip, depth, next_hop);
> 
>   TEST_LPM_ASSERT(lpm->tbl24[ip>>8].valid_group);
> - TEST_LPM_ASSERT(tbl8_group_index == lpm->tbl8[ip>>8].group_idx);
> + TEST_LPM_ASSERT(tbl8_group_index == lpm->tbl24[ip>>8].group_idx);
> 
>   rte_lpm_free(lpm);
>  #undef group_idx
> --
> 2.8.1

[dpdk-dev] [PATCH v7 08/21] eal/soc: implement SoC device list and dump

[Jianbo Liu]

On 28 October 2016 at 20:26, Shreyansh Jain  wrote:
> From: Jan Viktorin 
>
> SoC devices would be linked in a separate list (from PCI). This is used for
> probe function.
> A helper for dumping the device list is added.
>
> Signed-off-by: Jan Viktorin 
> Signed-off-by: Shreyansh Jain 
> Signed-off-by: Hemant Agrawal 
> ---
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 ++
>  lib/librte_eal/common/eal_common_soc.c  | 34 
> +
>  lib/librte_eal/common/include/rte_soc.h |  9 +++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 ++
>  4 files changed, 47 insertions(+)
>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map 
> b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index cf6fb8e..86e3cfd 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -171,11 +171,13 @@ DPDK_16.11 {
> rte_eal_dev_attach;
> rte_eal_dev_detach;
> rte_eal_map_resource;
> +   rte_eal_soc_dump;
> rte_eal_soc_register;
> rte_eal_soc_unregister;
> rte_eal_unmap_resource;
> rte_eal_vdrv_register;
> rte_eal_vdrv_unregister;
> +   soc_device_list;
> soc_driver_list;
>
>  } DPDK_16.07;
> diff --git a/lib/librte_eal/common/eal_common_soc.c 
> b/lib/librte_eal/common/eal_common_soc.c
> index 56135ed..5dcddc5 100644
> --- a/lib/librte_eal/common/eal_common_soc.c
> +++ b/lib/librte_eal/common/eal_common_soc.c
> @@ -31,6 +31,8 @@
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>
> +#include 
> +#include 
>  #include 
>
>  #include 
> @@ -40,6 +42,38 @@
>  /* Global SoC driver list */
>  struct soc_driver_list soc_driver_list =
> TAILQ_HEAD_INITIALIZER(soc_driver_list);
> +struct soc_device_list soc_device_list =
> +   TAILQ_HEAD_INITIALIZER(soc_device_list);
> +
> +/* dump one device */
> +static int
> +soc_dump_one_device(FILE *f, struct rte_soc_device *dev)
> +{
> +   int i;
> +
> +   fprintf(f, "%s", dev->addr.name);
> +   fprintf(f, " - fdt_path: %s\n",
> +   dev->addr.fdt_path ? dev->addr.fdt_path : "(none)");
> +
> +   for (i = 0; dev->id && dev->id[i].compatible; ++i)
> +   fprintf(f, "   %s\n", dev->id[i].compatible);
> +
> +   return 0;
> +}
> +
> +/* dump devices on the bus to an output stream */
> +void
> +rte_eal_soc_dump(FILE *f)
> +{
> +   struct rte_soc_device *dev = NULL;
> +
> +   if (!f)
> +   return;
> +
> +   TAILQ_FOREACH(dev, &soc_device_list, next) {
> +   soc_dump_one_device(f, dev);
> +   }
> +}
>
>  /* register a driver */
>  void
> diff --git a/lib/librte_eal/common/include/rte_soc.h 
> b/lib/librte_eal/common/include/rte_soc.h
> index 23b06a9..347e611 100644
> --- a/lib/librte_eal/common/include/rte_soc.h
> +++ b/lib/librte_eal/common/include/rte_soc.h
> @@ -56,8 +56,12 @@ extern "C" {
>
>  extern struct soc_driver_list soc_driver_list;
>  /**< Global list of SoC Drivers */
> +extern struct soc_device_list soc_device_list;
> +/**< Global list of SoC Devices */
>
>  TAILQ_HEAD(soc_driver_list, rte_soc_driver); /**< SoC drivers in D-linked Q. 
> */
> +TAILQ_HEAD(soc_device_list, rte_soc_device); /**< SoC devices in D-linked Q. 
> */
> +
>
>  struct rte_soc_id {
> const char *compatible; /**< OF compatible specification */
> @@ -142,6 +146,11 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
>  }
>
>  /**
> + * Dump discovered SoC devices.
> + */
> +void rte_eal_soc_dump(FILE *f);

If it is to dump device information (not driver), is it proper to
rename it rte_eal_soc_device_dump()?

> +
> +/**
>   * Register a SoC driver.
>   */
>  void rte_eal_soc_register(struct rte_soc_driver *driver);
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map 
> b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index ab6b985..0155025 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -175,11 +175,13 @@ DPDK_16.11 {
> rte_eal_dev_attach;
> rte_eal_dev_detach;
> rte_eal_map_resource;
> +   rte_eal_soc_dump;
> rte_eal_soc_register;
> rte_eal_soc_unregister;
> rte_eal_unmap_resource;
> rte_eal_vdrv_register;
> rte_eal_vdrv_unregister;
> +   soc_device_list;
> soc_driver_list;
>
>  } DPDK_16.07;
> --
> 2.7.4
>

[dpdk-dev] [PATCH v7 11/21] eal/soc: implement probing of drivers

[Jianbo Liu]

On 28 October 2016 at 20:26, Shreyansh Jain  wrote:
> Each SoC PMD registers a set of callback for scanning its own bus/infra and
> matching devices to drivers when probe is called.
> This patch introduces the infra for calls to SoC scan on rte_eal_soc_init()
> and match on rte_eal_soc_probe().
>
> Patch also adds test case for scan and probe.
>
> Signed-off-by: Jan Viktorin 
> Signed-off-by: Shreyansh Jain 
> Signed-off-by: Hemant Agrawal 
> --
> v4:
>  - Update test_soc for descriptive test function names
>  - Comments over test functions
>  - devinit and devuninint --> probe/remove
>  - RTE_VERIFY at some places
> ---
>  app/test/test_soc.c | 205 ++-
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
>  lib/librte_eal/common/eal_common_soc.c  | 213 
> +++-
>  lib/librte_eal/common/include/rte_soc.h |  75 -
>  lib/librte_eal/linuxapp/eal/eal.c   |   5 +
>  lib/librte_eal/linuxapp/eal/eal_soc.c   |  21 ++-
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   4 +
>  7 files changed, 519 insertions(+), 8 deletions(-)
>
.

>  /**
> + * SoC device scan callback, called from rte_eal_soc_init.
> + * For various SoC, the bus on which devices are attached maynot be compliant
> + * to a standard platform (or platform bus itself). In which case, extra
> + * steps are implemented by PMD to scan over the bus and add devices to SoC
> + * device list.
> + */
> +typedef void (soc_scan_t)(void);

I'm still not sure about the purpose of soc_scan, and how to use it.
If it's for each driver, it should at least struct rte_soc_driver * as
its parameter.
If it's for each bus, why it is in rte_soc_driver?
I know you will implement bus driver in the future, but we need to
make it clear for current simplified implementation.

> +
> +/**
> + * Custom device<=>driver match callback for SoC
> + * Unlike PCI, SoC devices don't have a fixed definition of device
> + * identification. PMDs can implement a specific matching function in which
> + * driver and device objects are provided to perform custom match.
> + */
> +typedef int (soc_match_t)(struct rte_soc_driver *, struct rte_soc_device *);
> +
> +/**
>   * A structure describing a SoC driver.
>   */
>  struct rte_soc_driver {
> @@ -104,6 +120,8 @@ struct rte_soc_driver {
> struct rte_driver driver;  /**< Inherit core driver. */
> soc_probe_t *probe;/**< Device probe */
> soc_remove_t *remove;  /**< Device remove */
> +   soc_scan_t *scan_fn;   /**< Callback for scanning SoC 
> bus*/
> +   soc_match_t *match_fn; /**< Callback to match dev<->drv */
> const struct rte_soc_id *id_table; /**< ID table, NULL terminated */
>  };
>
> @@ -146,12 +164,63 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
>  }
>
>  /**
> + * Default function for matching the Soc driver with device. Each driver can
> + * either use this function or define their own soc matching function.
> + * This function relies on the compatible string extracted from sysfs. But,
> + * a SoC might have different way of identifying its devices. Such SoC can
> + * override match_fn.
> + *
> + * @return
> + *  0 on success
> + * -1 when no match found
> +  */
> +int
> +rte_eal_soc_match_compat(struct rte_soc_driver *drv,
> +struct rte_soc_device *dev);
> +
> +/**
> + * Probe SoC devices for registered drivers.
> + *
> + * @return
> + * 0 on success
> + * !0 in case of any failure in probe
> + */
> +int rte_eal_soc_probe(void);
> +
> +/**
> + * Probe the single SoC device.
> + */
> +int rte_eal_soc_probe_one(const struct rte_soc_addr *addr);
> +
> +/**
> + * Close the single SoC device.
> + *
> + * Scan the SoC devices and find the SoC device specified by the SoC
> + * address, then call the remove() function for registered driver
> + * that has a matching entry in its id_table for discovered device.
> + *
> + * @param addr
> + * The SoC address to close.
> + * @return
> + *   - 0 on success.
> + *   - Negative on error.
> + */
> +int rte_eal_soc_detach(const struct rte_soc_addr *addr);
> +
> +/**
>   * Dump discovered SoC devices.
> + *
> + * @param f
> + * File to dump device info in.
>   */
>  void rte_eal_soc_dump(FILE *f);
>
>  /**
>   * Register a SoC driver.
> + *
> + * @param driver
> + * Object for SoC driver to register
> + * @return void
>   */
>  void rte_eal_soc_register(struct rte_soc_driver *driver);
>
> @@ -167,6 +236,10 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
>
>  /**
>   * Unregister a SoC driver.
> + *
> + * @param driver
> + * Object for SoC driver to unregister
> + * @return void
>   */
>  void rte_eal_soc_unregister(struct rte_soc_driver *driver);
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
> b/lib/librte_eal/linuxapp/eal/eal.c
> index 098ba02..bd775f3 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/li

[dpdk-dev] [PATCH v7 06/21] eal/soc: introduce very essential SoC infra definitions

[Jianbo Liu]

On 28 October 2016 at 20:26, Shreyansh Jain  wrote:
> From: Jan Viktorin 
>
> Define initial structures and functions for the SoC infrastructure.
> This patch supports only a very minimal functions for now.
> More features will be added in the following commits.
>
> Includes rte_device/rte_driver inheritance of
> rte_soc_device/rte_soc_driver.
>
> Signed-off-by: Jan Viktorin 
> Signed-off-by: Shreyansh Jain 
> Signed-off-by: Hemant Agrawal 
> ---
>  app/test/Makefile   |   1 +
>  app/test/test_soc.c |  90 +
>  lib/librte_eal/common/Makefile  |   2 +-
>  lib/librte_eal/common/eal_private.h |   4 +
>  lib/librte_eal/common/include/rte_soc.h | 138 
> 
>  5 files changed, 234 insertions(+), 1 deletion(-)
>  create mode 100644 app/test/test_soc.c
>  create mode 100644 lib/librte_eal/common/include/rte_soc.h
>
...


> +/**
> + * Utility function to write a SoC device name, this device name can later be
> + * used to retrieve the corresponding rte_soc_addr using above functions.
> + *
> + * @param addr
> + * The SoC address
> + * @param output
> + * The output buffer string
> + * @param size
> + * The output buffer size
> + * @return
> + *  0 on success, negative on error.
> + */
> +static inline void
> +rte_eal_soc_device_name(const struct rte_soc_addr *addr,
> +   char *output, size_t size)
> +{
> +   int ret;
> +
> +   RTE_VERIFY(addr != NULL);
> +   RTE_VERIFY(size >= strlen(addr->name));

Is it better to use (size > strlen(addr->name)?

> +   ret = snprintf(output, size, "%s", addr->name);
> +   RTE_VERIFY(ret >= 0);
> +}
> +
> +static inline int
> +rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
> +const struct rte_soc_addr *a1)
> +{
> +   if (a0 == NULL || a1 == NULL)
> +   return -1;
> +
> +   RTE_VERIFY(a0->name != NULL);
> +   RTE_VERIFY(a1->name != NULL);
> +
> +   return strcmp(a0->name, a1->name);
> +}
> +
> +#endif
> --
> 2.7.4
>

[dpdk-dev] [PATCH v7 03/21] eal/linux: generalize PCI kernel unbinding driver to EAL

[Shreyansh Jain]

Hello Jianbo,

Thanks a lot for your time in commenting this. My comments inline (as 
well as on other similar mails).

On Thursday 10 November 2016 07:54 AM, Jianbo Liu wrote:
> On 28 October 2016 at 20:26, Shreyansh Jain  wrote:
>> From: Jan Viktorin 
>>
>> Generalize the PCI-specific pci_unbind_kernel_driver. It is now divided
>> into two parts. First, determination of the path and string identification
>> of the device to be unbound. Second, the actual unbind operation which is
>> generic.
>>
>> BSD implementation updated as ENOTSUP
>>
>> Signed-off-by: Jan Viktorin 
>> Signed-off-by: Shreyansh Jain 
>> --
>> Changes since v2:
>>  - update BSD support for unbind kernel driver
>> ---
>>  lib/librte_eal/bsdapp/eal/eal.c   |  7 +++
>>  lib/librte_eal/bsdapp/eal/eal_pci.c   |  4 ++--
>>  lib/librte_eal/common/eal_private.h   | 13 +
>>  lib/librte_eal/linuxapp/eal/eal.c | 26 ++
>>  lib/librte_eal/linuxapp/eal/eal_pci.c | 33 +
>>  5 files changed, 57 insertions(+), 26 deletions(-)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/eal.c 
>> b/lib/librte_eal/bsdapp/eal/eal.c
>> index 35e3117..5271fc2 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal.c
>> @@ -633,3 +633,10 @@ rte_eal_process_type(void)
>>  {
>> return rte_config.process_type;
>>  }
>> +
>> +int
>> +rte_eal_unbind_kernel_driver(const char *devpath __rte_unused,
>> +const char *devid __rte_unused)
>> +{
>> +   return -ENOTSUP;
>> +}
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c 
>> b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> index 7ed0115..703f034 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> @@ -89,11 +89,11 @@
>>
>>  /* unbind kernel driver for this device */
>>  int
>> -pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)
>> +pci_unbind_kernel_driver(struct rte_pci_device *dev)
>>  {
>> RTE_LOG(ERR, EAL, "RTE_PCI_DRV_FORCE_UNBIND flag is not implemented "
>> "for BSD\n");
>> -   return -ENOTSUP;
>> +   return rte_eal_unbind_kernel_driver(dev);
>
> Missing the second parameter for devid.

Indeed. I will fix this.
Being BSD, I didn't compile test this part. I will have to find a way to 
fix this in my sanity before sending next series.

>
>>  }
>>
>>  /* Map pci device */
>> diff --git a/lib/librte_eal/common/eal_private.h 
>> b/lib/librte_eal/common/eal_private.h
>> index 9e7d8f6..b0c208a 100644
>> --- a/lib/librte_eal/common/eal_private.h
>> +++ b/lib/librte_eal/common/eal_private.h
>> @@ -256,6 +256,19 @@ int rte_eal_alarm_init(void);
>>  int rte_eal_check_module(const char *module_name);
>>
>>  /**
>> + * Unbind kernel driver bound to the device specified by the given devpath,
>> + * and its string identification.
>> + *
>> + * @param devpath  path to the device directory ("/sys/.../devices/")
>> + * @param devididentification of the device ()
>> + *
>> + * @return
>> + *  -1  unbind has failed
>> + *   0  module has been unbound
>> + */
>> +int rte_eal_unbind_kernel_driver(const char *devpath, const char *devid);
>> +
>> +/**
>>   * Get cpu core_id.
>>   *
>>   * This function is private to the EAL.
>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
>> b/lib/librte_eal/linuxapp/eal/eal.c
>> index 2075282..5f6676d 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -943,3 +943,29 @@ rte_eal_check_module(const char *module_name)
>> /* Module has been found */
>> return 1;
>>  }
>> +
>> +int
>> +rte_eal_unbind_kernel_driver(const char *devpath, const char *devid)
>> +{
>> +   char filename[PATH_MAX];
>> +   FILE *f;
>> +
>> +   snprintf(filename, sizeof(filename),
>> +"%s/driver/unbind", devpath);
>> +
>> +   f = fopen(filename, "w");
>> +   if (f == NULL) /* device was not bound */
>> +   return 0;
>> +
>> +   if (fwrite(devid, strlen(devid), 1, f) == 0) {
>> +   RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__,
>> +   filename);
>> +   goto error;
>> +   }
>> +
>> +   fclose(f);
>> +   return 0;
>> +error:
>> +   fclose(f);
>> +   return -1;
>> +}
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c 
>> b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> index 876ba38..a03553f 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> @@ -59,38 +59,23 @@ int
>>  pci_unbind_kernel_driver(struct rte_pci_device *dev)
>>  {
>> int n;
>> -   FILE *f;
>> -   char filename[PATH_MAX];
>> -   char buf[BUFSIZ];
>> +   char devpath[PATH_MAX];
>> +   char devid[BUFSIZ];
>> struct rte_pci_addr *loc = &dev->addr;
>>
>> -   /* open /sys/bus/pci/devices/:BB:CC.D/driver */
>> -   snprintf(filename, sizeof(filename),
>> 

[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

[Yao, Lei A]

I'm testing some DPDK sample under VMware. During the testing work, I find 
l3fwd+ ixgbe vf can work ,but  L3fwd + i40evf  can't work. So I reported this 
issue to Bjorn. From my perspective, if can add new parameter in l3fwd sample 
like what have already don?t in  testpmd "crc-strip enable" is a better way 
to resolve this issue. 

Lei 

-Original Message-
From: Topel, Bjorn 
Sent: Wednesday, November 9, 2016 9:10 PM
To: Zhang, Helin ; Ananyev, Konstantin 
; dev at dpdk.org
Cc: Xu, Qian Q ; Yao, Lei A ; 
Wu, Jingjing ; thomas.monjalon at 6wind.com
Subject: Re: [dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

Bj?rn/Konstantin wrote:
>> Finally, why doesn't l3fwd have the CRC stripped?
>
> I don?t know any good reason for that for l3fwd or any other sample 
> app. I think it is just a 'historical' reason.

Ok! Then I'd suggest changing the l3fwd default to actually *strip* CRC instead 
of not doing it. Lei, any comments?

Helin wrote:
> Yes, i40e driver changed a little bit on that according to the review 
> comments during implementation, comparing to igb and ixgbe.
> I'd suggest to re-invesitgate if we can do the similar thing in igb 
> and ixgbe driver.

Good. Let's do that!

> Any critical issue now? Or just an improvement comments?

Not from my perspective. The issue is that Lei needs some kind of work-around 
for l3fwd with i40evf, so I'll let Lei comment on how critical it is.


Bj?rn

[dpdk-dev] [PATCH v7 06/21] eal/soc: introduce very essential SoC infra definitions

[Shreyansh Jain]

On Thursday 10 November 2016 09:39 AM, Jianbo Liu wrote:
> On 28 October 2016 at 20:26, Shreyansh Jain  wrote:
>> From: Jan Viktorin 
>>
>> Define initial structures and functions for the SoC infrastructure.
>> This patch supports only a very minimal functions for now.
>> More features will be added in the following commits.
>>
>> Includes rte_device/rte_driver inheritance of
>> rte_soc_device/rte_soc_driver.
>>
>> Signed-off-by: Jan Viktorin 
>> Signed-off-by: Shreyansh Jain 
>> Signed-off-by: Hemant Agrawal 
>> ---
>>  app/test/Makefile   |   1 +
>>  app/test/test_soc.c |  90 +
>>  lib/librte_eal/common/Makefile  |   2 +-
>>  lib/librte_eal/common/eal_private.h |   4 +
>>  lib/librte_eal/common/include/rte_soc.h | 138 
>> 
>>  5 files changed, 234 insertions(+), 1 deletion(-)
>>  create mode 100644 app/test/test_soc.c
>>  create mode 100644 lib/librte_eal/common/include/rte_soc.h
>>
> 
>
>
>> +/**
>> + * Utility function to write a SoC device name, this device name can later 
>> be
>> + * used to retrieve the corresponding rte_soc_addr using above functions.
>> + *
>> + * @param addr
>> + * The SoC address
>> + * @param output
>> + * The output buffer string
>> + * @param size
>> + * The output buffer size
>> + * @return
>> + *  0 on success, negative on error.
>> + */
>> +static inline void
>> +rte_eal_soc_device_name(const struct rte_soc_addr *addr,
>> +   char *output, size_t size)
>> +{
>> +   int ret;
>> +
>> +   RTE_VERIFY(addr != NULL);
>> +   RTE_VERIFY(size >= strlen(addr->name));
>
> Is it better to use (size > strlen(addr->name)?

Yes, I missed the '\0' in this.
I will fix this.

>
>> +   ret = snprintf(output, size, "%s", addr->name);
>> +   RTE_VERIFY(ret >= 0);
>> +}
>> +
>> +static inline int
>> +rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
>> +const struct rte_soc_addr *a1)
>> +{
>> +   if (a0 == NULL || a1 == NULL)
>> +   return -1;
>> +
>> +   RTE_VERIFY(a0->name != NULL);
>> +   RTE_VERIFY(a1->name != NULL);
>> +
>> +   return strcmp(a0->name, a1->name);
>> +}
>> +
>> +#endif
>> --
>> 2.7.4
>>
>


-- 
-
Shreyansh

[dpdk-dev] [PATCH v7 08/21] eal/soc: implement SoC device list and dump

[Shreyansh Jain]

On Thursday 10 November 2016 08:36 AM, Jianbo Liu wrote:
> On 28 October 2016 at 20:26, Shreyansh Jain  wrote:
>> From: Jan Viktorin 
>>
>> SoC devices would be linked in a separate list (from PCI). This is used for
>> probe function.
>> A helper for dumping the device list is added.
>>
>> Signed-off-by: Jan Viktorin 
>> Signed-off-by: Shreyansh Jain 
>> Signed-off-by: Hemant Agrawal 
>> ---
>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 ++
>>  lib/librte_eal/common/eal_common_soc.c  | 34 
>> +
>>  lib/librte_eal/common/include/rte_soc.h |  9 +++
>>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 ++
>>  4 files changed, 47 insertions(+)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map 
>> b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> index cf6fb8e..86e3cfd 100644
>> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> @@ -171,11 +171,13 @@ DPDK_16.11 {
>> rte_eal_dev_attach;
>> rte_eal_dev_detach;
>> rte_eal_map_resource;
>> +   rte_eal_soc_dump;
>> rte_eal_soc_register;
>> rte_eal_soc_unregister;
>> rte_eal_unmap_resource;
>> rte_eal_vdrv_register;
>> rte_eal_vdrv_unregister;
>> +   soc_device_list;
>> soc_driver_list;
>>
>>  } DPDK_16.07;
>> diff --git a/lib/librte_eal/common/eal_common_soc.c 
>> b/lib/librte_eal/common/eal_common_soc.c
>> index 56135ed..5dcddc5 100644
>> --- a/lib/librte_eal/common/eal_common_soc.c
>> +++ b/lib/librte_eal/common/eal_common_soc.c
>> @@ -31,6 +31,8 @@
>>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>   */
>>
>> +#include 
>> +#include 
>>  #include 
>>
>>  #include 
>> @@ -40,6 +42,38 @@
>>  /* Global SoC driver list */
>>  struct soc_driver_list soc_driver_list =
>> TAILQ_HEAD_INITIALIZER(soc_driver_list);
>> +struct soc_device_list soc_device_list =
>> +   TAILQ_HEAD_INITIALIZER(soc_device_list);
>> +
>> +/* dump one device */
>> +static int
>> +soc_dump_one_device(FILE *f, struct rte_soc_device *dev)
>> +{
>> +   int i;
>> +
>> +   fprintf(f, "%s", dev->addr.name);
>> +   fprintf(f, " - fdt_path: %s\n",
>> +   dev->addr.fdt_path ? dev->addr.fdt_path : "(none)");
>> +
>> +   for (i = 0; dev->id && dev->id[i].compatible; ++i)
>> +   fprintf(f, "   %s\n", dev->id[i].compatible);
>> +
>> +   return 0;
>> +}
>> +
>> +/* dump devices on the bus to an output stream */
>> +void
>> +rte_eal_soc_dump(FILE *f)
>> +{
>> +   struct rte_soc_device *dev = NULL;
>> +
>> +   if (!f)
>> +   return;
>> +
>> +   TAILQ_FOREACH(dev, &soc_device_list, next) {
>> +   soc_dump_one_device(f, dev);
>> +   }
>> +}
>>
>>  /* register a driver */
>>  void
>> diff --git a/lib/librte_eal/common/include/rte_soc.h 
>> b/lib/librte_eal/common/include/rte_soc.h
>> index 23b06a9..347e611 100644
>> --- a/lib/librte_eal/common/include/rte_soc.h
>> +++ b/lib/librte_eal/common/include/rte_soc.h
>> @@ -56,8 +56,12 @@ extern "C" {
>>
>>  extern struct soc_driver_list soc_driver_list;
>>  /**< Global list of SoC Drivers */
>> +extern struct soc_device_list soc_device_list;
>> +/**< Global list of SoC Devices */
>>
>>  TAILQ_HEAD(soc_driver_list, rte_soc_driver); /**< SoC drivers in D-linked 
>> Q. */
>> +TAILQ_HEAD(soc_device_list, rte_soc_device); /**< SoC devices in D-linked 
>> Q. */
>> +
>>
>>  struct rte_soc_id {
>> const char *compatible; /**< OF compatible specification */
>> @@ -142,6 +146,11 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
>>  }
>>
>>  /**
>> + * Dump discovered SoC devices.
>> + */
>> +void rte_eal_soc_dump(FILE *f);
>
> If it is to dump device information (not driver), is it proper to
> rename it rte_eal_soc_device_dump()?

Well, 'SoC'=='device' in this context. Isn't it?
In which case, 'soc_device' is just redundant/reuse of a descriptive word.

Or, on a second thought, 'SoC' represents a bus type here. In which 
case, this name sounds better. This would also warrant 
rte_eal_soc_register->rte_eal_soc_device_register.

But, if I need to keep it in sync with PCI (rte_eal_pci_register), I 
think original naming is apt.
I prefer continuing with existing (rte_eal_soc_dump) for this reason.

>
>> +
>> +/**
>>   * Register a SoC driver.
>>   */
>>  void rte_eal_soc_register(struct rte_soc_driver *driver);
>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map 
>> b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> index ab6b985..0155025 100644
>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> @@ -175,11 +175,13 @@ DPDK_16.11 {
>> rte_eal_dev_attach;
>> rte_eal_dev_detach;
>> rte_eal_map_resource;
>> +   rte_eal_soc_dump;
>> rte_eal_soc_register;
>> rte_eal_soc_unregister;
>> rte_eal_unmap_resource;

[dpdk-dev] [PATCH v7 11/21] eal/soc: implement probing of drivers

[Shreyansh Jain]

On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
> On 28 October 2016 at 20:26, Shreyansh Jain  wrote:
>> Each SoC PMD registers a set of callback for scanning its own bus/infra and
>> matching devices to drivers when probe is called.
>> This patch introduces the infra for calls to SoC scan on rte_eal_soc_init()
>> and match on rte_eal_soc_probe().
>>
>> Patch also adds test case for scan and probe.
>>
>> Signed-off-by: Jan Viktorin 
>> Signed-off-by: Shreyansh Jain 
>> Signed-off-by: Hemant Agrawal 
>> --
>> v4:
>>  - Update test_soc for descriptive test function names
>>  - Comments over test functions
>>  - devinit and devuninint --> probe/remove
>>  - RTE_VERIFY at some places
>> ---
>>  app/test/test_soc.c | 205 
>> ++-
>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
>>  lib/librte_eal/common/eal_common_soc.c  | 213 
>> +++-
>>  lib/librte_eal/common/include/rte_soc.h |  75 -
>>  lib/librte_eal/linuxapp/eal/eal.c   |   5 +
>>  lib/librte_eal/linuxapp/eal/eal_soc.c   |  21 ++-
>>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   4 +
>>  7 files changed, 519 insertions(+), 8 deletions(-)
>>
> ..
>
>>  /**
>> + * SoC device scan callback, called from rte_eal_soc_init.
>> + * For various SoC, the bus on which devices are attached maynot be 
>> compliant
>> + * to a standard platform (or platform bus itself). In which case, extra
>> + * steps are implemented by PMD to scan over the bus and add devices to SoC
>> + * device list.
>> + */
>> +typedef void (soc_scan_t)(void);
>
> I'm still not sure about the purpose of soc_scan, and how to use it.

For each device to be used by DPDK, which cannot be scanned/identified 
using the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' 
provides a way for driver to make those devices part of device lists.

Ideally, 'scan' is not a function of a driver. It is a bus function - 
which is missing in this case.

> If it's for each driver, it should at least struct rte_soc_driver * as
> its parameter.

Its for each driver - assuming that each non-PCI driver which implements 
it knows how to find devices which it can control (for example, special 
area in sysfs, or even platform bus).

> If it's for each bus, why it is in rte_soc_driver?

Short answer - lack of a better place. It should be in dev.h probably 
(rte_device/driver) but it would look out of place (as that represents 
PCI devices also which cannot implement it - all PCI devices are scanned 
in one go irrespective of driver)

> I know you will implement bus driver in the future, but we need to
> make it clear for current simplified implementation.

Current implementation makes only a single assumption - that rather than 
relying on EAL for identifying devices (as being done now), next best 
option in existing framework (driver) should have control of finding 
devices.

This is primarily to make the SoC work parallel to PCI implementation 
without much top-down changes in EAL.

Bus model, improvises it by moving this implementation a little above in 
hierarchy - in rte_bus<-rte_driver<-PMD.

I understand your apprehension - 'driver-scanning-for-devices' is indeed 
not correct real world analogy. It is just a place holder for enabling 
those drivers/PMDs which cannot work in absence of the right model.
And that is still work in progress.

>
>> +
>> +/**
>> + * Custom device<=>driver match callback for SoC
>> + * Unlike PCI, SoC devices don't have a fixed definition of device
>> + * identification. PMDs can implement a specific matching function in which
>> + * driver and device objects are provided to perform custom match.
>> + */
>> +typedef int (soc_match_t)(struct rte_soc_driver *, struct rte_soc_device *);
>> +
>> +/**
>>   * A structure describing a SoC driver.
>>   */
>>  struct rte_soc_driver {
>> @@ -104,6 +120,8 @@ struct rte_soc_driver {
>> struct rte_driver driver;  /**< Inherit core driver. */
>> soc_probe_t *probe;/**< Device probe */
>> soc_remove_t *remove;  /**< Device remove */
>> +   soc_scan_t *scan_fn;   /**< Callback for scanning SoC 
>> bus*/
>> +   soc_match_t *match_fn; /**< Callback to match dev<->drv 
>> */
>> const struct rte_soc_id *id_table; /**< ID table, NULL terminated */
>>  };
>>
>> @@ -146,12 +164,63 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
>>  }
>>
>>  /**
>> + * Default function for matching the Soc driver with device. Each driver can
>> + * either use this function or define their own soc matching function.
>> + * This function relies on the compatible string extracted from sysfs. But,
>> + * a SoC might have different way of identifying its devices. Such SoC can
>> + * override match_fn.
>> + *
>> + * @return
>> + *  0 on success
>> + * -1 when no match found
>> +  */
>> +int
>> +rte_eal_soc_match_compat(struct r

[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

[Björn Töpel]

Lei wrote:
> I'm testing some DPDK sample under VMware. During the testing work, I
> find l3fwd+ ixgbe vf can work ,but  L3fwd + i40evf  can't work. So I
> reported this issue to Bjorn. From my perspective, if can add new
> parameter in l3fwd sample like what have already don?t in  testpmd
> "crc-strip enable" is a better way to resolve this issue.

(Please don't top-post.)

As discussed in the thread, it might be better to just change the
default in l3fwd from .hw_strip_crc = 0 to 1.

I'll be looking into changing igbvf and ixgbevf to match the semantics
of i40evf.


Bj?rn

[dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

Hello David, list,

I need some help and clarification regarding some changes I am doing to 
cleanup the EAL code.

There are some changes which should be done for 
eth_driver/rte_eth_device structures:

1. most obvious, eth_driver should be renamed to rte_eth_driver.
2. eth_driver currently has rte_pci_driver embedded in it
  - there can be ethernet devices which are _not_ PCI
  - in which case, this structure should be removed.
3. Similarly, rte_eth_dev has rte_pci_device which should be replaced 
with rte_device.

This is what the current outline of eth_driver is:

++
| eth_driver |
| +-+|
| | rte_pci_driver  ||
| | +--+||
| | | rte_driver   |||
| | |  name[]  |||
| | |  ... |||
| | +--+||
| |  .probe ||
| |  .remove||
| |  ...||
| +-+|
|  .eth_dev_init |
|  .eth_dev_uninit   |
++

This is what I was thinking:

+-++--+
| rte_pci_driver  ||eth_driver|
| +--+|   _|_struct rte_driver *p |
| | rte_driver   <---/ | .eth_dev_init|
| |  ... ||| .eth_dev_uninit  |
| |  name||+--+
| |||
| +--+|
|  |
+-+

::Impact::
Various drivers use the rte_pci_driver embedded in the eth_driver object 
for device initialization.
  == They assume that rte_pci_driver is directly embedded and hence 
simply dereference.
  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file

With the above change, such drivers would have to access rte_driver and 
then perform container_of to obtain their respective rte_xxx_driver.
  == this would be useful in case there is a non-PCI driver

::Problem::
I am not sure of reason as to why eth_driver embedded rte_pci_driver in 
first place - other than a convenient way to define it before PCI driver 
registration.

As all the existing PMDs are impacted - am I missing something here in 
making the above change?

Probably, similar is the case for rte_eth_dev.

-
Shreyansh

[dpdk-dev] [PATCH v7 11/21] eal/soc: implement probing of drivers

[Jianbo Liu]

On 10 November 2016 at 14:10, Shreyansh Jain  wrote:
> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
>>
>> On 28 October 2016 at 20:26, Shreyansh Jain 
>> wrote:
>>>
>>> Each SoC PMD registers a set of callback for scanning its own bus/infra
>>> and
>>> matching devices to drivers when probe is called.
>>> This patch introduces the infra for calls to SoC scan on
>>> rte_eal_soc_init()
>>> and match on rte_eal_soc_probe().
>>>
>>> Patch also adds test case for scan and probe.
>>>
>>> Signed-off-by: Jan Viktorin 
>>> Signed-off-by: Shreyansh Jain 
>>> Signed-off-by: Hemant Agrawal 
>>> --
>>> v4:
>>>  - Update test_soc for descriptive test function names
>>>  - Comments over test functions
>>>  - devinit and devuninint --> probe/remove
>>>  - RTE_VERIFY at some places
>>> ---
>>>  app/test/test_soc.c | 205
>>> ++-
>>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
>>>  lib/librte_eal/common/eal_common_soc.c  | 213
>>> +++-
>>>  lib/librte_eal/common/include/rte_soc.h |  75 -
>>>  lib/librte_eal/linuxapp/eal/eal.c   |   5 +
>>>  lib/librte_eal/linuxapp/eal/eal_soc.c   |  21 ++-
>>>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   4 +
>>>  7 files changed, 519 insertions(+), 8 deletions(-)
>>>
>> ..
>>
>>>  /**
>>> + * SoC device scan callback, called from rte_eal_soc_init.
>>> + * For various SoC, the bus on which devices are attached maynot be
>>> compliant
>>> + * to a standard platform (or platform bus itself). In which case, extra
>>> + * steps are implemented by PMD to scan over the bus and add devices to
>>> SoC
>>> + * device list.
>>> + */
>>> +typedef void (soc_scan_t)(void);
>>
>>
>> I'm still not sure about the purpose of soc_scan, and how to use it.
>
>
> For each device to be used by DPDK, which cannot be scanned/identified using
> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
> for driver to make those devices part of device lists.
>
> Ideally, 'scan' is not a function of a driver. It is a bus function - which
> is missing in this case.
>
>> If it's for each driver, it should at least struct rte_soc_driver * as
>> its parameter.
>
>
> Its for each driver - assuming that each non-PCI driver which implements it
> knows how to find devices which it can control (for example, special area in
> sysfs, or even platform bus).
>

Considering there are several drivers in a platform bus, each driver
call the scan function, like the rte_eal_soc_scan_platform_bus() you
implemented.
The first will add soc devices to the list, but the remaining calls
are redundant.

The other issue is adding the driver parameter. Do you need extra
information from driver to scan the bus?

>> If it's for each bus, why it is in rte_soc_driver?
>
>
> Short answer - lack of a better place. It should be in dev.h probably
> (rte_device/driver) but it would look out of place (as that represents PCI
> devices also which cannot implement it - all PCI devices are scanned in one
> go irrespective of driver)
>
>> I know you will implement bus driver in the future, but we need to
>> make it clear for current simplified implementation.
>
>
> Current implementation makes only a single assumption - that rather than
> relying on EAL for identifying devices (as being done now), next best option
> in existing framework (driver) should have control of finding devices.
>
> This is primarily to make the SoC work parallel to PCI implementation
> without much top-down changes in EAL.
>
> Bus model, improvises it by moving this implementation a little above in
> hierarchy - in rte_bus<-rte_driver<-PMD.
>
> I understand your apprehension - 'driver-scanning-for-devices' is indeed not
> correct real world analogy. It is just a place holder for enabling those
> drivers/PMDs which cannot work in absence of the right model.
> And that is still work in progress.
>
>
>>
>>> +
>>> +/**
>>> + * Custom device<=>driver match callback for SoC
>>> + * Unlike PCI, SoC devices don't have a fixed definition of device
>>> + * identification. PMDs can implement a specific matching function in
>>> which
>>> + * driver and device objects are provided to perform custom match.
>>> + */
>>> +typedef int (soc_match_t)(struct rte_soc_driver *, struct rte_soc_device
>>> *);
>>> +
>>> +/**
>>>   * A structure describing a SoC driver.
>>>   */
>>>  struct rte_soc_driver {
>>> @@ -104,6 +120,8 @@ struct rte_soc_driver {
>>> struct rte_driver driver;  /**< Inherit core driver. */
>>> soc_probe_t *probe;/**< Device probe */
>>> soc_remove_t *remove;  /**< Device remove */
>>> +   soc_scan_t *scan_fn;   /**< Callback for scanning SoC
>>> bus*/
>>> +   soc_match_t *match_fn; /**< Callback to match
>>> dev<->drv */
>>> const struct rte_soc_id *id_table; /**< ID table, NULL terminated
>>> */
>>>  };
>>>
>>> @@ -146,12 +16

[dpdk-dev] Clarification for eth_driver changes

[Jianbo Liu]

On 10 November 2016 at 15:26, Shreyansh Jain  wrote:
> Hello David, list,
>
> I need some help and clarification regarding some changes I am doing to
> cleanup the EAL code.
>
> There are some changes which should be done for eth_driver/rte_eth_device
> structures:
>
> 1. most obvious, eth_driver should be renamed to rte_eth_driver.
> 2. eth_driver currently has rte_pci_driver embedded in it
>  - there can be ethernet devices which are _not_ PCI
>  - in which case, this structure should be removed.
> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with
> rte_device.
>
> This is what the current outline of eth_driver is:
>
> ++
> | eth_driver |
> | +-+|
> | | rte_pci_driver  ||
> | | +--+||
> | | | rte_driver   |||
> | | |  name[]  |||
> | | |  ... |||
> | | +--+||
> | |  .probe ||
> | |  .remove||
> | |  ...||
> | +-+|
> |  .eth_dev_init |
> |  .eth_dev_uninit   |
> ++
>
> This is what I was thinking:
>
> +-++--+
> | rte_pci_driver  ||eth_driver|
> | +--+|   _|_struct rte_driver *p |
> | | rte_driver   <---/ | .eth_dev_init|
> | |  ... ||| .eth_dev_uninit  |
> | |  name||+--+
> | |||
> | +--+|
> |  |
> +-+
>
> ::Impact::
> Various drivers use the rte_pci_driver embedded in the eth_driver object for
> device initialization.
>  == They assume that rte_pci_driver is directly embedded and hence simply
> dereference.
>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
>
> With the above change, such drivers would have to access rte_driver and then
> perform container_of to obtain their respective rte_xxx_driver.
>  == this would be useful in case there is a non-PCI driver
>
> ::Problem::
> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
> first place - other than a convenient way to define it before PCI driver
> registration.
>
> As all the existing PMDs are impacted - am I missing something here in
> making the above change?
>

How do you know eth_driver->p is pointing to a rte_pci_driver or rte_soc_driver?
Maybe you need to add a type/flag in rte_driver.

> Probably, similar is the case for rte_eth_dev.
>
> -
> Shreyansh

[dpdk-dev] [PATCH] net/qede: fix unknown speed errmsg for 25G link

[Thomas Monjalon]

2016-11-09 18:26, Rasesh Mody:
> From: Harish Patil 
> 
> Fix to use bitmapped values in NVM configuration for speed capability
> advertisement. This issue is specific to 25G NIC since it is capable
> of 25G and 10G speeds.
> 
> Fixes: 64c239b7f8b7 ("net/qede: fix advertising link speed capability")
> 
> Signed-off-by: Harish Patil 

Now that the feature seems well implemented, I think you can add
it to the feature matrix (doc/guides/nics/features/qede.ini).

[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

[Thomas Monjalon]

2016-11-10 07:17, Bj?rn T?pel:
> As discussed in the thread, it might be better to just change the
> default in l3fwd from .hw_strip_crc = 0 to 1.
> 
> I'll be looking into changing igbvf and ixgbevf to match the semantics
> of i40evf.

Just to make it sure, you mean returning an error in the driver when
a configuration cannot be applied, right?

[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

[Björn Töpel]

Thomas wrote:
 > Just to make it sure, you mean returning an error in the driver when
 > a configuration cannot be applied, right?

Yes, as in 1bbcc5d21129 ("i40evf: report error for unsupported CRC
stripping config"), where -EINVAL is returned.


Bj?rn

[dpdk-dev] Clarification for eth_driver changes

[Thomas Monjalon]

2016-11-10 15:51, Jianbo Liu:
> On 10 November 2016 at 15:26, Shreyansh Jain  
> wrote:
> > This is what the current outline of eth_driver is:
> >
> > ++
> > | eth_driver |
> > | +-+|
> > | | rte_pci_driver  ||
> > | | +--+||
> > | | | rte_driver   |||
> > | | |  name[]  |||
> > | | |  ... |||
> > | | +--+||
> > | |  .probe ||
> > | |  .remove||
> > | |  ...||
> > | +-+|
> > |  .eth_dev_init |
> > |  .eth_dev_uninit   |
> > ++
> >
> > This is what I was thinking:
> >
> > +-++--+
> > | rte_pci_driver  ||eth_driver|
> > | +--+|   _|_struct rte_driver *p |
> > | | rte_driver   <---/ | .eth_dev_init|
> > | |  ... ||| .eth_dev_uninit  |
> > | |  name||+--+
> > | |||
> > | +--+|
> > |  |
> > +-+
> >
> > ::Impact::
> > Various drivers use the rte_pci_driver embedded in the eth_driver object for
> > device initialization.
> >  == They assume that rte_pci_driver is directly embedded and hence simply
> > dereference.
> >  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
> >
> > With the above change, such drivers would have to access rte_driver and then
> > perform container_of to obtain their respective rte_xxx_driver.
> >  == this would be useful in case there is a non-PCI driver
> >
> > ::Problem::
> > I am not sure of reason as to why eth_driver embedded rte_pci_driver in
> > first place - other than a convenient way to define it before PCI driver
> > registration.
> >
> > As all the existing PMDs are impacted - am I missing something here in
> > making the above change?
> >
> 
> How do you know eth_driver->p is pointing to a rte_pci_driver or 
> rte_soc_driver?
> Maybe you need to add a type/flag in rte_driver.

Why do you need any bus information at ethdev level?

[dpdk-dev] Clarification for eth_driver changes

[David Marchand]

Hello Shreyansh,

On Thu, Nov 10, 2016 at 8:26 AM, Shreyansh Jain  
wrote:
> I need some help and clarification regarding some changes I am doing to
> cleanup the EAL code.
>
> There are some changes which should be done for eth_driver/rte_eth_device
> structures:
>
> 1. most obvious, eth_driver should be renamed to rte_eth_driver.
> 2. eth_driver currently has rte_pci_driver embedded in it
>  - there can be ethernet devices which are _not_ PCI
>  - in which case, this structure should be removed.

Do we really need to keep a eth_driver ?
As far as I can see, it is only a convenient wrapper for existing pci
drivers, but in the end it is just a pci_driver with ethdev context in
it that could be pushed to each existing driver.

In my initial description
http://dpdk.org/ml/archives/dev/2016-January/031390.html, what I had
in mind was only having a rte_eth_device pointing to a generic
rte_device.
If we need to invoke some generic driver ops from ethdev (I can only
see the ethdev hotplug api, maybe I missed something), then we would
go through rte_eth_device -> rte_device -> rte_driver.
The rte_driver keeps its own bus/private logic in its code, and no
need to expose a type.


> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with
> rte_device.

Yes, that's the main change for me.


Thanks.

-- 
David Marchand

[dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

On Thursday 10 November 2016 01:21 PM, Jianbo Liu wrote:
> On 10 November 2016 at 15:26, Shreyansh Jain  
> wrote:
>> Hello David, list,
>>
>> I need some help and clarification regarding some changes I am doing to
>> cleanup the EAL code.
>>
>> There are some changes which should be done for eth_driver/rte_eth_device
>> structures:
>>
>> 1. most obvious, eth_driver should be renamed to rte_eth_driver.
>> 2. eth_driver currently has rte_pci_driver embedded in it
>>  - there can be ethernet devices which are _not_ PCI
>>  - in which case, this structure should be removed.
>> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with
>> rte_device.
>>
>> This is what the current outline of eth_driver is:
>>
>> ++
>> | eth_driver |
>> | +-+|
>> | | rte_pci_driver  ||
>> | | +--+||
>> | | | rte_driver   |||
>> | | |  name[]  |||
>> | | |  ... |||
>> | | +--+||
>> | |  .probe ||
>> | |  .remove||
>> | |  ...||
>> | +-+|
>> |  .eth_dev_init |
>> |  .eth_dev_uninit   |
>> ++
>>
>> This is what I was thinking:
>>
>> +-++--+
>> | rte_pci_driver  ||eth_driver|
>> | +--+|   _|_struct rte_driver *p |
>> | | rte_driver   <---/ | .eth_dev_init|
>> | |  ... ||| .eth_dev_uninit  |
>> | |  name||+--+
>> | |||
>> | +--+|
>> |  |
>> +-+
>>
>> ::Impact::
>> Various drivers use the rte_pci_driver embedded in the eth_driver object for
>> device initialization.
>>  == They assume that rte_pci_driver is directly embedded and hence simply
>> dereference.
>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
>>
>> With the above change, such drivers would have to access rte_driver and then
>> perform container_of to obtain their respective rte_xxx_driver.
>>  == this would be useful in case there is a non-PCI driver
>>
>> ::Problem::
>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
>> first place - other than a convenient way to define it before PCI driver
>> registration.
>>
>> As all the existing PMDs are impacted - am I missing something here in
>> making the above change?
>>
>
> How do you know eth_driver->p is pointing to a rte_pci_driver or 
> rte_soc_driver?
> Maybe you need to add a type/flag in rte_driver.

My take: PMD implementation would specify this - similar to how it is 
done now. A PCI PMD would perform a container_of(rte_pci_driver,...).
I don't think we need a differentiation here - primarily because 
generic doesn't handle the eth_driver.

>
>> Probably, similar is the case for rte_eth_dev.
>>
>> -
>> Shreyansh
>


-- 
-
Shreyansh

[dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

On Thursday 10 November 2016 01:33 PM, Thomas Monjalon wrote:
> 2016-11-10 15:51, Jianbo Liu:
>> On 10 November 2016 at 15:26, Shreyansh Jain  
>> wrote:
>>> This is what the current outline of eth_driver is:
>>>
>>> ++
>>> | eth_driver |
>>> | +-+|
>>> | | rte_pci_driver  ||
>>> | | +--+||
>>> | | | rte_driver   |||
>>> | | |  name[]  |||
>>> | | |  ... |||
>>> | | +--+||
>>> | |  .probe ||
>>> | |  .remove||
>>> | |  ...||
>>> | +-+|
>>> |  .eth_dev_init |
>>> |  .eth_dev_uninit   |
>>> ++
>>>
>>> This is what I was thinking:
>>>
>>> +-++--+
>>> | rte_pci_driver  ||eth_driver|
>>> | +--+|   _|_struct rte_driver *p |
>>> | | rte_driver   <---/ | .eth_dev_init|
>>> | |  ... ||| .eth_dev_uninit  |
>>> | |  name||+--+
>>> | |||
>>> | +--+|
>>> |  |
>>> +-+
>>>
>>> ::Impact::
>>> Various drivers use the rte_pci_driver embedded in the eth_driver object for
>>> device initialization.
>>>  == They assume that rte_pci_driver is directly embedded and hence simply
>>> dereference.
>>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
>>>
>>> With the above change, such drivers would have to access rte_driver and then
>>> perform container_of to obtain their respective rte_xxx_driver.
>>>  == this would be useful in case there is a non-PCI driver
>>>
>>> ::Problem::
>>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
>>> first place - other than a convenient way to define it before PCI driver
>>> registration.
>>>
>>> As all the existing PMDs are impacted - am I missing something here in
>>> making the above change?
>>>
>>
>> How do you know eth_driver->p is pointing to a rte_pci_driver or 
>> rte_soc_driver?
>> Maybe you need to add a type/flag in rte_driver.
>
> Why do you need any bus information at ethdev level?
>

AFAIK, we don't need it. Above text is not stating anything on that 
grounds either, I think. Isn't it?

-
Shreyansh

[dpdk-dev] Clarification for eth_driver changes

[Thomas Monjalon]

2016-11-10 14:12, Shreyansh Jain:
> On Thursday 10 November 2016 01:33 PM, Thomas Monjalon wrote:
> > 2016-11-10 15:51, Jianbo Liu:
> >> On 10 November 2016 at 15:26, Shreyansh Jain  
> >> wrote:
> >>> This is what the current outline of eth_driver is:
> >>>
> >>> ++
> >>> | eth_driver |
> >>> | +-+|
> >>> | | rte_pci_driver  ||
> >>> | | +--+||
> >>> | | | rte_driver   |||
> >>> | | |  name[]  |||
> >>> | | |  ... |||
> >>> | | +--+||
> >>> | |  .probe ||
> >>> | |  .remove||
> >>> | |  ...||
> >>> | +-+|
> >>> |  .eth_dev_init |
> >>> |  .eth_dev_uninit   |
> >>> ++
> >>>
> >>> This is what I was thinking:
> >>>
> >>> +-++--+
> >>> | rte_pci_driver  ||eth_driver|
> >>> | +--+|   _|_struct rte_driver *p |
> >>> | | rte_driver   <---/ | .eth_dev_init|
> >>> | |  ... ||| .eth_dev_uninit  |
> >>> | |  name||+--+
> >>> | |||
> >>> | +--+|
> >>> |  |
> >>> +-+
> >>>
> >>> ::Impact::
> >>> Various drivers use the rte_pci_driver embedded in the eth_driver object 
> >>> for
> >>> device initialization.
> >>>  == They assume that rte_pci_driver is directly embedded and hence simply
> >>> dereference.
> >>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
> >>>
> >>> With the above change, such drivers would have to access rte_driver and 
> >>> then
> >>> perform container_of to obtain their respective rte_xxx_driver.
> >>>  == this would be useful in case there is a non-PCI driver
> >>>
> >>> ::Problem::
> >>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
> >>> first place - other than a convenient way to define it before PCI driver
> >>> registration.
> >>>
> >>> As all the existing PMDs are impacted - am I missing something here in
> >>> making the above change?
> >>>
> >>
> >> How do you know eth_driver->p is pointing to a rte_pci_driver or 
> >> rte_soc_driver?
> >> Maybe you need to add a type/flag in rte_driver.
> >
> > Why do you need any bus information at ethdev level?
> 
> AFAIK, we don't need it. Above text is not stating anything on that 
> grounds either, I think. Isn't it?

No, I was replying to Jianbo.
Anyway, David made a more interesting comment.

[dpdk-dev] [PATCH v7 11/21] eal/soc: implement probing of drivers

[Shreyansh Jain]

On Thursday 10 November 2016 01:11 PM, Jianbo Liu wrote:
> On 10 November 2016 at 14:10, Shreyansh Jain  
> wrote:
>> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
>>>
>>> On 28 October 2016 at 20:26, Shreyansh Jain 
>>> wrote:

 Each SoC PMD registers a set of callback for scanning its own bus/infra
 and
 matching devices to drivers when probe is called.
 This patch introduces the infra for calls to SoC scan on
 rte_eal_soc_init()
 and match on rte_eal_soc_probe().

 Patch also adds test case for scan and probe.

 Signed-off-by: Jan Viktorin 
 Signed-off-by: Shreyansh Jain 
 Signed-off-by: Hemant Agrawal 
 --
 v4:
  - Update test_soc for descriptive test function names
  - Comments over test functions
  - devinit and devuninint --> probe/remove
  - RTE_VERIFY at some places
 ---
  app/test/test_soc.c | 205
 ++-
  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
  lib/librte_eal/common/eal_common_soc.c  | 213
 +++-
  lib/librte_eal/common/include/rte_soc.h |  75 -
  lib/librte_eal/linuxapp/eal/eal.c   |   5 +
  lib/librte_eal/linuxapp/eal/eal_soc.c   |  21 ++-
  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   4 +
  7 files changed, 519 insertions(+), 8 deletions(-)

>>> ..
>>>
  /**
 + * SoC device scan callback, called from rte_eal_soc_init.
 + * For various SoC, the bus on which devices are attached maynot be
 compliant
 + * to a standard platform (or platform bus itself). In which case, extra
 + * steps are implemented by PMD to scan over the bus and add devices to
 SoC
 + * device list.
 + */
 +typedef void (soc_scan_t)(void);
>>>
>>>
>>> I'm still not sure about the purpose of soc_scan, and how to use it.
>>
>>
>> For each device to be used by DPDK, which cannot be scanned/identified using
>> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
>> for driver to make those devices part of device lists.
>>
>> Ideally, 'scan' is not a function of a driver. It is a bus function - which
>> is missing in this case.
>>
>>> If it's for each driver, it should at least struct rte_soc_driver * as
>>> its parameter.
>>
>>
>> Its for each driver - assuming that each non-PCI driver which implements it
>> knows how to find devices which it can control (for example, special area in
>> sysfs, or even platform bus).
>>
>
> Considering there are several drivers in a platform bus, each driver
> call the scan function, like the rte_eal_soc_scan_platform_bus() you
> implemented.
> The first will add soc devices to the list, but the remaining calls
> are redundant.

Indeed. This is exactly the issue we will face if we try and move this 
scan/match logic to PCI - all devices are identified in one step.

There is a difference in principle here:
A SoC device/driver combination is essentially focused towards a single 
type of bus<->devices. For example, a NXP PMD would implement a scan 
function which would scan for all devices on NXP's bus. This would not 
conflict with another XYZ SoC PMD which scans its specific bus.

There is caveat to this - the platform bus. There can be multiple 
drivers which can serve platform bus compliant devices. First 
PMD->scan() initiated for such a bus/device would leave all other scans 
redundant.

More similar caveats will come if we consider somewhat generic buses. At 
least I couldn't find any interest for such devices in the ML when I 
picked this series (from where Jan left it).

Probably when more common type of PMDs come in, some default scan 
implementation can check for skipping those devices which are already 
added. It would be redundant but harmless.

>
> The other issue is adding the driver parameter. Do you need extra
> information from driver to scan the bus?
>
>>> If it's for each bus, why it is in rte_soc_driver?
>>
>>
>> Short answer - lack of a better place. It should be in dev.h probably
>> (rte_device/driver) but it would look out of place (as that represents PCI
>> devices also which cannot implement it - all PCI devices are scanned in one
>> go irrespective of driver)
>>
>>> I know you will implement bus driver in the future, but we need to
>>> make it clear for current simplified implementation.
>>
>>
>> Current implementation makes only a single assumption - that rather than
>> relying on EAL for identifying devices (as being done now), next best option
>> in existing framework (driver) should have control of finding devices.
>>
>> This is primarily to make the SoC work parallel to PCI implementation
>> without much top-down changes in EAL.
>>
>> Bus model, improvises it by moving this implementation a little above in
>> hierarchy - in rte_bus<-rte_driver<-PMD.
>>
>> I understand your apprehension - 'driver-scanning-for-devices' is indeed not
>> correc

[dpdk-dev] Fwd: mbuf changes

[Alejandro Lucero]

I forgot to include dev at dpdk.org in my response.

My comment at the end o this email.

On Wed, Oct 26, 2016 at 10:28 AM, Alejandro Lucero <
alejandro.lucero at netronome.com> wrote:

>
>
> On Tue, Oct 25, 2016 at 2:05 PM, Bruce Richardson <
> bruce.richardson at intel.com> wrote:
>
>> On Tue, Oct 25, 2016 at 05:24:28PM +0530, Shreyansh Jain wrote:
>> > On Monday 24 October 2016 09:55 PM, Bruce Richardson wrote:
>> > > On Mon, Oct 24, 2016 at 04:11:33PM +, Wiles, Keith wrote:
>> > > >
>> > > > > On Oct 24, 2016, at 10:49 AM, Morten Br?rup <
>> mb at smartsharesystems.com> wrote:
>> > > > >
>> > > > > First of all: Thanks for a great DPDK Userspace 2016!
>> > > > >
>> > > > >
>> > > > >
>> > > > > Continuing the Userspace discussion about Olivier Matz?s proposed
>> mbuf changes...
>> > >
>> > > Thanks for keeping the discussion going!
>> > > > >
>> > > > >
>> > > > >
>> > > > > 1.
>> > > > >
>> > > > > Stephen Hemminger had a noteworthy general comment about keeping
>> metadata for the NIC in the appropriate section of the mbuf: Metadata
>> generated by the NIC?s RX handler belongs in the first cache line, and
>> metadata required by the NIC?s TX handler belongs in the second cache line.
>> This also means that touching the second cache line on ingress should be
>> avoided if possible; and Bruce Richardson mentioned that for this reason
>> m->next was zeroed on free().
>> > > > >
>> > > Thinking about it, I suspect there are more fields we can reset on
>> free
>> > > to save time on alloc. Refcnt, as discussed below is one of them, but
>> so
>> > > too could be the nb_segs field and possibly others.
>> > >
>> > > > >
>> > > > >
>> > > > > 2.
>> > > > >
>> > > > > There seemed to be consensus that the size of m->refcnt should
>> match the size of m->port because a packet could be duplicated on all
>> physical ports for L3 multicast and L2 flooding.
>> > > > >
>> > > > > Furthermore, although a single physical machine (i.e. a single
>> server) with 255 physical ports probably doesn?t exist, it might contain
>> more than 255 virtual machines with a virtual port each, so it makes sense
>> extending these mbuf fields from 8 to 16 bits.
>> > > >
>> > > > I thought we also talked about removing the m->port from the mbuf
>> as it is not really needed.
>> > > >
>> > > Yes, this was mentioned, and also the option of moving the port value
>> to
>> > > the second cacheline, but it appears that NXP are using the port value
>> > > in their NIC drivers for passing in metadata, so we'd need their
>> > > agreement on any move (or removal).
>> >
>> > I am not sure where NXP's NIC came into picture on this, but now that
>> it is
>> > highlighted, this field is required for libevent implementation [1].
>> >
>> > A scheduler sending an event, which can be a packet, would only have
>> > information of a flow_id. From this matching it back to a port, without
>> > mbuf->port, would be very difficult (costly). There may be way around
>> this
>> > but at least in current proposal I think port would be important to
>> have -
>> > even if in second cache line.
>> >
>> > But, off the top of my head, as of now it is not being used for any
>> specific
>> > purpose in NXP's PMD implementation.
>> >
>> > Even the SoC patches don't necessarily rely on it except using it
>> because it
>> > is available.
>> >
>> > @Bruce: where did you get the NXP context here from?
>> >
>> Oh, I'm just mis-remembering. :-( It was someone else who was looking for
>> this - Netronome, perhaps?
>>
>> CC'ing Alejandro in the hope I'm remembering correctly second time
>> round!
>>
>>
> Yes. Thanks Bruce!
>
> So Netronome uses the port field and, as I commented on the user meeting,
> we are happy with the field going from 8 to 16 bits.
>
> In our case, this is something some clients have demanded, and if I'm not
> wrong (I'll double check this asap), the port value is for knowing where
> the packet is coming from. Think about a switch in the NIC, with ports
> linked to VFs/VMs, and one or more physical ports. That port value is not
> related to DPDK ports but to the switch ports. Code in the host (DPDK or
> not) can receive packets from the wire or from VFs through the NIC. This is
> also true for packets received by VMs, but I guess the port value is just
> interested for host code.
>
>
>

I consulted this functionality internally and it seems we do not need this
anymore. In fact, I will remove the metadata port handling soon from our
PMD.



> /Bruce
>>
>
>

[dpdk-dev] Clarification for eth_driver changes

[Jianbo Liu]

Hi Thomas,

On 10 November 2016 at 16:58, Thomas Monjalon  
wrote:
> 2016-11-10 14:12, Shreyansh Jain:
>> On Thursday 10 November 2016 01:33 PM, Thomas Monjalon wrote:
>> > 2016-11-10 15:51, Jianbo Liu:
>> >> On 10 November 2016 at 15:26, Shreyansh Jain  
>> >> wrote:
>> >>> This is what the current outline of eth_driver is:
>> >>>
>> >>> ++
>> >>> | eth_driver |
>> >>> | +-+|
>> >>> | | rte_pci_driver  ||
>> >>> | | +--+||
>> >>> | | | rte_driver   |||
>> >>> | | |  name[]  |||
>> >>> | | |  ... |||
>> >>> | | +--+||
>> >>> | |  .probe ||
>> >>> | |  .remove||
>> >>> | |  ...||
>> >>> | +-+|
>> >>> |  .eth_dev_init |
>> >>> |  .eth_dev_uninit   |
>> >>> ++
>> >>>
>> >>> This is what I was thinking:
>> >>>
>> >>> +-++--+
>> >>> | rte_pci_driver  ||eth_driver|
>> >>> | +--+|   _|_struct rte_driver *p |
>> >>> | | rte_driver   <---/ | .eth_dev_init|
>> >>> | |  ... ||| .eth_dev_uninit  |
>> >>> | |  name||+--+
>> >>> | |||
>> >>> | +--+|
>> >>> |  |
>> >>> +-+
>> >>>
>> >>> ::Impact::
>> >>> Various drivers use the rte_pci_driver embedded in the eth_driver object 
>> >>> for
>> >>> device initialization.
>> >>>  == They assume that rte_pci_driver is directly embedded and hence simply
>> >>> dereference.
>> >>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
>> >>>
>> >>> With the above change, such drivers would have to access rte_driver and 
>> >>> then
>> >>> perform container_of to obtain their respective rte_xxx_driver.
>> >>>  == this would be useful in case there is a non-PCI driver
>> >>>
>> >>> ::Problem::
>> >>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
>> >>> first place - other than a convenient way to define it before PCI driver
>> >>> registration.
>> >>>
>> >>> As all the existing PMDs are impacted - am I missing something here in
>> >>> making the above change?
>> >>>
>> >>
>> >> How do you know eth_driver->p is pointing to a rte_pci_driver or 
>> >> rte_soc_driver?
>> >> Maybe you need to add a type/flag in rte_driver.
>> >
>> > Why do you need any bus information at ethdev level?
>>
>> AFAIK, we don't need it. Above text is not stating anything on that
>> grounds either, I think. Isn't it?
>
> No, I was replying to Jianbo.
> Anyway, David made a more interesting comment.

Indeed, no need as I checked the code.
It's not even a issue if using David's design.

Thanks!
Jianbo

[dpdk-dev] [PATCH v7 11/21] eal/soc: implement probing of drivers

[Thomas Monjalon]

2016-11-10 14:40, Shreyansh Jain:
> On Thursday 10 November 2016 01:11 PM, Jianbo Liu wrote:
> > On 10 November 2016 at 14:10, Shreyansh Jain  
> > wrote:
> >> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
> >>> I'm still not sure about the purpose of soc_scan, and how to use it.
> >>
> >>
> >> For each device to be used by DPDK, which cannot be scanned/identified 
> >> using
> >> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
> >> for driver to make those devices part of device lists.
> >>
> >> Ideally, 'scan' is not a function of a driver. It is a bus function - which
> >> is missing in this case.
> >>
> >>> If it's for each driver, it should at least struct rte_soc_driver * as
> >>> its parameter.
> >>
> >>
> >> Its for each driver - assuming that each non-PCI driver which implements it
> >> knows how to find devices which it can control (for example, special area 
> >> in
> >> sysfs, or even platform bus).
> >>
> >
> > Considering there are several drivers in a platform bus, each driver
> > call the scan function, like the rte_eal_soc_scan_platform_bus() you
> > implemented.
> > The first will add soc devices to the list, but the remaining calls
> > are redundant.
> 
> Indeed. This is exactly the issue we will face if we try and move this 
> scan/match logic to PCI - all devices are identified in one step.
> 
> There is a difference in principle here:
> A SoC device/driver combination is essentially focused towards a single 
> type of bus<->devices. For example, a NXP PMD would implement a scan 
> function which would scan for all devices on NXP's bus. This would not 
> conflict with another XYZ SoC PMD which scans its specific bus.
> 
> There is caveat to this - the platform bus. There can be multiple 
> drivers which can serve platform bus compliant devices. First 
> PMD->scan() initiated for such a bus/device would leave all other scans 
> redundant.
> 
> More similar caveats will come if we consider somewhat generic buses. At 
> least I couldn't find any interest for such devices in the ML when I 
> picked this series (from where Jan left it).
> 
> Probably when more common type of PMDs come in, some default scan 
> implementation can check for skipping those devices which are already 
> added. It would be redundant but harmless.

If several drivers use the same bus, it means the bus is standard enough
to be implemented in EAL. So the scan function of this bus should be
called only once when calling the generic EAL scan function.

[dpdk-dev] [PATCH] doc: postpone ABI changes for Tx prepare

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Wednesday, November 9, 2016 10:31 PM
> To: Kulasek, TomaszX 
> Cc: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] doc: postpone ABI changes for Tx prepare
> 
> The changes for the feature "Tx prepare" should be made in version 17.02.
> 
> Signed-off-by: Thomas Monjalon 

Acked-by: John McNamara 

[dpdk-dev] [PATCH] doc: postpone ABI changes for Tx prepare

[Kulasek, TomaszX]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, November 9, 2016 23:31
> To: Kulasek, TomaszX 
> Cc: dev at dpdk.org
> Subject: [PATCH] doc: postpone ABI changes for Tx prepare
> 
> The changes for the feature "Tx prepare" should be made in version 17.02.
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  doc/guides/rel_notes/deprecation.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 1a9e1ae..ab6014d 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -8,8 +8,8 @@ API and ABI deprecation notices are to be posted here.
>  Deprecation Notices
>  ---
> 
> -* In 16.11 ABI changes are planned: the ``rte_eth_dev`` structure will be
> -  extended with new function pointer ``tx_pkt_prep`` allowing
> verification
> +* In 17.02 ABI changes are planned: the ``rte_eth_dev`` structure will be
> +  extended with new function pointer ``tx_pkt_prepare`` allowing
> verification
>and processing of packet burst to meet HW specific requirements before
>transmit. Also new fields will be added to the ``rte_eth_desc_lim``
> structure:
>``nb_seg_max`` and ``nb_mtu_seg_max`` providing information about
> number of
> --
> 2.7.0

Acked-by: Tomasz Kulasek 

[dpdk-dev] [PATCH v1] doc: announce API and ABI change for librte_ether

[Pattan, Reshma]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bernard Iremonger
> Sent: Tuesday, October 18, 2016 2:38 PM
> To: dev at dpdk.org; Mcnamara, John 
> Cc: Iremonger, Bernard 
> Subject: [dpdk-dev] [PATCH v1] doc: announce API and ABI change for
> librte_ether
> 
> In 17.02 five rte_eth_dev_set_vf_*** functions will be removed from
> librte_ether, renamed and moved to the ixgbe PMD.
> 
> Signed-off-by: Bernard Iremonger 
> ---
>  doc/guides/rel_notes/deprecation.rst | 36
> 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 1d274d8..20e11ac 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -53,3 +53,39 @@ Deprecation Notices
>  * librte_ether: an API change is planned for 17.02 for the function
>``_rte_eth_dev_callback_process``. In 17.02 the function will return an
> ``int``
>instead of ``void`` and a fourth parameter ``void *ret_param`` will be
> added.
> +
> +* librte_ether: for 17.02 it is planned to deprecate the following five
> functions:
> +
> +  ``rte_eth_dev_set_vf_rxmode``
> +
> +  ``rte_eth_dev_set_vf_rx``
> +
> +  ``rte_eth_dev_set_vf_tx``
> +
> +  ``rte_eth_dev_set_vf_vlan_filter``
> +
> +  ``rte_eth_set_vf_rate_limit``
> +
> +  The following fields will be removed from ``struct eth_dev_ops``:
> +
> +  ``eth_set_vf_rx_mode_t``
> +
> +  ``eth_set_vf_rx_t``
> +
> +  ``eth_set_vf_tx_t``
> +
> +  ``eth_set_vf_vlan_filter_t``
> +
> +  ``eth_set_vf_rate_limit_t``
> +
> +  The functions will be renamed to the following, and moved to the ``ixgbe``
> PMD.
> +
> +  ``rte_pmd_ixgbe_set_vf_rxmode``
> +
> +  ``rte_pmd_ixgbe_set_vf_rx``
> +
> +  ``rte_pmd_ixgbe_set_vf_tx``
> +
> +  ``rte_pmd_ixgbe_set_vf_vlan_filter``
> +
> +  ``rte_pmd_ixgbe_set_vf_rate_limit``
> --
> 2.10.1

Acked-by: Reshma Pattan 

[dpdk-dev] [PATCH v1] doc: announce API and ABI change for librte_ether

[Ferruh Yigit]

On 11/4/2016 1:39 PM, Mcnamara, John wrote:
> 
> 
>> -Original Message-
>> From: Iremonger, Bernard
>> Sent: Tuesday, October 18, 2016 2:38 PM
>> To: dev at dpdk.org; Mcnamara, John 
>> Cc: Iremonger, Bernard 
>> Subject: [PATCH v1] doc: announce API and ABI change for librte_ether
>>
>> In 17.02 five rte_eth_dev_set_vf_*** functions will be removed from
>> librte_ether, renamed and moved to the ixgbe PMD.
>>
>> Signed-off-by: Bernard Iremonger 
> 
> Acked-by: John McNamara 

Acked-by: Ferruh Yigit 

[dpdk-dev] Clarification for eth_driver changes

[Stephen Hemminger]

I also think drv_flags should part of device not PCI. Most of the flags
there like link state support are generic. If it isn't changed for this
release will probably have to break ABI to fully support VMBUS

[dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

Hello David,

On Thursday 10 November 2016 01:46 PM, David Marchand wrote:
> Hello Shreyansh,
>
> On Thu, Nov 10, 2016 at 8:26 AM, Shreyansh Jain  
> wrote:
>> I need some help and clarification regarding some changes I am doing to
>> cleanup the EAL code.
>>
>> There are some changes which should be done for eth_driver/rte_eth_device
>> structures:
>>
>> 1. most obvious, eth_driver should be renamed to rte_eth_driver.
>> 2. eth_driver currently has rte_pci_driver embedded in it
>>  - there can be ethernet devices which are _not_ PCI
>>  - in which case, this structure should be removed.
>
> Do we really need to keep a eth_driver ?

No. As you have rightly mentioned below (as well as in your Jan'16 
post), it is a mere convenience.

> As far as I can see, it is only a convenient wrapper for existing pci
> drivers, but in the end it is just a pci_driver with ethdev context in
> it that could be pushed to each existing driver.

Indeed. My problem (or lack of understanding) is that all PMDs rely on 
it and I don't know in what all pattern they have envisioned using this 
model of ethdev->pci_dev, the initialization sequences.
rte_device->init/uninit would settle most of those worries, I think.

>
> In my initial description
> http://dpdk.org/ml/archives/dev/2016-January/031390.html, what I had
> in mind was only having a rte_eth_device pointing to a generic
> rte_device.

Though I had read it (during the rte_device/driver series) but didn't 
remember it while posting this. I agree with your point of doing away 
with eth_driver.

> If we need to invoke some generic driver ops from ethdev (I can only
> see the ethdev hotplug api, maybe I missed something), then we would
> go through rte_eth_device -> rte_device -> rte_driver.

Agree with you.

> The rte_driver keeps its own bus/private logic in its code, and no
> need to expose a type.

Agree.

>
>
>> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with
>> rte_device.
>
> Yes, that's the main change for me.

Indeed. This is a big change. It impacts a lot of EAL code.

>
>
> Thanks.
>

Intent of this email was to know if I am missing something in assuming 
that eth_driver is actually not being used much. I will keep the 
comments from your email in mind while making changes. Thanks.

_
Shreyansh

[dpdk-dev] Clarification for eth_driver changes

[Thomas Monjalon]

Hi Stephen,

2016-11-10 02:51, Stephen Hemminger:
> I also think drv_flags should part of device not PCI. Most of the flags
> there like link state support are generic. If it isn't changed for this
> release will probably have to break ABI to fully support VMBUS

When do you plan to send VMBUS patches?
Could you send a deprecation notice for this change?
Are you aware of the work started by Shreyansh to have a generic bus model?
Could you help in 17.02 timeframe to have a solid bus model?

Thanks

[dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

On Thursday 10 November 2016 04:21 PM, Stephen Hemminger wrote:
> I also think drv_flags should part of device not PCI. Most of the flags
> there like link state support are generic. If it isn't changed for this
> release will probably have to break ABI to fully support VMBUS
>

I didn't get your point.
Currently drv_flags is in rte_pci_driver.
You intend to say that it should be moved to rte_device?

And, all the changes being discussed here are for 17.02.

-
Shreyansh

[dpdk-dev] [PATCH] doc: add more tested platforms and nics and OSes

[Mcnamara, John]

> -Original Message-
> From: Pei, Yulong
> Sent: Thursday, November 10, 2016 10:03 AM
> To: dev at dpdk.org
> Cc: Mcnamara, John ; thomas.monjalon at 6wind.com;
> Pei, Yulong 
> Subject: [PATCH] doc: add more tested platforms and nics and OSes
> 
> Add more tested platforms and nics and OSes to the release notes.
> 
> Signed-off-by: Yulong Pei 

Acked-by: John McNamara 

[dpdk-dev] [PATCH] doc: announce API and ABI changes for librte_eal

[Shreyansh Jain]

Signed-off-by: Shreyansh Jain 
---
 doc/guides/rel_notes/deprecation.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 1a9e1ae..2af2476 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -35,3 +35,13 @@ Deprecation Notices
 * mempool: The functions for single/multi producer/consumer are deprecated
   and will be removed in 17.02.
   It is replaced by ``rte_mempool_generic_get/put`` functions.
+
+* ABI/API changes are planned for 17.02: ``rte_device``, ``rte_driver`` will be
+  impacted because of introduction of a new ``rte_bus`` hierarchy. This would
+  also impact the way devices are identified by EAL. A bus-device-driver model
+  will be introduced providing a hierarchical view of devices.
+
+* ``eth_driver`` is planned to be removed in 17.02. This currently serves as
+  a placeholder for PMDs to register themselves. Changes for ``rte_bus`` will
+  provide a way to handle device initialization currently being done in
+  ``eth_driver``.
-- 
2.7.4

[dpdk-dev] [PATCH] doc: postpone ABI changes for Tx prepare

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Wednesday, November 9, 2016 10:31 PM
> To: Kulasek, TomaszX 
> Cc: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] doc: postpone ABI changes for Tx prepare
> 
> The changes for the feature "Tx prepare" should be made in version 17.02.
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  doc/guides/rel_notes/deprecation.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst 
> b/doc/guides/rel_notes/deprecation.rst
> index 1a9e1ae..ab6014d 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -8,8 +8,8 @@ API and ABI deprecation notices are to be posted here.
>  Deprecation Notices
>  ---
> 
> -* In 16.11 ABI changes are planned: the ``rte_eth_dev`` structure will be
> -  extended with new function pointer ``tx_pkt_prep`` allowing verification
> +* In 17.02 ABI changes are planned: the ``rte_eth_dev`` structure will be
> +  extended with new function pointer ``tx_pkt_prepare`` allowing verification
>and processing of packet burst to meet HW specific requirements before
>transmit. Also new fields will be added to the ``rte_eth_desc_lim`` 
> structure:
>``nb_seg_max`` and ``nb_mtu_seg_max`` providing information about number of
> --

Acked-by: Konstantin Ananyev 

> 2.7.0

[dpdk-dev] [PATCH] i40e: Fix eth_i40e_dev_init sequence on ThunderX

[Satha Rao]

i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable
   results. To solve this include rte memory barriers

Signed-off-by: Satha Rao 
---
 drivers/net/i40e/base/i40e_adminq.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/base/i40e_adminq.c 
b/drivers/net/i40e/base/i40e_adminq.c
index 0d3a83f..1038a95 100644
--- a/drivers/net/i40e/base/i40e_adminq.c
+++ b/drivers/net/i40e/base/i40e_adminq.c
@@ -832,6 +832,7 @@ enum i40e_status_code i40e_asq_send_command(struct i40e_hw 
*hw,
}

val = rd32(hw, hw->aq.asq.head);
+   rte_rmb();
if (val >= hw->aq.num_asq_entries) {
i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE,
   "AQTX: head overrun at %d\n", val);
@@ -929,8 +930,10 @@ enum i40e_status_code i40e_asq_send_command(struct i40e_hw 
*hw,
(hw->aq.asq.next_to_use)++;
if (hw->aq.asq.next_to_use == hw->aq.asq.count)
hw->aq.asq.next_to_use = 0;
-   if (!details->postpone)
+   if (!details->postpone) {
wr32(hw, hw->aq.asq.tail, hw->aq.asq.next_to_use);
+   rte_wmb();
+   }

/* if cmd_details are not defined or async flag is not set,
 * we need to wait for desc write back
-- 
2.7.4

[dpdk-dev] [PATCH] i40e: Fix eth_i40e_dev_init sequence on ThunderX

[Jerin Jacob]

On Thu, Nov 10, 2016 at 04:04:27AM -0800, Satha Rao wrote:
> i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable
>results. To solve this include rte memory barriers
> 
> Signed-off-by: Satha Rao 
> ---
>  drivers/net/i40e/base/i40e_adminq.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/base/i40e_adminq.c 
> b/drivers/net/i40e/base/i40e_adminq.c
> index 0d3a83f..1038a95 100644
> --- a/drivers/net/i40e/base/i40e_adminq.c
> +++ b/drivers/net/i40e/base/i40e_adminq.c
> @@ -832,6 +832,7 @@ enum i40e_status_code i40e_asq_send_command(struct 
> i40e_hw *hw,
>   }
>  
>   val = rd32(hw, hw->aq.asq.head);
> + rte_rmb();

use rte_smp_rmb() variant to avoid performance regression on x86

>   if (val >= hw->aq.num_asq_entries) {
>   i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE,
>  "AQTX: head overrun at %d\n", val);
> @@ -929,8 +930,10 @@ enum i40e_status_code i40e_asq_send_command(struct 
> i40e_hw *hw,
>   (hw->aq.asq.next_to_use)++;
>   if (hw->aq.asq.next_to_use == hw->aq.asq.count)
>   hw->aq.asq.next_to_use = 0;
> - if (!details->postpone)
> + if (!details->postpone) {
>   wr32(hw, hw->aq.asq.tail, hw->aq.asq.next_to_use);
> + rte_wmb();

ditto

> + }
>  
>   /* if cmd_details are not defined or async flag is not set,
>* we need to wait for desc write back
> -- 
> 2.7.4
> 

[dpdk-dev] [PATCH] maintainers: add staging tree for network drivers

[Ferruh Yigit]

Signed-off-by: Ferruh Yigit 
---
CC: Bruce Richardson 
CC: Thomas Monjalon 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 065397b..d6bb8f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -252,6 +252,8 @@ F: examples/l2fwd-crypto/

 Networking Drivers
 --
+M: Ferruh Yigit 
+T: git://dpdk.org/next/dpdk-next-net

 Link bonding
 M: Declan Doherty 
-- 
2.9.3

[dpdk-dev] disable hugepages

[Keren Hochman]

I tried using the following dpdk options:
--no-huge --vdev eth_pcap0 ,rx_pcap=/t1,tx_pcap=/t2
*It's worked but the number of elements is limited, although the machine
has enough free memory. *rte_mempool_create is failed when I'm trying to
allocate more memory. Is there any limitation on the memory beside the
machine?

*Thanks, Keren *

On Wed, Nov 9, 2016 at 4:50 PM, Olivier Matz  wrote:

> Hi Keren,
>
> On 11/09/2016 03:40 PM, Keren Hochman wrote:
> > On Wed, Nov 9, 2016 at 3:40 PM, Christian Ehrhardt <
> > christian.ehrhardt at canonical.com> wrote:
> >
> >>
> >> On Wed, Nov 9, 2016 at 1:55 PM, Keren Hochman <
> >> keren.hochman at lightcyber.com> wrote:
> >>
> >>> how can I create mempool without hugepages?My application is running
> on a
> >>> pcap file so no huge pages is needed ?
> >>>
> >>
> >> Not sure if that is what you really want (Debug use only), but in
> general
> >> no-huge is available as EAL arg
> >>
> >> From http://pktgen.readthedocs.io/en/latest/usage_eal.html :
> >>
> >> EAL options for DEBUG use only:
> >>   --no-huge   : Use malloc instead of hugetlbfs
> >>
> > I need this option only for testing. How can I use rte_mempool_create if
> I
> > use --no-huge?
>
> When using --no-huge, the dpdk libraries (including mempool) allocate
> its memory in standard memory. Just keep in mind the physical addresses
> will be wrong, so this memory cannot be given to hw devices.
>
> Regards,
> Olivier
>

[dpdk-dev] disable hugepages

[Wiles, Keith]

> On Nov 10, 2016, at 6:32 AM, Keren Hochman  
> wrote:
> 
> I tried using the following dpdk options:
> --no-huge --vdev eth_pcap0 ,rx_pcap=/t1,tx_pcap=/t2
> *It's worked but the number of elements is limited, although the machine
> has enough free memory. *rte_mempool_create is failed when I'm trying to
> allocate more memory. Is there any limitation on the memory beside the
> machine?

DPDK will just use the standard linux memory allocator, so no limitation in 
DPDK. Now you could be hitting the limit as a user, need to check your system 
to make sure you can allocate that much memory to a user. Try using the command 
ulimit and see what it reports.

I do not remember exactly how to change limits except with ulimit command. I 
may have modified /etc/security/limits.conf file.

HTH

> 
> *Thanks, Keren *
> 
> On Wed, Nov 9, 2016 at 4:50 PM, Olivier Matz  
> wrote:
> 
>> Hi Keren,
>> 
>> On 11/09/2016 03:40 PM, Keren Hochman wrote:
>>> On Wed, Nov 9, 2016 at 3:40 PM, Christian Ehrhardt <
>>> christian.ehrhardt at canonical.com> wrote:
>>> 
 
 On Wed, Nov 9, 2016 at 1:55 PM, Keren Hochman <
 keren.hochman at lightcyber.com> wrote:
 
> how can I create mempool without hugepages?My application is running
>> on a
> pcap file so no huge pages is needed ?
> 
 
 Not sure if that is what you really want (Debug use only), but in
>> general
 no-huge is available as EAL arg
 
 From http://pktgen.readthedocs.io/en/latest/usage_eal.html :
 
 EAL options for DEBUG use only:
  --no-huge   : Use malloc instead of hugetlbfs
 
>>> I need this option only for testing. How can I use rte_mempool_create if
>> I
>>> use --no-huge?
>> 
>> When using --no-huge, the dpdk libraries (including mempool) allocate
>> its memory in standard memory. Just keep in mind the physical addresses
>> will be wrong, so this memory cannot be given to hw devices.
>> 
>> Regards,
>> Olivier
>> 

Regards,
Keith

[dpdk-dev] disable hugepages

[Olivier Matz]

On 11/10/2016 02:10 PM, Wiles, Keith wrote:
> 
>> On Nov 10, 2016, at 6:32 AM, Keren Hochman  
>> wrote:
>>
>> I tried using the following dpdk options:
>> --no-huge --vdev eth_pcap0 ,rx_pcap=/t1,tx_pcap=/t2
>> *It's worked but the number of elements is limited, although the machine
>> has enough free memory. *rte_mempool_create is failed when I'm trying to
>> allocate more memory. Is there any limitation on the memory beside the
>> machine?
> 
> DPDK will just use the standard linux memory allocator, so no limitation in 
> DPDK. Now you could be hitting the limit as a user, need to check your system 
> to make sure you can allocate that much memory to a user. Try using the 
> command ulimit and see what it reports.
> 
> I do not remember exactly how to change limits except with ulimit command. I 
> may have modified /etc/security/limits.conf file.

I don't think it's a ulimit issue.
Actually, the memory is reserved once at startup. The -m EAL
option allows to specify the amount of memory allocated:

  -m MB   Memory to allocate (see also --socket-mem)

So I guess setting it to an higher value (256?) would do the job.

Regards,
Olivier

[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

[Mori, Naoyuki]

Hi,

Re:
>Subject: Re: [dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping
>   for i40evf
>Message-ID: 
>Thomas wrote:
> > Just to make it sure, you mean returning an error in the driver when
> > a configuration cannot be applied, right?
>
>Yes, as in 1bbcc5d21129 ("i40evf: report error for unsupported CRC
>stripping config"), where -EINVAL is returned.
>
>Bj?rn

On my experience, OvS+DPDK has same issue.
You cannot run OvS on i40evf due to this CRC config mismatch, while OvS on 
ixgbevf works fine.
So, changing on i40evf PMD side would have more benefit, I believe.


[Details below]
ovs-vswitchd.log:
2016-11-10T08:53:31.290Z|00054|netdev_dpdk|WARN|Interface dpdk0 eth_dev setup 
error Invalid argument

because of
i40evf_dev_configure() returns ?EINVAL.
At here
---
i40evf_dev_configure(struct rte_eth_dev *dev)
{

/* For non-DPDK PF drivers, VF has no ability to disable HW
 * CRC strip, and is implicitly enabled by the PF.
 */
if (!conf->rxmode.hw_strip_crc) {
vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
if ((vf->version_major == I40E_VIRTCHNL_VERSION_MAJOR) &&
(vf->version_minor <= I40E_VIRTCHNL_VERSION_MINOR)) {
/* Peer is running non-DPDK PF driver. */
PMD_INIT_LOG(ERR, "VF can't disable HW CRC Strip");
return -EINVAL;
}
}

}
---

ixgbevf is same Intel NIC but implementation is different. It won?t return 
error.

ixgbevf_dev_configure(struct rte_eth_dev *dev)
{

/*
 * VF has no ability to enable/disable HW CRC
 * Keep the persistent behavior the same as Host PF
 */
#ifndef RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC
if (!conf->rxmode.hw_strip_crc) {
PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
conf->rxmode.hw_strip_crc = 1;
}
#else
if (conf->rxmode.hw_strip_crc) {
PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
conf->rxmode.hw_strip_crc = 0;
}
#endif

}

Regards and Thanks,
Mori

[dpdk-dev] [PATCH] maintainers: add staging tree for network drivers

[Thomas Monjalon]

>  Networking Drivers
>  --
> +M: Ferruh Yigit 
> +T: git://dpdk.org/next/dpdk-next-net

Acked-by: Thomas Monjalon 

It will be applied at the beginning of 17.02 cycle to reflect the change.

[dpdk-dev] [PATCH 1/2] net: remove dead driver names

[David Marchand]

Since b1fb53a39d88 ("ethdev: remove some PCI specific handling"),
rte_eth_dev_info_get() relies on dev->data->drv_name to report the driver
name to caller.

Having the pmds set driver_info->driver_name in the pmds is useless,
since ethdev overwrites it right after.
The only thing the pmd must do is:
- for pci drivers, call rte_eth_copy_pci_info() which then sets
  data->drv_name
- for vdev drivers, manually set data->drv_name

At this stage, virtio-user does not properly report a driver name (fixed in
next commit).

Signed-off-by: David Marchand 
---
 drivers/net/af_packet/rte_eth_af_packet.c | 5 +
 drivers/net/nfp/nfp_net.c | 1 -
 drivers/net/null/rte_eth_null.c   | 4 +---
 drivers/net/pcap/rte_eth_pcap.c   | 4 +---
 drivers/net/qede/qede_ethdev.c| 1 -
 drivers/net/ring/rte_eth_ring.c   | 4 +---
 drivers/net/vhost/rte_eth_vhost.c | 3 ---
 drivers/net/virtio/virtio_ethdev.c| 4 
 drivers/net/xenvirt/rte_eth_xenvirt.c | 5 +
 9 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c 
b/drivers/net/af_packet/rte_eth_af_packet.c
index ff45068..a66a657 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -115,8 +115,6 @@ static const char *valid_arguments[] = {
NULL
 };

-static const char *drivername = "AF_PACKET PMD";
-
 static struct rte_eth_link pmd_link = {
.link_speed = ETH_SPEED_NUM_10G,
.link_duplex = ETH_LINK_FULL_DUPLEX,
@@ -280,7 +278,6 @@ eth_dev_info(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
 {
struct pmd_internals *internals = dev->data->dev_private;

-   dev_info->driver_name = drivername;
dev_info->if_index = internals->if_index;
dev_info->max_mac_addrs = 1;
dev_info->max_rx_pktlen = (uint32_t)ETH_FRAME_LEN;
@@ -693,7 +690,7 @@ rte_pmd_init_internals(const char *name,
(*eth_dev)->dev_ops = &ops;
(*eth_dev)->driver = NULL;
(*eth_dev)->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-   (*eth_dev)->data->drv_name = drivername;
+   (*eth_dev)->data->drv_name = "AF_PACKET PMD";
(*eth_dev)->data->kdrv = RTE_KDRV_NONE;
(*eth_dev)->data->numa_node = numa_node;

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c6b1587..0c342ab 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1006,7 +1006,6 @@ nfp_net_infos_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)

hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);

-   dev_info->driver_name = dev->driver->pci_drv.driver.name;
dev_info->max_rx_queues = (uint16_t)hw->max_rx_queues;
dev_info->max_tx_queues = (uint16_t)hw->max_tx_queues;
dev_info->min_rx_bufsize = ETHER_MIN_MTU;
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 836d982..09d77fd 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -88,7 +88,6 @@ struct pmd_internals {


 static struct ether_addr eth_addr = { .addr_bytes = {0} };
-static const char *drivername = "Null PMD";
 static struct rte_eth_link pmd_link = {
.link_speed = ETH_SPEED_NUM_10G,
.link_duplex = ETH_LINK_FULL_DUPLEX,
@@ -295,7 +294,6 @@ eth_dev_info(struct rte_eth_dev *dev,
return;

internals = dev->data->dev_private;
-   dev_info->driver_name = drivername;
dev_info->max_mac_addrs = 1;
dev_info->max_rx_pktlen = (uint32_t)-1;
dev_info->max_rx_queues = RTE_DIM(internals->rx_null_queues);
@@ -555,7 +553,7 @@ eth_dev_null_create(const char *name,
eth_dev->driver = NULL;
data->dev_flags = RTE_ETH_DEV_DETACHABLE;
data->kdrv = RTE_KDRV_NONE;
-   data->drv_name = drivername;
+   data->drv_name = "Null PMD";
data->numa_node = numa_node;

/* finally assign rx and tx ops */
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 0162f44..8b4fba7 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -119,7 +119,6 @@ static struct ether_addr eth_addr = {
.addr_bytes = { 0, 0, 0, 0x1, 0x2, 0x3 }
 };

-static const char *drivername = "Pcap PMD";
 static struct rte_eth_link pmd_link = {
.link_speed = ETH_SPEED_NUM_10G,
.link_duplex = ETH_LINK_FULL_DUPLEX,
@@ -552,7 +551,6 @@ eth_dev_info(struct rte_eth_dev *dev,
 {
struct pmd_internals *internals = dev->data->dev_private;

-   dev_info->driver_name = drivername;
dev_info->if_index = internals->if_index;
dev_info->max_mac_addrs = 1;
dev_info->max_rx_pktlen = (uint32_t) -1;
@@ -842,7 +840,7 @@ pmd_init_internals(const char *name, const unsigned int 
nb_rx_queues,
(*eth_dev)->driver = NULL;
data->dev_flags = RTE_ETH_DEV_DETACHABLE;
data->kdrv = RTE_KDRV_NONE;
- 

[dpdk-dev] [PATCH 2/2] net: align ethdev and eal driver names

[David Marchand]

Some virtual pmds report a different name than the vdev driver name
registered in eal.
While it does not hurt, let's try to be consistent.

Signed-off-by: David Marchand 
---
 drivers/net/af_packet/rte_eth_af_packet.c  |  4 +++-
 drivers/net/bonding/rte_eth_bond_api.c |  7 +++
 drivers/net/bonding/rte_eth_bond_pmd.c |  4 ++--
 drivers/net/bonding/rte_eth_bond_private.h |  2 +-
 drivers/net/mpipe/mpipe_tilegx.c   | 21 -
 drivers/net/null/rte_eth_null.c|  4 +++-
 drivers/net/pcap/rte_eth_pcap.c|  4 +++-
 drivers/net/ring/rte_eth_ring.c|  4 +++-
 drivers/net/vhost/rte_eth_vhost.c  |  4 +++-
 drivers/net/virtio/virtio_ethdev.c |  6 +-
 drivers/net/virtio/virtio_ethdev.h |  2 ++
 drivers/net/virtio/virtio_user_ethdev.c|  2 +-
 drivers/net/xenvirt/rte_eth_xenvirt.c  |  3 ++-
 13 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c 
b/drivers/net/af_packet/rte_eth_af_packet.c
index a66a657..8cae165 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -437,6 +437,8 @@ open_packet_iface(const char *key __rte_unused,
return 0;
 }

+static struct rte_vdev_driver pmd_af_packet_drv;
+
 static int
 rte_pmd_init_internals(const char *name,
const int sockfd,
@@ -690,7 +692,7 @@ rte_pmd_init_internals(const char *name,
(*eth_dev)->dev_ops = &ops;
(*eth_dev)->driver = NULL;
(*eth_dev)->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-   (*eth_dev)->data->drv_name = "AF_PACKET PMD";
+   (*eth_dev)->data->drv_name = pmd_af_packet_drv.driver.name;
(*eth_dev)->data->kdrv = RTE_KDRV_NONE;
(*eth_dev)->data->numa_node = numa_node;

diff --git a/drivers/net/bonding/rte_eth_bond_api.c 
b/drivers/net/bonding/rte_eth_bond_api.c
index 2a3893a..a4e86ae 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 

 #include "rte_eth_bond.h"
 #include "rte_eth_bond_private.h"
@@ -44,8 +45,6 @@

 #define DEFAULT_POLLING_INTERVAL_10_MS (10)

-const char pmd_bond_driver_name[] = "rte_bond_pmd";
-
 int
 check_for_bonded_ethdev(const struct rte_eth_dev *eth_dev)
 {
@@ -54,7 +53,7 @@ check_for_bonded_ethdev(const struct rte_eth_dev *eth_dev)
return -1;

/* return 0 if driver name matches */
-   return eth_dev->data->drv_name != pmd_bond_driver_name;
+   return eth_dev->data->drv_name != pmd_bond_drv.driver.name;
 }

 int
@@ -221,7 +220,7 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t 
socket_id)
RTE_ETH_DEV_DETACHABLE;
eth_dev->driver = NULL;
eth_dev->data->kdrv = RTE_KDRV_NONE;
-   eth_dev->data->drv_name = pmd_bond_driver_name;
+   eth_dev->data->drv_name = pmd_bond_drv.driver.name;
eth_dev->data->numa_node =  socket_id;

rte_spinlock_init(&internals->lock);
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
b/drivers/net/bonding/rte_eth_bond_pmd.c
index a80b6fa..9bfd9f6 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2566,12 +2566,12 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
return 0;
 }

-static struct rte_vdev_driver bond_drv = {
+struct rte_vdev_driver pmd_bond_drv = {
.probe = bond_probe,
.remove = bond_remove,
 };

-RTE_PMD_REGISTER_VDEV(net_bonding, bond_drv);
+RTE_PMD_REGISTER_VDEV(net_bonding, pmd_bond_drv);
 RTE_PMD_REGISTER_ALIAS(net_bonding, eth_bond);

 RTE_PMD_REGISTER_PARAM_STRING(net_bonding,
diff --git a/drivers/net/bonding/rte_eth_bond_private.h 
b/drivers/net/bonding/rte_eth_bond_private.h
index d95d440..5a411e2 100644
--- a/drivers/net/bonding/rte_eth_bond_private.h
+++ b/drivers/net/bonding/rte_eth_bond_private.h
@@ -63,7 +63,7 @@

 extern const char *pmd_bond_init_valid_arguments[];

-extern const char pmd_bond_driver_name[];
+extern struct rte_vdev_driver pmd_bond_drv;

 /** Port Queue Mapping Structure */
 struct bond_rx_queue {
diff --git a/drivers/net/mpipe/mpipe_tilegx.c b/drivers/net/mpipe/mpipe_tilegx.c
index f00..f0ba91e 100644
--- a/drivers/net/mpipe/mpipe_tilegx.c
+++ b/drivers/net/mpipe/mpipe_tilegx.c
@@ -87,7 +87,6 @@ struct mpipe_local {
 static __thread struct mpipe_local mpipe_local;
 static struct mpipe_context mpipe_contexts[GXIO_MPIPE_INSTANCE_MAX];
 static int mpipe_instances;
-static const char *drivername = "MPIPE PMD";

 /* Per queue statistics. */
 struct mpipe_queue_stats {
@@ -1549,7 +1548,7 @@ mpipe_link_mac(const char *ifname, uint8_t *mac)
 }

 static int
-rte_pmd_mpipe_probe(const char *ifname,
+rte_pmd_mpipe_probe_common(struct rte_vdev_driver *drv, const char *ifname,
  const char *params __rte_unused)
 {
gxio_mpipe_context_t *context;
@@ -1606,7 +1605,7 @@ rte_pmd_mpipe_probe(const char *i

[dpdk-dev] [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

[Alejandro Lucero]

From: Bert van Leeuwen 

A device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS queues which
is used inside struct rte_eth_stats. Ideally, DPDK should be built with
RTE_ETHDEV_QUEUE_STAT_CNTRS to the maximum number of queues a device
can support, 65536, as uint16_t is used for keeping those values for
RX and TX. But of course, having such big arrays inside struct rte_eth_stats
is not a good idea.

Current default value is 16, which could likely be changed to 32 or 64
without too much opposition. And maybe it would be a good idea to modify
struct rte_eth_stats for allowing dynamically allocated arrays and maybe
some extra fields for keeping the array sizes.

Signed-off-by: Alejandro Lucero 
---
 lib/librte_ether/rte_ethdev.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..4209ad0 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1343,8 +1343,10 @@ get_xstats_count(uint8_t port_id)
} else
count = 0;
count += RTE_NB_STATS;
-   count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS;
-   count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS;
+   count += RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS) *
+RTE_NB_RXQ_STATS;
+   count += RTE_MIN(dev->data->nb_tx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS) *
+RTE_NB_TXQ_STATS;
return count;
 }

@@ -1358,6 +1360,7 @@ rte_eth_xstats_get_names(uint8_t port_id,
int cnt_expected_entries;
int cnt_driver_entries;
uint32_t idx, id_queue;
+   uint16_t num_q;

cnt_expected_entries = get_xstats_count(port_id);
if (xstats_names == NULL || cnt_expected_entries < 0 ||
@@ -1374,7 +1377,8 @@ rte_eth_xstats_get_names(uint8_t port_id,
"%s", rte_stats_strings[idx].name);
cnt_used_entries++;
}
-   for (id_queue = 0; id_queue < dev->data->nb_rx_queues; id_queue++) {
+   num_q = RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+   for (id_queue = 0; id_queue < num_q; id_queue++) {
for (idx = 0; idx < RTE_NB_RXQ_STATS; idx++) {
snprintf(xstats_names[cnt_used_entries].name,
sizeof(xstats_names[0].name),
@@ -1384,7 +1388,8 @@ rte_eth_xstats_get_names(uint8_t port_id,
}

}
-   for (id_queue = 0; id_queue < dev->data->nb_tx_queues; id_queue++) {
+   num_q = RTE_MIN(dev->data->nb_tx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+   for (id_queue = 0; id_queue < num_q; id_queue++) {
for (idx = 0; idx < RTE_NB_TXQ_STATS; idx++) {
snprintf(xstats_names[cnt_used_entries].name,
sizeof(xstats_names[0].name),
@@ -1420,14 +1425,18 @@ rte_eth_xstats_get(uint8_t port_id, struct 
rte_eth_xstat *xstats,
unsigned count = 0, i, q;
signed xcount = 0;
uint64_t val, *stats_ptr;
+   uint16_t nb_rxqs, nb_txqs;

RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);

dev = &rte_eth_devices[port_id];

+   nb_rxqs = RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+   nb_txqs = RTE_MIN(dev->data->nb_tx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+
/* Return generic statistics */
-   count = RTE_NB_STATS + (dev->data->nb_rx_queues * RTE_NB_RXQ_STATS) +
-   (dev->data->nb_tx_queues * RTE_NB_TXQ_STATS);
+   count = RTE_NB_STATS + (nb_rxqs * RTE_NB_RXQ_STATS) +
+   (nb_txqs * RTE_NB_TXQ_STATS);

/* implemented by the driver */
if (dev->dev_ops->xstats_get != NULL) {
@@ -1458,7 +1467,7 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat 
*xstats,
}

/* per-rxq stats */
-   for (q = 0; q < dev->data->nb_rx_queues; q++) {
+   for (q = 0; q < nb_rxqs; q++) {
for (i = 0; i < RTE_NB_RXQ_STATS; i++) {
stats_ptr = RTE_PTR_ADD(ð_stats,
rte_rxq_stats_strings[i].offset +
@@ -1469,7 +1478,7 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat 
*xstats,
}

/* per-txq stats */
-   for (q = 0; q < dev->data->nb_tx_queues; q++) {
+   for (q = 0; q < nb_txqs; q++) {
for (i = 0; i < RTE_NB_TXQ_STATS; i++) {
stats_ptr = RTE_PTR_ADD(ð_stats,
rte_txq_stats_strings[i].offset +
-- 
1.9.1

[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

[Thomas Monjalon]

2016-11-10 13:50, Mori, Naoyuki:
> Hi,
> 
> >Thomas wrote:
> > > Just to make it sure, you mean returning an error in the driver when
> > > a configuration cannot be applied, right?
> >
> >Yes, as in 1bbcc5d21129 ("i40evf: report error for unsupported CRC
> >stripping config"), where -EINVAL is returned.
> >
> >Bj?rn
> 
> On my experience, OvS+DPDK has same issue.
> You cannot run OvS on i40evf due to this CRC config mismatch, while OvS on 
> ixgbevf works fine.
> So, changing on i40evf PMD side would have more benefit, I believe.

No I think OVS should handle the configuration error.
We could also add a function to query this capability before configuring.

[dpdk-dev] [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

[Thomas Monjalon]

2016-11-10 14:00, Alejandro Lucero:
> From: Bert van Leeuwen 
> 
> A device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS queues which
> is used inside struct rte_eth_stats. Ideally, DPDK should be built with
> RTE_ETHDEV_QUEUE_STAT_CNTRS to the maximum number of queues a device
> can support, 65536, as uint16_t is used for keeping those values for
> RX and TX. But of course, having such big arrays inside struct rte_eth_stats
> is not a good idea.

RTE_ETHDEV_QUEUE_STAT_CNTRS come from a limitation in Intel devices.
They have limited number of registers to store the stats per queue.

> Current default value is 16, which could likely be changed to 32 or 64
> without too much opposition. And maybe it would be a good idea to modify
> struct rte_eth_stats for allowing dynamically allocated arrays and maybe
> some extra fields for keeping the array sizes.

Yes
and? what is your issue exactly? with which device?
Please explain the idea brought by your patch.

[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

[Mori, Naoyuki]

Hi Thomas,

> 2016-11-10 13:50, Mori, Naoyuki:
> > >Thomas wrote:
> > > > Just to make it sure, you mean returning an error in the driver when
> > > > a configuration cannot be applied, right?
> > >
> > >Yes, as in 1bbcc5d21129 ("i40evf: report error for unsupported CRC
> > >stripping config"), where -EINVAL is returned.
> > >
> > >Bj?rn
> >
> > On my experience, OvS+DPDK has same issue.
> > You cannot run OvS on i40evf due to this CRC config mismatch, while OvS on
> ixgbevf works fine.
> > So, changing on i40evf PMD side would have more benefit, I believe.
> 
> No I think OVS should handle the configuration error.
> We could also add a function to query this capability before configuring.

That would be fine, as long as it became a well-known standard.
But at least, hope i40evf and ixgbevf to be consistent in this CRC handling.


Thanks,
Mori

[dpdk-dev] [PATCH 0/2] enables vhost/virtio any layout feature

[Michael S. Tsirkin]

On Mon, Sep 26, 2016 at 02:40:54PM +0800, Yuanhan Liu wrote:
> The feature is actually supported both in virtio PMD and vhost lib.
> We just haven't enabled it yet. This patchset simply enables it.

Any input on handling versioning? Do people prefer to handle it
completely at the backend, or through libvirt?

> ---
> Yuanhan Liu (2):
>   vhost: enable any layout feature
>   net/virtio: enable any layout feature
> 
>  drivers/net/virtio/virtio_ethdev.h | 1 +
>  lib/librte_vhost/vhost.c   | 1 +
>  lib/librte_vhost/vhost.h   | 3 +++
>  3 files changed, 5 insertions(+)
> 
> -- 
> 1.9.0

[dpdk-dev] [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

[Alejandro Lucero]

On Thu, Nov 10, 2016 at 2:42 PM, Thomas Monjalon 
wrote:

> 2016-11-10 14:00, Alejandro Lucero:
> > From: Bert van Leeuwen 
> >
> > A device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS queues which
> > is used inside struct rte_eth_stats. Ideally, DPDK should be built with
> > RTE_ETHDEV_QUEUE_STAT_CNTRS to the maximum number of queues a device
> > can support, 65536, as uint16_t is used for keeping those values for
> > RX and TX. But of course, having such big arrays inside struct
> rte_eth_stats
> > is not a good idea.
>
> RTE_ETHDEV_QUEUE_STAT_CNTRS come from a limitation in Intel devices.
> They have limited number of registers to store the stats per queue.
>
> > Current default value is 16, which could likely be changed to 32 or 64
> > without too much opposition. And maybe it would be a good idea to modify
> > struct rte_eth_stats for allowing dynamically allocated arrays and maybe
> > some extra fields for keeping the array sizes.
>
> Yes
> and? what is your issue exactly? with which device?
> Please explain the idea brought by your patch.
>

Netronome NFP devices support 128 queues and future version will support
1024.

A particular VF, our PMD just supports VFs, could get as much as 128.
Although that is not likely, that could be an option for some client.

Clients want to use a DPDK coming with a distribution, so changing the
RTE_ETHDEV_QUEUE_STAT_CNTRS depending on the present devices is not an
option.

We would be happy if RTE_ETHDEV_QUEUE_STAT_CNTRS could be set to 1024,
covering current and future requirements for our cards, but maybe having
such big arrays inside struct rte_eth_stats is something people do not want
to have.

A solution could be to create such arrays dynamically based on the device
to get the stats from. For example, call to rte_eth_dev_configure could
have ax extra field for allocating a rte_eth_stats struct, which will be
based on nb_rx_q and nb_tx_q params already given to that function.

Maybe the first thing to know is what people think about just incrementing
RTE_ETHDEV_QUEUE_STAT_CNTRS to 1024.

So Thomas, what do you think about this?

[dpdk-dev] [PATCH] doc: announce API and ABI changes for librte_eal

[David Marchand]

On Thu, Nov 10, 2016 at 12:17 PM, Shreyansh Jain  
wrote:
> Signed-off-by: Shreyansh Jain 
> ---
>  doc/guides/rel_notes/deprecation.rst | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst 
> b/doc/guides/rel_notes/deprecation.rst
> index 1a9e1ae..2af2476 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -35,3 +35,13 @@ Deprecation Notices
>  * mempool: The functions for single/multi producer/consumer are deprecated
>and will be removed in 17.02.
>It is replaced by ``rte_mempool_generic_get/put`` functions.
> +
> +* ABI/API changes are planned for 17.02: ``rte_device``, ``rte_driver`` will 
> be
> +  impacted because of introduction of a new ``rte_bus`` hierarchy. This would
> +  also impact the way devices are identified by EAL. A bus-device-driver 
> model
> +  will be introduced providing a hierarchical view of devices.
> +
> +* ``eth_driver`` is planned to be removed in 17.02. This currently serves as
> +  a placeholder for PMDs to register themselves. Changes for ``rte_bus`` will
> +  provide a way to handle device initialization currently being done in
> +  ``eth_driver``.
> --
> 2.7.4
>

Acked-by: David Marchand 


-- 
David Marchand

[dpdk-dev] [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

[Thomas Monjalon]

2016-11-10 15:43, Alejandro Lucero:
> On Thu, Nov 10, 2016 at 2:42 PM, Thomas Monjalon  6wind.com>
> wrote:
> 
> > 2016-11-10 14:00, Alejandro Lucero:
> > > From: Bert van Leeuwen 
> > >
> > > A device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS queues which
> > > is used inside struct rte_eth_stats. Ideally, DPDK should be built with
> > > RTE_ETHDEV_QUEUE_STAT_CNTRS to the maximum number of queues a device
> > > can support, 65536, as uint16_t is used for keeping those values for
> > > RX and TX. But of course, having such big arrays inside struct
> > rte_eth_stats
> > > is not a good idea.
> >
> > RTE_ETHDEV_QUEUE_STAT_CNTRS come from a limitation in Intel devices.
> > They have limited number of registers to store the stats per queue.
> >
> > > Current default value is 16, which could likely be changed to 32 or 64
> > > without too much opposition. And maybe it would be a good idea to modify
> > > struct rte_eth_stats for allowing dynamically allocated arrays and maybe
> > > some extra fields for keeping the array sizes.
> >
> > Yes
> > and? what is your issue exactly? with which device?
> > Please explain the idea brought by your patch.
> >
> 
> Netronome NFP devices support 128 queues and future version will support
> 1024.
> 
> A particular VF, our PMD just supports VFs, could get as much as 128.
> Although that is not likely, that could be an option for some client.
> 
> Clients want to use a DPDK coming with a distribution, so changing the
> RTE_ETHDEV_QUEUE_STAT_CNTRS depending on the present devices is not an
> option.
> 
> We would be happy if RTE_ETHDEV_QUEUE_STAT_CNTRS could be set to 1024,
> covering current and future requirements for our cards, but maybe having
> such big arrays inside struct rte_eth_stats is something people do not want
> to have.
> 
> A solution could be to create such arrays dynamically based on the device
> to get the stats from. For example, call to rte_eth_dev_configure could
> have ax extra field for allocating a rte_eth_stats struct, which will be
> based on nb_rx_q and nb_tx_q params already given to that function.
> 
> Maybe the first thing to know is what people think about just incrementing
> RTE_ETHDEV_QUEUE_STAT_CNTRS to 1024.
> 
> So Thomas, what do you think about this?

I think this patch is doing something else :)

I'm not sure what is better between big arrays and variable size.
I think you must explain these 2 options in another thread,
because I'm not sure you will have enough attention in a thread starting with
"check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS".

[dpdk-dev] [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

[Alejandro Lucero]

On Thu, Nov 10, 2016 at 4:01 PM, Thomas Monjalon 
wrote:

> 2016-11-10 15:43, Alejandro Lucero:
> > On Thu, Nov 10, 2016 at 2:42 PM, Thomas Monjalon <
> thomas.monjalon at 6wind.com>
> > wrote:
> >
> > > 2016-11-10 14:00, Alejandro Lucero:
> > > > From: Bert van Leeuwen 
> > > >
> > > > A device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS queues which
> > > > is used inside struct rte_eth_stats. Ideally, DPDK should be built
> with
> > > > RTE_ETHDEV_QUEUE_STAT_CNTRS to the maximum number of queues a device
> > > > can support, 65536, as uint16_t is used for keeping those values for
> > > > RX and TX. But of course, having such big arrays inside struct
> > > rte_eth_stats
> > > > is not a good idea.
> > >
> > > RTE_ETHDEV_QUEUE_STAT_CNTRS come from a limitation in Intel devices.
> > > They have limited number of registers to store the stats per queue.
> > >
> > > > Current default value is 16, which could likely be changed to 32 or
> 64
> > > > without too much opposition. And maybe it would be a good idea to
> modify
> > > > struct rte_eth_stats for allowing dynamically allocated arrays and
> maybe
> > > > some extra fields for keeping the array sizes.
> > >
> > > Yes
> > > and? what is your issue exactly? with which device?
> > > Please explain the idea brought by your patch.
> > >
> >
> > Netronome NFP devices support 128 queues and future version will support
> > 1024.
> >
> > A particular VF, our PMD just supports VFs, could get as much as 128.
> > Although that is not likely, that could be an option for some client.
> >
> > Clients want to use a DPDK coming with a distribution, so changing the
> > RTE_ETHDEV_QUEUE_STAT_CNTRS depending on the present devices is not an
> > option.
> >
> > We would be happy if RTE_ETHDEV_QUEUE_STAT_CNTRS could be set to 1024,
> > covering current and future requirements for our cards, but maybe having
> > such big arrays inside struct rte_eth_stats is something people do not
> want
> > to have.
> >
> > A solution could be to create such arrays dynamically based on the device
> > to get the stats from. For example, call to rte_eth_dev_configure could
> > have ax extra field for allocating a rte_eth_stats struct, which will be
> > based on nb_rx_q and nb_tx_q params already given to that function.
> >
> > Maybe the first thing to know is what people think about just
> incrementing
> > RTE_ETHDEV_QUEUE_STAT_CNTRS to 1024.
> >
> > So Thomas, what do you think about this?
>
> I think this patch is doing something else :)
>
>
Sure. But the problem the patch solves is pointing to this, IMHO, bigger
issue.


> I'm not sure what is better between big arrays and variable size.
> I think you must explain these 2 options in another thread,
> because I'm not sure you will have enough attention in a thread starting
> with
> "check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS".
>

Agree. I'll do that then.

Thanks

[dpdk-dev] [PATCH] doc: move testpmd guide with other tools

[Mcnamara, John]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, November 8, 2016 10:03 AM
> To: Mcnamara, John 
> Cc: dev at dpdk.org; Christian Ehrhardt 
> Subject: [PATCH] doc: move testpmd guide with other tools
> 
> The guide testpmd_app_ug/ is moved inside the new tools/ guide as a
> section.
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  MAINTAINERS| 2 +-
>  doc/guides/conf.py | 2 +-
>  doc/guides/contributing/documentation.rst  | 1 -
>  doc/guides/index.rst   | 1 -
>  doc/guides/tools/index.rst | 2 +-
>  doc/guides/{testpmd_app_ug => tools/testpmd}/build_app.rst | 0
>  doc/guides/{testpmd_app_ug => tools/testpmd}/index.rst | 4 ++--
>  doc/guides/{testpmd_app_ug => tools/testpmd}/intro.rst | 0
>  doc/guides/{testpmd_app_ug => tools/testpmd}/run_app.rst   | 0
>  doc/guides/{testpmd_app_ug => tools/testpmd}/testpmd_funcs.rst | 0
>  10 files changed, 5 insertions(+), 7 deletions(-)  rename
> doc/guides/{testpmd_app_ug => tools/testpmd}/build_app.rst (100%)  rename
> doc/guides/{testpmd_app_ug => tools/testpmd}/index.rst (96%)  rename
> doc/guides/{testpmd_app_ug => tools/testpmd}/intro.rst (100%)  rename
> doc/guides/{testpmd_app_ug => tools/testpmd}/run_app.rst (100%)  rename
> doc/guides/{testpmd_app_ug => tools/testpmd}/testpmd_funcs.rst (100%)

Hi,

I had a look at the html output before and after this patch and I don't quite 
agree with it. I see that you are trying to clean up and make the documentation 
more consistent but I don't know if this is the right way to do it.

The problem is that TestPMD is a bit of an outlier. It isn't a sample 
application and it isn't really a test application despite the name (it is more 
of a tester application). Also I don't think that it is a tool/utility like the 
other apps in the target directory (if it is seen as a tool then it should be 
renamed to something like dpdk-tester for consistency). Testpmd also has quite 
a lot of documentation, more than any of our other apps or utilities, which 
again makes it an outlier.

So my preference is to leave TestPMD in the high level index.

However, I do think the High level index should be cleaned up a bit and the 
items re-ordered into a more logical sequence. I'll submit a patch for that.

John

[dpdk-dev] [PATCH] pdump: fix log message to display correct error number

[Reshma Pattan]

The ethdev Rx/Tx remove callback apis doesn't set rte_errno during
failures, instead they just return negative error number, so using
that number in logs instead of rte_errno upon Rx and Tx callback
removal failures.

Fixes: 278f9454 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan 
---
 lib/librte_pdump/rte_pdump.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 504a1ce..5968683 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -269,7 +269,7 @@ pdump_regitser_rx_callbacks(uint16_t end_q, uint8_t port, 
uint16_t queue,
if (ret < 0) {
RTE_LOG(ERR, PDUMP,
"failed to remove rx callback, 
errno=%d\n",
-   rte_errno);
+   -ret);
return ret;
}
cbs->cb = NULL;
@@ -324,7 +324,7 @@ pdump_regitser_tx_callbacks(uint16_t end_q, uint8_t port, 
uint16_t queue,
if (ret < 0) {
RTE_LOG(ERR, PDUMP,
"failed to remove tx callback, 
errno=%d\n",
-   rte_errno);
+   -ret);
return ret;
}
cbs->cb = NULL;
-- 
2.7.4

[dpdk-dev] [PATCH] doc: fix l3fwd mode selection from compile to run time

[Reshma Pattan]

The l3fwd application route lookup mode can be selected at run time
but not at compile time. This patch corrects the statement in the doc.

Fixes: d0dff9ba ("doc: sample application user guide")

Signed-off-by: Reshma Pattan 
---
 doc/guides/sample_app_ug/l3_forward.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/sample_app_ug/l3_forward.rst 
b/doc/guides/sample_app_ug/l3_forward.rst
index e2e6223..ab916b9 100644
--- a/doc/guides/sample_app_ug/l3_forward.rst
+++ b/doc/guides/sample_app_ug/l3_forward.rst
@@ -42,7 +42,7 @@ The initialization and run-time paths are very similar to 
those of the :doc:`l2_
 The main difference from the L2 Forwarding sample application is that the 
forwarding decision
 is made based on information read from the input packet.

-The lookup method is either hash-based or LPM-based and is selected at compile 
time. When the selected lookup method is hash-based,
+The lookup method is either hash-based or LPM-based and is selected at run 
time. When the selected lookup method is hash-based,
 a hash object is used to emulate the flow classification stage.
 The hash object is used in correlation with a flow table to map each input 
packet to its flow at runtime.

-- 
2.7.4

[dpdk-dev] [PATCH] doc: fix l3fwd mode selection from compile to run time

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Reshma Pattan
> Sent: Thursday, November 10, 2016 4:30 PM
> To: dev at dpdk.org
> Cc: Pattan, Reshma 
> Subject: [dpdk-dev] [PATCH] doc: fix l3fwd mode selection from compile to
> run time
> 
> The l3fwd application route lookup mode can be selected at run time but
> not at compile time. This patch corrects the statement in the doc.
> 
> Fixes: d0dff9ba ("doc: sample application user guide")
> 
> Signed-off-by: Reshma Pattan 

Acked-by: John McNamara 

[dpdk-dev] [PATCH] pdump: fix log message to display correct error number

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Reshma Pattan
> Sent: Thursday, November 10, 2016 4:30 PM
> To: dev at dpdk.org
> Cc: Reshma Pattan 
> Subject: [dpdk-dev] [PATCH] pdump: fix log message to display correct
> error number
> 
> The ethdev Rx/Tx remove callback apis doesn't set rte_errno during
> failures, instead they just return negative error number, so using that
> number in logs instead of rte_errno upon Rx and Tx callback removal
> failures.
> 
> Fixes: 278f9454 ("pdump: add new library for packet capture")
> 
> Signed-off-by: Reshma Pattan 

Acked-by: John McNamara 

[dpdk-dev] [PATCH] pdump: fix log message to display correct error number

[De Lara Guarch, Pablo]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Reshma Pattan
> Sent: Thursday, November 10, 2016 4:30 PM
> To: dev at dpdk.org
> Cc: Reshma Pattan
> Subject: [dpdk-dev] [PATCH] pdump: fix log message to display correct error
> number
> 
> The ethdev Rx/Tx remove callback apis doesn't set rte_errno during
> failures, instead they just return negative error number, so using
> that number in logs instead of rte_errno upon Rx and Tx callback
> removal failures.
> 
> Fixes: 278f9454 ("pdump: add new library for packet capture")
> 
> Signed-off-by: Reshma Pattan 

Acked-by: Pablo de Lara 

[dpdk-dev] [PATCH] doc: add known issue on QAT PMD into release notes

[Fiona Trahe]

Issue is with the digest appended feature on QAT PMD.
A workaround is also documented.

Signed-off-by: Fiona Trahe 
---
 doc/guides/rel_notes/release_16_11.rst | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_16_11.rst 
b/doc/guides/rel_notes/release_16_11.rst
index 365b5a3..5f925b5 100644
--- a/doc/guides/rel_notes/release_16_11.rst
+++ b/doc/guides/rel_notes/release_16_11.rst
@@ -213,7 +213,18 @@ Known Issues
   Therefore, in order to use L3fwd-power, vector mode should be disabled
   from the config file.

-
+* **Digest address must be supplied for crypto auth operation on QAT PMD.**
+
+  The cryptodev API specifies that if the rte_crypto_sym_op.digest.data field,
+  and by inference the digest.phys_addr field which points to the same 
location,
+  is not set for an auth operation the driver is to understand that the digest
+  result is located immediately following the region over which the digest is
+  computed. The QAT PMD doesn't correctly handle this case and reads and writes
+  to an incorrect location. 
+  
+  Callers can workaround this by always supplying the digest virtual and
+  physical address fields in the rte_crypto_sym_op for an auth operation.
+   
 API Changes
 ---

-- 
2.5.0

[dpdk-dev] [PATCH] doc: add known issue on QAT PMD into release notes

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Fiona Trahe
> Sent: Thursday, November 10, 2016 4:47 PM
> To: dev at dpdk.org
> Cc: De Lara Guarch, Pablo ; Trahe, Fiona
> ; Griffin, John 
> Subject: [dpdk-dev] [PATCH] doc: add known issue on QAT PMD into release
> notes
> 
> Issue is with the digest appended feature on QAT PMD.
> A workaround is also documented.
> 
> Signed-off-by: Fiona Trahe 

Acked-by: John McNamara 

[dpdk-dev] [PATCH] doc: add known issue on QAT PMD into release notes

[Trahe, Fiona]

> -Original Message-
> From: Trahe, Fiona
> Sent: Thursday, November 10, 2016 4:47 PM
> To: dev at dpdk.org
> Cc: De Lara Guarch, Pablo ; Trahe, Fiona
> ; Griffin, John 
> Subject: [PATCH] doc: add known issue on QAT PMD into release notes
> 
> Issue is with the digest appended feature on QAT PMD.
> A workaround is also documented.
> 
> Signed-off-by: Fiona Trahe 
> ---
Self-nack 
Checkpatch white space errors - will send an update shortly

[dpdk-dev] [PATCH] doc: add sub-repositories information

[Ferruh Yigit]

DPDK switched to main and sub-repositories approach, this patch
documents new approach and updates development process according.

Signed-off-by: Ferruh Yigit 
---
 doc/guides/contributing/patches.rst | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 729aea7..bec9bfc 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -20,7 +20,14 @@ The DPDK development process has the following features:
 * There is a mailing list where developers submit patches.
 * There are maintainers for hierarchical components.
 * Patches are reviewed publicly on the mailing list.
-* Successfully reviewed patches are merged to the master branch of the 
repository.
+* Successfully reviewed patches are merged to the repository.
+
+|
+
+* There are main repository ``dpdk`` and sub-repositories ``dpdk-next-*``.
+* A patch should be sent for its target repository. Like net drivers should be 
on top of dpdk-next-net repository.
+* All sub-repositories merged into main repository for -rc1 and -rc2 versions 
of the release.
+* After -rc2 release all patches should target main repository.

 The mailing list for DPDK development is `dev at dpdk.org 
`_.
 Contributors will need to `register for the mailing list 
`_ in order to submit patches.
@@ -33,12 +40,17 @@ Refer to the `Pro Git Book `_ 
for further informat
 Getting the Source Code
 ---

-The source code can be cloned using either of the following::
+The source code can be cloned using either of the following:

-git clone git://dpdk.org/dpdk
+main repository::

+git clone git://dpdk.org/dpdk
 git clone http://dpdk.org/git/dpdk

+sub-repositories (`list `_)::
+
+git clone git://dpdk.org/next/dpdk-next-*
+git clone http://dpdk.org/git/next/dpdk-next-*

 Make your Changes
 -
-- 
2.9.3

[dpdk-dev] [PATCH v2] doc: add known issue on QAT PMD into release notes

[Fiona Trahe]

Issue is with the digest appended feature on QAT PMD.
A workaround is also documented.

Signed-off-by: Fiona Trahe 
Acked-by: John McNamara 
---
v2
 - fixed trailing whitespace checkpatch errors

 doc/guides/rel_notes/release_16_11.rst | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_16_11.rst 
b/doc/guides/rel_notes/release_16_11.rst
index 365b5a3..5f925b5 100644
--- a/doc/guides/rel_notes/release_16_11.rst
+++ b/doc/guides/rel_notes/release_16_11.rst
@@ -213,7 +213,18 @@ Known Issues
   Therefore, in order to use L3fwd-power, vector mode should be disabled
   from the config file.

-
+* **Digest address must be supplied for crypto auth operation on QAT PMD.**
+
+  The cryptodev API specifies that if the rte_crypto_sym_op.digest.data field,
+  and by inference the digest.phys_addr field which points to the same 
location,
+  is not set for an auth operation the driver is to understand that the digest
+  result is located immediately following the region over which the digest is
+  computed. The QAT PMD doesn't correctly handle this case and reads and writes
+  to an incorrect location.
+
+  Callers can workaround this by always supplying the digest virtual and
+  physical address fields in the rte_crypto_sym_op for an auth operation.
+
 API Changes
 ---

-- 
2.5.0

[dpdk-dev] [PATCH v2] doc: add known issue on QAT PMD into release notes

[John Griffin]

On 10/11/16 17:27, Fiona Trahe wrote:
> Issue is with the digest appended feature on QAT PMD.
> A workaround is also documented.
>
> Signed-off-by: Fiona Trahe 
> Acked-by: John McNamara 
> ---
> v2
>   - fixed trailing whitespace checkpatch errors
>
Acked-by: John Griffin 

[dpdk-dev] [PATCH 2/2] net: align ethdev and eal driver names

[Ferruh Yigit]

On 11/10/2016 1:51 PM, David Marchand wrote:
> Some virtual pmds report a different name than the vdev driver name
> registered in eal.
> While it does not hurt, let's try to be consistent.
> 
> Signed-off-by: David Marchand 
> ---

Since you did all the work, instead of second patch what do you think
doing something like [1] (basically adding eth_dev->rte_driver link) and
when done for all vdevs, remove eth_dev->data->drv_name completely?


[1]
diff --git a/drivers/net/null/rte_eth_null.c
b/drivers/net/null/rte_eth_null.c
index e4fd68f..d657133 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -553,9 +553,9 @@ eth_dev_null_create(const char *name,
TAILQ_INIT(ð_dev->link_intr_cbs);

eth_dev->driver = NULL;
+   eth_dev->rte_driver = &pmd_null_drv.driver;
data->dev_flags = RTE_ETH_DEV_DETACHABLE;
data->kdrv = RTE_KDRV_NONE;
-   data->drv_name = pmd_null_drv.driver.name;
data->numa_node = numa_node;

/* finally assign rx and tx ops */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..0527c4a 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -259,6 +259,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
}
eth_dev->pci_dev = pci_dev;
eth_dev->driver = eth_drv;
+   eth_dev->rte_driver = pci_drv->driver.name;
eth_dev->data->rx_mbuf_alloc_failed = 0;

/* init user callbacks */
@@ -1557,7 +1558,7 @@ rte_eth_dev_info_get(uint8_t port_id, struct
rte_eth_dev_info *dev_info)
RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
(*dev->dev_ops->dev_infos_get)(dev, dev_info);
dev_info->pci_dev = dev->pci_dev;
-   dev_info->driver_name = dev->data->drv_name;
+   dev_info->driver_name = dev->rte_driver->name;
dev_info->nb_rx_queues = dev->data->nb_rx_queues;
dev_info->nb_tx_queues = dev->data->nb_tx_queues;
 }
@@ -3214,7 +3215,6 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
struct rte_pci_device *pci_de

eth_dev->data->kdrv = pci_dev->kdrv;
eth_dev->data->numa_node = pci_dev->device.numa_node;
-   eth_dev->data->drv_name = pci_dev->driver->driver.name;
 }

 int
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..63e7931 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1642,6 +1642,7 @@ struct rte_eth_dev {
 */
struct rte_eth_rxtx_callback
*pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
uint8_t attached; /**< Flag indicating the port is attached */
+   struct rte_driver *rte_driver;
 } __rte_cache_aligned;

 struct rte_eth_dev_sriov {

[dpdk-dev] [PATCH] maintainers: claim responsability for xen

[Konrad Rzeszutek Wilk]

On Wed, Nov 9, 2016 at 5:03 PM, Thomas Monjalon
 wrote:
> 2016-11-07 07:38, Jianfeng Tan:
>> As some users are still using xen as the hypervisor, I suggest to
>> continue support for xen in DPDK. And from 16.11, I will be the
>> maintainer of all xen-related files.
>>
>> Signed-off-by: Jianfeng Tan 
>
> Applied
>
> Please Jianfeng, could you start your new role by sending a status
> of Xen dom0 support in DPDK? I think there are some issues in latest releases.
>

Pls also CC me and xen-devel at lists.xenproject.org if possible.

Thanks!

[dpdk-dev] [PATCH] doc: move testpmd guide with other tools

[Wiles, Keith]

> On Nov 10, 2016, at 5:02 PM, Thomas Monjalon  
> wrote:
> 
> 2016-11-10 16:11, Mcnamara, John:
>> I had a look at the html output before and after this patch and I don't 
>> quite agree with it. I see that you are trying to clean up and make the 
>> documentation more consistent but I don't know if this is the right way to 
>> do it.
>> 
>> The problem is that TestPMD is a bit of an outlier. It isn't a sample 
>> application and it isn't really a test application despite the name (it is 
>> more of a tester application). Also I don't think that it is a tool/utility 
>> like the other apps in the target directory (if it is seen as a tool then it 
>> should be renamed to something like dpdk-tester for consistency). Testpmd 
>> also has quite a lot of documentation, more than any of our other apps or 
>> utilities, which again makes it an outlier.
> 
> Yes testpmd is not the same kind of tool as others. It helps for tests,
> debugging and demos.
> 
> About the name, as it is packaged as part of the runtime, I think we should
> find a better name. As you said it should start with "dpdk-" and it should
> contain "net" as it does not test the crypto drivers.
> Something like dpdk-testpmd-net.

To me the name dpdk-testpmd-net is a bit long and does testpmd really just test 
PMDs. I was thinking of the name dpdk-tester is really pretty short and 
descriptive IMO. Adding net or pmd to the name does not really add anything as 
dpdk is kind of networking. Just my $0.04 worth. 

> 
>> So my preference is to leave TestPMD in the high level index.
> 
> OK I understand your opinion.
> 
>> However, I do think the High level index should be cleaned up a bit and the 
>> items re-ordered into a more logical sequence. I'll submit a patch for that.
> 
> OK thanks

Regards,
Keith

[dpdk-dev] [PATCH v3] mempool: Add sanity check when secondary link-in less mempools than primary

[Jean Tourrilhes]

If the mempool ops the caller wants to use is not registered, the
library will segfault in an obscure way when trying to use that
mempool. It's better to catch it early and warn the user.

If the primary and secondary process were build using different build
systems, the list of constructors included by the linker in each
binary might be different. Mempools are registered via constructors, so
the linker magic will directly impact which mempools are registered with
the primary and the secondary.
DPDK currently assumes that the secondary has a superset of the
mempools registered at the primary, and they are in the same order
(same index in primary and secondary). In some build scenario, the
secondary might not initialise any mempools at all.

This would also catch cases where there is a bug in the mempool
registration, or some memory corruptions, but this has not been
observed.

Signed-off-by: Jean Tourrilhes 
---
 lib/librte_mempool/rte_mempool.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index e94e56f..bbb6723 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -1273,6 +1273,29 @@ rte_mempool_lookup(const char *name)
return NULL;
}

+   /* Sanity check : secondary may have initialised less mempools
+* than primary due to linker and constructor magic. Or maybe
+* there is a mempool corruption or bug. In any case, we can't
+* go on, we will segfault in an obscure way.
+* This does not detect the cases where the constructor order
+* is different between primary and secondary or where the
+* index points to the wrong ops. This would require more
+* extensive changes, and is much less likely. Jean II */
+   if (mp->ops_index >= (int32_t) rte_mempool_ops_table.num_ops) {
+   unsigned i;
+   /* Dump list of mempool ops for further investigation.
+* In most cases, list is empty... */
+   for (i = 0; i < rte_mempool_ops_table.num_ops; i++) {
+   RTE_LOG(ERR, EAL, "Registered mempool[%d] is %s\n",
+   i, rte_mempool_ops_table.ops[i].name);
+   }
+   /* Do not dump mempool list itself, it will segfault. */
+   rte_panic("Cannot find ops for mempool, ops_index %d, "
+ "num_ops %d - maybe due to build process or "
+ "linker configuration\n",
+ mp->ops_index, rte_mempool_ops_table.num_ops);
+   }
+
return mp;
 }