DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Michał Krawczyk" <mk@semihalf.com>,
	"Stephen Hemminger" <stephen@networkplumber.org>
Cc: dev@dpdk.org, "Tzalik, Guy" <gtzalik@amazon.com>,
	Marcin Wojtas <mw@semihalf.com>,
	"Matushevsky, Alexander" <matua@amazon.com>,
	mba <mba@semihalf.com>, "Chauskin, Igor" <igorch@amazon.com>
Subject: Re: [dpdk-dev] [PATCH 1/2] net/ena: upgrade HAL for new HW features
Date: Thu, 9 Jan 2020 14:27:12 +0000	[thread overview]
Message-ID: <54235c88-57e1-bb2e-4067-c39a47e9d7f6@intel.com> (raw)
In-Reply-To: <CAJMMOfPCUNm3DbZzdOkGGZ0VvsQCkW7ycHbpyeFHdbNV8fN=mQ@mail.gmail.com>

On 12/13/2019 8:01 PM, Michał Krawczyk wrote:
> pt., 13 gru 2019 o 17:34 Stephen Hemminger <stephen@networkplumber.org>
> napisał(a):
>>
>> On Fri, 13 Dec 2019 14:32:15 +0100
>> Michal Krawczyk <mk@semihalf.com> wrote:
>>
>>> This version of the HAL allows to use the latest HW features, like
>>> rx offsets.
>>>
>>> Driver was adjusted to the new version to fix the build.
>>>
>>> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
>>> Signed-off-by: Maciej Bielski <mba@semihalf.com>
>>
>> You are mixing multiple changes into one patch.
>> This makes it harder to review (find the real bits) and also
>> harder for bisection.
>>
>> It makes sense to fix whitespace and related stuff in one
>> patch if you are fixing one function and the nearby code
>> already needed work. But please avoid larger scale change
>> put together.
>>
>> In your patch I see:
>>         - remove unnecessary whitespace
>>         - drop unnecessary inline
>>         - add missing newline in log messages
>>         - check for NULL pointer
>>
>> If possible could you preserve the per-commit updates for base
>> code, rather than one lump diff.
> 
> Hi Stephen,
> 
> We are not developing HAL (ena_com) on our own. We are getting it from the
> Amazon team and we don't have history of it's development. Tha's why we are
> upstreaming a diff between two versions of the ena_com.
> 
> Moreover this HAL is common for Linux kernel driver, FreeBSD kernel driver
> and Windows kernel driver. Because of that, we are trying to avoid to
> modify it on our side unless it's really necessary (for example, if
> compilation fails on for the DPDK).

Hi Michał,

Overall this is the common situation for most of the base code, that is why we
are a little more flexible on them and not enforcing the coding convention etc..

But as a high level rule, still please try to split the base code drop to
logical pieces as much as possible, and I am aware that this requires additional
effort.
I will proceed with this patchset, but please bare in mind this for next time.

Regards,
ferruh


> 
> In fact, we are doing changes to the PMD and our platform file
> (ena_plat_dpdk.h which is a glue) in this patch, but that is required in
> order to do not have patch which is not compiling.
> Both the PMD code and the glue code can be changed without affecting other
> platforms.
> 

  reply	other threads:[~2020-01-09 14:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-13 13:32 [dpdk-dev] [PATCH 0/2] net/ena: add support for Rx offsets Michal Krawczyk
2019-12-13 13:32 ` [dpdk-dev] [PATCH 1/2] net/ena: upgrade HAL for new HW features Michal Krawczyk
2019-12-13 16:34   ` Stephen Hemminger
2019-12-13 20:01     ` Michał Krawczyk
2020-01-09 14:27       ` Ferruh Yigit [this message]
2020-01-09 15:26         ` Michał Krawczyk
2020-01-09 14:30   ` Ferruh Yigit
2020-01-09 15:39     ` [dpdk-dev] [PATCH v2 " Michal Krawczyk
2020-01-10  9:04       ` Ferruh Yigit
2020-01-10  9:39         ` Michał Krawczyk
2020-01-10 10:32       ` Ferruh Yigit
2019-12-13 13:32 ` [dpdk-dev] [PATCH 2/2] net/ena: add support for RX_OFFSET feature Michal Krawczyk
2020-01-10 10:32   ` Ferruh Yigit

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=54235c88-57e1-bb2e-4067-c39a47e9d7f6@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=gtzalik@amazon.com \
    --cc=igorch@amazon.com \
    --cc=matua@amazon.com \
    --cc=mba@semihalf.com \
    --cc=mk@semihalf.com \
    --cc=mw@semihalf.com \
    --cc=stephen@networkplumber.org \
    /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).