[dpdk-dev] time to kill rte_pci_dev_ids.h

[Thomas Monjalon]

2016-01-05 11:37, Stephen Hemminger:
> Has anyone looked at getting rid of rte_pci_dev_ids.h?
> The current method with #ifdef's and putting all devices in one file
> really doesn't scale well. Something more like other OS's where
> the data is only in each device driver would be better.

I agree.
The PCI ids should be embedded in drivers.
If I remember well, it's already the case for some of them.
I would love to see a tool able to list the supported devices
by reading a binary libraries.

[dpdk-dev] [PATCH v5 1/3] vhost: Add callback and private data for vhost PMD

[Tetsuya Mukawa]

On 2015/12/29 6:59, Rich Lane wrote:
> On Wed, Dec 23, 2015 at 11:58 PM, Tetsuya Mukawa  wrote:
>> Hi Rich and Yuanhan,
>>
>> I guess we have 2 implementations here.
>>
>> 1. rte_eth_vhost_get_queue_event() returns each event.
>> 2. rte_eth_vhost_get_queue_status() returns current status of the queues.
>>
>> I guess option "2" is more generic manner to handle interrupts from
>> device driver.
>> In the case of option "1", if DPDK application doesn't call
>> rte_eth_vhost_get_queue_event(), the vhost PMD needs to keep all events.
>> This may exhaust memory.
>>
> Option 1 can be implemented in constant space by only tracking the latest
> state of each
> queue. I pushed a rough implementation to https://github.com/rlane/dpdk
> vhost-queue-callback.
>
> One more example is current link status interrupt handling.

Hi Rich,

I appreciate your implementation.
I can understand what's your idea, and agree with it.


Hi Yuanhan,

What do you think his implementation?

Thanks.
Tetsuya

[dpdk-dev] [PATCH v1 2/2] virtio: Extend virtio-net PMD to support container environment

[Tetsuya Mukawa]

On 2015/12/28 20:57, Pavel Fedin wrote:
>  Hello!
>
>> diff --git a/drivers/net/virtio/virtio_pci.h 
>> b/drivers/net/virtio/virtio_pci.h
>> index 47f722a..d4ede73 100644
>> --- a/drivers/net/virtio/virtio_pci.h
>> +++ b/drivers/net/virtio/virtio_pci.h
>> @@ -165,6 +165,9 @@ struct virtqueue;
>>
>>  struct virtio_hw {
>>  struct virtqueue *cvq;
>> +#ifdef RTE_LIBRTE_VIRTIO_HOST_MODE
>> +void*qsession;
>> +#endif
>>  uint32_tio_base;
>>  uint32_tguest_features;
>>  uint32_tmax_tx_queues;
>> @@ -226,6 +229,26 @@ outl_p(unsigned int data, unsigned int port)
>>  }
>>  #endif
>>
>> +#ifdef RTE_LIBRTE_VIRTIO_HOST_MODE
>> +
>> +uint32_t virtio_ioport_read(struct virtio_hw *, uint64_t, char type);
>> +void virtio_ioport_write(struct virtio_hw *, uint64_t, uint64_t, char type);
>> +
>> +#define VIRTIO_READ_REG_1(hw, reg) \
>> +virtio_ioport_read(hw, reg, 'b')
>> +#define VIRTIO_WRITE_REG_1(hw, reg, value) \
>> +virtio_ioport_write(hw, reg, value, 'b')
>> +#define VIRTIO_READ_REG_2(hw, reg) \
>> +virtio_ioport_read(hw, reg, 'w')
>> +#define VIRTIO_WRITE_REG_2(hw, reg, value) \
>> +virtio_ioport_write(hw, reg, value, 'w')
>> +#define VIRTIO_READ_REG_4(hw, reg) \
>> +virtio_ioport_read(hw, reg, 'l')
>> +#define VIRTIO_WRITE_REG_4(hw, reg, value) \
>> +virtio_ioport_write(hw, reg, value, 'l')
>> +
>> +#else /* RTE_LIBRTE_VIRTIO_HOST_MODE */
>> +
>  I have a concern against such compile-time switches. What if we want the 
> same code to work for both 'real' virtio and socket-based?
> Shouldn't we introduce some function pointers here to be able to switch them 
> at runtime?

Hi Pavel,

Thanks for commenting.
In that case, you will run QEMU, then create containers in the guest.
Do you have an use case for this usage?

Anyway, such a feature depends on how to allocate share memory.
So far, this patch allow you to run both virtio-net 'real' and 'virtual'
PMDs on guest, but it will be changed to remove contiguous memory
restriction.
Could you please see an other thread that we talk about the restriction
in? (I will add you to CC.)

Thanks,
Tetsuya

[dpdk-dev] [PATCH v1 0/2] Virtio-net PMD Extension to work on host

[Tetsuya Mukawa]

On 2015/12/28 20:06, Tetsuya Mukawa wrote:
> On 2015/12/24 23:05, Tan, Jianfeng wrote:
>> Hi Tetsuya,
>>
>> After several days' studying your patch, I have some questions as follows:
>>
>> 1. Is physically-contig memory really necessary?
>> This is a too strong requirement IMHO. IVSHMEM doesn't require this in its 
>> original meaning. So how do you think of
>> Huawei Xie's idea of using virtual address for address translation? (In 
>> addition, virtual address of mem_table could be
>> different in application and QTest, but this can be addressed because 
>> SET_MEM_TABLE msg will be intercepted by
>> QTest)
> Hi Jianfeng,
>
> Thanks for your suggestion.
> Huawei's idea may solve contig-mem restriction.
> Let me have time to check it more.

Hi Jianfeng,

I made sure we can remove the restriction with Huawei's idea.
One thing I concern is below.
If we don't use contiguous memory, this PMD will not work with other
'physical' PMDs like e1000 PMD, virtio-net PMD, and etc.
(This is because allocated memory may not  be physically contiguous.)

One of examples is that if we implement like above, in QEMU guest, we
can handle a host NIC directly, but in container, we will not be able to
handle the device.
This will be a restriction for this virtual addressing changing.

Do you know an use case that the user wants to handle 'physical' PMD and
'virtual' virtio-net PMD together?

Tetsuya,

[dpdk-dev] [PATCH v1 0/2] Virtio-net PMD Extension to work on host

[Tan, Jianfeng]

On 1/6/2016 11:57 AM, Tetsuya Mukawa wrote:
> On 2015/12/28 20:06, Tetsuya Mukawa wrote:
>> On 2015/12/24 23:05, Tan, Jianfeng wrote:
>>> Hi Tetsuya,
>>>
>>> After several days' studying your patch, I have some questions as follows:
>>>
>>> 1. Is physically-contig memory really necessary?
>>> This is a too strong requirement IMHO. IVSHMEM doesn't require this in its 
>>> original meaning. So how do you think of
>>> Huawei Xie's idea of using virtual address for address translation? (In 
>>> addition, virtual address of mem_table could be
>>> different in application and QTest, but this can be addressed because 
>>> SET_MEM_TABLE msg will be intercepted by
>>> QTest)
>> Hi Jianfeng,
>>
>> Thanks for your suggestion.
>> Huawei's idea may solve contig-mem restriction.
>> Let me have time to check it more.
> Hi Jianfeng,
>
> I made sure we can remove the restriction with Huawei's idea.
> One thing I concern is below.
> If we don't use contiguous memory, this PMD will not work with other
> 'physical' PMDs like e1000 PMD, virtio-net PMD, and etc.
> (This is because allocated memory may not  be physically contiguous.)
>
> One of examples is that if we implement like above, in QEMU guest, we
> can handle a host NIC directly, but in container, we will not be able to
> handle the device.
> This will be a restriction for this virtual addressing changing.
>
> Do you know an use case that the user wants to handle 'physical' PMD and
> 'virtual' virtio-net PMD together?
>
> Tetsuya,
Hi Tetsuya,

