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 43A834619D; Thu, 6 Feb 2025 18:56:23 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C76AD402BC; Thu, 6 Feb 2025 18:56:22 +0100 (CET) Received: from mail-il1-f169.google.com (mail-il1-f169.google.com [209.85.166.169]) by mails.dpdk.org (Postfix) with ESMTP id 701D84026C for ; Thu, 6 Feb 2025 18:56:21 +0100 (CET) Received: by mail-il1-f169.google.com with SMTP id e9e14a558f8ab-3d04d767d3aso9770415ab.2 for ; Thu, 06 Feb 2025 09:56:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; t=1738864580; x=1739469380; 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=UvxZxivncBueIxHdasICpkShHGNyF22oVr/HMCzx1tA=; b=IcQqsuo2rZ+0J9junuvNSvqIx4DZAke8NV8ttJeQalQ1qnHtpw4xJ3LlTONVjuHp19 8U5LaXY/9exaWLMLnzIS6Do7Sl00sh2eD6mklcvOwgj+hrhJBRb01kEjATelA9qR2I44 3oDvLPQz0GvrXddf6TWYIunfmUGmTf9w/wMjK7+kuvJ4+nd1CU/Vw3Sl/eStGAi6GrwK Kxba5/3kLZu56W29fiDO1Vg7NAtLD5rAQ0c5j18jjv+M668bZExLEoEy7z0G+HJXGaiu Y9eA9KOHfpb5aY4bYHLyuGd1QqDKbzPOnzXxXhRVXLvOgEufrAAvhtGg4atM+rySePS0 MmUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738864580; x=1739469380; 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=UvxZxivncBueIxHdasICpkShHGNyF22oVr/HMCzx1tA=; b=kvtZFocaBKWHGydrgIwoGd7uI5SYLYGyywXpAhDhonzgJFXugt5KdDxYH4yiHtXd2E 1EgdG2PA863RbMaDP2SlYk9vyR1/oZ9OCo676EjU2B1Mfg+ZDkzefWm5f5tSrho8byc3 g4WjIwgAP7xWHXboWMYSfU9zPcXaUKXG/GniK5VJdM9JEOJYKFp40XquzHcmFXyT8my0 8rAeLtknQZ/2qRKNBr3JO9cDRNUQBIG80IfWlB86NX9L1mspGAFk972QcKDLT9kn8CCn FwXbecWefwD6v4WdFSnm+YHYN4Z0VrIeTyNZnIj+zUXpIRkLFV4x40uDTul1w1uPqA6j mt8g== X-Gm-Message-State: AOJu0YwEHOJtL1fu9zGSonG2nLzcn9/gk0PsaGpaJolF9lFwEwJqYitc G61n198pvRrs8YP26thQtw1jDB2U6IF9Oxw0xPDC/9c/ofzXnLXNsynzAa0P+kqfrAgAld//1T/ ngzyk3Y2+Ks/TmWPrt8FJcY9tv5Erc4dHRBMOqg== X-Gm-Gg: ASbGncuv59DxiRotrpuLNQoh3cP15HXZ/6PSF/URbVBIirZrRsw/tUEB4jdSK4FQ7C7 l8uYrl1tA5fgau4AX6S1RNGe7q1v3wn06wQrp8jzco33+b0KZ76Nj6xaa3djQ4o2IBr0EVeOBOT avDtVayHxM3JiHV0cBdENEFTGmhLys X-Google-Smtp-Source: AGHT+IFbSvFng7wdYqW2Pv0S1IKckoK7QWQKlgbZEoeOWh4fH2eSheTGCRa7pm/yxFEROi7/mlmybtpPDT2Suy8LQUY= X-Received: by 2002:a05:6e02:12e8:b0:3d0:2477:83ec with SMTP id e9e14a558f8ab-3d04f917cbdmr58042865ab.14.1738864580652; Thu, 06 Feb 2025 09:56:20 -0800 (PST) MIME-Version: 1.0 References: <20250116195640.68885-1-ariel.otilibili@6wind.com> <20250201100300.2194018-1-ariel.otilibili@6wind.com> <20250201100300.2194018-3-ariel.otilibili@6wind.com> In-Reply-To: From: Ariel Otilibili Date: Thu, 6 Feb 2025 18:56:09 +0100 X-Gm-Features: AWEUYZnMFyThafupN6QAceXOxoiGX4A3uXyKHhcqaUsGMmZyWpzs0agXv7SDyks Message-ID: Subject: Re: [PATCH v7 2/2] net/af_xdp: Refactor af_xdp_tx_zc To: Maryam Tahhan Cc: "dev@dpdk.org" , "stable@dpdk.org" , Thomas Monjalon , David Marchand , Stephen Hemminger , Ciara Loftus Content-Type: multipart/alternative; boundary="000000000000a49359062d7cf567" 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 --000000000000a49359062d7cf567 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Maryam, On Thu, Feb 6, 2025 at 3:09=E2=80=AFAM Maryam Tahhan w= rote: > > > On Sat 1 Feb 2025, 10:03 Ariel Otilibili, > wrote: > >> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c >> b/drivers/net/af_xdp/rte_eth_af_xdp.c >> index 092bcb73aa0a..840a12dbf508 100644 >> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c >> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c >> @@ -536,6 +536,31 @@ kick_tx(struct pkt_tx_queue *txq, struct >> xsk_ring_cons *cq) >> } >> } >> >> +static inline struct xdp_desc * >> +reserve_and_fill(struct pkt_tx_queue *txq, struct rte_mbuf *mbuf, >> + struct xsk_umem_info *umem) >> +{ >> + struct xdp_desc *desc =3D NULL; >> + uint32_t *idx_tx =3D NULL; >> > > Took me a while to spot this but this needs to be a uint32_t not a pointe= r > to uint32_t, because xsk_ring_prod__reserve() does not allocate memory fo= r > idx_tx it just expects to dereference a pointer to a uint32_t to store th= e > index... > Only now could I answer back; sorry the late response. And thanks for your thorough investigation. I didn't get the chance to use *testpmd;* which arguments did you pass? I ran a scan-build while compiling, as well as a compilation with sanitized flags. And I saw no error in af_xdp source files. meson --wipe build -Db_sanitize=3Daddress ninja -C build 2>&1 san_`date -Iseconds`.log ninja -C build 2>&1 | tee --append san_`date -Iseconds`.log ninja -C build scan-build 2>&1 | tee --append san_`date -Iseconds`.log How did you spot the error? > > + uint64_t addr, offset; >> + >> + if (!xsk_ring_prod__reserve(&txq->tx, 1, idx_tx)) >> + goto out; >> + >> + desc =3D xsk_ring_prod__tx_desc(&txq->tx, *idx_tx); >> + desc->len =3D mbuf->pkt_len; >> + >> + addr =3D (uint64_t)mbuf - (uint64_t)umem->buffer >> + - umem->mb_pool->header_size; >> + offset =3D rte_pktmbuf_mtod(mbuf, uint64_t) - (uint64_t)mbuf >> + + umem->mb_pool->header_size; >> + offset =3D offset << XSK_UNALIGNED_BUF_OFFSET_SHIFT; >> + desc->addr =3D addr | offset; >> + >> +out: >> + return desc; >> +} >> + >> > > --000000000000a49359062d7cf567 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Maryam,

