Re: [dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Yong Wang <yongwang@vmware.com>]

On 3/10/15, 11:35 AM, "Stephen Hemminger" <stephen@networkplumber.org>
wrote:

>On Mon, 9 Mar 2015 23:32:48 +0000
>Yong Wang <yongwang@vmware.com> wrote:
>
>> >
>> >I didn’t see where mtu is used to calculate how many rx descriptors
>>will
>> >be needed for each packet.  Furthermore, as pointed out by the
>>following
>> >code comments, the device requires the first rx buffer of a packet be
>>of
>> >type VMXNET3_RXD_BTYPE_HEAD with the remaining buffers of type
>> >VMXNET3_RXD_BTYPE_NODY.  This needs to be taken care of when
>>populating rx
>> >rings in vmxnet3_post_rx_bufs(). Currently we don’t do this because no
>> >scatter-rx is supported and only one descriptor is needed for a packet
>> >(thus all types should be HEAD). Otherwise, the device will complain
>>with  
>> 
>> To clarify, in this case only the 1st ring will be used and thus all
>>types
>> will be HEAD.
>> 
>> >error returned.  For the 2nd rx ring, type needs to be BODY for all
>> >descriptors still.
>
>Looking in more detail, the code as submitted looks fine:
>
>* The # of rx descriptors needed for each packet is responsibility of
>  the application. Before and after this patch the # of Rx descriptors
>  for each ring (head/body) is determined by vmxnet3_rx_queue_setup.
>  And the driver always allocates the same number of descriptors
>  for both Rx rings.

Where is vmxnet3_rx_queue_setup? It’s not in the current code base nor in
your patch and I failed to see how rx descriptors are populated.

Could you explain what do you mean by responsibility of the application?
If mtu is set to 9000, the driver needs to make sure rx buf per packet is
set to the correct size with proper type set, right?

>
>  One could argue that was a bug in the original driver, since it
>needlessly
>  allocated mbufs for the second ring and never used them. I suppose
>  as a optimization in future, the second ring could be sized as 0
>  and no buffers allocated unless doing rx-scatter (this is what the
>  Linux driver does. But that change is independent of this.

We can just keep it as is as we will need it if someone decides to support
LRO for vmxnet3.

>
>* Buffers are correctly reallocated as needed when consumed.
>
>* The code works for bulk transfer and performance tests in jumbo mode.

You mean the standalone patch you posted without the
vmxnet3_rx_queue_setup you mentioned?

>
>I see no requirement to change this patch.
>