[dpdk-dev] [PATCH] cfgfile: support looking up sections by index

2016-01-16 Thread Rich Lane
This is useful when sections have duplicate names.

Signed-off-by: Rich Lane 
---
 lib/librte_cfgfile/rte_cfgfile.c | 16 
 lib/librte_cfgfile/rte_cfgfile.h | 23 +++
 2 files changed, 39 insertions(+)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index a677dad..0bb40a4 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -333,6 +333,22 @@ rte_cfgfile_section_entries(struct rte_cfgfile *cfg, const 
char *sectionname,
return i;
 }

+int
+rte_cfgfile_section_entries_by_index(struct rte_cfgfile *cfg, int index,
+   struct rte_cfgfile_entry *entries, int max_entries)
+{
+   int i;
+   const struct rte_cfgfile_section *sect;
+
+   if (index >= cfg->num_sections)
+   return -1;
+
+   sect = cfg->sections[index];
+   for (i = 0; i < max_entries && i < sect->num_entries; i++)
+   entries[i] = *sect->entries[i];
+   return i;
+}
+
 const char *
 rte_cfgfile_get_entry(struct rte_cfgfile *cfg, const char *sectionname,
const char *entryname)
diff --git a/lib/librte_cfgfile/rte_cfgfile.h b/lib/librte_cfgfile/rte_cfgfile.h
index d443782..8e69971 100644
--- a/lib/librte_cfgfile/rte_cfgfile.h
+++ b/lib/librte_cfgfile/rte_cfgfile.h
@@ -155,6 +155,29 @@ int rte_cfgfile_section_entries(struct rte_cfgfile *cfg,
struct rte_cfgfile_entry *entries,
int max_entries);