On Th= u, Feb 6, 2025 at 3:09=E2=80=AFAM Maryam Tahhan <mtahhan@redhat.com> wrote:


On Sat 1 Feb 2025, 1= 0:03 Ariel Otilibili, <ariel.otilibili@6wind.com> wrote:
diff --git a/drivers/net/af_xdp/rte_= eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 092bcb73aa0a..840a12dbf508 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -536,6 +536,31 @@ kick_tx(struct pkt_tx_queue *txq, struct xsk_ring_cons= *cq)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0}

+static inline struct xdp_desc *
+reserve_and_fill(struct pkt_tx_queue *txq, struct rte_mbuf *mbuf,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 struct xsk_umem_in= fo *umem)
+{
+=C2=A0 =C2=A0 =C2=A0 =C2=A0struct xdp_desc *desc =3D NULL;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0uint32_t *idx_tx =3D NULL;

Took me a while to sp= ot this but this needs to be a uint32_t not a pointer to uint32_t, because = xsk_ring_prod__reserve() does not allocate memory for idx_tx it just expect= s to dereference a pointer to a uint32_t to store the index...
<= /blockquote>

Only now could I answer back; sorry the lat= e response.=C2=A0And thanks for your thorough investigation.

=
I didn't get the chance to use testpmd; which argumen= ts did you pass?

I ran a scan-build while comp= iling, as well as a compilation with sanitized flags. And I saw no error in= af_xdp source files.

meson --wipe build -Db_sanit= ize=3Daddress
ninja -C build 2>&1 san_`date -Iseconds`.log
nin= ja -C build 2>&1 | tee --append san_`date -Iseconds`.log
ninja -C= build scan-build 2>&1 | tee --append san_`date -Iseconds`.log=C2=A0=

How did you spot the error?

<= /div>
+=C2=A0 =C2=A0 =C2=A0 =C2=A0uint64_t addr, offset;
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!xsk_ring_prod__reserve(&txq->tx, 1,= idx_tx))
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto out;
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0desc =3D xsk_ring_prod__tx_desc(&txq->tx= , *idx_tx);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0desc->len =3D mbuf->pkt_len;
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0addr =3D (uint64_t)mbuf - (uint64_t)umem->bu= ffer
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0- umem->mb_pool-= >header_size;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0offset =3D rte_pktmbuf_mtod(mbuf, uint64_t) - (= uint64_t)mbuf
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0+ umem->mb_pool-= >header_size;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0offset =3D offset << XSK_UNALIGNED_BUF_OF= FSET_SHIFT;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0desc->addr =3D addr | offset;
+
+out:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0return desc;
+}
+
<snip>
--000000000000a49359062d7cf567--