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

[Vlad Zolotarov <vladz@cloudius-systems.com>]

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 <vladz@cloudius-systems.com>
>>> 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?

PATCH4 doesn't have any refactoring - it fixes a design bug (actually 
two). The description of the patch is separated into two sections: one 
that describes how I did it and the second (starts with "Bugs fixed:") 
describes which bugs it fixes.

When I re-read its description again now I see that I forgot to mention 
the second issue it fixes: there is the same problem for both Vector Rx 
callback initialization and for Bulk Rx callback initialization. In both 
cases in the current master tree the queue initialization code tries to 
initialize the port property (rx_pkt_bulk callback) which depends on all 
queues properties thus may be done only after all queues properties have 
been analyzed. More than that, some callbacks types may be used only 
when some combination of preconditions is met by all Rx queues: e.g. 
vector Rx may be used only when bulk (checks queue configuration) and 
vector (checks queue and port configuration) preconditions are met. 
Therefore I decided to introduce two port states that are a logical AND 
function of the appropriate queues' states. These states are used by a 
port configuration function that now can properly configure the callbacks.

So, again - no refactoring here - a pure bug fix... ;)

>
> 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.