[dpdk-dev] [PATCH] lib/librte_vhost: exchange kickfd and callfd to avoid confusion

[Ouyang, Changchun]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie
> Sent: Friday, March 6, 2015 6:39 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] lib/librte_vhost: exchange kickfd and callfd to
> avoid confusion
> 
> Previous vhost implementation wrongly name kickfd as callfd and callfd as
> kickfd.
> It is functional correct, but causes confusion.
> 
> Signed-off-by: Huawei Xie 

Acked-by: Changchun Ouyang 

[dpdk-dev] [PATCH] test whether file descriptor is valid before close it

[Ouyang, Changchun]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie
> Sent: Friday, March 6, 2015 7:06 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] test whether file descriptor is valid before close
> it
> 
> This avoids closing -1 in our case.
> 
> Signed-off-by: Huawei Xie 

Acked-by: Changchun Ouyang 

[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian

[Xuelin Shi]

Hi Bruce,

Yes, the patch http://dpdk.org/dev/patchwork/patch/3132/ should be abandoned.

Thanks,
Shi xuelin

> -Original Message-
> From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> Sent: Friday, March 06, 2015 19:14
> To: Shi Xuelin-B29237
> Cc: thomas.monjalon at 6wind.com; dev at dpdk.org
> Subject: Re: [PATCH] librte_lpm: define tbl entry reversely for big
> endian
> 
> On Thu, Mar 05, 2015 at 02:12:12AM +, Xuelin Shi wrote:
> > Hi Bruce,
> >
> > Yes, it needs to swap the fields. The bit field is first identified as
> the uint8_t and then packed.
> >
> > Thanks,
> > Shi xuelin
> >
> Am I right in thinking that this patch set supercedes that for
> "lpm: use field access instead of type conversion"
> http://dpdk.org/dev/patchwork/patch/3132/ ?
> 
> > > -Original Message-
> > > From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> > > Sent: Wednesday, March 04, 2015 18:48
> > > To: Shi Xuelin-B29237
> > > Cc: thomas.monjalon at 6wind.com; dev at dpdk.org
> > > Subject: Re: [PATCH] librte_lpm: define tbl entry reversely for big
> > > endian
> > >
> > > On Wed, Mar 04, 2015 at 02:34:12PM +0800, xuelin.shi at freescale.com
> wrote:
> > > > From: Xuelin Shi 
> > > >
> > > > This module uses type conversion between struct and int.
> > > > Also truncation and comparison is used with this int.
> > > > It is not safe for different endian arch.
> > > >
> > > > Add ifdef for big endian struct to fix this issue.
> > > >
> > > > Signed-off-by: Xuelin Shi 
> > > > ---
> > > >  lib/librte_lpm/rte_lpm.h | 19 +++
> > > >  1 file changed, 19 insertions(+)
> > > >
> > > > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h
> > > > index
> > > > 1af150c..08a2859 100644
> > > > --- a/lib/librte_lpm/rte_lpm.h
> > > > +++ b/lib/librte_lpm/rte_lpm.h
> > > > @@ -96,6 +96,7 @@ extern "C" {
> > > >  /** Bitmask used to indicate successful lookup */
> > > >  #define RTE_LPM_LOOKUP_SUCCESS  0x0100
> > > >
> > > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > >  /** @internal Tbl24 entry structure. */  struct
> rte_lpm_tbl24_entry {
> > > > /* Stores Next hop or group index (i.e. gindex)into tbl8. */
> @@
> > > > -117,6 +118,24 @@ struct rte_lpm_tbl8_entry {
> > > > uint8_t valid_group :1; /**< Group validation flag. */
> > > > uint8_t depth   :6; /**< Rule depth. */
> > > >  };
> > > > +#else
> > > > +struct rte_lpm_tbl24_entry {
> > > > +   uint8_t depth   :6;
> > > > +   uint8_t ext_entry   :1;
> > > > +   uint8_t valid   :1;
> > >
> > > Since endianness only refers to the order of bytes within a word, do
> > > the bitfields within the uint8_t really need to be swapped around too?
> > >
> > > /Bruce
> >
> > >
> > > > +   union {
> > > > +   uint8_t tbl8_gindex;
> > > > +   uint8_t next_hop;
> > > > +   };
> > > > +};
> > > > +
> > > > +struct rte_lpm_tbl8_entry {
> > > > +   uint8_t depth   :6;
> > > > +   uint8_t valid_group :1;
> > > > +   uint8_t valid   :1;
> > > > +   uint8_t next_hop;
> > > > +};
> > > > +#endif
> > > >
> > > >  /** @internal Rule structure. */
> > > >  struct rte_lpm_rule {
> > > > --
> > > > 1.9.1
> > > >

[dpdk-dev] [PATCH] testpmd: Fix port validation code of "port stop all" command

[Tetsuya Mukawa]

On 2015/03/06 22:53, De Lara Guarch, Pablo wrote:
> Hi Michael,
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Qiu, Michael
>> Sent: Thursday, March 05, 2015 1:33 PM
>> To: Tetsuya Mukawa; dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] testpmd: Fix port validation code of "port
>> stop all" command
>>
>> Hi, Tetsuya and Pablo
>> This is not a full fix, I have generate the full fix patch two days ago,

Hi Michel,

I am sorry for late replying, and thanks for your work.

> Sorry I did not see this earlier. Did you upstream this patch already?
> I acked Tetsuya's patch, as it was simple and works, but I cannot find
> this one.
>
> Thanks,
> Pablo
>
>> See below:
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 49be819..ec53923 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -384,6 +384,9 @@ port_infos_display(portid_t port_id)
>>  int
>>  port_id_is_invalid(portid_t port_id, enum print_warning warning)
>>  {
>> +   if (port_id == (portid_t)RTE_PORT_ALL)
>> +   return 0;
>> +

I am not clearly sure that we need to add above 'if statement'.

>> if (ports[port_id].enabled)
>> return 0;
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index e556b4c..1c4c651 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -1326,6 +1326,9 @@ start_port(portid_t pid)
>> return -1;
>> }
>>
>> +   if (port_id_is_invalid(pid, ENABLED_WARN))
>> +   return 0;
>> +

Same as above.

>> if (init_fwd_streams() < 0) {
>> printf("Fail from init_fwd_streams()\n");
>> return -1;
>> @@ -1482,10 +1485,14 @@ stop_port(portid_t pid)
>> dcb_test = 0;
>> dcb_config = 0;
>> }
>> +
>> +   if (port_id_is_invalid(pid, ENABLED_WARN))
>> +   return;
>> +

Same as above.

>> printf("Stopping ports...\n");
>>
>> FOREACH_PORT(pi, ports) {
>> -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
>> +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>> continue;
>>
>> port = &ports[pi];
>> @@ -1517,10 +1524,13 @@ close_port(portid_t pid)
>> return;
>> }
>>
>> +   if (port_id_is_invalid(pid, ENABLED_WARN))
>> +return;
>> +

Same as above.

>> printf("Closing ports...\n");
>>
>> FOREACH_PORT(pi, ports) {
>> -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
>> +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>> continue;
>>
>> port = &ports[pi];
>> --
>> 1.9.3

FOREACH_PORT() returns valid ports, so is it not enough to check like above?
I am not clearly understand which case we need to add above extra if
statements.
Could you please describe?

But I agree we cannot use my previous patch.
We need to fix not only stop_port() but also close_port() like start_port().

Thanks,
Tetsuya

>> Thanks,
>> Michael
>>
>> On 3/5/2015 3:31 PM, Tetsuya Mukawa wrote:
>>> When "port stop all" is executed, the command doesn't work as it should
>>> because of wrong port validation. The patch fixes this issue.
>>>
>>> Reported-by: Pablo de Lara 
>>> Signed-off-by: Tetsuya Mukawa 
>>> ---
>>>  app/test-pmd/testpmd.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>> index 61291be..bb65342 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -1484,7 +1484,7 @@ stop_port(portid_t pid)
>>> printf("Stopping ports...\n");
>>>
>>> FOREACH_PORT(pi, ports) {
>>> -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
>>> +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>>> continue;
>>>
>>> port = &ports[pi];

[dpdk-dev] [PATCH] test whether file descriptor is valid before close it

[Tetsuya Mukawa]

On 2015/03/06 20:05, Huawei Xie wrote:
> This avoids closing -1 in our case.
>
> Signed-off-by: Huawei Xie 
> ---
>  lib/librte_vhost/virtio-net.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index 6917fcf..4672e67 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -185,13 +185,13 @@ cleanup_device(struct virtio_net *dev)
>   }
>  
>   /* Close any event notifiers opened by device. */
> - if (dev->virtqueue[VIRTIO_RXQ]->callfd)
> + if ((int)dev->virtqueue[VIRTIO_RXQ]->callfd >= 0)
>   close((int)dev->virtqueue[VIRTIO_RXQ]->callfd);
> - if (dev->virtqueue[VIRTIO_RXQ]->kickfd)
> + if ((int)dev->virtqueue[VIRTIO_RXQ]->kickfd >= 0)
>   close((int)dev->virtqueue[VIRTIO_RXQ]->kickfd);
> - if (dev->virtqueue[VIRTIO_TXQ]->callfd)
> + if ((int)dev->virtqueue[VIRTIO_TXQ]->callfd >= 0)
>   close((int)dev->virtqueue[VIRTIO_TXQ]->callfd);
> - if (dev->virtqueue[VIRTIO_TXQ]->kickfd)
> + if ((int)dev->virtqueue[VIRTIO_TXQ]->kickfd >= 0)
>   close((int)dev->virtqueue[VIRTIO_TXQ]->kickfd);
>  }
>  
Acked-by: Tetsuya Mukawa 

[dpdk-dev] [PATCH] lib/librte_vhost: exchange kickfd and callfd to avoid confusion

[Tetsuya Mukawa]

On 2015/03/06 19:39, Huawei Xie wrote:
> Previous vhost implementation wrongly name kickfd as callfd and callfd as 
> kickfd.
> It is functional correct, but causes confusion.
>
> Signed-off-by: Huawei Xie 
> ---
>  examples/vhost/main.c |  6 +++---
>  lib/librte_vhost/rte_virtio_net.h |  4 ++--
>  lib/librte_vhost/vhost_rxtx.c |  6 +++---
>  lib/librte_vhost/vhost_user/virtio-net-user.c | 12 ++--
>  lib/librte_vhost/virtio-net.c | 12 ++--
>  5 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 334e2fe..61ea671 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -1434,7 +1434,7 @@ put_desc_to_used_list_zcp(struct vhost_virtqueue *vq, 
> uint16_t desc_idx)
>  
>   /* Kick the guest if necessary. */
>   if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
> - eventfd_write((int)vq->kickfd, 1);
> + eventfd_write((int)vq->callfd, 1);
>  }
>  
>  /*
> @@ -1627,7 +1627,7 @@ txmbuf_clean_zcp(struct virtio_net *dev, struct vpool 
> *vpool)
>  
>   /* Kick guest if required. */
>   if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
> - eventfd_write((int)vq->kickfd, 1);
> + eventfd_write((int)vq->callfd, 1);
>  
>   return 0;
>  }
> @@ -1775,7 +1775,7 @@ virtio_dev_rx_zcp(struct virtio_net *dev, struct 
> rte_mbuf **pkts,
>  
>   /* Kick the guest if necessary. */
>   if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
> - eventfd_write((int)vq->kickfd, 1);
> + eventfd_write((int)vq->callfd, 1);
>  
>   return count;
>  }
> diff --git a/lib/librte_vhost/rte_virtio_net.h 
> b/lib/librte_vhost/rte_virtio_net.h
> index 611a3d4..2fc1c44 100644
> --- a/lib/librte_vhost/rte_virtio_net.h
> +++ b/lib/librte_vhost/rte_virtio_net.h
> @@ -86,8 +86,8 @@ struct vhost_virtqueue {
>   uint16_tvhost_hlen; /**< Vhost header 
> length (varies depending on RX merge buffers. */
>   volatile uint16_t   last_used_idx;  /**< Last index used on 
> the available ring */
>   volatile uint16_t   last_used_idx_res;  /**< Used for multiple 
> devices reserving buffers. */
> - eventfd_t   callfd; /**< Currently unused 
> as polling mode is enabled. */
> - eventfd_t   kickfd; /**< Used to notify the 
> guest (trigger interrupt). */
> + eventfd_t   callfd; /**< Used to notify the 
> guest (trigger interrupt). */
> + eventfd_t   kickfd; /**< Currently unused 
> as polling mode is enabled. */
>   struct buf_vector   buf_vec[BUF_VECTOR_MAX];/**< for 
> scatter RX. */
>  } __rte_cache_aligned;
>  
> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
> index c7c9550..535c7a1 100644
> --- a/lib/librte_vhost/vhost_rxtx.c
> +++ b/lib/librte_vhost/vhost_rxtx.c
> @@ -180,7 +180,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
>  
>   /* Kick the guest if necessary. */
>   if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
> - eventfd_write((int)vq->kickfd, 1);
> + eventfd_write((int)vq->callfd, 1);
>   return count;
>  }
>  
> @@ -507,7 +507,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t 
> queue_id,
>  
>   /* Kick the guest if necessary. */
>   if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
> - eventfd_write((int)vq->kickfd, 1);
> + eventfd_write((int)vq->callfd, 1);
>   }
>  
>   return count;
> @@ -725,6 +725,6 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t 
> queue_id,
>   vq->used->idx += entry_success;
>   /* Kick guest if required. */
>   if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
> - eventfd_write((int)vq->kickfd, 1);
> + eventfd_write((int)vq->callfd, 1);
>   return entry_success;
>  }
> diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c 
> b/lib/librte_vhost/vhost_user/virtio-net-user.c
> index 97c5177..e0c7394 100644
> --- a/lib/librte_vhost/vhost_user/virtio-net-user.c
> +++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
> @@ -286,13 +286,13 @@ user_get_vring_base(struct vhost_device_ctx ctx,
>* sent and only sent in vhost_vring_stop.
>* TODO: cleanup the vring, it isn't usable since here.
>*/
> - if (((int)dev->virtqueue[VIRTIO_RXQ]->callfd) >= 0) {
> - close(dev->virtqueue[VIRTIO_RXQ]->callfd);
> - dev->virtqueue[VIRTIO_RXQ]->callfd = (eventfd_t)-1;
> + if (((int)dev->virtqueue[VIRTIO_RXQ]->kickfd) >= 0) {
> + close(dev->virtqueue[VIRTIO_RXQ]->kickfd);
> + dev->virtqueue[VIRTIO_RXQ]->kickfd = (eventfd_t)-1;
>   }
> - if (((int)dev->virtqueue[V

[dpdk-dev] [PATCH 1/3] BSD: Support Port Hotplug function

[Tetsuya Mukawa]

On 2015/03/06 19:32, Iremonger, Bernard wrote:
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tetsuya Mukawa
>> Sent: Wednesday, March 4, 2015 3:12 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH 1/3] BSD: Support Port Hotplug function
>>
>> This patch adds Hotplug support to BSD.
>>
>> Signed-off-by: Tetsuya Mukawa 
>> ---
>>  lib/librte_eal/bsdapp/eal/eal_pci.c   | 169 
>> +-
>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map |   6 +
>>  lib/librte_eal/common/include/rte_pci.h   |   1 +
>>  lib/librte_ether/rte_ethdev.c |   1 +
>>  4 files changed, 174 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c 
>> b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> index 9193f80..fc5a088 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> @@ -156,6 +156,26 @@ fail:
>>  return NULL;
>>  }
>>
>> +/* unmap a particular resource */
>> +static int
>> +pci_unmap_resource(void *requested_addr, size_t size) {
>> +if (requested_addr == NULL)
>> +return -EINVAL;
>> +
>> +/* Unmap the PCI memory resource of device */
>> +if (munmap(requested_addr, size)) {
>> +RTE_LOG(ERR, EAL, "%s(): cannot munmap(%p, 0x%lx): %s\n",
>> +__func__, requested_addr, (unsigned long)size,
>> +strerror(errno));
>> +} else {
>> +RTE_LOG(DEBUG, EAL, "  PCI memory unmapped at %p\n",
>> +requested_addr);
>> +return -EFAULT;
>> +}
>> +return 0;
>> +}
>> +
>>  static int
>>  pci_uio_map_secondary(struct rte_pci_device *dev)  { @@ -270,6 +290,76 @@
>> pci_uio_map_resource(struct rte_pci_device *dev)
>>  return (0);
>>  }
>>
>> +static int
>> +pci_uio_unmap(struct uio_resource *uio_res) {
>> +int ret;
>> +unsigned i;
>> +
>> +if (uio_res == NULL)
>> +return -EINVAL;
>> +
>> +for (i = 0; i != uio_res->nb_maps; i++) {
>> +ret = pci_unmap_resource(uio_res->maps[i].addr,
>> +(size_t)uio_res->maps[i].size);
>> +if (ret < 0)
>> +return ret;
>> +}
>> +return 0;
>> +}
>> +
>> +static struct uio_resource *
>> +pci_uio_find_resource(struct rte_pci_device *dev) {
>> +struct uio_resource *uio_res;
>> +
>> +if (dev == NULL)
>> +return NULL;
>> +
>> +TAILQ_FOREACH(uio_res, uio_res_list, next) {
>> +
>> +/* skip this element if it doesn't match our PCI address */
>> +if (!rte_eal_compare_pci_addr(&uio_res->pci_addr, &dev->addr))
>> +return uio_res;
>> +}
>> +return NULL;
>> +}
>> +
>> +/* map the PCI resource of a PCI device in virtual memory */ static int
>> +pci_uio_unmap_resource(struct rte_pci_device *dev) {
>> +struct uio_resource *uio_res;
>> +
>> +if (dev == NULL)
>> +return -EINVAL;
>> +
>> +/* find an entry for the device */
>> +uio_res = pci_uio_find_resource(dev);
>> +if (uio_res == NULL)
>> +return -ENODEV;
>> +
>> +/* secondary processes - just free maps */
>> +if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>> +return pci_uio_unmap(uio_res);
>> +
>> +TAILQ_REMOVE(uio_res_list, uio_res, next);
>> +
>> +/* unmap all resources */
>> +pci_uio_unmap(uio_res);
>> +
>> +/* free_uio resource */
>> +rte_free(uio_res);
>> +
>> +/* close fd if in primary process */
>> +close(dev->intr_handle.fd);
>> +
>> +dev->intr_handle.fd = -1;
>> +dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
>> +
>> +return 0;
>> +}
>> +
>>  /* Scan one pci sysfs entry, and fill the devices list from it. */  static 
>> int  pci_scan_one(int dev_pci_fd,
>> struct pci_conf *conf) @@ -307,6 +397,9 @@ pci_scan_one(int dev_pci_fd, 
>> struct pci_conf *conf)
>>  /* FreeBSD has no NUMA support (yet) */
>>  dev->numa_node = 0;
>>
>> +/* FreeBSD has only one pass through driver */
>> +dev->pt_driver = RTE_PT_NIC_UIO;
>> +
>>  /* parse resources */
>>  switch (conf->pc_hdr & PCIM_HDRTYPE) {
>>  case PCIM_HDRTYPE_NORMAL:
>> @@ -376,8 +469,8 @@ skipdev:
>>   * Scan the content of the PCI bus, and add the devices in the devices
>>   * list. Call pci_scan_one() for each pci entry found.
>>   */
>> -static int
>> -pci_scan(void)
>> +int
>> +rte_eal_pci_scan(void)
>>  {
>>  int fd = -1;
>>  unsigned dev_count = 0;
>> @@ -487,6 +580,76 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, 
>> struct
>> rte_pci_device *d
>>  return 1;
>>  }
>>
>> +/*
>> + * If vendor/device ID match, call the devuninit() function of the
>> + * driver.
>> + */
>> +int
>> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,
>> +struct rte_pci_device *dev)
>> +{
>> +struct rte_pci_id *id_table;
>> +int ret;
>> +
>> +if ((dr == NULL) || (dev == NULL

[dpdk-dev] [PATCH v4] af_packet: Fix some klocwork errors

[Ouyang Changchun]

Fix possible memory leak issue: free kvlist before return;
Fix possible resource lost issue: close qssockfd before return;

Signed-off-by: Changchun Ouyang 
---
Change in v4:
  - Check sockfd in internals->rx_queue against 0.

Change in v3:
  - Also close sockets for all queues.

Change in v2:
  - Make the error exit point a common path.

 lib/librte_pmd_af_packet/rte_eth_af_packet.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c 
b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
index 80e9bdf..acdf408 100644
--- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
+++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
@@ -691,9 +691,14 @@ error:
rte_free((*internals)->rx_queue[q].rd);
if ((*internals)->tx_queue[q].rd)
rte_free((*internals)->tx_queue[q].rd);
+   if (((*internals)->rx_queue[q].sockfd != 0) &&
+   ((*internals)->rx_queue[q].sockfd != qsockfd))
+   close((*internals)->rx_queue[q].sockfd);
}
rte_free(*internals);
}
+   if (qsockfd != -1)
+   close(qsockfd);
return -1;
 }

@@ -823,16 +828,21 @@ rte_pmd_af_packet_devinit(const char *name, const char 
*params)
ret = rte_kvargs_process(kvlist, ETH_AF_PACKET_IFACE_ARG,
 &open_packet_iface, &sockfd);
if (ret < 0)
-   return -1;
+   goto error;
}

ret = rte_eth_from_packet(name, &sockfd, numa_node, kvlist);
close(sockfd); /* no longer needed */

if (ret < 0)
-   return -1;
+   goto error;

+   rte_kvargs_free(kvlist);
return 0;
+
+error:
+   rte_kvargs_free(kvlist);
+   return -1;
 }

 static struct rte_driver pmd_af_packet_drv = {
-- 
1.8.4.2

[dpdk-dev] [PATCH] testpmd: Fix port validation code of "port stop all" command

[Qiu, Michael]

On 3/9/2015 10:22 AM, Tetsuya Mukawa wrote:
> On 2015/03/06 22:53, De Lara Guarch, Pablo wrote:
>> Hi Michael,
>>
>>> -Original Message-
>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Qiu, Michael
>>> Sent: Thursday, March 05, 2015 1:33 PM
>>> To: Tetsuya Mukawa; dev at dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH] testpmd: Fix port validation code of "port
>>> stop all" command
>>>
>>> Hi, Tetsuya and Pablo
>>> This is not a full fix, I have generate the full fix patch two days ago,
> Hi Michel,
>
> I am sorry for late replying, and thanks for your work.
>
>> Sorry I did not see this earlier. Did you upstream this patch already?
>> I acked Tetsuya's patch, as it was simple and works, but I cannot find
>> this one.
>>
>> Thanks,
>> Pablo
>>
>>> See below:
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index 49be819..ec53923 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -384,6 +384,9 @@ port_infos_display(portid_t port_id)
>>>  int
>>>  port_id_is_invalid(portid_t port_id, enum print_warning warning)
>>>  {
>>> +   if (port_id == (portid_t)RTE_PORT_ALL)
>>> +   return 0;
>>> +
> I am not clearly sure that we need to add above 'if statement'.

Because some times RTE_PORT_ALL will pass to port start/stop/close, but
the check will be invalid.

Actually, we should see it as valid, then all port valid check will work
for start/stop/close action

>
>>> if (ports[port_id].enabled)
>>> return 0;
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>> index e556b4c..1c4c651 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -1326,6 +1326,9 @@ start_port(portid_t pid)
>>> return -1;
>>> }
>>>
>>> +   if (port_id_is_invalid(pid, ENABLED_WARN))
>>> +   return 0;
>>> +
> Same as above.
>
>>> if (init_fwd_streams() < 0) {
>>> printf("Fail from init_fwd_streams()\n");
>>> return -1;
>>> @@ -1482,10 +1485,14 @@ stop_port(portid_t pid)
>>> dcb_test = 0;
>>> dcb_config = 0;
>>> }
>>> +
>>> +   if (port_id_is_invalid(pid, ENABLED_WARN))
>>> +   return;
>>> +
> Same as above.
>
>>> printf("Stopping ports...\n");
>>>
>>> FOREACH_PORT(pi, ports) {
>>> -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
>>> +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>>> continue;
>>>
>>> port = &ports[pi];
>>> @@ -1517,10 +1524,13 @@ close_port(portid_t pid)
>>> return;
>>> }
>>>
>>> +   if (port_id_is_invalid(pid, ENABLED_WARN))
>>> +return;
>>> +
> Same as above.
>
>>> printf("Closing ports...\n");
>>>
>>> FOREACH_PORT(pi, ports) {
>>> -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
>>> +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>>> continue;
>>>
>>> port = &ports[pi];
>>> --
>>> 1.9.3
> FOREACH_PORT() returns valid ports, so is it not enough to check like above?
> I am not clearly understand which case we need to add above extra if
> statements.
> Could you please describe?

Yes,  just consider this situation, the valid port number are [0, 1],
but you try to to stop prot number 2, what will happen?

Noting will be show, at least we need an error log.

So it must be check.

Thanks,
Michael
> But I agree we cannot use my previous patch.
> We need to fix not only stop_port() but also close_port() like start_port().
>
> Thanks,
> Tetsuya
>
>>> Thanks,
>>> Michael
>>>
>>> On 3/5/2015 3:31 PM, Tetsuya Mukawa wrote:
 When "port stop all" is executed, the command doesn't work as it should
 because of wrong port validation. The patch fixes this issue.

 Reported-by: Pablo de Lara 
 Signed-off-by: Tetsuya Mukawa 
 ---
  app/test-pmd/testpmd.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
 index 61291be..bb65342 100644
 --- a/app/test-pmd/testpmd.c
 +++ b/app/test-pmd/testpmd.c
 @@ -1484,7 +1484,7 @@ stop_port(portid_t pid)
printf("Stopping ports...\n");

FOREACH_PORT(pi, ports) {
 -  if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
 +  if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
continue;

port = &ports[pi];
>
>

[dpdk-dev] Guest Machine is not Pingable from Host Machine

[Arkajit Ghosh]

Hi, 

I have executed below commands:

./utilities/ovs-vsctl add-br ovsbr0
./utilities/ovs-vsctl set bridge datapath_type=netdev
./utilities/ovs-vsctl add-port ovsbr0 dpdk0 -- set Interface dpdk0 type=dpdk
./utilities/ovs-vsctl add-port ovsbr0 dpdk1 -- set Interface dpdk1 type=dpdk
./utilities/ovs-vsctl show

Please find the attachement for bridge and dpdk-port creation..

Now, If i do "ifconfig -a" the created bridge ovsbr0 is showing Broadcast 
Running and Multicast but ip address was not given. So i had assign an 
IP-address to ovsbr0 which previously Eth1 had. 

So, now I try to ping this guest machine ip address from its host.? Here is the 
output. 

ping 192.168.1.7
PING 192.168.1.7 (192.168.1.7) 56(84) bytes of data.
>From 192.168.1.40 icmp_seq=1 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=5 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=6 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=7 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=8 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=9 Destination Host Unreachable

I believe it should be pinged from its host machine. Can please let? me know 
what is the issue and how to proceed.

Thanks & Regards
Arkajit Ghosh


-Ngo Doan Lap  wrote: -
To: Arkajit Ghosh 
From: Ngo Doan Lap 
Date: 03/06/2015 08:52PM
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] Guest Machine is not Pingable from Host Machine

Hi,
Can you show the log output of OVS? and the ouput of following command
cd path/to/ovs/utilities/
?./ovs-ofctl show br0
?./ovs-ofctl dump-flows br0
(Assum that br0 is a bridge name that you had created)

On Fri, Mar 6, 2015 at 10:14 PM, Arkajit Ghosh  wrote:
Hi,

 Can anyone please suggest how to proceed.


 Thanks & Regards
 Arkajit Ghosh

 -Arkajit Ghosh/DEL/TCS wrote: -
 To: dev at dpdk.org
 From: Arkajit Ghosh/DEL/TCS
 Date: 03/04/2015 12:39PM
 Subject: Guest Machine is not Pingable from Host Machine

 Hi Team,

 Guest machine is not pingable from Host machine after creating a bridge with 
datapath_type "netdev" in the configuration database and adding? dpdk ports. 
Can anyone please let me know what is the issue.

 Thanks in advance.

 Thanks & Regards
 Arkajit Ghosh


 =-=-=
 Notice: The information contained in this e-mail
 message and/or attachments to it may contain
 confidential or privileged information. If you are
 not the intended recipient, any dissemination, use,
 review, distribution, printing or copying of the
 information contained in this e-mail message
 and/or attachments to it are strictly prohibited. If
 you have received this communication in error,
 please notify us by reply e-mail or telephone and
 immediately and permanently delete the message
 and any attachments. Thank you





-- 
Thanks and Best Regards,
Ngo Doan Lap
Mobile: 0977.833.757  

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

[Qiu, Michael]

On 3/8/2015 2:39 AM, Thomas Monjalon wrote:
> 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.

Hi, Thomas

You are right, I will send out v3 patch to fix this

Thanks,
Michael

>

[dpdk-dev] [PATCH] testpmd: Fix port validation code of "port stop all" command

[Tetsuya Mukawa]

On 2015/03/09 12:49, Qiu, Michael wrote:
> On 3/9/2015 10:22 AM, Tetsuya Mukawa wrote:
>> On 2015/03/06 22:53, De Lara Guarch, Pablo wrote:
>>> Hi Michael,
>>>
 -Original Message-
 From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Qiu, Michael
 Sent: Thursday, March 05, 2015 1:33 PM
 To: Tetsuya Mukawa; dev at dpdk.org
 Subject: Re: [dpdk-dev] [PATCH] testpmd: Fix port validation code of "port
 stop all" command

 Hi, Tetsuya and Pablo
 This is not a full fix, I have generate the full fix patch two days ago,
>> Hi Michel,
>>
>> I am sorry for late replying, and thanks for your work.
>>
>>> Sorry I did not see this earlier. Did you upstream this patch already?
>>> I acked Tetsuya's patch, as it was simple and works, but I cannot find
>>> this one.
>>>
>>> Thanks,
>>> Pablo
>>>
 See below:

 diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
 index 49be819..ec53923 100644
 --- a/app/test-pmd/config.c
 +++ b/app/test-pmd/config.c
 @@ -384,6 +384,9 @@ port_infos_display(portid_t port_id)
  int
  port_id_is_invalid(portid_t port_id, enum print_warning warning)
  {
 +   if (port_id == (portid_t)RTE_PORT_ALL)
 +   return 0;
 +
>> I am not clearly sure that we need to add above 'if statement'.
> Because some times RTE_PORT_ALL will pass to port start/stop/close, but
> the check will be invalid.
>
> Actually, we should see it as valid, then all port valid check will work
> for start/stop/close action
>
 if (ports[port_id].enabled)
 return 0;

 diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
 index e556b4c..1c4c651 100644
 --- a/app/test-pmd/testpmd.c
 +++ b/app/test-pmd/testpmd.c
 @@ -1326,6 +1326,9 @@ start_port(portid_t pid)
 return -1;
 }

 +   if (port_id_is_invalid(pid, ENABLED_WARN))
 +   return 0;
 +
>> Same as above.
>>
 if (init_fwd_streams() < 0) {
 printf("Fail from init_fwd_streams()\n");
 return -1;
 @@ -1482,10 +1485,14 @@ stop_port(portid_t pid)
 dcb_test = 0;
 dcb_config = 0;
 }
 +
 +   if (port_id_is_invalid(pid, ENABLED_WARN))
 +   return;
 +
>> Same as above.
>>
 printf("Stopping ports...\n");

 FOREACH_PORT(pi, ports) {
 -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
 +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 continue;

 port = &ports[pi];
 @@ -1517,10 +1524,13 @@ close_port(portid_t pid)
 return;
 }

 +   if (port_id_is_invalid(pid, ENABLED_WARN))
 +return;
 +
>> Same as above.
>>
 printf("Closing ports...\n");

 FOREACH_PORT(pi, ports) {
 -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
 +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
 continue;

 port = &ports[pi];
 --
 1.9.3
>> FOREACH_PORT() returns valid ports, so is it not enough to check like above?
>> I am not clearly understand which case we need to add above extra if
>> statements.
>> Could you please describe?
> Yes,  just consider this situation, the valid port number are [0, 1],
> but you try to to stop prot number 2, what will happen?
>
> Noting will be show, at least we need an error log.
>
> So it must be check.

Hi Michael,

Thanks, I've understood it.
Have you already submitted it as patch?
I could not find it in patchwork.
I will send an ack to your patch.

Thanks,
Tetsuya

> Thanks,
> Michael
>> But I agree we cannot use my previous patch.
>> We need to fix not only stop_port() but also close_port() like start_port().
>>
>> Thanks,
>> Tetsuya
>>
 Thanks,
 Michael

 On 3/5/2015 3:31 PM, Tetsuya Mukawa wrote:
> When "port stop all" is executed, the command doesn't work as it should
> because of wrong port validation. The patch fixes this issue.
>
> Reported-by: Pablo de Lara 
> Signed-off-by: Tetsuya Mukawa 
> ---
>  app/test-pmd/testpmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 61291be..bb65342 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1484,7 +1484,7 @@ stop_port(portid_t pid)
>   printf("Stopping ports...\n");
>
>   FOREACH_PORT(pi, ports) {
> - if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
> + if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>   continue;
>
>   port = &ports[pi];
>>

[dpdk-dev] [PATCH] librte_eal/common: Fix cast from pointer to integer of different size

[Wang, Zhihong]

> -Original Message-
> From: Qiu, Michael
> Sent: Friday, March 06, 2015 11:13 AM
> To: dev at dpdk.org
> Cc: Qiu, Michael; Wang, Zhihong
> Subject: [PATCH] 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 
> Signed-off-by: Zhihong Wang 
> ---
> v4 --> v3:
>   fix dstofss/bits to size_t in rte_memcpy()
> v3 --> v2:
> make dstofss and srcofs to be type size_t
> casting type use uintptr_t
> 
> v2 --> v1:
> Remove unnecessary casting (void *)
> 
>  lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 14 +++---
>  1 file changed, 7 insertions(+), 7 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..6ec4434 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> @@ -196,8 +196,8 @@ static inline void *  rte_memcpy(void *dst, const void
> *src, size_t n)  {
>   void *ret = dst;
> - int dstofss;
> - int bits;
> + size_t dstofss;
> + size_t bits;
> 
>   /**
>* Copy less than 16 bytes
> @@ -271,7 +271,7 @@ COPY_BLOCK_64_BACK31:
>   /**
>* Make store aligned when copy size exceeds 512 bytes
>*/
> - dstofss = 32 - (int)((long long)(void *)dst & 0x1F);
> + dstofss = 32 - ((uintptr_t)dst & 0x1F);
>   n -= dstofss;
>   rte_mov32((uint8_t *)dst, (const uint8_t *)src);
>   src = (const uint8_t *)src + dstofss;
> @@ -493,8 +493,8 @@ rte_memcpy(void *dst, const void *src, size_t n)  {
>   __m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7,
> xmm8;
>   void *ret = dst;
> - int dstofss;
> - int srcofs;
> + size_t dstofss;
> + size_t srcofs;
> 
>   /**
>* Copy less than 16 bytes
> @@ -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 - ((uintptr_t)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 = ((uintptr_t)src & 0x0F);
> 
>   /**
>* For aligned copy
> --
> 1.9.3

Acked-by:  Wang, Zhihong 

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

[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 
Acked-by: Yerden Zhumabekov 
---
v3 --> v2:
Add sub function for #else which returns 0
v2 --> v1:
 Make crc32 instruction only works in X86 platform

 lib/librte_hash/rte_hash_crc.h | 46 +-
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/lib/librte_hash/rte_hash_crc.h b/lib/librte_hash/rte_hash_crc.h
index d28bb2a..f1dbded 100644
--- a/lib/librte_hash/rte_hash_crc.h
+++ b/lib/librte_hash/rte_hash_crc.h
@@ -47,6 +47,7 @@ extern "C" {
 #include 
 #include 
 #include 
+#include 

 /* Lookup tables for software implementation of CRC32C */
 static const uint32_t crc32c_tables[8][256] = {{
@@ -364,6 +365,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)
 {
@@ -375,16 +377,6 @@ crc32c_sse42_u32(uint32_t data, uint32_t init_val)
 }

 static inline uint32_t
-crc32c_sse42_u64(uint64_t data, uint64_t init_val)
-{
-   __asm__ volatile(
-   "crc32q %[data], %[init_val];"
-   : [init_val] "+r" (init_val)
-   : [data] "rm" (data));
-   return init_val;
-}
-
-static inline uint32_t
 crc32c_sse42_u64_mimic(uint64_t data, uint64_t init_val)
 {
union {
@@ -397,6 +389,40 @@ crc32c_sse42_u64_mimic(uint64_t data, uint64_t init_val)
init_val = crc32c_sse42_u32(d.u32[1], init_val);
return init_val;
 }
+#else
+static inline uint32_t
+crc32c_sse42_u32(__rte_unused uint32_t data,
+__rte_unused uint32_t init_val)
+{
+   return 0;
+}
+
+static inline uint32_t
+crc32c_sse42_u64_mimic(__rte_unused uint32_t data,
+  __rte_unused uint32_t init_val)
+{
+   return 0;
+}
+#endif
+
+#ifdef RTE_ARCH_X86_64
+static inline uint32_t
+crc32c_sse42_u64(uint64_t data, uint64_t init_val)
+{
+   __asm__ volatile(
+   "crc32q %[data], %[init_val];"
+   : [init_val] "+r" (init_val)
+   : [data] "rm" (data));
+   return init_val;
+}
+#else
+static inline uint32_t
+crc32c_sse42_u64(__rte_unused uint64_t data,
+__rte_unused uint64_t init_val)
+{
+   return 0;
+}
+#endif

 #define CRC32_SW(1U << 0)
 #define CRC32_SSE42 (1U << 1)
-- 
1.9.3

[dpdk-dev] [PATCH] testpmd: Fix port validation code of "port stop all" command

[Qiu, Michael]

On 3/9/2015 1:21 PM, Tetsuya Mukawa wrote:
> On 2015/03/09 12:49, Qiu, Michael wrote:
>> On 3/9/2015 10:22 AM, Tetsuya Mukawa wrote:
>>> On 2015/03/06 22:53, De Lara Guarch, Pablo wrote:
 Hi Michael,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Qiu, Michael
> Sent: Thursday, March 05, 2015 1:33 PM
> To: Tetsuya Mukawa; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] testpmd: Fix port validation code of "port
> stop all" command
>
> Hi, Tetsuya and Pablo
> This is not a full fix, I have generate the full fix patch two days ago,
>>> Hi Michel,
>>>
>>> I am sorry for late replying, and thanks for your work.
>>>
 Sorry I did not see this earlier. Did you upstream this patch already?
 I acked Tetsuya's patch, as it was simple and works, but I cannot find
 this one.

 Thanks,
 Pablo

> See below:
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 49be819..ec53923 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -384,6 +384,9 @@ port_infos_display(portid_t port_id)
>  int
>  port_id_is_invalid(portid_t port_id, enum print_warning warning)
>  {
> +   if (port_id == (portid_t)RTE_PORT_ALL)
> +   return 0;
> +
>>> I am not clearly sure that we need to add above 'if statement'.
>> Because some times RTE_PORT_ALL will pass to port start/stop/close, but
>> the check will be invalid.
>>
>> Actually, we should see it as valid, then all port valid check will work
>> for start/stop/close action
>>
> if (ports[port_id].enabled)
> return 0;
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index e556b4c..1c4c651 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1326,6 +1326,9 @@ start_port(portid_t pid)
> return -1;
> }
>
> +   if (port_id_is_invalid(pid, ENABLED_WARN))
> +   return 0;
> +
>>> Same as above.
>>>
> if (init_fwd_streams() < 0) {
> printf("Fail from init_fwd_streams()\n");
> return -1;
> @@ -1482,10 +1485,14 @@ stop_port(portid_t pid)
> dcb_test = 0;
> dcb_config = 0;
> }
> +
> +   if (port_id_is_invalid(pid, ENABLED_WARN))
> +   return;
> +
>>> Same as above.
>>>
> printf("Stopping ports...\n");
>
> FOREACH_PORT(pi, ports) {
> -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
> +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
> continue;
>
> port = &ports[pi];
> @@ -1517,10 +1524,13 @@ close_port(portid_t pid)
> return;
> }
>
> +   if (port_id_is_invalid(pid, ENABLED_WARN))
> +return;
> +
>>> Same as above.
>>>
> printf("Closing ports...\n");
>
> FOREACH_PORT(pi, ports) {
> -   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
> +   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
> continue;
>
> port = &ports[pi];
> --
> 1.9.3
>>> FOREACH_PORT() returns valid ports, so is it not enough to check like above?
>>> I am not clearly understand which case we need to add above extra if
>>> statements.
>>> Could you please describe?
>> Yes,  just consider this situation, the valid port number are [0, 1],
>> but you try to to stop prot number 2, what will happen?
>>
>> Noting will be show, at least we need an error log.
>>
>> So it must be check.
> Hi Michael,
>
> Thanks, I've understood it.
> Have you already submitted it as patch?
> I could not find it in patchwork.
> I will send an ack to your patch.

I have not send yet,

I will send out now and add will add you in cc list.

Thanks,
Michael
> Thanks,
> Tetsuya
>
>> Thanks,
>> Michael
>>> But I agree we cannot use my previous patch.
>>> We need to fix not only stop_port() but also close_port() like start_port().
>>>
>>> Thanks,
>>> Tetsuya
>>>
> Thanks,
> Michael
>
> On 3/5/2015 3:31 PM, Tetsuya Mukawa wrote:
>> When "port stop all" is executed, the command doesn't work as it should
>> because of wrong port validation. The patch fixes this issue.
>>
>> Reported-by: Pablo de Lara 
>> Signed-off-by: Tetsuya Mukawa 
>> ---
>>  app/test-pmd/testpmd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 61291be..bb65342 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -1484,7 +1484,7 @@ stop_port(portid_t pid)
>>  printf("Stopping ports...\n")

[dpdk-dev] [PATCH] testpmd: Fix action of operationg invalid port

[Michael Qiu]

Currently, if try to  start/stop/close one invalid prot,
no error shows in testpmd.
This is a bug, need check the port number.

Signed-off-by: Michael Qiu 
---
 app/test-pmd/config.c  |  3 +++
 app/test-pmd/testpmd.c | 14 --
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 49be819..ec53923 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -384,6 +384,9 @@ port_infos_display(portid_t port_id)
 int
 port_id_is_invalid(portid_t port_id, enum print_warning warning)
 {
+   if (port_id == (portid_t)RTE_PORT_ALL)
+   return 0;
+
if (ports[port_id].enabled)
return 0;

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e556b4c..1c4c651 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1326,6 +1326,9 @@ start_port(portid_t pid)
return -1;
}

+   if (port_id_is_invalid(pid, ENABLED_WARN))
+   return 0;
+
if (init_fwd_streams() < 0) {
printf("Fail from init_fwd_streams()\n");
return -1;
@@ -1482,10 +1485,14 @@ stop_port(portid_t pid)
dcb_test = 0;
dcb_config = 0;
}
+
+   if (port_id_is_invalid(pid, ENABLED_WARN))
+   return;
+
printf("Stopping ports...\n");

FOREACH_PORT(pi, ports) {
-   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
+   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
continue;

port = &ports[pi];
@@ -1517,10 +1524,13 @@ close_port(portid_t pid)
return;
}

+   if (port_id_is_invalid(pid, ENABLED_WARN))
+return;
+
printf("Closing ports...\n");

FOREACH_PORT(pi, ports) {
-   if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
+   if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
continue;

port = &ports[pi];
-- 
1.9.3

[dpdk-dev] [PATCH] testpmd: Fix action of operationg invalid port

[Tetsuya Mukawa]

On 2015/03/09 15:05, Michael Qiu wrote:
> Currently, if try to  start/stop/close one invalid prot,
> no error shows in testpmd.
> This is a bug, need check the port number.
>
> Signed-off-by: Michael Qiu 
> ---
>  app/test-pmd/config.c  |  3 +++
>  app/test-pmd/testpmd.c | 14 --
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 49be819..ec53923 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -384,6 +384,9 @@ port_infos_display(portid_t port_id)
>  int
>  port_id_is_invalid(portid_t port_id, enum print_warning warning)
>  {
> + if (port_id == (portid_t)RTE_PORT_ALL)
> + return 0;
> +
>   if (ports[port_id].enabled)
>   return 0;
>  
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index e556b4c..1c4c651 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1326,6 +1326,9 @@ start_port(portid_t pid)
>   return -1;
>   }
>  
> + if (port_id_is_invalid(pid, ENABLED_WARN))
> + return 0;
> +
>   if (init_fwd_streams() < 0) {
>   printf("Fail from init_fwd_streams()\n");
>   return -1;
> @@ -1482,10 +1485,14 @@ stop_port(portid_t pid)
>   dcb_test = 0;
>   dcb_config = 0;
>   }
> +
> + if (port_id_is_invalid(pid, ENABLED_WARN))
> + return;
> +
>   printf("Stopping ports...\n");
>  
>   FOREACH_PORT(pi, ports) {
> - if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
> + if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>   continue;
>  
>   port = &ports[pi];
> @@ -1517,10 +1524,13 @@ close_port(portid_t pid)
>   return;
>   }
>  
> + if (port_id_is_invalid(pid, ENABLED_WARN))
> +return;
> +
>   printf("Closing ports...\n");
>  
>   FOREACH_PORT(pi, ports) {
> - if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)
> + if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>   continue;
>  
>   port = &ports[pi];

Acked-by: Tetsuya Mukawa 

[dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking

[Qiu, Michael]

On 3/2/2015 5:04 PM, zhihong.wang at intel.com wrote:
> Fixed strict-aliasing rules breaking errors for some GCC version.
>
> Signed-off-by: Zhihong Wang 
> ---

As this should be a quick fix, this workaround is workable.
Acked-by: Michael Qiu 

>  .../common/include/arch/x86/rte_memcpy.h   | 44 
> --
>  1 file changed, 24 insertions(+), 20 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 69a5c6f..f412099 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src, size_t 
> n)
>  static inline void *
>  rte_memcpy(void *dst, const void *src, size_t n)
>  {
> + uintptr_t dstu = (uintptr_t)dst;
> + uintptr_t srcu = (uintptr_t)src;
>   void *ret = dst;
>   int dstofss;
>   int bits;
> @@ -204,22 +206,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>*/
>   if (n < 16) {
>   if (n & 0x01) {
> - *(uint8_t *)dst = *(const uint8_t *)src;
> - src = (const uint8_t *)src + 1;
> - dst = (uint8_t *)dst + 1;
> + *(uint8_t *)dstu = *(const uint8_t *)srcu;
> + srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> + dstu = (uintptr_t)((uint8_t *)dstu + 1);
>   }
>   if (n & 0x02) {
> - *(uint16_t *)dst = *(const uint16_t *)src;
> - src = (const uint16_t *)src + 1;
> - dst = (uint16_t *)dst + 1;
> + *(uint16_t *)dstu = *(const uint16_t *)srcu;
> + srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> + dstu = (uintptr_t)((uint16_t *)dstu + 1);
>   }
>   if (n & 0x04) {
> - *(uint32_t *)dst = *(const uint32_t *)src;
> - src = (const uint32_t *)src + 1;
> - dst = (uint32_t *)dst + 1;
> + *(uint32_t *)dstu = *(const uint32_t *)srcu;
> + srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> + dstu = (uintptr_t)((uint32_t *)dstu + 1);
>   }
>   if (n & 0x08) {
> - *(uint64_t *)dst = *(const uint64_t *)src;
> + *(uint64_t *)dstu = *(const uint64_t *)srcu;
>   }
>   return ret;
>   }
> @@ -458,6 +460,8 @@ static inline void *
>  rte_memcpy(void *dst, const void *src, size_t n)
>  {
>   __m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7, xmm8;
> + uintptr_t dstu = (uintptr_t)dst;
> + uintptr_t srcu = (uintptr_t)src;
>   void *ret = dst;
>   int dstofss;
>   int srcofs;
> @@ -467,22 +471,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>*/
>   if (n < 16) {
>   if (n & 0x01) {
> - *(uint8_t *)dst = *(const uint8_t *)src;
> - src = (const uint8_t *)src + 1;
> - dst = (uint8_t *)dst + 1;
> + *(uint8_t *)dstu = *(const uint8_t *)srcu;
> + srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> + dstu = (uintptr_t)((uint8_t *)dstu + 1);
>   }
>   if (n & 0x02) {
> - *(uint16_t *)dst = *(const uint16_t *)src;
> - src = (const uint16_t *)src + 1;
> - dst = (uint16_t *)dst + 1;
> + *(uint16_t *)dstu = *(const uint16_t *)srcu;
> + srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> + dstu = (uintptr_t)((uint16_t *)dstu + 1);
>   }
>   if (n & 0x04) {
> - *(uint32_t *)dst = *(const uint32_t *)src;
> - src = (const uint32_t *)src + 1;
> - dst = (uint32_t *)dst + 1;
> + *(uint32_t *)dstu = *(const uint32_t *)srcu;
> + srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> + dstu = (uintptr_t)((uint32_t *)dstu + 1);
>   }
>   if (n & 0x08) {
> - *(uint64_t *)dst = *(const uint64_t *)src;
> + *(uint64_t *)dstu = *(const uint64_t *)srcu;
>   }
>   return ret;
>   }

[dpdk-dev] [PATCH v4] af_packet: Fix some klocwork errors

[Qiu, Michael]

On 3/9/2015 11:49 AM, Ouyang Changchun wrote:
> Fix possible memory leak issue: free kvlist before return;
> Fix possible resource lost issue: close qssockfd before return;
>
> Signed-off-by: Changchun Ouyang 
> ---
> Change in v4:
>   - Check sockfd in internals->rx_queue against 0.
>
> Change in v3:
>   - Also close sockets for all queues.
>  
> Change in v2:
>   - Make the error exit point a common path.
>
>  lib/librte_pmd_af_packet/rte_eth_af_packet.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c 
> b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> index 80e9bdf..acdf408 100644
> --- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> +++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> @@ -691,9 +691,14 @@ error:
>   rte_free((*internals)->rx_queue[q].rd);
>   if ((*internals)->tx_queue[q].rd)
>   rte_free((*internals)->tx_queue[q].rd);
> + if (((*internals)->rx_queue[q].sockfd != 0) &&
> + ((*internals)->rx_queue[q].sockfd != qsockfd))
> + close((*internals)->rx_queue[q].sockfd);
>   }
>   rte_free(*internals);
>   }
> + if (qsockfd != -1)

Hi, Ouyang

qsockfd was first initialized:
for (q = 0; q < nb_queues; q++) {
/* Open an AF_PACKET socket for this queue... */
qsockfd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));

What will happen if gets an error before here, is there any issue? I
think better to initialize it when declare it.

Thanks,
Michael
> + close(qsockfd);
>   return -1;
>  }
>  
> @@ -823,16 +828,21 @@ rte_pmd_af_packet_devinit(const char *name, const char 
> *params)
>   ret = rte_kvargs_process(kvlist, ETH_AF_PACKET_IFACE_ARG,
>&open_packet_iface, &sockfd);
>   if (ret < 0)
> - return -1;
> + goto error;
>   }
>  
>   ret = rte_eth_from_packet(name, &sockfd, numa_node, kvlist);
>   close(sockfd); /* no longer needed */
>  
>   if (ret < 0)
> - return -1;
> + goto error;
>  
> + rte_kvargs_free(kvlist);
>   return 0;
> +
> +error:
> + rte_kvargs_free(kvlist);
> + return -1;
>  }
>  
>  static struct rte_driver pmd_af_packet_drv = {

[dpdk-dev] [PATCH v4 2/5] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices

[Vlad Zolotarov]

On 03/08/15 23:12, Thomas Monjalon wrote:
> Hi Vlad,
>
> 2015-03-08 16:04, Vlad Zolotarov:
>> 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 
> You are mixing a fix (this patch) and enhancements (LRO) in the same series.
> Could you separate them please, as LRO is not going into 2.0 but this fix
> is a good candidate.

Pls, note that all patches in this series except for PATCH3 and PATCH5 
are fixing real bugs. I can send them as a separate series if u'd like.
Pls., confirm.

>
> Thanks

[dpdk-dev] [PATCH v4 5/5] ixgbe: Add LRO support

[Vlad Zolotarov]

On 03/08/15 23:20, Thomas Monjalon wrote:
> 2015-03-08 16:04, Vlad Zolotarov:
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -312,6 +312,9 @@ enum rte_eth_tx_mq_mode {
>>   #define ETH_VMDQ_DCB_TX ETH_MQ_TX_VMDQ_DCB
>>   #define ETH_DCB_TX  ETH_MQ_TX_DCB
>>   
>> +/* TODO: Remove this when DPDK version bumps up to 2.0.1 */
> The last digit is reserved for maintenance.
> LRO should be merged in 2.1.0.
> By the way, why removing this macros when it's integrated? I think it would
> be a good idea to start using this kind of macro for new API.
> It's better than version checking because it supports backports.
> Opinion?

I agree. I was just under the impression that u hate macros in general 
(like I do). ;)  That's why I put this reminder... ;)
But  I agree that in situations like this having a macro defining a 
specific API presence is a good practice.

>
>> +#define RTE_ETHDEV_HAS_LRO_SUPPORT
> Is it the right location to define it?
> Is it better than defining it just above one of the new fields or at the
> beginning of the file?

I vote for a beginning of the file. If we have more like these having 
them spread across the file would make it hard to track Especially when 
one macro is responsible for more than one API line.

>
>> +
>>   /**
>>* A structure used to configure the RX features of an Ethernet port.
>>*/
>> @@ -320,14 +323,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 +1519,7 @@ 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) */
>>  all_multicast : 1, /**< RX all multicast mode ON(1) / OFF(0). */
>>  dev_started : 1;   /**< Device state: STARTED(1) / STOPPED(0). 
>> */
>>   };

[dpdk-dev] [PATCH v4 2/5] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices

[Vlad Zolotarov]

On 03/08/15 23:12, Thomas Monjalon wrote:
> Hi Vlad,
>
> 2015-03-08 16:04, Vlad Zolotarov:
>> 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 
> You are mixing a fix (this patch) and enhancements (LRO) in the same series.

I'd also like to clarify that I've "mixed" them because the bugs were 
found in the area of the enhancement therefore I united them in the same 
series.
Frankly, I wasn't thinking about the DPDK versions it's going into when 
I sent the series... ;)

> Could you separate them please, as LRO is not going into 2.0 but this fix
> is a good candidate.
>
> Thanks

[dpdk-dev] [PATCH v4 2/5] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices

[Thomas Monjalon]

2015-03-09 09:08, Vlad Zolotarov:
> 
> On 03/08/15 23:12, Thomas Monjalon wrote:
> > Hi Vlad,
> >
> > 2015-03-08 16:04, Vlad Zolotarov:
> >> 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 
> > You are mixing a fix (this patch) and enhancements (LRO) in the same series.
> > Could you separate them please, as LRO is not going into 2.0 but this fix
> > is a good candidate.
> 
> Pls, note that all patches in this series except for PATCH3 and PATCH5 
> are fixing real bugs. I can send them as a separate series if u'd like.
> Pls., confirm.

Yes you're right, patch 1 is also a fix and patch 4 seems to solve other
issues. However, patch 4 makes also some refactoring and seems a bit risky.
We need an ixgbe maintainer to decide wether we can merge it before the
release. Or is it possible to have fixes of the patch 4 without the
refactoring?

Thanks Vlad.
Sorry to request such split but this PMD is sensible and I don't want to
have a risk of making it worst in release 2.0.

[dpdk-dev] [PATCH v3] pci: save list of detached devices, and re-probe during driver unload

[Raz Amir]

Hi, gentle reminder about this patch...

On Mar 4, 2015, at 1:15 PM, 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 
---
lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 32 
1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c 
b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
index 5ae8560..78e4dea 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_DEVICES128
+static device_t detached_devices[MAX_DETACHED_DEVICES] = {};
+static int num_detached = 0;

struct nic_uio_softc {
   device_tdev_t;
@@ -289,16 +292,37 @@ nic_uio_load(void)

   dev = pci_find_bsf(bus, device, function);
   if (dev != NULL)
-for (i = 0; i < NUM_DEVICES; i++)
-if (pci_get_vendor(dev) == devices[i].vend &&
-pci_get_device(dev) == devices[i].dev)
-device_detach(dev);
+continue;
+
+for (i = 0; i < NUM_DEVICES; i++)
+if (pci_get_vendor(dev) == devices[i].vend &&
+pci_get_device(dev) == devices[i].dev) {
+if (num_detached < MAX_DETACHED_DEVICES) {
+printf("nic_uio_load: detaching and storing 
dev=%p\n", dev);
+detached_devices[num_detached++] = dev;
+}
+else
+printf("nic_uio_load: reached 
MAX_DETACHED_DEVICES=%d. dev=%p won't be reattached\n",
+MAX_DETACHED_DEVICES, dev);
+device_detach(dev);
+}
   }
}

static void
nic_uio_unload(void)
{
+int i;
+printf("nic_uio_unload: entered ... \n");
+
+for (i = 0; i < num_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] rte_mbuf: scattered pktmbufs freeing optimization

[Olivier MATZ]

Hi Vadim,

On 03/07/2015 12:24 AM, Vadim Suraev wrote:
> 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?

I think it's the expected behavior as the code of mempool_get()
tries to minimize the number of tests. In this situation, even if
len(mempool) + len(cache) is greater than the number of requested
objects, we are almost out of buffers, so returning ENOBUF is not
a problem.

If the user wants to ensure that he can allocates at least X buffers,
he can create the pool with:
  mempool_create(X + cache_size * MAX_LCORE)

> By the way, rte_mempool_count returns a ring count + sum of all local
> caches, IMHO it could mislead, even twice.

Right, today rte_mempool_count() cannot really be used for something
else than debug or stats. Adding rte_mempool_common_count() and
rte_mempool_cache_len() may be useful to give the user a better control
(and they will be faster because they won't browse the cache lengths of
all lcores).

But we have to keep in mind that for multi-consumer pools checking the
common_count before retrieving objects is useless because the other
lcores can retrieve objects at the same time.

Regards,
Olivier

[dpdk-dev] [PATCH v4] af_packet: Fix some klocwork errors

[Ouyang, Changchun]

Hi Michael,

> -Original Message-
> From: Qiu, Michael
> Sent: Monday, March 9, 2015 2:39 PM
> To: Ouyang, Changchun; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] af_packet: Fix some klocwork errors
> 
> On 3/9/2015 11:49 AM, Ouyang Changchun wrote:
> > Fix possible memory leak issue: free kvlist before return; Fix
> > possible resource lost issue: close qssockfd before return;
> >
> > Signed-off-by: Changchun Ouyang 
> > ---
> > Change in v4:
> >   - Check sockfd in internals->rx_queue against 0.
> >
> > Change in v3:
> >   - Also close sockets for all queues.
> >
> > Change in v2:
> >   - Make the error exit point a common path.
> >
> >  lib/librte_pmd_af_packet/rte_eth_af_packet.c | 14 --
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> > b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> > index 80e9bdf..acdf408 100644
> > --- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> > +++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> > @@ -691,9 +691,14 @@ error:
> > rte_free((*internals)->rx_queue[q].rd);
> > if ((*internals)->tx_queue[q].rd)
> > rte_free((*internals)->tx_queue[q].rd);
> > +   if (((*internals)->rx_queue[q].sockfd != 0) &&
> > +   ((*internals)->rx_queue[q].sockfd !=
> qsockfd))
> > +   close((*internals)->rx_queue[q].sockfd);
> > }
> > rte_free(*internals);
> > }
> > +   if (qsockfd != -1)
> 
> Hi, Ouyang
> 
> qsockfd was first initialized:
> for (q = 0; q < nb_queues; q++) {
> /* Open an AF_PACKET socket for this queue... */
> qsockfd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
> 
> What will happen if gets an error before here, is there any issue? I think
> better to initialize it when declare it.
>
Make sense,  will make next version to resolve it
Thanks
Changchun

[dpdk-dev] [PATCH v5] af_packet: Fix some klocwork errors

[Ouyang Changchun]

Fix possible memory leak issue: free kvlist before return;
Fix possible resource lost issue: close qssockfd before return;

Signed-off-by: Changchun Ouyang 
---
Change in v5:
  - Initialize qsockfd with -1;

Change in v4:
  - Check sockfd in internals->rx_queue against 0.

Change in v3:
  - Also close sockets for all queues.

Change in v2:
  - Make the error exit point a common path.

 lib/librte_pmd_af_packet/rte_eth_af_packet.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c 
b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
index 80e9bdf..2a27ebc 100644
--- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
+++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
@@ -442,7 +442,8 @@ rte_pmd_init_internals(const char *name,
struct tpacket_req *req;
struct pkt_rx_queue *rx_queue;
struct pkt_tx_queue *tx_queue;
-   int rc, qsockfd, tpver, discard;
+   int rc, tpver, discard;
+   int qsockfd = -1;
unsigned int i, q, rdsize;
int fanout_arg __rte_unused, bypass __rte_unused;

@@ -691,9 +692,14 @@ error:
rte_free((*internals)->rx_queue[q].rd);
if ((*internals)->tx_queue[q].rd)
rte_free((*internals)->tx_queue[q].rd);
+   if (((*internals)->rx_queue[q].sockfd != 0) &&
+   ((*internals)->rx_queue[q].sockfd != qsockfd))
+   close((*internals)->rx_queue[q].sockfd);
}
rte_free(*internals);
}
+   if (qsockfd != -1)
+   close(qsockfd);
return -1;
 }

