Re: [PATCH v7 2/2] net/af_xdp: Refactor af_xdp_tx_zc

[Ariel Otilibili <ariel.otilibili@6wind.com>]

[-- Attachment #1: Type: text/plain, Size: 2251 bytes --]

Hi Maryam,

On Thu, Feb 6, 2025 at 3:09 AM Maryam Tahhan <mtahhan@redhat.com> wrote:

>
>
> On Sat 1 Feb 2025, 10: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)
>>                 }
>>  }
>>
>> +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 = NULL;
>> +       uint32_t *idx_tx = NULL;
>>
>
> Took me a while to spot 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 expects to dereference a pointer to a uint32_t to store the
> 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=address
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 = xsk_ring_prod__tx_desc(&txq->tx, *idx_tx);
>> +       desc->len = mbuf->pkt_len;
>> +
>> +       addr = (uint64_t)mbuf - (uint64_t)umem->buffer
>> +               - umem->mb_pool->header_size;
>> +       offset = rte_pktmbuf_mtod(mbuf, uint64_t) - (uint64_t)mbuf
>> +               + umem->mb_pool->header_size;
>> +       offset = offset << XSK_UNALIGNED_BUF_OFFSET_SHIFT;
>> +       desc->addr = addr | offset;
>> +
>> +out:
>> +       return desc;
>> +}
>> +
>> <snip>
>
>

[-- Attachment #2: Type: text/html, Size: 3566 bytes --]