From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1041042D22; Thu, 22 Jun 2023 11:27:22 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EF50540EE1; Thu, 22 Jun 2023 11:27:21 +0200 (CEST) Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) by mails.dpdk.org (Postfix) with ESMTP id 9C22D406B8 for ; Thu, 22 Jun 2023 11:27:20 +0200 (CEST) Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-3fa71db4208so4851605e9.0 for ; Thu, 22 Jun 2023 02:27:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1687426040; x=1690018040; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=AxQNQ39/8EGJ81gZOHSFavnvEYsTXequ6GB31rkuX08=; b=uRzST47/N8Q+xiChr5Onb7nEchKasHrXK9Vg3KoULMIWUwPv5m3I1IAxagCcTbuBPN zK5d1GQDFhBHNl8CVHmgNT68kGTWjCH5FYhNhcKBCeD8qrnRggmsKUAtw1B79XAtNWaV SuelplJyo0eT+cvsfJo3z8ATbqE0OEABDR/SB1O7fXgPyRwECkmfdP9GcehLYtAs95Hw 6Lv4T6ZTqXl0xzD4rnu26NpnutteXIC1czMbS2RFzBeEo68ZKJOuZJqEJUifIzX8Am8f A31P+tFlCF3vcPjrwHuX/9wLVZG3QQJkXSAAwsFq5d1jJcQGTckMX+t11lQwtgqPbgM7 XHgg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687426040; x=1690018040; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=AxQNQ39/8EGJ81gZOHSFavnvEYsTXequ6GB31rkuX08=; b=Ndc39zHl174lbgPTDcta1H0ZtkONalTbxsOsUkFcsyYxlqR0EYgdeplLBENG7WqReF DFe50jvVyv5vwmuJitZXOPAIItbZNdg8Efib7AchflAkkS/i+2NP0Ba29TBvVWV9jhLG HuCyxjrjlHpdTDdbNCkX2tfdFIfK0n/dZYa+Qi+D2Uy+lvbvkyQYK3aJ7d/KwdwkF7wj g9sed04j2ozn69VxLUBqEgwJsB7RwwhlV+JMqZn7Iphlk5POTBsJuu85JRQsI9/d4kjM Li7QzXlHhO00KDNtKVIv3S1XgKbsl90NLVIe4tu5kDjvLlCxwI6TjJmzaAHXg02hz2V7 HWCA== X-Gm-Message-State: AC+VfDwjsPuPAJS+FzkTVQ/cm5u+r0ycU79vbPoNv/FzoaKWM5GmHTCj 3ys0rtmfZYFTGEyeBntf17FFn3cIAl0NOWrEtivHosj+OJKaYKkfemw= X-Google-Smtp-Source: ACHHUZ5BHKSgDCAecywRmCHvEkZ7UX9Nxbg3TPAW0yp5wUsfwl2zmSAqXXzzUvC6HRAr2mvmpLKEcd/iY4vEJbP0luE= X-Received: by 2002:a05:600c:2942:b0:3f7:5d:4a06 with SMTP id n2-20020a05600c294200b003f7005d4a06mr1017390wmd.1.1687426040255; Thu, 22 Jun 2023 02:27:20 -0700 (PDT) MIME-Version: 1.0 References: <20211117112847.7362-6-david.marchand@redhat.com> <23115010.6Emhk5qWAg@thomas> In-Reply-To: From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Thu, 22 Jun 2023 11:27:08 +0200 Message-ID: Subject: Re: [PATCH v3] build: select optional libraries To: David Marchand Cc: Bruce Richardson , Thomas Monjalon , dev@dpdk.org, bluca@debian.org Content-Type: multipart/alternative; boundary="000000000000b6ef9b05feb47dac" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --000000000000b6ef9b05feb47dac Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Jun 21, 2023 at 11:54=E2=80=AFAM David Marchand wrote: > On Tue, Jun 20, 2023 at 5:05=E2=80=AFPM Bruce Richardson > wrote: > > > > On Tue, Jun 20, 2023 at 04:33:15PM +0200, Thomas Monjalon wrote: > > > 20/06/2023 11:03, Bruce Richardson: > > > > On Tue, Jun 20, 2023 at 10:48:50AM +0200, David Marchand wrote: > > > > > On Tue, Jun 20, 2023 at 10:45=E2=80=AFAM Bruce Richardson > > > > > wrote: > > > > > > > > > > I notice the change in behaviour for enabling the > deprecated libs. Is there > > > > > > > > > > any other change in behaviour for current users? > > > > > > > > > > > > > > > > > > The only change I see, is that this implementation breaks > enabling > > > > > > > > > deprecated libs via disable_libs. > > > > > > > > > It may break existing developer build directory and maybe > some > > > > > > > > > packaging scripts, this is why I am a bit puzzled. > > > > > > > > > > > > > > > > > > Relooking at the disable_libs option current > implementation, it seems > > > > > > > > > backward to pass a disable_libs option when you want to > build some > > > > > > > > > deprecated library. > > > > > > > > > It is more straightforward to request building libraries > via > > > > > > > > > -Denable_libs=3D explicitly or > -Denable_libs=3D* > > > > > > > > > implicitly. > > > > > > > > > > > > > > > > > > But again, we may be breaking something for people who > relied on this behavior. > > > > > > > > > > > > > > > > > > > > > > > > > That's what I expected, and I think that is ok. I just > wanted to check that > > > > > > > > the change in behaviour was only for the deprecated libs > case. > > > > > > > > > > > > > > Thomas, wdyt? > > > > > > > It requires some release note, at least. > > > > > > > > > > > > > I am assuming this is not targetting this release though, right= ? > Assuming > > > > > > 23.11, we can put in a deprecation note informing of the change > ahead of > > > > > > time too. > > > > > > > > > > I was hoping to get it in this release. > > > > > But I am fine with postponing and announcing the change beforehan= d. > > > > > > > > > Given the fact that we are likely changing behaviour, and the fact > that the > > > > deprecated libs makes it more complicated than the drivers one > (since we > > > > have always on, default on and default off cases to consider), I > think it's > > > > best we don't rush this. > > > > > > I'm not sure what is the best behaviour. > > > I tend to think such options should be simple to understand > > > with only 3 cases: > > > - no option -> default > > > - enable option -> only core mandatory and listed libraries > > > - disable option -> all but the listed libraries > > > It looks simpler to forbid having both enable and disable libraries. > > > > > > Would you be open to change the behaviour of the drivers options? > > > > > > > [reducing CC list a bit] > > > > As a further follow-up, I really think we need to move slowly and more > > carefully on this. While I can see the simplicity in disallowing the tw= o > > options to be specified, depending on how we go about choosing the > > enabling of the deprecated libs, we may want to keep support for allowi= ng > > both. > > I agree, we need more time on this topic, sorry for being pushy earlier > :-). > > > > > > Specifically, my current concern/thinking is: > > * David points out that using the "disabled_libs" options may not make > the > > most logic sense for *enabling* deprecated libs. > > * While that is true, I think the usability of enabling them via > > "enabled_libs" could be pretty terrible - unless we start adding more > > complications. Specifically, if someone wants to just enable KNI in t= he > > build using "enable_libs", specifying "-Denable_libs=3Dkni" will not = do > > what they want - sure it will enable kni, but will disable dozens of > > other parts of DPDK. > > * Therefore, I think keeping the disabling of deprecated parts of DPDK > via > > disabled_libs is easier on users - though agreed it is slightly less > > logical. However, if we forbid using enabled and disabled options > > together, that would mean that to switch to enabled libs style, the > user > > has to set both enabled libs, *and* clear the default disabled libs > option > > of the deprecated ones. > > * Therefore, right now I'm tending more towards something like the stat= us > > quo - disabling deprecated via disable option, but allowing both enab= le > > and disable options together. This hasn't caused us issues with drive= rs > > thus far, so I'd be hopeful for using it for libs. > > Mixing enable/disable_drivers has been possible since the introduction > of enable_drivers. > Looking at the code, it is unclear the intention was to make them > exclusive. > Is there no usecase for using both options in some soc configuration? > Juraj, do you have an opinion? > > I seem to remember that there wasn't any intention of using them together - I think I did it this way because it was essentially "free" (i.e. the ability to use them together was an extra feature on top of what we needed)= . The SoC considerations were for cross compilation between Arm microarchitectures/SoCs - use enable_drivers mainly to enable only a handful of relevant drivers for small SoC's when building on a server grade SoC. The same applies for disable_drivers - the server grade SoC where we're building may have extra dependencies that we may not want/need on the target (possibly server-grade) system. They don't need to be used in conjunction in these scenarios. > > > > > The other alternative, is we come up with: > > * another scheme for managing deprecated libs > > * a special keyword for enabled libs to cover the default case, that on= e > > can use to add on the deprecated libs, without having to call out eac= h > > and every other optional library. > > I think a separate option to control deprecated libraries would be better= . > This option would control whether the deprecation libs are part of the > optional libraries list. > The optional libraries list would then be evaluated according to > enable/disable_libs. > > I'll try this to see how it behaves. > > > -- > David Marchand > > --000000000000b6ef9b05feb47dac Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Wed, Jun 21, 2023 at 11:54=E2=80= =AFAM David Marchand <david= .marchand@redhat.com> wrote:
On Tue, Jun 20, 2023 at 5:05=E2=80=AFPM Bruce Richardso= n
<bruce.r= ichardson@intel.com> wrote:
>
> On Tue, Jun 20, 2023 at 04:33:15PM +0200, Thomas Monjalon wrote:
> > 20/06/2023 11:03, Bruce Richardson:
> > > On Tue, Jun 20, 2023 at 10:48:50AM +0200, David Marchand wro= te:
> > > > On Tue, Jun 20, 2023 at 10:45=E2=80=AFAM Bruce Richards= on
> > > > <bruce.richardson@intel.com> wrote:
> > > > > > > > > I notice the change in behavio= ur for enabling the deprecated libs. Is there
> > > > > > > > > any other change in behaviour = for current users?
> > > > > > > >
> > > > > > > > The only change I see, is that this= implementation breaks enabling
> > > > > > > > deprecated libs via disable_libs. > > > > > > > > It may break existing developer bui= ld directory and maybe some
> > > > > > > > packaging scripts, this is why I am= a bit puzzled.
> > > > > > > >
> > > > > > > > Relooking at the disable_libs optio= n current implementation, it seems
> > > > > > > > backward to pass a disable_libs opt= ion when you want to build some
> > > > > > > > deprecated library.
> > > > > > > > It is more straightforward to reque= st building libraries via
> > > > > > > > -Denable_libs=3D<deprecated_lib&= gt; explicitly or -Denable_libs=3D*
> > > > > > > > implicitly.
> > > > > > > >
> > > > > > > > But again, we may be breaking somet= hing for people who relied on this behavior.
> > > > > > > >
> > > > > > >
> > > > > > > That's what I expected, and I think = that is ok. I just wanted to check that
> > > > > > > the change in behaviour was only for the= deprecated libs case.
> > > > > >
> > > > > > Thomas, wdyt?
> > > > > > It requires some release note, at least.
> > > > > >
> > > > > I am assuming this is not targetting this release = though, right? Assuming
> > > > > 23.11, we can put in a deprecation note informing = of the change ahead of
> > > > > time too.
> > > >
> > > > I was hoping to get it in this release.
> > > > But I am fine with postponing and announcing the change= beforehand.
> > > >
> > > Given the fact that we are likely changing behaviour, and th= e fact that the
> > > deprecated libs makes it more complicated than the drivers o= ne (since we
> > > have always on, default on and default off cases to consider= ), I think it's
> > > best we don't rush this.
> >
> > I'm not sure what is the best behaviour.
> > I tend to think such options should be simple to understand
> > with only 3 cases:
> > - no option -> default
> > - enable option -> only core mandatory and listed libraries > > - disable option -> all but the listed libraries
> > It looks simpler to forbid having both enable and disable librari= es.
> >
> > Would you be open to change the behaviour of the drivers options?=
> >
>
> [reducing CC list a bit]
>
> As a further follow-up, I really think we need to move slowly and more=
> carefully on this. While I can see the simplicity in disallowing the t= wo
> options to be specified, depending on how we go about choosing the
> enabling of the deprecated libs, we may want to keep support for allow= ing
> both.

