[ovs-dev] [PATCH v2 1/1] dpdk: Update to use DPDK v20.11.

[Ilya Maximets]

On 12/14/20 10:06 PM, Luca Boccassi wrote:
> On Mon, 2020-12-14 at 19:11 +0100, Ilya Maximets wrote:
>> On 12/14/20 3:35 PM, Luca Boccassi wrote:
>>> On Mon, 2020-12-14 at 15:24 +0100, Ilya Maximets wrote:
>>>> On 12/14/20 3:17 PM, Luca Boccassi wrote:
>>>>> On Mon, 2020-12-14 at 15:05 +0100, David Marchand wrote:
>>>>>> On Mon, Dec 14, 2020 at 12:45 PM Ilya Maximets <i.maximets at ovn.org> wrote:
>>>>>>>> On DPDK side, it should only be a couple of headers that are causing compilation
>>>>>>>> issues, and we can investigate what we can do with them - at least some of them
>>>>>>>> can be reworked to support a non-SSE4.2 fallback, I believe. If we can rework
>>>>>>>> them that would then remove the SSE4.2 requirement for any code paths not using DPDK.
>>>>>>>> In parallel we can also see if the -march flag could be just replaced with an -msse
>>>>>>>> one.
>>>>>>>
>>>>>>> Sounds reasonable.  Removing of the SSE4.2 requirement would be great.
>>>>>>>
>>>>>>> But we still have an issue with this particular patch for OVS right now that can
>>>>>>> not wait for the next stable DPDK version.
>>>>>>>
>>>>>>> I'd stick with stripping out '-march=.*' from DPDK_INCLUDE (patch above) and
>>>>>>> conditionally adding '-mssse3' if compiler supports (we already have this and
>>>>>>> had no issues building with DPDK in this configuration starting from DPDK 2.0.
>>>>>>> I also worked with OVS + DPDK on arm and it was never required to pass any
>>>>>>> specific compiler flags to OVS in order to build and work correctly).
>>>>>>> This will be re-considered once DPDK changes accepted.
>>>>>>
>>>>>> OVS has lived with SSSE3 so far, while the requirement to SSE4.2 was
>>>>>> added back in dpdk 17.08.
>>>>>> Stripping -march for now is the simplest/best approach.
>>>>>
>>>>> That's because we fixed it on the distro side long ago. This _will_
>>>>> reintroduce regressions. Please don't add more work on us.
>>>>
>>>> Could you, please, point to these regressions, so we can check them?
>>>>
>>>> "there are no issues" vs "there are issues" dialog makes no much sense.
>>>>
>>>> Best regards, Ilya Maximets.
>>>
>>> It's future tense - I can't quite post bug reports from the future
>>> unfortunately, my home network is fast but it doesn't yet break
>>> causality :-)
>>
>> I asked because you said that it "_will_ _reintroduce_ regressions" meaning
>> there were already regressions that was fixed.  So, I'm assuming that
>> there are no known issues with building OVS with -mssse3 only.
> 
> Sorry, I didn't explain myself properly - I wasn't referring to mssse3
> specifically, but to the lack of a clear and well defined way of
> passing information from libdpdk down to applications that "just works"
> regardless of what happens and changes.
> 
>>> The whole workstream of adding pkg-config in DPDK started in Debian and
>>> Ubuntu from myself and Christian long before upstream supported it,
>>> years earlier in fact. We did that because there are significant and
>>> critical information, decided by DPDK at build time, that needs to flow
>>> down to applications linking against it for everything to work
>>> properly.
>>
>> I understand that and I really appreciate all the work you did.  It took
>> a lot of efforts to bring DPDK build system to the state it is now.  And
>> it is a much cleaner way to do things, indeed.
>>
>> OTOH, this is, actually, the first real integration of this new DPDK
>> pkg-config based system into other upstream project as a main and only
>> way to build.  We already fixed some issues right before DPDK 20.11
>> release, but unfortunately, not all of them.  And it's hard to expect
>> everything to be perfect right from the beginning.
> 
> Not true, actually. The router formerly known as Vyatta has been using
> dpdk via pkg-config since we first added it in Debian/Ubuntu - actually
> even before that. About 5 years, give or take?