@@ -823,16 +829,21 @@ rte_pmd_af_packet_devinit(const char *name, const char 
*params)
ret = rte_kvargs_process(kvlist, ETH_AF_PACKET_IFACE_ARG,
 &open_packet_iface, &sockfd);
if (ret < 0)
-   return -1;
+   goto error;
}

ret = rte_eth_from_packet(name, &sockfd, numa_node, kvlist);
close(sockfd); /* no longer needed */

if (ret < 0)
-   return -1;
+   goto error;

+   rte_kvargs_free(kvlist);
return 0;
+
+error:
+   rte_kvargs_free(kvlist);
+   return -1;
 }

 static struct rte_driver pmd_af_packet_drv = {
-- 
1.8.4.2

[dpdk-dev] [PATCH] testpmd: bond port creation did not enable bond port

[Jiajia, SunX]

Tested-by: Jiajia, SunX 
- Tested Commit: 391b5f425c4613e649e5353ed8161e75967b0f3a
- OS: Fedora20 3.11.10-301.fc20.x86_64
- GCC: gcc version 4.8.3
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection 
[8086:10fb]
- Target x86_64-native-linuxapp-gcc 
- Total 22 cases, 22 passed, 0 failed

TOPO:
* Connections ports between tester/ixia and DUT
  - TESTER(Or IXIA)---DUT
  - portA--port0
  - portB--port1
  - portC--port2
  - portD--port3


Test Setup#1 for Functional test


Tester has 4 ports(portA--portD), and DUT has 4 ports(port0--port3), then 
connect portA to port0, 
portB to port1, portC to port2, portD to port3. 



- Case: Basic bonding--Create bonded devices and slaves
  Description: 
Use Setup#1.
Create bonded device and add some ports as salve of bonded 
device,
Then removed slaves or added slaves or change the bonding primary 
slave
Or change bonding mode and so on.
  Expected test result:
Verify the basic functions are normal.

- Case: Basic bonding--MAC Address Test
  Description: 
Use Setup#1.
Create bonded device and add some ports as slaves of bonded 
device,
Check that the changes of  the bonded device and slave MAC
  Expected test result:
Verify the behavior of bonded device and slave according to the 
mode.

- Case: Basic bonding--Device Promiscuous Mode Test
  Description: 
Use Setup#1.
Create bonded device and add some ports as slaves of bonded 
device,
Set promiscuous mode on or off, then send packets to the bonded 
device
Or slaves.
  Expected test result:
Verify the RX/TX status of bonded device and slaves according to 
the mode.

- Case: Mode 0(Round Robin) TX/RX test
  Description: 
Use Setup#1.
Create bonded device with mode 0 and add 3 ports as slaves of 
bonded device,
Forward packets between bonded device and unbounded device, start 
to forward,
And send packets to unbound device or slaves.
  Expected test result:
Verify the RX/TX status of bonded device and slaves in mode 0.

- Case: Mode 0(Round Robin) Bring one slave link down
  Description: 
Use Setup#1.
Create bonded device with mode 0 and add 3 ports as slaves of 
bonded device,
Forward packets between bonded device and unbounded device, start 
to forward,
Bring the link on either port 0, 1 or 2 down. And send packets to 
unbound 
device or slaves.
  Expected test result:
Verify the RX/TX status of bonded device and slaves in mode 0.

- Case: Mode 0(Round Robin) Bring all slave links down
  Description: 
Use Setup#1.
Create bonded device with mode 0 and add 3 ports as slaves of 
bonded device,
Forward packets between bonded device and unbounded device, start 
to forward,
Bring the links down on all bonded ports. And send packets to 
unbound 
device or slaves.
  Expected test result:
Verify the RX/TX status of bonded device and slaves in mode 0.

- Case: Mode 1(Active Backup) TX/RX Test
  Description: 
Use Setup#1.
Create bonded device with mode 1 and add 3 ports as slaves of 
bonded device,
Forward packets between bonded device and unbounded device, start 
to forward,
And send packets to unbound device or slaves.
  Expected test result:
Verify the RX/TX status of bonded device and slaves in mode 1.

- Case: Mode 1(Active Backup) Change active slave, RX/TX test
  Description: 
Use Setup#1.
Continuing from previous test case.Change the active slave port 
from port0 
to port1.Verify that the bonded device's MAC has changed to 
slave1's MAC.

testpmd> set bonding primary 1 4 

   Repeat the transmission and reception(TX/RX) test verify that data 
is now 
   transmitted and received through the new active slave and no longer 
through
   port0
  Expected test result:
Verify the RX/TX status of bonded device and slaves in mode 1.

- Case: Mode 1(Active Backup) Link up/down active eth dev
  Description: 
Use Setup#1.

   Bring link between port A and port0 down. If tester is ixia, can use 
   IxExplorer to set the "Simulate Cable Disconnect" at the port 
property.  
   Verify that the active slave has been changed from port0. Repeat the 
   transmission and reception test verify that data is now transmitted 
and
   received through the new active slave and no longer through port0

   Bring port0 to link down at the remote end.Verify the

[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Xuelin Shi
> Sent: Monday, March 9, 2015 1:54 AM
> To: Richardson, Bruce
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for
> big endian
>
> Yes, the patch http://dpdk.org/dev/patchwork/patch/3132/ should be
> abandoned.
> 

Hi,

If you register and login to the DPDK patchwork site you can mark the patch as 
"Not Applicable" or whatever category now applies.

http://dpdk.org/dev/patchwork/user/login/

John

[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian

[Xuelin Shi]

Hi,

OK, done. Marked as "not applicable".

Thanks,
Shi 

> -Original Message-
> From: Mcnamara, John [mailto:john.mcnamara at intel.com]
> Sent: Monday, March 09, 2015 17:13
> To: Shi Xuelin-B29237; Richardson, Bruce
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely
> for big endian
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Xuelin Shi
> > Sent: Monday, March 9, 2015 1:54 AM
> > To: Richardson, Bruce
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely
> > for big endian
> >
> > Yes, the patch http://dpdk.org/dev/patchwork/patch/3132/ should be
> > abandoned.
> >
> 
> Hi,
> 
> If you register and login to the DPDK patchwork site you can mark the
> patch as "Not Applicable" or whatever category now applies.
> 
> http://dpdk.org/dev/patchwork/user/login/
> 
> John

[dpdk-dev] [PATCH v1 0/3]: bug fixes in the ixgbe PF PMD Rx flow

[Vlad Zolotarov]

This series contains some bug fixes that were found during my work on the ixgbe 
LRO
patches. Sending this series separately on Thomas request so that it may be 
integrated
into the 2.0 release.

Vlad Zolotarov (3):
  ixgbe: Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when
reading/setting HW ring descriptor fields
  ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices
  ixgbe: Unify the rx_pkt_bulk callback initialization

 lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h |   2 +
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |  13 ++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 199 +---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |  22 +++-
 4 files changed, 161 insertions(+), 75 deletions(-)

-- 
2.1.0

[dpdk-dev] [PATCH v1 1/3] ixgbe: Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring descriptor fields

[Vlad Zolotarov]

Fixed the above in ixgbe_rx_alloc_bufs() and in ixgbe_recv_scattered_pkts().

Signed-off-by: Vlad Zolotarov 
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 9ecf3e5..b033e04 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1028,7 +1028,7 @@ 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 */
@@ -1051,7 +1051,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;
}
@@ -1559,13 +1559,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. */
-- 
2.1.0

[dpdk-dev] [PATCH v1 2/3] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices

[Vlad Zolotarov]

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 b033e04..ce9658e 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -3677,7 +3677,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 (dev->data->dev_conf.rxmode.hw_strip_crc)
rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
-- 
2.1.0

[dpdk-dev] [PATCH v1 3/3] ixgbe: Unify the rx_pkt_bulk callback initialization

[Vlad Zolotarov]

   - Set the callback in a single function that is called from
 ixgbe_dev_rx_init() for a primary process and from eth_ixgbe_dev_init()
 for a secondary processes. This is instead of multiple, hard to track 
places.
   - Added ixgbe_hw.rx_bulk_alloc_allowed - see ixgbe_hw.rx_vec_allowed 
description below.
   - Added ixgbe_hw.rx_vec_allowed: like with Bulk Allocation, Vector Rx is
 enabled or disabled on a per-port level. All queues have to meet the 
appropriate
 preconditions and if any of them doesn't - the feature has to be disabled.
 Therefore ixgbe_hw.rx_vec_allowed will be updated during each queues 
configuration
 (rte_eth_rx_queue_setup()) and then used in rte_eth_dev_start() to 
configure the
 appropriate callbacks. The same happens with ixgbe_hw.rx_vec_allowed in a 
Bulk Allocation
 context.
   - Bugs fixed:
  - Vector scattered packets callback was called regardless the appropriate
preconditions:
 - Vector Rx specific preconditions.
 - Bulk Allocation preconditions.
  - Vector Rx was enabled/disabled according to the last queue setting and 
not
based on all queues setting (which may be different for each queue).

Signed-off-by: Vlad Zolotarov 
---
 lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h |   2 +
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |  13 ++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 183 +---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |  22 +++-
 4 files changed, 152 insertions(+), 68 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h 
b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
index c67d462..9a66370 100644
--- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
+++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
@@ -3657,6 +3657,8 @@ struct ixgbe_hw {
bool force_full_reset;
bool allow_unsupported_sfp;
bool wol_enabled;
+   bool rx_bulk_alloc_allowed;
+   bool rx_vec_allowed;
 };

 #define ixgbe_call_func(hw, func, params, error) \
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c 
b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 9bdc046..9d3de1a 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -760,8 +760,8 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct 
eth_driver *eth_drv,
   "Using default TX function.");
}

