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 AE8A843BCF; Fri, 1 Mar 2024 14:40:16 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4BEFF43497; Fri, 1 Mar 2024 14:40:16 +0100 (CET) Received: from inbox.dpdk.org (inbox.dpdk.org [95.142.172.178]) by mails.dpdk.org (Postfix) with ESMTP id BE59343368 for ; Fri, 1 Mar 2024 14:40:14 +0100 (CET) Received: by inbox.dpdk.org (Postfix, from userid 33) id B428343BD7; Fri, 1 Mar 2024 14:40:14 +0100 (CET) From: bugzilla@dpdk.org To: dev@dpdk.org Subject: [DPDK/ethdev Bug 1392] mlx5: random superfluous setting mbuf:hash.fdir.hi Date: Fri, 01 Mar 2024 13:40:14 +0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: new X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: DPDK X-Bugzilla-Component: ethdev X-Bugzilla-Version: unspecified X-Bugzilla-Keywords: X-Bugzilla-Severity: minor X-Bugzilla-Who: konstantin.v.ananyev@yandex.ru X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: Normal X-Bugzilla-Assigned-To: dev@dpdk.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_id short_desc product version rep_platform op_sys bug_status bug_severity priority component assigned_to reporter target_milestone Message-ID: Content-Type: multipart/alternative; boundary=17093004141.3f50db.663319 Content-Transfer-Encoding: 7bit X-Bugzilla-URL: http://bugs.dpdk.org/ Auto-Submitted: auto-generated X-Auto-Response-Suppress: All MIME-Version: 1.0 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 --17093004141.3f50db.663319 Date: Fri, 1 Mar 2024 14:40:14 +0100 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://bugs.dpdk.org/ Auto-Submitted: auto-generated X-Auto-Response-Suppress: All https://bugs.dpdk.org/show_bug.cgi?id=3D1392 Bug ID: 1392 Summary: mlx5: random superfluous setting mbuf:hash.fdir.hi Product: DPDK Version: unspecified Hardware: x86 OS: All Status: UNCONFIRMED Severity: minor Priority: Normal Component: ethdev Assignee: dev@dpdk.org Reporter: konstantin.v.ananyev@yandex.ru Target Milestone: --- Reproducible on latest version of DPDK main branch (24.03 rc1). While mocking with DPDK+mlx5 device(MT27800 Family [ConnectX-5] 1017) noticed that mlx5_rx_burst_vec() on x86 can sometimes store junk values inside mbuf:hash.fdir.hi, even though rte_flow 'MARK' action was not requested and RTE_MBUF_F_RX_FDIR is not set in the mbuf:ol_flags. It can be easily reproduced with testpmd too, to catch it I added the following code into it: --- a/app/test-pmd/testpmd.h +++ b/app/test-pmd/testpmd.h @@ -872,6 +872,10 @@ common_fwd_stream_receive(struct fwd_stream *fs, struct rte_mbuf **burst, if (record_burst_stats) fs->rx_burst_stats.pkt_burst_spread[nb_rx]++; fs->rx_packets +=3D nb_rx; + + for (uint16_t i =3D 0; i !=3D nb_rx; i++) + RTE_VERIFY(burst[i]->hash.fdir.hi =3D=3D 0 || + (burst[i]->ol_flags & RTE_MBUF_F_RX_FDIR) !=3D 0); return nb_rx; } Then: ./dpdk-testpmd --lcores=3D'49,51' -n 6 -a ca:00.0 -a ca:00.1 -a cb:00.0 -a cb:00.1 -- --rxd=3D1024 --txd=3D1024 --rss-ip --rxq=3D2 --txq=3D2 ... EAL: PANIC in common_fwd_stream_receive(): line 877 assert "burst[i]->hash.fdir.hi =3D=3D 0 || (burst[i]->ol_fl= ags & RTE_MBUF_F_RX_FDIR) !=3D 0" failed 0: /home/kananyev/dpdk.org/x84_64-default-linuxapp-gcc10-dbg/app/dpdk-testp= md (rte_dump_stack+0x1f) [1781b0d] ... (gdb) print/x burst[i]->hash.fdir $5 =3D {{{hash =3D 0xc176, id =3D 0xd46c}, lo =3D 0xd46cc176}, hi =3D 0x76c= 16c} (gdb) print/x burst[i]->ol_flags $6 =3D 0x182 // RTE_MBUF_F_RX_RSS_HASH | RTE_MBUF_F_RX_IP_CKSUM_GOOD | RTE_MBUF_F_RX_L4_CKSUM_GOOD A bit of analysis (to put it upfront - I am not that familiar with mlx5 PMD, so would need mlx5 maintainer to have a proper look): first, hash.fdir.hi are being updated here: #0 _mm_storeu_si128 (__B=3D..., __P=3D0x17eb98aa4) at /usr/lib64/gcc/x86_64-suse-linux/10/include/emmintrin.h:728 #1 rxq_cq_process_v (rxq=3D0x1727b1f80, cq=3D0x11f3c0a800, elts=3D0x1727b3= 240, pkts=3D0x7ffff2f23ea0, pkts_n=3D32, err=3D0x7ffff2f23d98, comp=3D0x7fff= f2f21038) at ../drivers/net/mlx5/mlx5_rxtx_vec_sse.h:697 It just copies cq[..].sop_drop_qpn value here: (gdb) print/x cq[pos + p3].sop_drop_qpn $30 =3D 0x2b1fc59d What is interesting, it copies it unconditionally: const __m128i flow_mark_adj =3D _mm_set_epi32(rxq->mark * (-1), 0, 0, 0); ... pkt_mb3 =3D _mm_shuffle_epi8(cqes[3], shuf_mask); ... pkt_mb3 =3D _mm_add_epi32(pkt_mb3, flow_mark_adj); ... _mm_storeu_si128((void *)&pkts[pos + 3]->pkt_len, pkt_mb3); While scalar version of RX updates hash.fdir.hi conditionally, i.e. only wh= en rxq->mark is set. Then later vector version realizes that corresponding cqe[] entry was a compressed one, so it continues with rxq_cq_decompress_v() for the same mbuf. But rxq_cq_decompress_v() updates hash.fdir.hi (and corresponding ol_flags) *only* when rxq->mark is set. Otherwise, it doesn't touch hash.fdir.hi at all. So, as rxq->mark=3D=3D0, we ended up with 'mbuf.hash.fdir.hi' set to some = junk value, while (ol_flags & RTE_MBUF_F_RX_FDIR) =3D=3D 0.=20=20 Note that by itself, it is probably not a big issue, as that happens when RTE_MBUF_F_RX_FDIR is not set. So majority of well behaving app will run just fine, and this small problem will be un-noticed. Though event (and probably sched) framework silently and un-conditionally re-uses mbuf::hash.fdir.hi for its own purposes: struct { uint32_t reserved1; uint16_t reserved2; uint16_t txq; /**< The event eth Tx adapter uses this field * to store Tx queue id. * @see rte_event_eth_tx_adapter_txq_set() */ } txadapter; /**< Eventdev ethdev Tx adapter */ So some of libevent functions expects hash.txadapter.txq to be set into particular=20 value, and such superfluous update of mbuf:hash.fdir.hi can cause a crash. That's how I hit that issue first: https://bugs.dpdk.org/show_bug.cgi?id=3D= 1391 Plus it still looks like sort of inconsistency and undefined bheaviour. So I decided to report it. Possible fix (again, I am not sure that it the best way, but at least it works): inside rxq_cq_process_v(), just zero-out hash.fdir.hi when rxq->mark is not set:=20=20=20=20 diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h index 2bdd1f676d..8ab5dcc19e 100644 --- a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h +++ b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h @@ -600,6 +600,8 @@ rxq_cq_process_v(struct mlx5_rxq_data *rxq, volatile st= ruct mlx5_cqe *cq, 0, rxq->crc_present * RTE_ETHER_CRC_LEN); const __m128i flow_mark_adj =3D _mm_set_epi32(rxq->mark * (-1), 0, = 0, 0); + const __m128i flow_mark_msk =3D + _mm_set_epi32(rxq->mark * (-1), -1, -1, -1); /* * A. load first Qword (8bytes) in one loop. * B. copy 4 mbuf pointers from elts ring to returning pkts. @@ -693,6 +695,8 @@ rxq_cq_process_v(struct mlx5_rxq_data *rxq, volatile st= ruct mlx5_cqe *cq, /* C.4 adjust flow mark. */ pkt_mb3 =3D _mm_add_epi32(pkt_mb3, flow_mark_adj); pkt_mb2 =3D _mm_add_epi32(pkt_mb2, flow_mark_adj); + pkt_mb3 =3D _mm_and_si128(pkt_mb3, flow_mark_msk); + pkt_mb2 =3D _mm_and_si128(pkt_mb2, flow_mark_msk); /* D.1 fill in mbuf - rx_descriptor_fields1. */ _mm_storeu_si128((void *)&pkts[pos + 3]->pkt_len, pkt_mb3); _mm_storeu_si128((void *)&pkts[pos + 2]->pkt_len, pkt_mb2); @@ -720,6 +724,8 @@ rxq_cq_process_v(struct mlx5_rxq_data *rxq, volatile st= ruct mlx5_cqe *cq, /* C.4 adjust flow mark. */ pkt_mb1 =3D _mm_add_epi32(pkt_mb1, flow_mark_adj); pkt_mb0 =3D _mm_add_epi32(pkt_mb0, flow_mark_adj); + pkt_mb1 =3D _mm_and_si128(pkt_mb1, flow_mark_msk); + pkt_mb0 =3D _mm_and_si128(pkt_mb0, flow_mark_msk); /* E.1 extract op_own byte. */ op_own_tmp1 =3D _mm_unpacklo_epi32(cqes[0], cqes[1]); op_own =3D _mm_unpackhi_epi64(op_own_tmp1, op_own_tmp2); --=20 You are receiving this mail because: You are the assignee for the bug.= --17093004141.3f50db.663319 Date: Fri, 1 Mar 2024 14:40:14 +0100 MIME-Version: 1.0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://bugs.dpdk.org/ Auto-Submitted: auto-generated X-Auto-Response-Suppress: All
Bug ID 1392
Summary mlx5: random superfluous setting mbuf:hash.fdir.hi
Product DPDK
Version unspecified
Hardware x86
OS All
Status UNCONFIRMED
Severity minor
Priority Normal
Component ethdev
Assignee dev@dpdk.org
Reporter konstantin.v.ananyev@yandex.ru
Target Milestone --- 

