Re: [dpdk-dev] [PATCH 1/2] net/ena: upgrade HAL for new HW features

["Michał Krawczyk" <mk@semihalf.com>]

czw., 9 sty 2020 o 15:27 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> 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.
>

Hi Ferruh,

Sure, I will keep that in mind and during next release I will try to split it.

Thanks,
Michal