I'm having trouble finding any information about that.  Anyway, IIUC,
vRouter wasn't intended to work without DPDK, so this is a bit different
case, i.e. it doesn't make much sense to have different from DPDK set of
cflags if your application works only with DPDK.  So, they have ho such
problem.

> 
>>> Second-guessing this information and unconditionally overriding it will
>>> only produce more work for those of us who have to integrate the whole
>>> thing in a way that works> 
>>> So please don't do that. If you want to add _manual_ overrides that
>>> someone who rebuilds locally can choose to use, that's fine by me.
>>>
>>
>> While it is convenient for Ubuntu and Debian to keep -march, since you're
>> building two separate packages and you don't really care that other package
>> will not be able to work on lower end system, this is not convenient and
>> also unclear for users, who will build their own version of OVS from sources
>> and also for other distributions, who builds only one OVS package.
>> Unclear it is because these cflags are passed silently, so the user could
>> not even know that their OVS build doesn't benefit from the hardware they
>> have.
> 
> There will always be some compiler flag that is "silently passed",
> simply because DPDK requires it. You are just swapping one that is
> guaranteed to work because it's defined at the point where it's needed,
> with one randomly picked that might work today, but will bitrot before
> long.

Let's hope that DPDK project will remove this kind of dependencies from
header files someday.  Raising the lower requirement for the CPU without
any real reason doesn't sound good to me.

> 
>> I do not understand why stripping of '-march' will add any extra work for
>> Debian/Ubuntu maintainers.  If it's possible to build OVS with current
>> version of DPDK in upstream CI, why this should be a problem for a
>> downstream OVS packages?
> 
> Because things will change in one way or the other, and whatever
> override you decide now to pick will bitrot, so instead of "just
> working" OVS will break and cause yet more back-and-forth and busywork
> on our side. And the considerable time spent to make it "just work"
> will have been for naught.

If solution "just work", you don't need workarounds for it.  Current solution
does _not_ "just work".  It breaks running openvswitch on older CPUs.  That is
why we need workarounds.  Once it "just work" we'll gladly remove them.

>> If you really want to have this -march flag, you could revert the stripping
>> part while building an OVS package.  This should be much easier than
>> educating every single user that there are unnecessary flags passed from
>> DPDK pkg-config and how to override them.  And there are lots of users who
>> builds their own OVS packages.
>> As an option, we could add '--do-not-strip-dpdk-march' configure flag, for
>> your convenience.
>>
>> From the OVS upstream perspective, we're evaluating and testing every
>> single DPDK stable release (.1, .2, ...) before actually recommending it
>> to be used with OVS (DPDK frequently has incompatible things/regressions in
>> their stable releases).  This way upstream OVS will detect build failures
>> related to meson/pkg-config and fix them before switching to a new version
>> of DPDK.  So, there should be no issues.  Same for runtime issues, but I
>> doubt there are any critical runtime dependencies from the compiler flags
>> in DPDK headers.
> 
> Well, all DPDK stable releases are ran through the OVS project
> regression tests before release (for which we are very grateful), so if
> you say that there are frequent regressions despite that, it would
> suggest that the tests you have are not enough.

OVS project has only build tests that are checked by our CI.  No runtime
tests there.  Most of the validation before upgrading to a new stable DPDK
version is done manually by Intel engineers.  Automatic tests done by NHU
lab are very simple and covers only 1-2 trivial cases.
There is a system-dpdk testsuite, but it only covers few very simple cases
too and I'm not sure if anyone runs this testsuite automatically.

OVS really needs more DPDK related tests, more automatic DPDK tests.
If anyone wants to contribute their time to write more automatic tests,
especially ones that doesn't require any specific hardware, they are
very welcome.  More reviewers for such changes are very welcome too.

Best regards, Ilya Maximets.