From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id E4628425E7;
	Wed, 20 Sep 2023 08:00:58 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id AA3EC402DD;
	Wed, 20 Sep 2023 08:00:58 +0200 (CEST)
Received: from mail-qv1-f54.google.com (mail-qv1-f54.google.com
 [209.85.219.54]) by mails.dpdk.org (Postfix) with ESMTP id C3F3040277
 for <dev@dpdk.org>; Wed, 20 Sep 2023 08:00:56 +0200 (CEST)
Received: by mail-qv1-f54.google.com with SMTP id
 6a1803df08f44-65643a83758so22953126d6.0
 for <dev@dpdk.org>; Tue, 19 Sep 2023 23:00:56 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=semihalf.com; s=google; t=1695189656; x=1695794456; darn=dpdk.org;
 h=cc:to:subject:message-id:date:from:in-reply-to:references
 :mime-version:from:to:cc:subject:date:message-id:reply-to;
 bh=bM+uzpAayQfP8P+Zaa3ecPtFYKqT/zLlue2eU/ys/wM=;
 b=qDumxztRW1qHl6mLAPtATIJaRnb0OVUfxJ+5DyeCz8bVn6TS2F6olA/zW6DqAcbF6V
 cY2LvKmx4tlhwP2Hm/+Xq959DD4A4xaGs3IZX1cgOOt1rPo9G7/gwdilVs94hm7Kk1of
 t1WQNgOtJ0EvW8NBpaW5do+qdh4/s+e+GP35tashLffXTkLAl0E5hR2thnYYWNdGWa44
 qXvFecLdaIvvehojuX4vCEjklQd5nO0FuEY60QL2jEeoopODWJmVuPN4UTHoduJ71MnG
 QbRBB9Bc4u2RtCQof0rY6/4anMcmoUcYF3aJ+wT+UE+8TDRv6oQ6EKwgEzTMLIWybYbm
 AlZA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1695189656; x=1695794456;
 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=bM+uzpAayQfP8P+Zaa3ecPtFYKqT/zLlue2eU/ys/wM=;
 b=lM2WmMdJuLs2FpjKgR61aN6iBizk18Flj+EiB1tqbLt1U34ojCQo/F2nRi8LHyOPFs
 ytKdf+NfBmgZqL3aJC3GDB+E4NGvYam8+NU2in6e7pZtwDMvUwhwkL+6nlyZovNA/5Ol
 CIS458KidpgH7GqTA/JEuUgeBZI3IaaOar/Qhf3BmD8kiW3GpE+Wp7VdjfGbh3/MeJsx
 Fg+Pw/mtLG8Ai7uff/CjjFSt2m6F1rB0e1XxCji/27rk4a1HgTn7Fnpi7d+0gcyHc4dR
 gzgSRDDyTaGLk0Ri+NUck6d9U0669yd2nXRlUK2znbTdn0Q59lscnaalM240FhUcgTNB
 lvag==
X-Gm-Message-State: AOJu0Yxf2vvi85w6CX856QLDvLx5T5YfYbt26bSx1bp3N1msyg1ZMNDm
 miVvXhOL6F4lDV7FuY31eS7aASezRIOu5ndmCYT4kg==
X-Google-Smtp-Source: AGHT+IHz8/f/luOyMihGMj4CB9qbJFQZtifLXZ9NwsWuLzeSCLAaISICXlhd90rr8Vv9oO26VdRf4rYNq+loDgQN7Lg=
X-Received: by 2002:a05:6214:459a:b0:656:5441:a13f with SMTP id
 op26-20020a056214459a00b006565441a13fmr1788457qvb.45.1695189656052; Tue, 19
 Sep 2023 23:00:56 -0700 (PDT)
