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 7653D43C24; Wed, 28 Feb 2024 11:43:06 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BF3D4410EA; Wed, 28 Feb 2024 11:43:05 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 40D4C406FF for ; Wed, 28 Feb 2024 11:43:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709116983; 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=2ZTFRXSOfNSIUycgNJg9c+/hYy8Ygwih4UVEI8wfX7A=; b=Gy+sY4xnWtK+VpMm27P8ANyg1FjuIHxeQY/riokLQQD3xrgfGWRWnhXf7tJTtOOWvPXAsd G+iHGUk7wpVrPHo7TaaefiquqafpZJMkNGptURSsWCWZUwGHSiBfeNjFizawg3zrmVIUPH +oyFE5ytHNw75J2xR/KnR9TtGIc0UTU= Received: from mail-lj1-f199.google.com (mail-lj1-f199.google.com [209.85.208.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-127-B--d2AtzM6GvWaK501IOww-1; Wed, 28 Feb 2024 05:43:02 -0500 X-MC-Unique: B--d2AtzM6GvWaK501IOww-1 Received: by mail-lj1-f199.google.com with SMTP id 38308e7fff4ca-2d298d601adso17010381fa.2 for ; Wed, 28 Feb 2024 02:43:01 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709116980; x=1709721780; 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=2ZTFRXSOfNSIUycgNJg9c+/hYy8Ygwih4UVEI8wfX7A=; b=lCxFiwMFVw/e/NnGep/2xZqlIgpEjtiWbUUBZeUnIrc2IH3QSebqDt7ZcFs8BCbEMm nxV47QSirq5PkkOY2RyWuqnF3LG9zXU/kdHaeZNg7vIZD7d41YiZHRU1Easp4iHNZk5f 6Cdf03U/JJOqA95oHbecPCq37dhx7s+AK4ngtX1Ui0goqEaMgr7AK8nTIGehnulzPPI8 +RZCySZ5h7UcKjYBXm2e2GCpZlqMbERV8VHIhMVwZ5at8PkF4vf8zEGlf49aAIGcujtp qqDr5MNZlyyUwGI1Q8SukcL0wv40eVA8LEGhP/c5AKNeV4i9WjISZHiyuEMTUemdSFia bEAA== X-Forwarded-Encrypted: i=1; AJvYcCW7RCJ+KEWMlT0+oCAkmusaWLT2kTEui7B+TY1FcyUnHSaFMMXYaVwoiyQsVslOlCVep3NFTyyZho7FrvQ= X-Gm-Message-State: AOJu0Yy3Wd+mT5AQPnAjY0MNvQkrsyIbMR2xQcXjGUE32WVk2KAhNv9L ujCqL9WeOSJ1EmGD3gVaRKv9qkJPPP5KILcCWODO8dXhq9HCeC3tgVgXhgN2oo2iSfmanSwB6Xm 8ZNLjUr0hSGquJe0KAwD/8igePt9nXrb3JzbDMK4ee696WGO7WpPsGhEyE/gKJRy1YeE+J0wBJJ RH48stOB1zNUoDV/4= X-Received: by 2002:a2e:a4cf:0:b0:2d2:64c9:214b with SMTP id p15-20020a2ea4cf000000b002d264c9214bmr7107577ljm.50.1709116980619; Wed, 28 Feb 2024 02:43:00 -0800 (PST) X-Google-Smtp-Source: AGHT+IG6SZbQcVZOePkGaJOYQrmgYytuFNElUWCaqIOm3s+BOFfBDJ5/c5rNu0BA661O4F0qVa/MqN0LRZkcyIQc7V8= X-Received: by 2002:a2e:a4cf:0:b0:2d2:64c9:214b with SMTP id p15-20020a2ea4cf000000b002d264c9214bmr7107555ljm.50.1709116980259; Wed, 28 Feb 2024 02:43:00 -0800 (PST) MIME-Version: 1.0 References: <1706657173-26166-1-git-send-email-roretzla@linux.microsoft.com> <1709012499-12813-1-git-send-email-roretzla@linux.microsoft.com> <1709012499-12813-21-git-send-email-roretzla@linux.microsoft.com> <20240227172306.GB16014@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> In-Reply-To: <20240227172306.GB16014@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: David Marchand Date: Wed, 28 Feb 2024 11:42:48 +0100 Message-ID: Subject: Re: [PATCH v6 20/23] mbuf: remove and stop using rte marker fields To: Tyler Retzlaff , Thomas Monjalon Cc: Dodji Seketeli , dev@dpdk.org, Ajit Khaparde , Andrew Boyer , Andrew Rybchenko , Bruce Richardson , Chenbo Xia , Chengwen Feng , Dariusz Sosnowski , David Christensen , Hyong Youb Kim , Jerin Jacob , Jie Hai , Jingjing Wu , John Daley , Kevin Laatz , Kiran Kumar K , Konstantin Ananyev , Maciej Czekaj , Matan Azrad , Maxime Coquelin , Nithin Dabilpuram , Ori Kam , Ruifeng Wang , Satha Rao , Somnath Kotur , Suanming Mou , Sunil Kumar Kori , Viacheslav Ovsiienko , Yisen Zhuang , Yuying Zhang , mb@smartsharesystems.com, ci@dpdk.org 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 Tue, Feb 27, 2024 at 6:23=E2=80=AFPM Tyler Retzlaff wrote: > > On Tue, Feb 27, 2024 at 04:18:10PM +0100, David Marchand wrote: > > Hello Dodji, > > > > On Tue, Feb 27, 2024 at 6:44=E2=80=AFAM Tyler Retzlaff > > wrote: > > > > > > RTE_MARKER typedefs are a GCC extension unsupported by MSVC. Remove > > > RTE_MARKER fields from rte_mbuf struct. > > > > > > Maintain alignment of fields after removed cacheline1 marker by placi= ng > > > C11 alignas(RTE_CACHE_LINE_MIN_SIZE). > > > > > > Update implementation of rte_mbuf_prefetch_part1() and > > > rte_mbuf_prefetch_part2() inline functions calculate pointer for > > > prefetch of cachline0 and cachline1 without using removed markers. > > > > > > Update static_assert of rte_mbuf struct fields to reference data_off = and > > > packet_type fields that occupy the original offsets of the marker > > > fields. > > > > > > Signed-off-by: Tyler Retzlaff > > > > This change is reported as a potential ABI change. > > > > For the context, this patch > > https://patchwork.dpdk.org/project/dpdk/patch/1709012499-12813-21-git-s= end-email-roretzla@linux.microsoft.com/ > > removes null-sized markers (those fields were using RTE_MARKER, see > > https://git.dpdk.org/dpdk/tree/lib/eal/include/rte_common.h#n583) from > > the rte_mbuf struct. > > I would argue this change do not impact ABI as the layout of the mbuf > > object is not impacted. > > It isn't a surprise that the change got flagged because the 0 sized > fields being removed probably not something the checker understands. > So no ABI change just API break (as was requested). > > > As reported by the CI: > > > > [C] 'function const rte_eth_rxtx_callback* > > rte_eth_add_first_rx_callback(uint16_t, uint16_t, rte_rx_callback_fn, > > void*)' at rte_ethdev.c:5768:1 has some indirect sub-type changes: > > parameter 3 of type 'typedef rte_rx_callback_fn' has sub-type chang= es: > > underlying type 'typedef uint16_t (typedef uint16_t, typedef > > uint16_t, rte_mbuf**, typedef uint16_t, typedef uint16_t, void*)*' > > changed: > > in pointed to type 'function type typedef uint16_t (typedef > > uint16_t, typedef uint16_t, rte_mbuf**, typedef uint16_t, typedef > > uint16_t, void*)': > > parameter 3 of type 'rte_mbuf**' has sub-type changes: > > in pointed to type 'rte_mbuf*': > > in pointed to type 'struct rte_mbuf' at rte_mbuf_core.h:4= 70:1: > > type size hasn't changed > > 4 data member deletions: > > 'RTE_MARKER cacheline0', at offset 0 (in bits) at > > rte_mbuf_core.h:467:1 > > 'RTE_MARKER64 rearm_data', at offset 128 (in bits) > > at rte_mbuf_core.h:490:1 > > 'RTE_MARKER rx_descriptor_fields1', at offset 256 > > (in bits) at rte_mbuf_core.h:517:1 > > 'RTE_MARKER cacheline1', at offset 512 (in bits) at > > rte_mbuf_core.h:598:1 > > no data member change (1 filtered); > > > > Error: ABI issue reported for abidiff --suppr > > /home/runner/work/dpdk/dpdk/devtools/libabigail.abignore > > --no-added-syms --headers-dir1 reference/usr/local/include > > --headers-dir2 install/usr/local/include > > reference/usr/local/lib/librte_ethdev.so.24.0 > > install/usr/local/lib/librte_ethdev.so.24.1 > > ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged > > this as a potential issue). > > > > Opinions? > > > > Btw, I see no way to suppress this (except a global [suppress_type] > > name =3D rte_mbuf)... > > I am unfamiliar with the ABI checker I'm afraid i have no suggestion to > offer. Maybe we can just ignore the failure for this one series when we > decide it is ready to be merged and don't suppress the checker? The ABI check compares a current build with a (cached) reference build. There is no "let's ignore this specific error" mechanism at the moment. And I suspect it would be non-trivial to add (parsing abidiff text output... brrr). Changing the check so that it compares against origin/main (for example) every time is doable *on paper*, but it would consume a lot of cpu for maintainers (like Thomas, Ferruh or me) and the CI. CI scripts would have to be updated too. Fow now, one thing we can do is to change the reference to point at the exact commit that introduces a change we know safe. This requires a little sync between people (maintainers / users of test-meson-builds.h) and UNH CI, but this is doable. On the other hand, by the time we merge this series, libabigail may have fixed this for us already? ;-) --=20 David Marchand