-   if (eth_dev->data->scattered_rx)
-   eth_dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
+   set_rx_function(eth_dev);
+
return 0;
}
pci_dev = eth_dev->pci_dev;
@@ -772,6 +772,13 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct 
eth_driver *eth_drv,
hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
hw->allow_unsupported_sfp = 1;

+   /*
+* Initialize to TRUE. If any of Rx queues doesn't meet the bulk
+* allocation or vector Rx preconditions we will reset it.
+*/
+   hw->rx_bulk_alloc_allowed = true;
+   hw->rx_vec_allowed = true;
+
/* Initialize the shared code (base driver) */
 #ifdef RTE_NIC_BYPASS
diag = ixgbe_bypass_init_shared_code(hw);
@@ -1641,6 +1648,8 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)

/* Clear stored conf */
dev->data->scattered_rx = 0;
+   hw->rx_bulk_alloc_allowed = false;
+   hw->rx_vec_allowed = false;

/* Clear recorded link status */
memset(&link, 0, sizeof(link));
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index ce9658e..72c65df 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -2074,12 +2074,12 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused 
struct igb_rx_queue *rxq)

 /* Reset dynamic igb_rx_queue fields back to defaults */
 static void
-ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
+ixgbe_reset_rx_queue(struct ixgbe_hw *hw, struct igb_rx_queue *rxq)
 {
static const union ixgbe_adv_rx_desc zeroed_desc = { .read = {
.pkt_addr = 0}};
unsigned i;
-   uint16_t len;
+   uint16_t len = rxq->nb_rx_desc;

/*
 * By default, the Rx queue setup function allocates enough memory for
@@ -2091,14 +2091,9 @@ ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
 * constraints here to see if we need to zero out memory after the end
 * of the H/W descriptor ring.
 */
-#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
-   if (check_rx_burst_bulk_alloc_preconditions(rxq) == 0)
+   if (hw->rx_bulk_alloc_allowed)
/* zero out extra memory */
-   len = (uint16_t)(rxq->nb_rx_desc + RTE_PMD_IXGBE_RX_MAX_BURST);
-   else
-#endif
-   /* do not zero out extra memory */
-   len = rxq->nb_rx_desc;
+   len += RTE_PMD_IXGBE_RX_MAX_BURST;

/*
 * Zero out HW ring memory. Zero out extra memory at the end of
@@ -2140,7 +213

[dpdk-dev] [PATCH v5 0/3]This series adds the missing flow for enabling the LRO in the ethdev and

[Vlad Zolotarov]

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%

This is an almost factor 80 improvement.

New in v5:
   - Split the series into "bug fixes" and "all the rest" so that the former 
could be
 integrated into a 2.0 release.
   - Put the RTE_ETHDEV_HAS_LRO_SUPPORT definition at the beginning of 
rte_ethdev.h.
   - Removed the "TODO: Remove me" comment near RTE_ETHDEV_HAS_LRO_SUPPORT.

New in v4:
   - Remove CONFIG_RTE_ETHDEV_LRO_SUPPORT from config/common_linuxapp.
   - Define RTE_ETHDEV_HAS_LRO_SUPPORT in rte_ethdev.h.
   - As a result of "ixgbe: check rxd number to avoid mbuf leak" (352078e8e) 
Vector Rx
 had to get the same treatment as Rx Bulk Alloc (see PATCH4 for more 
details).

New in v3:
   - ixgbe_rx_alloc_bufs(): Always reset refcnt of the buffers to 1. Otherwise 
rte_pktmbuf_free()
 won't free them.

New in v2:
   - Removed rte_eth_dev_data.lro_bulk_alloc and added 
ixgbe_hw.rx_bulk_alloc_allowed
 instead.
   - Unified the rx_pkt_bulk callback setting (a separate new patch).
   - Fixed a few styling and spelling issues.


Vlad Zolotarov (3):
  ixgbe: Cleanups
  ixgbe: Code refactoring
  ixgbe: Add LRO support

 lib/librte_ether/rte_ethdev.h   |   9 +-
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   6 +
 lib/librte_pmd_ixgbe/ixgbe_ethdev.h |   5 +
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 703 
 lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |   6 +
 5 files changed, 662 insertions(+), 67 deletions(-)

-- 
2.1.0

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Vlad Zolotarov]

   - Removed the not needed casting.
   - 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 | 27 ---
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 72c65df..609b5fd 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
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);
@@ -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;
@@ -3560,6 +3558,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);
@@ -3582,7 +3581,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;
@@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
/*
 * Configure jumbo frame support, if any.
 */
-   if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
+   if (rx_conf->jumbo_frame == 1) {
hlreg0 |= IXGBE_HLREG0_JUMBOEN;
maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS);
maxfrs &= 0x;
-   maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16);
+   maxfrs |= (rx_conf->max_rx_pkt_len << 16);
IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs);
} else
hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
@@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 * Reset crc_len in case it was changed after queue setup by a
 * call to configure.
 */