Reproducible on latest version of =
DPDK main branch (24.03 rc1).

While mocking with DPDK+mlx5 device(MT27800 Family [ConnectX-5] 1017)
noticed that mlx5_rx_burst_vec() on x86 can sometimes store
junk values inside  mbuf:hash.fdir.hi, even though rte_flow 'MARK'
action was not requested and RTE_MBUF_F_RX_FDIR is not set in the
mbuf:ol_flags.
It can be easily reproduced with testpmd too,
to catch it I added the following code into it:

--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -872,6 +872,10 @@ common_fwd_stream_receive(struct fwd_s=
tream *fs, struct
rte_mbuf **burst,
        if (record_burst_stats)
                fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
        fs->rx_packets +=3D nb_rx;
+
+       for (uint16_t i =3D 0; i !=3D nb_rx; i++)
+               RTE_VERIFY(burst[i]->hash.fdir.hi =3D=3D 0 ||
+                       (burst[i]->ol_flags & RTE_MBUF_F_RX_FDIR) !=
=3D 0);
        return nb_rx;
 }

Then:
./dpdk-testpmd --lcores=3D'49,51' -n 6 -a ca:00.0 -a ca:00.1 -a cb:00.0 -a
cb:00.1 --  --rxd=3D1024 --txd=3D1024 --rss-ip --rxq=3D2 --txq=3D2
...
EAL: PANIC in common_fwd_stream_receive():
line 877        assert "burst[i]->hash.fdir.hi =3D=3D 0 || (burst[i=
]->ol_flags &
RTE_MBUF_F_RX_FDIR) !=3D 0" failed
0: /home/kananyev/dpdk.org/x84_64-default-linuxapp-gcc10-dbg/app/dpdk-testp=
md
(rte_dump_stack+0x1f) [1781b0d]
...
(gdb) print/x burst[i]->hash.fdir
$5 =3D {{{hash =3D 0xc176, id =3D 0xd46c}, lo =3D 0xd46cc176}, hi =3D 0x76c=
16c}
(gdb) print/x burst[i]->ol_flags
$6 =3D 0x182 // RTE_MBUF_F_RX_RSS_HASH |  RTE_MBUF_F_RX_IP_CKSUM_GOOD |
RTE_MBUF_F_RX_L4_CKSUM_GOOD

