On Wed, Mar 10, 2021 at 1:12 PM Juraj Linkeš <juraj.lin...@pantheon.tech> wrote:
>
>
>
> > -----Original Message-----
> > From: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
> > Sent: Tuesday, March 9, 2021 8:55 PM
> > To: Juraj Linkeš <juraj.lin...@pantheon.tech>; Jerin Jacob
> > <jerinjac...@gmail.com>
> > Cc: Bruce Richardson <bruce.richard...@intel.com>; Ruifeng Wang
> > <ruifeng.w...@arm.com>; vcchu...@amazon.com; Dharmik Thakkar
> > <dharmik.thak...@arm.com>; hemant.agra...@nxp.com; Ajit Khaparde
> > (ajit.khapa...@broadcom.com) <ajit.khapa...@broadcom.com>;
> > ferruh.yi...@intel.com; abo...@pensando.io; lir...@marvell.com;
> > dev@dpdk.org; nd <n...@arm.com>; nd <n...@arm.com>
> > Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> >
> > <snip>
> > > >
> > > > > > > > >
> > > > > > > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > > > > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past.
> > > > > > > > > > Do you have
> > > > > > > > > any further input?
> > > > > > > > > >
> > > > > > > > > > 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.
> > > > > > > > > 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.
> > > > > > > > > Thoughts?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > If we have both, I think limiting to only one is by far
> > > > > > > > > the sanest
> > > option.
> > > > > > +1
> > > > > >
> > > > > > > > > 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.
> > > > > > > > >
> > > > > > > >
> > > > > > > > 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:
> > > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/exter
> > > > > > > > na
> > > > > > > > l/
> > > > > > > > pa
> > > > > > > > ck ag
> > > > > > > >
> > > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > > >
> > > > > > > > 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?
> > > > > >
> > > > >
> > > > > 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.
> > > > Is my understanding correct?
> > >
> > > Yes.
> > With this it becomes very easy to skip compiling on a platform.
> >
>
> It wouldn't be mandatory. Maybe I should've said we would be able to choose 
> between three behaviors - the current (where everything is built), with 
> allowlist or with blocklist.
>
> But maybe the worry is that someone will use the allowlist without fully 
> understanding the consequences?
>
> > >
> > > > 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.
> > >
> > > > >
> > > > > > 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?
> > > > > >
> > > > >
> > > > > 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/packag
> > > es/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.
> >
>
> That's an interesting suggestion. Jerin, what do you think? Why did you want 
> to have an allowlist? Would this work?

# The very reason why VPP chooses to have allow list so that they can
control what needs to include.
# Another use case is like, in SoCs have fixed internal devices, we
can have optimized build for that can have only allow list
of the drivers that care about
# For server market, block list makes sense
# For embedded SoC, allow list makes sense.
# Ideal situation is if we support both.
# I dont quite understand the above comments for internal use vs
external use. If it is exposed
as a meson option then I think, it does not matter. Right?

>
> > >
> > > 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.
> > >
> > > > >
> > > > > > By having an allowlist, we will end up with a large part of the
> > > > > > code that might not compile on Arm platforms.
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > 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?
> > > > > > > > >
> > > > > > > >
> > > > > > > > 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.
> > > > > > > >
> > > > > > > > > /Bruce
> > > > > > > >

Reply via email to