-   rxq->crc_len = (uint8_t)
-   ((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 :
-   ETHER_CRC_LEN);
+   rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN;

/* Setup the Base and Length of the Rx Descriptor Rings */
bus_addr = rxq->rx_ring_phys_addr;
@@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
/*
 * Configure Header Split
 */
-   if (dev->data->dev_conf.rxmode.header_split) {
+   if (rx_conf->header_split) {
if (hw->mac.type == ixgbe_mac_82599EB) {
/* Must setup the PSRTYPE register */
uint32_t psrtype;
@@ -3648,7 +3645,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
IXGBE_PSRTYPE_IPV6HDR;
IXGBE_WRITE_REG(hw, 
IXGBE_PSRTYPE(rxq->reg_idx), psrtype);
}
-   srrctl = ((dev->data->dev_conf.rxmode.split_hdr_size <<
+   srrctl = ((rx_conf->split_hdr_size <<
IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) &
IXGBE_SRRCTL_BSIZEHDR_MASK);
srrctl |= IXGBE_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS;
@@ -3699,7 +3696,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 */
rxcsum = IXGBE_READ_REG(hw, IXGBE_RXCSUM);
rxcsum |= IXGBE_RXCSUM_PCSD;
-   if (dev->data->dev_conf.rxmode.hw_ip_checksum)
+   if (rx_conf->hw_ip_checksum)
rxcsum |= IXGBE_RXCSUM_IPPCSE;
else
rxcsum &= ~IXGBE_RXCSUM_IPPCSE;
@@ -3709,7 +3706,7 @@ ixgbe_dev_rx_in

[dpdk-dev] [PATCH v5 2/3] ixgbe: Code refactoring

[Vlad Zolotarov]

- 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 

New in v3:
   - ixgbe_rx_alloc_bufs(): Always reset refcnt of the buffers to 1.
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 114 +++---
 1 file changed, 69 insertions(+), 45 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 609b5fd..37aca5d 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;
+   if (reset_mbuf) {
+   mb->next = NULL;
+   mb->nb_segs = 1;
+   mb->port = rxq->port_id;
+   }
+
rte_mbuf_refcnt_set(mb, 1);
-   mb->next = NULL;
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(des

[dpdk-dev] [PATCH v5 3/3] ixgbe: Add LRO support

[Vlad Zolotarov]

- 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 
---
New in v5:
   - Put the RTE_ETHDEV_HAS_LRO_SUPPORT definition at the beginning of 
rte_ethdev.h.
   - Removed the "TODO: Remove me" comment near RTE_ETHDEV_HAS_LRO_SUPPORT.

New in v4:
   - Define RTE_ETHDEV_HAS_LRO_SUPPORT in rte_ethdev.h instead of
 RTE_ETHDEV_LRO_SUPPORT defined in config/common_linuxapp.

New in v2:
   - Removed rte_eth_dev_data.lro_bulk_alloc.
   - Fixed a few styling and spelling issues.
---
 lib/librte_ether/rte_ethdev.h   |   9 +-
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   6 +
 lib/librte_pmd_ixgbe/ixgbe_ethdev.h |   5 +
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 562 +++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |   6 +
 5 files changed, 581 insertions(+), 7 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 8db3127..44f081f 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -172,6 +172,9 @@ extern "C" {

 #include 

+/* Use this macro to check if LRO API is supported */
+#define RTE_ETHDEV_HAS_LRO_SUPPORT
+
 #include 
 #include 
 #include 
@@ -320,14 +323,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 +1519,7 @@ 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) */
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 9d3de1a..765174d 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -1648,6 +1648,7 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)

/* Clear stored conf */
dev->data->scattered_rx = 0;
+   dev->data->lro = 0;
hw->rx_bulk_alloc_allowed = false;
hw->rx_vec_allowed = false;

@@ -2018,6 +2019,11 @@ 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;
+
+   if (hw->mac.type == ixgbe_mac_82599EB ||
+   hw->mac.type == ixgbe_mac_X540)
+   dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_TCP_LRO;
+
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/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 37aca5d..593adb9 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1366,6 +1366,15 @@ ixgbe_recv_pkts

[dpdk-dev] [PATCH v3] pci: save list of detached devices, and re-probe during driver unload

[Bruce Richardson]

On Mon, Mar 09, 2015 at 10:07:29AM +0200, Raz Amir wrote:
> Hi, gentle reminder about this patch...
> 
> On Mar 4, 2015, at 1:15 PM, 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 
> ---
> lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 32 
> 1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c 
> b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
> index 5ae8560..78e4dea 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_DEVICES128
> +static device_t detached_devices[MAX_DETACHED_DEVICES] = {};
> +static int num_detached = 0;
> 
> struct nic_uio_softc {
>device_tdev_t;
> @@ -289,16 +292,37 @@ nic_uio_load(void)
> 
>dev = pci_find_bsf(bus, device, function);
>if (dev != NULL)
> -for (i = 0; i < NUM_DEVICES; i++)
> -if (pci_get_vendor(dev) == devices[i].vend &&
> -pci_get_device(dev) == devices[i].dev)
> -device_detach(dev);
> +continue;
Since we have changed the body to be a continue statement, I think the condition
above needs to be flipped.

/Bruce

[dpdk-dev] [PATCH v1 1/3] ixgbe: Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring descriptor fields

[Ananyev, Konstantin]

Hi Vlad,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> Sent: Monday, March 09, 2015 10:13 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v1 1/3] ixgbe: Use the 
> rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring descriptor
> fields
> 
> Fixed the above in ixgbe_rx_alloc_bufs() and in ixgbe_recv_scattered_pkts().
> 
> Signed-off-by: Vlad Zolotarov 
> ---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 9ecf3e5..b033e04 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1028,7 +1028,7 @@ 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;

Wonder Why you changed from uint64_t to __le64 here?
Effectively __le64 is the same a uint64_t, and I think it is better to use 
always the same type across all PMD code for consistency.
Konstantin 


>   int diag, i;
> 
>   /* allocate buffers in bulk directly into the S/W ring */
> @@ -1051,7 +1051,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;
>   }
> @@ -1559,13 +1559,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. */
> --
> 2.1.0

[dpdk-dev] [PATCH v5 0/3]This series adds the missing flow for enabling the LRO in the ethdev and

[Vlad Zolotarov]

On 03/09/15 12:20, Vlad Zolotarov wrote:
> 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%
>   
> This is an almost factor 80 improvement.

Pls., note that this series is a followup series to the "bug fixes in 
the ixgbe PF PMD Rx flow" series I've sent before.

thanks,
vlad

>
> New in v5:
> - Split the series into "bug fixes" and "all the rest" so that the former 
> could be
>   integrated into a 2.0 release.
> - Put the RTE_ETHDEV_HAS_LRO_SUPPORT definition at the beginning of 
> rte_ethdev.h.
> - Removed the "TODO: Remove me" comment near RTE_ETHDEV_HAS_LRO_SUPPORT.
>
> New in v4:
> - Remove CONFIG_RTE_ETHDEV_LRO_SUPPORT from config/common_linuxapp.
> - Define RTE_ETHDEV_HAS_LRO_SUPPORT in rte_ethdev.h.
> - As a result of "ixgbe: check rxd number to avoid mbuf leak" (352078e8e) 
> Vector Rx
>   had to get the same treatment as Rx Bulk Alloc (see PATCH4 for more 
> details).
>   
> New in v3:
> - ixgbe_rx_alloc_bufs(): Always reset refcnt of the buffers to 1. 
> Otherwise rte_pktmbuf_free()
>   won't free them.
>
> New in v2:
> - Removed rte_eth_dev_data.lro_bulk_alloc and added 
> ixgbe_hw.rx_bulk_alloc_allowed
>   instead.
> - Unified the rx_pkt_bulk callback setting (a separate new patch).
> - Fixed a few styling and spelling issues.
>
>
> Vlad Zolotarov (3):
>ixgbe: Cleanups
>ixgbe: Code refactoring
>ixgbe: Add LRO support
>
>   lib/librte_ether/rte_ethdev.h   |   9 +-
>   lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   6 +
>   lib/librte_pmd_ixgbe/ixgbe_ethdev.h |   5 +
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 703 
> 
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |   6 +
>   5 files changed, 662 insertions(+), 67 deletions(-)
>

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> Sent: Monday, March 09, 2015 10:21 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
>- Removed the not needed casting.
>- 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 | 27 ---
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 72c65df..609b5fd 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>   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);

I think all these extra casts came in to keep icc 12.* compiling without 
warnings.
I am agree that they are unnecessary.
Though if we still have to support icc 12.* we either need to keep them, or find
some other way to keep it happy.
Konstantin  

>   rxep = &rxq->sw_ring[alloc_idx];
>   diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep,
>   rxq->rx_free_thresh);
> @@ -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;
> @@ -3560,6 +3558,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);
> @@ -3582,7 +3581,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;
> @@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>   /*
>* Configure jumbo frame support, if any.
>*/
> - if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
> + if (rx_conf->jumbo_frame == 1) {
>   hlreg0 |= IXGBE_HLREG0_JUMBOEN;
>   maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS);
>   maxfrs &= 0x;
> - maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16);
> + maxfrs |= (rx_conf->max_rx_pkt_len << 16);
>   IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs);
>   } else
>   hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
> @@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>* Reset crc_len in case it was changed after queue setup by a
>* call to configure.
>*/
> - rxq->crc_len = (uint8_t)
> - ((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 :
> - ETHER_CRC_LEN);
> + rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN;
> 
>   /* Setup the Base and Length of the Rx Descriptor Rings */
>   bus_addr = rxq->rx_ring_phys_addr;
> @@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>   /*
>* Configure Header Split
>*/
> - if (dev->data->dev_conf.rxmode.header_split) {
> + if (rx_conf->header_split) {
>   if (hw->mac.type == ixgbe_mac_82599EB) {
>   /* Must setup the PSRTYPE register */
>   uint32_t psrtype;
> @@ -3648,7 +3645,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>   IXGBE_PSRTYPE_IPV6HDR;
>   IXGBE_WRITE_REG(hw, 
> IXGBE_PSRTYPE(rxq->reg_idx), psrtype);
>   }
> - srrctl = ((dev->data->dev_conf.rxmode.split_hdr_size <<
> + srrctl = ((rx_conf->split_hdr_size <<
>   IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) &
> 

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

[David Marchand]

So, a little summary.

On Fri, Mar 6, 2015 at 5:20 PM, Stephen Hemminger <
stephen at networkplumber.org> wrote:

> > > The virtio driver needs to use in/out instructions therefore it must
> initialize
> > > using iopl(2) system call. The problem is that virtio initialization
> happens very
> > > early, and any application that uses daemon() or calls eal_init later
> in another
> > > context will fail.
>

Part of this is wrong.
The manual tells that this should be inherited.
I added some fork and daemon to test programs of mine which confirmed the
manual seems to be right.

The real problem is that iopl is not called at "constructor" time for it to
be inherited by irq thread.


> The issue is that virtio has no place it can do iopl() and have the IRQ
> thread
> work. It only shows up on real code where application is daemon, not in a
> toy
> demo or test application.
>

Contrary to what you asserted, the problem does happen when using testpmd
(if this is the toy you are talking about).
So, I would say your real world application is no better than testpmd.


I will post the right fixes.

-- 
David Marchand

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Pawel Wodkowski]

On 2015-03-09 11:49, Ananyev, Konstantin wrote:
>
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>> Sent: Monday, March 09, 2015 10:21 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>
>> - Removed the not needed casting.
>> - 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 | 27 ---
>>   1 file changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> index 72c65df..609b5fd 100644
>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>>  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);
>
> I think all these extra casts came in to keep icc 12.* compiling without 
> warnings.
> I am agree that they are unnecessary.
> Though if we still have to support icc 12.* we either need to keep them, or 
> find
> some other way to keep it happy.
> Konstantin
>

What warnings icc 12.* is throwing? Only warning I can think of here is 
signed -> unsigned implicit cast. Changing '1' to '1U' helps?


-- 
Pawel

[dpdk-dev] Memory issues seen while running pktgen with DPDK sample application

[Bruce Richardson]

On Sun, Mar 08, 2015 at 09:40:06PM +0530, Shankari Vaidyalingam wrote:
> Hi,
> 
> I'm trying to send packets from pktgen to exception path sample application.
> My configuration is like this:
> 
> |--Terminal 1---  ---Terminal 2
> |   |  |
>   |
> |  Pktgen   |-->  NIC port 0 --|--->tap0 --> Appln --->tap1|
> |   |
> |-|
> |---
> Tried the following but getting an error with memory fault:
> 
> 
> Pktgen: Started this first
> Exception path sample application: When I started this I got the error:
> Please let me know how to resolve this error:
> 

As well as limiting memory, you also need to provide a different hugepage
filename prefix for each independent DPDK process you are running on the system.
See: 
http://dpdk.org/doc/guides/prog_guide/multi_proc_support.html#running-multiple-independent-dpdk-applications

Regards,
/Bruce

> 
> 
> sudo ./build/exceptiopath -c 0x03 -n 2 -- -p 0x01 -i 2 -o 1
> [sudo] password for controller:
> EAL: Detected lcore 0 as core 0 on socket 0
> EAL: Detected lcore 1 as core 1 on socket 0
> EAL: Support maximum 64 logical core(s) by configuration.
> EAL: Detected 2 lcore(s)
> EAL: No free hugepages reported in hugepages-2048kB
> PANIC in rte_eal_init():
> Cannot get hugepage information
> 6: [./build/exception_path() [0x418cc5]]
> 5: [/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)
> [0x7fda2c77976d]]
> 4: [./build/exception_path(main+0x39) [0x4183e9]]
> 3: [./build/exception_path(rte_eal_init+0x10ac) [0x45f8ac]]
> 2: [./build/exception_path(__rte_panic+0xc1) [0x41824a]]
> 1: [./build/exception_path(rte_dump_stack+0x23) [0x466013]]
> 
> 
> 
> Then I tried the following steps
> 
> (2) Started the sample path application first
> Pktgen next I got the below error:
> 
>   Pktgen created by: Keith Wiles -- >>> Powered by Intel? DPDK <<<
> ---
> EAL: Detected lcore 0 as core 0 on socket 0
> EAL: Detected lcore 1 as core 1 on socket 0
> EAL: Support maximum 64 logical core(s) by configuration.
> EAL: Detected 2 lcore(s)
> EAL: No free hugepages reported in hugepages-2048kB
> PANIC in rte_eal_init():
> Cannot get hugepage information
> 6: [./app/build/pktgen() [0x422ba5]]
> 5: [/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)
> [0x7fb46e0b476d]]
> 4: [./app/build/pktgen(main+0x116) [0x422286]]
> 3: [./app/build/pktgen(rte_eal_init+0x10ac) [0x4ac4dc]]
> 2: [./app/build/pktgen(__rte_panic+0xc1) [0x422007]]
> 1: [./app/build/pktgen(rte_dump_stack+0x23) [0x4b2c43]]
> 
> (3) I tried specifying no of pages that both the pktgen and sample
> application needs to take by specifying it in commandline in the EAL
> options:
> But that also didnt work:
> 
> EAL: Virtual area found at 0x7fcff620 (size = 0x20)
> EAL: Not enough memory available on socket 1! Requested: 128MB, available:
> 0MB
> PANIC in rte_eal_init():
> Cannot init memory
> 6: [./build/exception_path() [0x418cc5]]
> 5: [/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)
> [0x7fd0790f876d]]
> 4: [./build/exception_path(main+0x39) [0x4183e9]]
> 3: [./build/exception_path(rte_eal_init+0x1808) [0x460008]]
> 2: [./build/exception_path(__rte_panic+0xc1) [0x41824a]]
> 1: [./build/exception_path(rte_dump_stack+0x23) [0x466013]]
> 
> (4) Then I went forward to edit the no of hugepages allocated in the script
> /etc/sysctl.conf and then
> ran the setup.sh script under the tools dir to allocate hugepages for each
> separate application
> 
> This time also I faced issues related to memory running out:
> 
> HugePages: 0 kB
> HugePages_Total: 256
> HugePages_Free:  256
> HugePages_Rsvd:0
> HugePages_Surp:0
> Hugepagesize:   2048 kB
> 
> EAL: Requesting 256 pages of size 2MB from socket 0
> EAL: TSC frequency is ~2689707 KHz
> EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable
> clock cycles !
> EAL: Master core 0 is ready (tid=4fd54800)
> EAL: Core 1 is ready (tid=2c7fe700)
> EAL: PCI device :00:08.0 on NUMA socket -1
> EAL:   probe driver: 8086:100f rte_em_pmd
> EAL:   PCI memory mapped at 0x7f894fcfe000
> EAL: PCI device :00:09.0 on NUMA socket -1
> EAL:   probe driver: 8086:100f rte_em_pmd
> EAL:   PCI memory mapped at 0x7f894fcde000
> EAL: PCI device :00:0a.0 on NUMA socket -1
> EAL:   probe driver: 8086:100f rte_em_pmd
> EAL:   :00:0a.0 not managed by UIO driver, skipping
> EAL: PCI device :00:11.0 on NUMA socket -1
> EAL:   probe driver: 8086:100f rte_em_pmd
> EAL:   :00:11.0 not managed by UIO driver, skipping
> EAL: PCI device :00:0a.0 on NUMA socket -1
> EAL:   probe driver: 8086:100f rte_em_pmd
> EAL:   :00:0a.0 not managed by UIO driver, skipping
> EAL: PCI device :00:11.0 on NUMA socket -1
> EAL:   probe driver: 8086:100f rte_em_pmd
> EAL:   :00:11.0 not managed by UIO driver, skipping
>

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Ananyev, Konstantin]