I agree, we need more time on this topic, sorry for being pushy earlier :-)= .


>
> Specifically, my current concern/thinking is:
> * David points out that using the "disabled_libs" options ma= y not make the
>=C2=A0 =C2=A0most logic sense for *enabling* deprecated libs.
> * While that is true, I think the usability of enabling them via
>=C2=A0 =C2=A0"enabled_libs" could be pretty terrible - unless= we start adding more
>=C2=A0 =C2=A0complications. Specifically, if someone wants to just enab= le KNI in the
>=C2=A0 =C2=A0build using "enable_libs", specifying "-Den= able_libs=3Dkni" will not do
>=C2=A0 =C2=A0what they want - sure it will enable kni, but will disable= dozens of
>=C2=A0 =C2=A0other parts of DPDK.
> * Therefore, I think keeping the disabling of deprecated parts of DPDK= via
>=C2=A0 =C2=A0disabled_libs is easier on users - though agreed it is sli= ghtly less
>=C2=A0 =C2=A0logical. However, if we forbid using enabled and disabled = options
>=C2=A0 =C2=A0together, that would mean that to switch to enabled libs s= tyle, the user
>=C2=A0 =C2=A0has to set both enabled libs, *and* clear the default disa= bled libs option
>=C2=A0 =C2=A0of the deprecated ones.
> * Therefore, right now I'm tending more towards something like the= status
>=C2=A0 =C2=A0quo - disabling deprecated via disable option, but allowin= g both enable
>=C2=A0 =C2=A0and disable options together. This hasn't caused us is= sues with drivers
>=C2=A0 =C2=A0thus far, so I'd be hopeful for using it for libs.