I have no use case in hand, which handles 'physical' PMDs and 'virtual' 
virtio-net PMD together.
(Pavel Fedin once tried to run ovs in container, but that case just uses 
virtual virtio devices, I
don't know if he has plan to add 'physical' PMDs as well.)

Actually, it's not completely contradictory to make them work together. 
Like this:
a. containers with root privilege
We can initialize memory as legacy way. (TODO: besides 
physical-contiguous, we try allocate
virtual-contiguous big area for all memsegs as well.)
a.1 For vhost-net, before sending memory regions into kernel, we can 
merge those virtual-contiguous regions into one region.
a.2 For vhost-user, we can merge memory regions in the vhost. The 
blocker is that for now, maximum fd num was restricted
by VHOST_MEMORY_MAX_NREGIONS=8 (so in 2M-hugepage's case, 16M shared 
memory is not nearly enough).

b. containers without root privilege
No need to worry about this problem, because it lacks of privilege to 
construct physical-contiguous memory.

Thanks,
Jianfeng

[dpdk-dev] [RFC 0/5] virtio support for container

[Tan, Jianfeng]

On 12/31/2015 11:39 PM, Pavel Fedin wrote:
>   Hello!
>
>   Last minute note. I have found the problem but have no time to research and 
> fix it.
>   It happens because ovs first creates the device, starts it, then stops it, 
> and reconfigures queues. The second queue allocation
> happens from within netdev_set_multiq(). Then ovs restarts the device and 
> proceeds to actually using it.
>   But, queues are not initialized properly in DPDK after the second 
> allocation. Because of this thing:
>
>   /* On restart after stop do not touch queues */
>   if (hw->started)
>   return 0;
Hi Fedin,

As you see, I also think it is a bug. A device should be ok to 
start/stop/start...

I already send a patch to fix this.
http://dpdk.org/ml/archives/dev/2016-January/031010.html

Thanks,
Jianfeng
>
>   It keeps us away from calling virtio_dev_rxtx_start(), which should in turn 
> call virtio_dev_vring_start(), which calls
> vring_init(). So, VIRTQUEUE_NUSED() dies badly because vq->vq_ring all 
> contains NULLs.
>   See you all after 10th. And happy New Year again!
>
>

[dpdk-dev] [PATCH v1 2/2] virtio: Extend virtio-net PMD to support container environment

[Tan, Jianfeng]

On 1/6/2016 11:57 AM, Tetsuya Mukawa wrote:
> On 2015/12/28 20:57, Pavel Fedin wrote:
>>   Hello!
>>
>>> diff --git a/drivers/net/virtio/virtio_pci.h 
>>> b/drivers/net/virtio/virtio_pci.h
>>> index 47f722a..d4ede73 100644
>>> --- a/drivers/net/virtio/virtio_pci.h
>>> +++ b/drivers/net/virtio/virtio_pci.h
>>> @@ -165,6 +165,9 @@ struct virtqueue;
>>>
>>>   struct virtio_hw {
>>> struct virtqueue *cvq;
>>> +#ifdef RTE_LIBRTE_VIRTIO_HOST_MODE
>>> +   void*qsession;
>>> +#endif
>>> uint32_tio_base;
>>> uint32_tguest_features;
>>> uint32_tmax_tx_queues;
>>> @@ -226,6 +229,26 @@ outl_p(unsigned int data, unsigned int port)
>>>   }
>>>   #endif
>>>
>>> +#ifdef RTE_LIBRTE_VIRTIO_HOST_MODE
>>> +
>>> +uint32_t virtio_ioport_read(struct virtio_hw *, uint64_t, char type);
>>> +void virtio_ioport_write(struct virtio_hw *, uint64_t, uint64_t, char 
>>> type);
>>> +
>>> +#define VIRTIO_READ_REG_1(hw, reg) \
>>> +   virtio_ioport_read(hw, reg, 'b')
>>> +#define VIRTIO_WRITE_REG_1(hw, reg, value) \
>>> +   virtio_ioport_write(hw, reg, value, 'b')
>>> +#define VIRTIO_READ_REG_2(hw, reg) \
>>> +   virtio_ioport_read(hw, reg, 'w')
>>> +#define VIRTIO_WRITE_REG_2(hw, reg, value) \
>>> +   virtio_ioport_write(hw, reg, value, 'w')
>>> +#define VIRTIO_READ_REG_4(hw, reg) \
>>> +   virtio_ioport_read(hw, reg, 'l')
>>> +#define VIRTIO_WRITE_REG_4(hw, reg, value) \
>>> +   virtio_ioport_write(hw, reg, value, 'l')
>>> +
>>> +#else /* RTE_LIBRTE_VIRTIO_HOST_MODE */
>>> +
>>   I have a concern against such compile-time switches. What if we want the 
>> same code to work for both 'real' virtio and socket-based?
>> Shouldn't we introduce some function pointers here to be able to switch them 
>> at runtime?
> Hi Pavel,
>
> Thanks for commenting.
> In that case, you will run QEMU, then create containers in the guest.
> Do you have an use case for this usage?
>
> Anyway, such a feature depends on how to allocate share memory.
> So far, this patch allow you to run both virtio-net 'real' and 'virtual'
> PMDs on guest, but it will be changed to remove contiguous memory
> restriction.
> Could you please see an other thread that we talk about the restriction
> in? (I will add you to CC.)
>
> Thanks,
> Tetsuya
Hi Tetsuya,

I prefer to a compiled library to work well in both VM and container.

For this issue, we can address this issue using Yuanhan's way to address 
virtio 1.0 support.
(He introduces struct virtio_pci_ops)

Thanks,
Jianfeng

[dpdk-dev] [PATCH 02/12] pmd/cxgbe: add dev_ptype_info_get implementation

[Rahul Lakkireddy]

Hi Jianfeng,

On Thursday, December 12/31/15, 2015 at 14:53:09 +0800, Jianfeng Tan wrote:
> Signed-off-by: Jianfeng Tan 
> ---
>  drivers/net/cxgbe/cxgbe_ethdev.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c 
> b/drivers/net/cxgbe/cxgbe_ethdev.c
> index 97ef152..f17d5d5 100644
> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -767,6 +767,22 @@ static int cxgbe_flow_ctrl_set(struct rte_eth_dev 
> *eth_dev,
>&pi->link_cfg);
>  }
>  
> +static int cxgbe_dev_ptype_info_get(struct rte_eth_dev *eth_dev __rte_unused,
> + uint32_t ptype_mask, uint32_t ptypes[])
> +{

No need for the __rte_unused for eth_dev above since it's being used for
if conditional below.

> + int num = 0;
> +
> + if (eth_dev->rx_pkt_burst == cxgbe_recv_pkts) {
> + if ((ptype_mask & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_MASK) {
> + ptypes[num++] = RTE_PTYPE_L3_IPV4;
> + ptypes[num++] = RTE_PTYPE_L3_IPV6;
> + }
> + } else
> + num = -ENOTSUP;
> +
> + return num;
> +}
> +
>  static struct eth_dev_ops cxgbe_eth_dev_ops = {
>   .dev_start  = cxgbe_dev_start,
>   .dev_stop   = cxgbe_dev_stop,
> @@ -777,6 +793,7 @@ static struct eth_dev_ops cxgbe_eth_dev_ops = {
>   .allmulticast_disable   = cxgbe_dev_allmulticast_disable,
>   .dev_configure  = cxgbe_dev_configure,
>   .dev_infos_get  = cxgbe_dev_info_get,
> + .dev_ptype_info_get = cxgbe_dev_ptype_info_get,
>   .link_update= cxgbe_dev_link_update,
>   .mtu_set= cxgbe_dev_mtu_set,
>   .tx_queue_setup = cxgbe_dev_tx_queue_setup,
> -- 
> 2.1.4
> 

Thanks,
Rahul

[dpdk-dev] [PATCH v1 2/2] virtio: Extend virtio-net PMD to support container environment

[Tetsuya Mukawa]

On 2016/01/06 14:56, Tan, Jianfeng wrote:
>
>
> On 1/6/2016 11:57 AM, Tetsuya Mukawa wrote:
>> On 2015/12/28 20:57, Pavel Fedin wrote:
>>>   Hello!
>>>
 diff --git a/drivers/net/virtio/virtio_pci.h
 b/drivers/net/virtio/virtio_pci.h
 index 47f722a..d4ede73 100644
 --- a/drivers/net/virtio/virtio_pci.h
 +++ b/drivers/net/virtio/virtio_pci.h
 @@ -165,6 +165,9 @@ struct virtqueue;

   struct virtio_hw {
   struct virtqueue *cvq;
 +#ifdef RTE_LIBRTE_VIRTIO_HOST_MODE
 +void*qsession;
 +#endif
   uint32_tio_base;
   uint32_tguest_features;
   uint32_tmax_tx_queues;
 @@ -226,6 +229,26 @@ outl_p(unsigned int data, unsigned int port)
   }
   #endif

 +#ifdef RTE_LIBRTE_VIRTIO_HOST_MODE
 +
 +uint32_t virtio_ioport_read(struct virtio_hw *, uint64_t, char type);
 +void virtio_ioport_write(struct virtio_hw *, uint64_t, uint64_t,
 char type);
 +
 +#define VIRTIO_READ_REG_1(hw, reg) \
 +virtio_ioport_read(hw, reg, 'b')
 +#define VIRTIO_WRITE_REG_1(hw, reg, value) \
 +virtio_ioport_write(hw, reg, value, 'b')
 +#define VIRTIO_READ_REG_2(hw, reg) \
 +virtio_ioport_read(hw, reg, 'w')
 +#define VIRTIO_WRITE_REG_2(hw, reg, value) \
 +virtio_ioport_write(hw, reg, value, 'w')
 +#define VIRTIO_READ_REG_4(hw, reg) \
 +virtio_ioport_read(hw, reg, 'l')
 +#define VIRTIO_WRITE_REG_4(hw, reg, value) \
 +virtio_ioport_write(hw, reg, value, 'l')
 +
 +#else /* RTE_LIBRTE_VIRTIO_HOST_MODE */
 +
>>>   I have a concern against such compile-time switches. What if we
>>> want the same code to work for both 'real' virtio and socket-based?
>>> Shouldn't we introduce some function pointers here to be able to
>>> switch them at runtime?
>> Hi Pavel,
>>
>> Thanks for commenting.
>> In that case, you will run QEMU, then create containers in the guest.
>> Do you have an use case for this usage?
>>
>> Anyway, such a feature depends on how to allocate share memory.
>> So far, this patch allow you to run both virtio-net 'real' and 'virtual'
>> PMDs on guest, but it will be changed to remove contiguous memory
>> restriction.
>> Could you please see an other thread that we talk about the restriction
>> in? (I will add you to CC.)
>>
>> Thanks,
>> Tetsuya
> Hi Tetsuya,
>
> I prefer to a compiled library to work well in both VM and container.
>
> For this issue, we can address this issue using Yuanhan's way to
> address virtio 1.0 support.
> (He introduces struct virtio_pci_ops)
>
> Thanks,
> Jianfeng
>
>
>

Sounds great!
I will check it.

Tetsuya

[dpdk-dev] [PATCH v1 0/2] Virtio-net PMD Extension to work on host

[Tetsuya Mukawa]

On 2016/01/06 14:42, Tan, Jianfeng wrote:
>
>
> On 1/6/2016 11:57 AM, Tetsuya Mukawa wrote:
>> On 2015/12/28 20:06, Tetsuya Mukawa wrote:
>>> On 2015/12/24 23:05, Tan, Jianfeng wrote:
 Hi Tetsuya,

 After several days' studying your patch, I have some questions as
 follows:

 1. Is physically-contig memory really necessary?
 This is a too strong requirement IMHO. IVSHMEM doesn't require this
 in its original meaning. So how do you think of
 Huawei Xie's idea of using virtual address for address translation?
 (In addition, virtual address of mem_table could be
 different in application and QTest, but this can be addressed
 because SET_MEM_TABLE msg will be intercepted by
 QTest)
>>> Hi Jianfeng,
>>>
>>> Thanks for your suggestion.
>>> Huawei's idea may solve contig-mem restriction.
>>> Let me have time to check it more.
>> Hi Jianfeng,
>>
>> I made sure we can remove the restriction with Huawei's idea.
>> One thing I concern is below.
>> If we don't use contiguous memory, this PMD will not work with other
>> 'physical' PMDs like e1000 PMD, virtio-net PMD, and etc.
>> (This is because allocated memory may not  be physically contiguous.)
>>
>> One of examples is that if we implement like above, in QEMU guest, we
>> can handle a host NIC directly, but in container, we will not be able to
>> handle the device.
>> This will be a restriction for this virtual addressing changing.
>>
>> Do you know an use case that the user wants to handle 'physical' PMD and
>> 'virtual' virtio-net PMD together?
>>
>> Tetsuya,
> Hi Tetsuya,
>
> I have no use case in hand, which handles 'physical' PMDs and
> 'virtual' virtio-net PMD together.
> (Pavel Fedin once tried to run ovs in container, but that case just
> uses virtual virtio devices, I
> don't know if he has plan to add 'physical' PMDs as well.)
>
> Actually, it's not completely contradictory to make them work
> together. Like this:
> a. containers with root privilege
> We can initialize memory as legacy way. (TODO: besides
> physical-contiguous, we try allocate
> virtual-contiguous big area for all memsegs as well.)

Hi Jianfeng,

Yes, I agree with you.
If the feature is really needed, we will be able to have work around.

>
> a.1 For vhost-net, before sending memory regions into kernel, we can
> merge those virtual-contiguous regions into one region.
> a.2 For vhost-user, we can merge memory regions in the vhost. The
> blocker is that for now, maximum fd num was restricted
> by VHOST_MEMORY_MAX_NREGIONS=8 (so in 2M-hugepage's case, 16M shared
> memory is not nearly enough).
>

With current your implementation, when 'virtual' virtio-net PMD is used,
'phys_addr' will be virtual address in EAL layer.

struct rte_memseg {
phys_addr_t phys_addr;  /**< Start physical address. */
union {
void *addr; /**< Start virtual address. */
uint64_t addr_64;   /**< Makes sure addr is always 64
bits */
};
...
};

How about choosing it in virtio-net PMD?
(In the case of 'virtual', just use 'addr' instead of using 'phys_addr'.)
For example, port0 may use physical address, but port1 may use virtual
address.

With this, of course, we don't have an issue with 'physical' virtio-net PMD.
Also, with 'virtual' virtio-net PMD, we can use virtual address and fd
that represents the big virtual address space.
(TODO: Need to change rte_memseg and EAL to keep fd and offset?)
Then, you don't worry about VHOST_MEMORY_MAX_NREGIONS, because we have
only one fd.

> b. containers without root privilege
> No need to worry about this problem, because it lacks of privilege to
> construct physical-contiguous memory.
>

Yes, we cannot run 'physical' PMDs in this type of container.
Anyway, I will check it more, if we really need it.

Thanks,
Tetsuya

[dpdk-dev] [PATCH v5 1/3] vhost: Add callback and private data for vhost PMD

[Yuanhan Liu]

On Wed, Jan 06, 2016 at 12:56:58PM +0900, Tetsuya Mukawa wrote:
> On 2015/12/29 6:59, Rich Lane wrote:
> > On Wed, Dec 23, 2015 at 11:58 PM, Tetsuya Mukawa  
> > wrote:
> >> Hi Rich and Yuanhan,
> >>
> >> I guess we have 2 implementations here.
> >>
> >> 1. rte_eth_vhost_get_queue_event() returns each event.
> >> 2. rte_eth_vhost_get_queue_status() returns current status of the queues.
> >>
> >> I guess option "2" is more generic manner to handle interrupts from
> >> device driver.
> >> In the case of option "1", if DPDK application doesn't call
> >> rte_eth_vhost_get_queue_event(), the vhost PMD needs to keep all events.
> >> This may exhaust memory.
> >>
> > Option 1 can be implemented in constant space by only tracking the latest
> > state of each
> > queue. I pushed a rough implementation to https://github.com/rlane/dpdk
> > vhost-queue-callback.
> >
> > One more example is current link status interrupt handling.
> 
> Hi Rich,
> 
> I appreciate your implementation.
> I can understand what's your idea, and agree with it.
> 
> 
> Hi Yuanhan,
> 
> What do you think his implementation?

With a quick glimpse, it looks good to me.

--yliu

[dpdk-dev] [PATCH 02/12] pmd/cxgbe: add dev_ptype_info_get implementation

[Tan, Jianfeng]

On 1/6/2016 3:11 PM, Rahul Lakkireddy wrote:
> Hi Jianfeng,
>
> On Thursday, December 12/31/15, 2015 at 14:53:09 +0800, Jianfeng Tan wrote:
>> Signed-off-by: Jianfeng Tan 
>> ---
>>   drivers/net/cxgbe/cxgbe_ethdev.c | 17 +
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c 
>> b/drivers/net/cxgbe/cxgbe_ethdev.c
>> index 97ef152..f17d5d5 100644
>> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
>> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
>> @@ -767,6 +767,22 @@ static int cxgbe_flow_ctrl_set(struct rte_eth_dev 
>> *eth_dev,
>>   &pi->link_cfg);
>>   }
>>   
>> +static int cxgbe_dev_ptype_info_get(struct rte_eth_dev *eth_dev 
>> __rte_unused,
>> +uint32_t ptype_mask, uint32_t ptypes[])
>> +{
> No need for the __rte_unused for eth_dev above since it's being used for
> if conditional below.
Hi Rahul,

Thanks! Will fix it in next version.

Jianfeng

[dpdk-dev] [PATCH 1/2] mlx4: add callback to set primary mac address

[Adrien Mazarguil]

On Tue, Jan 05, 2016 at 07:00:08PM +0100, David Marchand wrote:
> Signed-off-by: David Marchand 
> ---
>  drivers/net/mlx4/mlx4.c |   17 +
>  1 file changed, 17 insertions(+)

Acked-by: Adrien Mazarguil 

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] [PATCH 2/2] mlx5: add callback to set primary mac address

[Adrien Mazarguil]

On Tue, Jan 05, 2016 at 07:00:09PM +0100, David Marchand wrote:
> Signed-off-by: David Marchand 
> ---
>  drivers/net/mlx5/mlx5.c |1 +
>  drivers/net/mlx5/mlx5.h |1 +
>  drivers/net/mlx5/mlx5_mac.c |   16 
>  3 files changed, 18 insertions(+)

Acked-by: Adrien Mazarguil 

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if packet type is set

[Adrien Mazarguil]

On Tue, Jan 05, 2016 at 04:50:31PM +, Ananyev, Konstantin wrote:
[...]
> > -Original Message-
> > From: N?lio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
[...]
> > I think we miss a comment here in how those 2/6/4 values are chosen
> > because, according to the mask, I expect 16 possibilities but I get
> > less.  It will help a lot anyone who needs to add a new type.
> > 
> > Extending the snprintf behavior above, it is best to remove the mask
> > argument altogether and have rte_eth_dev_get_ptype_info() return the
> > entire list every time.  Applications need to iterate on the result in
> > any case.
> 
> I think we'd better keep mask argument.
> In many cases upper layer only interested in some particular  subset of
> all packet types that HW can recognise.
> Let say l3fwd only cares about  RTE_PTYPE_L3_MASK, it is not interested in L4,
> tunnelling packet types, etc.
> If caller needs to know all recognised ptypes, he can set mask==-1,
> In that case all supported packet types will be returned.

There are other drawbacks to the mask argument in my opinion. The API will
have to be updated again as soon as 32 bits aren't enough to represent all
possible masks. We can't predict it will be large enough forever but on the
other hand, using uint64_t seems overkill at this point.

I think this use for masks should be avoided when performance does not
matter much, as in this case, user application cannot know the number of
entries in advance and must rely on the returned value to iterate.

A helper function can be added to convert a RTE_PTYPE_* value to the layer
it belongs to (using enum to define possible values).

If we absolutely want a mean to filter returned values, I suggest we use
this enum instead of the mask argument. Since it won't be a mask, it won't
have to be updated every time a new protocol requires extending one.

> >   rte_eth_dev_get_ptype_info(uint8_t port_id, uint32_t ptypes[],
> >  size_t max_entries)
> > 
> > 
> > 
> > Another point, I have read the example patch (l3fwd) but I don't
> > understand why the PMD is not responsible for filling the packet type in
> > the MBUF (packet parsing is done by parse_packet_type()).  Why the extra
> > computation?
> 
> As I understand there are 3 possibilities now:
> 1. HW supports ptype recognition and SW ptype parsing is never done
> (--parse-ptype is not specified).
> 2. HW supports ptype recognition, but and SW ptype parsing is done anyway
> (--parse-ptype is specified).
> 3. HW doesn't support and ptype recognition, and and SW ptype parsing is done
> (--parse-ptype is specified).
> 
> I suppose the question is what for introduce '--parse-ptype' at all?
> My thought because of #2, so people can easily check what will be the 
> performance impact of SW parsing. 
> 
> Konstantin
> 
> > 
> > I see it more like an offload request (as checksum, etc...) and if the
> > NIC does not support it then the application does the necessary overload.
> > 
> > Best regards,
> > 
> > --
> > N?lio Laranjeiro
> > 6WIND

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] Is there any example application to used DPDK packet distributor library?

[Pattan, Reshma]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of ???
> Sent: Wednesday, September 30, 2015 6:45 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] Is there any example application to used DPDK packet
> distributor library?
> 
> Dear DPDK experts.
> 
> I am Ick-Sung Choi living in South Korea.
> 
> I have a question about DPDK? packet distributor library.
> 
> Is there any example application to used DPDK packet distributor library?
> 
> I am trying to experiment simple function using DPDK packet distributor 
> library.
> 

Hi,

examples/distributor/main.c is the example which explains distributor library 
usage.
Also, you can refer unit tests app/test/test_distributor.c and  
app/test/test_distributor.c/test_distributor_perf.c

Thanks,
Reshma

[dpdk-dev] Is there any example application to used DPDK packet distributor library?

[최익성]

 Dear Reshma.
Thank you very much for your answer.
I found the related codes.
I have done some coding related them  and I finished my experiments.
Thank you very much. 
Sincerely Yours,
Ick-Sung Choi.

-Original Message-
From: "Pattan, Reshma" 
To: "???"; "dev at dpdk.org"; 
Cc: 
Sent: 2016-01-06 (?) 19:37:35
Subject: RE: [dpdk-dev] Is there any example application to used DPDK packet 
distributor library?





> -Original Message-

> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of ???

> Sent: Wednesday, September 30, 2015 6:45 AM

> To: dev at dpdk.org

> Subject: [dpdk-dev] Is there any example application to used DPDK packet

> distributor library?

> 

> Dear DPDK experts.

> 

> I am Ick-Sung Choi living in South Korea.

> 

> I have a question about DPDK? packet distributor library.

> 

> Is there any example application to used DPDK packet distributor library?

> 

> I am trying to experiment simple function using DPDK packet distributor 
library.

> 



Hi,



examples/distributor/main.c is the example which explains distributor library 
usage.

Also, you can refer unit tests app/test/test_distributor.c and  
app/test/test_distributor.c/test_distributor_perf.c



Thanks,

Reshma

[dpdk-dev] [PATCH] Patch introducing API to read/write Intel Architecture Model Specific Registers (MSR), rte_msr_read and rte_msr_write functions.

[Andralojc, WojciechX]

> From: Andralojc, WojciechX
> Sent: Thursday, December 17, 2015 12:13 PM
> To: dev at dpdk.org
> Cc: Andralojc, WojciechX
> Subject: [PATCH] Patch introducing API to read/write Intel Architecture Model
> Specific Registers (MSR), rte_msr_read and rte_msr_write functions.
> 
> There is work in progress to implement Intel Cache Allocation Technology (CAT)
> support in DPDK, this technology is programmed through MSRs.
> In the future it will be possible to program CAT through Linux cgroups and 
> DPDK
> CAT implementation will take advantage of it.
> 
> MSR R/W's are privileged ring 0 operations and they must be done in kernel
> space. For this reason implementation utilizes Linux MSR driver.
> 
> Signed-off-by: Wojciech Andralojc 

I've got suggestion offline that as MSRs are IA specific,
I should not give the dummy APIs for the other arches
and move MSR access functions into the EAL specific APIs
or some place more arch specific. 
Do you find submitted MSR patch OK?
or do you agree with the above feedback and patch should be re-worked?
I am looking forward to your feedback

Thank you!

Wojciech Andralojc
--
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[dpdk-dev] [PATCH v2 0/5] virtio: Tx performance improvements

[Thomas Monjalon]

2016-01-05 08:10, Xie, Huawei:
> On 10/26/2015 10:06 PM, Xie, Huawei wrote:
> > On 10/19/2015 1:16 PM, Stephen Hemminger wrote:
> >> This is a tested version of the virtio Tx performance improvements
> >> that I posted earlier on the list, and described at the DPDK Userspace
> >> meeting in Dublin. Together they get a 25% performance improvement for
> >> both small packet and large multi-segment packet case when testing
> >> from DPDK guest application to Linux KVM host.
> >>
> >> Stephen Hemminger (5):
> >>   virtio: clean up space checks on xmit
> >>   virtio: don't use unlikely for normal tx stuff
> >>   virtio: use indirect ring elements
> >>   virtio: use any layout on transmit
> >>   virtio: optimize transmit enqueue
> > There is one open why merge-able header is used in tx path. Since old
> > implementation is also using the merge-able header in tx path if this
> > feature is negotiated, i choose to ack the patch and address this later
> > if not now.
> >
> > Acked-by: Huawei Xie 
> 
> Thomas:
> This patch isn't in the patchwork. Does Stephen need to send a new one?

Yes please, I cannot find them in patchwork.

[dpdk-dev] Traffic scheduling in DPDK

[ravulakollu.ku...@wipro.com]

Thanks Jasvinder,

Does this application works on systems with multiple NUMA Nodes ?

Thanks,
Uday

-Original Message-
From: Singh, Jasvinder [mailto:jasvinder.si...@intel.com]
Sent: Tuesday, January 05, 2016 3:40 PM
To: Ravulakollu Udaya Kumar (WT01 - Product Engineering Service)
Cc: dev at dpdk.org
Subject: RE: [dpdk-dev] Traffic scheduling in DPDK

Hi Uday,

>
> Thanks Jasvinder , I am running the below command
>
> ./build/qos_sched -c 0xe -n 1  -- --pfc "0,1,3,2" --cfg ./profile.cfg
>
> Bound two 1G physical ports to DPDK , and started running the above
> command with the default profile mentioned in profile.cfg .
> I am using lcore 3 and 2 for RX and TX. It was not successful, getting
> the below error.
>
> APP: Initializing port 0... PMD: eth_igb_rx_queue_setup():
> sw_ring=0x7f5b20ba2240 hw_ring=0x7f5b20ba2680 dma_addr=0xbf87a2680
> PMD: eth_igb_tx_queue_setup(): To improve 1G driver performance,
> consider setting the TX WTHRESH value to 4, 8, or 16.
> PMD: eth_igb_tx_queue_setup(): sw_ring=0x7f5b20b910c0
> hw_ring=0x7f5b20b92100 dma_addr=0xbf8792100
> PMD: eth_igb_start(): <<
> done:  Link Up - speed 1000 Mbps - full-duplex
> APP: Initializing port 1... PMD: eth_igb_rx_queue_setup():
> sw_ring=0x7f5b20b80a40 hw_ring=0x7f5b20b80e80 dma_addr=0xbf8780e80
> PMD: eth_igb_tx_queue_setup(): To improve 1G driver performance,
> consider setting the TX WTHRESH value to 4, 8, or 16.
> PMD: eth_igb_tx_queue_setup(): sw_ring=0x7f5b20b6f8c0
> hw_ring=0x7f5b20b70900 dma_addr=0xbf8770900
> PMD: eth_igb_start(): <<
> done:  Link Up - speed 1000 Mbps - full-duplex
> SCHED: Low level config for pipe profile 0:
> Token bucket: period = 3277, credits per period = 8, size = 100
> Traffic classes: period = 500, credits per period = [12207,
> 12207, 12207, 12207]
> Traffic class 3 oversubscription: weight = 0
> WRR cost: [1, 1, 1, 1], [1, 1, 1, 1], [1, 1, 1, 1], [1, 1, 1, 1]
> EAL: Error - exiting with code: 1
>   Cause: Unable to config sched subport 0, err=-2


In default profile.cfg, It is assumed that all the nic ports have 10 Gbps rate. 
The above error occurs when subport's  tb_rate (10Gbps) is found more than NIC 
port's capacity (1 Gbps). Therefore, you need to use either 10 Gbps ports in 
your application or have to amend the profile.cfg to work with 1 Gbps port.  
Please refer to  DPDK QoS framework document for more details on various 
parameters - http://dpdk.org/doc/guides/prog_guide/qos_framework.html


> -Original Message-
> From: Singh, Jasvinder [mailto:jasvinder.singh at intel.com]
> Sent: Monday, January 04, 2016 9:26 PM
> To: Ravulakollu Udaya Kumar (WT01 - Product Engineering Service);
> dev at dpdk.org
> Subject: RE: [dpdk-dev] Traffic scheduling in DPDK
>
> Hi Uday,
>
>
> > I have an issue in running qos_sched application in DPDK .Could
> > someone tell me how to run the command  and what each parameter does
> > In the below mentioned text.
> >
> > Application mandatory parameters:
> > --pfc "RX PORT, TX PORT, RX LCORE, WT LCORE" : Packet flow
> configuration
> >multiple pfc can be configured in command line
>
>
> RX PORT - Specifies the packets receive port TX PORT - Specifies the
> packets transmit port RXCORE - Specifies the  Core used for Packet
> reception and Classification stage of the QoS application.
> WTCORE-  Specifies the  Core used for Packet enqueue/dequeue operation
> (QoS scheduling)  and subsequently transmitting the packets out.
>
> Multiple pfc  can be specified depending upon the number of instances
> of qos sched required in application.  For example- in order to run
> two instance, following can be used-
>
> ./build/qos_sched -c 0x7e -n 4 -- --pfc "0,1,2,3,4" --pfc "2,3,5,6"
> --cfg "profile.cfg"
>
> First instance of qos sched receives packets from port 0 and transmits
> its packets through port 1 ,while second qos sched will receives
> packets from port 2 and transmit through port 3. In case of single qos
> sched instance, following can be used-
>
> ./build/qos_sched -c 0x1e -n 4 -- --pfc "0,1,2,3,4" --cfg "profile.cfg"
>
>
> Thanks,
> Jasvinder
> The information contained in this electronic message and any
> attachments to this message are intended for the exclusive use of the
> addressee(s) and may contain proprietary, confidential or privileged
> information. If you are not the intended recipient, you should not
> disseminate, distribute or copy this e- mail. Please notify the sender
> immediately and destroy all copies of this message and any
> attachments. WARNING: Computer viruses can be transmitted via email.
> The recipient should check this email and any attachments for the
> presence of viruses. The company accepts no liability for any damage
> caused by any virus transmitted by this email. www.wipro.com
The information contained in this electronic message and any attachments to 
this message are intended for the exclusive use of the addressee(s) and may 
contain proprietary, confidential or privileged informa

[dpdk-dev] Traffic scheduling in DPDK

[Singh, Jasvinder]

> -Original Message-
> From: ravulakollu.kumar at wipro.com [mailto:ravulakollu.kumar at wipro.com]
> Sent: Wednesday, January 6, 2016 12:40 PM
> To: Singh, Jasvinder
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] Traffic scheduling in DPDK
> 
> Thanks Jasvinder,
> 
> Does this application works on systems with multiple NUMA Nodes ?
> 

