RE: [PATCH v2 06/13] net/txgbe: check length of Tx packets

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Fri, Nov 1, 2024 9:23 AM, Ferruh Yigit wrote:
> On 10/28/2024 2:31 AM, Jiawen Wu wrote:
> > Add checking of the Tx packet length to avoid TDM fatal error as far as
> > possible. Set the pkt_len=1518 for invalid packet in simple Tx code path,
> > and drop it directly in featured Tx code path.
> >
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > ---
> >  drivers/net/txgbe/txgbe_rxtx.c          | 33 +++++++++++++++++++++++++
> >  drivers/net/txgbe/txgbe_rxtx_vec_neon.c | 10 +++++---
> >  drivers/net/txgbe/txgbe_rxtx_vec_sse.c  | 11 ++++++---
> >  3 files changed, 48 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/txgbe/txgbe_rxtx.c b/drivers/net/txgbe/txgbe_rxtx.c
> > index 2d2b437643..06acbd0881 100644
> > --- a/drivers/net/txgbe/txgbe_rxtx.c
> > +++ b/drivers/net/txgbe/txgbe_rxtx.c
> > @@ -160,6 +160,8 @@ tx4(volatile struct txgbe_tx_desc *txdp, struct rte_mbuf **pkts)
> >  	for (i = 0; i < 4; ++i, ++txdp, ++pkts) {
> >  		buf_dma_addr = rte_mbuf_data_iova(*pkts);
> >  		pkt_len = (*pkts)->data_len;
> > +		if (pkt_len < RTE_ETHER_HDR_LEN)
> > +			pkt_len = TXGBE_FRAME_SIZE_DFT;
> >
> >  		/* write data to descriptor */
> >  		txdp->qw0 = rte_cpu_to_le_64(buf_dma_addr);
> > @@ -180,6 +182,8 @@ tx1(volatile struct txgbe_tx_desc *txdp, struct rte_mbuf **pkts)
> >
> >  	buf_dma_addr = rte_mbuf_data_iova(*pkts);
> >  	pkt_len = (*pkts)->data_len;
> > +	if (pkt_len < RTE_ETHER_HDR_LEN)
> > +		pkt_len = TXGBE_FRAME_SIZE_DFT;
> >
> >  	/* write data to descriptor */
> >  	txdp->qw0 = cpu_to_le64(buf_dma_addr);
> > @@ -813,6 +817,30 @@ txgbe_parse_tun_ptid(struct rte_mbuf *tx_pkt, uint8_t tun_len)
> >  	return ptid;
> >  }
> >
> > +static inline bool
> > +txgbe_check_pkt_err(struct rte_mbuf *tx_pkt)
> > +{
> > +	uint32_t total_len = 0, nb_seg = 0;
> > +	struct rte_mbuf *mseg;
> > +
> > +	mseg = tx_pkt;
> > +	do {
> > +		if (mseg->data_len == 0)
> > +			return true;
> > +		total_len += mseg->data_len;
> > +		nb_seg++;
> > +		mseg = mseg->next;
> > +	} while (mseg != NULL);
> > +
> > +	if (tx_pkt->pkt_len != total_len || tx_pkt->pkt_len == 0)
> > +		return true;
> > +
> > +	if (tx_pkt->nb_segs != nb_seg || tx_pkt->nb_segs > 64)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >
> 
> Hi Jiawen,
> 
> Above are generic checks, we may add this function to ethdev driver
> header (ethdev_driver.h) so that any PMD can use it, what do you think?

In fact, the limitation of tx_pkt->nb_segs is already implemented in
.tx_pkt_prepare. But users developing their own apps don't necessarily call
this function. So I'd like to add these in txgbe_xmit_pkts(). What are you
going to do about it?