* [dpdk-dev] [PATCH] net/mlx5: fix overwriting bit-fields in SW Rx queue
@ 2018-01-08 22:06 Yongseok Koh
2018-01-09 17:38 ` [dpdk-dev] [PATCH v2] " Yongseok Koh
0 siblings, 1 reply; 3+ messages in thread
From: Yongseok Koh @ 2018-01-08 22:06 UTC (permalink / raw)
To: adrien.mazarguil, nelio.laranjeiro; +Cc: dev, Yongseok Koh, stable
Bit-fields in mlx5_rxq_data can be chagned on the fly by a control plane -
e.g. rxq->mark. However, vectorized Rx uses a bit-field to mark pending
errors. Even if one bit is written, consuquence is to write the whole
integer and this can cause a synchronization issue - two entities write to
a same block without locking. As the pending_err bit is entirely internal
use for the datapath, this can be replaced with a local variable.
Fixes: 6cb559d67b83 ("net/mlx5: add vectorized Rx/Tx burst for x86")
Fixes: 570acdb1da8a ("net/mlx5: add vectorized Rx/Tx burst for ARM")
Cc: stable@dpdk.org
Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
drivers/net/mlx5/mlx5_rxtx.h | 3 +--
drivers/net/mlx5/mlx5_rxtx_vec.c | 6 +++---
drivers/net/mlx5/mlx5_rxtx_vec_neon.h | 9 ++++++---
drivers/net/mlx5/mlx5_rxtx_vec_sse.h | 8 ++++++--
4 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 90f129168..eb7a3db85 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -114,8 +114,7 @@ struct mlx5_rxq_data {
unsigned int elts_n:4; /* Log 2 of Mbufs. */
unsigned int rss_hash:1; /* RSS hash result is enabled. */
unsigned int mark:1; /* Marked flow available on the queue. */
- unsigned int pending_err:1; /* CQE error needs to be handled. */
- unsigned int :14; /* Remaining bits. */
+ unsigned int :15; /* Remaining bits. */
volatile uint32_t *rq_db;
volatile uint32_t *cq_db;
uint16_t port_id;
diff --git a/drivers/net/mlx5/mlx5_rxtx_vec.c b/drivers/net/mlx5/mlx5_rxtx_vec.c
index 8d23dae7e..01edcaf67 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec.c
+++ b/drivers/net/mlx5/mlx5_rxtx_vec.c
@@ -219,7 +219,6 @@ rxq_handle_pending_error(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts,
rxq->stats.ipackets -= (pkts_n - n);
rxq->stats.ibytes -= err_bytes;
#endif
- rxq->pending_err = 0;
return n;
}
@@ -241,9 +240,10 @@ mlx5_rx_burst_vec(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
{
struct mlx5_rxq_data *rxq = dpdk_rxq;
uint16_t nb_rx;
+ uint64_t err = 0;
- nb_rx = rxq_burst_v(rxq, pkts, pkts_n);
- if (unlikely(rxq->pending_err))
+ nb_rx = rxq_burst_v(rxq, pkts, pkts_n, &err);
+ if (unlikely(err))
nb_rx = rxq_handle_pending_error(rxq, pkts, nb_rx);
return nb_rx;
}
diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
index c5d5b0519..62155dbfd 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
@@ -658,12 +658,16 @@ rxq_cq_to_ptype_oflags_v(struct mlx5_rxq_data *rxq,
* Array to store received packets.
* @param pkts_n
* Maximum number of packets in array.
+ * @param[out] err
+ * Pointer to a flag. Set non-zero value if pkts array has at least one error
+ * packet to handle.
*
* @return
* Number of packets received including errors (<= pkts_n).
*/
static inline uint16_t
-rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
+rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n,
+ uint64_t *err)
{
const uint16_t q_n = 1 << rxq->cqe_n;
const uint16_t q_mask = q_n - 1;
@@ -963,8 +967,7 @@ rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
opcode = vceq_u16(resp_err_check, opcode);
opcode = vbic_u16(opcode, invalid_mask);
/* D.4 mark if any error is set */
- rxq->pending_err |=
- !!vget_lane_u64(vreinterpret_u64_u16(opcode), 0);
+ *err |= vget_lane_u64(vreinterpret_u64_u16(opcode), 0);
/* C.4 fill in mbuf - rearm_data and packet_type. */
rxq_cq_to_ptype_oflags_v(rxq, ptype_info, flow_tag,
opcode, &elts[pos]);
diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
index 0dd8145bc..24fc90aa7 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
@@ -662,12 +662,16 @@ rxq_cq_to_ptype_oflags_v(struct mlx5_rxq_data *rxq, __m128i cqes[4],
* Array to store received packets.
* @param pkts_n
* Maximum number of packets in array.
+ * @param[out] err
+ * Pointer to a flag. Set non-zero value if pkts array has at least one error
+ * packet to handle.
*
* @return
* Number of packets received including errors (<= pkts_n).
*/
static inline uint16_t
-rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
+rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n,
+ uint64_t *err)
{
const uint16_t q_n = 1 << rxq->cqe_n;
const uint16_t q_mask = q_n - 1;
@@ -929,7 +933,7 @@ rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
opcode = _mm_packs_epi32(opcode, zero);
opcode = _mm_andnot_si128(invalid_mask, opcode);
/* D.4 mark if any error is set */
- rxq->pending_err |= !!_mm_cvtsi128_si64(opcode);
+ *err |= _mm_cvtsi128_si64(opcode);
/* D.5 fill in mbuf - rearm_data and packet_type. */
rxq_cq_to_ptype_oflags_v(rxq, cqes, opcode, &pkts[pos]);
if (rxq->hw_timestamp) {
--
2.11.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* [dpdk-dev] [PATCH v2] net/mlx5: fix overwriting bit-fields in SW Rx queue
2018-01-08 22:06 [dpdk-dev] [PATCH] net/mlx5: fix overwriting bit-fields in SW Rx queue Yongseok Koh
@ 2018-01-09 17:38 ` Yongseok Koh
2018-01-10 15:22 ` Shahaf Shuler
0 siblings, 1 reply; 3+ messages in thread
From: Yongseok Koh @ 2018-01-09 17:38 UTC (permalink / raw)
To: adrien.mazarguil, nelio.laranjeiro; +Cc: dev, Yongseok Koh, stable
Bit-fields in mlx5_rxq_data can be changed on the fly by a control plane -
e.g. rxq->mark. However, vectorized Rx uses a bit-field to mark pending
errors. Even if one bit is written, consequence is to write the whole
integer and this can cause a synchronization issue - two entities write to
a same block without locking. As the pending_err bit is entirely internal
use for the datapath, this can be replaced with a local variable.
Fixes: 6cb559d67b83 ("net/mlx5: add vectorized Rx/Tx burst for x86")
Fixes: 570acdb1da8a ("net/mlx5: add vectorized Rx/Tx burst for ARM")
Cc: stable@dpdk.org
Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
v2:
* fix typos in the commit message.
drivers/net/mlx5/mlx5_rxtx.h | 3 +--
drivers/net/mlx5/mlx5_rxtx_vec.c | 6 +++---
drivers/net/mlx5/mlx5_rxtx_vec_neon.h | 9 ++++++---
drivers/net/mlx5/mlx5_rxtx_vec_sse.h | 8 ++++++--
4 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 90f129168..eb7a3db85 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -114,8 +114,7 @@ struct mlx5_rxq_data {
unsigned int elts_n:4; /* Log 2 of Mbufs. */
unsigned int rss_hash:1; /* RSS hash result is enabled. */
unsigned int mark:1; /* Marked flow available on the queue. */
- unsigned int pending_err:1; /* CQE error needs to be handled. */
- unsigned int :14; /* Remaining bits. */
+ unsigned int :15; /* Remaining bits. */
volatile uint32_t *rq_db;
volatile uint32_t *cq_db;
uint16_t port_id;
diff --git a/drivers/net/mlx5/mlx5_rxtx_vec.c b/drivers/net/mlx5/mlx5_rxtx_vec.c
index 8d23dae7e..01edcaf67 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec.c
+++ b/drivers/net/mlx5/mlx5_rxtx_vec.c
@@ -219,7 +219,6 @@ rxq_handle_pending_error(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts,
rxq->stats.ipackets -= (pkts_n - n);
rxq->stats.ibytes -= err_bytes;
#endif
- rxq->pending_err = 0;
return n;
}
@@ -241,9 +240,10 @@ mlx5_rx_burst_vec(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
{
struct mlx5_rxq_data *rxq = dpdk_rxq;
uint16_t nb_rx;
+ uint64_t err = 0;
- nb_rx = rxq_burst_v(rxq, pkts, pkts_n);
- if (unlikely(rxq->pending_err))
+ nb_rx = rxq_burst_v(rxq, pkts, pkts_n, &err);
+ if (unlikely(err))
nb_rx = rxq_handle_pending_error(rxq, pkts, nb_rx);
return nb_rx;
}
diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
index c5d5b0519..62155dbfd 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
@@ -658,12 +658,16 @@ rxq_cq_to_ptype_oflags_v(struct mlx5_rxq_data *rxq,
* Array to store received packets.
* @param pkts_n
* Maximum number of packets in array.
+ * @param[out] err
+ * Pointer to a flag. Set non-zero value if pkts array has at least one error
+ * packet to handle.
*
* @return
* Number of packets received including errors (<= pkts_n).
*/
static inline uint16_t
-rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
+rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n,
+ uint64_t *err)
{
const uint16_t q_n = 1 << rxq->cqe_n;
const uint16_t q_mask = q_n - 1;
@@ -963,8 +967,7 @@ rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
opcode = vceq_u16(resp_err_check, opcode);
opcode = vbic_u16(opcode, invalid_mask);
/* D.4 mark if any error is set */
- rxq->pending_err |=
- !!vget_lane_u64(vreinterpret_u64_u16(opcode), 0);
+ *err |= vget_lane_u64(vreinterpret_u64_u16(opcode), 0);
/* C.4 fill in mbuf - rearm_data and packet_type. */
rxq_cq_to_ptype_oflags_v(rxq, ptype_info, flow_tag,
opcode, &elts[pos]);
diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
index 0dd8145bc..24fc90aa7 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h
@@ -662,12 +662,16 @@ rxq_cq_to_ptype_oflags_v(struct mlx5_rxq_data *rxq, __m128i cqes[4],
* Array to store received packets.
* @param pkts_n
* Maximum number of packets in array.
+ * @param[out] err
+ * Pointer to a flag. Set non-zero value if pkts array has at least one error
+ * packet to handle.
*
* @return
* Number of packets received including errors (<= pkts_n).
*/
static inline uint16_t
-rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
+rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n,
+ uint64_t *err)
{
const uint16_t q_n = 1 << rxq->cqe_n;
const uint16_t q_mask = q_n - 1;
@@ -929,7 +933,7 @@ rxq_burst_v(struct mlx5_rxq_data *rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
opcode = _mm_packs_epi32(opcode, zero);
opcode = _mm_andnot_si128(invalid_mask, opcode);
/* D.4 mark if any error is set */
- rxq->pending_err |= !!_mm_cvtsi128_si64(opcode);
+ *err |= _mm_cvtsi128_si64(opcode);
/* D.5 fill in mbuf - rearm_data and packet_type. */
rxq_cq_to_ptype_oflags_v(rxq, cqes, opcode, &pkts[pos]);
if (rxq->hw_timestamp) {
--
2.11.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix overwriting bit-fields in SW Rx queue
2018-01-09 17:38 ` [dpdk-dev] [PATCH v2] " Yongseok Koh
@ 2018-01-10 15:22 ` Shahaf Shuler
0 siblings, 0 replies; 3+ messages in thread
From: Shahaf Shuler @ 2018-01-10 15:22 UTC (permalink / raw)
To: Yongseok Koh, Adrien Mazarguil, Nélio Laranjeiro
Cc: dev, Yongseok Koh, stable
Tuesday, January 9, 2018 7:39 PM, Yongseok Koh:
> Bit-fields in mlx5_rxq_data can be changed on the fly by a control plane - e.g.
> rxq->mark. However, vectorized Rx uses a bit-field to mark pending errors.
> Even if one bit is written, consequence is to write the whole integer and this
> can cause a synchronization issue - two entities write to a same block without
> locking. As the pending_err bit is entirely internal use for the datapath, this
> can be replaced with a local variable.
>
> Fixes: 6cb559d67b83 ("net/mlx5: add vectorized Rx/Tx burst for x86")
> Fixes: 570acdb1da8a ("net/mlx5: add vectorized Rx/Tx burst for ARM")
> Cc: stable@dpdk.org
>
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Applied to next-net-mlx, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-10 15:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-08 22:06 [dpdk-dev] [PATCH] net/mlx5: fix overwriting bit-fields in SW Rx queue Yongseok Koh
2018-01-09 17:38 ` [dpdk-dev] [PATCH v2] " Yongseok Koh
2018-01-10 15:22 ` Shahaf Shuler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).