It does. 

> Thanks,
> Uday
> 
> -Original Message-
> From: Singh, Jasvinder [mailto:jasvinder.singh at intel.com]
> Sent: Tuesday, January 05, 2016 3:40 PM
> To: Ravulakollu Udaya Kumar (WT01 - Product Engineering Service)
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] Traffic scheduling in DPDK
> 
> Hi Uday,
> 
> >
> > Thanks Jasvinder , I am running the below command
> >
> > ./build/qos_sched -c 0xe -n 1  -- --pfc "0,1,3,2" --cfg ./profile.cfg
> >
> > Bound two 1G physical ports to DPDK , and started running the above
> > command with the default profile mentioned in profile.cfg .
> > I am using lcore 3 and 2 for RX and TX. It was not successful, getting
> > the below error.
> >
> > APP: Initializing port 0... PMD: eth_igb_rx_queue_setup():
> > sw_ring=0x7f5b20ba2240 hw_ring=0x7f5b20ba2680
> dma_addr=0xbf87a2680
> > PMD: eth_igb_tx_queue_setup(): To improve 1G driver performance,
> > consider setting the TX WTHRESH value to 4, 8, or 16.
> > PMD: eth_igb_tx_queue_setup(): sw_ring=0x7f5b20b910c0
> > hw_ring=0x7f5b20b92100 dma_addr=0xbf8792100
> > PMD: eth_igb_start(): <<
> > done:  Link Up - speed 1000 Mbps - full-duplex
> > APP: Initializing port 1... PMD: eth_igb_rx_queue_setup():
> > sw_ring=0x7f5b20b80a40 hw_ring=0x7f5b20b80e80
> dma_addr=0xbf8780e80
> > PMD: eth_igb_tx_queue_setup(): To improve 1G driver performance,
> > consider setting the TX WTHRESH value to 4, 8, or 16.
> > PMD: eth_igb_tx_queue_setup(): sw_ring=0x7f5b20b6f8c0
> > hw_ring=0x7f5b20b70900 dma_addr=0xbf8770900
> > PMD: eth_igb_start(): <<
> > done:  Link Up - speed 1000 Mbps - full-duplex
> > SCHED: Low level config for pipe profile 0:
> > Token bucket: period = 3277, credits per period = 8, size = 100
> > Traffic classes: period = 500, credits per period = [12207,
> > 12207, 12207, 12207]
> > Traffic class 3 oversubscription: weight = 0
> > WRR cost: [1, 1, 1, 1], [1, 1, 1, 1], [1, 1, 1, 1], [1, 1, 1, 1]
> > EAL: Error - exiting with code: 1
> >   Cause: Unable to config sched subport 0, err=-2
> 
> 
> In default profile.cfg, It is assumed that all the nic ports have 10 Gbps 
> rate.
> The above error occurs when subport's  tb_rate (10Gbps) is found more than
> NIC port's capacity (1 Gbps). Therefore, you need to use either 10 Gbps ports
> in your application or have to amend the profile.cfg to work with 1 Gbps port.
> Please refer to  DPDK QoS framework document for more details on various
> parameters - http://dpdk.org/doc/guides/prog_guide/qos_framework.html
> 
> 
> > -Original Message-
> > From: Singh, Jasvinder [mailto:jasvinder.singh at intel.com]
> > Sent: Monday, January 04, 2016 9:26 PM
> > To: Ravulakollu Udaya Kumar (WT01 - Product Engineering Service);
> > dev at dpdk.org
> > Subject: RE: [dpdk-dev] Traffic scheduling in DPDK
> >
> > Hi Uday,
> >
> >
> > > I have an issue in running qos_sched application in DPDK .Could
> > > someone tell me how to run the command  and what each parameter
> does
> > > In the below mentioned text.
> > >
> > > Application mandatory parameters:
> > > --pfc "RX PORT, TX PORT, RX LCORE, WT LCORE" : Packet flow
> > configuration
> > >multiple pfc can be configured in command line
> >
> >
> > RX PORT - Specifies the packets receive port TX PORT - Specifies the
> > packets transmit port RXCORE - Specifies the  Core used for Packet
> > reception and Classification stage of the QoS application.
> > WTCORE-  Specifies the  Core used for Packet enqueue/dequeue operation
> > (QoS scheduling)  and subsequently transmitting the packets out.
> >
> > Multiple pfc  can be specified depending upon the number of instances
> > of qos sched required in application.  For example- in order to run
> > two instance, following can be used-
> >
> > ./build/qos_sched -c 0x7e -n 4 -- --pfc "0,1,2,3,4" --pfc "2,3,5,6"
> > --cfg "profile.cfg"
> >
> > First instance of qos sched receives packets from port 0 and transmits
> > its packets through port 1 ,while second qos sched will receives
> > packets from port 2 and transmit through port 3. In case of single qos
> > sched instance, following can be used-
> >
> > ./build/qos_sched -c 0x1e -n 4 -- --pfc "0,1,2,3,4" --cfg "profile.cfg"
> >
> >
> > Thanks,
> > Jasvinder
> > The information contained in this electronic message and any
> > attachments to this message are intended for the exclusive use of the
> > addressee(s) and may contain proprietary, confidential or privileged
> > information. If you are not the intended recipient, you should not
> > disseminate, distribute or copy this e- mail. Please notify the sender
> > immediately and destroy all copies of this message

