[v3,2/2] net/ice: fix scalar Tx path segment
Checks
Commit Message
The scalar Tx path would send empty buffer that causes the Tx queue to
overflow.
This patch adds the last buffer length judgment in tx_prepare to fix this
issue, rte_errno will be set to EINVAL and returned if the last buffer is
empty.
Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
Cc: stable@dpdk.org
Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
drivers/net/ice/ice_rxtx.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
Comments
> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Friday, November 11, 2022 8:04 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
>
> The scalar Tx path would send empty buffer that causes the Tx queue to
> overflow.
>
> This patch adds the last buffer length judgment in tx_prepare to fix this issue,
> rte_errno will be set to EINVAL and returned if the last buffer is empty.
>
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
> drivers/net/ice/ice_rxtx.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c index
> e6ddd2513d..69358f6a3a 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev,
> struct ice_tx_queue *txq)
> #define ICE_MIN_TSO_MSS 64
> #define ICE_MAX_TSO_MSS 9728
> #define ICE_MAX_TSO_FRAME_SIZE 262144
> +
> +/*Check for invalid mbuf*/
> +static inline uint16_t
> +ice_check_mbuf(struct rte_mbuf *tx_pkt) {
Better to name the function to exactly match what it does.
e.g.: ice_check_emtpy_mbuf
and also declare it as inline.
> + struct rte_mbuf *txd = tx_pkt;
> +
> + while (txd != NULL) {
> + if (txd->data_len == 0)
> + return -1;
> + txd = txd->next;
> + }
> +
> + return 0;
> +}
> +
> uint16_t
> ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> uint16_t nb_pkts)
> @@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> struct rte_mbuf **tx_pkts,
> struct ice_tx_queue *txq = tx_queue;
> struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
> uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
> + uint16_t nb_used;
>
> for (i = 0; i < nb_pkts; i++) {
> m = tx_pkts[i];
> @@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> struct rte_mbuf **tx_pkts,
> rte_errno = -ret;
> return i;
> }
> +
> + if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
> + ice_check_mbuf(m)) {
Why "!(ol_flags & RTE_MBUF_F_TX_TCP_SEG)" is needed here?
A empty mbuf with TSO enabled is still acceptable?
> + rte_errno = EINVAL;
> + PMD_DRV_LOG(ERR, "INVALID mbuf: last mbuf
> data_len=[0]");
> + return i;
> + }
> }
> return i;
> }
> --
> 2.34.1
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: 2022年11月11日 13:09
> To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
>
>
>
> > -----Original Message-----
> > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > Sent: Friday, November 11, 2022 8:04 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Ye, MingjinX
> > <mingjinx.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> > Jingjing <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> > Ferruh Yigit <ferruh.yigit@intel.com>; Li, Xiaoyun
> > <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> > Subject: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> >
> > The scalar Tx path would send empty buffer that causes the Tx queue to
> > overflow.
> >
> > This patch adds the last buffer length judgment in tx_prepare to fix
> > this issue, rte_errno will be set to EINVAL and returned if the last buffer is
> empty.
> >
> > Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> > Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > ---
> > drivers/net/ice/ice_rxtx.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > index e6ddd2513d..69358f6a3a 100644
> > --- a/drivers/net/ice/ice_rxtx.c
> > +++ b/drivers/net/ice/ice_rxtx.c
> > @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev
> > *dev, struct ice_tx_queue *txq)
> > #define ICE_MIN_TSO_MSS 64
> > #define ICE_MAX_TSO_MSS 9728
> > #define ICE_MAX_TSO_FRAME_SIZE 262144
> > +
> > +/*Check for invalid mbuf*/
> > +static inline uint16_t
> > +ice_check_mbuf(struct rte_mbuf *tx_pkt) {
>
> Better to name the function to exactly match what it does.
> e.g.: ice_check_emtpy_mbuf
> and also declare it as inline.
will optimize.
>
> > + struct rte_mbuf *txd = tx_pkt;
> > +
> > + while (txd != NULL) {
> > + if (txd->data_len == 0)
> > + return -1;
> > + txd = txd->next;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > uint16_t
> > ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> > uint16_t nb_pkts)
> > @@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > struct ice_tx_queue *txq = tx_queue;
> > struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
> > uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
> > + uint16_t nb_used;
> >
> > for (i = 0; i < nb_pkts; i++) {
> > m = tx_pkts[i];
> > @@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > rte_errno = -ret;
> > return i;
> > }
> > +
> > + if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
> > + ice_check_mbuf(m)) {
>
> Why "!(ol_flags & RTE_MBUF_F_TX_TCP_SEG)" is needed here?
> A empty mbuf with TSO enabled is still acceptable?
Should not be expected to be implemented in test-pmd, should detect.
>
> > + rte_errno = EINVAL;
> > + PMD_DRV_LOG(ERR, "INVALID mbuf: last mbuf
> > data_len=[0]");
> > + return i;
> > + }
> > }
> > return i;
> > }
> > --
> > 2.34.1
> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Friday, November 11, 2022 4:31 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
>
>
>
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: 2022年11月11日 13:09
> > To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ferruh
> > Yigit <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> > Liu, KevinX <kevinx.liu@intel.com>
> > Subject: RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> >
> >
> >
> > > -----Original Message-----
> > > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > > Sent: Friday, November 11, 2022 8:04 PM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > > YidingX <yidingx.zhou@intel.com>; Ye, MingjinX
> > > <mingjinx.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> > > Jingjing <jingjing.wu@intel.com>; Lu, Wenzhuo
> > > <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> > > Xiaoyun <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> > > Subject: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> > >
> > > The scalar Tx path would send empty buffer that causes the Tx queue
> > > to overflow.
> > >
> > > This patch adds the last buffer length judgment in tx_prepare to fix
> > > this issue, rte_errno will be set to EINVAL and returned if the last
> > > buffer is
> > empty.
> > >
> > > Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> > > Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > > ---
> > > drivers/net/ice/ice_rxtx.c | 24 ++++++++++++++++++++++++
> > > 1 file changed, 24 insertions(+)
> > >
> > > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > > index e6ddd2513d..69358f6a3a 100644
> > > --- a/drivers/net/ice/ice_rxtx.c
> > > +++ b/drivers/net/ice/ice_rxtx.c
> > > @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev
> > > *dev, struct ice_tx_queue *txq)
> > > #define ICE_MIN_TSO_MSS 64
> > > #define ICE_MAX_TSO_MSS 9728
> > > #define ICE_MAX_TSO_FRAME_SIZE 262144
> > > +
> > > +/*Check for invalid mbuf*/
> > > +static inline uint16_t
> > > +ice_check_mbuf(struct rte_mbuf *tx_pkt) {
> >
> > Better to name the function to exactly match what it does.
> > e.g.: ice_check_emtpy_mbuf
> > and also declare it as inline.
> will optimize.
> >
> > > + struct rte_mbuf *txd = tx_pkt;
> > > +
> > > + while (txd != NULL) {
> > > + if (txd->data_len == 0)
> > > + return -1;
> > > + txd = txd->next;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > uint16_t
> > > ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> > > uint16_t nb_pkts)
> > > @@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > > struct rte_mbuf **tx_pkts,
> > > struct ice_tx_queue *txq = tx_queue;
> > > struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
> > > uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
> > > + uint16_t nb_used;
> > >
> > > for (i = 0; i < nb_pkts; i++) {
> > > m = tx_pkts[i];
> > > @@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > > struct rte_mbuf **tx_pkts,
> > > rte_errno = -ret;
> > > return i;
> > > }
> > > +
> > > + if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
> > > + ice_check_mbuf(m)) {
> >
> > Why "!(ol_flags & RTE_MBUF_F_TX_TCP_SEG)" is needed here?
> > A empty mbuf with TSO enabled is still acceptable?
> Should not be expected to be implemented in test-pmd, should detect.
Why related with testpmd?, testpmd is not the only application, this is the question for how to identify a violated mbuf.
> >
> > > + rte_errno = EINVAL;
> > > + PMD_DRV_LOG(ERR, "INVALID mbuf: last mbuf
> > > data_len=[0]");
> > > + return i;
> > > + }
> > > }
> > > return i;
> > > }
> > > --
> > > 2.34.1
@@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev, struct ice_tx_queue *txq)
#define ICE_MIN_TSO_MSS 64
#define ICE_MAX_TSO_MSS 9728
#define ICE_MAX_TSO_FRAME_SIZE 262144
+
+/*Check for invalid mbuf*/
+static inline uint16_t
+ice_check_mbuf(struct rte_mbuf *tx_pkt)
+{
+ struct rte_mbuf *txd = tx_pkt;
+
+ while (txd != NULL) {
+ if (txd->data_len == 0)
+ return -1;
+ txd = txd->next;
+ }
+
+ return 0;
+}
+
uint16_t
ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts)
@@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
struct ice_tx_queue *txq = tx_queue;
struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
+ uint16_t nb_used;
for (i = 0; i < nb_pkts; i++) {
m = tx_pkts[i];
@@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
rte_errno = -ret;
return i;
}
+
+ if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
+ ice_check_mbuf(m)) {
+ rte_errno = EINVAL;
+ PMD_DRV_LOG(ERR, "INVALID mbuf: last mbuf data_len=[0]");
+ return i;
+ }
}
return i;
}