+/** Get section entries as key-value pairs
+*
+* The index of a section is the same as the index of its name in the
+* result of rte_cfgfile_sections. This API can be used when there are
+* multiple sections with the same name.
+*
+* @param cfg
+*   Config file
+* @param index
+*   Section index
+* @param entries
+*   Pre-allocated array of at least max_entries entries where the section
+*   entries are stored as key-value pair after successful invocation
+* @param max_entries
+*   Maximum number of section entries to be stored in entries array
+* @return
+*   Number of entries populated on success, negative error code otherwise
+*/
+int rte_cfgfile_section_entries_by_index(struct rte_cfgfile *cfg,
+   int index,
+   struct rte_cfgfile_entry *entries,
+   int max_entries);
+
 /** Get value of the named entry in named config file section
 *
 * @param cfg
-- 
1.9.1



[dpdk-dev] Proposal for a big eal / ethdev cleanup

2016-01-16 Thread David Marchand
Hello Jan,

On Thu, Jan 14, 2016 at 12:46 PM, Jan Viktorin  
wrote:
> On Thu, 14 Jan 2016 11:38:16 +0100
> David Marchand  wrote:
>
>> Hello all,
>>
>> Here is a proposal of a big cleanup in ethdev (cryptodev would have to
>> follow) and eal structures.
>> This is something I wanted to do for quite some time and the arrival of
>> a new bus makes me think we need it.
>>
>> This is an alternative to what Jan proposed [1].
>
> As I need to extend DPDK by a non-PCI bus system, I would prefer any such
> working solution :). By [1], you probably mean:
>
> [1] http://comments.gmane.org/gmane.comp.networking.dpdk.devel/30973
>
> (I didn't find it in the e-mail.)

Thought I put the reference after my signature.
Anyway, yes, I was referring to your thread.


>> ABI is most likely broken with this, but I think this discussion can come 
>> later.
>
> I was trying in my initial approach to stay as much API and ABI backwards
> compatible as possible to be acceptable into upstream. As just a few
> people have shown their interest in these changes, I consider the ABI
> compatibility very important.
>
> I can see, that your approach does not care too much... Otherwise, it looks
> good to me. It is closer to the Linux drivers infra, so it seems to be clearer
> then the current one.

I did this on purpose.
>From my point of view, we will have an API/ABI breakage in this code
at one point.
So I sent this mail to show where I'd like us to go, because this
looks saner on the mid/long term.


>> First some context on how dpdk is initialized at the moment :
>>
>> Let's imagine a system with one ixgbe pci device and take some
>> part of ixgbe driver as an example.
>>
>> static struct eth_driver rte_ixgbe_pmd = {
>> .pci_drv = {
>> .name = "rte_ixgbe_pmd",
>> .id_table = pci_id_ixgbe_map,
>> .drv_flags = RTE_PCI_DRV_NEED_MAPPING |
>> RTE_PCI_DRV_INTR_LSC | RTE_PCI_DRV_DETACHABLE,
>> },
>> .eth_dev_init = eth_ixgbe_dev_init,
>> .eth_dev_uninit = eth_ixgbe_dev_uninit,
>> .dev_private_size = sizeof(struct ixgbe_adapter),
>> };
>
> Note, that the biggest issue here is that the eth_driver has no way to
> distinguish among PCI or other subsystem. There is no field that helps
> the generic ethdev code (librte_ether) to decide what bus we are on
> (and it needs to know it in the current DPDK).
>
> Another point is that the ethdev infra should not know about the
> underlying bus infra. The question is whether we do a big clean
> up (extract PCI-bus code out) or a small clean up (give the ethdev
> infra a hint which bus system it deals with).

Yes, and I think these two choices are reflected by our two respective
proposals :-)


>> So now, what I had in mind is something like this.
>> It is far from perfect and I certainly did some shortcuts in my
>> reasoning.
>>
>>
>> Generic device/driver
>>
>> - introduce a rte_device structure,
>> - a rte_device has a name, that identifies it in a unique way across
>> all buses, maybe something like pci::00:01.0, and for vdev,
>> vdev:name
>
> Having a prefix is good but would this break the user API? Is the
> name exposed to users?

Maybe define new apis, and wrap the old one in it ?
Not too sure, I need to experiment.


>> - current rte_driver does not need to know about the pmd_type
>> (pdev/vdev), this is only a way to order drivers init in eal, we could
>> use the rte_driver names to order them or maybe remove this ordering
>
> What is the main reason to have pdev/vdev distinction here? After I
> register the driver, does eal really need to know the pmd_type?

Already did some cleanup in the past because of pmd_type :
http://dpdk.org/browse/dpdk/commit/?id=78aecefed955917753bfb6f44ae970dde4c652d0
http://dpdk.org/browse/dpdk/commit/?id=6bc2415c3387ae72f2ce3677f0e3540e734583d5

For me, there is no reason but the init ordering (vdevs come before
pdevs), and I am pretty sure we don't need this for ethdev.


> Moreover, there is no way how to pass arguments to pdevs, only to
> vdevs. This was shortly disscussed in [2] for the szedata2 PMD.
>
> [2] http://dpdk.org/ml/archives/dev/2015-October/026285.html

Shorty discussed with Thomas, yes.
But after some experiments, it appears that you can pass devargs after
a whitelist option at init (--pci-whitelist
:xx:xx.x,whataniceoptionwehavehere=1).
This does not work for hotplug.
This is undocumented and this looks like a workaround, so I would
prefer we come up with a better api for 2.3.


>> Impact on PCI device/driver
>>
>> - rte_pci_device is modified to embed a rte_device (embedding makes it
>> possible later to cast the rte_device and get the rte_pci_device in pci
>> specific functions)
>> - no need for a rte_pci_driver reference in rte_pci_device, since we
>> have the rte_device driver
>
> I've noticed that DPDK does not use any kind of the container_of macro
> like the Linux Kernel does. Instead, some considerations like the
> rte_pci_driver MUST be placed at 

[dpdk-dev] [PATCH 09/11] doc: refresh headers list

2016-01-16 Thread David Marchand
Hello John,

On Tue, Jan 12, 2016 at 3:06 PM, Mcnamara, John  
wrote:
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of David Marchand
>> Sent: Sunday, January 10, 2016 12:51 PM
>> To: dev at dpdk.org
>> Cc: thomas.monjalon at dpdk.org
>> Subject: [dpdk-dev] [PATCH 09/11] doc: refresh headers list
>>
>> Since we are going to remove a header in next commit, let's first refresh
>> documentation.
>
> I don't like these parts of the docs that list files since they
> go out of date quite easily and, in general, the same information
> can be conveyed by just listing the directories. (That isn't
> future-proof either but it should be less subject to change.)

Well, we could imagine something automatic (in the build process), but
I agree that the quickest solution is to get rid of it.

>
> In this case you could just remove everything in the console section
> after the output from "ls x86_64-native-linuxapp-gcc" like this:
>
>
> Each build directory contains include files, libraries, and applications like 
> the following::
>
> $ ls
> app   tools
> configMAINTAINERS
> Makefile  GNUmakefile
> drivers   mk
> examples  pkg
> doc   README
> lib   scripts
> LICENSE.GPL   LICENSE.LGPL
> i686-native-linuxapp-gcc  x86_64-native-linuxapp-gcc
> i686-native-linuxapp-icc  x86_64-native-linuxapp-icc
>
> $ ls x86_64-native-linuxapp-gcc
> app  build  include  kmod  lib  Makefile
>

Well, from my pov, it is the same issue here.
How about just removing all those files listings ?
I am not sure they really help.

If we go with this, I will send this patch out of the series since it
would not really belong to it.


Regards,
-- 
David Marchand


[dpdk-dev] [PATCH 00/11] kill global pci device id list

2016-01-16 Thread David Marchand
Hello Helin,

On Sun, Jan 10, 2016 at 4:53 PM, Zhang, Helin  wrote:
> Thanks for your huge contribution!
> May you help to describe more details of why you made these huge changes?

As far as I can see, the only reason why we have a centralised header
maintained in eal with all pci device ids is the need to identify pci
devices that require a special treatment before starting a dpdk application
(here, bind those pci devices to igb_uio / vfio).

This patchset splits this header into small pieces maintained by the drivers
themselves, then tries to come up with a way to retrieve those pci device ids
from the final dpdk application and from the drivers compiled as shared
libraries.

With this, supported pci device ids are maintained by the drivers themselves
rather than eal, pci devices ids requiring uio/vfio binding are still available.


Regards,
-- 
David Marchand


[dpdk-dev] [PATCH v3 8/8] virtio: move VIRTIO_READ/WRITE_REG_X into virtio_pci.c

2016-01-16 Thread Santosh Shukla
On Thu, Jan 14, 2016 at 1:12 PM, Yuanhan Liu
 wrote:
> virtio_pci.c become the only file references those macros; move them there.
>

My patch VFIO series need virtio_rd/wr So keeping these api in
virtio_pci.h make more sense to me.


[dpdk-dev] [PATCH v1] doc: fix navigation levels in html sidebar

2016-01-16 Thread Mcnamara, John


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, January 15, 2016 10:01 PM
> To: Mcnamara, John
> Cc: dev at dpdk.org
> Subject: Re: [PATCH v1] doc: fix navigation levels in html sidebar
> 
> 2016-01-15 16:35, John McNamara:
> > Fix issue where the navigation levels weren't displayed in the html
> > sidebar with the new read_the_docs theme.
> >
> > This was due to the :titlesonly: directive in the high level index.rst
> > and also due to a stray newline in the parsed version number.
> 
> I'm interested to know the relation between titles and version number.

Hi Thomas,

There isn't any direct relationship I think.

Removing the :titlesonly: directive made the sublevels visible, but only to the 
first level. Removing the stray newline in the version number made the other 
levels visible. This may be a bug in the Sphinx RTD theme or just some strange 
CSS interaction.

Thanks for rebuilding the online docs.

John.








[dpdk-dev] [PATCH v4 07/14] virtio: vfio: add api support to rd/wr ioport bar

2016-01-16 Thread Santosh Shukla
On Fri, Jan 15, 2016 at 11:33 AM, Yuanhan Liu
 wrote:
> On Thu, Jan 14, 2016 at 06:58:30PM +0530, Santosh Shukla wrote:
>> For vfio case - Use pread/pwrite api to access virtio
>> ioport space.
>>
>> Signed-off-by: Santosh Shukla 
>> Signed-off-by: Rizwan Ansari 
>> Signed-off-by: Rakesh Krishnamurthy 
>> ---
> ...
>> +/* vfio rd/rw virtio apis */
>> +static inline void ioport_inb(const struct rte_pci_device *pci_dev,
>> +   uint8_t reg, uint8_t *val)
>
> Minor nit: dpdk perfers to seperate return type and function name in
> different line:
>
>   static inline void
>   ioport_inb()
>   {
>

ok.
>> +{
>> + if (rte_eal_pci_read_bar(pci_dev, (uint8_t *)val, sizeof(uint8_t), reg,
>   ^^^
>
> Unnecessary cast; and few more belows.
>

yes,
> --yliu


[dpdk-dev] [PATCH v4 06/14] eal: pci: vfio: add rd/wr func for pci bar space

2016-01-16 Thread Santosh Shukla
On Fri, Jan 15, 2016 at 11:18 AM, Yuanhan Liu
 wrote:
> On Thu, Jan 14, 2016 at 06:58:29PM +0530, Santosh Shukla wrote:
> ...
>> +int rte_eal_pci_read_bar(const struct rte_pci_device *device,
>> +  void *buf, size_t len, off_t offset,
>> +  int bar_idx)
>> +
>> +{
>> +#ifdef VFIO_PRESENT
>> + const struct rte_intr_handle *intr_handle = &device->intr_handle;
>> + return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
>> +#else
>> + /* UIO's not applicable */
>> + RTE_SET_USED(device);
>> + RTE_SET_USED(buf);
>> + RTE_SET_USED(len);
>> + RTE_SET_USED(offset);
>> + RTE_SET_USED(bar_idx);
>> + return 0;
>> +#endif
>
> Maybe you could do that like what pci_map_device() does:
>
> switch(dev->kdrv) {
> case RTE_KDRV_NIC_VFIO:
> return pci_vfio_read_bar(..);
> break;
> default:
> RTE_LOG( not supported by driver: %d\n"..);
> break;
> }
>
> With that, you could get rid of those ugly RTE_SET_USED
>

Even better, Thanks!
> --yliu


[dpdk-dev] [PATCH v4 01/14] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

2016-01-16 Thread Santosh Shukla
On Fri, Jan 15, 2016 at 12:21 PM, Yuanhan Liu
 wrote:
> On Thu, Jan 14, 2016 at 06:58:24PM +0530, Santosh Shukla wrote:
>> virtio_recv_pkts_vec and other virtio vector friend apis are written for 
>> sse/avx
>> instructions. For arm64 in particular, virtio vector implementation does not
>> exist(todo).
>>
>> So virtio pmd driver wont build for targets like i686, arm64.  By making
>> RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will 
>> work
>> in non-vectored virtio mode.
>
> While revisiting this patch, I'm thinking you may squash both patch 2
> and patch 11 into this one.
>

Make sense!, we'll do in v5.

>>
>> Signed-off-by: Santosh Shukla 
>> ---
>>  config/common_linuxapp   |1 +
>>  drivers/net/virtio/Makefile  |2 +-
>>  drivers/net/virtio/virtio_rxtx.c |7 +++
>>  3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/config/common_linuxapp b/config/common_linuxapp
>> index 74bc515..8677697 100644
>> --- a/config/common_linuxapp
>> +++ b/config/common_linuxapp
>> @@ -274,6 +274,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
>> +CONFIG_RTE_VIRTIO_INC_VECTOR=y
>>
>>  #
>>  # Compile burst-oriented VMXNET3 PMD driver
>> diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
>> index 43835ba..25a842d 100644
>> --- a/drivers/net/virtio/Makefile
>> +++ b/drivers/net/virtio/Makefile
>> @@ -50,7 +50,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtqueue.c
>>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_pci.c
>>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx.c
>>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_ethdev.c
>> -SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
>> +SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c
>>
>>  # this lib depends upon:
>>  DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal lib/librte_ether
>> diff --git a/drivers/net/virtio/virtio_rxtx.c 
>> b/drivers/net/virtio/virtio_rxtx.c
>> index 74b39ef..23be1ff 100644
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -438,7 +438,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>
>>   dev->data->rx_queues[queue_idx] = vq;
>>
>> +#ifdef RTE_VIRTIO_INC_VECTOR
>>   virtio_rxq_vec_setup(vq);
>> +#endif
>
> You should put such macros for the declaration in virtio_rxtx.h as well.
>
>
> And note that you may miss one:
>
>
> 325 /**
> 326 * Enqueue allocated buffers*
> 327 ***/
> 328 if (use_simple_rxtx)
> ==> 329 error = 
> virtqueue_enqueue_recv_refill_simple(vq, m);
> 330 else
> 331 error = 
> virtqueue_enqueue_recv_refill(vq, m);
> 332 if (error) {
> 333 rte_pktmbuf_free(m);
> 334 break;
> 335 }
>
>
> virtqueue_enqueue_recv_refill_simple() is defined inside virtio_rxtx_simple.c,
> which is built only when CONFIG_RTE_VIRTIO_INC_VECTOR is set. But I see no
> such check here.
>
> Note that this will not break the build, as gcc just ignores it, for 
> use_simple_rxtx
> is 0 by default, thus the "if" part code is not compiled at all. Even for 
> that,
> I think it's better to put an explicit macro check here.
>

we'll make changes in v5, Thanks for review comment.

> --yliu


[dpdk-dev] UX Bug in Sphinx HTML Layout for Programming Guide (and maybe other guides?)

2016-01-16 Thread Thomas Monjalon
2016-01-15 16:23, Mcnamara, John:
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Matthew Hall
> > Sent: Wednesday, January 13, 2016 5:26 PM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] UX Bug in Sphinx HTML Layout for Programming Guide
> > (and maybe other guides?)
> > 
> > When you go to this link:
> > 
> > http://dpdk.org/doc/guides/prog_guide/perf_opt_guidelines.html
> > 
> > There is a bug in the Sphinx layout, where the subchapters of a chapter
> > are invisible even after the chapter is clicked.
> 
> Hi Thomas,
> 
> I found the issue causing this and I'll submit a patch shortly.
> 
> If possible could you apply it a copy of the 2.2 code and rebuild the online 
> docs.

Yes, done.
Thanks guys


[dpdk-dev] [PATCH v1] doc: fix navigation levels in html sidebar

2016-01-16 Thread Thomas Monjalon
2016-01-15 16:35, John McNamara:
> Fix issue where the navigation levels weren't displayed in the
> html sidebar with the new read_the_docs theme.
> 
> This was due to the :titlesonly: directive in the high level
> index.rst and also due to a stray newline in the parsed version
> number.
> 
> Signed-off-by: John McNamara 

Applied, thanks