[dpdk-dev] [PATCH v3 0/3] fix RTE_PROC_PRIMARY_OR_ERR_RET RTE_PROC_PRIMARY_OR_RET

[Ananyev, Konstantin]

> -Original Message-
> From: Pattan, Reshma
> Sent: Tuesday, January 05, 2016 4:35 PM
> To: Ananyev, Konstantin; Doherty, Declan; thomas.monjalon at 6wind.com
> Cc: dev at dpdk.org; Pattan, Reshma
> Subject: [PATCH v3 0/3] fix RTE_PROC_PRIMARY_OR_ERR_RET 
> RTE_PROC_PRIMARY_OR_RET
> 
> From: reshmapa 
> 
> Patches 1 and 2 removes RTE_PROC_PRIMARY_OR_ERR_RET and
> RTE_PROC_PRIMARY_OR_RET macro usage from rte_ether and rte_cryptodev 
> libraries to allow API
> access to secondary process.
> 
> Patch 3 allows users to configure ethdev with zero rx/tx queues, but both 
> should not be zero.
> Fix rte_eth_dev_tx_queue_config, rte_eth_dev_rx_queue_config to allocate 
> memory for rx/tx queues
> only when number of rx/tx queues are nonzero.
> 
> v3:
> * Removed checkpatch fixes of lib/librte_ether/rte_ethdev.h from patch number 
> 1.
> 
> Reshma Pattan (3):
>   librte_ether: remove RTE_PROC_PRIMARY_OR_ERR_RET and
> RTE_PROC_PRIMARY_OR_RET
>   librte_cryptodev: remove RTE_PROC_PRIMARY_OR_RET
>   librte_ether: fix rte_eth_dev_configure
> 
>  lib/librte_cryptodev/rte_cryptodev.c |   42 
>  lib/librte_ether/rte_ethdev.c|   86 
> ++
>  2 files changed, 25 insertions(+), 103 deletions(-)
> 
> --