MIME-Version: 1.0
References: <20230802211150.939121-1-sivaprasad.tummala@amd.com>
 <20230811060755.481572-1-sivaprasad.tummala@amd.com>
 <20230811060755.481572-2-sivaprasad.tummala@amd.com>
 <CALVGJWJoCsnOk_b4+uXCjYEs1krN-s=wEerpJWuTFnaY_nsrLQ@mail.gmail.com>
 <CAJFAV8wh9XigLwMkHu5bc0WRKPbSmi1veg3jeiEjZfXKu8qekQ@mail.gmail.com>
In-Reply-To: <CAJFAV8wh9XigLwMkHu5bc0WRKPbSmi1veg3jeiEjZfXKu8qekQ@mail.gmail.com>
From: =?UTF-8?Q?Stanis=C5=82aw_Kardach?= <kda@semihalf.com>
Date: Wed, 20 Sep 2023 08:00:41 +0200
Message-ID: <CALVGJWLebc=w9RC_Sywo1Ou7B08EQrXCOwco_G+tgqYvx=p8ag@mail.gmail.com>
Subject: Re: [PATCH v2 2/2] eal: remove NUMFLAGS enumeration
To: David Marchand <david.marchand@redhat.com>
Cc: Sivaprasad Tummala <sivaprasad.tummala@amd.com>,
 Ruifeng Wang <ruifeng.wang@arm.com>, 
 Min Zhou <zhoumin@loongson.cn>, David Christensen <drc@linux.vnet.ibm.com>, 
 Bruce Richardson <bruce.richardson@intel.com>, 
 Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>, dev <dev@dpdk.org>, 
 Ferruh Yigit <ferruh.yigit@amd.com>, Thomas Monjalon <thomas@monjalon.net>
Content-Type: multipart/alternative; boundary="00000000000046b8560605c419de"
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

--00000000000046b8560605c419de
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Sep 19, 2023 at 4:47=E2=80=AFPM David Marchand <david.marchand@redh=
at.com>
wrote:
<snip>
> > Also I see you're still removing the RTE_CPUFLAG_NUMFLAGS (what I call
a last element canary). Why? If you're concerned with ABI, then we're
talking about an application linking dynamically with DPDK or talking via
some RPC channel with another DPDK application. So clashing with this
definition does not come into question. One should rather use
rte_cpu_get_flag_enabled().
> > Also if you want to introduce new features, one would add them yo the
rte_cpuflags headers, unless you'd like to not add those and keep an
undocumented list "above" the last defined element.
> > Could you explain a bit more Your use-case?
>
> Hey Stanislaw,
>
> Talking generically, one problem with such pattern (having a LAST, or
> MAX enum) is when an array sized with such a symbol is exposed.
> As I mentionned in the past, this can have unwanted effects:
>
https://patchwork.dpdk.org/project/dpdk/patch/20230919140430.3251493-1-davi=
d.marchand@redhat.com/
I agree, though I'd argue "LAST" and "MAX" semantics are a bit different.
"LAST" delimits the known enumeration territory while "MAX" is more of a
`constepxr` value type.
>
> Another issue is when an existing enum meaning changes: from the
> application pov, the (old) MAX value is incorrect, but for the library
> pov, a new meaning has been associated.
> This may trigger bugs in the application when calling a function that
> returns such an enum which never return this MAX value in the past.
>
> For at least those two reasons, removing those canary elements is
> being done in DPDK.
>
> This specific removal has been announced:
>
https://patchwork.dpdk.org/project/dpdk/patch/20230919140430.3251493-1-davi=
d.marchand@redhat.com/
Thanks for pointing this out but did you mean to link to the patch again
here?
>
> Now, practically, when I look at the cpuflags API, I don't see us
> exposed to those two issues wrt rte_cpu_flag_t, so maybe this change
> is unneeded.
> But on the other hand, is it really an issue for an application to
> lose this (internal) information?
I doubt it, maybe it could be used as a sanity check for choosing proper
functors in the application. Though the initial description of the reason
behind this patch was to not break the ABI and I don't think it does that.
What it does is enforces users to use explicit cpu flag values which is a
good thing. Though if so, then it should be stated in the commit
description.
>