> -Original Message-
> From: Wodkowski, PawelX
> Sent: Monday, March 09, 2015 11:09 AM
> To: Ananyev, Konstantin; Vlad Zolotarov; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
> >
> >
> >> -Original Message-
> >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> >> Sent: Monday, March 09, 2015 10:21 AM
> >> To: dev at dpdk.org
> >> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >>
> >> - Removed the not needed casting.
> >> - 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 | 27 ---
> >>   1 file changed, 12 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> >> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> index 72c65df..609b5fd 100644
> >> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
> >>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);
> >
> > I think all these extra casts came in to keep icc 12.* compiling without 
> > warnings.
> > I am agree that they are unnecessary.
> > Though if we still have to support icc 12.* we either need to keep them, or 
> > find
> > some other way to keep it happy.
> > Konstantin
> >
> 
> What warnings icc 12.* is throwing? 

Try and see :)

>Only warning I can think of here is
> signed -> unsigned implicit cast. 

If I remember things correctly, it considered result at right side of '=' 
operator as unsigned int,
and then complained that we assign it to smaller size (unsigned short) operand. 
 

>Changing '1' to '1U' helps?

Don't think so, but you are welcome to try.

Konstantin

> 
> 
> --
> Pawel

[dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking

[Thomas Monjalon]

> > Fixed strict-aliasing rules breaking errors for some GCC version.
> >
> > Signed-off-by: Zhihong Wang 
> 
> Acked-by:Cunming Liang  >

Applied, thanks

[dpdk-dev] [PATCH] librte_eal/common: Fix cast from pointer to integer of different size

[Thomas Monjalon]

> > ./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 
> > Signed-off-by: Zhihong Wang 
> > ---
> > v4 --> v3:
> > fix dstofss/bits to size_t in rte_memcpy()
> > v3 --> v2:
> > make dstofss and srcofs to be type size_t
> > casting type use uintptr_t
> > 
> > v2 --> v1:
> > Remove unnecessary casting (void *)
> 
> Acked-by:  Wang, Zhihong 

Applied, thanks

[dpdk-dev] [PATCH] testpmd: Fix action of operationg invalid port

[Thomas Monjalon]

> > Currently, if try to  start/stop/close one invalid prot,
> > no error shows in testpmd.
> > This is a bug, need check the port number.
> >
> > Signed-off-by: Michael Qiu 
> 
> Acked-by: Tetsuya Mukawa 

Applied, thanks

[dpdk-dev] [PATCH v3 0/2] New HW VLAN command in testpmd

[Thomas Monjalon]

> This patch enables testpmd user can config port hw_vlan with more fine 
> granularity:
> hw vlan filter, hw vlan strip, and hw vlan extend;
> 
> Update testpmd doc accordingly.
> 
> Changchun Ouyang (2):
>   testpmd: HW vlan command
>   doc: Update for new HW vlan command

It may help in validation of this release.
Applied, thanks

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

[Thomas Monjalon]

2015-03-07 21:38, 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

Applied, thanks

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

[Thomas Monjalon]

> > 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 
> 
> Acked-by: Pablo de Lara 

Applied, thanks

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

[Thomas Monjalon]

> > 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()
> 
> Acked-by: Pablo de Lara 

Applied, thanks

[dpdk-dev] [PATCH] testpmd: bond port creation did not enable bond port

[Thomas Monjalon]

> > When invoking creation of bonded device using:
> > create bonded device mode socket in testpmd the bonded port was not
> > enabled at the end of cmd_create_bonded_device_parsed function.
> > This caused commands 'show port info' and 'show port stats' not working
> > properly with bonding device. This patch fixed it.
> > 
> > Signed-off-by: Michal Jastrzebski 
> 
> Acked-by: Pablo de Lara 

Applied, thanks

[dpdk-dev] [PATCH] test whether file descriptor is valid before close it

[Thomas Monjalon]

> > This avoids closing -1 in our case.
> > 
> > Signed-off-by: Huawei Xie 
> 
> Acked-by: Changchun Ouyang 

Applied, thanks

[dpdk-dev] [PATCH] lib/librte_vhost: exchange kickfd and callfd to avoid confusion

[Thomas Monjalon]

> > Previous vhost implementation wrongly name kickfd as callfd and callfd as
> > kickfd.
> > It is functional correct, but causes confusion.
> > 
> > Signed-off-by: Huawei Xie 
> 
> Acked-by: Changchun Ouyang 

Applied, thanks

[dpdk-dev] [PATCH] lib/librte_vhost: use loop instead of goto

[Thomas Monjalon]

> > This patch reorder the code a bit to use loop instead of goto.
> > Besides, remove abudant check 'fd != -1'.
> > 
> > Signed-off-by: Huawei Xie 
> 
> Acked-by: Konstantin Ananyev 

Applied, thanks

[dpdk-dev] [PATCH] lib/librte_vhost: combine select with sleep

[Thomas Monjalon]

> > combine sleep into select when there is no file descriptors to be monitored.
> > 
> > Signed-off-by: Huawei Xie 
> 
> Acked-by: Konstantin Ananyev 

Applied, thanks

[dpdk-dev] [PATCH] app/test-pmd: Fix log issue without nic binded

[Thomas Monjalon]

> > As hotplug has been enabled, start the testpmd with no nic binded
> > will show one error log "Please stop the ports first":
> > 
> > Interactive-mode selected
> > Please stop the ports first
> > Done
> > testpmd>
> > 
> > This issue is cause by the logic of check link status.
> > 
> > Signed-off-by: Michael Qiu 
> 
> Acked-by: Pablo de Lara 

Applied, thanks

[dpdk-dev] [PATCH v2] ixgbe: Fix vf tx issue

[Thomas Monjalon]

> > X550 should use the correct macro to set the VFTDT and VFRDT register 
> > address.
> > This patch fixes the VF TX issue for Sageville.
> >
> > Signed-off-by: Changchun Ouyang 
> 
> Acked-by:Cunming Liang  >

Applied, thanks

[dpdk-dev] [PATCH v1 1/3] ixgbe: Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring descriptor fields

[Vlad Zolotarov]

On 03/09/15 12:29, Ananyev, Konstantin wrote:
> Hi Vlad,
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>> Sent: Monday, March 09, 2015 10:13 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v1 1/3] ixgbe: Use the 
>> rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring descriptor
>> fields
>>
>> Fixed the above in ixgbe_rx_alloc_bufs() and in ixgbe_recv_scattered_pkts().
>>
>> Signed-off-by: Vlad Zolotarov 
>> ---
>>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 13 +++--
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> index 9ecf3e5..b033e04 100644
>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> @@ -1028,7 +1028,7 @@ 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;
> Wonder Why you changed from uint64_t to __le64 here?
> Effectively __le64 is the same a uint64_t,

I'm afraid the above it's not completely correct. See below.

> and I think it is better to use always the same type across all PMD code for 
> consistency.

Pls., note that "dma_addr" is only used (see below)...

> Konstantin
>
>
>>  int diag, i;
>>
>>  /* allocate buffers in bulk directly into the S/W ring */
>> @@ -1051,7 +1051,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;

here. ;) And the type of both hdr_addr and pkt_addr is __le64.
I don't exactly understand what do u mean by "use the same type across 
all PMD code for consistency" - there are a lot of types used in the PMD 
code and __le64 is one of them... ;)

Now more seriously, let's recall what is the semantics of the __leXX 
types - they represent the integer in the "little endian" format. Here, 
NIC expects the physical address in a little endian format, thus the 
descriptor is defined the way it is defined - using __le64. The same 
relates to dma_addr local variable in this patch - it contains the 
physical (more correctly "DMA-able") address of the Rx buffer in the 
form NIC expects it to be written in the descriptor.

So, why to use __leXX anyway? - Debugging the (invalid) endianess is a 
real headache. Therefore there are a few static code analysis tools like 
"sparse" that allow to detect such inconsistencies (see here 
http://en.wikipedia.org/wiki/Sparse) and __leXX is a helper to allow 
tools like sparse to detect such problems.

In addition after spending some time writing patches for Linux netdev 
list u develop a strong habit for such stuff - Dave and others are very 
strict about such things... ;)

So, is it the same as uint64_t? I guess now it's clear why it is now... ;)

>>  }
>> @@ -1559,13 +1559,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. */
>> --
>> 2.1.0

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Vlad Zolotarov]

On 03/09/15 12:49, Ananyev, Konstantin wrote:
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>> Sent: Monday, March 09, 2015 10:21 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>
>> - Removed the not needed casting.
>> - 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 | 27 ---
>>   1 file changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> index 72c65df..609b5fd 100644
>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>>  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);
> I think all these extra casts came in to keep icc 12.* compiling without 
> warnings.
> I am agree that they are unnecessary.
> Though if we still have to support icc 12.* we either need to keep them, or 
> find
> some other way to keep it happy.
Fix icc maybe?
I'm sorry, but what do I miss here? Both alloc_idx, rxq->rx_free_trigger 
and rxq->rx_free_thresh are uint16_t

So, according to C standard the result is also uint16_t thus no casting 
is needed:

the result of a subtraction generating a negative number in an unsigned type is 
well-defined:

 1. [...] A computation involving unsigned operands can never
overflow, because a result that cannot be represented by the
resulting unsigned integer type is reduced modulo the number
that is one greater than the largest value that can be
represented by the resulting type. (ISO/IEC 9899:1999 (E) ?6.2.5/9)


Could u, pls., be more specific and send here the error generated by icc 
after my patches?

thanks,
vlad



> Konstantin
>
>>  rxep = &rxq->sw_ring[alloc_idx];
>>  diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep,
>>  rxq->rx_free_thresh);
>> @@ -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;
>> @@ -3560,6 +3558,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);
>> @@ -3582,7 +3581,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;
>> @@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>  /*
>>   * Configure jumbo frame support, if any.
>>   */
>> -if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
>> +if (rx_conf->jumbo_frame == 1) {
>>  hlreg0 |= IXGBE_HLREG0_JUMBOEN;
>>  maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS);
>>  maxfrs &= 0x;
>> -maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16);
>> +maxfrs |= (rx_conf->max_rx_pkt_len << 16);
>>  IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs);
>>  } else
>>  hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
>> @@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>   * Reset crc_len in case it was changed after queue setup by a
>>   * call to configure.
>>   */
>> -rxq->crc_len = (uint8_t)
>> -((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 :
>> -ETHER_CRC_LEN);
>> +rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN;
>>
>>  /* Setup the Base and Length of the Rx Descriptor Rings */
>>  bus_addr = rxq->rx_ring_phys_addr;
>> @@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_

[dpdk-dev] [PATCH] app/testpmd: fix potential out of bounds read

[Julien Cretin]

After the last enabled port has been seen, and the last time we
evaluate the loop condition, there is an out of bounds read in
ports[p].enabled because p is equal to size, which is the length of
ports.

Signed-off-by: Julien Cretin 
---
 app/test-pmd/testpmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 3e1327e..a9d15f3 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -333,7 +333,7 @@ find_next_port(portid_t p, struct rte_port *ports, int size)
if (ports == NULL)
rte_exit(-EINVAL, "failed to find a next port id\n");

-   while ((ports[p].enabled == 0) && (p < size))
+   while ((p < size) && (ports[p].enabled == 0))
p++;
return p;
 }
-- 
1.9.1

[dpdk-dev] [PATCH 1/5] ixgbe: make txq_ops const

[Bruce Richardson]

On Fri, Mar 06, 2015 at 06:23:20PM -0800, Stephen Hemminger wrote:
> All virtual function tables should be const so they are put
> in text segment rather than data.
> 
> Signed-off-by: Stephen Hemminger 
Acked-by: Bruce Richardson 