Mixing enable/disable_drivers has been possible since the introduction
of enable_drivers.
Looking at the code, it is unclear the intention was to make them exclusive= .
Is there no usecase for using both options in some soc configuration?
Juraj, do you have an opinion?


I seem to remember that there wasn'= ;t any intention of using them together - I think I did it this way because= it was essentially "free" (i.e. the ability to use them together= was an extra feature on top of what we needed).

T= he SoC considerations were for cross compilation between Arm microarchitect= ures/SoCs - use enable_drivers mainly to enable only a handful of relevant = drivers for small SoC's when building on a server grade SoC. The same a= pplies for disable_drivers - the server grade SoC where we're building = may have extra dependencies that we may not want/need on the target (possib= ly server-grade) system. They don't need to be used in conjunction in t= hese scenarios.
=C2=A0

>
> The other alternative, is we come up with:
> * another scheme for managing deprecated libs
> * a special keyword for enabled libs to cover the default case, that o= ne
>=C2=A0 =C2=A0can use to add on the deprecated libs, without having to c= all out each
>=C2=A0 =C2=A0and every other optional library.

I think a separate option to control deprecated libraries would be better.<= br> This option would control whether the deprecation libs are part of the
optional libraries list.
The optional libraries list would then be evaluated according to
enable/disable_libs.

I'll try this to see how it behaves.


--
David Marchand

--000000000000b6ef9b05feb47dac--