--00000000000046b8560605c419de
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"auto">On Tue, Sep 19, 2023 at 4:47=E2=80=AFPM David Marchand &l=
t;<a href=3D"mailto:david.marchand@redhat.com" rel=3D"noreferrer noreferrer=
 noreferrer" target=3D"_blank">david.marchand@redhat.com</a>&gt; wrote:<br>
&lt;snip&gt;<br>
&gt; &gt; Also I see you&#39;re still removing the RTE_CPUFLAG_NUMFLAGS (wh=
at I call a last element canary). Why? If you&#39;re concerned with ABI, th=
en we&#39;re talking about an application linking dynamically with DPDK or =
talking via some RPC channel with another DPDK application. So clashing wit=
h this definition does not come into question. One should rather use rte_cp=
u_get_flag_enabled().<br>
&gt; &gt; Also if you want to introduce new features, one would add them yo=
 the rte_cpuflags headers, unless you&#39;d like to not add those and keep =
an undocumented list &quot;above&quot; the last defined element.<br>
&gt; &gt; Could you explain a bit more Your use-case?<br>
&gt;<br>
&gt; Hey Stanislaw,<br>
&gt;<br>
&gt; Talking generically, one problem with such pattern (having a LAST, or<=
br>
&gt; MAX enum) is when an array sized with such a symbol is exposed.<br>
&gt; As I mentionned in the past, this can have unwanted effects:<br>
&gt; <a href=3D"https://patchwork.dpdk.org/project/dpdk/patch/2023091914043=
0.3251493-1-david.marchand@redhat.com/" rel=3D"noreferrer noreferrer norefe=
rrer noreferrer" target=3D"_blank">https://patchwork.dpdk.org/project/dpdk/=
patch/20230919140430.3251493-1-david.marchand@redhat.com/</a><br>
I agree, though I&#39;d argue &quot;LAST&quot; and &quot;MAX&quot; semantic=
s are a bit different. &quot;LAST&quot; delimits the known enumeration terr=
itory while &quot;MAX&quot; is more of a `constepxr` value type.<br>
&gt;<br>
&gt; Another issue is when an existing enum meaning changes: from the<br>
&gt; application pov, the (old) MAX value is incorrect, but for the library=
<br>
&gt; pov, a new meaning has been associated.<br>
&gt; This may trigger bugs in the application when calling a function that<=
br>
&gt; returns such an enum which never return this MAX value in the past.<br=
>
&gt;<br>
&gt; For at least those two reasons, removing those canary elements is<br>
&gt; being done in DPDK.<br>
&gt;<br>
&gt; This specific removal has been announced:<br>
&gt; <a href=3D"https://patchwork.dpdk.org/project/dpdk/patch/2023091914043=
0.3251493-1-david.marchand@redhat.com/" rel=3D"noreferrer noreferrer norefe=
rrer noreferrer" target=3D"_blank">https://patchwork.dpdk.org/project/dpdk/=
patch/20230919140430.3251493-1-david.marchand@redhat.com/</a><br>
Thanks for pointing this out but did you mean to link to the patch again he=
re?<br>
&gt;<br>
&gt; Now, practically, when I look at the cpuflags API, I don&#39;t see us<=
br>
&gt; exposed to those two issues wrt rte_cpu_flag_t, so maybe this change<b=
r>
&gt; is unneeded.<br>
&gt; But on the other hand, is it really an issue for an application to<br>
&gt; lose this (internal) information?<br>I doubt it, maybe it could be use=
d as a sanity check for choosing proper functors in the application. Though=
 the initial description of the reason behind this patch was to not break t=
he ABI and I don&#39;t think it does that. What it does is enforces users t=
o use explicit cpu flag values which is a good thing. Though if so, then it=
 should be stated in the commit description.<div dir=3D"auto">
&gt;</div></div>

--00000000000046b8560605c419de--