[dpdk-dev] [PATCH] rte_mbuf: scattered pktmbufs freeing optimization

2015-03-07 Thread Vadim Suraev
Hi, Olivier,
I realized that if local cache for the mempool is enabled and greater than
0,
if, say, the mempool size is X and local cache length is Y (and it is not
empty,Y>0)
an attempt to allocate a bulk, whose size is greater than local cache size
(max) and greater than X-Y (which is the number of entries in the ring)
will fail.
The reason is:
__mempool_get_bulk will check whether the bulk to be allocated is greater
than mp->cache_size and will fall to ring_dequeue.
And the ring does not contain enough entries in this case while the sum of
ring entries + cache length may be greater or equal to the bulk's size, so
theoretically the bulk could be allocated.
Is it an expected behaviour? Am I missing something?
By the way, rte_mempool_count returns a ring count + sum of all local
caches, IMHO it could mislead, even twice.
Regards,
 Vadim.

On Wed, Mar 4, 2015 at 10:54 AM, Olivier MATZ 
wrote:

> Hi Vadim,
>
> On 02/27/2015 06:09 PM, Vadim Suraev wrote:
>
>>  >Indeed, this function looks useful, and I also have a work in progress
>>  >on this topic, but currently it is not well tested.
>> I'm sorry, I didn't know. I'll not interfere with my patch))
>>
>
> That not what I wanted to say :)
>
> You are very welcome with your patch, I just wanted to notify
> that I am also working in the same area and that's why I listed
> the things I'm currently working on.
>
>
>   >About the inlining, I have no objection now, although Stephen may be
>>  >right. I think we can consider un-inline some functions, based on
>>  >performance measurements.
>> I've also noticed in many cases it makes no difference. Seems to be some
>> trade-off.
>>
>>  >- clarify the difference between raw_alloc/raw_free and
>>  >  mempool_get/mempool_put: For instance, I think that the reference
>>  >  counter initialization should be managed by rte_pktmbuf_reset() like
>>  >  the rest of the fields, therefore raw_alloc/raw_free could be replaced
>> It looks useful to me since not all of the fields are used in every
>> particular application so
>> if the allocation is decoupled from reset, one can save some cycles.
>>
>
> Yes, this is also a trade-off between maintainability and speed, and
> speed is probably the first constraint for the dpdk. But maybe we can
> find an alternative that is both fast and maintainable.
>
> Thanks,
> Olivier
>
>


[dpdk-dev] [PATCH] librte_pmd_e1000: power down the serdes link

2015-03-07 Thread Shelton Chia
Signed-off-by: Shelton Chia 
---
 lib/librte_pmd_e1000/igb_ethdev.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_pmd_e1000/igb_ethdev.c 
b/lib/librte_pmd_e1000/igb_ethdev.c
index 504ae74..314ef2a 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -948,7 +948,10 @@ eth_igb_stop(struct rte_eth_dev *dev)
}

/* Power down the phy. Needed to make the link go Down */
-   e1000_power_down_phy(hw);
+   if (hw->phy.media_type == e1000_media_type_copper)
+   e1000_power_down_phy(hw);
+   else
+   e1000_shutdown_fiber_serdes_link(hw);

igb_dev_clear_queues(dev);

-- 
2.3.0



[dpdk-dev] [PATCH 1/2] virtio: initialize iopl when device is initialized

2015-03-07 Thread David Marchand
On Sat, Mar 7, 2015 at 12:43 AM, Stephen Hemminger <
stephen at networkplumber.org> wrote:

> On Fri, 6 Mar 2015 23:04:33 +0100
> David Marchand  wrote:
>
> > In the end, the fix would be to move rte_eal_intr_init() after
> rte_eal_dev_init().
>
>
> That would work, but was worried it would break other devices.
>

I had checked the pmds before proposing.

rte_intr_* api is called only from the pdev (meaning pci) drivers at the
moment.

rte_eal_dev_init() means we are still in the "pmd init" phase for the pdev
drivers which means we are still initialising per-driver things for them
since they have no idea of the devices to handle (the devices are
discovered later with the pci scan).
In this phase, the code tells that they are not registering interrupts (and
I don't see how they could register interrupts without knowing devices).

The only problem would be for future vdev drivers (current drivers do not
use rte_intr_*).
But this problem would occur because the driver/device datamodel is not
consistent.
If we could rework this to have devices instantiated at the same phase,
then the problem won't happen.
So, we can hope we will have reworked the driver/device datamodel in dpdk,
before this problem hits us.


I only had checked the pmds, so I may have missed something else that uses
interrupts (if any) but I think this is worth trying and fixing for 2.0.
Thomas, opinion ?


-- 
David Marchand


[dpdk-dev] [PATCH 1/3 v2] librte_hash: Fix unsupported instruction `crc32' in i686 platform

2015-03-07 Thread Thomas Monjalon
2015-03-06 01:39, Qiu, Michael:
> On 3/6/2015 1:11 AM, Thomas Monjalon wrote:
> > 2015-03-06 00:55, Michael Qiu:
> >> CC rte_hash.o
> >> Error: unsupported instruction `crc32'
> >>
> >> The root cause is that i686 platform does not support 'crc32q'
> >> Need make it only available in x86_64 platform
> >>
> >> Signed-off-by: Michael Qiu 
> >> ---
> >> v2 --> v1:
> >>  Make crc32 instruction only works in X86 platform
> >>  lib/librte_hash/rte_hash_crc.h | 12 
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/lib/librte_hash/rte_hash_crc.h 
> >> b/lib/librte_hash/rte_hash_crc.h
> >> index d28bb2a..c0a789e 100644
> >> --- a/lib/librte_hash/rte_hash_crc.h
> >> +++ b/lib/librte_hash/rte_hash_crc.h
> >> @@ -364,6 +364,7 @@ crc32c_2words(uint64_t data, uint32_t init_val)
> >>return crc;
> >>  }
> >>  
> >> +#if defined RTE_ARCH_I686 || defined RTE_ARCH_X86_64
> >>  static inline uint32_t
> >>  crc32c_sse42_u32(uint32_t data, uint32_t init_val)
> >>  {
> >> @@ -373,7 +374,9 @@ crc32c_sse42_u32(uint32_t data, uint32_t init_val)
> >>: [data] "rm" (data));
> >>return init_val;
> >>  }
> >> +#endif
> > 
> > Wouldn't it be more elegant to define a stub which returns 0 in #else
> > in order to remove #ifdef below?
> > Not sure, matter of taste.
> 
> It may be not a good idea, see rte_hash_crc_8byte(), if no crc32
> support, it will use crc32c_2words(), if we define a stub which returns
> 0 in #else, then we need always check the return value whether it is
> none-zero otherwise need fallback.