> ---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 2 +-
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.h | 2 +-
>  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 9ecf3e5..e6aec8f 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1754,7 +1754,7 @@ ixgbe_reset_tx_queue(struct igb_tx_queue *txq)
>   IXGBE_CTX_NUM * sizeof(struct ixgbe_advctx_info));
>  }
>  
> -static struct ixgbe_txq_ops def_txq_ops = {
> +static const struct ixgbe_txq_ops def_txq_ops = {
>   .release_mbufs = ixgbe_tx_queue_release_mbufs,
>   .free_swring = ixgbe_tx_free_swring,
>   .reset = ixgbe_reset_tx_queue,
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.h 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.h
> index 329007c..a85839e 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.h
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.h
> @@ -211,7 +211,7 @@ struct igb_tx_queue {
>   uint32_tctx_curr;  /**< Hardware context states. */
>   /** Hardware context0 history. */
>   struct ixgbe_advctx_info ctx_cache[IXGBE_CTX_NUM];
> - struct ixgbe_txq_ops *ops;  /**< txq ops */
> + const struct ixgbe_txq_ops *ops;   /**< txq ops */
>   uint8_t tx_deferred_start; /**< not in global dev start. */
>  };
>  
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> index 1f46f0f..11e9f12 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> @@ -723,7 +723,7 @@ ixgbe_reset_tx_queue(struct igb_tx_queue *txq)
>   IXGBE_CTX_NUM * sizeof(struct ixgbe_advctx_info));
>  }
>  
> -static struct ixgbe_txq_ops vec_txq_ops = {
> +static const struct ixgbe_txq_ops vec_txq_ops = {
>   .release_mbufs = ixgbe_tx_queue_release_mbufs,
>   .free_swring = ixgbe_tx_free_swring,
>   .reset = ixgbe_reset_tx_queue,
> -- 
> 2.1.4
> 

[dpdk-dev] [PATCH 2/5] ixgbe: make register maps const

[Bruce Richardson]

On Fri, Mar 06, 2015 at 06:23:21PM -0800, Stephen Hemminger wrote:
> These are const data structures, just put them in txt segment
> rather than having compiler emit code to set them up on the stack.
> 
> Signed-off-by: Stephen Hemminger 
Acked-by: Bruce Richardson 

> ---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index e6aec8f..8706c1e 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -860,14 +860,14 @@ rx_desc_hlen_type_rss_to_pkt_flags(uint32_t hl_tp_rs)
>  {
>   uint64_t pkt_flags;
>  
> - static uint64_t ip_pkt_types_map[16] = {
> + static const uint64_t ip_pkt_types_map[16] = {
>   0, PKT_RX_IPV4_HDR, PKT_RX_IPV4_HDR_EXT, PKT_RX_IPV4_HDR_EXT,
>   PKT_RX_IPV6_HDR, 0, 0, 0,
>   PKT_RX_IPV6_HDR_EXT, 0, 0, 0,
>   PKT_RX_IPV6_HDR_EXT, 0, 0, 0,
>   };
>  
> - static uint64_t ip_rss_types_map[16] = {
> + static const uint64_t ip_rss_types_map[16] = {
>   0, PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, PKT_RX_RSS_HASH,
>   0, PKT_RX_RSS_HASH, 0, PKT_RX_RSS_HASH,
>   PKT_RX_RSS_HASH, 0, 0, 0,
> -- 
> 2.1.4
> 

[dpdk-dev] [PATCH v1 3/3] ixgbe: Unify the rx_pkt_bulk callback initialization

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> Sent: Monday, March 9, 2015 10:13 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v1 3/3] ixgbe: Unify the rx_pkt_bulk callback
> initialization
> 
> @@ -3641,23 +3676,17 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>   buf_size = (uint16_t) ((srrctl & IXGBE_SRRCTL_BSIZEPKT_MASK)
> <<
>  IXGBE_SRRCTL_BSIZEPKT_SHIFT);
> 
> - if (dev->data->dev_conf.rxmode.enable_scatter ||
> - /* It adds dual VLAN length for supporting dual VLAN */
> - (dev->data->dev_conf.rxmode.max_rx_pkt_len +
> - 2 * IXGBE_VLAN_TAG_SIZE) > buf_size){
> - if (!dev->data->scattered_rx)
> - PMD_INIT_LOG(DEBUG, "forcing scatter mode");
> + /* It adds dual VLAN length for supporting dual VLAN */
> + if (dev->data->dev_conf.rxmode.max_rx_pkt_len +
> + 2 * IXGBE_VLAN_TAG_SIZE > buf_size)
>   dev->data->scattered_rx = 1;
> -#ifdef RTE_IXGBE_INC_VECTOR
> - if (rte_is_power_of_2(rxq->nb_rx_desc))
> - dev->rx_pkt_burst =
> - ixgbe_recv_scattered_pkts_vec;
> - else
> -#endif
> - dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
> - }
>   }
> 
> + if (rx_conf->enable_scatter)
> + dev->data->scattered_rx = 1;

Hi,

There is a compilation issue here when this patch is applied to the current 
master (with previous 2 patches applied as well):

make T=x86_64-native-linuxapp-gcc install CC=gcc
...
== Build lib/librte_pmd_ixgbe
  CC ixgbe_rxtx.o
/home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 
'ixgbe_dev_rx_init':
/home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c:3689:6: error: 'rx_conf' 
undeclared (first use in this function)
/home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c:3689:6: note: each undeclared 
identifier is reported only once for each function it appears in
make[5]: *** [ixgbe_rxtx.o] Error 1

This is using the default DPDK linuxapp config.

John

[dpdk-dev] [PATCH 3/5] ixgbe: make bulk alloc static

[Bruce Richardson]

On Fri, Mar 06, 2015 at 06:23:22PM -0800, Stephen Hemminger wrote:
> Only used in this file, make it static.
> 
> Signed-off-by: Stephen Hemminger 
Acked-by: Bruce Richardson 

> ---
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 5 -
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 2 +-
>  2 files changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h 
> b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
> index a549f5c..ffe3471 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
> @@ -341,11 +341,6 @@ void ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev);
>  uint16_t ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>   uint16_t nb_pkts);
>  
> -#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
> -uint16_t ixgbe_recv_pkts_bulk_alloc(void *rx_queue, struct rte_mbuf 
> **rx_pkts,
> - uint16_t nb_pkts);
> -#endif
> -
>  uint16_t ixgbe_recv_scattered_pkts(void *rx_queue,
>   struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
>  
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 8706c1e..0f32296 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1145,7 +1145,7 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  }
>  
>  /* split requests into chunks of size RTE_PMD_IXGBE_RX_MAX_BURST */
> -uint16_t
> +static uint16_t
>  ixgbe_recv_pkts_bulk_alloc(void *rx_queue, struct rte_mbuf **rx_pkts,
>  uint16_t nb_pkts)
>  {
> -- 
> 2.1.4
> 

[dpdk-dev] [PATCH 4/5] ixgbe: rename set_tx_function

[Bruce Richardson]

On Fri, Mar 06, 2015 at 06:23:23PM -0800, Stephen Hemminger wrote:
> All global functions in a driver should use the same prefix
> to avoid any future name collisions.
> 
> Signed-off-by: Stephen Hemminger 

I think this patch is missing one instance of set_tx_function at line 1963 of 
ixgbe_rxtx.c that needs to be renamed.

/Bruce
> ---
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 2 +-
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 2 +-
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.h   | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c 
> b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 9bdc046..e1504f4 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -753,7 +753,7 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct 
> eth_driver *eth_drv,
>* Tx queue may not initialized by primary process */
>   if (eth_dev->data->tx_queues) {
>   txq = 
> eth_dev->data->tx_queues[eth_dev->data->nb_tx_queues-1];
> - set_tx_function(eth_dev, txq);
> + ixgbe_set_tx_function(eth_dev, txq);
>   } else {
>   /* Use default TX function if we get here */
>   PMD_INIT_LOG(INFO, "No TX queues configured yet. "
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 0f32296..c5ba687 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1765,7 +1765,7 @@ static const struct ixgbe_txq_ops def_txq_ops = {
>   * in dev_init by secondary process when attaching to an existing ethdev.
>   */
>  void
> -set_tx_function(struct rte_eth_dev *dev, struct igb_tx_queue *txq)
> +ixgbe_set_tx_function(struct rte_eth_dev *dev, struct igb_tx_queue *txq)
>  {
>   /* Use a simple Tx queue (no offloads, no multi segs) if possible */
>   if (((txq->txq_flags & IXGBE_SIMPLE_FLAGS) == IXGBE_SIMPLE_FLAGS)
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.h 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.h
> index a85839e..42d59f9 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.h
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.h
> @@ -253,7 +253,7 @@ struct ixgbe_txq_ops {
>   * the queue parameters. Used in tx_queue_setup by primary process and then
>   * in dev_init by secondary process when attaching to an existing ethdev.
>   */
> -void set_tx_function(struct rte_eth_dev *dev, struct igb_tx_queue *txq);
> +void ixgbe_set_tx_function(struct rte_eth_dev *dev, struct igb_tx_queue 
> *txq);
>  
>  #ifdef RTE_IXGBE_INC_VECTOR
>  uint16_t ixgbe_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
> -- 
> 2.1.4
> 

[dpdk-dev] [PATCH 5/5] ixgbe: rename igb_* to ixgbe_*

[Bruce Richardson]

On Fri, Mar 06, 2015 at 06:23:24PM -0800, Stephen Hemminger wrote:
> To avoid any possible confusion or breakage, rename all the structures
> of ixgbe driver to use ixgbe_ rather than igb_ because igb is a
> different driver.
> 
> Signed-off-by: Stephen Hemminger 
Apart from one small fix to a call to set_tx_function() that belongs in patch
4, this looks ok.

Acked-by: Bruce Richardson 

> ---
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c   |   2 +-
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 124 
> +-
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.h |  26 +++
>  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c |  52 +++---
>  4 files changed, 102 insertions(+), 102 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c 
> b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index e1504f4..5473858 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -748,7 +748,7 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct 
> eth_driver *eth_drv,
>* RX and TX function.
>*/
>   if (rte_eal_process_type() != RTE_PROC_PRIMARY){
> - struct igb_tx_queue *txq;
> + struct ixgbe_tx_queue *txq;
>   /* TX queue function in primary, set by last queue initialized
>* Tx queue may not initialized by primary process */
>   if (eth_dev->data->tx_queues) {
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index c5ba687..1848a13 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -122,9 +122,9 @@ rte_rxmbuf_alloc(struct rte_mempool *mp)
>   * Return the total number of buffers freed.
>   */
>  static inline int __attribute__((always_inline))
> -ixgbe_tx_free_bufs(struct igb_tx_queue *txq)
> +ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
>  {
> - struct igb_tx_entry *txep;
> + struct ixgbe_tx_entry *txep;
>   uint32_t status;
>   int i;
>  
> @@ -208,11 +208,11 @@ tx1(volatile union ixgbe_adv_tx_desc *txdp, struct 
> rte_mbuf **pkts)
>   * Copy mbuf pointers to the S/W ring.
>   */
>  static inline void
> -ixgbe_tx_fill_hw_ring(struct igb_tx_queue *txq, struct rte_mbuf **pkts,
> +ixgbe_tx_fill_hw_ring(struct ixgbe_tx_queue *txq, struct rte_mbuf **pkts,
> uint16_t nb_pkts)
>  {
>   volatile union ixgbe_adv_tx_desc *txdp = &(txq->tx_ring[txq->tx_tail]);
> - struct igb_tx_entry *txep = &(txq->sw_ring[txq->tx_tail]);
> + struct ixgbe_tx_entry *txep = &(txq->sw_ring[txq->tx_tail]);
>   const int N_PER_LOOP = 4;
>   const int N_PER_LOOP_MASK = N_PER_LOOP-1;
>   int mainpart, leftover;
> @@ -244,7 +244,7 @@ static inline uint16_t
>  tx_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>uint16_t nb_pkts)
>  {
> - struct igb_tx_queue *txq = (struct igb_tx_queue *)tx_queue;
> + struct ixgbe_tx_queue *txq = (struct ixgbe_tx_queue *)tx_queue;
>   volatile union ixgbe_adv_tx_desc *tx_r = txq->tx_ring;
>   uint16_t n = 0;
>  
> @@ -352,7 +352,7 @@ ixgbe_xmit_pkts_simple(void *tx_queue, struct rte_mbuf 
> **tx_pkts,
>  }
>  
>  static inline void
> -ixgbe_set_xmit_ctx(struct igb_tx_queue* txq,
> +ixgbe_set_xmit_ctx(struct ixgbe_tx_queue* txq,
>   volatile struct ixgbe_adv_tx_context_desc *ctx_txd,
>   uint64_t ol_flags, union ixgbe_tx_offload tx_offload)
>  {
> @@ -442,7 +442,7 @@ ixgbe_set_xmit_ctx(struct igb_tx_queue* txq,
>   * or create a new context descriptor.
>   */
>  static inline uint32_t
> -what_advctx_update(struct igb_tx_queue *txq, uint64_t flags,
> +what_advctx_update(struct ixgbe_tx_queue *txq, uint64_t flags,
>   union ixgbe_tx_offload tx_offload)
>  {
>   /* If match with the current used context */
> @@ -498,9 +498,9 @@ tx_desc_ol_flags_to_cmdtype(uint64_t ol_flags)
>  
>  /* Reset transmit descriptors after they have been used */
>  static inline int
> -ixgbe_xmit_cleanup(struct igb_tx_queue *txq)
> +ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq)
>  {
> - struct igb_tx_entry *sw_ring = txq->sw_ring;
> + struct ixgbe_tx_entry *sw_ring = txq->sw_ring;
>   volatile union ixgbe_adv_tx_desc *txr = txq->tx_ring;
>   uint16_t last_desc_cleaned = txq->last_desc_cleaned;
>   uint16_t nb_tx_desc = txq->nb_tx_desc;
> @@ -559,9 +559,9 @@ uint16_t
>  ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>   uint16_t nb_pkts)
>  {
> - struct igb_tx_queue *txq;
> - struct igb_tx_entry *sw_ring;
> - struct igb_tx_entry *txe, *txn;
> + struct ixgbe_tx_queue *txq;
> + struct ixgbe_tx_entry *sw_ring;
> + struct ixgbe_tx_entry *txe, *txn;
>   volatile union ixgbe_adv_tx_desc *txr;
>   volatile union ixgbe_adv_tx_desc *txd;
>   struct rte_mbuf *tx_pkt;
> @@ -938,10 +938,10 @@ rx_desc_error_to_pkt_flags(uint32_t rx_status)
>  #error "PMD IXGBE: LOOK_AHEAD must be 8\n"
>  #endif
>  static inline int
> -ixgbe_rx_scan_hw

[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian

[Bruce Richardson]

On Wed, Mar 04, 2015 at 02:34:12PM +0800, xuelin.shi at freescale.com wrote:
> From: Xuelin Shi 
> 
> This module uses type conversion between struct and int.
> Also truncation and comparison is used with this int.
> It is not safe for different endian arch.
> 
> Add ifdef for big endian struct to fix this issue.
> 
> Signed-off-by: Xuelin Shi 
> ---
>  lib/librte_lpm/rte_lpm.h | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h
> index 1af150c..08a2859 100644
> --- a/lib/librte_lpm/rte_lpm.h
> +++ b/lib/librte_lpm/rte_lpm.h
> @@ -96,6 +96,7 @@ extern "C" {
>  /** Bitmask used to indicate successful lookup */
>  #define RTE_LPM_LOOKUP_SUCCESS  0x0100
>  
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
>  /** @internal Tbl24 entry structure. */
>  struct rte_lpm_tbl24_entry {
>   /* Stores Next hop or group index (i.e. gindex)into tbl8. */
> @@ -117,6 +118,24 @@ struct rte_lpm_tbl8_entry {
>   uint8_t valid_group :1; /**< Group validation flag. */
>   uint8_t depth   :6; /**< Rule depth. */
>  };
> +#else
> +struct rte_lpm_tbl24_entry {
> + uint8_t depth   :6;
> + uint8_t ext_entry   :1;
> + uint8_t valid   :1;
> + union {
> + uint8_t tbl8_gindex;
> + uint8_t next_hop;
> + };
> +};
> +
> +struct rte_lpm_tbl8_entry {
> + uint8_t depth   :6;
> + uint8_t valid_group :1;
> + uint8_t valid   :1;
> + uint8_t next_hop;
> +};
> +#endif
>  
>  /** @internal Rule structure. */
>  struct rte_lpm_rule {
> -- 
> 1.9.1
> 

Get an error compiling this up (using clang on FreeBSD).

  CC rte_lpm.o
  In file included from /usr/home/bruce/dpdk.org/lib/librte_lpm/rte_lpm.c:57:
  /usr/home/bruce/dpdk.org/lib/librte_lpm/rte_lpm.h:99:5: fatal error: 
'RTE_BYTE_ORDER' is not defined, evaluates to 0 [-Wundef]
#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
  ^
  1 error generated.

Adding "#include " should fix the issue.

Existing unit tests on IA (little endian) pass fine there-after, but I think for
this patch it would be good to have an ack from someone who can validate on
a big endian system, since this is what this patch is meant to enable.

/Bruce

[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian

[Thomas Monjalon]

2015-03-09 09:12, Mcnamara, John:
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Xuelin Shi
> > Yes, the patch http://dpdk.org/dev/patchwork/patch/3132/ should be
> > abandoned.
> 
> If you register and login to the DPDK patchwork site you can mark the patch
> as "Not Applicable" or whatever category now applies.
> 
> http://dpdk.org/dev/patchwork/user/login/

"superseded" would be more appropriate here.
Generally, "not applicable" should be used for emails which are not real
patches or really wrongly formatted.

[dpdk-dev] [PATCH] doc: update of testpmd user guide

[Helin Zhang]

It updated the user guide of testpmd, for the newly supported commands
of getting/setting hash functions, and the command of showing port
information.

Signed-off-by: Helin Zhang 
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 76 +++--
 1 file changed, 73 insertions(+), 3 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a99e14d..cc01c89 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -138,16 +138,32 @@ For example:
 * Infos for port 0 *

 MAC address: XX:XX:XX:XX:XX:XX
+Connect to socket: 0
+memory allocation on the socket: 0
 Link status: up
-Link speed: 1 Mbps
+Link speed: 4 Mbps
 Link duplex: full-duplex
 Promiscuous mode: enabled
 Allmulticast mode: disabled
-Maximum number of MAC addresses: 127
+Maximum number of MAC addresses: 64
+Maximum number of MAC addresses of hash filtering: 0
 VLAN offload:
 strip on
 filter on
 qinq(extend) off
+Redirection table size: 512
+Supported flow types:
+  ipv4-frag
+  ipv4-tcp
+  ipv4-udp
+  ipv4-sctp
+  ipv4-other
+  ipv6-frag
+  ipv6-tcp
+  ipv6-udp
+  ipv6-sctp
+  ipv6-other
+  l2_payload

 show port rss reta
 ~~
@@ -968,7 +984,7 @@ port config - RSS

 Set the RSS (Receive Side Scaling) mode on or off:

-port config all rss (ip|udp|none)
+port config all rss (all|ip|tcp|udp|sctp|ether|none)

 RSS is on by default.

@@ -1518,3 +1534,57 @@ For example, to select the first 16 bytes from the 
offset 4 (bytes) of packet?
 .. code-block:: console

testpmd> flow_director_flex_payload 0 l4 
(4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19)
+
+get_sym_hash_ena_per_port
+~
+
+Get symmetric hash enable configuration per port.
+
+get_sym_hash_ena_per_port (port_id)
+
+For example, to get symmetric hash enable configuration of port 1.
+
+.. code-block:: console
+
+testpmd> get_sym_hash_ena_per_port 1
+
+set_sym_hash_ena_per_port
+~
+
+Set symmetric hash enable configuration per port to enable or disable.
+
+set_sym_hash_ena_per_port (port_id) (enable|disable)
+
+For example, to set symmetric hash enable configuration of port 1 to enable.
+
+.. code-block:: console
+
+testpmd> set_sym_hash_ena_per_port 1 enable
+
+get_hash_global_config
+~~
+
+Get the global configurations of hash filters.
+
+get_hash_global_config (port_id)
+
+For example, to get the global configurations of hash filters of port 1.
+
+.. code-block:: console
+
+testpmd> get_hash_global_config 1
+
+set_hash_global_config
+~~
+
+Set the global configurations of hash filters.
+
+set_hash_global_config (port_id) (toeplitz|simple_xor|default)
+(ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload)
+(enable|disable)
+
+For example, to enable simple_xor for flow type of ipv6 on port 2.
+
+.. code-block:: console
+
+testpmd> set_hash_global_config 2 simple_xor ipv6 enable
-- 
1.9.3

[dpdk-dev] [PATCH 0/2] fix virtio interrupt handling

[David Marchand]

This patchset fixes a segfault in eal linux when using virtio pmd with link
status change interrupts in place because of an incorrect io privilege level.


Ouyang,

Fixing this has revealed some problems to be fixed (for 2.0.0 at least for the
first):
- changing link status from qemu monitor triggers this error message (even if
  link status seems to be correctly set)

testpmd> EAL: Error reading interrupts status for fd 0

- log macros in virtio pmd (especially for init) are still under a build option.
  They should be aligned on other pmds to make it easier to debug, see ixgbe pmd
  and/or thread:
  http://dpdk.org/ml/archives/dev/2014-September/005450.html

Can you look at these ?

Thanks.

-- 
David Marchand

David Marchand (2):
  eal/linux: move plugin load to very start of eal init
  virtio: change io privilege level as early as possible

 lib/librte_eal/linuxapp/eal/eal.c |   14 +++
 lib/librte_pmd_virtio/virtio_ethdev.c |   71 -
 2 files changed, 50 insertions(+), 35 deletions(-)

-- 
1.7.10.4

[dpdk-dev] [PATCH 1/2] eal/linux: move plugin load to very start of eal init

[David Marchand]

Loading shared libraries should be done at the very start of eal init so that
the code statically built in dpdk and the code loaded from shared objects is
handled (almost) the same way wrt to call to rte_eal_init().
The only thing that must be done before is filling the solib_list which is done
by eal_parse_args().

Signed-off-by: David Marchand 
---
 lib/librte_eal/linuxapp/eal/eal.c |   14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
b/lib/librte_eal/linuxapp/eal/eal.c
index 16f9e7c..c1c103d 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -725,6 +725,13 @@ rte_eal_init(int argc, char **argv)
if (fctret < 0)
exit(1);

+   TAILQ_FOREACH(solib, &solib_list, next) {
+   RTE_LOG(INFO, EAL, "open shared lib %s\n", solib->name);
+   solib->lib_handle = dlopen(solib->name, RTLD_NOW);
+   if (solib->lib_handle == NULL)
+   RTE_LOG(WARNING, EAL, "%s\n", dlerror());
+   }
+
/* set log level as early as possible */
rte_set_log_level(internal_config.log_level);

@@ -797,13 +804,6 @@ rte_eal_init(int argc, char **argv)

rte_eal_mcfg_complete();

-   TAILQ_FOREACH(solib, &solib_list, next) {
-   RTE_LOG(INFO, EAL, "open shared lib %s\n", solib->name);
-   solib->lib_handle = dlopen(solib->name, RTLD_NOW);
-   if (solib->lib_handle == NULL)
-   RTE_LOG(WARNING, EAL, "%s\n", dlerror());
-   }
-
eal_thread_init_master(rte_config.master_lcore);

ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
-- 
1.7.10.4

[dpdk-dev] [PATCH 2/2] virtio: change io privilege level as early as possible

[David Marchand]

Playing with virtio link status triggers a segfault because of an incorrect io
privilege level.

To reproduce the problem, virtio device must be bound to igb_uio to have lsc
enabled.

$ lspci |grep Ethernet
00:03.0 Ethernet controller: Red Hat, Inc Virtio network device
$ modprobe uio
$ insmod ./x86_64-native-linuxapp-gcc/kmod/igb_uio.ko
$ echo :00:03.0 > /sys/bus/pci/devices/\:00\:03.0/driver/unbind
$ echo 1af4 1000 > /sys/bus/pci/drivers/igb_uio/new_id
$ ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 3 -w :00:03.0 -- -i 
--txqflags=0xf01
[snip]
EAL: PCI device :00:03.0 on NUMA socket -1
EAL:   probe driver: 1af4:1000 rte_virtio_pmd
Interactive-mode selected
Configuring Port 0 (socket 0)
Port 0: DE:AD:DE:01:02:03
Checking link statuses...
Port 0 Link Up - speed 1 Mbps - full-duplex
Done
testpmd>

Then, from qemu monitor:
(qemu) set_link virtio-net-pci.0 off

testpmd> Segmentation fault

A call to rte_eal_iopl_init() in a specific constructor has been added so that,
on Linux, iopl() is called asap:
- for statically linked virtio pmd, the constructor will be called at binary
  start,
- for shared virtio pmd (loaded using -d eal option), it will be called at
  dlopen() in rte_eal_init().

On linux, iopl() effects are inherited by children threads, so calling it in a
constructor will ensure that any thread / process created after rte_eal_init()
inherits the io privilege level needed by the pmd.

We keep call to rte_eal_iopl_init() in the pmd init macro so that the pmd will
not register if the io privilege level is insufficient.

Besides, I moved rte_virtio_pmd_init() and related structure near its calling
site, so that all init code can be looked at in a glance.
The only change here is a new comment.

Signed-off-by: David Marchand 
---
 lib/librte_pmd_virtio/virtio_ethdev.c |   71 -
 1 file changed, 43 insertions(+), 28 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c 
b/lib/librte_pmd_virtio/virtio_ethdev.c
index d239083..29332fa 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -1228,34 +1228,6 @@ eth_virtio_dev_init(__rte_unused struct eth_driver 
*eth_drv,
return 0;
 }

-static struct eth_driver rte_virtio_pmd = {
-   {
-   .name = "rte_virtio_pmd",
-   .id_table = pci_id_virtio_map,
-   },
-   .eth_dev_init = eth_virtio_dev_init,
-   .dev_private_size = sizeof(struct virtio_hw),
-};
-
-/*
- * Driver initialization routine.
- * Invoked once at EAL init time.
- * Register itself as the [Poll Mode] Driver of PCI virtio devices.
- * Returns 0 on success.
- */
-static int
-rte_virtio_pmd_init(const char *name __rte_unused,
-   const char *param __rte_unused)
-{
-   if (rte_eal_iopl_init() != 0) {
-   PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
-   return -1;
-   }
-
-   rte_eth_driver_register(&rte_virtio_pmd);
-   return 0;
-}
-
 /*
  * Only 1 queue is supported, no queue release related operation
  */
@@ -1484,6 +1456,49 @@ __rte_unused uint8_t is_rx)
return 0;
 }

+/*
+ * Early init function, this is needed so that iopl() on linux is done before
+ * rte_eal_init (since it creates other threads that must inherit from the one
+ * that calls rte_eal_init).
+ */
+static void __attribute__((constructor, used))
+rte_virtio_early_init(void)
+{
+   /* return value is checked at pmd init time, no need to check here, see
+* below. */
+   rte_eal_iopl_init();
+}
+
+static struct eth_driver rte_virtio_pmd = {
+   {
+   .name = "rte_virtio_pmd",
+   .id_table = pci_id_virtio_map,
+   },
+   .eth_dev_init = eth_virtio_dev_init,
+   .dev_private_size = sizeof(struct virtio_hw),
+};
+
+/*
+ * Driver initialization routine.
+ * Invoked once at EAL init time.
+ * Register itself as the [Poll Mode] Driver of PCI virtio devices.
+ * Returns 0 on success.
+ */
+static int
+rte_virtio_pmd_init(const char *name __rte_unused,
+   const char *param __rte_unused)
+{
+   /* rte_eal_iopl_init() is already called by rte_virtio_early_init().
+* Call it again here, to be sure we did get the io level we wanted. */
+   if (rte_eal_iopl_init() != 0) {
+   PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
+   return -1;
+   }
+
+   rte_eth_driver_register(&rte_virtio_pmd);
+   return 0;
+}
+
 static struct rte_driver rte_virtio_driver = {
.type = PMD_PDEV,
.init = rte_virtio_pmd_init,
-- 
1.7.10.4

[dpdk-dev] [PATCH 1/2] eal/linux: move plugin load to very start of eal init

[Neil Horman]

On Mon, Mar 09, 2015 at 03:56:38PM +0100, David Marchand wrote:
> Loading shared libraries should be done at the very start of eal init so that
> the code statically built in dpdk and the code loaded from shared objects is
> handled (almost) the same way wrt to call to rte_eal_init().
> The only thing that must be done before is filling the solib_list which is 
> done
> by eal_parse_args().
> 
> Signed-off-by: David Marchand 
> ---
>  lib/librte_eal/linuxapp/eal/eal.c |   14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
> b/lib/librte_eal/linuxapp/eal/eal.c
> index 16f9e7c..c1c103d 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -725,6 +725,13 @@ rte_eal_init(int argc, char **argv)
>   if (fctret < 0)
>   exit(1);
>  
> + TAILQ_FOREACH(solib, &solib_list, next) {
> + RTE_LOG(INFO, EAL, "open shared lib %s\n", solib->name);
> + solib->lib_handle = dlopen(solib->name, RTLD_NOW);
> + if (solib->lib_handle == NULL)
> + RTE_LOG(WARNING, EAL, "%s\n", dlerror());
> + }
> +
>   /* set log level as early as possible */
>   rte_set_log_level(internal_config.log_level);
>  
> @@ -797,13 +804,6 @@ rte_eal_init(int argc, char **argv)
>  
>   rte_eal_mcfg_complete();
>  
> - TAILQ_FOREACH(solib, &solib_list, next) {
> - RTE_LOG(INFO, EAL, "open shared lib %s\n", solib->name);
> - solib->lib_handle = dlopen(solib->name, RTLD_NOW);
> - if (solib->lib_handle == NULL)
> - RTE_LOG(WARNING, EAL, "%s\n", dlerror());
> - }
> -
>   eal_thread_init_master(rte_config.master_lcore);
>  
>   ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
> -- 
> 1.7.10.4
> 
> 

I don't see anything explicitly wrong with this, but at the same time it doesn't
seem to fix anything.  Is there a particular bug that you're fixing in relation
to your cover letter here?  Or is there some expectation that PMD's loaded in
this fashion expect the dpdk to be completely uninitalized?  That would seem
like a strange operational requirement to me.

Neil

[dpdk-dev] [PATCH] claim responsibility for KVM virtio PMD, vhost backend, and XEN virtio solution.

[Huawei Xie]

---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 07fdf5e..b4327d3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -131,6 +131,12 @@ F: app/test-pmd/mempool_*
 F: examples/vhost_xen/
 F: doc/guides/prog_guide/intel_dpdk_xen_based_packet_switch_sol.rst

+Linux Xen VIRTIO
+M: Huawei Xie 
+F: lib/librte_pmd_xenvirt/
+F: examples/vhost_xen/
+F: doc/guides/prog_guide/intel_dpdk_xen_based_packet_switch_sol.rst
+
 FreeBSD EAL (with overlaps)
 M: Bruce Richardson 
 F: lib/librte_eal/bsdapp/Makefile
@@ -226,6 +232,7 @@ F: lib/librte_pmd_mlx4/
 F: doc/guides/prog_guide/mlx4_poll_mode_drv.rst

 RedHat virtio
+M: Huawei Xie 
 M: Changchun Ouyang 
 F: lib/librte_pmd_virtio/
 F: doc/guides/prog_guide/poll_mode_drv_emulated_virtio_nic.rst
-- 
1.8.1.4

[dpdk-dev] [PATCH] claim responsibility for KVM virtio PMD, vhost backend, and XEN virtio solution.

[Huawei Xie]

---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 07fdf5e..b4327d3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -131,6 +131,12 @@ F: app/test-pmd/mempool_*
 F: examples/vhost_xen/
 F: doc/guides/prog_guide/intel_dpdk_xen_based_packet_switch_sol.rst

+Linux Xen VIRTIO
+M: Huawei Xie 
+F: lib/librte_pmd_xenvirt/
+F: examples/vhost_xen/
+F: doc/guides/prog_guide/intel_dpdk_xen_based_packet_switch_sol.rst
+
 FreeBSD EAL (with overlaps)
 M: Bruce Richardson 
 F: lib/librte_eal/bsdapp/Makefile
@@ -226,6 +232,7 @@ F: lib/librte_pmd_mlx4/
 F: doc/guides/prog_guide/mlx4_poll_mode_drv.rst

 RedHat virtio
+M: Huawei Xie 
 M: Changchun Ouyang 
 F: lib/librte_pmd_virtio/
 F: doc/guides/prog_guide/poll_mode_drv_emulated_virtio_nic.rst
-- 
1.8.1.4

[dpdk-dev] Guest Machine is not Pingable from Host Machine

[Arkajit Ghosh]

Hi,

Can any one please suggest how to proceed.


Thanks & Regards
Arkajit Ghosh


-Arkajit Ghosh/DEL/TCS wrote: -
To: Ngo Doan Lap 
From: Arkajit Ghosh/DEL/TCS
Date: 03/09/2015 09:39AM
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] Guest Machine is not Pingable from Host Machine


Hi, 

I have executed below commands:

./utilities/ovs-vsctl add-br ovsbr0
./utilities/ovs-vsctl set bridge datapath_type=netdev
./utilities/ovs-vsctl add-port ovsbr0 dpdk0 -- set Interface dpdk0 type=dpdk
./utilities/ovs-vsctl add-port ovsbr0 dpdk1 -- set Interface dpdk1 type=dpdk
./utilities/ovs-vsctl show

Please find the attachement for bridge and dpdk-port creation..

Now, If i do "ifconfig -a" the created bridge ovsbr0 is showing Broadcast 
Running and Multicast but ip address was not given. So i had assign an 
IP-address to ovsbr0 which previously Eth1 had. 

So, now I try to ping this guest machine ip address from its host.? Here is the 
output. 

ping 192.168.1.7
PING 192.168.1.7 (192.168.1.7) 56(84) bytes of data.
>From 192.168.1.40 icmp_seq=1 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=5 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=6 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=7 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=8 Destination Host Unreachable
>From 192.168.1.40 icmp_seq=9 Destination Host Unreachable

I believe it should be pinged from its host machine. Can please let? me know 
what is the issue and how to proceed.

Thanks & Regards
Arkajit Ghosh


-Ngo Doan Lap  wrote: -
To: Arkajit Ghosh 
From: Ngo Doan Lap 
Date: 03/06/2015 08:52PM
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] Guest Machine is not Pingable from Host Machine

Hi,
Can you show the log output of OVS? and the ouput of following command
cd path/to/ovs/utilities/
?./ovs-ofctl show br0
?./ovs-ofctl dump-flows br0
(Assum that br0 is a bridge name that you had created)

On Fri, Mar 6, 2015 at 10:14 PM, Arkajit Ghosh  wrote:
Hi,

 Can anyone please suggest how to proceed.


 Thanks & Regards
 Arkajit Ghosh

 -Arkajit Ghosh/DEL/TCS wrote: -
 To: dev at dpdk.org
 From: Arkajit Ghosh/DEL/TCS
 Date: 03/04/2015 12:39PM
 Subject: Guest Machine is not Pingable from Host Machine

 Hi Team,

 Guest machine is not pingable from Host machine after creating a bridge with 
datapath_type "netdev" in the configuration database and adding? dpdk ports. 
Can anyone please let me know what is the issue.

 Thanks in advance.

 Thanks & Regards
 Arkajit Ghosh


 =-=-=
 Notice: The information contained in this e-mail
 message and/or attachments to it may contain
 confidential or privileged information. If you are
 not the intended recipient, any dissemination, use,
 review, distribution, printing or copying of the
 information contained in this e-mail message
 and/or attachments to it are strictly prohibited. If
 you have received this communication in error,
 please notify us by reply e-mail or telephone and
 immediately and permanently delete the message
 and any attachments. Thank you





-- 
Thanks and Best Regards,
Ngo Doan Lap
Mobile: 0977.833.757   

[attachment "DPDK_Output_Result.odt" removed by Arkajit Ghosh/DEL/TCS]

[dpdk-dev] [PATCH v1 3/3] ixgbe: Unify the rx_pkt_bulk callback initialization

[Vlad Zolotarov]

On 03/09/15 15:39, Mcnamara, John wrote:
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>> Sent: Monday, March 9, 2015 10:13 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v1 3/3] ixgbe: Unify the rx_pkt_bulk callback
>> initialization
>>
>> @@ -3641,23 +3676,17 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>  buf_size = (uint16_t) ((srrctl & IXGBE_SRRCTL_BSIZEPKT_MASK)
>> <<
>> IXGBE_SRRCTL_BSIZEPKT_SHIFT);
>>
>> -if (dev->data->dev_conf.rxmode.enable_scatter ||
>> -/* It adds dual VLAN length for supporting dual VLAN */
>> -(dev->data->dev_conf.rxmode.max_rx_pkt_len +
>> -2 * IXGBE_VLAN_TAG_SIZE) > buf_size){
>> -if (!dev->data->scattered_rx)
>> -PMD_INIT_LOG(DEBUG, "forcing scatter mode");
>> +/* It adds dual VLAN length for supporting dual VLAN */
>> +if (dev->data->dev_conf.rxmode.max_rx_pkt_len +
>> +2 * IXGBE_VLAN_TAG_SIZE > buf_size)
>>  dev->data->scattered_rx = 1;
>> -#ifdef RTE_IXGBE_INC_VECTOR
>> -if (rte_is_power_of_2(rxq->nb_rx_desc))
>> -dev->rx_pkt_burst =
>> -ixgbe_recv_scattered_pkts_vec;
>> -else
>> -#endif
>> -dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
>> -}
>>  }
>>
>> +if (rx_conf->enable_scatter)
>> +dev->data->scattered_rx = 1;
> Hi,
>
> There is a compilation issue here when this patch is applied to the current 
> master (with previous 2 patches applied as well):
>
> make T=x86_64-native-linuxapp-gcc install CC=gcc
> ...
> == Build lib/librte_pmd_ixgbe
>CC ixgbe_rxtx.o
> /home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 
> 'ixgbe_dev_rx_init':
> /home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c:3689:6: error: 'rx_conf' 
> undeclared (first use in this function)
> /home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c:3689:6: note: each 
> undeclared identifier is reported only once for each function it appears in
> make[5]: *** [ixgbe_rxtx.o] Error 1
>
> This is using the default DPDK linuxapp config.

Oops. I see. After all the acrobatics I've done with the patches I must 
have forgotten to check the compilation of this series stand alone - 
shame on me!
Let me fix it and resend the patches...

>
> John
>

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Vlad Zolotarov]

On 03/09/15 13:29, Ananyev, Konstantin wrote:
>
>> -Original Message-
>> From: Wodkowski, PawelX
>> Sent: Monday, March 09, 2015 11:09 AM
>> To: Ananyev, Konstantin; Vlad Zolotarov; dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>
>> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
>>>
 -Original Message-
 From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
 Sent: Monday, March 09, 2015 10:21 AM
 To: dev at dpdk.org
 Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

  - Removed the not needed casting.
  - 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 | 27 ---
1 file changed, 12 insertions(+), 15 deletions(-)

 diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
 b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
 index 72c65df..609b5fd 100644
 --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
 +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
 @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
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);
>>> I think all these extra casts came in to keep icc 12.* compiling without 
>>> warnings.
>>> I am agree that they are unnecessary.
>>> Though if we still have to support icc 12.* we either need to keep them, or 
>>> find
>>> some other way to keep it happy.
>>> Konstantin
>>>
>> What warnings icc 12.* is throwing?
> Try and see :)
>
>> Only warning I can think of here is
>> signed -> unsigned implicit cast.
> If I remember things correctly, it considered result at right side of '=' 
> operator as unsigned int,
> and then complained that we assign it to smaller size (unsigned short) 
> operand.

If that's the case - that's a clear compiler bug.

>
>> Changing '1' to '1U' helps?
> Don't think so, but you are welcome to try.
>
> Konstantin
>
>>
>> --
>> Pawel

[dpdk-dev] [PATCH 1/2] example: fix minor bug in l2fwd-jobstats init sequence

[Pawel Wodkowski]

Fix check of returned values during application init phase.

Signed-off-by: Pawel Wodkowski 
---
 examples/l2fwd-jobstats/main.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index a5a1aaa..ba9d3c0 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -997,11 +997,11 @@ main(int argc, char **argv)
drain_tsc, 0);

rte_timer_init(&qconf->flush_timer);
-   rte_timer_reset(&qconf->flush_timer, drain_tsc, PERIODICAL, 
lcore_id,
-   &l2fwd_flush_job, NULL);
+   ret = rte_timer_reset(&qconf->flush_timer, drain_tsc, 
PERIODICAL,
+   lcore_id, &l2fwd_flush_job, NULL);

if (ret < 0) {
-   rte_exit(1, "Failed to add flush job for lcore %u: %s",
+   rte_exit(1, "Failed to reset flush job timer for lcore 
%u: %s",
lcore_id, rte_strerror(-ret));
}

@@ -1019,8 +1019,13 @@ main(int argc, char **argv)
rte_jobstats_set_update_period_function(job, 
l2fwd_job_update_cb);

rte_timer_init(&qconf->rx_timers[i]);
-   rte_timer_reset(&qconf->rx_timers[i], 0, PERIODICAL, 
lcore_id,
+   ret = rte_timer_reset(&qconf->rx_timers[i], 0, 
PERIODICAL, lcore_id,
&l2fwd_fwd_job, (void *)(uintptr_t)i);
+
+   if (ret < 0) {
+   rte_exit(1, "Failed to reset lcore %u port %u 
job timer: %s",
+   lcore_id, 
qconf->rx_port_list[i], rte_strerror(-ret));
+   }
}
}

-- 
1.9.1

[dpdk-dev] [PATCH v4 2/5] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices

[Vlad Zolotarov]

On 03/09/15 09:58, Thomas Monjalon wrote:
> 2015-03-09 09:08, Vlad Zolotarov:
>> On 03/08/15 23:12, Thomas Monjalon wrote:
>>> Hi Vlad,
>>>
>>> 2015-03-08 16:04, Vlad Zolotarov:
 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 
>>> You are mixing a fix (this patch) and enhancements (LRO) in the same series.
>>> Could you separate them please, as LRO is not going into 2.0 but this fix
>>> is a good candidate.
>> Pls, note that all patches in this series except for PATCH3 and PATCH5
>> are fixing real bugs. I can send them as a separate series if u'd like.
>> Pls., confirm.
> Yes you're right, patch 1 is also a fix and patch 4 seems to solve other
> issues. However, patch 4 makes also some refactoring and seems a bit risky.
> We need an ixgbe maintainer to decide wether we can merge it before the
> release. Or is it possible to have fixes of the patch 4 without the
> refactoring?
>
> Thanks Vlad.
> Sorry to request such split but this PMD is sensible and I don't want to
> have a risk of making it worst in release 2.0.

Well, IMHO PATCH4 here has the most important and critical fix among 
other patches - the whole port is going to be configured according to 
the last queue configuration. Consider the case when queues are 
configured differently and the last one meets vector Rx requirements: 
bulk allocation requirements, descriptors number is a power of 2 (there 
are per-port requirements too). But the first queue may be configured to 
have a different number of descriptors, which is NOT a power of two. 
Currently the port's rx_pkt_burst callback will be configured to use a 
vercot Rx callback and we would hit the same issue reported by Stephen 
and "fixed" in 352078e8e.

[dpdk-dev] [PATCH 1/2] example: fix minor bug in l2fwd-jobstats init sequence

[Pawel Wodkowski]

On 2015-03-09 17:05, Pawel Wodkowski wrote:
> [PATCH 1/2]

This is standalone patch. '1/2' generated by accident.


-- 
Pawel

[dpdk-dev] [PATCH v2 0/3]: bug fixes in the ixgbe PF PMD Rx flow

[Vlad Zolotarov]

This series contains some bug fixes that were found during my work on the ixgbe 
LRO
patches. Sending this series separately on Thomas request so that it may be 
integrated
into the 2.0 release.

New in v2:
   - Fixed a compilation failure.

Vlad Zolotarov (3):
  ixgbe: Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when
reading/setting HW ring descriptor fields
  ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices
  ixgbe: Unify the rx_pkt_bulk callback initialization

 lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h |   2 +
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |  13 ++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 199 +---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |  22 +++-
 4 files changed, 161 insertions(+), 75 deletions(-)

-- 
2.1.0

[dpdk-dev] [PATCH v2 1/3] ixgbe: Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring descriptor fields

[Vlad Zolotarov]

Fixed the above in ixgbe_rx_alloc_bufs() and in ixgbe_recv_scattered_pkts().

Signed-off-by: Vlad Zolotarov 
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 9ecf3e5..b033e04 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1028,7 +1028,7 @@ 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 */
@@ -1051,7 +1051,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;
}
@@ -1559,13 +1559,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. */
-- 
2.1.0

[dpdk-dev] [PATCH v2 2/3] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices

[Vlad Zolotarov]

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 b033e04..ce9658e 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -3677,7 +3677,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 (dev->data->dev_conf.rxmode.hw_strip_crc)
rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP;
-- 
2.1.0

[dpdk-dev] [PATCH v2 3/3] ixgbe: Unify the rx_pkt_bulk callback initialization

[Vlad Zolotarov]

   - Set the callback in a single function that is called from
 ixgbe_dev_rx_init() for a primary process and from eth_ixgbe_dev_init()
 for a secondary processes. This is instead of multiple, hard to track 
places.
   - Added ixgbe_hw.rx_bulk_alloc_allowed - see ixgbe_hw.rx_vec_allowed 
description below.
   - Added ixgbe_hw.rx_vec_allowed: like with Bulk Allocation, Vector Rx is
 enabled or disabled on a per-port level. All queues have to meet the 
appropriate
 preconditions and if any of them doesn't - the feature has to be disabled.
 Therefore ixgbe_hw.rx_vec_allowed will be updated during each queues 
configuration
 (rte_eth_rx_queue_setup()) and then used in rte_eth_dev_start() to 
configure the
 appropriate callbacks. The same happens with ixgbe_hw.rx_vec_allowed in a 
Bulk Allocation
 context.
   - Bugs fixed:
  - Vector scattered packets callback was called regardless the appropriate
preconditions:
 - Vector Rx specific preconditions.
 - Bulk Allocation preconditions.
  - Vector Rx was enabled/disabled according to the last queue setting and 
not
based on all queues setting (which may be different for each queue).

Signed-off-by: Vlad Zolotarov 
---
New in v2:
   - Fixed an broken compilation.
---
 lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h |   2 +
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |  13 ++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 183 +---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |  22 +++-
 4 files changed, 152 insertions(+), 68 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h 
b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
index c67d462..9a66370 100644
--- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
+++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
@@ -3657,6 +3657,8 @@ struct ixgbe_hw {
bool force_full_reset;
bool allow_unsupported_sfp;
bool wol_enabled;
+   bool rx_bulk_alloc_allowed;
+   bool rx_vec_allowed;
 };

 #define ixgbe_call_func(hw, func, params, error) \
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c 
b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 9bdc046..9d3de1a 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -760,8 +760,8 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct 
eth_driver *eth_drv,
   "Using default TX function.");
}

-   if (eth_dev->data->scattered_rx)
-   eth_dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
+   set_rx_function(eth_dev);
+
return 0;
}
pci_dev = eth_dev->pci_dev;
@@ -772,6 +772,13 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct 
eth_driver *eth_drv,
hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
hw->allow_unsupported_sfp = 1;

+   /*
+* Initialize to TRUE. If any of Rx queues doesn't meet the bulk
+* allocation or vector Rx preconditions we will reset it.
+*/
+   hw->rx_bulk_alloc_allowed = true;
+   hw->rx_vec_allowed = true;
+
/* Initialize the shared code (base driver) */
 #ifdef RTE_NIC_BYPASS
diag = ixgbe_bypass_init_shared_code(hw);
@@ -1641,6 +1648,8 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)

/* Clear stored conf */
dev->data->scattered_rx = 0;
+   hw->rx_bulk_alloc_allowed = false;
+   hw->rx_vec_allowed = false;

/* Clear recorded link status */
memset(&link, 0, sizeof(link));
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index ce9658e..a00f5c9 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -2074,12 +2074,12 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused 
struct igb_rx_queue *rxq)

 /* Reset dynamic igb_rx_queue fields back to defaults */
 static void
-ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
+ixgbe_reset_rx_queue(struct ixgbe_hw *hw, struct igb_rx_queue *rxq)
 {
static const union ixgbe_adv_rx_desc zeroed_desc = { .read = {
.pkt_addr = 0}};
unsigned i;
-   uint16_t len;
+   uint16_t len = rxq->nb_rx_desc;

/*
 * By default, the Rx queue setup function allocates enough memory for
@@ -2091,14 +2091,9 @@ ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
 * constraints here to see if we need to zero out memory after the end
 * of the H/W descriptor ring.
 */
-#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
-   if (check_rx_burst_bulk_alloc_preconditions(rxq) == 0)
+   if (hw->rx_bulk_alloc_allowed)
/* zero out extra memory */
-   len = (uint16_t)(rxq->nb_rx_desc + RTE_PMD_IXGBE_RX_MAX_BURST);
-   else
-#endif
-   /* do not zero out extra memory */
-   len = rxq->nb_rx_desc;
+   len += RTE_PMD_IXGBE_RX_MAX_BURST;

/*
 * Zero out HW ring memory. Ze

[dpdk-dev] [PATCH v1 3/3] ixgbe: Unify the rx_pkt_bulk callback initialization

[Vlad Zolotarov]

On 03/09/15 15:39, Mcnamara, John wrote:
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>> Sent: Monday, March 9, 2015 10:13 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v1 3/3] ixgbe: Unify the rx_pkt_bulk callback
>> initialization
>>
>> @@ -3641,23 +3676,17 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>  buf_size = (uint16_t) ((srrctl & IXGBE_SRRCTL_BSIZEPKT_MASK)
>> <<
>> IXGBE_SRRCTL_BSIZEPKT_SHIFT);
>>
>> -if (dev->data->dev_conf.rxmode.enable_scatter ||
>> -/* It adds dual VLAN length for supporting dual VLAN */
>> -(dev->data->dev_conf.rxmode.max_rx_pkt_len +
>> -2 * IXGBE_VLAN_TAG_SIZE) > buf_size){
>> -if (!dev->data->scattered_rx)
>> -PMD_INIT_LOG(DEBUG, "forcing scatter mode");
>> +/* It adds dual VLAN length for supporting dual VLAN */
>> +if (dev->data->dev_conf.rxmode.max_rx_pkt_len +
>> +2 * IXGBE_VLAN_TAG_SIZE > buf_size)
>>  dev->data->scattered_rx = 1;
>> -#ifdef RTE_IXGBE_INC_VECTOR
>> -if (rte_is_power_of_2(rxq->nb_rx_desc))
>> -dev->rx_pkt_burst =
>> -ixgbe_recv_scattered_pkts_vec;
>> -else
>> -#endif
>> -dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
>> -}
>>  }
>>
>> +if (rx_conf->enable_scatter)
>> +dev->data->scattered_rx = 1;
> Hi,
>
> There is a compilation issue here when this patch is applied to the current 
> master (with previous 2 patches applied as well):
>
> make T=x86_64-native-linuxapp-gcc install CC=gcc
> ...
> == Build lib/librte_pmd_ixgbe
>CC ixgbe_rxtx.o
> /home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 
> 'ixgbe_dev_rx_init':
> /home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c:3689:6: error: 'rx_conf' 
> undeclared (first use in this function)
> /home/user/dpdk/lib/librte_pmd_ixgbe/ixgbe_rxtx.c:3689:6: note: each 
> undeclared identifier is reported only once for each function it appears in
> make[5]: *** [ixgbe_rxtx.o] Error 1
>
> This is using the default DPDK linuxapp config.

v2 is out. Again, my apologize. ;)

>
> John
>

[dpdk-dev] [PATCH v1 1/3] ixgbe: Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring descriptor fields

[Ananyev, Konstantin]

> -Original Message-
> From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com]
> Sent: Monday, March 09, 2015 12:43 PM
> To: Ananyev, Konstantin; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 1/3] ixgbe: Use the 
> rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring
> descriptor fields
> 
> 
> 
> On 03/09/15 12:29, Ananyev, Konstantin wrote:
> > Hi Vlad,
> >
> >> -Original Message-
> >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> >> Sent: Monday, March 09, 2015 10:13 AM
> >> To: dev at dpdk.org
> >> Subject: [dpdk-dev] [PATCH v1 1/3] ixgbe: Use the 
> >> rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring
> descriptor
> >> fields
> >>
> >> Fixed the above in ixgbe_rx_alloc_bufs() and in 
> >> ixgbe_recv_scattered_pkts().
> >>
> >> Signed-off-by: Vlad Zolotarov 
> >> ---
> >>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 13 +++--
> >>   1 file changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> >> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> index 9ecf3e5..b033e04 100644
> >> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> @@ -1028,7 +1028,7 @@ 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;
> > Wonder Why you changed from uint64_t to __le64 here?
> > Effectively __le64 is the same a uint64_t,
> 
> I'm afraid the above it's not completely correct. See below.
> 
> > and I think it is better to use always the same type across all PMD code 
> > for consistency.
> 
> Pls., note that "dma_addr" is only used (see below)...
> 
> > Konstantin
> >
> >
> >>int diag, i;
> >>
> >>/* allocate buffers in bulk directly into the S/W ring */
> >> @@ -1051,7 +1051,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;
> 
> here. ;) And the type of both hdr_addr and pkt_addr is __le64.
> I don't exactly understand what do u mean by "use the same type across
> all PMD code for consistency" - there are a lot of types used in the PMD
> code and __le64 is one of them... ;)
> 

> Now more seriously, let's recall what is the semantics of the __leXX
> types - they represent the integer in the "little endian" format. Here,
> NIC expects the physical address in a little endian format, thus the
> descriptor is defined the way it is defined - using __le64. The same
> relates to dma_addr local variable in this patch - it contains the
> physical (more correctly "DMA-able") address of the Rx buffer in the
> form NIC expects it to be written in the descriptor.
> 
> So, why to use __leXX anyway? - Debugging the (invalid) endianess is a
> real headache. Therefore there are a few static code analysis tools like
> "sparse" that allow to detect such inconsistencies (see here
> http://en.wikipedia.org/wiki/Sparse) and __leXX is a helper to allow
> tools like sparse to detect such problems.

I meant that for librte_pmd_ixgbe these types are equivalent:
lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h:
#ifndef __le64
#define __le64  u64
#endif

lib/librte_pmd_ixgbe/ixgbe/ixgbe_osdep.h:
typedef uint64_t   u64;

So why not to use just uint64_t as the rest if librt_pmd_ixgbe/ixgbe_*.[c,h]?

Have to admit, didn't know about the sparse and that ability.
Seems like useful one.
Though, as I understand, to make any use of it with DPDK,
we'll have to use sparse specific attributes:
In one of our files define __le64 as '__attribute__((bitwise)) uint64_t' 
or something similar, right?
Otherwise there is no much point in using all these '__leXX' types,
except probably to show an intention, correct? 
Konstantin

> 
> In addition after spending some time writing patches for Linux netdev
> list u develop a strong habit for such stuff - Dave and others are very
> strict about such things... ;)
> 
> So, is it the same as uint64_t? I guess now it's clear why it is now... ;)
> 
> >>}
> >> @@ -1559,13 +1559,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)
> >> -

[dpdk-dev] [PATCHv2] headers: typeof -> __typeof__ to unbreak C++11 code

[Thomas Monjalon]

2015-03-06 15:59, Konstantin Ananyev:
> v2:
> Instead of changing all the affected files,
> define 'typeofi' and 'asm' if needed.
> 
> When compiling C++11-code or above (--std=c++11), the build fails with
> lots of
> 
>   rte_eth_ctrl.h:517:3: note: in expansion of macro RTE_ALIGN
> (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> ^
> 
> When reading the GCC info pages, I get the feeling that __typeof__ is
> a better choice, and that indeed works when including the headers in
> C++ files (--std=c++11).
> 
> There are some typeof()s left in C files, the patch only touches the
> public API.
> 
> Signed-off-by: Simon Kagstrom 
> Signed-off-by: Konstantin Ananyev 

Applied, thanks

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Ananyev, Konstantin]

> -Original Message-
> From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com]
> Sent: Monday, March 09, 2015 3:58 PM
> To: Ananyev, Konstantin; Wodkowski, PawelX; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
> 
> 
> On 03/09/15 13:29, Ananyev, Konstantin wrote:
> >
> >> -Original Message-
> >> From: Wodkowski, PawelX
> >> Sent: Monday, March 09, 2015 11:09 AM
> >> To: Ananyev, Konstantin; Vlad Zolotarov; dev at dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >>
> >> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
> >>>
>  -Original Message-
>  From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>  Sent: Monday, March 09, 2015 10:21 AM
>  To: dev at dpdk.org
>  Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
>   - Removed the not needed casting.
>   - 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 | 27 ---
> 1 file changed, 12 insertions(+), 15 deletions(-)
> 
>  diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
>  b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>  index 72c65df..609b5fd 100644
>  --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>  +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>  @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>   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);
> >>> I think all these extra casts came in to keep icc 12.* compiling without 
> >>> warnings.
> >>> I am agree that they are unnecessary.
> >>> Though if we still have to support icc 12.* we either need to keep them, 
> >>> or find
> >>> some other way to keep it happy.
> >>> Konstantin
> >>>
> >> What warnings icc 12.* is throwing?
> > Try and see :)
> >
> >> Only warning I can think of here is
> >> signed -> unsigned implicit cast.
> > If I remember things correctly, it considered result at right side of '=' 
> > operator as unsigned int,
> > and then complained that we assign it to smaller size (unsigned short) 
> > operand.
> 
> If that's the case - that's a clear compiler bug.

