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 66D67425E4; Wed, 20 Sep 2023 09:35:04 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DD75040E96; Wed, 20 Sep 2023 09:35:03 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 2CC2640E78 for ; Wed, 20 Sep 2023 09:35:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695195301; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mV6KNV5QRbu3O8LJsd7JReDCSzWTVTR+z4IWMDfoBJo=; b=d40HzDodwSA21osvCnha710gL/jyyPlvfvR2QlHimRIh90PGuE0VKxII5JNcOIe0EbIIwx PhjOhi4t5Rb3mDGc+HksZeIMYzaFwywRheOx2QCmJuSQAI2EcFdCQrbiCoeNktQLSOXaf2 gu3pEktQAidtRfjlj01cGWahtyuhITw= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-164-ZxPrHHu2Of2lUiPdIM6iVA-1; Wed, 20 Sep 2023 03:35:00 -0400 X-MC-Unique: ZxPrHHu2Of2lUiPdIM6iVA-1 Received: by mail-lj1-f197.google.com with SMTP id 38308e7fff4ca-2ba1949656bso81287391fa.0 for ; Wed, 20 Sep 2023 00:35:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695195299; x=1695800099; h=content-transfer-encoding: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=mV6KNV5QRbu3O8LJsd7JReDCSzWTVTR+z4IWMDfoBJo=; b=Ol31XVLPuPEbD0U4/FGJoeZXaqIFpTw/YIyaR1CUxOKtdTHlwdjOADwmpnegRHXsjI XNEEXQKyi1yinZjd5QAmirYVDkW1ZD5WsFjREMeQvQl0w5Wrp+We39qV76uqvTnWSIQY 1D+Z3aPqLI/RqjTpYQVkb1skriX5qoK82bNh1uyc0utiGgNSXTcVuN5bgxkcEb8ivX92 mNaDBupYR/YPiD3Hla4VdtUQoPZHJRkoUVuPsTeeugIvDAkS3QHXpHzbuNNE6sgKaSMI U9mMTcGFwjHHkd1pujGI/DSnBCt6jtzNMHkAzfkFvl1F6v9sZ+4vNAQzPYg4sT5ST8I6 73Gw== X-Gm-Message-State: AOJu0YzShcRO1F61GNDdTcnr54rtofOwoWWCUoufMT04N0W2j0phTXyf EYDmavg8fYj1TmEzCyFKyJfk6nToIp8q/xKKRKl22zZgook3PnrvidZdHb4rRU0ZPszL4kNjZkx Ko8VZNFLXWFr5qC4xWOE= X-Received: by 2002:a2e:9589:0:b0:2bf:e61b:c980 with SMTP id w9-20020a2e9589000000b002bfe61bc980mr1481015ljh.8.1695195298901; Wed, 20 Sep 2023 00:34:58 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEuGPhrwJs+bT0vwtHdmLUasyDh+VrMSQwvGwFVRvm/lnzj2733pAdxwkdICdS5qVdwIVu+o9HHVM4ouW8ajsc= X-Received: by 2002:a2e:9589:0:b0:2bf:e61b:c980 with SMTP id w9-20020a2e9589000000b002bfe61bc980mr1480997ljh.8.1695195298571; Wed, 20 Sep 2023 00:34:58 -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> In-Reply-To: From: David Marchand Date: Wed, 20 Sep 2023 09:34:47 +0200 Message-ID: Subject: Re: [PATCH v2 2/2] eal: remove NUMFLAGS enumeration To: =?UTF-8?Q?Stanis=C5=82aw_Kardach?= , Sivaprasad Tummala Cc: Ruifeng Wang , Min Zhou , David Christensen , Bruce Richardson , Konstantin Ananyev , dev , Ferruh Yigit , Thomas Monjalon X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Wed, Sep 20, 2023 at 8:01=E2=80=AFAM Stanis=C5=82aw Kardach wrote: > > On Tue, Sep 19, 2023 at 4:47=E2=80=AFPM David Marchand wrote: > > > > Also I see you're still removing the RTE_CPUFLAG_NUMFLAGS (what I cal= l a last element canary). Why? If you're concerned with ABI, then we're tal= king about an application linking dynamically with DPDK or talking via some= RPC channel with another DPDK application. So clashing with this definitio= n does not come into question. One should rather use rte_cpu_get_flag_enabl= ed(). > > > 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 undoc= umented 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-= david.marchand@redhat.com/ Argh... who broke copy/paste in my browser ?! Wrt to MAX and arrays, I wanted to point at: http://inbox.dpdk.org/dev/CAJFAV8xs5CVdE2xwRtaxk5vE_PiQMV5LY5tKStk3R1gOuRTs= Uw@mail.gmail.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-= david.marchand@redhat.com/ > Thanks for pointing this out but did you mean to link to the patch again = here? Sorry, same here, bad copy/paste :-(. The intended link is: https://git.dpdk.org/dpdk/commit/?id=3D5da7c13521 The deprecation notice was badly formulated and this patch here is consistent with it. > > > > 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 b= ehind this patch was to not break the ABI and I don't think it does that. W= hat it does is enforces users to use explicit cpu flag values which is a go= od thing. Though if so, then it should be stated in the commit description. I agree. Siva, can you work on a new revision? Thanks. --=20 David Marchand