I don't think so.
The stub won't never been called because they are protected by the cpuflag
condition.



[dpdk-dev] [PATCH] Move mk/rte.extvars.mk to mk/internal/rte.extvars.mk

2015-03-07 Thread Thomas Monjalon
2015-03-06 09:46, Olivier MATZ:
> Hi Keith,
> 
> On 03/04/2015 06:13 PM, Keith Wiles wrote:
> > Move the rte.extvars.mk to an internal directory and
> > update rte.vars.mk to find the file in the new location.
> > 
> > Signed-off-by: Keith Wiles 
> 
> Acked-by: Olivier Matz 
> (for after 2.0 I guess as it's not a fix)

It's a good clean-up without risk. It should go in.

> Note for Thomas: when you apply it, git complains there is a blank
> line at the end of the file that is moved, I think you can strip it:
> 
>   dpdk/.git/rebase-apply/patch:94: new blank line at EOF.
>   +

OK, thanks



[dpdk-dev] [PATCH v3 0/2] testpmd: check return value of rte_eth_dev_vlan_filter()

2015-03-07 Thread De Lara Guarch, Pablo


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Michal Jastrzebski
> Sent: Friday, February 20, 2015 10:26 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v3 0/2] testpmd: check return value of
> rte_eth_dev_vlan_filter()
> 
> v3 changes:
> - add seperate patch with testpmd_funcs.rst changes
> 
> v2 changes:
> - fix formatting errors
> 
> This patchset modifies testpmd behavior when setting:
> rx_vlan add all vf_port (enabling all vlanids
> to be passed thru rx filter on VF).
> Rx_vlan_all_filter_set() function,
> checks if the next vlanid can be enabled by the driver.
> Number of vlanids is limited by the NIC and thus the NIC
> do not allow to enable more vlanids than it can allocate
> in VFTA table.
> tespmd_funcs.rst file is modified to provide a brief description
> why enabling all vlan ids may not be possible.
> 
> Michal Jastrzebski (2):
>   doc: add information about limited number of vlan_ids
>   testpmd: check return value of rte_eth_dev_vlan_filter()
> 
>  app/test-pmd/config.c   |   15 +--
>  app/test-pmd/testpmd.h  |2 +-
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |2 ++
>  lib/librte_ether/rte_ethdev.c   |4 ++--
>  4 files changed, 14 insertions(+), 9 deletions(-)
> 
> --
> 1.7.9.5

Acked-by: Pablo de Lara 


[dpdk-dev] [PATCH] virtio: Add default_txconf

2015-03-07 Thread De Lara Guarch, Pablo


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Takuya ASADA
> Sent: Monday, February 23, 2015 7:51 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] virtio: Add default_txconf
> 
> When I tried to launch test-pmd on KVM guest of Fedora21, I got following
> error:
> 
> Configuring Port 0 (socket 0)
> Fail to configure port 0 tx queues
> EAL: Error - exiting with code: 1
>   Cause: Start ports failed
> 
> I found that the error caused here, and actual error message was "TX
> checksum offload not supported":
> http://dpdk.org/browse/dpdk/tree/lib/librte_pmd_virtio/virtio_rxtx.c#n425
> 
> This patch adds default_txconf on virtio pmd, to avoid the error.
> 
> Signed-off-by: Takuya ASADA 
> ---
>  lib/librte_pmd_virtio/virtio_ethdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c
> b/lib/librte_pmd_virtio/virtio_ethdev.c
> index b3b5bb6..9c183bb 100644
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -1188,6 +1188,9 @@ virtio_dev_info_get(struct rte_eth_dev *dev,
> struct rte_eth_dev_info *dev_info)
>   dev_info->min_rx_bufsize = VIRTIO_MIN_RX_BUFSIZE;
>   dev_info->max_rx_pktlen = VIRTIO_MAX_RX_PKTLEN;
>   dev_info->max_mac_addrs = VIRTIO_MAX_MAC_ADDRS;
> + dev_info->default_txconf = (struct rte_eth_txconf) {
> + .txq_flags = ETH_TXQ_FLAGS_NOOFFLOADS
> + };
>  }
> 
>  /*
> --
> 2.1.0

Acked-by: Pablo de Lara