[dpdk-dev] [PATCH v5 5/6] vhost: batch update used ring
On Thu, Sep 15, 2016 at 06:38:06PM +0200, Maxime Coquelin wrote:
> >>>+static inline void __attribute__((always_inline))
> >>>+flush_used_ring(struct virtio_net *dev, struct vhost_virtqueue *vq,
> >>>+ uint32_t used_idx_start)
> >>>+{
> >>>+ if (used_idx_start + vq->shadow_used_idx < vq->size) {
> >>>+ rte_memcpy(&vq->used->ring[used_idx_start],
> >>>+ &vq->shadow_used_ring[0],
> >>>+ vq->shadow_used_idx *
> >>>+ sizeof(struct vring_used_elem));
> >>>+ vhost_log_used_vring(dev, vq,
> >>>+ offsetof(struct vring_used,
> >>>+ ring[used_idx_start]),
> >>>+ vq->shadow_used_idx *
> >>>+ sizeof(struct vring_used_elem));
> >>>+ } else {
> >>>+ uint32_t part_1 = vq->size - used_idx_start;
> >>>+ uint32_t part_2 = vq->shadow_used_idx - part_1;
> >>>+
> >>>+ rte_memcpy(&vq->used->ring[used_idx_start],
> >>>+ &vq->shadow_used_ring[0],
> >>>+ part_1 *
> >>>+ sizeof(struct vring_used_elem));
> >>>+ vhost_log_used_vring(dev, vq,
> >>>+ offsetof(struct vring_used,
> >>>+ ring[used_idx_start]),
> >>>+ part_1 *
> >>>+ sizeof(struct vring_used_elem));
> >>>+ rte_memcpy(&vq->used->ring[0],
> >>>+ &vq->shadow_used_ring[part_1],
> >>>+ part_2 *
> >>>+ sizeof(struct vring_used_elem));
> >>>+ vhost_log_used_vring(dev, vq,
> >>>+ offsetof(struct vring_used,
> >>>+ ring[0]),
> >>>+ part_2 *
> >>>+ sizeof(struct vring_used_elem));
> >>>+ }
> >>> }
> >>Is expanding the code done for performance purpose?
> >
> >Hi Maxime,
> >
> >Yes theoretically this has the least branch number.
> >And I think the logic is simpler this way.
> Ok, in that case, maybe you could create a function to
> do the rte_memcpy and the vhost_log_used on a given range.
Agreed, that will be better; it could avoid repeating similar code
block 3 times.
> I don't have a strong opinion on this, if Yuanhan is fine
> with current code, that's ok for me.
>From what I know, that's kind of DPDK prefered way, to expand code
when necessary. For example, 9ec201f5d6e7 ("mbuf: provide bulk
allocation").
So I'm fine with it.
--yliu
[dpdk-dev] [PATCH v5 5/6] vhost: batch update used ring
> -Original Message-
> From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> Sent: Sunday, September 18, 2016 10:56 AM
> To: Maxime Coquelin
> Cc: Wang, Zhihong ; dev at dpdk.org;
> thomas.monjalon at 6wind.com
> Subject: Re: [PATCH v5 5/6] vhost: batch update used ring
>
> On Thu, Sep 15, 2016 at 06:38:06PM +0200, Maxime Coquelin wrote:
> > >>>+static inline void __attribute__((always_inline))
> > >>>+flush_used_ring(struct virtio_net *dev, struct vhost_virtqueue *vq,
> > >>>+uint32_t used_idx_start)
> > >>>+{
> > >>>+if (used_idx_start + vq->shadow_used_idx < vq->size) {
> > >>>+rte_memcpy(&vq->used->ring[used_idx_start],
> > >>>+&vq->shadow_used_ring[0],
> > >>>+vq->shadow_used_idx *
> > >>>+sizeof(struct vring_used_elem));
> > >>>+vhost_log_used_vring(dev, vq,
> > >>>+offsetof(struct vring_used,
> > >>>+ring[used_idx_start]),
> > >>>+vq->shadow_used_idx *
> > >>>+sizeof(struct vring_used_elem));
> > >>>+} else {
> > >>>+uint32_t part_1 = vq->size - used_idx_start;
> > >>>+uint32_t part_2 = vq->shadow_used_idx - part_1;
> > >>>+
> > >>>+rte_memcpy(&vq->used->ring[used_idx_start],
> > >>>+&vq->shadow_used_ring[0],
> > >>>+part_1 *
> > >>>+sizeof(struct vring_used_elem));
> > >>>+vhost_log_used_vring(dev, vq,
> > >>>+offsetof(struct vring_used,
> > >>>+ring[used_idx_start]),
> > >>>+part_1 *
> > >>>+sizeof(struct vring_used_elem));
> > >>>+rte_memcpy(&vq->used->ring[0],
> > >>>+&vq->shadow_used_ring[part_1],
> > >>>+part_2 *
> > >>>+sizeof(struct vring_used_elem));
> > >>>+vhost_log_used_vring(dev, vq,
> > >>>+offsetof(struct vring_used,
> > >>>+ring[0]),
> > >>>+part_2 *
> > >>>+sizeof(struct vring_used_elem));
> > >>>+}
> > >>> }
> > >>Is expanding the code done for performance purpose?
> > >
> > >Hi Maxime,
> > >
> > >Yes theoretically this has the least branch number.
> > >And I think the logic is simpler this way.
> > Ok, in that case, maybe you could create a function to
> > do the rte_memcpy and the vhost_log_used on a given range.
>
> Agreed, that will be better; it could avoid repeating similar code
> block 3 times.
Okay. Thanks for the suggestion, Maxime and Yuanhan.
>
> > I don't have a strong opinion on this, if Yuanhan is fine
> > with current code, that's ok for me.
>
> From what I know, that's kind of DPDK prefered way, to expand code
> when necessary. For example, 9ec201f5d6e7 ("mbuf: provide bulk
> allocation").
>
> So I'm fine with it.
>
> --yliu
[dpdk-dev] [PATCH v2 0/2] Add HMAC_MD5 to Intel QuickAssist Technology driver
Tested-by: Yong Liu - Tested Branch: dpdk-next-crypto/master - Tested Commit: 3e329d659b62097d15ec86ff92acc8effaf28cd2 - OS: Fedora20 3.11.10-301.fc20.x86_64 - GCC: gcc version 4.8.3 20140911 - CPU: Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz - NIC: Intel Corporation Device Fortville [8086:1583] - QAT: Intel Corporation Coleto Creek PCIe Endpoint [8086:0435] - Default x86_64-native-linuxapp-gcc configuration - Prerequisites: - Total 2 cases, 2 passed, 0 failed - Prerequisites command / instruction: Enable CONFIG_RTE_LIBRTE_PMD_QAT in config/common_base and rebuild dpdk Bind QAT VF devices to igb_uio - Case: Description: HMAC_MD5 authentication unit test Command / instruction: Start test application with normal eal parameter. ./$RTE_TARGET/app/test -c f -n 4 -- -i Run crypto QAT unit test suite and verify MD5_HMAC cases passed - Case: Description: l2fwd_crypto with HMAC_MD5 authentication test Command / instruction: Start l2fwd_crypto with QAT technology enable. Authentication method is MD5_HMAC, auth key is also inputted in. l2fwd-crypto -c 0xf -n4 -- -p0x1 --chain HASH_ONLY --cdev_type HW \ --auth_algo MD5_HMAC --auth_op GENERATE \ --auth_key 00:01:02:03:04:05:06:07:08:09:0a:0b:0c:0d:0e:0f Send 65 packets with random 64 bytes payload and capture forwarded packets. Check all forwarded packets contained of 16 bytes hashed value. Check hash values are same as automatic calculated value. > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Deepak Kumar Jain > Sent: Friday, September 09, 2016 11:45 PM > To: dev at dpdk.org > Cc: De Lara Guarch, Pablo; Jain, Deepak K > Subject: [dpdk-dev] [PATCH v2 0/2] Add HMAC_MD5 to Intel QuickAssist > Technology driver > > This patchset add capability to use HMAC_MD5 hash algorithm to Intel > QuickAssist Technology driver and test cases to cryptodev test files. > > This patchset depends on the following patches/patchsets: > > "crypto/qat: make the session struct variable in size" > (http://dpdk.org/dev/patchwork/patch/15125/) > > Arkadiusz Kusztal (2): > crypto/qat: add MD5 HMAC capability to Intel QAT driver > app/test: add test cases for MD5 HMAC for Intel QAT driver > > Changes from v1: > * Added new feature information in release_16_11.rst file. > > app/test/test_cryptodev.c| 185 > +++ > app/test/test_cryptodev_hmac_test_vectors.h | 121 +++ > doc/guides/cryptodevs/qat.rst| 1 + > doc/guides/rel_notes/release_16_11.rst | 5 + > drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 34 + > drivers/crypto/qat/qat_crypto.c | 28 +++- > 6 files changed, 372 insertions(+), 2 deletions(-) > create mode 100644 app/test/test_cryptodev_hmac_test_vectors.h > > -- > 2.5.5
[dpdk-dev] [PATCH v3 0/2] add aes-sha224-hmac support to Intel QAT driver
Tested-by: Yong Liu - Tested Branch: dpdk-next-crypto/master - Tested Commit: 3e329d659b62097d15ec86ff92acc8effaf28cd2 - OS: Fedora20 3.11.10-301.fc20.x86_64 - GCC: gcc version 4.8.3 20140911 - CPU: Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz - NIC: Intel Corporation Device Fortville [8086:1583] - QAT: Intel Corporation Coleto Creek PCIe Endpoint [8086:0435] - Default x86_64-native-linuxapp-gcc configuration - Prerequisites: - Total 2 cases, 2 passed, 0 failed - Prerequisites command / instruction: Enable CONFIG_RTE_LIBRTE_PMD_QAT in config/common_base and rebuild dpdk Bind QAT VF devices to igb_uio - Case: Description: SHA224_HMAC authentication unit test Command / instruction: Start test application with normal eal parameter. ./$RTE_TARGET/app/test -c f -n 4 -- -i Run crypto QAT unit test suite and verify SHA224_HMAC cases passed - Case: Description: l2fwd_crypto with SHA224_HMAC authentication test Command / instruction: Start l2fwd_crypto with QAT technology enable. Authentication method is SHA224_HMAC, auth key is also inputted in. Authentication key length for SHA224_HMAC should be 64 bytes. ./examples/l2fwd-crypto/build/app/l2fwd-crypto -c0xf -n4 -- -p0x1 \ --chain HASH_ONLY --cdev_type ANY --auth_algo SHA224_HMAC \ --auth_op GENERATE --auth_key $auth_key Send 65 packets with random 64 bytes payload and capture forwarded packets. Check all forwarded packets contained of 28 bytes hashed value. Check hash values are same as automatic calculated value. > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Deepak Kumar Jain > Sent: Friday, September 16, 2016 12:27 AM > To: dev at dpdk.org > Cc: De Lara Guarch, Pablo; Jain, Deepak K > Subject: [dpdk-dev] [PATCH v3 0/2] add aes-sha224-hmac support to Intel > QAT driver > > This patchset adds support of aes-sha224-hmac in Intel(R) QuickAssist > Technology driver. > > This patchset depends on following patchset: > "crypto/qat: add MD5 HMAC capability to Intel QAT driver" > (http://dpdk.org/dev/patchwork/patch/15754/) > > Jain, Deepak K (2): > crypto/qat: add aes-sha224-hmac capability to Intel QAT driver > app/test: add test cases for aes-sha224-hmac for Intel QAT driver > > Changes in v3: > * Cover letter updated with correct information about sha224-hmac. > > Changes in v2: > * Added new feature information in release_16_11.rst file. > * Added information about sha224-hmac in capabilities. > > app/test/test_cryptodev_aes.c| 6 +++-- > doc/guides/cryptodevs/qat.rst| 1 + > doc/guides/rel_notes/release_16_11.rst | 1 + > drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 33 > > drivers/crypto/qat/qat_crypto.c | 25 +- > 5 files changed, 63 insertions(+), 3 deletions(-) > > -- > 2.5.5
[dpdk-dev] [PATCH v2 0/2] add aes-sha384-hmac support to Intel QAT driver
Tested-by: Yong Liu - Tested Branch: dpdk-next-crypto/master - Tested Commit: 3e329d659b62097d15ec86ff92acc8effaf28cd2 - OS: Fedora20 3.11.10-301.fc20.x86_64 - GCC: gcc version 4.8.3 20140911 - CPU: Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz - NIC: Intel Corporation Device Fortville [8086:1583] - QAT: Intel Corporation Coleto Creek PCIe Endpoint [8086:0435] - Default x86_64-native-linuxapp-gcc configuration - Prerequisites: - Total 2 cases, 2 passed, 0 failed - Prerequisites command / instruction: Enable CONFIG_RTE_LIBRTE_PMD_QAT in config/common_base and rebuild dpdk Bind QAT VF devices to igb_uio - Case: Description: SHA384_HMAC authentication unit test Command / instruction: Start test application with normal eal parameter. ./$RTE_TARGET/app/test -c f -n 4 -- -i Run crypto QAT unit test suite and verify SHA384_HMAC cases passed - Case: Description: l2fwd_crypto with SHA384_HMAC authentication test Command / instruction: Start l2fwd_crypto with QAT technology enable. Authentication method is SHA384_HMAC, auth key is also inputted in. Authentication key length for SHA384_HMAC should be 128 bytes. ./examples/l2fwd-crypto/build/app/l2fwd-crypto -c0xf -n4 -- -p0x1 \ --chain HASH_ONLY --cdev_type ANY --auth_algo SHA384_HMAC \ --auth_op GENERATE --auth_key $auth_key Send 65 packets with random 64 bytes payload and capture forwarded packets. Check all forwarded packets contained of 48 bytes hashed value. Check hash values are same as calculated value. > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Deepak Kumar Jain > Sent: Tuesday, September 13, 2016 3:51 AM > To: dev at dpdk.org > Cc: De Lara Guarch, Pablo; Jain, Deepak K > Subject: [dpdk-dev] [PATCH v2 0/2] add aes-sha384-hmac support to Intel > QAT driver > > This patchset adds support of aes-sha384-hmac in Intel(R) QuickAssist > Technology driver. > > This patchset depends on following patchset: > "crypto/qat: add aes-sha224-hmac capability to Intel QAT driver" > (http://dpdk.org/dev/patchwork/patch/15776/) > > > Jain, Deepak K (2): > crypto/qat: add aes-sha384-hmac capability to Intel QAT driver > app/test: add test cases for aes-sha384-hmac for Intel QAT driver > > Changes from V1: > * Added new feature information in release_16_11.rst file. > * Added information about sha384-hmac in capabilities. > > app/test/test_cryptodev_aes.c| 6 +++-- > doc/guides/cryptodevs/qat.rst| 1 + > doc/guides/rel_notes/release_16_11.rst | 1 + > drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 33 > > drivers/crypto/qat/qat_crypto.c | 31 > +++--- > 5 files changed, 66 insertions(+), 6 deletions(-) > > -- > 2.5.5
[dpdk-dev] [PATCH v3 00/15] Introduce SoC device/driver framework for EAL
On 9 September 2016 at 16:43, Shreyansh Jain wrote: > Introduction: > = > > This patch set is direct derivative of Jan's original series [1],[2]. > > - As this deviates substantially from original series, if need be I can >post it as a separate patch rather than v2. Please suggest. > - Also, there are comments on original v1 ([4]) which are _not_ >incorporated in this series as they refer to section no more in new >version. > - This v3 version is based on the rte_driver/device patchset v9 [10]. >That series introduced device structures (rte_driver/rte_device) >generalizing devices into PCI, VDEV, XXX. For the purpose of this >patchset, XXX=>SOC. > > Aim: > > > As of now EAL is primarly focused on PCI initialization/probing. > > rte_eal_init() > |- rte_eal_pci_init(): Find PCI devices from sysfs > |- ... > |- rte_eal_memzone_init() > |- ... > `- rte_eal_pci_probe(): Driver<=>Device initialization > > This patchset introduces SoC framework which would enable SoC drivers and > drivers to be plugged into EAL, very similar to how PCI drivers/devices are > done today. > > This is a stripped down version of PCI framework which allows the SoC PMDs > to implement their own routines for detecting devices and linking devices to > drivers. > > 1) Changes to EAL > rte_eal_init() > |- rte_eal_pci_init(): Find PCI devices from sysfs > |- rte_eal_soc_init(): Calls PMDs->scan_fn > |- ... > |- rte_eal_memzone_init() > |- ... > |- rte_eal_pci_probe(): Driver<=>Device initialization, PMD->devinit() > `- rte_eal_soc_probe(): Calls PMDs->match_fn and PMDs->devinit(); > > 2) New device/driver structures: > - rte_soc_driver (inheriting rte_driver) > - rte_soc_device (inheriting rte_device) > - rte_eth_dev and eth_driver embedded rte_soc_device and rte_soc_driver, > respectively. > > 3) The SoC PMDs need to: > - define rte_soc_driver with necessary scan and match callbacks > - Register themselves using DRIVER_REGISTER_SOC() > - Implement respective bus scanning in the scan callbacks to add necessary >devices to SoC device list > - Implement necessary eth_dev_init/uninint for ethernet instances > > 4) Design considerations that are same as PCI: > - SoC initialization is being done through rte_eal_init(), just after PCI >initialization is done. > - As in case of PCI, probe is done after rte_eal_pci_probe() to link the >devices detected with the drivers registered. > - Device attach/detach functions are available and have been designed on >the lines of PCI framework. > - PMDs register using DRIVER_REGISTER_SOC, very similar to >DRIVER_REGISTER_PCI for PCI devices. > - Linked list of SoC driver and devices exists independent of the other >driver/device list, but inheriting rte_driver/rte_driver, these are also >part of a global list. > > 5) Design considerations that are different from PCI: > - Each driver implements its own scan and match function. PCI uses the BDF >format to read the device from sysfs, but this _may_not_ be a case for a >SoC ethernet device. >= This is an important change from initial proposal by Jan in [2]. Unlike >his attempt to use /sys/bus/platform, this patch relies on the PMD to It could be many redundant code if Each PMD driver has the scan function if its own. I think Jan's implementation is common to many platform drivers. >detect the devices. This is because SoC may require specific or >additional info for device detection. Further, SoC may have embedded Can you give us more precise definition about SoC driver? Does it include the driver in ARM server? >devices/MACs which require initialization which cannot be covered through >sysfs parsing. I think it can be done in devinit, not in scan function. devinit can be different for each driver. >= PCI based PMDs rely on EAL's capability to detect devices. This >proposal puts the onus on PMD to detect devices, add to soc_device_list >and wait for Probe. Matching, of device<=>driver is again PMD's callback. >
[dpdk-dev] Dear Tetsuya Makawa, Hotplug and PMD for NFV connected port
Dear Tetsuya Makawa, I was watching your video on DPDK Hotplug module There is a mention about PMD for NFV connected port. At that time of the video (2015) it mentions no support. Now is the support available? From which version we have support please? Thank you very much M Jay
[dpdk-dev] [PATCH v3 00/15] Introduce SoC device/driver framework for EAL
On Sun, 18 Sep 2016 13:58:50 +0800 Jianbo Liu wrote: > On 9 September 2016 at 16:43, Shreyansh Jain > wrote: > > Introduction: > > = > > > > This patch set is direct derivative of Jan's original series [1],[2]. > > > > - As this deviates substantially from original series, if need be I can > >post it as a separate patch rather than v2. Please suggest. > > - Also, there are comments on original v1 ([4]) which are _not_ > >incorporated in this series as they refer to section no more in new > >version. > > - This v3 version is based on the rte_driver/device patchset v9 [10]. > >That series introduced device structures (rte_driver/rte_device) > >generalizing devices into PCI, VDEV, XXX. For the purpose of this > >patchset, XXX=>SOC. [...] > > > > 5) Design considerations that are different from PCI: > > - Each driver implements its own scan and match function. PCI uses the BDF > >format to read the device from sysfs, but this _may_not_ be a case for a > >SoC ethernet device. > >= This is an important change from initial proposal by Jan in [2]. Unlike > >his attempt to use /sys/bus/platform, this patch relies on the PMD to > > It could be many redundant code if Each PMD driver has the scan > function if its own. > I think Jan's implementation is common to many platform drivers. I personally can find a use case for having a custom scan function. However, we should at least provide a default implementation. Probably, both the scan and match functions should be used to _override_ a default behaviour. So, only drivers that require to scan devices in a specific way would provide a custom function for this. I agree, that this can sometimes lead to code duplication. Moreover, it opens door for a very non-standard, unsecure and wrong-by-design approaches. I'd like more to provide one or more scan implementations in EAL and do not put this responsibility on PMDs. > > >detect the devices. This is because SoC may require specific or > >additional info for device detection. Further, SoC may have embedded Can you provide an example for "additional info for device detection"? > > Can you give us more precise definition about SoC driver? Does it > include the driver in ARM server? I am sorry but I don't understand this question. What you mean by a "driver in ARM server"? Do you mean a kernel driver? There is no "SoC driver" in the text so what definition are asking for? > > >devices/MACs which require initialization which cannot be covered through > >sysfs parsing. I think, the description itself is incorrect. If a device's initialization cannot be satisfied vie sysfs, it means that you have to write a specific probe function. This is not related to scan in any way. However, there may be a group of devices which are not managed by the standard platform_driver of the Linux Kernel (or other OS). In that case, the custom scan function would be helpful. I can imagine a device in a fully I/O coherent platform that only requires to access the /dev/mem only (for the register space). It is unsecure but it would work without any OS-driver. However, I consider it a corner case. It can be useful for testing sometimes but not very helpful for production. We should however support mainly the standard devices which are always represented by the OS. Otherwise, such system would introduce security issues. > > I think it can be done in devinit, not in scan function. devinit can > be different for each driver. +1 > > >= PCI based PMDs rely on EAL's capability to detect devices. This > >proposal puts the onus on PMD to detect devices, add to soc_device_list > >and wait for Probe. Matching, of device<=>driver is again PMD's callback. > > Regards Jan -- Jan ViktorinE-mail: Viktorin at RehiveTech.com System ArchitectWeb:www.RehiveTech.com RehiveTech Brno, Czech Republic
[dpdk-dev] [PATCH] crypto/qat: fix memzone creation to use a fixed size string
On Wed, Sep 14, 2016 at 04:32:46PM +0100, John Griffin wrote: > Hi Liu, > Comments embedded. > > Rgds, > John. > > On 05/09/16 04:23, Yuanhan Liu wrote: > >On Thu, Sep 01, 2016 at 11:21:38AM +0100, John Griffin wrote: > >>Remove the dependency on dev->driver->pci_drv.name when > >>creating the memzone for the qat hardware queues. > >>The pci_drv.name may grow too large for RTE_MEMZONE_NAMESIZE. > > > >Will the "may grow too large" cause any issues? If so, state it here. If > >not, marking this patch as a "fix" patch doesn't make sense to me then. > We discovered this when applying a future patch (2141c21966) and it exposed > this issue. > Problem is we create a memzone per hardware queue pair and if the memzone > name is too large, then this code will not produce a unique > name and two qps will end using the same memzone. Thanks for the info, and I think you should put it in the commit log: it helps people to really know what might go wrong without this fix. --yliu
[dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id
On Wed, Sep 14, 2016 at 10:35:53AM +0200, Thomas Monjalon wrote: > 2016-09-14 15:21, Yuanhan Liu: > > On Wed, Sep 14, 2016 at 09:10:48AM +0200, Thomas Monjalon wrote: > > > 2016-09-14 12:43, Yuanhan Liu: > > > > On Tue, Sep 13, 2016 at 05:10:09PM +0200, Thomas Monjalon wrote: > > > > > 2016-09-13 14:47, Ciara Loftus: > > > > > > In some cases when using the vHost PMD, certain vHost library > > > > > > functions > > > > > > may still need to be accessed. One such example is the > > > > > > rte_vhost_get_queue_num function which returns the number of > > > > > > virtqueues > > > > > > reported by the guest - information which is not exposed by the PMD. > > > > > > > > > > > > This commit introduces a new rte_eth_vhost function that returns the > > > > > > 'vid' associated with a given port id. This allows the PMD user to > > > > > > call > > > > > > vHost library functions which require the 'vid' value. > > > > > > > > > > I think we should not add any API to the PMDs. > > > > > > > > In general, I agree with you. > > > > > > > > > Maybe you are looking for a generic API in ethdev. > > > > > > > > But maybe it's a bit hard to define a "right" generic API here. For this > > > > case, the generic API I can think of could be: > > > > > > > > - an API to get queue num, like rte_eth_get_queue_enabled_num > > > > I barely know NIC pmd drivers, but I doubt it's useful/meaningful for > > > > them. > > > > > > > > - an API to get a PMD driver private (or specific) data. > > > > For vhost-pmd, it's vid. Again, I don't know what it could be for > > > > other nic > > > > drivers. > > > > > > > > This one may be a better option here, because it expose a key field to > > > > the application, vid, with which the application can invoke more vhost > > > > APIs. And apparently, it's not feasible to try to define a generic API > > > > for some (if not each) vhost APIs. > > > > > > There could be a similar need in other PMD. > > > If we can get an opaque identifier of the device which is not the port id, > > > we could call some specific functions of the driver not implemented in > > > the generic ethdev API. > > > > That means you have to add/export the PMD API first. Isn't it against what > > you are proposing -- "I think we should not add any API to the PMDs" ;) > > Yes you are totally right :) > Except that in vhost case, we would not have any API in the PMD. > But it would allow to have some specific API in other PMDs for the features > which do not fit in a generic API. So, does that mean you are okay with this patch now? I mean, okay to introduce a vhost PMD API? --yliu > Just a thought, not sure yet.
[dpdk-dev] [PATCH v3 00/15] Introduce SoC device/driver framework for EAL
On 18 September 2016 at 15:22, Jan Viktorin wrote: > On Sun, 18 Sep 2016 13:58:50 +0800 > Jianbo Liu wrote: > >> On 9 September 2016 at 16:43, Shreyansh Jain >> wrote: >> > Introduction: >> > = >> > >> > This patch set is direct derivative of Jan's original series [1],[2]. >> > >> > - As this deviates substantially from original series, if need be I can >> >post it as a separate patch rather than v2. Please suggest. >> > - Also, there are comments on original v1 ([4]) which are _not_ >> >incorporated in this series as they refer to section no more in new >> >version. >> > - This v3 version is based on the rte_driver/device patchset v9 [10]. >> >That series introduced device structures (rte_driver/rte_device) >> >generalizing devices into PCI, VDEV, XXX. For the purpose of this >> >patchset, XXX=>SOC. > > [...] > >> > >> > 5) Design considerations that are different from PCI: >> > - Each driver implements its own scan and match function. PCI uses the BDF >> >format to read the device from sysfs, but this _may_not_ be a case for a >> >SoC ethernet device. >> >= This is an important change from initial proposal by Jan in [2]. >> > Unlike >> >his attempt to use /sys/bus/platform, this patch relies on the PMD to >> >> It could be many redundant code if Each PMD driver has the scan >> function if its own. >> I think Jan's implementation is common to many platform drivers. > > I personally can find a use case for having a custom scan function. > However, we should at least provide a default implementation. Probably, > both the scan and match functions should be used to _override_ a default > behaviour. So, only drivers that require to scan devices in a specific > way would provide a custom function for this. > And for each platform/product > I agree, that this can sometimes lead to code duplication. Moreover, it > opens door for a very non-standard, unsecure and wrong-by-design > approaches. I'd like more to provide one or more scan implementations > in EAL and do not put this responsibility on PMDs. > >> >> >detect the devices. This is because SoC may require specific or >> >additional info for device detection. Further, SoC may have embedded > > Can you provide an example for "additional info for device detection"? > >> >> Can you give us more precise definition about SoC driver? Does it >> include the driver in ARM server? > > I am sorry but I don't understand this question. > > What you mean by a "driver in ARM server"? Do you mean a kernel driver? > > There is no "SoC driver" in the text so what definition are asking for? > This patchset introduces rte_soc_driver, which is inheriting from rte_driver. I want to know what devices can use this SoC driver/device framework. Is it for the devices from ARM servers, or embedded systems of different vendors? And this framework is too generalized, if we don't try to understand "soc" in rte_soc_driver, we can use it for PCI devices. :) Thanks! Jianbo
[dpdk-dev] [PATCH v3 00/15] Introduce SoC device/driver framework for EAL
On Sun, 18 Sep 2016 16:56:54 +0800 Jianbo Liu wrote: > On 18 September 2016 at 15:22, Jan Viktorin > wrote: > > On Sun, 18 Sep 2016 13:58:50 +0800 > > Jianbo Liu wrote: > > > >> On 9 September 2016 at 16:43, Shreyansh Jain > >> wrote: > >> > Introduction: > >> > = > >> > > >> > This patch set is direct derivative of Jan's original series [1],[2]. > >> > > >> > - As this deviates substantially from original series, if need be I can > >> >post it as a separate patch rather than v2. Please suggest. > >> > - Also, there are comments on original v1 ([4]) which are _not_ > >> >incorporated in this series as they refer to section no more in new > >> >version. > >> > - This v3 version is based on the rte_driver/device patchset v9 [10]. > >> >That series introduced device structures (rte_driver/rte_device) > >> >generalizing devices into PCI, VDEV, XXX. For the purpose of this > >> >patchset, XXX=>SOC. > > > > [...] > > > >> > > >> > 5) Design considerations that are different from PCI: > >> > - Each driver implements its own scan and match function. PCI uses the > >> > BDF > >> >format to read the device from sysfs, but this _may_not_ be a case > >> > for a > >> >SoC ethernet device. > >> >= This is an important change from initial proposal by Jan in [2]. > >> > Unlike > >> >his attempt to use /sys/bus/platform, this patch relies on the PMD to > >> > > >> > >> It could be many redundant code if Each PMD driver has the scan > >> function if its own. > >> I think Jan's implementation is common to many platform drivers. > > > > I personally can find a use case for having a custom scan function. > > However, we should at least provide a default implementation. Probably, > > both the scan and match functions should be used to _override_ a default > > behaviour. So, only drivers that require to scan devices in a specific > > way would provide a custom function for this. > > > And for each platform/product > > > I agree, that this can sometimes lead to code duplication. Moreover, it > > opens door for a very non-standard, unsecure and wrong-by-design > > approaches. I'd like more to provide one or more scan implementations > > in EAL and do not put this responsibility on PMDs. > > > >> > >> >detect the devices. This is because SoC may require specific or > >> >additional info for device detection. Further, SoC may have embedded > > > > Can you provide an example for "additional info for device detection"? > > > >> > >> Can you give us more precise definition about SoC driver? Does it > >> include the driver in ARM server? > > > > I am sorry but I don't understand this question. > > > > What you mean by a "driver in ARM server"? Do you mean a kernel driver? > > > > There is no "SoC driver" in the text so what definition are asking for? > > > This patchset introduces rte_soc_driver, which is inheriting from rte_driver. > I want to know what devices can use this SoC driver/device framework. > Is it for the devices from ARM servers, or embedded systems of > different vendors? First, this is not an ARM-specific feature. Consider any MAC connected to the processor via some on-chip bus. In the world of ARM, it is usually a kind of AMBA bus. I think, the Intel Xeon with FPGA would be a good non-ARM example. Here they provide the Quick Path bus (but I don't know the details). So, you cannot access such device as PCI. It is usually not possible to distinguish the bus type easily (Linux calls this a platform device). So, an rte_soc_device denotes a device integrated on the chip (SoC, System-on-Chip). Such devices can have a lower access latency because they are closer to the processor. So, if you have a server system driver by a SoC with integrated MACs (no PCI-E involved), there is no way how to access them from DPDK. An rte_soc_device represents such devices and provides a way how to access them from DPDK. That is the goal... You can have an embedded device (router, switch, monitoring device, NAT, firewall, anything in a "small box" with high throughput demands) that perfectly fits into this SoC framework because it would be usually based on some SoC (ARM, ARM64, ...). > And this framework is too generalized, if we don't try to understand > "soc" in rte_soc_driver, we can use it for PCI devices. :) No, you cannot use it for PCI devices, don't worry. There should be no PCI facilities like access to BARs :). But, I think I got your point. It seems to be generalized because there is no real standard in this area. Any vendor of SoC can provide her custom on-chip bus. The original idea was to build on top of the platform_device from Linux which hides this information from you (unless there is some bus-specific DMA which we must handle in the DPDK PMD). We could provide an rte_amba_device instead but there is no advantage in this. The amba bus is defined on the RTL level and not on the software level (no BARs, no device dis
[dpdk-dev] [PATCH v3 00/15] Introduce SoC device/driver framework for EAL
> -Original Message- > From: Jan Viktorin [mailto:viktorin at rehivetech.com] > On Sun, 18 Sep 2016 16:56:54 +0800 > Jianbo Liu wrote: > > > On 18 September 2016 at 15:22, Jan Viktorin > wrote: > > > On Sun, 18 Sep 2016 13:58:50 +0800 > > > Jianbo Liu wrote: > > > > > >> On 9 September 2016 at 16:43, Shreyansh Jain > wrote: > > >> > Introduction: > > >> > = > > >> > > > >> > This patch set is direct derivative of Jan's original series [1],[2]. > > >> > > > >> > - As this deviates substantially from original series, if need be I > > >> > can > > >> >post it as a separate patch rather than v2. Please suggest. > > >> > - Also, there are comments on original v1 ([4]) which are _not_ > > >> >incorporated in this series as they refer to section no more in new > > >> >version. > > >> > - This v3 version is based on the rte_driver/device patchset v9 [10]. > > >> >That series introduced device structures (rte_driver/rte_device) > > >> >generalizing devices into PCI, VDEV, XXX. For the purpose of this > > >> >patchset, XXX=>SOC. > > > > > > [...] > > > > > >> > > > >> > 5) Design considerations that are different from PCI: > > >> > - Each driver implements its own scan and match function. PCI uses the > BDF > > >> >format to read the device from sysfs, but this _may_not_ be a case > > >> > for a > > >> >SoC ethernet device. > > >> >= This is an important change from initial proposal by Jan in [2]. > > >> > Unlike > > >> >his attempt to use /sys/bus/platform, this patch relies on the > > >> > PMD to > > >> > > >> It could be many redundant code if Each PMD driver has the scan > > >> function if its own. > > >> I think Jan's implementation is common to many platform drivers. > > > > > > I personally can find a use case for having a custom scan function. > > > However, we should at least provide a default implementation. > > > Probably, both the scan and match functions should be used to > > > _override_ a default behaviour. So, only drivers that require to > > > scan devices in a specific way would provide a custom function for this. > > > > > And for each platform/product > > > > > I agree, that this can sometimes lead to code duplication. Moreover, > > > it opens door for a very non-standard, unsecure and wrong-by-design > > > approaches. I'd like more to provide one or more scan > > > implementations in EAL and do not put this responsibility on PMDs. [Hemant] A common/default scan function can be added, provided at least one or more PMD driver support it. w.r.t Jan's original scan function, it was not suitable for any of the NXP SoC's whether ARM or PowerPC. Unable to validate the Jan's scan function on a real platform, we have skipped it for next phase. Addition of a default scan function can only be done in next phase, when we find a suitable SoC PMD driver supporting it. > > > > > >> > > >> >detect the devices. This is because SoC may require specific or > > >> >additional info for device detection. Further, SoC may have > > >> > embedded > > > > > > Can you provide an example for "additional info for device detection"? > > > > > >> > > >> Can you give us more precise definition about SoC driver? Does it > > >> include the driver in ARM server? > > > > > > I am sorry but I don't understand this question. > > > > > > What you mean by a "driver in ARM server"? Do you mean a kernel driver? > > > > > > There is no "SoC driver" in the text so what definition are asking for? > > > > > This patchset introduces rte_soc_driver, which is inheriting from > > rte_driver. > > I want to know what devices can use this SoC driver/device framework. > > Is it for the devices from ARM servers, or embedded systems of > > different vendors? > > First, this is not an ARM-specific feature. Consider any MAC connected to the > processor via some on-chip bus. In the world of ARM, it is usually a kind of > AMBA bus. I think, the Intel Xeon with FPGA would be a good non-ARM example. > Here they provide the Quick Path bus (but I don't know the details). So, you > cannot access such device as PCI. It is usually not possible to distinguish > the bus > type easily (Linux calls this a platform device). > > So, an rte_soc_device denotes a device integrated on the chip (SoC, System-on- > Chip). Such devices can have a lower access latency because they are closer to > the processor. > > So, if you have a server system driver by a SoC with integrated MACs (no PCI-E > involved), there is no way how to access them from DPDK. An rte_soc_device > represents such devices and provides a way how to access them from DPDK. > That is the goal... > > You can have an embedded device (router, switch, monitoring device, NAT, > firewall, anything in a "small box" with high throughput demands) that > perfectly > fits into this SoC framework because it would be usually based on some SoC > (ARM, ARM64, ...). > > > And this framework is too generalized, if we do
[dpdk-dev] [PATCH v3 00/15] Introduce SoC device/driver framework for EAL
On Sun, 18 Sep 2016 09:41:55 + Hemant Agrawal wrote: > > -Original Message- > > From: Jan Viktorin [mailto:viktorin at rehivetech.com] > [...] > > > And for each platform/product > > > > > > > I agree, that this can sometimes lead to code duplication. Moreover, > > > > it opens door for a very non-standard, unsecure and wrong-by-design > > > > approaches. I'd like more to provide one or more scan > > > > implementations in EAL and do not put this responsibility on PMDs. > Hi Hemant. > [Hemant] A common/default scan function can be added, provided at least one > or more PMD driver support it. > w.r.t Jan's original scan function, it was not suitable for any of the NXP > SoC's whether ARM or PowerPC. > > Unable to validate the Jan's scan function on a real platform, we have > skipped it for next phase. > Addition of a default scan function can only be done in next phase, when we > find a suitable SoC PMD driver supporting it. Quite frankly, the situation is same for me. I still have no clue about your approach which seems to be pretty non-standard. I have no way how to test it. My approach can be tested on any Linux machine with platform devices and device-tree enabled. You would see that I detect those devices (I don't mean any certain network device, I mean all platform devices) and if you provide a driver with a proper compatible string it will be set for you. I presume that I don't have any upstreamable PMD for this at the moment.
[dpdk-dev] pcap: memory leak in eth_pcap_rx_jumbo
Hi, It seems there is a memory leak in eth_pcap_rx_jumbo() in case rte_pktmbuf_alloc() fails on any segment that is not the initial segment. If someone can confirm it, I'll patch it. TIA, Dror
[dpdk-dev] [PATCH v2] net/vhost: add pmd xstats
On Wed, Sep 14, 2016 at 07:43:36AM +, Yang, Zhiyong wrote:
> Hi, yuanhan:
> Thanks so much for your detailed comments.
>
> > -Original Message-
> > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com]
> > Sent: Wednesday, September 14, 2016 2:20 PM
> > To: Yang, Zhiyong
> > Cc: dev at dpdk.org; thomas.monjalon at 6wind.com; pmatilai at redhat.com
> > Subject: Re: [PATCH v2] net/vhost: add pmd xstats
> >
> > On Fri, Sep 09, 2016 at 04:15:27PM +0800, Zhiyong Yang wrote:
> > > +struct vhost_xstats {
> > > + uint64_t stat[16];
> > > +};
> > > +
> > > struct vhost_queue {
> > > int vid;
> > > rte_atomic32_t allow_queuing;
> > > @@ -85,7 +89,8 @@ struct vhost_queue {
> > > uint64_t missed_pkts;
> > > uint64_t rx_bytes;
> > > uint64_t tx_bytes;
> >
> > I'd suggest to put those statistic counters to vhost_stats struct, which
> > could
> > simplify the xstats_reset code a bit.
> >
>
> I consider this point when I define it, but those statistic counters are used
> by pmd stats,
> not only by pmd xstats, I'm not clear if I can modify those code. If
> permitted,
> I will do it as you said.
For sure you can modify the code :) I just would suggest to do in an
single patch, as stated before (and below).
> > > + char name[RTE_ETH_VHOST_XSTATS_NAME_SIZE];
> > > + uint64_t offset;
> > > +};
> > > +
> > > +/* [rt]_qX_ is prepended to the name string here */ static void
> > > +vhost_dev_xstats_reset(struct rte_eth_dev *dev) {
> > > + struct vhost_queue *vqrx = NULL;
> > > + struct vhost_queue *vqtx = NULL;
> > > + unsigned int i = 0;
> > > +
> > > + for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > > + if (!dev->data->rx_queues[i])
> > > + continue;
> > > + vqrx = (struct vhost_queue *)dev->data->rx_queues[i];
> >
> > Unnecessary cast.
> >
> The definition of rx_queues is void **rx_queues;
Yes, but rx_queues[i] is with type "void *", so the cast is not necessary.
> > > + }
> > > + }
> > > + /* non-multi/broadcast, multi/broadcast, including those
> > > + * that were discarded or not sent.
> >
> > Hmmm, I don't follow it. You may want to reword it.
> >
> > > from rfc2863
> >
> > Which section and which page?
> >
> The packets received are not considered "discarded", because receiving
> packets via
> Memory, not by physical NIC. Mainly for the number of transmit the packets
It still took me some time to understand you.
> RFC2863 page 35 ifOutUcastPkts(non-multi/broadcast)
So, by your term, unicast == non-multicast/broadcast? If so, I'd suggest
to use term "unicast" but not something else: it just introduces confusions.
And in this case, I guess you were trying to say:
/*
* According to RFC 2863 section ifHCOutMulticastPkts, ... and ...,
* the counter "unicast", "multicast" and "broadcast" are also
* increased when packets are not transmitted successfully.
*/
Well, you might still need reword it.
After taking a bit closer look at the code, I'd suggest to do the countings
like following:
- introduce a help function to increase the "broadcast" or "multicast"
counter, say "count_multicast_broadcast(struct vhost_stats *stats, mbuf)".
- introduce a generic function to update the generic counters: this
function just counts those packets have been successfully transmitted
or received.
It also invoke above helper function for multicast counting.
It basically looks like the function vhost_update_packet_xstats,
execpt that it doesn't handle those failure packets: it will be
handled in following part.
- since the case "couting multicast and broadcast with failure packts"
only happens in the Tx side, we could do those countings only in
eth_vhost_tx():
nb_tx = rte_vhost_enqueue_burst(r->vid,
r->virtqueue_id, bufs, nb_bufs);
missed = nb_bufs - nb_tx;
/* put above comment here */
if (missed) {
for_each_mbuf(mbuf) {
count_multicast_broadcast(&vq->stats, mbuf);
}
}
- there is no need to update "stat[10]" (unicast), since it can be calculated
from other counters, meaning we could just get the right value on query.
This could save some cycles.
Feel free to phone me if you have any doubts.
--yliu
[dpdk-dev] [PATCH v2] drivers/net:new PMD using tun/tap host interface
On Fri, Sep 16, 2016 at 11:22:13AM -0500, Keith Wiles wrote: > The rte_eth_tap.c PMD creates a device using TUN/TAP interfaces > on the local host. The PMD allows for DPDK and the host to > communicate using a raw device interface on the host and in > the DPDK application. The device created is a Tap device with > a L2 packet header. > > v2 - merge all of the patches into one patch. > Fix a typo on naming the tap device. > Update the maintainers list > > Signed-off-by: Keith Wiles Hi, FYI, my robot caught some build errors with this patch applied. --yliu --- In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:71:2: error: redefinition of enumerator 'IFF_UP' IFF_UP = 1<<0, /* sysfs */ ^ /usr/include/net/if.h:45:17: note: expanded from macro 'IFF_UP' # define IFF_UP IFF_UP ^ /usr/include/net/if.h:44:5: note: previous definition is here IFF_UP = 0x1, /* Interface is up. */ ^ In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:72:2: error: redefinition of enumerator 'IFF_BROADCAST' IFF_BROADCAST = 1<<1, /* __volatile__ */ ^ /usr/include/net/if.h:47:24: note: expanded from macro 'IFF_BROADCAST' # define IFF_BROADCAST IFF_BROADCAST ^ /usr/include/net/if.h:46:5: note: previous definition is here IFF_BROADCAST = 0x2,/* Broadcast address valid. */ ^ In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:73:2: error: redefinition of enumerator 'IFF_DEBUG' IFF_DEBUG = 1<<2, /* sysfs */ ^ /usr/include/net/if.h:49:20: note: expanded from macro 'IFF_DEBUG' # define IFF_DEBUG IFF_DEBUG ^ /usr/include/net/if.h:48:5: note: previous definition is here IFF_DEBUG = 0x4,/* Turn on debugging. */ ^ In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:74:2: error: redefinition of enumerator 'IFF_LOOPBACK' IFF_LOOPBACK= 1<<3, /* __volatile__ */ ^ /usr/include/net/if.h:51:23: note: expanded from macro 'IFF_LOOPBACK' # define IFF_LOOPBACK IFF_LOOPBACK ^ /usr/include/net/if.h:50:5: note: previous definition is here IFF_LOOPBACK = 0x8, /* Is a loopback net. */ ^ In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:75:2: error: redefinition of enumerator 'IFF_POINTOPOINT' IFF_POINTOPOINT = 1<<4, /* __volatile__ */ ^ /usr/include/net/if.h:53:26: note: expanded from macro 'IFF_POINTOPOINT' # define IFF_POINTOPOINT IFF_POINTOPOINT ^ /usr/include/net/if.h:52:5: note: previous definition is here IFF_POINTOPOINT = 0x10, /* Interface is point-to-point link. */ ^ In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:76:2: error: redefinition of enumerator 'IFF_NOTRAILERS' IFF_NOTRAILERS = 1<<5, /* sysfs */ ^ /usr/include/net/if.h:55:25: note: expanded from macro 'IFF_NOTRAILERS' # define IFF_NOTRAILERS IFF_NOTRAILERS ^ /usr/include/net/if.h:54:5: note: previous definition is here IFF_NOTRAILERS = 0x20, /* Avoid use of trailers. */ ^ In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:77:2: error: redefinition of enumerator 'IFF_RUNNING' IFF_RUNNING = 1<<6, /* __volatile__ */ ^ /usr/include/net/if.h:57:22: note: expanded from macro 'IFF_RUNNING' # define IFF_RUNNINGIFF_RUNNING ^ /usr/include/net/if.h:56:5: note: previous definition is here IFF_RUNNING = 0x40, /* Resources allocated. */ ^ In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:78:2: error: redefinition of enumerator 'IFF_NOARP' IFF_NOARP = 1<<7, /* sysfs */ ^ /usr/include/net/if.h:59:20: note: expanded from macro 'IFF_NOARP' # define IFF_NOARP IFF_NOARP ^ /usr/include/net/if.h:58:5: note: previous definition is here IFF_NOARP = 0x80, /* No address resolution protocol. */ ^ In file included from /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: /usr/include/linux/if.h:79:2: error: redefinition of enumerator 'IFF_PROMISC' IFF_PROMISC = 1<<8, /* sysfs */ ^ /usr/include/net/if.
[dpdk-dev] [PATCH v5 2/6] vhost: rewrite enqueue
On Thu, Sep 08, 2016 at 11:39:24PM -0400, Zhihong Wang wrote: > This patch implements the vhost logic from scratch into a single function > designed for high performance and better maintainability. As always, your commit log just states what have been done, but doesn't tell why such changes have been made. For example, you said "it's designed for high performance", then you'd better explain why your version would introduce high performance. You need a reason, as well as some numbers (percent change) to prove it: it's not that right to keep the numbers inside: I'm sure people outside intel are also willing and happy to know those numbers. For this patch, I think it's more about the maintainability improvement but not performance: the performance tunning patches are done later after all. Another example is, in patch 6, you said "It reduces CPU pipeline stall cycles significantly", but you didn't say why there is pipeline stall before and why your patch reduces it. All those are important things that deserves some explanation. So, I'd ask you to re-visit all your patches in this set, to think what you could add to make the commit better and more informative. Besides that, I think this patchset looks fine to me. I may just need another time to look it more carefully, then I think I can merge (v6). BTW, thanks for the great work! --yliu > This is the baseline version of the new code, more optimization will be > added in the following patches in this patch set. > > Signed-off-by: Zhihong Wang > ---
[dpdk-dev] [PATCH v2] drivers/net:new PMD using tun/tap host interface
Regards, Keith > On Sep 18, 2016, at 8:25 AM, Yuanhan Liu > wrote: > > On Fri, Sep 16, 2016 at 11:22:13AM -0500, Keith Wiles wrote: >> The rte_eth_tap.c PMD creates a device using TUN/TAP interfaces >> on the local host. The PMD allows for DPDK and the host to >> communicate using a raw device interface on the host and in >> the DPDK application. The device created is a Tap device with >> a L2 packet header. >> >> v2 - merge all of the patches into one patch. >> Fix a typo on naming the tap device. >> Update the maintainers list >> >> Signed-off-by: Keith Wiles > > Hi, > > FYI, my robot caught some build errors with this patch applied. All of the below errors are from Linux header files and not the Tap driver. How are you building DPDK, which config file are you using, OS version, Some details would be nice to understand how these errors are being generated and how I can recreate them. > > --yliu > > --- > In file included from > /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: > /usr/include/linux/if.h:71:2: error: redefinition of enumerator 'IFF_UP' >IFF_UP = 1<<0, /* sysfs */ >^ > /usr/include/net/if.h:45:17: note: expanded from macro 'IFF_UP' > # define IFF_UP IFF_UP >^ > /usr/include/net/if.h:44:5: note: previous definition is here >IFF_UP = 0x1, /* Interface is up. */ >^ > In file included from > /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: > /usr/include/linux/if.h:72:2: error: redefinition of enumerator > 'IFF_BROADCAST' >IFF_BROADCAST = 1<<1, /* __volatile__ */ >^ > /usr/include/net/if.h:47:24: note: expanded from macro 'IFF_BROADCAST' > # define IFF_BROADCAST IFF_BROADCAST >^ > /usr/include/net/if.h:46:5: note: previous definition is here >IFF_BROADCAST = 0x2,/* Broadcast address valid. */ >^ > In file included from > /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: > /usr/include/linux/if.h:73:2: error: redefinition of enumerator 'IFF_DEBUG' >IFF_DEBUG = 1<<2, /* sysfs */ >^ > /usr/include/net/if.h:49:20: note: expanded from macro 'IFF_DEBUG' > # define IFF_DEBUG IFF_DEBUG >^ > /usr/include/net/if.h:48:5: note: previous definition is here >IFF_DEBUG = 0x4,/* Turn on debugging. */ >^ > In file included from > /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: > /usr/include/linux/if.h:74:2: error: redefinition of enumerator 'IFF_LOOPBACK' >IFF_LOOPBACK= 1<<3, /* __volatile__ */ >^ > /usr/include/net/if.h:51:23: note: expanded from macro 'IFF_LOOPBACK' > # define IFF_LOOPBACK IFF_LOOPBACK >^ > /usr/include/net/if.h:50:5: note: previous definition is here >IFF_LOOPBACK = 0x8, /* Is a loopback net. */ >^ > In file included from > /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: > /usr/include/linux/if.h:75:2: error: redefinition of enumerator > 'IFF_POINTOPOINT' >IFF_POINTOPOINT = 1<<4, /* __volatile__ */ >^ > /usr/include/net/if.h:53:26: note: expanded from macro 'IFF_POINTOPOINT' > # define IFF_POINTOPOINT IFF_POINTOPOINT > ^ > /usr/include/net/if.h:52:5: note: previous definition is here >IFF_POINTOPOINT = 0x10, /* Interface is point-to-point link. */ >^ > In file included from > /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: > /usr/include/linux/if.h:76:2: error: redefinition of enumerator > 'IFF_NOTRAILERS' >IFF_NOTRAILERS = 1<<5, /* sysfs */ >^ > /usr/include/net/if.h:55:25: note: expanded from macro 'IFF_NOTRAILERS' > # define IFF_NOTRAILERS IFF_NOTRAILERS >^ > /usr/include/net/if.h:54:5: note: previous definition is here >IFF_NOTRAILERS = 0x20, /* Avoid use of trailers. */ >^ > In file included from > /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: > /usr/include/linux/if.h:77:2: error: redefinition of enumerator 'IFF_RUNNING' >IFF_RUNNING = 1<<6, /* __volatile__ */ >^ > /usr/include/net/if.h:57:22: note: expanded from macro 'IFF_RUNNING' > # define IFF_RUNNINGIFF_RUNNING >^ > /usr/include/net/if.h:56:5: note: previous definition is here >IFF_RUNNING = 0x40, /* Resources allocated. */ >^ > In file included from > /yeti/vm/ubuntu-initrd-16.04-x86_64-build/dpdk/drivers/net/tap/rte_eth_tap.c:50: > /usr/include/linux/if.h:78:2: error: redefinition of enumerator 'IFF_NOARP' >IFF_NOARP = 1<<7, /* sysfs */ >^ > /usr/include/net/if.h:59:20: note: exp
