Thread (463 messages) 463 messages, 15 authors, 2021-04-15

Re: [dpdk-dev] [PATCH v16 1/3] build: disable/enable drivers in Arm builds

From: Honnappa Nagarahalli <hidden>
Date: 2021-03-09 19:55:01

<snip>
quoted
quoted
quoted
quoted
quoted
quoted
On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
quoted
Honnappa, Thomas, Bruce, Jerin, you've comments in the past.
Do you have
any further input?
quoted
I think we just need to agree on the allowlist/blocklist
mechanism. The current
commit allows specifying either an allowlist or a blocklist,
but not
both.
quoted
quoted
quoted
quoted
However, it would possible to implement specifying both -
first we'll allow what's in allowlist and then we'll remove
from that set what's
in blocklist.
quoted
quoted
Thoughts?
quoted
If we have both, I think limiting to only one is by far the sanest
option.
quoted
quoted
quoted
+1
quoted
quoted
quoted
I'm not fully convinced by the need to have both, since the
blocklist allows wildcarding and exception cases. For
example "net/[!i]*" will blocklist all net drivers except
those starting with an "i". Admittedly, for usability
purposes having an allowlist might
work better.
quoted
quoted
If we only want to build a handful of drivers then the list
could be very long
(which was the original reason behind having an allowlist), such as
here:
quoted
quoted
quoted
quoted
quoted
https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/externa
l/
pa
ck ag
es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
quoted
quoted
quoted
quoted
quoted
An allowlist could also help with maintenance - users won't
need to add
new drivers to their blocklists (if that's what users need, like
in the case of VPP).

+1 for allowlist.
May be I am missing something here. By creating an allowlist, does
it mean drivers are disabled (from compilation) by default? For a
server platform, where almost all the drivers can be compiled,
does the allowlist
contain all the drivers?
quoted
If no allowlist is specified, then everythin will be built - nothing
will be filtered.
That's confusing.
If a platform like bluefield has an allow list, a new PMD that gets
added will not be compiled for that platform unless someone explicitly adds
it to the allow list.
quoted
Is my understanding correct?
Yes.
With this it becomes very easy to skip compiling on a platform.
quoted
Whereas if the bluefield platform had a block list, then the new PMD
would be compiled for bluefield platform.
Again, yes.

Supporting both would give us the option to choose between the two
behaviors.
quoted
quoted
quoted
If we assume by default everything should compile on Arm platform,
but allow for few exceptions (where things are really painful to
fix, for
ex: compiler needs to be fixed), having a blocklist should be
shorter/better?
quoted
quoted
quoted
The blocklist is, I think, agreed upon by everyone. The question is
whether we want to support the allowlist alongside it and there seem
to be enough reasons to do that.
Sorry, may be this is answered already, but, what additional benefit
does allowlist provide over the blocklist?
VPP could use it:
https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packages/dpdk
.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD

They're disabling almost everything so an allowlist would fit there. And they
won't need to update the list when a new driver is added (which they won't
need).
This is different from how we started this discussion. The current discussion was for DPDK internal use. But the one you are referencing above is for users of DPDK. I am fine for providing the allow list for the users of DPDK. But for DPDK internal, I think block list is enough.
I think it was Jerin who suggested the allowlist. I don't know of an Arm
usecase for it, but maybe he has an example.
quoted
quoted
quoted
By having an allowlist, we will end up with a large part of the
code that might not compile on Arm platforms.
quoted
quoted
quoted
One final thought, if we add a driver allowlist for cross
files, should we also add one as a top-level meson option
also for
consistency?
quoted
quoted
quoted
quoted
This definitely makese sense. I was thinking about this and
wasn't sure
whether I should put it into this commit or a separate one. The
commit evolved a bit and now that it's just an implementation of
an allow/blocklist it makes sense to include a meson option in
it I think
- I'll put it into the next version.
quoted
quoted
/Bruce
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help