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 B811A43C23; Wed, 28 Feb 2024 11:43:05 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 92C1F406FF; 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 3C7BF402AE 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-25-S-6OiJz-PNmMJH3MNPboZw-1; Wed, 28 Feb 2024 05:43:02 -0500 X-MC-Unique: S-6OiJz-PNmMJH3MNPboZw-1 Received: by mail-lj1-f199.google.com with SMTP id 38308e7fff4ca-2d298d601adso17010361fa.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=mDX9XvhliXk+dipwNMmGnMlqw4Q8x8DSK6UKpbQ9rpeh4rhl4pHCwGry3HjMMMKF26 UQ0TXi4tXkgaPfljWEWQjwPaY3YQbrI2o1ODatKXaCJDiTulUJNkg5aamALp4G/4gHFN n5UB/KclfpEkZ6UOEdIeC/06LLOnE6fHdo4knRaAK5qyJuvbliq7SjicIyeWjDxydAmW +7pfssExKLUjcRATB5Tp5Y5+xMruNDh73JcuyNKM1mpWN75HhqcD1VtMUw2LcJeTDJwp wUZnTG4eaK9wV4jfaMtE2txB1GhH0ka/6xrYBJ2IKO29MHJoBGR83YgjZ7/WuCgjztbz xzQw== X-Forwarded-Encrypted: i=1; AJvYcCVQvLv13ZmG/8p3QdYxX0jBG8cSRnv5RkjRT9uNgVQvjmGHLKRXqglK0OrgsF0hXIl76GWtUgIkdRh4Iw== X-Gm-Message-State: AOJu0Yw95XnZ+iDGOBQeSMcdR3fF8scHn9ob7M7sUHmr+fU5T29kJIlC DB6lPZmGRCakuISb0OHkSsuSSK7dvkJTsYEH/d3lkgRyo0Y234CRry/u4m4kzRANa7+S4mIm9yF i3uGvzzdrgAwivPIR/U/RnjCpnby8/Y9FQnBr+TY3joh91e+GvRualNJRu0s64wMLq4e64+Lg3R ziRrgAJl4m/V1tng== X-Received: by 2002:a2e:a4cf:0:b0:2d2:64c9:214b with SMTP id p15-20020a2ea4cf000000b002d264c9214bmr7107598ljm.50.1709116980633; 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: ci@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK CI discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ci-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