A bit of analysis (to put it upfront - I am not that familiar with mlx5 PMD,
so would need mlx5 maintainer to have a proper look):

first, hash.fdir.hi are being updated here:
#0  _mm_storeu_si128 (__B=3D..., __P=3D0x17eb98aa4)
    at /usr/lib64/gcc/x86_64-suse-linux/10/include/emmintrin.h:728
#1  rxq_cq_process_v (rxq=3D0x1727b1f80, cq=3D0x11f3c0a800, elts=3D0x1727b3=
240,
    pkts=3D0x7ffff2f23ea0, pkts_n=3D32, err=3D0x7ffff2f23d98, comp=3D0x7fff=
f2f21038)
    at ../drivers/net/mlx5/mlx5_rxtx_vec_sse.h:697

It just copies cq[..].sop_drop_qpn value here:
(gdb) print/x cq[pos + p3].sop_drop_qpn
$30 =3D 0x2b1fc59d

What is interesting, it copies it unconditionally:
const __m128i flow_mark_adj =3D _mm_set_epi32(rxq->mark * (-1), 0, 0, 0);
...
pkt_mb3 =3D _mm_shuffle_epi8(cqes[3], shuf_mask);
...
pkt_mb3 =3D _mm_add_epi32(pkt_mb3, flow_mark_adj);
...
_mm_storeu_si128((void *)&pkts[pos + 3]->pkt_len, pkt_mb3);