Acked-by: Konstantin Ananyev 

> 1.7.4.1

[dpdk-dev] [PATCH v5 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd

[Ananyev, Konstantin]

> -Original Message-
> From: Wang, Zhihong
> Sent: Wednesday, December 30, 2015 10:00 PM
> To: dev at dpdk.org
> Cc: Ananyev, Konstantin; stephen at networkplumber.org; Qiu, Michael; Wang, 
> Zhihong
> Subject: [PATCH v5 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd
> 
> Handle SIGINT and SIGTERM in l3fwd.
> 
> Signed-off-by: Zhihong Wang 
> Acked-by: Michael Qiu 
> ---

Acked-by: Konstantin Ananyev 

[dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if packet type is set

[Ananyev, Konstantin]

> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Wednesday, January 06, 2016 10:01 AM
> To: Ananyev, Konstantin
> Cc: N?lio Laranjeiro; Tan, Jianfeng; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if packet 
> type is set
> 
> On Tue, Jan 05, 2016 at 04:50:31PM +, Ananyev, Konstantin wrote:
> [...]
> > > -Original Message-
> > > From: N?lio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
> [...]
> > > I think we miss a comment here in how those 2/6/4 values are chosen
> > > because, according to the mask, I expect 16 possibilities but I get
> > > less.  It will help a lot anyone who needs to add a new type.
> > >
> > > Extending the snprintf behavior above, it is best to remove the mask
> > > argument altogether and have rte_eth_dev_get_ptype_info() return the
> > > entire list every time.  Applications need to iterate on the result in
> > > any case.
> >
> > I think we'd better keep mask argument.
> > In many cases upper layer only interested in some particular  subset of
> > all packet types that HW can recognise.
> > Let say l3fwd only cares about  RTE_PTYPE_L3_MASK, it is not interested in 
> > L4,
> > tunnelling packet types, etc.
> > If caller needs to know all recognised ptypes, he can set mask==-1,
> > In that case all supported packet types will be returned.
> 
> There are other drawbacks to the mask argument in my opinion. The API will
> have to be updated again as soon as 32 bits aren't enough to represent all
> possible masks. We can't predict it will be large enough forever but on the
> other hand, using uint64_t seems overkill at this point.

Inside rte_mbuf packet_type itself is a 32 bit value.
These 32 bits are divided into several fields to mark packet types,
i.e: bits [0-3] are for all possible L2 types, bits [4-7] for L3 types, etc.
As long as packet_type itself is 32bits, 32bit mask is sufficient. 
If we'll ever run out of 32 bits in packet_type itself, it will be ABI change 
anyway.

> 
> I think this use for masks should be avoided when performance does not
> matter much, as in this case, user application cannot know the number of
> entries in advance and must rely on the returned value to iterate.

User doesn't know numbers of entries in advance anyway (with and without the 
mask).
That's why this function was introduced at first place.
With mask it just a bit more handy, in case user cares only about particular 
subset of supported
packet types (only L2 let say). 

> 
> A helper function can be added to convert a RTE_PTYPE_* value to the layer
> it belongs to (using enum to define possible values).

Not sure what for?

> 
> If we absolutely want a mean to filter returned values, I suggest we use
> this enum instead of the mask argument.
> Since it won't be a mask, it won't
> have to be updated every time a new protocol requires extending one.

Number of bits PTYPE_L2/L3/L4,... layers are already defined.
So let say RTE_PTYPE_L2_MASK shouldn't change if you'll add new L2 ptype -
there are few reserved values right now.  
if one day we'll run out bits in let say RTE_PTYPE_L2_MASK  and will have to 
increase its size -
it would mean change of the packet_type layout and possible ABI breakage 
anyway. 
Konstantin

> 
> > >   rte_eth_dev_get_ptype_info(uint8_t port_id, uint32_t ptypes[],
> > >  size_t max_entries)
> > >
> > >
> > >
> > > Another point, I have read the example patch (l3fwd) but I don't
> > > understand why the PMD is not responsible for filling the packet type in
> > > the MBUF (packet parsing is done by parse_packet_type()).  Why the extra
> > > computation?
> >
> > As I understand there are 3 possibilities now:
> > 1. HW supports ptype recognition and SW ptype parsing is never done
> > (--parse-ptype is not specified).
> > 2. HW supports ptype recognition, but and SW ptype parsing is done anyway
> > (--parse-ptype is specified).
> > 3. HW doesn't support and ptype recognition, and and SW ptype parsing is 
> > done
> > (--parse-ptype is specified).
> >
> > I suppose the question is what for introduce '--parse-ptype' at all?
> > My thought because of #2, so people can easily check what will be the 
> > performance impact of SW parsing.
> >
> > Konstantin
> >
> > >
> > > I see it more like an offload request (as checksum, etc...) and if the
> > > NIC does not support it then the application does the necessary overload.
> > >
> > > Best regards,
> > >
> > > --
> > > N?lio Laranjeiro
> > > 6WIND
> 
> --
> Adrien Mazarguil
> 6WIND

[dpdk-dev] [PATCH 0/3] ABI changes (RETA, cmdline)

[Nelio Laranjeiro]

Change ABIs as announced:

 * rte_eth_rss_reta_entry64.reta field to handle large number of
   queues (above 256).

 * Increase cmdline buffers to support long command lines, include long
   RETA update commands.

Nelio Laranjeiro (3):
  ethdev: change RETA type in rte_eth_rss_reta_entry64
  cmdline: increase command line buffer
  mlx5: increase RETA table size

 app/test-pmd/cmdline.c|  4 ++--
 doc/guides/rel_notes/deprecation.rst  | 10 --
 drivers/net/mlx5/mlx5_defs.h  |  2 +-
 lib/librte_cmdline/cmdline_parse.h|  2 +-
 lib/librte_cmdline/cmdline_parse_string.h |  2 +-
 lib/librte_cmdline/cmdline_rdline.h   |  2 +-
 lib/librte_ether/rte_ethdev.c |  2 +-
 lib/librte_ether/rte_ethdev.h |  2 +-
 8 files changed, 8 insertions(+), 18 deletions(-)

-- 
2.1.4

[dpdk-dev] [PATCH 1/3] ethdev: change RETA type in rte_eth_rss_reta_entry64

[Nelio Laranjeiro]

Several NICs can handle 512 entries/queues in their RETA table, an 8 bit field
is not large enough for them.

Signed-off-by: Nelio Laranjeiro 
---
 app/test-pmd/cmdline.c   | 4 ++--
 doc/guides/rel_notes/deprecation.rst | 5 -
 lib/librte_ether/rte_ethdev.c| 2 +-
 lib/librte_ether/rte_ethdev.h| 2 +-
 4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 73298c9..9c7cda0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1767,7 +1767,7 @@ parse_reta_config(const char *str,
int i;
unsigned size;
uint16_t hash_index, idx, shift;
-   uint8_t nb_queue;
+   uint16_t nb_queue;
char s[256];
const char *p, *p0 = str;
char *end;
@@ -1800,7 +1800,7 @@ parse_reta_config(const char *str,
}

hash_index = (uint16_t)int_fld[FLD_HASH_INDEX];
-   nb_queue = (uint8_t)int_fld[FLD_QUEUE];
+   nb_queue = (uint16_t)int_fld[FLD_QUEUE];

if (hash_index >= nb_entries) {
printf("Invalid RETA hash index=%d\n", hash_index);
diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index e94d4a2..e8a3ed6 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -15,11 +15,6 @@ Deprecation Notices
 * The ethdev structures rte_eth_link, rte_eth_dev_info and rte_eth_conf
   must be updated to support 100G link and to have a cleaner link speed API.

-* ABI changes is planned for the reta field in struct rte_eth_rss_reta_entry64
-  which handles at most 256 queues (8 bits) while newer NICs support larger
-  tables (512 queues).
-  It should be integrated in release 2.3.
-
 * ABI changes are planned for struct rte_eth_fdir_flow in order to support
   extend flow director's input set. The release 2.2 does not contain these ABI
   changes, but release 2.3 will, and no backwards compatibility is planned.
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index ed971b4..b0aa94d 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1857,7 +1857,7 @@ rte_eth_check_reta_mask(struct rte_eth_rss_reta_entry64 
*reta_conf,
 static int
 rte_eth_check_reta_entry(struct rte_eth_rss_reta_entry64 *reta_conf,
 uint16_t reta_size,
-uint8_t max_rxq)
+uint16_t max_rxq)
 {
uint16_t i, idx, shift;

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index bada8ad..8302a2d 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -520,7 +520,7 @@ struct rte_eth_mirror_conf {
 struct rte_eth_rss_reta_entry64 {
uint64_t mask;
/**< Mask bits indicate which entries need to be updated/queried. */
-   uint8_t reta[RTE_RETA_GROUP_SIZE];
+   uint16_t reta[RTE_RETA_GROUP_SIZE];
/**< Group of 64 redirection table entries. */
 };

-- 
2.1.4

[dpdk-dev] [PATCH 2/3] cmdline: increase command line buffer

[Nelio Laranjeiro]

For RETA query/update with a table of 512 entries, buffers are too small to
handle the request.

Signed-off-by: Nelio Laranjeiro 
---
 doc/guides/rel_notes/deprecation.rst  | 5 -
 lib/librte_cmdline/cmdline_parse.h| 2 +-
 lib/librte_cmdline/cmdline_parse_string.h | 2 +-
 lib/librte_cmdline/cmdline_rdline.h   | 2 +-
 4 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index e8a3ed6..9930b5a 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -39,8 +39,3 @@ Deprecation Notices
   and table action handlers will be updated:
   the pipeline parameter will be added, the packets mask parameter will be
   either removed (for input port action handler) or made input-only.
-
-* ABI changes are planned in cmdline buffer size to allow the use of long
-  commands (such as RETA update in testpmd).  This should impact
-  CMDLINE_PARSE_RESULT_BUFSIZE, STR_TOKEN_SIZE and RDLINE_BUF_SIZE.
-  It should be integrated in release 2.3.
diff --git a/lib/librte_cmdline/cmdline_parse.h 
b/lib/librte_cmdline/cmdline_parse.h
index 4b25c45..89b28b1 100644
--- a/lib/librte_cmdline/cmdline_parse.h
+++ b/lib/librte_cmdline/cmdline_parse.h
@@ -81,7 +81,7 @@ extern "C" {
 #define CMDLINE_PARSE_COMPLETED_BUFFER  2

 /* maximum buffer size for parsed result */
-#define CMDLINE_PARSE_RESULT_BUFSIZE 8192
+#define CMDLINE_PARSE_RESULT_BUFSIZE 65536

 /**
  * Stores a pointer to the ops struct, and the offset: the place to
diff --git a/lib/librte_cmdline/cmdline_parse_string.h 
b/lib/librte_cmdline/cmdline_parse_string.h
index c205622..61e0627 100644
--- a/lib/librte_cmdline/cmdline_parse_string.h
+++ b/lib/librte_cmdline/cmdline_parse_string.h
@@ -66,7 +66,7 @@ extern "C" {
 #endif

 /* size of a parsed string */
-#define STR_TOKEN_SIZE 128
+#define STR_TOKEN_SIZE 8192

 typedef char cmdline_fixed_string_t[STR_TOKEN_SIZE];

diff --git a/lib/librte_cmdline/cmdline_rdline.h 
b/lib/librte_cmdline/cmdline_rdline.h
index b9aad9b..07f8faa 100644
--- a/lib/librte_cmdline/cmdline_rdline.h
+++ b/lib/librte_cmdline/cmdline_rdline.h
@@ -93,7 +93,7 @@ extern "C" {
 #endif

 /* configuration */
-#define RDLINE_BUF_SIZE 256
+#define RDLINE_BUF_SIZE 16384
 #define RDLINE_PROMPT_SIZE  32
 #define RDLINE_VT100_BUF_SIZE  8
 #define RDLINE_HISTORY_BUF_SIZE BUFSIZ
-- 
2.1.4

[dpdk-dev] [PATCH 3/3] mlx5: increase RETA table size

[Nelio Laranjeiro]

ConnectX-4 NICs can handle at most 512 entries in RETA table.

Signed-off-by: Nelio Laranjeiro 
---
 drivers/net/mlx5/mlx5_defs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index bb82c9a..ae5eda9 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -47,7 +47,7 @@
 #define MLX5_PMD_TX_PER_COMP_REQ 64

 /* RSS Indirection table size. */
-#define RSS_INDIRECTION_TABLE_SIZE 128
+#define RSS_INDIRECTION_TABLE_SIZE 512

 /* Maximum number of Scatter/Gather Elements per Work Request. */
 #ifndef MLX5_PMD_SGE_WR_N
-- 
2.1.4

[dpdk-dev] [PATCH v3 1/4] eal: Introduce new cache macro definitions

[Jerin Jacob]

On Mon, Jan 04, 2016 at 02:15:53PM +0100, Olivier MATZ wrote:
> Hi Jerin,

Hi Olivier,

> 
> Please see some comments below.
> 
> On 12/14/2015 05:32 AM, Jerin Jacob wrote:
> > - RTE_CACHE_MIN_LINE_SIZE(Supported minimum cache line size)
> > - __rte_cache_min_aligned(Force minimum cache line alignment)
> > - RTE_CACHE_LINE_SIZE_LOG2(Express cache line size in terms of log2)
> > 
> > Signed-off-by: Jerin Jacob 
> > Suggested-by: Konstantin Ananyev 
> > ---
> >  lib/librte_eal/common/include/rte_memory.h | 16 
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/lib/librte_eal/common/include/rte_memory.h 
> > b/lib/librte_eal/common/include/rte_memory.h
> > index 9c9e40f..b67a76f 100644
> > --- a/lib/librte_eal/common/include/rte_memory.h
> > +++ b/lib/librte_eal/common/include/rte_memory.h
> > @@ -77,11 +77,27 @@ enum rte_page_sizes {
> > (RTE_CACHE_LINE_SIZE * ((size + RTE_CACHE_LINE_SIZE - 1) / 
> > RTE_CACHE_LINE_SIZE))
> >  /**< Return the first cache-aligned value greater or equal to size. */
> >  
> > +/**< Cache line size in terms of log2 */
> > +#if RTE_CACHE_LINE_SIZE == 64
> > +#define RTE_CACHE_LINE_SIZE_LOG2 6
> > +#elif RTE_CACHE_LINE_SIZE == 128
> > +#define RTE_CACHE_LINE_SIZE_LOG2 7
> > +#else
> > +#error "Unsupported cache line size"
> > +#endif
> > +
> > +#define RTE_CACHE_MIN_LINE_SIZE 64 /**< Minimum Cache line size. */
> > +
> 
> I think RTE_CACHE_LINE_MIN_SIZE or RTE_MIN_CACHE_LINE_SIZE would
> be clearer than RTE_CACHE_MIN_LINE_SIZE.

OK. I will resend the next version with RTE_CACHE_LINE_MIN_SIZE

> 
> >  /**
> >   * Force alignment to cache line.
> >   */
> >  #define __rte_cache_aligned __rte_aligned(RTE_CACHE_LINE_SIZE)
> >  
> > +/**
> > + * Force minimum cache line alignment.
> > + */
> > +#define __rte_cache_min_aligned __rte_aligned(RTE_CACHE_MIN_LINE_SIZE)
> 
> I'm not really convinced that __rte_cache_min_aligned is straightforward
> for someone reading the code that it means "aligned to the minimum cache
> line size supported by the dpdk".
> 
> In the two cases you are using this macro (mbuf structure and queue
> info), I'm wondering if using __attribute__((aligned(64))) wouldn't be
> clearer?
> - for mbuf, it could be a local define, like MBUF_ALIGN_SIZE
> - for queue info, using 64 makes sense as it's used to reserve space
>   for future use
> 
> What do you think?

IMO, it makes sense to keep "__rte_cache_min_aligned" as it will useful
for forcing the minimum alignment requirements in DPDK in future
structures.

Thoughts?

> 
> 
> Regards,
> Olivier

[dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if packet type is set

[Adrien Mazarguil]

On Wed, Jan 06, 2016 at 02:29:07PM +, Ananyev, Konstantin wrote:
> 
> 
> > -Original Message-
> > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> > Sent: Wednesday, January 06, 2016 10:01 AM
> > To: Ananyev, Konstantin
> > Cc: N?lio Laranjeiro; Tan, Jianfeng; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if 
> > packet type is set
> > 
> > On Tue, Jan 05, 2016 at 04:50:31PM +, Ananyev, Konstantin wrote:
> > [...]
> > > > -Original Message-
> > > > From: N?lio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
> > [...]
> > > > I think we miss a comment here in how those 2/6/4 values are chosen
> > > > because, according to the mask, I expect 16 possibilities but I get
> > > > less.  It will help a lot anyone who needs to add a new type.
> > > >
> > > > Extending the snprintf behavior above, it is best to remove the mask
> > > > argument altogether and have rte_eth_dev_get_ptype_info() return the
> > > > entire list every time.  Applications need to iterate on the result in
> > > > any case.
> > >
> > > I think we'd better keep mask argument.
> > > In many cases upper layer only interested in some particular  subset of
> > > all packet types that HW can recognise.
> > > Let say l3fwd only cares about  RTE_PTYPE_L3_MASK, it is not interested 
> > > in L4,
> > > tunnelling packet types, etc.
> > > If caller needs to know all recognised ptypes, he can set mask==-1,
> > > In that case all supported packet types will be returned.
> > 
> > There are other drawbacks to the mask argument in my opinion. The API will
> > have to be updated again as soon as 32 bits aren't enough to represent all
> > possible masks. We can't predict it will be large enough forever but on the
> > other hand, using uint64_t seems overkill at this point.
> 
> Inside rte_mbuf packet_type itself is a 32 bit value.
> These 32 bits are divided into several fields to mark packet types,
> i.e: bits [0-3] are for all possible L2 types, bits [4-7] for L3 types, etc.
> As long as packet_type itself is 32bits, 32bit mask is sufficient. 
> If we'll ever run out of 32 bits in packet_type itself, it will be ABI change 
> anyway.

Sure, however why not do it now this issue has been raised so this function
doesn't need updating the day it breaks? I know there's a million other
places with a similar problem but I'm all for making new code future proof.

Perhaps in this particular case there is no way to hit the limit (although
there are only four unused bits left to extend RTE_PTYPE masks) but we've
seen this happen too many times with subsequent ABI breakage.

> > I think this use for masks should be avoided when performance does not
> > matter much, as in this case, user application cannot know the number of
> > entries in advance and must rely on the returned value to iterate.
> 
> User doesn't know numbers of entries in advance anyway (with and without the 
> mask).
> That's why this function was introduced at first place.
> 
> With mask it just a bit more handy, in case user cares only about particular 
> subset of supported
> packet types (only L2 let say). 

OK, so we definitely need something to let applications know the layer a
given packet type belongs to, I'm sure it can be done in a convenient way
that won't be limited to the underlying type of the mask.

> > A helper function can be added to convert a RTE_PTYPE_* value to the layer
> > it belongs to (using enum to define possible values).
> 
> Not sure what for?

This is assuming rte_eth_dev_get_ptype_info() doesn't filter anything (no
"mask" argument). In that case a separate function must be added to convert
RTE_PTYPE_* values to a layer, so applications can look for interesting
packet types while parsing plist[] on their own.

This layer information could be defined as an enum, i.e.:

 enum rte_ptype_info {
 RTE_PTYPE_UNKNOWN,
 RTE_PTYPE_L2,
 RTE_PTYPE_L3,
...
 };

Or even an int value (2 standing for for "layer 2" etc. Tunnel encapsulation
wouldn't be described easily that way though). It's just an idea.

> > If we absolutely want a mean to filter returned values, I suggest we use
> > this enum instead of the mask argument.
> > Since it won't be a mask, it won't
> > have to be updated every time a new protocol requires extending one.
> 
> Number of bits PTYPE_L2/L3/L4,... layers are already defined.
> So let say RTE_PTYPE_L2_MASK shouldn't change if you'll add new L2 ptype -
> there are few reserved values right now.  
> if one day we'll run out bits in let say RTE_PTYPE_L2_MASK  and will have to 
> increase its size -
> it would mean change of the packet_type layout and possible ABI breakage 
> anyway. 

I'm aware of this, only pointing out we tend to rely on masks and type
boundaries a bit too much when there are other methods that are as (if not
more) convenient.

Perhaps some sort of tunneled packet types beyond inner L4 consuming the
four remaining bits will be added? That could happen s

[dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if packet type is set

[Ananyev, Konstantin]

> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Wednesday, January 06, 2016 3:45 PM
> To: Ananyev, Konstantin
> Cc: N?lio Laranjeiro; Tan, Jianfeng; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if packet 
> type is set
> 
> On Wed, Jan 06, 2016 at 02:29:07PM +, Ananyev, Konstantin wrote:
> >
> >
> > > -Original Message-
> > > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> > > Sent: Wednesday, January 06, 2016 10:01 AM
> > > To: Ananyev, Konstantin
> > > Cc: N?lio Laranjeiro; Tan, Jianfeng; dev at dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if 
> > > packet type is set
> > >
> > > On Tue, Jan 05, 2016 at 04:50:31PM +, Ananyev, Konstantin wrote:
> > > [...]
> > > > > -Original Message-
> > > > > From: N?lio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
> > > [...]
> > > > > I think we miss a comment here in how those 2/6/4 values are chosen
> > > > > because, according to the mask, I expect 16 possibilities but I get
> > > > > less.  It will help a lot anyone who needs to add a new type.
> > > > >
> > > > > Extending the snprintf behavior above, it is best to remove the mask
> > > > > argument altogether and have rte_eth_dev_get_ptype_info() return the
> > > > > entire list every time.  Applications need to iterate on the result in
> > > > > any case.
> > > >
> > > > I think we'd better keep mask argument.
> > > > In many cases upper layer only interested in some particular  subset of
> > > > all packet types that HW can recognise.
> > > > Let say l3fwd only cares about  RTE_PTYPE_L3_MASK, it is not interested 
> > > > in L4,
> > > > tunnelling packet types, etc.
> > > > If caller needs to know all recognised ptypes, he can set mask==-1,
> > > > In that case all supported packet types will be returned.
> > >
> > > There are other drawbacks to the mask argument in my opinion. The API will
> > > have to be updated again as soon as 32 bits aren't enough to represent all
> > > possible masks. We can't predict it will be large enough forever but on 
> > > the
> > > other hand, using uint64_t seems overkill at this point.
> >
> > Inside rte_mbuf packet_type itself is a 32 bit value.
> > These 32 bits are divided into several fields to mark packet types,
> > i.e: bits [0-3] are for all possible L2 types, bits [4-7] for L3 types, etc.
> > As long as packet_type itself is 32bits, 32bit mask is sufficient.
> > If we'll ever run out of 32 bits in packet_type itself, it will be ABI 
> > change anyway.
> 
> Sure, however why not do it now this issue has been raised so this function
> doesn't need updating the day it breaks? I know there's a million other
> places with a similar problem but I'm all for making new code future proof.

If rte_mbuf packet_type will have to be increased to 64bit long, then
this function will have to change anyway (with or without mask parameter).
It will have to become:

rte_eth_dev_get_ptype_info(uint8_t portid, uint64_t ptypes[], ...)

So I think we don't have to worry about mask parameter itself.

> 
> Perhaps in this particular case there is no way to hit the limit (although
> there are only four unused bits left to extend RTE_PTYPE masks) but we've
> seen this happen too many times with subsequent ABI breakage.

When ptype was introduced we tried to reserve some free space for each layer 
(L2/L3/L4/...),
so it wouldn't be overrun immediately.
But of course if there would be a new HW that can recognise dozen new packet 
types - it is possible.
Do you have any particular use-case in mind? 

> 
> > > I think this use for masks should be avoided when performance does not
> > > matter much, as in this case, user application cannot know the number of
> > > entries in advance and must rely on the returned value to iterate.
> >
> > User doesn't know numbers of entries in advance anyway (with and without 
> > the mask).
> > That's why this function was introduced at first place.
> >
> > With mask it just a bit more handy, in case user cares only about 
> > particular subset of supported
> > packet types (only L2 let say).
> 
> OK, so we definitely need something to let applications know the layer a
> given packet type belongs to, I'm sure it can be done in a convenient way
> that won't be limited to the underlying type of the mask.
> 
> > > A helper function can be added to convert a RTE_PTYPE_* value to the layer
> > > it belongs to (using enum to define possible values).
> >
> > Not sure what for?
> 
> This is assuming rte_eth_dev_get_ptype_info() doesn't filter anything (no
> "mask" argument). In that case a separate function must be added to convert
> RTE_PTYPE_* values to a layer, so applications can look for interesting
> packet types while parsing plist[] on their own.

Honestly, I don't see why do you need that.
You already do know that  let say RTE_PTYPE_L3_IPV4 belongs to L3.
Why do you need some extra enum her

[dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if packet type is set

[Adrien Mazarguil]

On Wed, Jan 06, 2016 at 04:44:43PM +, Ananyev, Konstantin wrote:
> 
> 
> > -Original Message-
> > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> > Sent: Wednesday, January 06, 2016 3:45 PM
> > To: Ananyev, Konstantin
> > Cc: N?lio Laranjeiro; Tan, Jianfeng; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if 
> > packet type is set
> > 
> > On Wed, Jan 06, 2016 at 02:29:07PM +, Ananyev, Konstantin wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> > > > Sent: Wednesday, January 06, 2016 10:01 AM
> > > > To: Ananyev, Konstantin
> > > > Cc: N?lio Laranjeiro; Tan, Jianfeng; dev at dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if 
> > > > packet type is set
> > > >
> > > > On Tue, Jan 05, 2016 at 04:50:31PM +, Ananyev, Konstantin wrote:
> > > > [...]
> > > > > > -Original Message-
> > > > > > From: N?lio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
> > > > [...]
> > > > > > I think we miss a comment here in how those 2/6/4 values are chosen
> > > > > > because, according to the mask, I expect 16 possibilities but I get
> > > > > > less.  It will help a lot anyone who needs to add a new type.
> > > > > >
> > > > > > Extending the snprintf behavior above, it is best to remove the mask
> > > > > > argument altogether and have rte_eth_dev_get_ptype_info() return the
> > > > > > entire list every time.  Applications need to iterate on the result 
> > > > > > in
> > > > > > any case.
> > > > >
> > > > > I think we'd better keep mask argument.
> > > > > In many cases upper layer only interested in some particular  subset 
> > > > > of
> > > > > all packet types that HW can recognise.
> > > > > Let say l3fwd only cares about  RTE_PTYPE_L3_MASK, it is not 
> > > > > interested in L4,
> > > > > tunnelling packet types, etc.
> > > > > If caller needs to know all recognised ptypes, he can set mask==-1,
> > > > > In that case all supported packet types will be returned.
> > > >
> > > > There are other drawbacks to the mask argument in my opinion. The API 
> > > > will
> > > > have to be updated again as soon as 32 bits aren't enough to represent 
> > > > all
> > > > possible masks. We can't predict it will be large enough forever but on 
> > > > the
> > > > other hand, using uint64_t seems overkill at this point.
> > >
> > > Inside rte_mbuf packet_type itself is a 32 bit value.
> > > These 32 bits are divided into several fields to mark packet types,
> > > i.e: bits [0-3] are for all possible L2 types, bits [4-7] for L3 types, 
> > > etc.
> > > As long as packet_type itself is 32bits, 32bit mask is sufficient.
> > > If we'll ever run out of 32 bits in packet_type itself, it will be ABI 
> > > change anyway.
> > 
> > Sure, however why not do it now this issue has been raised so this function
> > doesn't need updating the day it breaks? I know there's a million other
> > places with a similar problem but I'm all for making new code future proof.
> 
> If rte_mbuf packet_type will have to be increased to 64bit long, then
> this function will have to change anyway (with or without mask parameter).
> It will have to become:
> 
> rte_eth_dev_get_ptype_info(uint8_t portid, uint64_t ptypes[], ...)
> 
> So I think we don't have to worry about mask parameter itself.

Well, yes, besides I overlooked ptypes[] itself is 32 bit, working around
the type width of the mask wouldn't help much.

> > Perhaps in this particular case there is no way to hit the limit (although
> > there are only four unused bits left to extend RTE_PTYPE masks) but we've
> > seen this happen too many times with subsequent ABI breakage.
> 
> When ptype was introduced we tried to reserve some free space for each layer 
> (L2/L3/L4/...),
> so it wouldn't be overrun immediately.
> But of course if there would be a new HW that can recognise dozen new packet 
> types - it is possible.
> Do you have any particular use-case in mind? 

No, that was just to illustrate my point.

> > > > I think this use for masks should be avoided when performance does not
> > > > matter much, as in this case, user application cannot know the number of
> > > > entries in advance and must rely on the returned value to iterate.
> > >
> > > User doesn't know numbers of entries in advance anyway (with and without 
> > > the mask).
> > > That's why this function was introduced at first place.
> > >
> > > With mask it just a bit more handy, in case user cares only about 
> > > particular subset of supported
> > > packet types (only L2 let say).
> > 
> > OK, so we definitely need something to let applications know the layer a
> > given packet type belongs to, I'm sure it can be done in a convenient way
> > that won't be limited to the underlying type of the mask.
> > 
> > > > A helper function can be added to convert a RTE_PTYPE_* value to the 
> > > > layer
> > > > it belongs to (using enum to def

[dpdk-dev] [PATCH] Patch introducing API to read/write Intel Architecture Model Specific Registers (MSR), rte_msr_read and rte_msr_write functions.

[Jerin Jacob]

On Wed, Jan 06, 2016 at 11:47:28AM +, Andralojc, WojciechX wrote:
> > From: Andralojc, WojciechX
> > Sent: Thursday, December 17, 2015 12:13 PM
> > To: dev at dpdk.org
> > Cc: Andralojc, WojciechX
> > Subject: [PATCH] Patch introducing API to read/write Intel Architecture 
> > Model
> > Specific Registers (MSR), rte_msr_read and rte_msr_write functions.
> > 
> > There is work in progress to implement Intel Cache Allocation Technology 
> > (CAT)
> > support in DPDK, this technology is programmed through MSRs.
> > In the future it will be possible to program CAT through Linux cgroups and 
> > DPDK
> > CAT implementation will take advantage of it.
> > 
> > MSR R/W's are privileged ring 0 operations and they must be done in kernel
> > space. For this reason implementation utilizes Linux MSR driver.
> > 
> > Signed-off-by: Wojciech Andralojc 
> 
> I've got suggestion offline that as MSRs are IA specific,
> I should not give the dummy APIs for the other arches
> and move MSR access functions into the EAL specific APIs
> or some place more arch specific. 
> Do you find submitted MSR patch OK?
> or do you agree with the above feedback and patch should be re-worked?

+1

IMO, No need to expose this function as EAL as other archs can't
implement this.I think, a IA specific function under
lib/librte_eal/common/include/arch/x86/ and removing rte_* from
internal architecture functions looks more appropriate

Jerin

> I am looking forward to your feedback
> 
> Thank you!
> 
> Wojciech Andralojc
> --
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
> 
> 
> This e-mail and any attachments may contain confidential material for the sole
> use of the intended recipient(s). Any review or distribution by others is
> strictly prohibited. If you are not the intended recipient, please contact the
> sender and delete all copies.
>