Might be, though if we still have to support it, there is no much choice I am 
afraid.

> 
> >
> >> Changing '1' to '1U' helps?
> > Don't think so, but you are welcome to try.
> >
> > Konstantin
> >
> >>
> >> --
> >> Pawel

[dpdk-dev] rte_memcpy.h: additional cflags required with OVS

[Kavanagh, Mark B]

Hi,

Compilation of Open vSwitch fails when linked against current HEAD of DPDK 
(f2552cd5).

The source of this issue appears to be commit ID 9144d6b: "eal/x86: optimize 
memcpy for SSE and AVX", and can be resolved by passing an additional argument 
to OVS when building same (CFLAGS="-march=native").
It seems that without this flag, OVS doesn't pick up one or more SSE #defines 
in DPDK, and doesn't include a relevant intrinsic header (emmintrin.h), leading 
to an 'implicit declaration' error for instrinsic '_mm_storeu_si128'.

Has anyone else observed this behavior?

Thanks,
Mark

[dpdk-dev] [PATCH 0/3] dpdk2.0-rc1 build error fix

[Thomas Monjalon]

2015-03-05 21:15, Michael Qiu:
> This patch set is to fix dpdk2.0-rc1 compile error.
> 
> Michael Qiu (3):
>   librte_hash: Fix unsupported instruction `crc32' in i686 platform
>   app/test: Fix size_t printf formart issue
>   librte_eal/common: Fix redeclaration of enumerator ?REG_EAX?

v3 of each patch applied, thanks

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Vladislav Zolotarov]

On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" 
wrote:
>
>
>
> > -Original Message-
> > From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com]
> > Sent: Monday, March 09, 2015 3:58 PM
> > To: Ananyev, Konstantin; Wodkowski, PawelX; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >
> >
> >
> > On 03/09/15 13:29, Ananyev, Konstantin wrote:
> > >
> > >> -Original Message-
> > >> From: Wodkowski, PawelX
> > >> Sent: Monday, March 09, 2015 11:09 AM
> > >> To: Ananyev, Konstantin; Vlad Zolotarov; dev at dpdk.org
> > >> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> > >>
> > >> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
> > >>>
> >  -Original Message-
> >  From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> >  Sent: Monday, March 09, 2015 10:21 AM
> >  To: dev at dpdk.org
> >  Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> > 
> >   - Removed the not needed casting.
> >   - 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 | 27
---
> > 1 file changed, 12 insertions(+), 15 deletions(-)
> > 
> >  diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >  index 72c65df..609b5fd 100644
> >  --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >  +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >  @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
> >   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);
> > >>> I think all these extra casts came in to keep icc 12.* compiling
without warnings.
> > >>> I am agree that they are unnecessary.
> > >>> Though if we still have to support icc 12.* we either need to keep
them, or find
> > >>> some other way to keep it happy.
> > >>> Konstantin
> > >>>
> > >> What warnings icc 12.* is throwing?
> > > Try and see :)
> > >
> > >> Only warning I can think of here is
> > >> signed -> unsigned implicit cast.
> > > If I remember things correctly, it considered result at right side of
'=' operator as unsigned int,
> > > and then complained that we assign it to smaller size (unsigned
short) operand.
> >
> > If that's the case - that's a clear compiler bug.
>
> Might be, though if we still have to support it, there is no much choice
I am afraid.

Well to begin with could anybody who has this icc thing (preferably the
latest version) post the compilation errors u are talking about. Let's
continue this discussion with some specific things in hands. So far there
were a lot of "maybe"s and "as far as i remember"s. Could u, guys, pls., be
more specific so that i could address the real issues and not just your
fears? ;)

Thanks,
Vlad

>
> >
> > >
> > >> Changing '1' to '1U' helps?
> > > Don't think so, but you are welcome to try.
> > >
> > > Konstantin
> > >
> > >>
> > >> --
> > >> Pawel
>
,

[dpdk-dev] rte_memcpy.h: additional cflags required with OVS

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Kavanagh, Mark B
> Sent: Monday, March 9, 2015 4:44 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] rte_memcpy.h: additional cflags required with OVS
> 
> Compilation of Open vSwitch fails when linked against current HEAD of DPDK
> (f2552cd5).
> 
> The source of this issue appears to be commit ID 9144d6b: "eal/x86:
> optimize memcpy for SSE and AVX", and can be resolved by passing an
> additional argument to OVS when building same (CFLAGS="-march=native").
> It seems that without this flag, OVS doesn't pick up one or more SSE
> #defines in DPDK, and doesn't include a relevant intrinsic header
> (emmintrin.h), leading to an 'implicit declaration' error for instrinsic
> '_mm_storeu_si128'.
> 
> Has anyone else observed this behavior?

Hi Mark,

I can confirm that behavior and that it was introduced by the new optimized 
memcpy. I'll look into how it might be mitigated, for Open VSwitch with DPDK.

In the meantime the following might work for OVS:

$ ./configure CFLAGS='-Wno-bad-function-cast -march=native' 
--with-dpdk=$DPDK_BUILD
$ make

But I'll look into it in more detail and update.

John
-- 

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav Zolotarov
> Sent: Monday, March 9, 2015 5:14 PM
> To: Ananyev, Konstantin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
> On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
> 
> wrote:
> > > > If I remember things correctly, it considered result at right side
> > > > of
> '=' operator as unsigned int,
> > > > and then complained that we assign it to smaller size (unsigned
> short) operand.
> > >
> > > If that's the case - that's a clear compiler bug.
> >
> > Might be, though if we still have to support it, there is no much
> > choice
> I am afraid.
> 
> Well to begin with could anybody who has this icc thing (preferably the
> latest version) post the compilation errors u are talking about

Hi,

For what it is worth there aren't any warnings with ICC 13 and the 1/3 patch (+ 
the previous patchset):

  $ make T=x86_64-native-linuxapp-icc install CC=icc
  Build complete

  $ git log --pretty=format:"%h - %an: %s" -4  
  b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
  3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ...
  10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ...
  2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_...

  $ icc --version
  icc (ICC) 13.1.1 20130313

John.
-- 

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Vladislav Zolotarov]

On Mar 9, 2015 8:01 PM, "Mcnamara, John"  wrote:
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav Zolotarov
> > Sent: Monday, March 9, 2015 5:14 PM
> > To: Ananyev, Konstantin
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >
> > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
> > 
> > wrote:
> > > > > If I remember things correctly, it considered result at right side
> > > > > of
> > '=' operator as unsigned int,
> > > > > and then complained that we assign it to smaller size (unsigned
> > short) operand.
> > > >
> > > > If that's the case - that's a clear compiler bug.
> > >
> > > Might be, though if we still have to support it, there is no much
> > > choice
> > I am afraid.
> >
> > Well to begin with could anybody who has this icc thing (preferably the
> > latest version) post the compilation errors u are talking about
>
> Hi,
>
> For what it is worth there aren't any warnings with ICC 13 and the 1/3
patch (+ the previous patchset):
>
>   $ make T=x86_64-native-linuxapp-icc install CC=icc
>   Build complete
>
>   $ git log --pretty=format:"%h - %an: %s" -4
>   b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
>   3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ...
>   10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ...
>   2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_...
>
>   $ icc --version
>   icc (ICC) 13.1.1 20130313

That's worth a lot to me!.. :D

>
> John.
> --
>
>

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Vladislav Zolotarov]

On Mar 9, 2015 8:01 PM, "Mcnamara, John"  wrote:
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav Zolotarov
> > Sent: Monday, March 9, 2015 5:14 PM
> > To: Ananyev, Konstantin
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >
> > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
> > 
> > wrote:
> > > > > If I remember things correctly, it considered result at right side
> > > > > of
> > '=' operator as unsigned int,
> > > > > and then complained that we assign it to smaller size (unsigned
> > short) operand.
> > > >
> > > > If that's the case - that's a clear compiler bug.
> > >
> > > Might be, though if we still have to support it, there is no much
> > > choice
> > I am afraid.
> >
> > Well to begin with could anybody who has this icc thing (preferably the
> > latest version) post the compilation errors u are talking about
>
> Hi,
>
> For what it is worth there aren't any warnings with ICC 13 and the 1/3
patch (+ the previous patchset):
>
>   $ make T=x86_64-native-linuxapp-icc install CC=icc
>   Build complete
>
>   $ git log --pretty=format:"%h - %an: %s" -4
>   b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
>   3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ...
>   10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ...
>   2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_...
>
>   $ icc --version
>   icc (ICC) 13.1.1 20130313

Thanks a lot, John.

>
> John.
> --
>
>

[dpdk-dev] [PATCH v1 1/3] ixgbe: Use the rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring descriptor fields

[Vlad Zolotarov]

On 03/09/15 18:35, Ananyev, Konstantin wrote:
>
>> -Original Message-
>> From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com]
>> Sent: Monday, March 09, 2015 12:43 PM
>> To: Ananyev, Konstantin; dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v1 1/3] ixgbe: Use the 
>> rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring
>> descriptor fields
>>
>>
>>
>> On 03/09/15 12:29, Ananyev, Konstantin wrote:
>>> Hi Vlad,
>>>
 -Original Message-
 From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
 Sent: Monday, March 09, 2015 10:13 AM
 To: dev at dpdk.org
 Subject: [dpdk-dev] [PATCH v1 1/3] ixgbe: Use the 
 rte_le_to_cpu_xx()/rte_cpu_to_le_xx() when reading/setting HW ring
>> descriptor
 fields

 Fixed the above in ixgbe_rx_alloc_bufs() and in 
 ixgbe_recv_scattered_pkts().

 Signed-off-by: Vlad Zolotarov 
 ---
lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)

 diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
 b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
 index 9ecf3e5..b033e04 100644
 --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
 +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
 @@ -1028,7 +1028,7 @@ 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;
>>> Wonder Why you changed from uint64_t to __le64 here?
>>> Effectively __le64 is the same a uint64_t,
>> I'm afraid the above it's not completely correct. See below.
>>
>>> and I think it is better to use always the same type across all PMD code 
>>> for consistency.
>> Pls., note that "dma_addr" is only used (see below)...
>>
>>> Konstantin
>>>
>>>
int diag, i;

/* allocate buffers in bulk directly into the S/W ring */
 @@ -1051,7 +1051,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;
>> here. ;) And the type of both hdr_addr and pkt_addr is __le64.
>> I don't exactly understand what do u mean by "use the same type across
>> all PMD code for consistency" - there are a lot of types used in the PMD
>> code and __le64 is one of them... ;)
>>
>> Now more seriously, let's recall what is the semantics of the __leXX
>> types - they represent the integer in the "little endian" format. Here,
>> NIC expects the physical address in a little endian format, thus the
>> descriptor is defined the way it is defined - using __le64. The same
>> relates to dma_addr local variable in this patch - it contains the
>> physical (more correctly "DMA-able") address of the Rx buffer in the
>> form NIC expects it to be written in the descriptor.
>>
>> So, why to use __leXX anyway? - Debugging the (invalid) endianess is a
>> real headache. Therefore there are a few static code analysis tools like
>> "sparse" that allow to detect such inconsistencies (see here
>> http://en.wikipedia.org/wiki/Sparse) and __leXX is a helper to allow
>> tools like sparse to detect such problems.
> I meant that for librte_pmd_ixgbe these types are equivalent:
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h:
> #ifndef __le64
> #define __le64  u64
> #endif
>
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_osdep.h:
> typedef uint64_t   u64;
>
> So why not to use just uint64_t as the rest if librt_pmd_ixgbe/ixgbe_*.[c,h]?

I'm sorry but it seems to me that I have already mentioned that it 
wasn't the first time __leXX is used in the ixgbe_*.[ch]. All structs 
describing the descriptors of HW rings in ixgbe_type.h use them, so I'm 
just continuing what has already been done.

>
> Have to admit, didn't know about the sparse and that ability.
> Seems like useful one.
> Though, as I understand, to make any use of it with DPDK,
> we'll have to use sparse specific attributes:
> In one of our files define __le64 as '__attribute__((bitwise)) uint64_t'
> or something similar, right?

Right.

> Otherwise there is no much point in using all these '__leXX' types,
> except probably to show an intention, correct?

Not exactly. If u use these types everywhere where it's needed it's only 
6 lines to patch (__le16,32,64 + __be16,32,64) to make sparse work. And 
if u don't - there are thousands of lines to check somehow.

thanks,
vlad
> Konstantin
>
>> In addition after spending some time writing patches for Linux netdev
>> list u develop a strong habit for such stuff - Dave and others are very
>> strict about such things... ;)
>>
>> So, is it the same as uint64_t? I guess now it's clear why it