DPDK patches and discussions
 help / color / mirror / Atom feed
From: Vlad Zolotarov <vladz@cloudius-systems.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4 2/5] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices
Date: Mon, 09 Mar 2015 21:28:39 +0200	[thread overview]
Message-ID: <54FDF467.1090301@cloudius-systems.com> (raw)
In-Reply-To: <2357467.DFWlgGcAaX@xps13>



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.

  parent reply	other threads:[~2015-03-09 19:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-08 14:04 [dpdk-dev] [PATCH v4 0/5]: Add LRO support to ixgbe PMD Vlad Zolotarov
2015-03-08 14:04 ` [dpdk-dev] [PATCH v4 1/5] ixgbe: Cleanups Vlad Zolotarov
2015-03-08 14:04 ` [dpdk-dev] [PATCH v4 2/5] ixgbe: Bug fix: Properly configure Rx CRC stripping for x540 devices Vlad Zolotarov
2015-03-08 21:12   ` Thomas Monjalon
2015-03-09  7:08     ` Vlad Zolotarov
2015-03-09  7:58       ` Thomas Monjalon
2015-03-09 16:10         ` Vlad Zolotarov
2015-03-09 19:28         ` Vlad Zolotarov [this message]
2015-03-09 19:30         ` Vlad Zolotarov
2015-03-09  7:31     ` Vlad Zolotarov
2015-03-08 14:04 ` [dpdk-dev] [PATCH v4 3/5] ixgbe: Code refactoring Vlad Zolotarov
2015-03-08 14:04 ` [dpdk-dev] [PATCH v4 4/5] ixgbe: Unify the rx_pkt_bulk callback initialization Vlad Zolotarov
2015-03-08 14:04 ` [dpdk-dev] [PATCH v4 5/5] ixgbe: Add LRO support Vlad Zolotarov
2015-03-08 21:20   ` Thomas Monjalon
2015-03-09  7:13     ` Vlad Zolotarov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54FDF467.1090301@cloudius-systems.com \
    --to=vladz@cloudius-systems.com \
    --cc=dev@dpdk.org \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).