While scalar version of RX updates hash.fdir.hi conditionally, i.e. only wh=
en
rxq->mark is set.
Then later vector version realizes that corresponding cqe[] entry was a
compressed one, so
it continues with   rxq_cq_decompress_v() for the same mbuf.
But rxq_cq_decompress_v() updates hash.fdir.hi (and corresponding ol_flags)
*only* when rxq->mark is set.
Otherwise, it doesn't touch hash.fdir.hi at all.
So, as rxq->mark=3D=3D0,  we ended up with 'mbuf.hash.fdir.hi' set to so=
me junk
value,
while (ol_flags & RTE_MBUF_F_RX_FDIR) =3D=3D 0.=20=20


Note that by itself, it is probably not a big issue, as that happens when
RTE_MBUF_F_RX_FDIR is not set.
So majority of well behaving app will run just fine, and this small problem
will be un-noticed.
Though event (and probably sched) framework silently and un-conditionally
re-uses mbuf::hash.fdir.hi for its own purposes:
struct {
  uint32_t reserved1;
  uint16_t reserved2;
  uint16_t txq;
  /**< The event eth Tx adapter uses this field
   * to store Tx queue id.
   * @see rte_event_eth_tx_adapter_txq_set()
   */
} txadapter; /**< Eventdev ethdev Tx adapter */

So some of libevent functions expects hash.txadapter.txq to be set into
particular=20
value, and such superfluous update of mbuf:hash.fdir.hi can cause a crash.
That's how I hit that issue first: h=
ttps://bugs.dpdk.org/show_bug.cgi?id=3D1391

Plus it still looks like sort of inconsistency and undefined bheaviour.
So I decided to report it.
Possible fix (again, I am not sure that it the best way, but at least it
works):
inside rxq_cq_process_v(),  just zero-out hash.fdir.hi when rxq->mark is=
 not
set:=20=20=20=20

diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
index 2bdd1f676d..8ab5dcc19e 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
@@ -600,6 +600,8 @@ rxq_cq_process_v(struct mlx5_rxq_data *=
rxq, volatile struct
mlx5_cqe *cq,
                              0,
                              rxq->crc_present * RTE_ETHER_CRC_LEN);
        const __m128i flow_mark_adj =3D _mm_set_epi32(rxq->mark * (-1), =
0, 0, 0);
+       const __m128i flow_mark_msk =3D
+               _mm_set_epi32(rxq->mark * (-1), -1, -1, -1);
        /*
         * A. load first Qword (8bytes) in one loop.
         * B. copy 4 mbuf pointers from elts ring to returning pkts.
@@ -693,6 +695,8 @@ rxq_cq_process_v(struct mlx5_rxq_data *=
rxq, volatile struct
mlx5_cqe *cq,
                /* C.4 adjust flow mark. */
                pkt_mb3 =3D _mm_add_epi32(pkt_mb3, flow_mark_adj);
                pkt_mb2 =3D _mm_add_epi32(pkt_mb2, flow_mark_adj);
+               pkt_mb3 =3D _mm_and_si128(pkt_mb3, flow_mark_msk);
+               pkt_mb2 =3D _mm_and_si128(pkt_mb2, flow_mark_msk);
                /* D.1 fill in mbuf - rx_descriptor_fields1. */
                _mm_storeu_si128((void *)&pkts[pos + 3]->pkt_len, pk=
t_mb3);
                _mm_storeu_si128((void *)&pkts[pos + 2]->pkt_len, pk=
t_mb2);
@@ -720,6 +724,8 @@ rxq_cq_process_v(struct mlx5_rxq_data *=
rxq, volatile struct
mlx5_cqe *cq,
                /* C.4 adjust flow mark. */
                pkt_mb1 =3D _mm_add_epi32(pkt_mb1, flow_mark_adj);
                pkt_mb0 =3D _mm_add_epi32(pkt_mb0, flow_mark_adj);
+               pkt_mb1 =3D _mm_and_si128(pkt_mb1, flow_mark_msk);
+               pkt_mb0 =3D _mm_and_si128(pkt_mb0, flow_mark_msk);
                /* E.1 extract op_own byte. */
                op_own_tmp1 =3D _mm_unpacklo_epi32(cqes[0], cqes[1]);
                op_own =3D _mm_unpackhi_epi64(op_own_tmp1, op_own_tmp2);

You are receiving this mail because:
  • You are the assignee for the bug.
=20=20=20=20=20=20=20=20=20=20
= --17093004141.3f50db.663319--