[dpdk-dev] [PATCH v2] doc: Update doc for vhost sample
Add some contents for vhost sample. Signed-off-by: Changchun Ouyang --- Change in v2: -- Refine its format to fit well with other parts. doc/guides/sample_app_ug/vhost.rst | 52 +- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/doc/guides/sample_app_ug/vhost.rst b/doc/guides/sample_app_ug/vhost.rst index fa53db6..76997da 100644 --- a/doc/guides/sample_app_ug/vhost.rst +++ b/doc/guides/sample_app_ug/vhost.rst @@ -640,19 +640,57 @@ To call the QEMU wrapper automatically from libvirt, the following configuration Common Issues ~ -**QEMU failing to allocate memory on hugetlbfs.** +* QEMU failing to allocate memory on hugetlbfs: -file_ram_alloc: can't mmap RAM pages: Cannot allocate memory +file_ram_alloc: can't mmap RAM pages: Cannot allocate memory -When running QEMU the above error implies that it has failed to allocate memory for the Virtual Machine on the hugetlbfs. -This is typically due to insufficient hugepages being free to support the allocation request. -The number of free hugepages can be checked as follows: +When running QEMU the above error implies that it has failed to allocate memory for the Virtual Machine on +the hugetlbfs. This is typically due to insufficient hugepages being free to support the allocation request. +The number of free hugepages can be checked as follows: -.. code-block:: console +.. code-block:: console user at target:cat /sys/kernel/mm/hugepages/hugepages- / nr_hugepages -The command above indicates how many hugepages are free to support QEMU's allocation request. +The command above indicates how many hugepages are free to support QEMU's allocation request. + +* User space VHOST work properly with the guest with 2M sized hug pages: + +The guest may have 2M or 1G sized huge pages file, the user space VHOST can work properly in both cases. + +* User space VHOST will not work with QEMU without '-mem-prealloc' option: + +The current implementation work properly only when the guest memory is pre-allocated, so it is required to +use the correct QEMU version(e.g. 1.6) which supports '-mem-prealloc'; The option '-mem-prealloc' must be +specified explicitly in QEMU command line. + +* User space VHOST will not work with QEMU version without shared memory mapping: + +As shared memory mapping is mandatory for user space VHOST to work properly with the guest as user space VHOST +needs access the shared memory from the guest to receive and transmit packets. It is important to make sure +the QEMU version used supports shared memory mapping. + +* Using libvirt "virsh create" the qemu-wrap.py spawns a new process to run "qemu-kvm". This impacts the behavior +of the "virsh destroy" which kills the process running "qemu-wrap.py" without actually destroying the VM (leaves +the "qemu-kvm" process running): + +This following patch can fix this issue: +http://dpdk.org/ml/archives/dev/2014-June/003607.html + +* In Ubuntu environment, QEMU fail to start a new guest normally with user space VHOST due to hug pages can't be +allocated for the new guest.* + +The solution for this issue could be adding "-boot c" into QEMU command line to make sure the huge pages are +allocated properly and then the guest will startup normally. + +Use "cat /proc/meminfo" to check if there is any change in value of HugePages_Total and HugePages_Free after the +guest startup. + +* Logging message: "eventfd_link: module verification failed: signature and/or required key missing - tainting kernel"* + +Ignore the above logging message. The message occurs due to the new module eventfd_link, which is not a standard +module of Linux, but it is necessary for user space VHOST current implementation(CUSE-based) to communicate with +the guest. Running DPDK in the Virtual Machine --- -- 1.8.4.2
[dpdk-dev] [PATCH] librte_eal/common: Fix cast from pointer to integer of different size
On 3/2/2015 7:39 PM, Gonzalez Monroy, Sergio wrote: > On 02/03/2015 07:27, Michael Qiu wrote: >> /i686-native-linuxapp-gcc/include/rte_memcpy.h:592:23: error: >> cast from pointer to integer of different size >> [-Werror=pointer-to-int-cast] >> >>dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; >> >> Type 'long long' is 64-bit in i686 platform while 'void *' >> is 32-bit. >> >> Signed-off-by: Michael Qiu >> --- >> lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h >> b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h >> index 7b2d382..6565c00 100644 >> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h >> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h >> @@ -589,12 +589,12 @@ COPY_BLOCK_64_BACK15: >> * unaligned copy functions require up to 15 bytes >> * backwards access. >> */ >> -dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; >> +dstofss = 16 - (int)((long)(void *)dst & 0x0F) + 16; > You may as well remove the (void *) casting, I don't think it is necessary. Yes, you are right. The original type is (void *). Thanks, Michael >> n -= dstofss; >> rte_mov32((uint8_t *)dst, (const uint8_t *)src); >> src = (const uint8_t *)src + dstofss; >> dst = (uint8_t *)dst + dstofss; >> -srcofs = (int)((long long)(const void *)src & 0x0F); >> +srcofs = (int)((long)(const void *)src & 0x0F); > Same here for (const void *) > > Sergio >> >> /** >> * For aligned copy >
[dpdk-dev] [PATCH v2] librte_eal/common: Fix cast from pointer to integer of different size
/i686-native-linuxapp-gcc/include/rte_memcpy.h:592:23: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; Type 'long long' is 64-bit in i686 platform while 'void *' is 32-bit. Signed-off-by: Michael Qiu --- v2 --> v1: Remove unnecessary casting (void *) lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h index 7b2d382..85a5f4d 100644 --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h @@ -589,12 +589,12 @@ COPY_BLOCK_64_BACK15: * unaligned copy functions require up to 15 bytes * backwards access. */ - dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; + dstofss = 16 - (int)((long)dst & 0x0F) + 16; n -= dstofss; rte_mov32((uint8_t *)dst, (const uint8_t *)src); src = (const uint8_t *)src + dstofss; dst = (uint8_t *)dst + dstofss; - srcofs = (int)((long long)(const void *)src & 0x0F); + srcofs = (int)((long)src & 0x0F); /** * For aligned copy -- 1.9.3
[dpdk-dev] [PATCH] doc: New option for vhost sample
Since codes for this new option(--vlan-strip) are already merged into mainline, see: commit: e3d61d1609cb9b3ea851c7776bfbb60dcfe8844a. So the doc need update for that. Signed-off-by: Changchun Ouyang --- doc/guides/sample_app_ug/vhost.rst | 8 1 file changed, 8 insertions(+) diff --git a/doc/guides/sample_app_ug/vhost.rst b/doc/guides/sample_app_ug/vhost.rst index 76997da..b97ca6d 100644 --- a/doc/guides/sample_app_ug/vhost.rst +++ b/doc/guides/sample_app_ug/vhost.rst @@ -468,6 +468,14 @@ The value is 64 by default. user at target:~$ ./build/app/vhost-switch -c f -n 4 --huge-dir /mnt/huge -- --zero-copy 1 --tx-desc-num [0, n] +**VLAN strip.** +The VLAN strip option enable/disable the VLAN strip on host, if disabled, the guest will receive the packets with VLAN tag. +It is enabled by default. + +.. code-block:: console + +user at target:~$ ./build/app/vhost-switch -c f -n 4 --huge-dir /mnt/huge -- --vlan-strip [0, 1] + Running the Virtual Machine (QEMU) -- -- 1.8.4.2
[dpdk-dev] [PATCH] lib/librte_vhost: remove vhost device from data plane when receive VHOST_SET_MEM_TABLE message
This patch fixes the segfault issue in the case vhost receives new
VHOST_SET_MEM_TABLE message without VHOST_VRING_GET_VRING_BASE(which we uses as
the stop message).
Signed-off-by: Huawei Xie
---
lib/librte_vhost/vhost_user/virtio-net-user.c | 4
1 file changed, 4 insertions(+)
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c
b/lib/librte_vhost/vhost_user/virtio-net-user.c
index 97c5177..aa08706 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -109,6 +109,10 @@ user_set_mem_table(struct vhost_device_ctx ctx, struct
VhostUserMsg *pmsg)
if (dev == NULL)
return -1;
+ /* Remove from the data plane. */
+ if (dev->flags & VIRTIO_DEV_RUNNING)
+ notify_ops->destroy_device(dev);
+
if (dev->mem) {
free_mem_region(dev);
free(dev->mem);
--
1.8.1.4
[dpdk-dev] [PATCH] doc: correct the format of quota
remove the coma character by using ' character Signed-off-by: Jingjing Wu --- doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst index a99e14d..b6dd5bd 100644 --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst @@ -1513,7 +1513,7 @@ Configure flexible payload selection. flow_director_flex_payload (port_id) (raw|l2|l3|l4) (config) -For example, to select the first 16 bytes from the offset 4 (bytes) of packet?s payload as flexible payload. +For example, to select the first 16 bytes from the offset 4 (bytes) of packet's payload as flexible payload. .. code-block:: console -- 1.9.3
[dpdk-dev] [PATCH v2] librte_eal/common: Fix cast from pointer to integer of different size
On 2015-03-03 03:20, Michael Qiu wrote: > /i686-native-linuxapp-gcc/include/rte_memcpy.h:592:23: error: > cast from pointer to integer of different size > [-Werror=pointer-to-int-cast] > >dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; > > Type 'long long' is 64-bit in i686 platform while 'void *' > is 32-bit. > > Signed-off-by: Michael Qiu > --- > v2 --> v1: > Remove unnecessary casting (void *) > > lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h > b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h > index 7b2d382..85a5f4d 100644 > --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h > +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h > @@ -589,12 +589,12 @@ COPY_BLOCK_64_BACK15: >* unaligned copy functions require up to 15 bytes >* backwards access. >*/ > - dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; > + dstofss = 16 - (int)((long)dst & 0x0F) + 16; > n -= dstofss; > rte_mov32((uint8_t *)dst, (const uint8_t *)src); > src = (const uint8_t *)src + dstofss; > dst = (uint8_t *)dst + dstofss; > - srcofs = (int)((long long)(const void *)src & 0x0F); > + srcofs = (int)((long)src & 0x0F); > > /** >* For aligned copy > I think you should use here size_t, (u)ptrdiff_t or (u)intptr_t as this will be more portable. Also type of dstofss and srcofs should be changed accordingly. -- Pawel
[dpdk-dev] [PATCH] eal/linux: fix build
Compilation fails in some distributions because of missing unistd.h
needed for pread/pwrite (seen with Suse):
lib/librte_eal/linuxapp/eal/eal_pci_uio.c:62:2:
error: implicit declaration of function ?pread?
Fixes: 4a499c649590 ("eal/linux: enable uio_pci_generic support")
Signed-off-by: Thomas Monjalon
---
lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 35d31c5..0b397ca 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -32,6 +32,7 @@
*/
#include
+#include
#include
#include
#include
--
2.2.2
[dpdk-dev] [PATCH] doc: Update doc for ixgbe VF RSS
Since the following commit has already been merged into dpdk.org mainline,
commit: 42d2f78abcb77ecb769be4149df550308169ef0f
So update the prog guide for it.
Signed-off-by: Changchun Ouyang
---
.../prog_guide/i40e_ixgbe_igb_virt_func_drv.rst| 38 ++
1 file changed, 38 insertions(+)
diff --git a/doc/guides/prog_guide/i40e_ixgbe_igb_virt_func_drv.rst
b/doc/guides/prog_guide/i40e_ixgbe_igb_virt_func_drv.rst
index 41e316e..3039ba6 100755
--- a/doc/guides/prog_guide/i40e_ixgbe_igb_virt_func_drv.rst
+++ b/doc/guides/prog_guide/i40e_ixgbe_igb_virt_func_drv.rst
@@ -172,6 +172,44 @@ For example,
Launch the DPDK testpmd/example or your own host daemon application using
the DPDK PMD library.
+* Using the DPDK PMD PF ixgbe driver to enable VF RSS:
+
+Same steps as above to install the modules of uio, igb_uio, specify
max_vfs for PCI device, and
+launch the DPDK testpmd/example or your own host daemon application using
the DPDK PMD library.
+
+The available queue number(at most 4) per VF depends on the total number
of pool, which is
+determined by the max number of VF at PF initialization stage and the
number of queue specified
+in config:
+
+* If the max number of VF is set in the range of 1 to 32:
+
+If the number of rxq is specified as 4(e.g. '--rxq 4' in testpmd),
then there are totally 32
+pools(ETH_32_POOLS), and each VF could have 4 or less(e.g. 2) queues;
+
+If the number of rxq is specified as 2(e.g. '--rxq 2' in testpmd),
then there are totally 32
+pools(ETH_32_POOLS), and each VF could have 2 queues;
+
+* If the max number of VF is in the range of 33 to 64:
+
+If the number of rxq is 4 ('--rxq 4' in testpmd), then error message
is expected as rxq is not
+correct at this case;
+
+If the number of rxq is 2 ('--rxq 2' in testpmd), then there is
totally 64 pools(ETH_64_POOLS),
+and each VF have 2 queues;
+
+On host, to enable VF RSS functionality, rx mq mode should be set as
ETH_MQ_RX_VMDQ_RSS
+or ETH_MQ_RX_RSS mode, and SRIOV mode should be activated(max_vfs >= 1).
+It also needs config VF RSS information like hash function, RSS key, RSS
key length.
+
+.. code-block:: console
+
+testpmd -c 0x -n 4 -- --coremask= --rxq=4 --txq=4 -i
+
+The limitation for VF RSS on Intel? 82599 10 Gigabit Ethernet Controller
is:
+The hash and key are shared among PF and all VF, the RETA table with 128
entries is also shared
+among PF and all VF; So it could not to provide a method to query the hash
and reta content per
+VF on guest, while, if possible, please query them on host(PF) for the
shared RETA information.
+
Virtual Function enumeration is performed in the following sequence by the
Linux* pci driver for a dual-port NIC.
When you enable the four Virtual Functions with the above command, the four
enabled functions have a Function#
represented by (Bus#, Device#, Function#) in sequence starting from 0 to 3.
--
1.8.4.2
[dpdk-dev] [PATCH] eal/linux: fix build
On Tue, Mar 3, 2015 at 9:44 AM, Thomas Monjalon
wrote:
> Compilation fails in some distributions because of missing unistd.h
> needed for pread/pwrite (seen with Suse):
> lib/librte_eal/linuxapp/eal/eal_pci_uio.c:62:2:
> error: implicit declaration of function ?pread?
>
> Fixes: 4a499c649590 ("eal/linux: enable uio_pci_generic support")
>
> Signed-off-by: Thomas Monjalon
> ---
> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> index 35d31c5..0b397ca 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> @@ -32,6 +32,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
>
>
Acked-by: David Marchand
--
David Marchand
[dpdk-dev] [PATCH] ixgbe: fix data access on big endian cpu
From: Xuelin Shi
enforce rules for cpu and ixgbe exchanging data.
1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...)
2. cpu fill data to ixgbe must use rte_cpu_to_le_xx(...)
Signed-off-by: Xuelin Shi
---
lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 115 --
1 file changed, 72 insertions(+), 43 deletions(-)
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index e6766b3..fb01a4a 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -140,7 +140,7 @@ ixgbe_tx_free_bufs(struct igb_tx_queue *txq)
int i;
/* check DD bit on threshold descriptor */
- status = txq->tx_ring[txq->tx_next_dd].wb.status;
+ status = rte_le_to_cpu_32(txq->tx_ring[txq->tx_next_dd].wb.status);
if (! (status & IXGBE_ADVTXD_STAT_DD))
return 0;
@@ -186,11 +186,14 @@ tx4(volatile union ixgbe_adv_tx_desc *txdp, struct
rte_mbuf **pkts)
pkt_len = (*pkts)->data_len;
/* write data to descriptor */
- txdp->read.buffer_addr = buf_dma_addr;
+ txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
+
txdp->read.cmd_type_len =
- ((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
+ rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
+
txdp->read.olinfo_status =
- (pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+ rte_cpu_to_le_32(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+
rte_prefetch0(&(*pkts)->pool);
}
}
@@ -206,11 +209,14 @@ tx1(volatile union ixgbe_adv_tx_desc *txdp, struct
rte_mbuf **pkts)
pkt_len = (*pkts)->data_len;
/* write data to descriptor */
- txdp->read.buffer_addr = buf_dma_addr;
+ txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
+
txdp->read.cmd_type_len =
- ((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
+ rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
+
txdp->read.olinfo_status =
- (pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+ rte_cpu_to_le_32(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+
rte_prefetch0(&(*pkts)->pool);
}
@@ -297,7 +303,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
* a divisor of the ring size
*/
tx_r[txq->tx_next_rs].read.cmd_type_len |=
- rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
+ rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
txq->tx_next_rs = (uint16_t)(txq->tx_rs_thresh - 1);
txq->tx_tail = 0;
@@ -316,7 +322,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
*/
if (txq->tx_tail > txq->tx_next_rs) {
tx_r[txq->tx_next_rs].read.cmd_type_len |=
- rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
+ rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
txq->tx_next_rs = (uint16_t)(txq->tx_next_rs +
txq->tx_rs_thresh);
if (txq->tx_next_rs >= txq->nb_tx_desc)
@@ -517,6 +523,7 @@ ixgbe_xmit_cleanup(struct igb_tx_queue *txq)
uint16_t nb_tx_desc = txq->nb_tx_desc;
uint16_t desc_to_clean_to;
uint16_t nb_tx_to_clean;
+ uint32_t stat;
/* Determine the last descriptor needing to be cleaned */
desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq->tx_rs_thresh);
@@ -525,7 +532,9 @@ ixgbe_xmit_cleanup(struct igb_tx_queue *txq)
/* Check to make sure the last descriptor to clean is done */
desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
- if (! (txr[desc_to_clean_to].wb.status & IXGBE_TXD_STAT_DD))
+
+ stat = rte_le_to_cpu_32(txr[desc_to_clean_to].wb.status);
+ if (! (stat & IXGBE_TXD_STAT_DD))
{
PMD_TX_FREE_LOG(DEBUG,
"TX descriptor %4u is not done"
@@ -556,7 +565,7 @@ ixgbe_xmit_cleanup(struct igb_tx_queue *txq)
* up to the last descriptor with the RS bit set
* are done. Only reset the threshold descriptor.
*/
- txr[desc_to_clean_to].wb.status = 0;
+ txr[desc_to_clean_to].wb.status = rte_cpu_to_le_32(0);
/* Update the txq to reflect the last descriptor that was cleaned */
txq->last_desc_cleaned = desc_to_clean_to;
@@ -813,12 +822,14 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
*/
slen = m_seg->data_len;
buf_dma_addr = RTE_MBUF_DATA_DMA_ADDR(m_seg);
+
txd->read.buffer_addr =
- rte_cpu_to_le_64(buf_dma_addr);
+ rte_cpu_to_le_64(buf_dma_addr);
txd-
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: > > On 22/02/15 12:51, Marc Sune wrote: > >I don't like the proposed patch, but I am recovering this old thread > >because I agree on the problem statement. > > > >On 04/04/14 11:57, Ananyev, Konstantin wrote: > >>Hi Cyril, > >>We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you can use > >>to enable debug, or any other compiler/linker options you need. > >>Wonder, why that is not enough? > > > >EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why > >setting individually: > > > >diff --git a/config/common_linuxapp b/config/common_linuxapp > >index 2f9643b..04adc0d 100644 > >--- a/config/common_linuxapp > >+++ b/config/common_linuxapp > >@@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y > > # Compile generic ethernet library > > # > > CONFIG_RTE_LIBRTE_ETHER=y > >-CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n > >+CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y > > > > > >to put an example, does not set -g and -O0 in that particular module only. > >No one would ever use something compiled in DEBUG in production anyway. > > > >I always end up modifying manually Makefiles in the lib library that I am > >interested in having insides, overriding CFLAGS=-O3, which is not that > >nice. > > I would like some feedback on this idea. If the community sees benefit, I > will work on a patch for this. > > Marc > So, your proposal is to patch things so that any time one sets DEBUG=y in the build-time config for a library, we change the '-O3' to '-O0' and set -g also. Correct? If that's the case, I see no harm in doing such a thing. I wonder how useful the '-g' option would be, just limited to a single library, though. One would suspect that it may be better applied globally, so that one can see the state of the application as it makes the call into the library. /Bruce > > > >Marc > > > >>Thanks > >>Konstantin > >> > >>-Original Message- > >>From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Cyril Chemparathy > >>Sent: Thursday, April 03, 2014 6:31 PM > >>To: dev at dpdk.org > >>Subject: [dpdk-dev] [PATCH] mk: add support for gdb debug info > >>generation > >> > >>It is often useful to build with debug enabled, we add a config > >>(CONFIG_RTE_TOOLCHAIN_DEBUG) to do so. > >> > >>Note: This patch does not include corresponding changes for ICC. The > >>author pleads abject ignorance in this regard, and welcomes > >>recommendations. :-) > >> > >>Signed-off-by: Cyril Chemparathy > >>--- > >> config/defconfig_x86_64-default-linuxapp-gcc | 1 + > >> mk/toolchain/gcc/rte.vars.mk | 5 + > >> 2 files changed, 6 insertions(+) > >> > >>diff --git a/config/defconfig_x86_64-default-linuxapp-gcc > >>b/config/defconfig_x86_64-default-linuxapp-gcc > >>index f11ffbf..3b36efd 100644 > >>--- a/config/defconfig_x86_64-default-linuxapp-gcc > >>+++ b/config/defconfig_x86_64-default-linuxapp-gcc > >>@@ -67,6 +67,7 @@ CONFIG_RTE_ARCH_X86_64=y # CONFIG_RTE_TOOLCHAIN="gcc" > >> CONFIG_RTE_TOOLCHAIN_GCC=y > >>+CONFIG_RTE_TOOLCHAIN_DEBUG=n > >># > >> # Use intrinsics or assembly code for key routines diff --git > >>a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk index > >>0edb93f..81ed3fa 100644 > >>--- a/mk/toolchain/gcc/rte.vars.mk > >>+++ b/mk/toolchain/gcc/rte.vars.mk > >>@@ -68,6 +68,11 @@ ifeq (,$(findstring -O0,$(EXTRA_CFLAGS))) endif > >>endif > >> +ifeq ($(CONFIG_RTE_TOOLCHAIN_DEBUG),y) > >>+TOOLCHAIN_CFLAGS += -g -ggdb > >>+TOOLCHAIN_LDFLAGS += -g -ggdb > >>+endif > >>+ > >> WERROR_FLAGS := -W -Wall -Werror -Wstrict-prototypes > >>-Wmissing-prototypes WERROR_FLAGS += -Wmissing-declarations > >>-Wold-style-definition -Wpointer-arith WERROR_FLAGS += -Wcast-align > >>-Wnested-externs -Wcast-qual > >>-- > >>1.8.3.1 > >> > > >
[dpdk-dev] [PATCH v2] librte_eal/common: Fix cast from pointer to integer of different size
On 3/3/2015 4:25 PM, Wodkowski, PawelX wrote: > On 2015-03-03 03:20, Michael Qiu wrote: >> /i686-native-linuxapp-gcc/include/rte_memcpy.h:592:23: error: >> cast from pointer to integer of different size >> [-Werror=pointer-to-int-cast] >> >>dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; >> >> Type 'long long' is 64-bit in i686 platform while 'void *' >> is 32-bit. >> >> Signed-off-by: Michael Qiu >> --- >> v2 --> v1: >> Remove unnecessary casting (void *) >> >> lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h >> b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h >> index 7b2d382..85a5f4d 100644 >> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h >> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h >> @@ -589,12 +589,12 @@ COPY_BLOCK_64_BACK15: >> * unaligned copy functions require up to 15 bytes >> * backwards access. >> */ >> -dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; >> +dstofss = 16 - (int)((long)dst & 0x0F) + 16; >> n -= dstofss; >> rte_mov32((uint8_t *)dst, (const uint8_t *)src); >> src = (const uint8_t *)src + dstofss; >> dst = (uint8_t *)dst + dstofss; >> -srcofs = (int)((long long)(const void *)src & 0x0F); >> +srcofs = (int)((long)src & 0x0F); >> >> /** >> * For aligned copy >> > I think you should use here size_t, (u)ptrdiff_t or (u)intptr_t as this Yes, but 'long' is enough, does it limit anything, or has any issue with multiple platforms? > will be more portable. > Also type of dstofss and srcofs should be changed accordingly. No, I think, it should be offset. Thanks, Michael >
[dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload
Thanks. What's next with the suggested parch? On Mar 2, 2015, at 3:29 PM, Thomas Monjalon wrote: 2015-03-02 13:58, Raz Amir: > BTW, I don't see it in incoming patches page at > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > Is it because I missed something in the code contribution instruction? It's here: http://dpdk.org/dev/patchwork/patch/3800/
[dpdk-dev] [PATCH] tools/dpdk_nic_bind: Fix can't bind virtio-pci issue
On Thu, Feb 26, 2015 at 12:57:49PM +0800, Ouyang Changchun wrote: First off, I think the title needs to be changed. How about something like: "dpdk_nic_bind: don't exit if an unused module is missing" > In virtio test, on guest > 1. Bind virtio port to igb_uio driver; > 2. Remove igb_uio module; > 3. Bind virtio port to virtio-pci driver, it fails and reports: >"Error - no supported modules are loaded" > > The tool should check the to-be-bound driver flag, if it is dpdk > driver(igb_uio, vfio etc), > and the corresponding module is not loaded, then exit, otherwise, just report > a warning, > and continue to bind the non-dpdk driver(like virtio-pci) to dev. This is a good description of the problem. Once you change the title, I think an initial sentence is needed here by way of intro - such as: "The dpdk_nic_bind script will not allow ports to be bound or unbound if none of the kernel modules supported by DPDK is loaded. This patch relaxes this restriction by checking if a DPDK module is actually requested. The example below illustrates this problem:" > > Signed-off-by: Changchun Ouyang > --- > tools/dpdk_nic_bind.py | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py > index 2483056..8523f82 100755 > --- a/tools/dpdk_nic_bind.py > +++ b/tools/dpdk_nic_bind.py > @@ -175,8 +175,11 @@ def check_modules(): > > # check if we have at least one loaded module > if True not in [mod["Found"] for mod in mods] and b_flag is not None: > -print "Error - no supported modules are loaded" > -sys.exit(1) > +if b_flag in dpdk_drivers: > +print "Error - no supported modules(DPDK driver) are loaded" > +sys.exit(1) > +else: > +print "Warning - no supported modules(DPDK driver) are loaded" > > # change DPDK driver list to only contain drivers that are loaded > dpdk_drivers = [mod["Name"] for mod in mods if mod["Found"]] > -- > 1.8.4.2 > This is a worthwhile change. However, I think it could be made better: * If we try to bind a device to ixgbe, and ixgbe is not loaded, we get no error from the script * If igb_uio is loaded and we try to bind a device to vfio which is not loaded again we get no error from the script. If would be nice if instead of this checking explicitly for DPDK modules, we just check if the module to be bound is present and give an error if not. /Bruce
[dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload
On Tue, Mar 03, 2015 at 01:30:29PM +0200, Raz Amir wrote: > Thanks. > What's next with the suggested parch? > Hi, This patch needs to be reviewed and acked before it can be merged. I'll take a look at it and test it today, I hope, if nobody else reviews and comments on it before I get to it. Regards, /Bruce > On Mar 2, 2015, at 3:29 PM, Thomas Monjalon > wrote: > > 2015-03-02 13:58, Raz Amir: > > BTW, I don't see it in incoming patches page at > > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > > Is it because I missed something in the code contribution instruction? > > It's here: http://dpdk.org/dev/patchwork/patch/3800/ >
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
On 03/03/15 10:33, Bruce Richardson wrote: > On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: >> On 22/02/15 12:51, Marc Sune wrote: >>> I don't like the proposed patch, but I am recovering this old thread >>> because I agree on the problem statement. >>> >>> On 04/04/14 11:57, Ananyev, Konstantin wrote: Hi Cyril, We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you can use to enable debug, or any other compiler/linker options you need. Wonder, why that is not enough? >>> EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why >>> setting individually: >>> >>> diff --git a/config/common_linuxapp b/config/common_linuxapp >>> index 2f9643b..04adc0d 100644 >>> --- a/config/common_linuxapp >>> +++ b/config/common_linuxapp >>> @@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y >>> # Compile generic ethernet library >>> # >>> CONFIG_RTE_LIBRTE_ETHER=y >>> -CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n >>> +CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y >>> >>> >>> to put an example, does not set -g and -O0 in that particular module only. >>> No one would ever use something compiled in DEBUG in production anyway. >>> >>> I always end up modifying manually Makefiles in the lib library that I am >>> interested in having insides, overriding CFLAGS=-O3, which is not that >>> nice. >> I would like some feedback on this idea. If the community sees benefit, I >> will work on a patch for this. >> >> Marc >> > So, your proposal is to patch things so that any time one sets DEBUG=y in the > build-time config for a library, we change the '-O3' to '-O0' and set -g also. > Correct? I am not sure what you mean by 'patch things'. I would simply enable the build system to override the default compilation flags (now DPDK-wide, or specifically librte_ wide) when _DEBUG=y for a library, changing compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I have to check if -O0 already implicitly means -fno-inline (even for __attribute__((always_inline)) ). I did a quick test. I chose KNI because it didn't have a DEBUG flag for the user-space library. For other libraries, the existing _DEBUG setting would be enough: marc at dpdk:~/dpdk$ git diff HEAD diff --git a/config/common_linuxapp b/config/common_linuxapp index 97f1c9e..8a3cef8 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y # CONFIG_RTE_LIBRTE_KNI=y CONFIG_RTE_KNI_PREEMPT_DEFAULT=y +CONFIG_RTE_LIBRTE_KNI_DEBUG=y CONFIG_RTE_KNI_KO_DEBUG=n CONFIG_RTE_KNI_VHOST=n CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile index 7107832..895f64e 100644 --- a/lib/librte_kni/Makefile +++ b/lib/librte_kni/Makefile @@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk # library name LIB = librte_kni.a -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) EXPORT_MAP := rte_kni_version.map diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 63a41e2..eee477d 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -78,6 +78,13 @@ endif ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) LDLIBS += -lrte_kni + +ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline +else +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing +endif + endif endif Thoughts? > > If that's the case, I see no harm in doing such a thing. I wonder how useful > the '-g' option would be, just limited to a single library, though. One would > suspect that it may be better applied globally, so that one can see the state > of the application as it makes the call into the library. It is IMHO. I am usually doing it this way, specially for ethdev initialization and the specific pmd (we probably need more fine grained return codes there). If you enable the debug for that library, you generally want to check the code and the state there, and you can access the input parameters of the function and the state of that library itself, which is generally enough, I think. Special cases are inline functions used by other libraries / user application, which are compiled either with the global DPDK flags (-O3) or by the user's application flags. Perhaps it also makes sense, in addition, to have a global "DEBUG" knob, which would enable to compile the entire DPDK library code with -O0 -g and possibly also with -fno-inline. This would also help debugging the inline functions. Marc > > /Bruce > >>> Marc >>> Thanks Konstantin -Original Message- From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Cyril Chemparathy Sent: Thursday, April 03, 2014 6:31 PM To: dev at dpdk.org Subject: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation It is often useful to build with debug enabled, we add a config (CONFIG_RTE_TOOLCH
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
On 03/03/2015 02:19 PM, Marc Sune wrote: > > On 03/03/15 10:33, Bruce Richardson wrote: >> On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: >>> On 22/02/15 12:51, Marc Sune wrote: I don't like the proposed patch, but I am recovering this old thread because I agree on the problem statement. On 04/04/14 11:57, Ananyev, Konstantin wrote: > Hi Cyril, > We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you can use > to enable debug, or any other compiler/linker options you need. > Wonder, why that is not enough? EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why setting individually: diff --git a/config/common_linuxapp b/config/common_linuxapp index 2f9643b..04adc0d 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y # Compile generic ethernet library # CONFIG_RTE_LIBRTE_ETHER=y -CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n +CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y to put an example, does not set -g and -O0 in that particular module only. No one would ever use something compiled in DEBUG in production anyway. I always end up modifying manually Makefiles in the lib library that I am interested in having insides, overriding CFLAGS=-O3, which is not that nice. >>> I would like some feedback on this idea. If the community sees >>> benefit, I >>> will work on a patch for this. >>> >>> Marc >>> >> So, your proposal is to patch things so that any time one sets DEBUG=y >> in the >> build-time config for a library, we change the '-O3' to '-O0' and set >> -g also. >> Correct? > > I am not sure what you mean by 'patch things'. I would simply enable the > build system to override the default compilation flags (now DPDK-wide, > or specifically librte_ wide) when _DEBUG=y for a library, changing > compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I > have to check if -O0 already implicitly means -fno-inline (even for > __attribute__((always_inline)) ). > > I did a quick test. I chose KNI because it didn't have a DEBUG flag for > the user-space library. For other libraries, the existing _DEBUG setting > would be enough: > > marc at dpdk:~/dpdk$ git diff HEAD > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 97f1c9e..8a3cef8 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > # > CONFIG_RTE_LIBRTE_KNI=y > CONFIG_RTE_KNI_PREEMPT_DEFAULT=y > +CONFIG_RTE_LIBRTE_KNI_DEBUG=y > CONFIG_RTE_KNI_KO_DEBUG=n > CONFIG_RTE_KNI_VHOST=n > CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile > index 7107832..895f64e 100644 > --- a/lib/librte_kni/Makefile > +++ b/lib/librte_kni/Makefile > @@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > # library name > LIB = librte_kni.a > > -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing > +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) > > EXPORT_MAP := rte_kni_version.map > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk > index 63a41e2..eee477d 100644 > --- a/mk/rte.app.mk > +++ b/mk/rte.app.mk > @@ -78,6 +78,13 @@ endif > ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > LDLIBS += -lrte_kni > + > +ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) > +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline > +else > +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing > +endif > + > endif > endif > > > Thoughts? My 5c is that if anything, DPDK needs *less* places that muck around with compiler flags, not more. If you something like this for all the libraries in DPDK the number doesn't just increase a bit, it explodes. I dont see that much point in this thing, but I'd approach it by defining the debug flags someplace central, say DEBUG_FLAGS, and append that to the common cflags when *_DEBUG config is enabled. At least with gcc the last option wins so if you just append -O0 when debugging then that's what wins, the earlier -O3 does not matter. - Panu - - Panu -
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
On 03/03/15 13:40, Panu Matilainen wrote: > On 03/03/2015 02:19 PM, Marc Sune wrote: >> >> On 03/03/15 10:33, Bruce Richardson wrote: >>> On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: On 22/02/15 12:51, Marc Sune wrote: > I don't like the proposed patch, but I am recovering this old thread > because I agree on the problem statement. > > On 04/04/14 11:57, Ananyev, Konstantin wrote: >> Hi Cyril, >> We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you >> can use >> to enable debug, or any other compiler/linker options you need. >> Wonder, why that is not enough? > EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why > setting individually: > > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 2f9643b..04adc0d 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y > # Compile generic ethernet library > # > CONFIG_RTE_LIBRTE_ETHER=y > -CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n > +CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y > > > to put an example, does not set -g and -O0 in that particular module > only. > No one would ever use something compiled in DEBUG in production > anyway. > > I always end up modifying manually Makefiles in the lib library that > I am > interested in having insides, overriding CFLAGS=-O3, which is not > that > nice. I would like some feedback on this idea. If the community sees benefit, I will work on a patch for this. Marc >>> So, your proposal is to patch things so that any time one sets DEBUG=y >>> in the >>> build-time config for a library, we change the '-O3' to '-O0' and set >>> -g also. >>> Correct? >> >> I am not sure what you mean by 'patch things'. I would simply enable the >> build system to override the default compilation flags (now DPDK-wide, >> or specifically librte_ wide) when _DEBUG=y for a library, changing >> compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I >> have to check if -O0 already implicitly means -fno-inline (even for >> __attribute__((always_inline)) ). >> >> I did a quick test. I chose KNI because it didn't have a DEBUG flag for >> the user-space library. For other libraries, the existing _DEBUG setting >> would be enough: >> >> marc at dpdk:~/dpdk$ git diff HEAD >> diff --git a/config/common_linuxapp b/config/common_linuxapp >> index 97f1c9e..8a3cef8 100644 >> --- a/config/common_linuxapp >> +++ b/config/common_linuxapp >> @@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y >> # >> CONFIG_RTE_LIBRTE_KNI=y >> CONFIG_RTE_KNI_PREEMPT_DEFAULT=y >> +CONFIG_RTE_LIBRTE_KNI_DEBUG=y >> CONFIG_RTE_KNI_KO_DEBUG=n >> CONFIG_RTE_KNI_VHOST=n >> CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 >> diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile >> index 7107832..895f64e 100644 >> --- a/lib/librte_kni/Makefile >> +++ b/lib/librte_kni/Makefile >> @@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk >> # library name >> LIB = librte_kni.a >> >> -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing >> +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) >> >> EXPORT_MAP := rte_kni_version.map >> >> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >> index 63a41e2..eee477d 100644 >> --- a/mk/rte.app.mk >> +++ b/mk/rte.app.mk >> @@ -78,6 +78,13 @@ endif >> ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >> LDLIBS += -lrte_kni >> + >> +ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) >> +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline >> +else >> +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing >> +endif >> + >> endif >> endif >> >> >> Thoughts? > > My 5c is that if anything, DPDK needs *less* places that muck around > with compiler flags, not more. If you something like this for all the > libraries in DPDK the number doesn't just increase a bit, it explodes. If you check the part below this one in my original email, that you stripped out (without notice), the suggestion was also to add a global _DEBUG parameter for the entire DPDK set of libraries, to change all the CFLAGS at once (not in the attached PATCH). > > I dont see that much point in this thing, but I'd approach it by > defining the debug flags someplace central, say DEBUG_FLAGS, and > append that to the common cflags when *_DEBUG config is enabled. At > least with gcc the last option wins so if you just append -O0 when > debugging then that's what wins, the earlier -O3 does not matter. The original problem is the one you expose; libraries hardcode the CFLAGS, ignoring user-flags. There is no way to change this unless you change the Makefiles directly. But right now, each library does hardcode its *own* flags (check Makefiles for the libraries), so there is already not a unified approach here. I see for insta
[dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload
Thank you -Original Message- From: Bruce Richardson [mailto:bruce.richard...@intel.com] Sent: 03 March 2015 13:45 To: Raz Amir Cc: Thomas Monjalon; dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload On Tue, Mar 03, 2015 at 01:30:29PM +0200, Raz Amir wrote: > Thanks. > What's next with the suggested parch? > Hi, This patch needs to be reviewed and acked before it can be merged. I'll take a look at it and test it today, I hope, if nobody else reviews and comments on it before I get to it. Regards, /Bruce > On Mar 2, 2015, at 3:29 PM, Thomas Monjalon wrote: > > 2015-03-02 13:58, Raz Amir: > > BTW, I don't see it in incoming patches page at > > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > > Is it because I missed something in the code contribution instruction? > > It's here: http://dpdk.org/dev/patchwork/patch/3800/ >
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
On Tue, Mar 03, 2015 at 01:56:19PM +0100, Marc Sune wrote: > > On 03/03/15 13:40, Panu Matilainen wrote: > >On 03/03/2015 02:19 PM, Marc Sune wrote: > >> > >>On 03/03/15 10:33, Bruce Richardson wrote: > >>>On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: > On 22/02/15 12:51, Marc Sune wrote: > >I don't like the proposed patch, but I am recovering this old thread > >because I agree on the problem statement. > > > >On 04/04/14 11:57, Ananyev, Konstantin wrote: > >>Hi Cyril, > >>We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you > >>can use > >>to enable debug, or any other compiler/linker options you need. > >>Wonder, why that is not enough? > >EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why > >setting individually: > > > >diff --git a/config/common_linuxapp b/config/common_linuxapp > >index 2f9643b..04adc0d 100644 > >--- a/config/common_linuxapp > >+++ b/config/common_linuxapp > >@@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y > ># Compile generic ethernet library > ># > >CONFIG_RTE_LIBRTE_ETHER=y > >-CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n > >+CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y > > > > > >to put an example, does not set -g and -O0 in that particular module > >only. > >No one would ever use something compiled in DEBUG in production > >anyway. > > > >I always end up modifying manually Makefiles in the lib library that > >I am > >interested in having insides, overriding CFLAGS=-O3, which is not > >that > >nice. > I would like some feedback on this idea. If the community sees > benefit, I > will work on a patch for this. > > Marc > > >>>So, your proposal is to patch things so that any time one sets DEBUG=y > >>>in the > >>>build-time config for a library, we change the '-O3' to '-O0' and set > >>>-g also. > >>>Correct? > >> > >>I am not sure what you mean by 'patch things'. I would simply enable the > >>build system to override the default compilation flags (now DPDK-wide, > >>or specifically librte_ wide) when _DEBUG=y for a library, changing > >>compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I > >>have to check if -O0 already implicitly means -fno-inline (even for > >>__attribute__((always_inline)) ). > >> > >>I did a quick test. I chose KNI because it didn't have a DEBUG flag for > >>the user-space library. For other libraries, the existing _DEBUG setting > >>would be enough: > >> > >>marc at dpdk:~/dpdk$ git diff HEAD > >>diff --git a/config/common_linuxapp b/config/common_linuxapp > >>index 97f1c9e..8a3cef8 100644 > >>--- a/config/common_linuxapp > >>+++ b/config/common_linuxapp > >>@@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > >> # > >> CONFIG_RTE_LIBRTE_KNI=y > >> CONFIG_RTE_KNI_PREEMPT_DEFAULT=y > >>+CONFIG_RTE_LIBRTE_KNI_DEBUG=y > >> CONFIG_RTE_KNI_KO_DEBUG=n > >> CONFIG_RTE_KNI_VHOST=n > >> CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > >>diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile > >>index 7107832..895f64e 100644 > >>--- a/lib/librte_kni/Makefile > >>+++ b/lib/librte_kni/Makefile > >>@@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > >> # library name > >> LIB = librte_kni.a > >> > >>-CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing > >>+CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) > >> > >> EXPORT_MAP := rte_kni_version.map > >> > >>diff --git a/mk/rte.app.mk b/mk/rte.app.mk > >>index 63a41e2..eee477d 100644 > >>--- a/mk/rte.app.mk > >>+++ b/mk/rte.app.mk > >>@@ -78,6 +78,13 @@ endif > >> ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > >> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > >> LDLIBS += -lrte_kni > >>+ > >>+ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) > >>+CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline > >>+else > >>+CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing > >>+endif > >>+ > >> endif > >> endif > >> > >> > >>Thoughts? > > > >My 5c is that if anything, DPDK needs *less* places that muck around with > >compiler flags, not more. If you something like this for all the libraries > >in DPDK the number doesn't just increase a bit, it explodes. > > If you check the part below this one in my original email, that you stripped > out (without notice), the suggestion was also to add a global _DEBUG > parameter for the entire DPDK set of libraries, to change all the CFLAGS at > once (not in the attached PATCH). > > > > >I dont see that much point in this thing, but I'd approach it by defining > >the debug flags someplace central, say DEBUG_FLAGS, and append that to the > >common cflags when *_DEBUG config is enabled. At least with gcc the last > >option wins so if you just append -O0 when debugging then that's what > >wins, the earlier -O3 does not matter. > > The original problem is the one you expose; libraries hardcode the CFLAGS, > ignoring user-flags. There is no way to change this u
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
On 03/03/15 14:03, Bruce Richardson wrote: > On Tue, Mar 03, 2015 at 01:56:19PM +0100, Marc Sune wrote: >> On 03/03/15 13:40, Panu Matilainen wrote: >>> On 03/03/2015 02:19 PM, Marc Sune wrote: On 03/03/15 10:33, Bruce Richardson wrote: > On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: >> On 22/02/15 12:51, Marc Sune wrote: >>> I don't like the proposed patch, but I am recovering this old thread >>> because I agree on the problem statement. >>> >>> On 04/04/14 11:57, Ananyev, Konstantin wrote: Hi Cyril, We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you can use to enable debug, or any other compiler/linker options you need. Wonder, why that is not enough? >>> EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why >>> setting individually: >>> >>> diff --git a/config/common_linuxapp b/config/common_linuxapp >>> index 2f9643b..04adc0d 100644 >>> --- a/config/common_linuxapp >>> +++ b/config/common_linuxapp >>> @@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y >>> # Compile generic ethernet library >>> # >>> CONFIG_RTE_LIBRTE_ETHER=y >>> -CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n >>> +CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y >>> >>> >>> to put an example, does not set -g and -O0 in that particular module >>> only. >>> No one would ever use something compiled in DEBUG in production >>> anyway. >>> >>> I always end up modifying manually Makefiles in the lib library that >>> I am >>> interested in having insides, overriding CFLAGS=-O3, which is not >>> that >>> nice. >> I would like some feedback on this idea. If the community sees >> benefit, I >> will work on a patch for this. >> >> Marc >> > So, your proposal is to patch things so that any time one sets DEBUG=y > in the > build-time config for a library, we change the '-O3' to '-O0' and set > -g also. > Correct? I am not sure what you mean by 'patch things'. I would simply enable the build system to override the default compilation flags (now DPDK-wide, or specifically librte_ wide) when _DEBUG=y for a library, changing compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I have to check if -O0 already implicitly means -fno-inline (even for __attribute__((always_inline)) ). I did a quick test. I chose KNI because it didn't have a DEBUG flag for the user-space library. For other libraries, the existing _DEBUG setting would be enough: marc at dpdk:~/dpdk$ git diff HEAD diff --git a/config/common_linuxapp b/config/common_linuxapp index 97f1c9e..8a3cef8 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y # CONFIG_RTE_LIBRTE_KNI=y CONFIG_RTE_KNI_PREEMPT_DEFAULT=y +CONFIG_RTE_LIBRTE_KNI_DEBUG=y CONFIG_RTE_KNI_KO_DEBUG=n CONFIG_RTE_KNI_VHOST=n CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile index 7107832..895f64e 100644 --- a/lib/librte_kni/Makefile +++ b/lib/librte_kni/Makefile @@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk # library name LIB = librte_kni.a -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) EXPORT_MAP := rte_kni_version.map diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 63a41e2..eee477d 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -78,6 +78,13 @@ endif ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) LDLIBS += -lrte_kni + +ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline +else +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing +endif + endif endif Thoughts? >>> My 5c is that if anything, DPDK needs *less* places that muck around with >>> compiler flags, not more. If you something like this for all the libraries >>> in DPDK the number doesn't just increase a bit, it explodes. >> If you check the part below this one in my original email, that you stripped >> out (without notice), the suggestion was also to add a global _DEBUG >> parameter for the entire DPDK set of libraries, to change all the CFLAGS at >> once (not in the attached PATCH). >> >>> I dont see that much point in this thing, but I'd approach it by defining >>> the debug flags someplace central, say DEBUG_FLAGS, and append that to the >>> common cflags when *_DEBUG config is enabled. At least with gcc the last >>> option wins so if you just append -O0 when debugging then that's what >>> wins, the earlier -O3 does not matter. >> The o
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson > Sent: Tuesday, March 03, 2015 1:03 PM > To: Marc Sune > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation > > On Tue, Mar 03, 2015 at 01:56:19PM +0100, Marc Sune wrote: > > > > On 03/03/15 13:40, Panu Matilainen wrote: > > >On 03/03/2015 02:19 PM, Marc Sune wrote: > > >> > > >>On 03/03/15 10:33, Bruce Richardson wrote: > > >>>On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: > > On 22/02/15 12:51, Marc Sune wrote: > > >I don't like the proposed patch, but I am recovering this old thread > > >because I agree on the problem statement. > > > > > >On 04/04/14 11:57, Ananyev, Konstantin wrote: > > >>Hi Cyril, > > >>We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you > > >>can use > > >>to enable debug, or any other compiler/linker options you need. > > >>Wonder, why that is not enough? > > >EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why > > >setting individually: > > > > > >diff --git a/config/common_linuxapp b/config/common_linuxapp > > >index 2f9643b..04adc0d 100644 > > >--- a/config/common_linuxapp > > >+++ b/config/common_linuxapp > > >@@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y > > ># Compile generic ethernet library > > ># > > >CONFIG_RTE_LIBRTE_ETHER=y > > >-CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n > > >+CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y > > > > > > > > >to put an example, does not set -g and -O0 in that particular module > > >only. > > >No one would ever use something compiled in DEBUG in production > > >anyway. > > > > > >I always end up modifying manually Makefiles in the lib library that > > >I am > > >interested in having insides, overriding CFLAGS=-O3, which is not > > >that > > >nice. > > I would like some feedback on this idea. If the community sees > > benefit, I > > will work on a patch for this. > > > > Marc > > > > >>>So, your proposal is to patch things so that any time one sets DEBUG=y > > >>>in the > > >>>build-time config for a library, we change the '-O3' to '-O0' and set > > >>>-g also. > > >>>Correct? > > >> > > >>I am not sure what you mean by 'patch things'. I would simply enable the > > >>build system to override the default compilation flags (now DPDK-wide, > > >>or specifically librte_ wide) when _DEBUG=y for a library, changing > > >>compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I > > >>have to check if -O0 already implicitly means -fno-inline (even for > > >>__attribute__((always_inline)) ). > > >> > > >>I did a quick test. I chose KNI because it didn't have a DEBUG flag for > > >>the user-space library. For other libraries, the existing _DEBUG setting > > >>would be enough: > > >> > > >>marc at dpdk:~/dpdk$ git diff HEAD > > >>diff --git a/config/common_linuxapp b/config/common_linuxapp > > >>index 97f1c9e..8a3cef8 100644 > > >>--- a/config/common_linuxapp > > >>+++ b/config/common_linuxapp > > >>@@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > > >> # > > >> CONFIG_RTE_LIBRTE_KNI=y > > >> CONFIG_RTE_KNI_PREEMPT_DEFAULT=y > > >>+CONFIG_RTE_LIBRTE_KNI_DEBUG=y > > >> CONFIG_RTE_KNI_KO_DEBUG=n > > >> CONFIG_RTE_KNI_VHOST=n > > >> CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > > >>diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile > > >>index 7107832..895f64e 100644 > > >>--- a/lib/librte_kni/Makefile > > >>+++ b/lib/librte_kni/Makefile > > >>@@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > > >> # library name > > >> LIB = librte_kni.a > > >> > > >>-CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing > > >>+CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) > > >> > > >> EXPORT_MAP := rte_kni_version.map > > >> > > >>diff --git a/mk/rte.app.mk b/mk/rte.app.mk > > >>index 63a41e2..eee477d 100644 > > >>--- a/mk/rte.app.mk > > >>+++ b/mk/rte.app.mk > > >>@@ -78,6 +78,13 @@ endif > > >> ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > > >> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > > >> LDLIBS += -lrte_kni > > >>+ > > >>+ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) > > >>+CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline > > >>+else > > >>+CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing > > >>+endif > > >>+ > > >> endif > > >> endif > > >> > > >> > > >>Thoughts? > > > > > >My 5c is that if anything, DPDK needs *less* places that muck around with > > >compiler flags, not more. If you something like this for all the libraries > > >in DPDK the number doesn't just increase a bit, it explodes. > > > > If you check the part below this one in my original email, that you stripped > > out (without notice), the suggestion was also to add a global _DEBUG > > parameter for the entire DPDK set of libraries, to change all the CFLAGS at > > once (not in the
[dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload
On Thu, Feb 26, 2015 at 06:33:20AM +, Raz Amir wrote:
> Added code that saves the pointers to the detached devices, during
> driver loading, and during driver unloading, go over the list,
> and re-attach them by calling device_probe_and_attach
> on each device.
>
> Signed-off-by: Raz Amir
Couple of minor comments below. Otherwise all looks good to me.
Acked-by: Bruce Richardson
> ---
> lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 +-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
> b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
> index 5ae8560..7d702a5 100644
> --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
> +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
> @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$");
>
> #define MAX_BARS (PCIR_MAX_BAR_0 + 1)
>
> +#define MAX_DETACHED_DEVICES 128
> +static device_t detached_devices[MAX_DETACHED_DEVICES] = {};
> +static int last_detached = 0;
Maybe num_detached/nb_detached or even just "detached" instead of
"last_detached".
>
> struct nic_uio_softc {
> device_tdev_t;
> @@ -291,14 +294,35 @@ nic_uio_load(void)
> if (dev != NULL)
We are getting into some serious levels of indentation below, so maybe flip
this condition around and put in a "continue" instead, so that we can dedent
everything below that follows it.
> for (i = 0; i < NUM_DEVICES; i++)
> if (pci_get_vendor(dev) == devices[i].vend &&
> - pci_get_device(dev) ==
> devices[i].dev)
> + pci_get_device(dev) ==
> devices[i].dev) {
> + if (last_detached+1 <
> MAX_DETACHED_DEVICES) {
I don't think you need the +1 here.
> +
> printf("nic_uio_load: detaching and storing dev=%p\n", dev);
> +
> detached_devices[last_detached++] = dev;
> + }
> + else {
> +
> printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't be
> reattached\n",
> +
> MAX_DETACHED_DEVICES, dev);
> + }
DPDK coding style is not to put braces around single-line statements like this.
> +
Remove whitespace from this new line.
> device_detach(dev);
> + }
> }
> }
>
> static void
> nic_uio_unload(void)
> {
> + int i;
> + printf("nic_uio_unload: entered ... \n");
> +
> + for (i = 0; i < last_detached; i++) {
> + printf("nic_uio_unload: calling to device_probe_and_attach for
> dev=%p...\n",
> + detached_devices[i]);
> + device_probe_and_attach(detached_devices[i]);
> + printf("nic_uio_unload: done.\n");
> + }
> +
> + printf("nic_uio_unload: leaving ... \n");
> }
>
> static int
> --
> 2.1.2
>
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
2015-03-03 13:03, Bruce Richardson: > On Tue, Mar 03, 2015 at 01:56:19PM +0100, Marc Sune wrote: > > On 03/03/15 13:40, Panu Matilainen wrote: > > >My 5c is that if anything, DPDK needs *less* places that muck around with > > >compiler flags, not more. If you something like this for all the libraries > > >in DPDK the number doesn't just increase a bit, it explodes. > > > > If you check the part below this one in my original email, that you stripped > > out (without notice), the suggestion was also to add a global _DEBUG > > parameter for the entire DPDK set of libraries, to change all the CFLAGS at > > once (not in the attached PATCH). > > > > >I dont see that much point in this thing, but I'd approach it by defining > > >the debug flags someplace central, say DEBUG_FLAGS, and append that to the > > >common cflags when *_DEBUG config is enabled. At least with gcc the last > > >option wins so if you just append -O0 when debugging then that's what > > >wins, the earlier -O3 does not matter. > > > > The original problem is the one you expose; libraries hardcode the CFLAGS, > > ignoring user-flags. There is no way to change this unless you change the > > Makefiles directly. > > > > But right now, each library does hardcode its *own* flags (check Makefiles > > for the libraries), so there is already not a unified approach here. I see > > for instance KNI having -fno-strict-aliasing while other libraries don't. > > > > Having said that, there are moments, specially with -O3, in which to be able > > to reproduce a bug, you need to compile certain parts of code with -O3 and > > the rest with -O0 -g (the ones to be debugged). The approach proposed (both > > a global *and* a lib specific) allows that. > > > > Marc > > I believe that the global option of overriding the CFLAGS is already > sufficiently > covered - including being documented in programmers guide - by EXTRA_CFLAGS. > The > ability to turn off optimization support for a single library is not covered > anywhere, and that suggestion seems reasonable to me. For each library, we can > just append '-O0 -g' to the CFLAGS in that libraries makefile if the debug > option > is set. I don't see that as significantly complicating things [though I > wouldn't > make any changes to the rte.app.mk to allow this, just have it per lib in the > lib's makefile] The difficult thing with a build system, is to know which options and use cases we should support. Today you are suggesting some debug options for gdb. Tomorrow someone would like to have a valgrind support and someone else would like more options for a static analyzer. I think that these usages are restricted to developers use and they already can tune the Makefiles of the libs they are working on.
[dpdk-dev] [PATCH v2] librte_eal/common: Fix cast from pointer to integer of different size
> -Original Message- > From: Qiu, Michael > Sent: Tuesday, March 03, 2015 11:00 AM > To: Wodkowski, PawelX; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2] librte_eal/common: Fix cast from pointer to > integer of different size > > On 3/3/2015 4:25 PM, Wodkowski, PawelX wrote: > > On 2015-03-03 03:20, Michael Qiu wrote: > >> /i686-native-linuxapp-gcc/include/rte_memcpy.h:592:23: error: > >> cast from pointer to integer of different size > >> [-Werror=pointer-to-int-cast] > >> > >>dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; > >> > >> Type 'long long' is 64-bit in i686 platform while 'void *' > >> is 32-bit. > >> > >> Signed-off-by: Michael Qiu > >> --- > >> v2 --> v1: > >>Remove unnecessary casting (void *) > >> > >> lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h > b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h > >> index 7b2d382..85a5f4d 100644 > >> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h > >> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h > >> @@ -589,12 +589,12 @@ COPY_BLOCK_64_BACK15: > >> * unaligned copy functions require up to 15 bytes > >> * backwards access. > >> */ > >> - dstofss = 16 - (int)((long long)(void *)dst & 0x0F) + 16; > >> + dstofss = 16 - (int)((long)dst & 0x0F) + 16; > >>n -= dstofss; > >>rte_mov32((uint8_t *)dst, (const uint8_t *)src); > >>src = (const uint8_t *)src + dstofss; > >>dst = (uint8_t *)dst + dstofss; > >> - srcofs = (int)((long long)(const void *)src & 0x0F); > >> + srcofs = (int)((long)src & 0x0F); > >> > >>/** > >> * For aligned copy > >> > > I think you should use here size_t, (u)ptrdiff_t or (u)intptr_t as this > > Yes, but 'long' is enough, does it limit anything, or has any issue with > multiple platforms? > Those types ((u)ptrdiff_t, (u)intptr_t) exists specially for pointer-to-int and int-to-pointer casts. Using integer primitives might produce further warnings/error in the future and need further patches fixing the same place. Also why make offset variables signed and different type? This introduce a lot of unnecessary explicit and implicit casts or type promotions. > > will be more portable. > > Also type of dstofss and srcofs should be changed accordingly. > > No, I think, it should be offset. > > Thanks, > Michael > > -- Pawel
[dpdk-dev] [PATCH] dpdk: fix a crash during rte_table_hash_key16_ext overload
From: Miroslaw Walukiewicz The hash_key16_ext table allocates a cache entries to support table overload cases. The crash can occur when cache entry is free after use. The problem is with computing the index of the free cache entry. The patch fixes a problem. Signed-off-by: Mirek Walukiewicz --- lib/librte_table/rte_table_hash_key16.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/librte_table/rte_table_hash_key16.c b/lib/librte_table/rte_table_hash_key16.c index ee5f639..e0c99bd 100644 --- a/lib/librte_table/rte_table_hash_key16.c +++ b/lib/librte_table/rte_table_hash_key16.c @@ -535,9 +535,8 @@ rte_table_hash_entry_delete_key16_ext( memset(bucket, 0, sizeof(struct rte_bucket_4_16)); - bucket_index = (bucket - - ((struct rte_bucket_4_16 *) - f->memory)) - f->n_buckets; + bucket_index = (((uint8_t *)bucket - + (uint8_t *)f->memory)/f->bucket_size) - f->n_buckets; f->stack[f->stack_pos++] = bucket_index; }
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
On 03/03/15 14:31, Ananyev, Konstantin wrote: > >> -Original Message- >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson >> Sent: Tuesday, March 03, 2015 1:03 PM >> To: Marc Sune >> Cc: dev at dpdk.org >> Subject: Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation >> >> On Tue, Mar 03, 2015 at 01:56:19PM +0100, Marc Sune wrote: >>> On 03/03/15 13:40, Panu Matilainen wrote: On 03/03/2015 02:19 PM, Marc Sune wrote: > On 03/03/15 10:33, Bruce Richardson wrote: >> On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: >>> On 22/02/15 12:51, Marc Sune wrote: I don't like the proposed patch, but I am recovering this old thread because I agree on the problem statement. On 04/04/14 11:57, Ananyev, Konstantin wrote: > Hi Cyril, > We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you > can use > to enable debug, or any other compiler/linker options you need. > Wonder, why that is not enough? EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why setting individually: diff --git a/config/common_linuxapp b/config/common_linuxapp index 2f9643b..04adc0d 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y # Compile generic ethernet library # CONFIG_RTE_LIBRTE_ETHER=y -CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n +CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y to put an example, does not set -g and -O0 in that particular module only. No one would ever use something compiled in DEBUG in production anyway. I always end up modifying manually Makefiles in the lib library that I am interested in having insides, overriding CFLAGS=-O3, which is not that nice. >>> I would like some feedback on this idea. If the community sees >>> benefit, I >>> will work on a patch for this. >>> >>> Marc >>> >> So, your proposal is to patch things so that any time one sets DEBUG=y >> in the >> build-time config for a library, we change the '-O3' to '-O0' and set >> -g also. >> Correct? > I am not sure what you mean by 'patch things'. I would simply enable the > build system to override the default compilation flags (now DPDK-wide, > or specifically librte_ wide) when _DEBUG=y for a library, changing > compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I > have to check if -O0 already implicitly means -fno-inline (even for > __attribute__((always_inline)) ). > > I did a quick test. I chose KNI because it didn't have a DEBUG flag for > the user-space library. For other libraries, the existing _DEBUG setting > would be enough: > > marc at dpdk:~/dpdk$ git diff HEAD > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 97f1c9e..8a3cef8 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > # > CONFIG_RTE_LIBRTE_KNI=y > CONFIG_RTE_KNI_PREEMPT_DEFAULT=y > +CONFIG_RTE_LIBRTE_KNI_DEBUG=y > CONFIG_RTE_KNI_KO_DEBUG=n > CONFIG_RTE_KNI_VHOST=n > CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile > index 7107832..895f64e 100644 > --- a/lib/librte_kni/Makefile > +++ b/lib/librte_kni/Makefile > @@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > # library name > LIB = librte_kni.a > > -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing > +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) > > EXPORT_MAP := rte_kni_version.map > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk > index 63a41e2..eee477d 100644 > --- a/mk/rte.app.mk > +++ b/mk/rte.app.mk > @@ -78,6 +78,13 @@ endif > ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > LDLIBS += -lrte_kni > + > +ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) > +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline > +else > +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing > +endif > + > endif > endif > > > Thoughts? My 5c is that if anything, DPDK needs *less* places that muck around with compiler flags, not more. If you something like this for all the libraries in DPDK the number doesn't just increase a bit, it explodes. >>> If you check the part below this one in my original email, that you stripped >>> out (without notice), the suggestion was also to add a global _DEBUG >>> parameter for the entire DPDK set of libraries, to change all the CFLAGS at >>> once
[dpdk-dev] [PATCH 0/7] fix build with debug enabled
There are some compilation errors when debug options are enabled. We should start thinking to test every patch with an all-yes configuration. Unfortunately, we cannot force this kind of configuration because some libraries depend on the availability of some libraries. Thomas Monjalon (7): mempool: fix build with debug enabled fm10k: fix build with debug enabled virtio: fix build with mempool debug enabled virtio: fix build with debug enabled mlx4: fix build with mempool debug enabled mlx4: mute auto config in quiet mode bond: remove debug function to fix link with shared lib examples/bond/main.c| 5 - lib/librte_eal/common/include/rte_pci.h | 2 +- lib/librte_mempool/rte_mempool.h| 8 lib/librte_pmd_bond/rte_eth_bond_pmd.c | 13 - lib/librte_pmd_fm10k/fm10k_logs.h | 2 ++ lib/librte_pmd_mlx4/Makefile| 10 +++--- lib/librte_pmd_mlx4/mlx4.c | 22 ++ lib/librte_pmd_virtio/Makefile | 2 -- lib/librte_pmd_virtio/virtio_ethdev.c | 10 +++--- 9 files changed, 27 insertions(+), 47 deletions(-) -- 2.2.2
[dpdk-dev] [PATCH 1/7] mempool: fix build with debug enabled
error: format ?%p? expects argument of type ?void *?,
but argument 5 has type ?const struct rte_mempool *? [-Werror=format=]
mp type is (const struct rte_mempool *) and must be casted into a simpler
type to be printed.
Signed-off-by: Thomas Monjalon
---
lib/librte_mempool/rte_mempool.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index 974e8d7..39f7233 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -345,7 +345,7 @@ static inline void __mempool_check_cookies(const struct
rte_mempool *mp,
rte_log_set_history(0);
RTE_LOG(CRIT, MEMPOOL,
"obj=%p, mempool=%p, cookie=%" PRIx64
"\n",
- obj, mp, cookie);
+ obj, (const void *) mp, cookie);
rte_panic("MEMPOOL: bad header cookie (put)\n");
}
__mempool_write_header_cookie(obj, 1);
@@ -355,7 +355,7 @@ static inline void __mempool_check_cookies(const struct
rte_mempool *mp,
rte_log_set_history(0);
RTE_LOG(CRIT, MEMPOOL,
"obj=%p, mempool=%p, cookie=%" PRIx64
"\n",
- obj, mp, cookie);
+ obj, (const void *) mp, cookie);
rte_panic("MEMPOOL: bad header cookie (get)\n");
}
__mempool_write_header_cookie(obj, 0);
@@ -366,7 +366,7 @@ static inline void __mempool_check_cookies(const struct
rte_mempool *mp,
rte_log_set_history(0);
RTE_LOG(CRIT, MEMPOOL,
"obj=%p, mempool=%p, cookie=%" PRIx64
"\n",
- obj, mp, cookie);
+ obj, (const void *) mp, cookie);
rte_panic("MEMPOOL: bad header cookie
(audit)\n");
}
}
@@ -375,7 +375,7 @@ static inline void __mempool_check_cookies(const struct
rte_mempool *mp,
rte_log_set_history(0);
RTE_LOG(CRIT, MEMPOOL,
"obj=%p, mempool=%p, cookie=%" PRIx64 "\n",
- obj, mp, cookie);
+ obj, (const void *) mp, cookie);
rte_panic("MEMPOOL: bad trailer cookie\n");
}
}
--
2.2.2
[dpdk-dev] [PATCH 2/7] fm10k: fix build with debug enabled
error: implicit declaration of function ?RTE_LOG?
Fixes: a6061d9e7075 ("fm10k: register PF driver")
Signed-off-by: Thomas Monjalon
---
lib/librte_pmd_fm10k/fm10k_logs.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_pmd_fm10k/fm10k_logs.h
b/lib/librte_pmd_fm10k/fm10k_logs.h
index febd796..41a45ce 100644
--- a/lib/librte_pmd_fm10k/fm10k_logs.h
+++ b/lib/librte_pmd_fm10k/fm10k_logs.h
@@ -34,6 +34,8 @@
#ifndef _FM10K_LOGS_H_
#define _FM10K_LOGS_H_
+#include
+
#define PMD_INIT_LOG(level, fmt, args...) \
rte_log(RTE_LOG_ ## level, RTE_LOGTYPE_PMD, \
"PMD: %s(): " fmt "\n", __func__, ##args)
--
2.2.2
[dpdk-dev] [PATCH 3/7] virtio: fix build with mempool debug enabled
The mempool header forces error on -Wcast-qual:
error: cast discards ?const? qualifier from pointer target type
Let's fix it by removing const qualifier of pci driver from commit
5e9f6d1340ff ("pci: reference driver structure for each device")
It's needed because the driver flags are changed depending on using uio or not.
Actually these driver flags should be directly attached to each device.
Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")
Signed-off-by: Thomas Monjalon
---
lib/librte_eal/common/include/rte_pci.h | 2 +-
lib/librte_pmd_virtio/Makefile | 2 --
lib/librte_pmd_virtio/virtio_ethdev.c | 8 ++--
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_pci.h
b/lib/librte_eal/common/include/rte_pci.h
index b9cdf8b..995f814 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -158,7 +158,7 @@ struct rte_pci_device {
struct rte_pci_id id; /**< PCI ID. */
struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE]; /**< PCI
Memory Resource */
struct rte_intr_handle intr_handle; /**< Interrupt handle */
- const struct rte_pci_driver *driver;/**< Associated driver */
+ struct rte_pci_driver *driver; /**< Associated driver */
uint16_t max_vfs; /**< sriov enable if not zero */
int numa_node; /**< NUMA node connection */
struct rte_devargs *devargs;/**< Device user arguments */
diff --git a/lib/librte_pmd_virtio/Makefile b/lib/librte_pmd_virtio/Makefile
index 0baaf46..793067f 100644
--- a/lib/librte_pmd_virtio/Makefile
+++ b/lib/librte_pmd_virtio/Makefile
@@ -57,6 +57,4 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal
lib/librte_ether
DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_mempool lib/librte_mbuf
DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_net lib/librte_malloc
-CFLAGS_virtio_ethdev.o += -Wno-cast-qual
-
include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c
b/lib/librte_pmd_virtio/virtio_ethdev.c
index 88ecd57..4bad1e4 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -938,8 +938,6 @@ static int virtio_resource_init_by_uio(struct
rte_pci_device *pci_dev)
char filename[PATH_MAX];
unsigned long start, size;
unsigned int uio_num;
- struct rte_pci_driver *pci_drv =
- (struct rte_pci_driver *)pci_dev->driver;
if (get_uio_dev(&pci_dev->addr, dirname, sizeof(dirname), &uio_num) < 0)
return -1;
@@ -978,7 +976,7 @@ static int virtio_resource_init_by_uio(struct
rte_pci_device *pci_dev)
}
pci_dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
- pci_drv->drv_flags |= RTE_PCI_DRV_INTR_LSC;
+ pci_dev->driver->drv_flags |= RTE_PCI_DRV_INTR_LSC;
return 0;
}
@@ -993,8 +991,6 @@ static int virtio_resource_init_by_ioports(struct
rte_pci_device *pci_dev)
char pci_id[16];
int found = 0;
size_t linesz;
- struct rte_pci_driver *pci_drv =
- (struct rte_pci_driver *)pci_dev->driver;
snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
pci_dev->addr.domain,
@@ -1046,7 +1042,7 @@ static int virtio_resource_init_by_ioports(struct
rte_pci_device *pci_dev)
start, size);
/* can't support lsc interrupt without uio */
- pci_drv->drv_flags &= ~RTE_PCI_DRV_INTR_LSC;
+ pci_dev->driver->drv_flags &= ~RTE_PCI_DRV_INTR_LSC;
return 0;
}
--
2.2.2
[dpdk-dev] [PATCH 4/7] virtio: fix build with debug enabled
With CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_INIT=y:
error: ?devname? undeclared (first use in this function)
Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")
Signed-off-by: Thomas Monjalon
---
lib/librte_pmd_virtio/virtio_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c
b/lib/librte_pmd_virtio/virtio_ethdev.c
index 4bad1e4..d239083 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -971,7 +971,7 @@ static int virtio_resource_init_by_uio(struct
rte_pci_device *pci_dev)
pci_dev->intr_handle.fd = open(dirname, O_RDWR);
if (pci_dev->intr_handle.fd < 0) {
PMD_INIT_LOG(ERR, "Cannot open %s: %s\n",
- devname, strerror(errno));
+ dirname, strerror(errno));
return -1;
}
--
2.2.2
[dpdk-dev] [PATCH 5/7] mlx4: fix build with mempool debug enabled
The mempool header forces error on -Wcast-qual and makes verbs.h failing.
Let's include verbs before as a system header.
Fixes: 7fae69eeff13 ("mlx4: new poll mode driver")
Signed-off-by: Thomas Monjalon
---
lib/librte_pmd_mlx4/mlx4.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/lib/librte_pmd_mlx4/mlx4.c b/lib/librte_pmd_mlx4/mlx4.c
index 492cbbf..b5774c4 100644
--- a/lib/librte_pmd_mlx4/mlx4.c
+++ b/lib/librte_pmd_mlx4/mlx4.c
@@ -60,6 +60,16 @@
#include
#include
+/* Verbs header. */
+/* ISO C doesn't support unnamed structs/unions, disabling -pedantic. */
+#ifdef PEDANTIC
+#pragma GCC diagnostic ignored "-pedantic"
+#endif
+#include
+#ifdef PEDANTIC
+#pragma GCC diagnostic error "-pedantic"
+#endif
+
/* DPDK headers don't like -pedantic. */
#ifdef PEDANTIC
#pragma GCC diagnostic ignored "-pedantic"
@@ -81,18 +91,6 @@
#pragma GCC diagnostic error "-pedantic"
#endif
-/* Verbs header. */
-/* ISO C doesn't support unnamed structs/unions, disabling -pedantic. */
-#ifdef PEDANTIC
-#pragma GCC diagnostic ignored "-pedantic"
-#endif
-
-#include
-
-#ifdef PEDANTIC
-#pragma GCC diagnostic error "-pedantic"
-#endif
-
/* Generated configuration header. */
#include "mlx4_autoconf.h"
--
2.2.2
[dpdk-dev] [PATCH 6/7] mlx4: mute auto config in quiet mode
If verbose is off, auto-config-h.sh script should be quiet. Signed-off-by: Thomas Monjalon --- lib/librte_pmd_mlx4/Makefile | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/librte_pmd_mlx4/Makefile b/lib/librte_pmd_mlx4/Makefile index de50a5a..813 100644 --- a/lib/librte_pmd_mlx4/Makefile +++ b/lib/librte_pmd_mlx4/Makefile @@ -98,20 +98,24 @@ include $(RTE_SDK)/mk/rte.lib.mk export CC CFLAGS CPPFLAGS EXTRA_CFLAGS EXTRA_CPPFLAGS export AUTO_CONFIG_CFLAGS = -Wno-error +ifndef V +AUTOCONF_OUTPUT := >/dev/null +endif + mlx4_autoconf.h: $(RTE_SDK)/scripts/auto-config-h.sh $Q $(RM) -f -- '$@' $Q sh -- '$<' '$@' \ RSS_SUPPORT \ infiniband/verbs.h \ - enum IBV_EXP_DEVICE_UD_RSS + enum IBV_EXP_DEVICE_UD_RSS $(AUTOCONF_OUTPUT) $Q sh -- '$<' '$@' \ INLINE_RECV \ infiniband/verbs.h \ - enum IBV_EXP_DEVICE_ATTR_INLINE_RECV_SZ + enum IBV_EXP_DEVICE_ATTR_INLINE_RECV_SZ $(AUTOCONF_OUTPUT) $Q sh -- '$<' '$@' \ SEND_RAW_WR_SUPPORT \ infiniband/verbs.h \ - type 'struct ibv_send_wr_raw' + type 'struct ibv_send_wr_raw' $(AUTOCONF_OUTPUT) mlx4.o: mlx4_autoconf.h -- 2.2.2
[dpdk-dev] [PATCH 7/7] bond: remove debug function to fix link with shared lib
The function print_client_stats was used in the example without being
clearly exported in the map file. So it breaks linking with shared library
when debug is enabled.
It's better to remove this function as it probably could be implemented
with statistics API.
Fixes: cc7e8ae84faa ("add example application for link bonding mode 6")
Signed-off-by: Thomas Monjalon
---
examples/bond/main.c | 5 -
lib/librte_pmd_bond/rte_eth_bond_pmd.c | 13 -
2 files changed, 18 deletions(-)
diff --git a/examples/bond/main.c b/examples/bond/main.c
index e4457f2..7da6bfc 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -650,7 +650,6 @@ cmdline_parse_inst_t cmd_quit = {
},
};
-extern void print_client_stats(void);
struct cmd_show_result {
cmdline_fixed_string_t show;
};
@@ -680,10 +679,6 @@ static void cmd_show_parsed(__attribute__((unused)) void
*parsed_result,
global_flag_stru_p->port_packets[1],
global_flag_stru_p->port_packets[2]);
rte_spinlock_unlock(&global_flag_stru_p->lock);
-
-#if defined(RTE_LIBRTE_BOND_DEBUG_ALB_L1) || defined(RTE_LIBRTE_BOND_DEBUG_ALB)
- print_client_stats();
-#endif
}
cmdline_parse_token_string_t cmd_show_show =
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index 7dee5f2..af8113e 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -279,19 +279,6 @@ update_client_stats(uint32_t addr, uint8_t port, uint32_t
*TXorRXindicator)
}
-void print_client_stats(void);
-void print_client_stats(void)
-{
- int i = 0;
- char buf[MaxIPv4String];
-
- for (; i < active_clients; i++) {
- ipv4_addr_to_dot(client_stats[i].ipv4_addr, buf, MaxIPv4String);
- printf("port:%d client:%s RX:%d TX:%d\n", client_stats[i].port,
buf,
- client_stats[i].ipv4_rx_packets,
- client_stats[i].ipv4_tx_packets);
- }
-}
#ifdef RTE_LIBRTE_BOND_DEBUG_ALB
#define MODE6_DEBUG(info, src_ip, dst_ip, eth_h, arp_op, port, burstnumber)
\
RTE_LOG(DEBUG, PMD, \
--
2.2.2
[dpdk-dev] [PATCH v15] testpmd: Add port hotplug support
Hi Tetsuya,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tetsuya Mukawa
> Sent: Wednesday, February 25, 2015 7:32 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v15] testpmd: Add port hotplug support
>
> The patch introduces following commands.
> - port attach [ident]
> - port detach [port_id]
> - attach: attaching a port
> - detach: detaching a port
> - ident: pci address of physical device.
> Or device name and parameters of virtual device.
> (ex. :02:00.0, eth_pcap0,iface=eth0)
> - port_id: port identifier
>
> v15:
> - Replace rte_eal_dev_attach() by rte_eth_dev_attach()
> - Replace rte_eal_dev_detach() by rte_eth_dev_detach()
>
> v7:
> - Fix doc.
> (Thanks to Iremonger, Bernard)
> - Fix port checking implementation of star_port();
> (Thanks to Qiu, Michael)
> v5:
> - Add testpmd documentation.
> (Thanks to Iremonger, Bernard)
> v4:
> - Fix strings of command help.
>
> Signed-off-by: Tetsuya Mukawa
> ---
> app/test-pmd/cmdline.c | 137 +++
> app/test-pmd/config.c | 102 --
> app/test-pmd/parameters.c | 22 ++-
> app/test-pmd/testpmd.c | 199
> +---
> app/test-pmd/testpmd.h | 18 ++-
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 57
[...]
> @@ -1446,8 +1497,8 @@ stop_port(portid_t pid)
> }
> printf("Stopping ports...\n");
>
> - for (pi = 0; pi < nb_ports; pi++) {
> - if (pid < nb_ports && pid != pi)
> + FOREACH_PORT(pi, ports) {
> + if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
If using "port stop all", this function does not work as it should.
Problem is that pid = RTE_PORT_ALL = 255, and then ports[255].enabled is
undefined,
as ports array is allocated only for RTE_MAX_ETHPORTS (32 by default).
So, the solution could be either increasing the ports array to 256 items,
or check if we are passing RTE_PORT_ALL in pid.
What do you think?
Thanks,
Pablo
> continue;
>
> port = &ports[pi];
> @@ -1463,7 +1514,7 @@ stop_port(portid_t pid)
> need_check_link_status = 1;
> }
> if (need_check_link_status && !no_link_check)
> - check_all_ports_link_status(nb_ports, RTE_PORT_ALL);
> + check_all_ports_link_status(RTE_PORT_ALL);
>
> printf("Done\n");
> }
[dpdk-dev] [PATCH 5/7] mlx4: fix build with mempool debug enabled
On Tue, Mar 03, 2015 at 04:23:48PM +0100, Thomas Monjalon wrote:
> The mempool header forces error on -Wcast-qual and makes verbs.h failing.
> Let's include verbs before as a system header.
>
> Fixes: 7fae69eeff13 ("mlx4: new poll mode driver")
>
> Signed-off-by: Thomas Monjalon
> ---
> lib/librte_pmd_mlx4/mlx4.c | 22 ++
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_pmd_mlx4/mlx4.c b/lib/librte_pmd_mlx4/mlx4.c
> index 492cbbf..b5774c4 100644
> --- a/lib/librte_pmd_mlx4/mlx4.c
> +++ b/lib/librte_pmd_mlx4/mlx4.c
> @@ -60,6 +60,16 @@
> #include
> #include
>
> +/* Verbs header. */
> +/* ISO C doesn't support unnamed structs/unions, disabling -pedantic. */
> +#ifdef PEDANTIC
> +#pragma GCC diagnostic ignored "-pedantic"
> +#endif
> +#include
> +#ifdef PEDANTIC
> +#pragma GCC diagnostic error "-pedantic"
> +#endif
> +
> /* DPDK headers don't like -pedantic. */
> #ifdef PEDANTIC
> #pragma GCC diagnostic ignored "-pedantic"
> @@ -81,18 +91,6 @@
> #pragma GCC diagnostic error "-pedantic"
> #endif
>
> -/* Verbs header. */
> -/* ISO C doesn't support unnamed structs/unions, disabling -pedantic. */
> -#ifdef PEDANTIC
> -#pragma GCC diagnostic ignored "-pedantic"
> -#endif
> -
> -#include
> -
> -#ifdef PEDANTIC
> -#pragma GCC diagnostic error "-pedantic"
> -#endif
> -
> /* Generated configuration header. */
> #include "mlx4_autoconf.h"
>
> --
> 2.2.2
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
[dpdk-dev] [PATCH 6/7] mlx4: mute auto config in quiet mode
On Tue, Mar 03, 2015 at 04:23:49PM +0100, Thomas Monjalon wrote: > If verbose is off, auto-config-h.sh script should be quiet. > > Signed-off-by: Thomas Monjalon > --- > lib/librte_pmd_mlx4/Makefile | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_pmd_mlx4/Makefile b/lib/librte_pmd_mlx4/Makefile > index de50a5a..813 100644 > --- a/lib/librte_pmd_mlx4/Makefile > +++ b/lib/librte_pmd_mlx4/Makefile > @@ -98,20 +98,24 @@ include $(RTE_SDK)/mk/rte.lib.mk > export CC CFLAGS CPPFLAGS EXTRA_CFLAGS EXTRA_CPPFLAGS > export AUTO_CONFIG_CFLAGS = -Wno-error > > +ifndef V > +AUTOCONF_OUTPUT := >/dev/null > +endif > + > mlx4_autoconf.h: $(RTE_SDK)/scripts/auto-config-h.sh > $Q $(RM) -f -- '$@' > $Q sh -- '$<' '$@' \ > RSS_SUPPORT \ > infiniband/verbs.h \ > - enum IBV_EXP_DEVICE_UD_RSS > + enum IBV_EXP_DEVICE_UD_RSS $(AUTOCONF_OUTPUT) > $Q sh -- '$<' '$@' \ > INLINE_RECV \ > infiniband/verbs.h \ > - enum IBV_EXP_DEVICE_ATTR_INLINE_RECV_SZ > + enum IBV_EXP_DEVICE_ATTR_INLINE_RECV_SZ $(AUTOCONF_OUTPUT) > $Q sh -- '$<' '$@' \ > SEND_RAW_WR_SUPPORT \ > infiniband/verbs.h \ > - type 'struct ibv_send_wr_raw' > + type 'struct ibv_send_wr_raw' $(AUTOCONF_OUTPUT) > > mlx4.o: mlx4_autoconf.h > > -- > 2.2.2 Acked-by: Adrien Mazarguil -- Adrien Mazarguil 6WIND
[dpdk-dev] [PATCH] mk: add support for gdb debug info generation
> -Original Message- > From: Marc Sune [mailto:marc.sune at bisdn.de] > Sent: Tuesday, March 03, 2015 2:39 PM > To: Ananyev, Konstantin; Richardson, Bruce > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation > > > On 03/03/15 14:31, Ananyev, Konstantin wrote: > > > >> -Original Message- > >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson > >> Sent: Tuesday, March 03, 2015 1:03 PM > >> To: Marc Sune > >> Cc: dev at dpdk.org > >> Subject: Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info > >> generation > >> > >> On Tue, Mar 03, 2015 at 01:56:19PM +0100, Marc Sune wrote: > >>> On 03/03/15 13:40, Panu Matilainen wrote: > On 03/03/2015 02:19 PM, Marc Sune wrote: > > On 03/03/15 10:33, Bruce Richardson wrote: > >> On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: > >>> On 22/02/15 12:51, Marc Sune wrote: > I don't like the proposed patch, but I am recovering this old thread > because I agree on the problem statement. > > On 04/04/14 11:57, Ananyev, Konstantin wrote: > > Hi Cyril, > > We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you > > can use > > to enable debug, or any other compiler/linker options you need. > > Wonder, why that is not enough? > EXTRA_FLAGS var affects all the DPDK libraries. I was wondering why > setting individually: > > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 2f9643b..04adc0d 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y > # Compile generic ethernet library > # > CONFIG_RTE_LIBRTE_ETHER=y > -CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n > +CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y > > > to put an example, does not set -g and -O0 in that particular module > only. > No one would ever use something compiled in DEBUG in production > anyway. > > I always end up modifying manually Makefiles in the lib library that > I am > interested in having insides, overriding CFLAGS=-O3, which is not > that > nice. > >>> I would like some feedback on this idea. If the community sees > >>> benefit, I > >>> will work on a patch for this. > >>> > >>> Marc > >>> > >> So, your proposal is to patch things so that any time one sets DEBUG=y > >> in the > >> build-time config for a library, we change the '-O3' to '-O0' and set > >> -g also. > >> Correct? > > I am not sure what you mean by 'patch things'. I would simply enable the > > build system to override the default compilation flags (now DPDK-wide, > > or specifically librte_ wide) when _DEBUG=y for a library, changing > > compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I > > have to check if -O0 already implicitly means -fno-inline (even for > > __attribute__((always_inline)) ). > > > > I did a quick test. I chose KNI because it didn't have a DEBUG flag for > > the user-space library. For other libraries, the existing _DEBUG setting > > would be enough: > > > > marc at dpdk:~/dpdk$ git diff HEAD > > diff --git a/config/common_linuxapp b/config/common_linuxapp > > index 97f1c9e..8a3cef8 100644 > > --- a/config/common_linuxapp > > +++ b/config/common_linuxapp > > @@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > > # > > CONFIG_RTE_LIBRTE_KNI=y > > CONFIG_RTE_KNI_PREEMPT_DEFAULT=y > > +CONFIG_RTE_LIBRTE_KNI_DEBUG=y > > CONFIG_RTE_KNI_KO_DEBUG=n > > CONFIG_RTE_KNI_VHOST=n > > CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > > diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile > > index 7107832..895f64e 100644 > > --- a/lib/librte_kni/Makefile > > +++ b/lib/librte_kni/Makefile > > @@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > > # library name > > LIB = librte_kni.a > > > > -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing > > +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) > > > > EXPORT_MAP := rte_kni_version.map > > > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk > > index 63a41e2..eee477d 100644 > > --- a/mk/rte.app.mk > > +++ b/mk/rte.app.mk > > @@ -78,6 +78,13 @@ endif > > ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > > ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > > LDLIBS += -lrte_kni > > + > > +ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) > > +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline > > +else > > +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing > > +endif > > + > > endif >
[dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file
The ENQUEUE_PTRS and DEQUEUE_PTRS macros defined in rte_ring.h are not meant to be global and are not prefixed with the RTE_ prefix. Therefore undef the macros at end of file to avoid pollution of the global namespace, in case ends apps end up wanting to reuse those names. Signed-off-by: Bruce Richardson --- lib/librte_ring/rte_ring.h | 4 1 file changed, 4 insertions(+) diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h index bdf69b7..0d35648 100644 --- a/lib/librte_ring/rte_ring.h +++ b/lib/librte_ring/rte_ring.h @@ -1232,6 +1232,10 @@ rte_ring_dequeue_burst(struct rte_ring *r, void **obj_table, unsigned n) return rte_ring_mc_dequeue_burst(r, obj_table, n); } +/* undef un-prefixed macros which are local to this file */ +#undef ENQUEUE_PTRS +#undef DEQUEUE_PTRS + #ifdef __cplusplus } #endif -- 2.1.0
[dpdk-dev] [PATCH v1 0/5]: Add LRO support to ixgbe PMD
This series adds the missing flow for enabling the LRO in the ethdev and
adds a support for this feature in the ixgbe PMD. There is a big hope that this
initiative is going to be picked up by some Intel developer that would add the
LRO support
to other Intel PMDs. ;)
The series starts with some cleanup work in the code the final patch (the
actual adding of
the LRO support) is going to touch/use/change. There are still quite a few
issues in the ixgbe
PMD code left but they have to be a matter of a different series and I've left
a few "TODO"
remarks in the code.
The LRO ("RSC" in Intel's context) PMD completion handling code follows the
same design as the
corresponding Linux and FreeBSD implementation: pass the aggregation's cluster
HEAD buffer to
the NEXTP entry of the software ring till EOP is met.
HW configuration follows the corresponding specs: this feature is supported
only by x540 and
82599 PF devices.
The feature has been tested with seastar TCP stack with the following
configuration on Tx side:
- MTU: 400B
- 100 concurrent TCP connections.
The results were:
- Without LRO: total throughput: 0.12Gbps, coefficient of variance: 1.41%
- With LRO:total throughput: 8.21Gbps, coefficient of variance: 0.59%
Vlad Zolotarov (5):
ixgbe: Cleanups
ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices
ixgbe: Code refactoring
common_linuxapp: Added CONFIG_RTE_ETHDEV_LRO_SUPPORT option
ixgbe: Add LRO support
config/common_linuxapp | 1 +
lib/librte_ether/rte_ethdev.h | 7 +-
lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 17 +
lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 5 +
lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 724
lib/librte_pmd_ixgbe/ixgbe_rxtx.h | 6 +
6 files changed, 687 insertions(+), 73 deletions(-)
--
2.1.0
[dpdk-dev] [PATCH v1 1/5] ixgbe: Cleanups
- Removed the not needed casting.
- Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW
ring descriptor fields. There were a few places where fields were
accessed/written
directly, which would break on big endian platforms like Power PC.
- ixgbe_dev_rx_init(): shorten the lines by defining a local alias variable
to access
&dev->data->dev_conf.rxmode.
Signed-off-by: Vlad Zolotarov
---
lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 52 +--
1 file changed, 28 insertions(+), 24 deletions(-)
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 3059375..6c0e466 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1028,12 +1028,11 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
struct igb_rx_entry *rxep;
struct rte_mbuf *mb;
uint16_t alloc_idx;
- uint64_t dma_addr;
+ __le64 dma_addr;
int diag, i;
/* allocate buffers in bulk directly into the S/W ring */
- alloc_idx = (uint16_t)(rxq->rx_free_trigger -
- (rxq->rx_free_thresh - 1));
+ alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
rxep = &rxq->sw_ring[alloc_idx];
diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep,
rxq->rx_free_thresh);
@@ -1051,7 +1050,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
mb->port = rxq->port_id;
/* populate the descriptors */
- dma_addr = (uint64_t)mb->buf_physaddr + RTE_PKTMBUF_HEADROOM;
+ dma_addr = rte_cpu_to_le_64(RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb));
rxdp[i].read.hdr_addr = dma_addr;
rxdp[i].read.pkt_addr = dma_addr;
}
@@ -1061,10 +1060,9 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger);
/* update state of internal queue structure */
- rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_trigger +
- rxq->rx_free_thresh);
+ rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh;
if (rxq->rx_free_trigger >= rxq->nb_rx_desc)
- rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_thresh - 1);
+ rxq->rx_free_trigger = rxq->rx_free_thresh - 1;
/* no errors */
return 0;
@@ -1559,13 +1557,14 @@ ixgbe_recv_scattered_pkts(void *rx_queue, struct
rte_mbuf **rx_pkts,
first_seg->ol_flags = pkt_flags;
if (likely(pkt_flags & PKT_RX_RSS_HASH))
- first_seg->hash.rss = rxd.wb.lower.hi_dword.rss;
+ first_seg->hash.rss =
+ rte_le_to_cpu_32(rxd.wb.lower.hi_dword.rss);
else if (pkt_flags & PKT_RX_FDIR) {
first_seg->hash.fdir.hash =
- (uint16_t)((rxd.wb.lower.hi_dword.csum_ip.csum)
- & IXGBE_ATR_HASH_MASK);
+ rte_le_to_cpu_16(rxd.wb.lower.hi_dword.csum_ip.csum)
+ & IXGBE_ATR_HASH_MASK;
first_seg->hash.fdir.id =
- rxd.wb.lower.hi_dword.csum_ip.ip_id;
+ rte_le_to_cpu_16(rxd.wb.lower.hi_dword.csum_ip.ip_id);
}
/* Prefetch data of first segment, if configured to do so. */
@@ -2248,6 +2247,12 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
#ifdef RTE_IXGBE_INC_VECTOR
ixgbe_rxq_vec_setup(rxq);
#endif
+ /*
+* TODO: This must be moved to ixgbe_dev_rx_init() since rx_pkt_burst
+* is a global per-device callback thus bulk allocation may be used
+* only if all queues meet the above preconditions.
+*/
+
/* Check if pre-conditions are satisfied, and no Scattered Rx */
if (!use_def_burst_func && !dev->data->scattered_rx) {
#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
@@ -3523,6 +3528,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
uint32_t rxcsum;
uint16_t buf_size;
uint16_t i;
+ struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
PMD_INIT_FUNC_TRACE();
hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -3545,7 +3551,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
* Configure CRC stripping, if any.
*/
hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
- if (dev->data->dev_conf.rxmode.hw_strip_crc)
+ if (rx_conf->hw_strip_crc)
hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
else
hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP;
@@ -3553,11 +3559,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
/*
* Configure jumbo frame support, if any.
*/
- if (dev->data->dev_c
[dpdk-dev] [PATCH v1 2/5] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices
According to x540 spec chapter 8.2.4.8.9 CRCSTRIP field of RDRXCTL should
be configured to the same value as HLREG0.RXCRCSTRP.
Clearing the RDRXCTL.RSCFRSTSIZE field for x540 is not required by the spec
but seems harmless.
Signed-off-by: Vlad Zolotarov
---
lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 6c0e466..35a88d8 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -3687,7 +3687,8 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
IXGBE_WRITE_REG(hw, IXGBE_RXCSUM, rxcsum);
- if (hw->mac.type == ixgbe_mac_82599EB) {
+ if (hw->mac.type == ixgbe_mac_82599EB ||
+ hw->mac.type == ixgbe_mac_X540) {
rdrxctl = IXGBE_READ_REG(hw, IXGBE_RDRXCTL);
if (rx_conf->hw_strip_crc)
rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
--
2.1.0
[dpdk-dev] [PATCH v1 3/5] ixgbe: Code refactoring
- ixgbe_rx_alloc_bufs():
- Reset the rte_mbuf fields only when requested.
- Take the RDT update out of the function.
- Add the stub when RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC is not defined.
- ixgbe_recv_scattered_pkts():
- Take the code that updates the fields of the cluster's HEAD buffer into
the inline function.
Signed-off-by: Vlad Zolotarov
---
lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 116 +++---
1 file changed, 70 insertions(+), 46 deletions(-)
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 35a88d8..4c67a9e 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1022,7 +1022,7 @@ ixgbe_rx_scan_hw_ring(struct igb_rx_queue *rxq)
}
static inline int
-ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
+ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq, bool reset_mbuf)
{
volatile union ixgbe_adv_rx_desc *rxdp;
struct igb_rx_entry *rxep;
@@ -1043,11 +1043,14 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
for (i = 0; i < rxq->rx_free_thresh; ++i) {
/* populate the static rte mbuf fields */
mb = rxep[i].mbuf;
- rte_mbuf_refcnt_set(mb, 1);
- mb->next = NULL;
+ if (reset_mbuf) {
+ rte_mbuf_refcnt_set(mb, 1);
+ mb->next = NULL;
+ mb->nb_segs = 1;
+ mb->port = rxq->port_id;
+ }
+
mb->data_off = RTE_PKTMBUF_HEADROOM;
- mb->nb_segs = 1;
- mb->port = rxq->port_id;
/* populate the descriptors */
dma_addr = rte_cpu_to_le_64(RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb));
@@ -1055,10 +1058,6 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
rxdp[i].read.pkt_addr = dma_addr;
}
- /* update tail pointer */
- rte_wmb();
- IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger);
-
/* update state of internal queue structure */
rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh;
if (rxq->rx_free_trigger >= rxq->nb_rx_desc)
@@ -1110,7 +1109,9 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
/* if required, allocate new buffers to replenish descriptors */
if (rxq->rx_tail > rxq->rx_free_trigger) {
- if (ixgbe_rx_alloc_bufs(rxq) != 0) {
+ uint16_t cur_free_trigger = rxq->rx_free_trigger;
+
+ if (ixgbe_rx_alloc_bufs(rxq, true) != 0) {
int i, j;
PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
"queue_id=%u", (unsigned) rxq->port_id,
@@ -1130,6 +1131,10 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
return 0;
}
+
+ /* update tail pointer */
+ rte_wmb();
+ IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, cur_free_trigger);
}
if (rxq->rx_tail >= rxq->nb_rx_desc)
@@ -1169,6 +1174,13 @@ ixgbe_recv_pkts_bulk_alloc(void *rx_queue, struct
rte_mbuf **rx_pkts,
return nb_rx;
}
+#else
+static inline int
+ixgbe_rx_alloc_bufs(__rte_unused struct igb_rx_queue *rxq,
+ __rte_unused bool reset_mbuf)
+{
+ return -ENOMEM;
+}
#endif /* RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC */
uint16_t
@@ -1353,6 +1365,51 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts,
return (nb_rx);
}
+/**
+ * Initialize the first mbuf of the returned packet:
+ *- RX port identifier,
+ *- hardware offload data, if any:
+ * - RSS flag & hash,
+ * - IP checksum flag,
+ * - VLAN TCI, if any,
+ * - error flags.
+ * @head HEAD of the packet cluster
+ * @desc HW descriptor to get data from
+ * @port_id Port ID of the Rx queue
+ */
+static inline void ixgbe_fill_cluster_head_buf(
+ struct rte_mbuf *head,
+ union ixgbe_adv_rx_desc *desc,
+ uint8_t port_id,
+ uint32_t staterr)
+{
+ uint32_t hlen_type_rss;
+ uint64_t pkt_flags;
+
+ head->port = port_id;
+
+ /*
+* The vlan_tci field is only valid when PKT_RX_VLAN_PKT is
+* set in the pkt_flags field.
+*/
+ head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
+ hlen_type_rss = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
+ pkt_flags = rx_desc_hlen_type_rss_to_pkt_flags(hlen_type_rss);
+ pkt_flags |= rx_desc_status_to_pkt_flags(staterr);
+ pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
+ head->ol_flags = pkt_flags;
+
+ if (likely(pkt_flags & PKT_RX_RSS_HASH))
+ head->hash.rss = rte_le_to_cpu_32(desc->wb.lower.hi_dword.rss);
+ else if (pkt_flags & PKT_RX_FDIR) {
+ head->hash.fdir.hash =
+ rte_le_to_cpu_16(desc->wb.lower.hi_dword.csum_ip.
[dpdk-dev] [PATCH v1 4/5] common_linuxapp: Added CONFIG_RTE_ETHDEV_LRO_SUPPORT option
Enables LRO support in PMDs. Signed-off-by: Vlad Zolotarov --- config/common_linuxapp | 1 + 1 file changed, 1 insertion(+) diff --git a/config/common_linuxapp b/config/common_linuxapp index 97f1c9e..5b98595 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -137,6 +137,7 @@ CONFIG_RTE_MAX_ETHPORTS=32 CONFIG_RTE_LIBRTE_IEEE1588=n CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16 CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y +CONFIG_RTE_ETHDEV_LRO_SUPPORT=y # # Support NIC bypass logic -- 2.1.0
[dpdk-dev] [PATCH v1 5/5] ixgbe: Add LRO support
- Only x540 and 82599 devices support LRO.
- Add the appropriate HW configuration.
- Add RSC aware rx_pkt_burst() handlers:
- Implemented bulk allocation and non-bulk allocation versions.
- Add LRO-specific fields to rte_eth_rxmode, to rte_eth_dev_data
and to igb_rx_queue.
- Use the appropriate handler when LRO is requested.
Signed-off-by: Vlad Zolotarov
---
lib/librte_ether/rte_ethdev.h | 7 +-
lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 17 ++
lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 5 +
lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 563 +++-
lib/librte_pmd_ixgbe/ixgbe_rxtx.h | 6 +
5 files changed, 591 insertions(+), 7 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 8db3127..f5eff81 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -320,14 +320,15 @@ struct rte_eth_rxmode {
enum rte_eth_rx_mq_mode mq_mode;
uint32_t max_rx_pkt_len; /**< Only used if jumbo_frame enabled. */
uint16_t split_hdr_size; /**< hdr buf size (header_split enabled).*/
- uint8_t header_split : 1, /**< Header Split enable. */
+ uint16_t header_split : 1, /**< Header Split enable. */
hw_ip_checksum : 1, /**< IP/UDP/TCP checksum offload enable.
*/
hw_vlan_filter : 1, /**< VLAN filter enable. */
hw_vlan_strip: 1, /**< VLAN strip enable. */
hw_vlan_extend : 1, /**< Extended VLAN enable. */
jumbo_frame : 1, /**< Jumbo Frame Receipt enable. */
hw_strip_crc : 1, /**< Enable CRC stripping by hardware. */
- enable_scatter : 1; /**< Enable scatter packets rx handler */
+ enable_scatter : 1, /**< Enable scatter packets rx handler */
+ enable_lro : 1; /**< Enable LRO */
};
/**
@@ -1515,6 +1516,8 @@ struct rte_eth_dev_data {
uint8_t port_id; /**< Device [external] port identifier. */
uint8_t promiscuous : 1, /**< RX promiscuous mode ON(1) / OFF(0). */
scattered_rx : 1, /**< RX of scattered packets is ON(1) /
OFF(0) */
+ lro : 1, /**< RX LRO is ON(1) / OFF(0) */
+ lro_bulk_alloc: 1, /**< RX LRO with bulk alloc is ON(1) /
OFF(0) */
all_multicast : 1, /**< RX all multicast mode ON(1) / OFF(0). */
dev_started : 1; /**< Device state: STARTED(1) / STOPPED(0).
*/
};
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 9bdc046..a5a4cb8 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -762,6 +762,14 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct
eth_driver *eth_drv,
if (eth_dev->data->scattered_rx)
eth_dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
+
+ if (eth_dev->data->lro) {
+ if (eth_dev->data->lro_bulk_alloc)
+ eth_dev->rx_pkt_burst =
ixgbe_recv_pkts_lro_bulk_alloc;
+ else
+ eth_dev->rx_pkt_burst = ixgbe_recv_pkts_lro;
+ }
+
return 0;
}
pci_dev = eth_dev->pci_dev;
@@ -1641,6 +1649,8 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
/* Clear stored conf */
dev->data->scattered_rx = 0;
+ dev->data->lro = 0;
+ dev->data->lro_bulk_alloc = 0;
/* Clear recorded link status */
memset(&link, 0, sizeof(link));
@@ -2009,6 +2019,13 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
DEV_RX_OFFLOAD_IPV4_CKSUM |
DEV_RX_OFFLOAD_UDP_CKSUM |
DEV_RX_OFFLOAD_TCP_CKSUM;
+
+#ifdef RTE_ETHDEV_LRO_SUPPORT
+ if (hw->mac.type == ixgbe_mac_82599EB ||
+ hw->mac.type == ixgbe_mac_X540)
+ dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_TCP_LRO;
+#endif
+
dev_info->tx_offload_capa =
DEV_TX_OFFLOAD_VLAN_INSERT |
DEV_TX_OFFLOAD_IPV4_CKSUM |
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
index a549f5c..e206584 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
@@ -349,6 +349,11 @@ uint16_t ixgbe_recv_pkts_bulk_alloc(void *rx_queue, struct
rte_mbuf **rx_pkts,
uint16_t ixgbe_recv_scattered_pkts(void *rx_queue,
struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
+uint16_t ixgbe_recv_pkts_lro(void *rx_queue,
+ struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
+uint16_t ixgbe_recv_pkts_lro_bulk_alloc(void *rx_queue,
+ struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
+
uint16_t ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
b/li
[dpdk-dev] EAL: Error reading from file descriptor 17: Input/output error in DPDK 1.8.0
I do have Intel 8250 Gigabit Ethernet but run into error in testpmd with DPDK 1.8.0. Is this a known issue or I might miss something root at debian:/home/jkao/Media/dpdk-1.8.0# tools/dpdk_nic_bind.py --status Network devices using DPDK-compatible driver :06:04.0 '82540EM Gigabit Ethernet Controller' drv=igb_uio unused= Network devices using kernel driver === :03:00.0 'RTL8111/8168B PCI Express Gigabit Ethernet controller' if=eth0 drv=r8169 unused=igb_uio *Active* Other network devices = root at debian:/home/jkao/Media/dpdk-1.8.0# build/app/testpmd -c7 -n3 | more EAL: Detected lcore 0 as core 0 on socket 0 EAL: Detected lcore 1 as core 1 on socket 0 EAL: Detected lcore 2 as core 2 on socket 0 EAL: Detected lcore 3 as core 3 on socket 0 EAL: Detected lcore 4 as core 0 on socket 0 EAL: Detected lcore 5 as core 1 on socket 0 EAL: Detected lcore 6 as core 2 on socket 0 EAL: Detected lcore 7 as core 3 on socket 0 EAL: Support maximum 128 logical core(s) by configuration. EAL: Detected 8 lcore(s) EAL: Setting up memory... EAL: Ask a virtual area of 0x800 bytes EAL: Virtual area found at 0x7fa36ea0 (size = 0x800) EAL: Requesting 64 pages of size 2MB from socket 0 EAL: TSC frequency is ~2931250 KHz EAL: Master core 0 is ready (tid=77909800) PMD: ENICPMD trace: rte_enic_pmd_init EAL: Core 2 is ready (tid=6d95d700) EAL: Core 1 is ready (tid=6e1ae700) EAL: PCI device :06:04.0 on NUMA socket -1 EAL: probe driver: 8086:100e rte_em_pmd EAL: PCI memory mapped at 0x7fa376a0 EAL: PCI memory mapped at 0x7fa376a2 --More-- PMD: eth_em_dev_init(): port_id 0 vendorID=0x8086 deviceID=0x100e EAL: Error reading from file descriptor 17: Input/output error EAL: Error reading from file descriptor 17: Input/output error Jim
[dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file
2015-03-03 16:38, Bruce Richardson: > The ENQUEUE_PTRS and DEQUEUE_PTRS macros defined in rte_ring.h are > not meant to be global and are not prefixed with the RTE_ prefix. > Therefore undef the macros at end of file to avoid pollution of the > global namespace, in case ends apps end up wanting to reuse those names. > > Signed-off-by: Bruce Richardson > --- > lib/librte_ring/rte_ring.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h > index bdf69b7..0d35648 100644 > --- a/lib/librte_ring/rte_ring.h > +++ b/lib/librte_ring/rte_ring.h > @@ -1232,6 +1232,10 @@ rte_ring_dequeue_burst(struct rte_ring *r, void > **obj_table, unsigned n) > return rte_ring_mc_dequeue_burst(r, obj_table, n); > } > > +/* undef un-prefixed macros which are local to this file */ > +#undef ENQUEUE_PTRS > +#undef DEQUEUE_PTRS > + Thanks for trying to clean-up things. Note that if an application is using this macro name, it will be destroyed when including rte_ring.h. Globally, DPDK namespace is awful and I hope we will be able to improve it.
[dpdk-dev] [PATCH v2] ABI: Add abi checking utility
2015-02-02 13:18, Neil Horman:
> There was a request for an abi validation utiltyfor the ongoing ABI stability
> work. As it turns out there is a abi compliance checker in development that
> seems to be under active development and provides fairly detailed ABI
> compliance
> reports. Its not yet intellegent enough to understand symbol versioning, but
> it
> does provide the ability to identify symbols which have changed between
> releases, along with details of the change, and offers develoeprs the
> opportunity to identify which symbols then need versioning and validation for
> a
> given update via manaul testing.
There's a lot of typos in this text. Please check.
> This script automates the use of the compliance checker between two
> arbitrarily
> specified tags within the dpdk tree. To execute enter the $RTE_SDK directory
> and run:
>
> ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG
>
> where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification
> suitable for passing as the T= variable in the make config command.
>
> Signed-off-by: Neil Horman
>
> Change Notes:
>
> v2) Fixed some typos as requested by Thomas
> ---
> scripts/validate_abi.sh | 241
>
> 1 file changed, 241 insertions(+)
> create mode 100755 scripts/validate_abi.sh
>
> diff --git a/scripts/validate_abi.sh b/scripts/validate_abi.sh
> new file mode 100755
> index 000..31583df
> --- /dev/null
> +++ b/scripts/validate_abi.sh
> @@ -0,0 +1,241 @@
> +#!/bin/sh
> +# BSD LICENSE
> +#
> +# Copyright(c) 2015 Neil Horman. All rights reserved.
> +# All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +#
> +# * Redistributions of source code must retain the above copyright
> +# notice, this list of conditions and the following disclaimer.
> +# * Redistributions in binary form must reproduce the above copyright
> +# notice, this list of conditions and the following disclaimer in
> +# the documentation and/or other materials provided with the
> +# distribution.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +TAG1=$1
> +TAG2=$2
> +TARGET=$3
> +ABI_DIR=`mktemp -d -p /tmp ABI.XX`
> +
> +usage() {
> + echo "$0 "
> +}
> +
> +log() {
> + local level=$1
level is not used later?
> + shift
> + echo "$*"
> +}
> +
> +validate_tags() {
> + git tag -l | grep -q "$TAG1"
> + if [ $? -ne 0 ]
> + then
> + echo "$TAG1 is invalid"
> + return
> + fi
> + git tag -l | grep -q "$TAG2"
> + if [ $? -ne 0 ]
> + then
> + echo "$TAG2 is invalid"
> + return
> + fi
> +}
> +
> +validate_args() {
> + if [ -z "$TAG1" ]
> + then
> + echo "Must Specify TAG1"
> + return
> + fi
> + if [ -z "$TAG2" ]
> + then
> + echo "Must Specify TAG2"
> + return
> + fi
> + if [ -z "$TARGET" ]
> + then
> + echo "Must Specify a build target"
> + fi
> +}
> +
> +
> +cleanup_and_exit() {
> + rm -rf $ABI_DIR
> + exit $1
> +}
This function could be automatically invoked with trap.
> +###
> +#START
> +
> +
> +#Save the current branch
> +CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
Will it work when not on any branch?
> +
> +if [ -n "$VERBOSE" ]
> +then
> + export VERBOSE=/dev/stdout
> +else
> + export VERBOSE=/dev/null
> +fi
> +
> +# Validate that we have all the arguments we need
> +res=$(validate_args)
> +if [ -n "$res" ]
> +then
> + echo $res
Should be redirected to stderr >&2
> + usage
> + cleanup_and_exit 1
> +fi
> +
> +# Make sure our tags exist
> +res=$(validate_tags)
> +if [ -n "$res" ]
> +then
> + echo $res
> + cleanup_and_exit 1
> +fi
> +
> +ABICHECK=`which abi-compliance-checker 2>/dev/null`
Why not using the $() form like above?
I guess this is the tool:
http://ispras.linuxbase.org/index.php/ABI_compliance_checker
> +i
[dpdk-dev] [PATCH] eal/linux: fix build
> > Compilation fails in some distributions because of missing unistd.h
> > needed for pread/pwrite (seen with Suse):
> > lib/librte_eal/linuxapp/eal/eal_pci_uio.c:62:2:
> > error: implicit declaration of function ?pread?
> >
> > Fixes: 4a499c649590 ("eal/linux: enable uio_pci_generic support")
> >
> > Signed-off-by: Thomas Monjalon
> Acked-by: David Marchand
Applied
[dpdk-dev] [PATCH] testpmd: HW vlan command
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ouyang Changchun > Sent: Friday, February 13, 2015 12:04 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH] testpmd: HW vlan command > > This patch enables testpmd user can config port hw_vlan with more fine > granularity: > hw vlan filter, hw vlan strip, and hw vlan extend. > > Don't remove the original command(hw-vlan) considering that some user still > want to use > only one command to switch on/off all 3 options. > > Signed-off-by: Changchun Ouyang Acked-by: Pablo de Lara
[dpdk-dev] [PATCH v1 5/5] ixgbe: Add LRO support
On Tue, 3 Mar 2015 21:48:43 +0200 Vlad Zolotarov wrote: > + lro_bulk_alloc: 1, /**< RX LRO with bulk alloc is ON(1) / > OFF(0) */ This is an internal decision and should not be exposed in the API. We need less knobs not more.
[dpdk-dev] [PATCH v1 5/5] ixgbe: Add LRO support
On Tue, 3 Mar 2015 21:48:43 +0200 Vlad Zolotarov wrote: > + next_desc: > + /* > + * The code in this whole file uses the volatile pointer to > + * ensure the read ordering of the status and the rest of the > + * descriptor fields (on the compiler level only!!!). This is so > + * UGLY - why not to just use the compiler barrier instead? DPDK > + * even has the rte_compiler_barrier() for that. > + * > + * But most importantly this is just wrong because this doesn't > + * ensure memory ordering in a general case at all. For > + * instance, DPDK is supposed to work on Power CPUs where > + * compiler barrier may just not be enough! > + * > + * I tried to write only this function properly to have a > + * starting point (as a part of an LRO/RSC series) but the > + * compiler cursed at me when I tried to cast away the > + * "volatile" from rx_ring (yes, it's volatile too!!!). So, I'm > + * keeping it the way it is for now. > + * > + * The code in this file is broken in so many other places and > + * will just not work on a big endian CPU anyway therefore the > + * lines below will have to be revisited together with the rest > + * of the ixgbe PMD. > + * > + * TODO: > + *- Get rid of "volatile" crap and let the compiler do its > + * job. > + *- Use the proper memory barrier (rte_rmb()) to ensure the > + * memory ordering below. This comment screams "this is broken". Why not get proper architecture independent barriers in DPDK first.
[dpdk-dev] [PATCH v1 5/5] ixgbe: Add LRO support
On Tue, 3 Mar 2015 21:48:43 +0200
Vlad Zolotarov wrote:
> +
> + if (!bulk_alloc) {
> + __le64 dma =
> + rte_cpu_to_le_64(RTE_MBUF_DATA_DMA_ADDR_DEFAULT(nmb));
> + /*
> + * Update RX descriptor with the physical address of the
> + * new data buffer of the new allocated mbuf.
> + */
> + rxe->mbuf = nmb;
> +
> +rxm->data_off = RTE_PKTMBUF_HEADROOM;
Incorrect indentation.
[dpdk-dev] [PATCH v1 5/5] ixgbe: Add LRO support
On Tue, 3 Mar 2015 21:48:43 +0200
Vlad Zolotarov wrote:
> - Only x540 and 82599 devices support LRO.
> - Add the appropriate HW configuration.
> - Add RSC aware rx_pkt_burst() handlers:
>- Implemented bulk allocation and non-bulk allocation versions.
>- Add LRO-specific fields to rte_eth_rxmode, to rte_eth_dev_data
> and to igb_rx_queue.
>- Use the appropriate handler when LRO is requested.
>
> Signed-off-by: Vlad Zolotarov
Checkpatch warnings (edited to remove ones that should be ok)
WARNING: 'recieved' may be misspelled - perhaps 'received'?
#196: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1426:
+ * @rx_pkts table of recieved packets
WARNING: Missing a blank line after declarations
#223: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1453:
+ struct igb_rx_queue *rxq = rx_queue;
+ volatile union ixgbe_adv_rx_desc *rx_ring = rxq->rx_ring;
WARNING: labels should not be indented
#246: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1476:
+ next_desc:
WARNING: quoted string split across lines
#285: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1515:
+ PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
+ "staterr=0x%x data_len=%u",
WARNING: quoted string split across lines
#293: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1523:
+ PMD_RX_LOG(DEBUG, "RX mbuf alloc failed "
+ "port_id=%u queue_id=%u",
WARNING: Missing a blank line after declarations
#302: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1532:
+ uint16_t next_rdt = rxq->rx_free_trigger;
+ if (!ixgbe_rx_alloc_bufs(rxq, false)) {
WARNING: quoted string split across lines
#309: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1539:
+ PMD_RX_LOG(DEBUG, "RX bulk alloc failed "
+ "port_id=%u queue_id=%u",
ERROR: code indent should use tabs where possible
#350: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1580:
+rxm->data_off = RTE_PKTMBUF_HEADROOM;$
WARNING: please, no spaces at the start of a line
#350: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1580:
+rxm->data_off = RTE_PKTMBUF_HEADROOM;$
WARNING: quoted string split across lines
#452: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1682:
+ PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_tail=%u "
+ "nb_hold=%u nb_rx=%u",
WARNING: quoted string split across lines
#536: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2580:
+ PMD_INIT_LOG(DEBUG, "sw_ring=%p sw_rsc_ring=%p hw_ring=%p "
+ "dma_addr=0x%"PRIx64,
WARNING: Possible switch case/default not preceeded by break or fallthrough
comment
#617: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:3926:
+ default:
WARNING: quoted string split across lines
#648: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:3966:
+ PMD_INIT_LOG(CRIT, "LRO is requested on HW that doesn't "
+ "support it");
WARNING: quoted string split across lines
#693: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4022:
+ PMD_INIT_LOG(CRIT, "LRO can't be enabled when HW CRC "
+ "is disabled");
WARNING: quoted string split across lines
#711: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4084:
+ PMD_INIT_LOG(INFO, "split_hdr_size less than "
+ "128 bytes (%d)!",
WARNING: quoted string split across lines
#835: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4267:
+ PMD_INIT_LOG(INFO, "LRO is requested. Using a bulk "
+ "allocation version");
WARNING: quoted string split across lines
#840: FILE: lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4272:
+ PMD_INIT_LOG(INFO, "LRO is requested. Using a single "
+ "allocation version");
[dpdk-dev] [PATCH v6 0/8] Interrupt mode PMD
On Fri, 27 Feb 2015 11:38:25 +0100 David Marchand wrote: > Ok, so after looking at this patchset, I would say this is the right > direction, but still this is too limited. > The ethdev part and the vfio eventfds part look acceptable to me. > But thinking about it, I could just reuse a standard event library with the > eventfds I would get from ethdev without a need for a new eal api. I would prefer that there was just an fd and a callback. An application should be able to use what ever event model or library it wants. IMHO the existing interrupt thread model is incorrectly designed and creates lots of opportunities for races because of that. Look at the effort it has to use to pass the event back to link state code.
