From: Yuri Chipchev <yuric@marvell.com> Fixes stack corruption in mrvl_fill_bpool function in case num > MRVL_PP2_RXD_MAX Fixes: c3637258d894 ("net/mrvl: fix Rx descriptors number") Cc: stable@dpdk.org Signed-off-by: Yuri Chipchev <yuric@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index f25cf9e46..93fb30cdb 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1614,8 +1614,8 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) static int mrvl_fill_bpool(struct mrvl_rxq *rxq, int num) { - struct buff_release_entry entries[MRVL_PP2_RXD_MAX]; - struct rte_mbuf *mbufs[MRVL_PP2_RXD_MAX]; + struct buff_release_entry entries[num]; + struct rte_mbuf *mbufs[num]; int i, ret; unsigned int core_id; struct pp2_hif *hif; -- 2.28.0
From: Liron Himi <lironh@marvell.com> in case of non-ip frame the current code reached the 'default' case which result with function call to log a msg. those kind of calls should not be performed on fast-path. The performance for this kind of frames increased by 50% Fixes: acab7d58c ("net/mvpp2: convert to dynamic logging") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 93fb30cdb..8c3278ec4 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -2171,7 +2171,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, *l4_offset = *l3_offset + MRVL_ARP_LENGTH; break; default: - MRVL_LOG(DEBUG, "Failed to recognise l3 packet type"); break; } @@ -2183,7 +2182,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, packet_type |= RTE_PTYPE_L4_UDP; break; default: - MRVL_LOG(DEBUG, "Failed to recognise l4 packet type"); break; } -- 2.28.0
From: Yuri Chipchev <yuric@marvell.com> 4B of CRC was not included in the bytes statistics Fixes: bdffe0c70 ("net/mrvl: support basic stats") Cc: stable@dpdk.org Signed-off-by: Yuri Chipchev <yuric@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 8c3278ec4..ebd2bb1c8 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1278,7 +1278,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) rx_stats.drop_fullq + rx_stats.drop_bm + rxq->drop_mac; - stats->ibytes += rxq->bytes_recv; drop_mac += rxq->drop_mac; } @@ -1306,7 +1305,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) stats->q_opackets[idx] = tx_stats.deq_desc; stats->q_obytes[idx] = txq->bytes_sent; - stats->obytes += txq->bytes_sent; } ret = pp2_ppio_get_statistics(priv->ppio, &ppio_stats, 0); @@ -1315,6 +1313,8 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) return ret; } + stats->ibytes += ppio_stats.rx_bytes; + stats->obytes += ppio_stats.tx_bytes; stats->ipackets += ppio_stats.rx_packets - drop_mac; stats->opackets += ppio_stats.tx_packets; stats->imissed += ppio_stats.rx_fullq_dropped + -- 2.28.0
From: Liron Himi <lironh@marvell.com> vlan-flush in MUSDK is not supported yet. until it does, the code should be skipped as currently an redundant error message is displayed. Fixes: a8f3d6783 ("net/mrvl: support VLAN filtering") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index ebd2bb1c8..0985ccebe 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -672,15 +672,15 @@ mrvl_dev_start(struct rte_eth_dev *dev) } if (!priv->vlan_flushed) { - ret = pp2_ppio_flush_vlan(priv->ppio); - if (ret) { - MRVL_LOG(ERR, "Failed to flush vlan list"); - /* - * TODO - * once pp2_ppio_flush_vlan() is supported jump to out - * goto out; - */ - } + /* + * TODO + * once pp2_ppio_flush_vlan() is supported call it + * ret = pp2_ppio_flush_vlan(priv->ppio); + * if (ret) { + * MRVL_LOG(ERR, "Failed to flush vlan list"); + * goto out; + * } + */ priv->vlan_flushed = 1; } ret = mrvl_mtu_set(dev, dev->data->mtu); -- 2.28.0
From: Liron Himi <lironh@marvell.com> CRC is being removed by HW before packet get write to the buffer, so CRC len should not be included in MRU validation Fixes: 79ec62028 ("net/mvpp2: update MTU and MRU related calculations") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Yuri Chipchev <yuric@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 0985ccebe..dbb193496 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -441,8 +441,8 @@ mrvl_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) * when this feature has not been enabled/supported so far * (TODO check scattered_rx flag here once scattered RX is supported). */ - if (mru + MRVL_PKT_OFFS > mbuf_data_size) { - mru = mbuf_data_size - MRVL_PKT_OFFS; + if (mru - RTE_ETHER_CRC_LEN + MRVL_PKT_OFFS > mbuf_data_size) { + mru = mbuf_data_size + RTE_ETHER_CRC_LEN - MRVL_PKT_OFFS; mtu = MRVL_PP2_MRU_TO_MTU(mru); MRVL_LOG(WARNING, "MTU too big, max MTU possible limitted " "by current mbuf size: %u. Set MTU to %u, MRU to %u", -- 2.28.0
From: Liron Himi <lironh@marvell.com> need to add CRC len to the frame-size to compare against max_rx_pkt_len which includes it. Fixes: 79ec62028 ("net/mvpp2: update MTU and MRU related calculations") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Yuri Chipchev <yuric@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index dbb193496..3f05ebe00 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1711,7 +1711,8 @@ mrvl_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, return -EFAULT; } - frame_size = buf_size - RTE_PKTMBUF_HEADROOM - MRVL_PKT_EFFEC_OFFS; + frame_size = buf_size - RTE_PKTMBUF_HEADROOM - + MRVL_PKT_EFFEC_OFFS + RTE_ETHER_CRC_LEN; if (frame_size < max_rx_pkt_len) { MRVL_LOG(WARNING, "Mbuf size must be increased to %u bytes to hold up " -- 2.28.0
From: Liron Himi <lironh@marvell.com> Fixes: acab7d58c ("net/mvpp2: convert to dynamic logging") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Dana Tearosh <danat@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 3f05ebe00..d81b86c02 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -2320,7 +2320,7 @@ mrvl_rx_pkt_burst(void *rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) (!rx_done && num < q->priv->bpool_init_size))) { ret = mrvl_fill_bpool(q, MRVL_BURST_SIZE); if (ret) - MRVL_LOG(ERR, "Failed to fill bpool"); + MRVL_LOG(DEBUG, "Failed to fill bpool"); } else if (unlikely(num > q->priv->bpool_max_size)) { int i; int pkt_to_remove = num - q->priv->bpool_init_size; -- 2.28.0
Reviewed-by: Michael Shamis <michaelsh@marvell.com> -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com Sent: Wednesday, December 2, 2020 12:12 PM To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Liron Himi <lironh@marvell.com>; stable@dpdk.org; Yuri Chipchev <yuric@marvell.com> Subject: [dpdk-dev] [PATCH v1 05/38] net/mvpp2: remove CRC len from MRU validation From: Liron Himi <lironh@marvell.com> CRC is being removed by HW before packet get write to the buffer, so CRC len should not be included in MRU validation Fixes: 79ec62028 ("net/mvpp2: update MTU and MRU related calculations") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Yuri Chipchev <yuric@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 0985ccebe..dbb193496 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -441,8 +441,8 @@ mrvl_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) * when this feature has not been enabled/supported so far * (TODO check scattered_rx flag here once scattered RX is supported). */ - if (mru + MRVL_PKT_OFFS > mbuf_data_size) { - mru = mbuf_data_size - MRVL_PKT_OFFS; + if (mru - RTE_ETHER_CRC_LEN + MRVL_PKT_OFFS > mbuf_data_size) { + mru = mbuf_data_size + RTE_ETHER_CRC_LEN - MRVL_PKT_OFFS; mtu = MRVL_PP2_MRU_TO_MTU(mru); MRVL_LOG(WARNING, "MTU too big, max MTU possible limitted " "by current mbuf size: %u. Set MTU to %u, MRU to %u", -- 2.28.0
Reviewed-by: Michael Shamis <michaelsh@marvell.com> -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com Sent: Wednesday, December 2, 2020 12:12 PM To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Liron Himi <lironh@marvell.com>; stable@dpdk.org; Yuri Chipchev <yuric@marvell.com> Subject: [dpdk-dev] [PATCH v1 06/38] net/mvpp2: fix frame size checking From: Liron Himi <lironh@marvell.com> need to add CRC len to the frame-size to compare against max_rx_pkt_len which includes it. Fixes: 79ec62028 ("net/mvpp2: update MTU and MRU related calculations") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Yuri Chipchev <yuric@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index dbb193496..3f05ebe00 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1711,7 +1711,8 @@ mrvl_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, return -EFAULT; } - frame_size = buf_size - RTE_PKTMBUF_HEADROOM - MRVL_PKT_EFFEC_OFFS; + frame_size = buf_size - RTE_PKTMBUF_HEADROOM - + MRVL_PKT_EFFEC_OFFS + RTE_ETHER_CRC_LEN; if (frame_size < max_rx_pkt_len) { MRVL_LOG(WARNING, "Mbuf size must be increased to %u bytes to hold up " -- 2.28.0
Reviewed-by: Michael Shamis <michaelsh@marvell.com> -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com Sent: Wednesday, December 2, 2020 12:12 PM To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Liron Himi <lironh@marvell.com>; stable@dpdk.org; Dana Vardi <danat@marvell.com> Subject: [dpdk-dev] [PATCH v1 07/38] net/mvpp2: reduce prints on rx path From: Liron Himi <lironh@marvell.com> Fixes: acab7d58c ("net/mvpp2: convert to dynamic logging") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Dana Tearosh <danat@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 3f05ebe00..d81b86c02 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -2320,7 +2320,7 @@ mrvl_rx_pkt_burst(void *rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) (!rx_done && num < q->priv->bpool_init_size))) { ret = mrvl_fill_bpool(q, MRVL_BURST_SIZE); if (ret) - MRVL_LOG(ERR, "Failed to fill bpool"); + MRVL_LOG(DEBUG, "Failed to fill bpool"); } else if (unlikely(num > q->priv->bpool_max_size)) { int i; int pkt_to_remove = num - q->priv->bpool_init_size; -- 2.28.0
Reviewed-by: Michael Shamis <michaelsh@marvell.com> -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com Sent: Wednesday, December 2, 2020 12:12 PM To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Liron Himi <lironh@marvell.com>; stable@dpdk.org Subject: [dpdk-dev] [PATCH v1 04/38] net/mvpp2: skip vlan flush From: Liron Himi <lironh@marvell.com> vlan-flush in MUSDK is not supported yet. until it does, the code should be skipped as currently an redundant error message is displayed. Fixes: a8f3d6783 ("net/mrvl: support VLAN filtering") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index ebd2bb1c8..0985ccebe 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -672,15 +672,15 @@ mrvl_dev_start(struct rte_eth_dev *dev) } if (!priv->vlan_flushed) { - ret = pp2_ppio_flush_vlan(priv->ppio); - if (ret) { - MRVL_LOG(ERR, "Failed to flush vlan list"); - /* - * TODO - * once pp2_ppio_flush_vlan() is supported jump to out - * goto out; - */ - } + /* + * TODO + * once pp2_ppio_flush_vlan() is supported call it + * ret = pp2_ppio_flush_vlan(priv->ppio); + * if (ret) { + * MRVL_LOG(ERR, "Failed to flush vlan list"); + * goto out; + * } + */ priv->vlan_flushed = 1; } ret = mrvl_mtu_set(dev, dev->data->mtu); -- 2.28.0
Reviewed-by: Michael Shamis <michaelsh@marvell.com> -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com Sent: Wednesday, December 2, 2020 12:12 PM To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Yuri Chipchev <yuric@marvell.com>; stable@dpdk.org; Liron Himi <lironh@marvell.com> Subject: [dpdk-dev] [PATCH v1 03/38] net/mvpp2: fix rx/tx bytes statistics From: Yuri Chipchev <yuric@marvell.com> 4B of CRC was not included in the bytes statistics Fixes: bdffe0c70 ("net/mrvl: support basic stats") Cc: stable@dpdk.org Signed-off-by: Yuri Chipchev <yuric@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 8c3278ec4..ebd2bb1c8 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1278,7 +1278,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) rx_stats.drop_fullq + rx_stats.drop_bm + rxq->drop_mac; - stats->ibytes += rxq->bytes_recv; drop_mac += rxq->drop_mac; } @@ -1306,7 +1305,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) stats->q_opackets[idx] = tx_stats.deq_desc; stats->q_obytes[idx] = txq->bytes_sent; - stats->obytes += txq->bytes_sent; } ret = pp2_ppio_get_statistics(priv->ppio, &ppio_stats, 0); @@ -1315,6 +1313,8 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) return ret; } + stats->ibytes += ppio_stats.rx_bytes; + stats->obytes += ppio_stats.tx_bytes; stats->ipackets += ppio_stats.rx_packets - drop_mac; stats->opackets += ppio_stats.tx_packets; stats->imissed += ppio_stats.rx_fullq_dropped + -- 2.28.0
Reviewed-by: Michael Shamis <michaelsh@marvell.com> -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com Sent: Wednesday, December 2, 2020 12:12 PM To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Yuri Chipchev <yuric@marvell.com>; stable@dpdk.org; Liron Himi <lironh@marvell.com> Subject: [dpdk-dev] [PATCH v1 01/38] net/mvpp2: fix stack corruption From: Yuri Chipchev <yuric@marvell.com> Fixes stack corruption in mrvl_fill_bpool function in case num > MRVL_PP2_RXD_MAX Fixes: c3637258d894 ("net/mrvl: fix Rx descriptors number") Cc: stable@dpdk.org Signed-off-by: Yuri Chipchev <yuric@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index f25cf9e46..93fb30cdb 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1614,8 +1614,8 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) static int mrvl_fill_bpool(struct mrvl_rxq *rxq, int num) { - struct buff_release_entry entries[MRVL_PP2_RXD_MAX]; - struct rte_mbuf *mbufs[MRVL_PP2_RXD_MAX]; + struct buff_release_entry entries[num]; + struct rte_mbuf *mbufs[num]; int i, ret; unsigned int core_id; struct pp2_hif *hif; -- 2.28.0
Reviewed-by: Michael Shamis <michaelsh@marvell.com> -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com Sent: Wednesday, December 2, 2020 12:12 PM To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Liron Himi <lironh@marvell.com>; stable@dpdk.org Subject: [dpdk-dev] [PATCH v1 02/38] net/mvpp2: remove debug log on fast-path From: Liron Himi <lironh@marvell.com> in case of non-ip frame the current code reached the 'default' case which result with function call to log a msg. those kind of calls should not be performed on fast-path. The performance for this kind of frames increased by 50% Fixes: acab7d58c ("net/mvpp2: convert to dynamic logging") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 93fb30cdb..8c3278ec4 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -2171,7 +2171,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, *l4_offset = *l3_offset + MRVL_ARP_LENGTH; break; default: - MRVL_LOG(DEBUG, "Failed to recognise l3 packet type"); break; } @@ -2183,7 +2182,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, packet_type |= RTE_PTYPE_L4_UDP; break; default: - MRVL_LOG(DEBUG, "Failed to recognise l4 packet type"); break; } -- 2.28.0
On Wed, Dec 2, 2020 at 3:42 PM <lironh@marvell.com> wrote: > > From: Liron Himi <lironh@marvell.com> > > in case of non-ip frame the current code reached the 'default' In case of > case which result with function call to log a msg. > those kind of calls should not be performed on fast-path. > > The performance for this kind of frames increased by 50% > > Fixes: acab7d58c ("net/mvpp2: convert to dynamic logging") > Cc: stable@dpdk.org > > Signed-off-by: Liron Himi <lironh@marvell.com> > Reviewed-by: Liron Himi <lironh@marvell.com> Please remove the Reviewed-by if you have Signed-off, > --- > drivers/net/mvpp2/mrvl_ethdev.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c > index 93fb30cdb..8c3278ec4 100644 > --- a/drivers/net/mvpp2/mrvl_ethdev.c > +++ b/drivers/net/mvpp2/mrvl_ethdev.c > @@ -2171,7 +2171,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, > *l4_offset = *l3_offset + MRVL_ARP_LENGTH; > break; > default: > - MRVL_LOG(DEBUG, "Failed to recognise l3 packet type"); > break; > } > > @@ -2183,7 +2182,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, > packet_type |= RTE_PTYPE_L4_UDP; > break; > default: > - MRVL_LOG(DEBUG, "Failed to recognise l4 packet type"); There are other prints in fastpath such as "MRVL_LOG(ERR, "Failed to receive packets");", please remove those as well. > break; > } > > -- > 2.28.0 >
On Wed, Dec 23, 2020 at 3:15 PM Michael Shamis <michaelsh@marvell.com> wrote: > > Reviewed-by: Michael Shamis <michaelsh@marvell.com> > > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com > Sent: Wednesday, December 2, 2020 12:12 PM > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> > Cc: dev@dpdk.org; Liron Himi <lironh@marvell.com>; stable@dpdk.org > Subject: [dpdk-dev] [PATCH v1 04/38] net/mvpp2: skip vlan flush > > From: Liron Himi <lironh@marvell.com> > > vlan-flush in MUSDK is not supported yet. > until it does, the code should be skipped as currently an redundant error message is displayed. > > Fixes: a8f3d6783 ("net/mrvl: support VLAN filtering") > Cc: stable@dpdk.org > > Signed-off-by: Liron Himi <lironh@marvell.com> > Reviewed-by: Liron Himi <lironh@marvell.com> > --- > drivers/net/mvpp2/mrvl_ethdev.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index ebd2bb1c8..0985ccebe 100644 > --- a/drivers/net/mvpp2/mrvl_ethdev.c > +++ b/drivers/net/mvpp2/mrvl_ethdev.c > @@ -672,15 +672,15 @@ mrvl_dev_start(struct rte_eth_dev *dev) > } > > if (!priv->vlan_flushed) { > - ret = pp2_ppio_flush_vlan(priv->ppio); > - if (ret) { > - MRVL_LOG(ERR, "Failed to flush vlan list"); > - /* > - * TODO > - * once pp2_ppio_flush_vlan() is supported jump to out > - * goto out; > - */ > - } > + /* > + * TODO > + * once pp2_ppio_flush_vlan() is supported call it > + * ret = pp2_ppio_flush_vlan(priv->ppio); > + * if (ret) { > + * MRVL_LOG(ERR, "Failed to flush vlan list"); > + * goto out; > + * } > + */ This kind of TODO is no go in upstream code. I think, MUSDK can be updated to skip the error and have an API detect the feature is present or not? so that drivers can make intelligent decisions and have clean code. > priv->vlan_flushed = 1; > } > ret = mrvl_mtu_set(dev, dev->data->mtu); > -- > 2.28.0 >
On Wed, Dec 23, 2020 at 3:15 PM Michael Shamis <michaelsh@marvell.com> wrote: > > Reviewed-by: Michael Shamis <michaelsh@marvell.com> > > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com > Sent: Wednesday, December 2, 2020 12:12 PM > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> > Cc: dev@dpdk.org; Liron Himi <lironh@marvell.com>; stable@dpdk.org; Dana Vardi <danat@marvell.com> > Subject: [dpdk-dev] [PATCH v1 07/38] net/mvpp2: reduce prints on rx path > > From: Liron Himi <lironh@marvell.com> Please have git commit message for every patch. > > Fixes: acab7d58c ("net/mvpp2: convert to dynamic logging") > Cc: stable@dpdk.org > > Signed-off-by: Liron Himi <lironh@marvell.com> > Reviewed-by: Dana Tearosh <danat@marvell.com> > --- > drivers/net/mvpp2/mrvl_ethdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 3f05ebe00..d81b86c02 100644 > --- a/drivers/net/mvpp2/mrvl_ethdev.c > +++ b/drivers/net/mvpp2/mrvl_ethdev.c > @@ -2320,7 +2320,7 @@ mrvl_rx_pkt_burst(void *rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > (!rx_done && num < q->priv->bpool_init_size))) { > ret = mrvl_fill_bpool(q, MRVL_BURST_SIZE); > if (ret) > - MRVL_LOG(ERR, "Failed to fill bpool"); > + MRVL_LOG(DEBUG, "Failed to fill bpool"); Please squash all log-related patches to a single one. > } else if (unlikely(num > q->priv->bpool_max_size)) { > int i; > int pkt_to_remove = num - q->priv->bpool_init_size; > -- > 2.28.0 >
From: Yuri Chipchev <yuric@marvell.com> Fixes stack corruption in mrvl_fill_bpool function in case num > MRVL_PP2_RXD_MAX Fixes: c3637258d894 ("net/mrvl: fix Rx descriptors number") Cc: stable@dpdk.org Signed-off-by: Yuri Chipchev <yuric@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index f25cf9e46..93fb30cdb 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1614,8 +1614,8 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) static int mrvl_fill_bpool(struct mrvl_rxq *rxq, int num) { - struct buff_release_entry entries[MRVL_PP2_RXD_MAX]; - struct rte_mbuf *mbufs[MRVL_PP2_RXD_MAX]; + struct buff_release_entry entries[num]; + struct rte_mbuf *mbufs[num]; int i, ret; unsigned int core_id; struct pp2_hif *hif; -- 2.28.0
From: Liron Himi <lironh@marvell.com> In case of non-ip frame the current code reached the 'default' case which result with function call to log a msg. those kind of calls should not be performed on fast-path. The performance for this kind of frames increased by 50% Fixes: acab7d58c ("net/mvpp2: convert to dynamic logging") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 93fb30cdb..bfe496ff7 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -2171,7 +2171,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, *l4_offset = *l3_offset + MRVL_ARP_LENGTH; break; default: - MRVL_LOG(DEBUG, "Failed to recognise l3 packet type"); break; } @@ -2183,7 +2182,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, packet_type |= RTE_PTYPE_L4_UDP; break; default: - MRVL_LOG(DEBUG, "Failed to recognise l4 packet type"); break; } @@ -2253,10 +2251,9 @@ mrvl_rx_pkt_burst(void *rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) ret = pp2_ppio_recv(q->priv->ppio, q->priv->rxq_map[q->queue_id].tc, q->priv->rxq_map[q->queue_id].inq, descs, &nb_pkts); - if (unlikely(ret < 0)) { - MRVL_LOG(ERR, "Failed to receive packets"); + if (unlikely(ret < 0)) return 0; - } + mrvl_port_bpool_size[bpool->pp2_id][bpool->id][core_id] -= nb_pkts; for (i = 0; i < nb_pkts; i++) { @@ -2319,21 +2316,13 @@ mrvl_rx_pkt_burst(void *rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) if (unlikely(num <= q->priv->bpool_min_size || (!rx_done && num < q->priv->bpool_init_size))) { - ret = mrvl_fill_bpool(q, MRVL_BURST_SIZE); - if (ret) - MRVL_LOG(ERR, "Failed to fill bpool"); + mrvl_fill_bpool(q, MRVL_BURST_SIZE); } else if (unlikely(num > q->priv->bpool_max_size)) { int i; int pkt_to_remove = num - q->priv->bpool_init_size; struct rte_mbuf *mbuf; struct pp2_buff_inf buff; - MRVL_LOG(DEBUG, - "port-%d:%d: bpool %d oversize - remove %d buffers (pool size: %d -> %d)", - bpool->pp2_id, q->priv->ppio->port_id, - bpool->id, pkt_to_remove, num, - q->priv->bpool_init_size); - for (i = 0; i < pkt_to_remove; i++) { ret = pp2_bpool_get_buff(hif, bpool, &buff); if (ret) @@ -2526,12 +2515,8 @@ mrvl_tx_pkt_burst(void *txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) sq, q->queue_id, 0); sq_free_size = MRVL_PP2_TX_SHADOWQ_SIZE - sq->size - 1; - if (unlikely(nb_pkts > sq_free_size)) { - MRVL_LOG(DEBUG, - "No room in shadow queue for %d packets! %d packets will be sent.", - nb_pkts, sq_free_size); + if (unlikely(nb_pkts > sq_free_size)) nb_pkts = sq_free_size; - } for (i = 0; i < nb_pkts; i++) { struct rte_mbuf *mbuf = tx_pkts[i]; @@ -2648,10 +2633,6 @@ mrvl_tx_sg_pkt_burst(void *txq, struct rte_mbuf **tx_pkts, */ if (unlikely(total_descs > sq_free_size)) { total_descs -= nb_segs; - RTE_LOG(DEBUG, PMD, - "No room in shadow queue for %d packets! " - "%d packets will be sent.\n", - nb_pkts, i); break; } -- 2.28.0
From: Yuri Chipchev <yuric@marvell.com> 4B of CRC was not included in the bytes statistics Fixes: bdffe0c70 ("net/mrvl: support basic stats") Cc: stable@dpdk.org Signed-off-by: Yuri Chipchev <yuric@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index bfe496ff7..a1a3c0e31 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1278,7 +1278,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) rx_stats.drop_fullq + rx_stats.drop_bm + rxq->drop_mac; - stats->ibytes += rxq->bytes_recv; drop_mac += rxq->drop_mac; } @@ -1306,7 +1305,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) stats->q_opackets[idx] = tx_stats.deq_desc; stats->q_obytes[idx] = txq->bytes_sent; - stats->obytes += txq->bytes_sent; } ret = pp2_ppio_get_statistics(priv->ppio, &ppio_stats, 0); @@ -1315,6 +1313,8 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) return ret; } + stats->ibytes += ppio_stats.rx_bytes; + stats->obytes += ppio_stats.tx_bytes; stats->ipackets += ppio_stats.rx_packets - drop_mac; stats->opackets += ppio_stats.tx_packets; stats->imissed += ppio_stats.rx_fullq_dropped + -- 2.28.0
From: Liron Himi <lironh@marvell.com> VLAN-flush in MUSDK is not supported yet. until it does, the code should be removed as currently an redundant error message is displayed. Fixes: a8f3d6783 ("net/mrvl: support VLAN filtering") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 12 ------------ drivers/net/mvpp2/mrvl_ethdev.h | 1 - 2 files changed, 13 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index a1a3c0e31..559379fcd 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -671,18 +671,6 @@ mrvl_dev_start(struct rte_eth_dev *dev) priv->uc_mc_flushed = 1; } - if (!priv->vlan_flushed) { - ret = pp2_ppio_flush_vlan(priv->ppio); - if (ret) { - MRVL_LOG(ERR, "Failed to flush vlan list"); - /* - * TODO - * once pp2_ppio_flush_vlan() is supported jump to out - * goto out; - */ - } - priv->vlan_flushed = 1; - } ret = mrvl_mtu_set(dev, dev->data->mtu); if (ret) MRVL_LOG(ERR, "Failed to set MTU to %d", dev->data->mtu); diff --git a/drivers/net/mvpp2/mrvl_ethdev.h b/drivers/net/mvpp2/mrvl_ethdev.h index db6632f5b..eee5182ce 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.h +++ b/drivers/net/mvpp2/mrvl_ethdev.h @@ -186,7 +186,6 @@ struct mrvl_priv { uint8_t bpool_bit; uint8_t rss_hf_tcp; uint8_t uc_mc_flushed; - uint8_t vlan_flushed; uint8_t isolated; uint8_t multiseg; -- 2.28.0
From: Liron Himi <lironh@marvell.com> CRC is being removed by HW before packet get write to the buffer, so CRC len should not be included in MRU validation Fixes: 79ec62028 ("net/mvpp2: update MTU and MRU related calculations") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Yuri Chipchev <yuric@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 559379fcd..133006f3e 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -441,8 +441,8 @@ mrvl_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) * when this feature has not been enabled/supported so far * (TODO check scattered_rx flag here once scattered RX is supported). */ - if (mru + MRVL_PKT_OFFS > mbuf_data_size) { - mru = mbuf_data_size - MRVL_PKT_OFFS; + if (mru - RTE_ETHER_CRC_LEN + MRVL_PKT_OFFS > mbuf_data_size) { + mru = mbuf_data_size + RTE_ETHER_CRC_LEN - MRVL_PKT_OFFS; mtu = MRVL_PP2_MRU_TO_MTU(mru); MRVL_LOG(WARNING, "MTU too big, max MTU possible limitted " "by current mbuf size: %u. Set MTU to %u, MRU to %u", -- 2.28.0
From: Liron Himi <lironh@marvell.com> Need to add CRC len to the frame-size to compare against max_rx_pkt_len which includes it. Fixes: 79ec62028 ("net/mvpp2: update MTU and MRU related calculations") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Yuri Chipchev <yuric@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 133006f3e..f7e3f62e3 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1699,7 +1699,8 @@ mrvl_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, return -EFAULT; } - frame_size = buf_size - RTE_PKTMBUF_HEADROOM - MRVL_PKT_EFFEC_OFFS; + frame_size = buf_size - RTE_PKTMBUF_HEADROOM - + MRVL_PKT_EFFEC_OFFS + RTE_ETHER_CRC_LEN; if (frame_size < max_rx_pkt_len) { MRVL_LOG(WARNING, "Mbuf size must be increased to %u bytes to hold up " -- 2.28.0
On 1/22/2021 7:18 PM, lironh@marvell.com wrote: > From: Yuri Chipchev <yuric@marvell.com> > > 4B of CRC was not included in the bytes statistics > > Fixes: bdffe0c70 ("net/mrvl: support basic stats") > Cc: stable@dpdk.org > > Signed-off-by: Yuri Chipchev <yuric@marvell.com> > Reviewed-by: Liron Himi <lironh@marvell.com> > --- > drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c > index bfe496ff7..a1a3c0e31 100644 > --- a/drivers/net/mvpp2/mrvl_ethdev.c > +++ b/drivers/net/mvpp2/mrvl_ethdev.c > @@ -1278,7 +1278,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > rx_stats.drop_fullq + > rx_stats.drop_bm + > rxq->drop_mac; > - stats->ibytes += rxq->bytes_recv; > drop_mac += rxq->drop_mac; > } > > @@ -1306,7 +1305,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > > stats->q_opackets[idx] = tx_stats.deq_desc; > stats->q_obytes[idx] = txq->bytes_sent; > - stats->obytes += txq->bytes_sent; > } > > ret = pp2_ppio_get_statistics(priv->ppio, &ppio_stats, 0); > @@ -1315,6 +1313,8 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > return ret; > } > > + stats->ibytes += ppio_stats.rx_bytes; > + stats->obytes += ppio_stats.tx_bytes; > stats->ipackets += ppio_stats.rx_packets - drop_mac; > stats->opackets += ppio_stats.tx_packets; > stats->imissed += ppio_stats.rx_fullq_dropped + > Is this patch adding CRC to the stats? If so that is wrong, stats should not have the CRC. A recent clarification from Andrew: https://patches.dpdk.org/patch/86975/
-----Original Message----- From: Ferruh Yigit <ferruh.yigit@intel.com> Sent: Tuesday, 26 January 2021 19:03 To: Liron Himi <lironh@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Yuri Chipchev <yuric@marvell.com>; stable@dpdk.org; Andrew Rybchenko <arybchenko@solarflare.com> Subject: [EXT] Re: [dpdk-stable] [PATCH v2 03/37] net/mvpp2: fix Rx/Tx bytes statistics External Email ---------------------------------------------------------------------- On 1/22/2021 7:18 PM, lironh@marvell.com wrote: > From: Yuri Chipchev <yuric@marvell.com> > > 4B of CRC was not included in the bytes statistics > > Fixes: bdffe0c70 ("net/mrvl: support basic stats") > Cc: stable@dpdk.org > > Signed-off-by: Yuri Chipchev <yuric@marvell.com> > Reviewed-by: Liron Himi <lironh@marvell.com> > --- > drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/mvpp2/mrvl_ethdev.c > b/drivers/net/mvpp2/mrvl_ethdev.c index bfe496ff7..a1a3c0e31 100644 > --- a/drivers/net/mvpp2/mrvl_ethdev.c > +++ b/drivers/net/mvpp2/mrvl_ethdev.c > @@ -1278,7 +1278,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > rx_stats.drop_fullq + > rx_stats.drop_bm + > rxq->drop_mac; > - stats->ibytes += rxq->bytes_recv; > drop_mac += rxq->drop_mac; > } > > @@ -1306,7 +1305,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct > rte_eth_stats *stats) > > stats->q_opackets[idx] = tx_stats.deq_desc; > stats->q_obytes[idx] = txq->bytes_sent; > - stats->obytes += txq->bytes_sent; > } > > ret = pp2_ppio_get_statistics(priv->ppio, &ppio_stats, 0); @@ > -1315,6 +1313,8 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > return ret; > } > > + stats->ibytes += ppio_stats.rx_bytes; > + stats->obytes += ppio_stats.tx_bytes; > stats->ipackets += ppio_stats.rx_packets - drop_mac; > stats->opackets += ppio_stats.tx_packets; > stats->imissed += ppio_stats.rx_fullq_dropped + > Is this patch adding CRC to the stats? If so that is wrong, stats should not have the CRC. A recent clarification from Andrew: https://urldefense.proofpoint.com/v2/url?u=https-3A__patches.dpdk.org_patch_86975_&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=M6lEONY3jVnAkBjt0yC6oAsT7Jn2HI7LhNHCQYlnx1Q&m=iYhQSv2FUrmxLNQcxo6cAiLpoeqeTx1mQKadLot0jh0&s=lYaS0RY6n4QOlhzd3Z9PTxtR42lbqxPt1Pm1uODpWJg&e= [L.H.] okay, so based on this new clarification, this patch should not be merged to both next and stable branches. thanks
On 1/26/2021 5:25 PM, Liron Himi wrote:
>
>
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, 26 January 2021 19:03
> To: Liron Himi <lironh@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: dev@dpdk.org; Yuri Chipchev <yuric@marvell.com>; stable@dpdk.org; Andrew Rybchenko <arybchenko@solarflare.com>
> Subject: [EXT] Re: [dpdk-stable] [PATCH v2 03/37] net/mvpp2: fix Rx/Tx bytes statistics
>
> External Email
>
> ----------------------------------------------------------------------
> On 1/22/2021 7:18 PM, lironh@marvell.com wrote:
>> From: Yuri Chipchev <yuric@marvell.com>
>>
>> 4B of CRC was not included in the bytes statistics
>>
>> Fixes: bdffe0c70 ("net/mrvl: support basic stats")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Yuri Chipchev <yuric@marvell.com>
>> Reviewed-by: Liron Himi <lironh@marvell.com>
>> ---
>> drivers/net/mvpp2/mrvl_ethdev.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/mvpp2/mrvl_ethdev.c
>> b/drivers/net/mvpp2/mrvl_ethdev.c index bfe496ff7..a1a3c0e31 100644
>> --- a/drivers/net/mvpp2/mrvl_ethdev.c
>> +++ b/drivers/net/mvpp2/mrvl_ethdev.c
>> @@ -1278,7 +1278,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>> rx_stats.drop_fullq +
>> rx_stats.drop_bm +
>> rxq->drop_mac;
>> - stats->ibytes += rxq->bytes_recv;
>> drop_mac += rxq->drop_mac;
>> }
>>
>> @@ -1306,7 +1305,6 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct
>> rte_eth_stats *stats)
>>
>> stats->q_opackets[idx] = tx_stats.deq_desc;
>> stats->q_obytes[idx] = txq->bytes_sent;
>> - stats->obytes += txq->bytes_sent;
>> }
>>
>> ret = pp2_ppio_get_statistics(priv->ppio, &ppio_stats, 0); @@
>> -1315,6 +1313,8 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>> return ret;
>> }
>>
>> + stats->ibytes += ppio_stats.rx_bytes;
>> + stats->obytes += ppio_stats.tx_bytes;
>> stats->ipackets += ppio_stats.rx_packets - drop_mac;
>> stats->opackets += ppio_stats.tx_packets;
>> stats->imissed += ppio_stats.rx_fullq_dropped +
>>
>
> Is this patch adding CRC to the stats? If so that is wrong, stats should not have the CRC.
>
> A recent clarification from Andrew:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__patches.dpdk.org_patch_86975_&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=M6lEONY3jVnAkBjt0yC6oAsT7Jn2HI7LhNHCQYlnx1Q&m=iYhQSv2FUrmxLNQcxo6cAiLpoeqeTx1mQKadLot0jh0&s=lYaS0RY6n4QOlhzd3Z9PTxtR42lbqxPt1Pm1uODpWJg&e=
>
> [L.H.] okay, so based on this new clarification, this patch should not be merged to both next and stable branches.
> thanks
>
sure, I can drop it in next-net.
From: Yuri Chipchev <yuric@marvell.com> Fixes stack corruption in mrvl_fill_bpool function in case num > MRVL_PP2_RXD_MAX Fixes: c3637258d894 ("net/mrvl: fix Rx descriptors number") Cc: stable@dpdk.org Signed-off-by: Yuri Chipchev <yuric@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index f25cf9e46d..93fb30cdb8 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1614,8 +1614,8 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) static int mrvl_fill_bpool(struct mrvl_rxq *rxq, int num) { - struct buff_release_entry entries[MRVL_PP2_RXD_MAX]; - struct rte_mbuf *mbufs[MRVL_PP2_RXD_MAX]; + struct buff_release_entry entries[num]; + struct rte_mbuf *mbufs[num]; int i, ret; unsigned int core_id; struct pp2_hif *hif; -- 2.28.0
From: Liron Himi <lironh@marvell.com> In case of non-ip frame the current code reached the 'default' case which result with function call to log a msg. those kind of calls should not be performed on fast-path. The performance for this kind of frames increased by 50% Fixes: acab7d58c ("net/mvpp2: convert to dynamic logging") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 93fb30cdb8..bfe496ff74 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -2171,7 +2171,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, *l4_offset = *l3_offset + MRVL_ARP_LENGTH; break; default: - MRVL_LOG(DEBUG, "Failed to recognise l3 packet type"); break; } @@ -2183,7 +2182,6 @@ mrvl_desc_to_packet_type_and_offset(struct pp2_ppio_desc *desc, packet_type |= RTE_PTYPE_L4_UDP; break; default: - MRVL_LOG(DEBUG, "Failed to recognise l4 packet type"); break; } @@ -2253,10 +2251,9 @@ mrvl_rx_pkt_burst(void *rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) ret = pp2_ppio_recv(q->priv->ppio, q->priv->rxq_map[q->queue_id].tc, q->priv->rxq_map[q->queue_id].inq, descs, &nb_pkts); - if (unlikely(ret < 0)) { - MRVL_LOG(ERR, "Failed to receive packets"); + if (unlikely(ret < 0)) return 0; - } + mrvl_port_bpool_size[bpool->pp2_id][bpool->id][core_id] -= nb_pkts; for (i = 0; i < nb_pkts; i++) { @@ -2319,21 +2316,13 @@ mrvl_rx_pkt_burst(void *rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) if (unlikely(num <= q->priv->bpool_min_size || (!rx_done && num < q->priv->bpool_init_size))) { - ret = mrvl_fill_bpool(q, MRVL_BURST_SIZE); - if (ret) - MRVL_LOG(ERR, "Failed to fill bpool"); + mrvl_fill_bpool(q, MRVL_BURST_SIZE); } else if (unlikely(num > q->priv->bpool_max_size)) { int i; int pkt_to_remove = num - q->priv->bpool_init_size; struct rte_mbuf *mbuf; struct pp2_buff_inf buff; - MRVL_LOG(DEBUG, - "port-%d:%d: bpool %d oversize - remove %d buffers (pool size: %d -> %d)", - bpool->pp2_id, q->priv->ppio->port_id, - bpool->id, pkt_to_remove, num, - q->priv->bpool_init_size); - for (i = 0; i < pkt_to_remove; i++) { ret = pp2_bpool_get_buff(hif, bpool, &buff); if (ret) @@ -2526,12 +2515,8 @@ mrvl_tx_pkt_burst(void *txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) sq, q->queue_id, 0); sq_free_size = MRVL_PP2_TX_SHADOWQ_SIZE - sq->size - 1; - if (unlikely(nb_pkts > sq_free_size)) { - MRVL_LOG(DEBUG, - "No room in shadow queue for %d packets! %d packets will be sent.", - nb_pkts, sq_free_size); + if (unlikely(nb_pkts > sq_free_size)) nb_pkts = sq_free_size; - } for (i = 0; i < nb_pkts; i++) { struct rte_mbuf *mbuf = tx_pkts[i]; @@ -2648,10 +2633,6 @@ mrvl_tx_sg_pkt_burst(void *txq, struct rte_mbuf **tx_pkts, */ if (unlikely(total_descs > sq_free_size)) { total_descs -= nb_segs; - RTE_LOG(DEBUG, PMD, - "No room in shadow queue for %d packets! " - "%d packets will be sent.\n", - nb_pkts, i); break; } -- 2.28.0
From: Liron Himi <lironh@marvell.com> VLAN-flush in MUSDK is not supported yet. until it does, the code should be removed as currently an redundant error message is displayed. Fixes: a8f3d6783 ("net/mrvl: support VLAN filtering") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 12 ------------ drivers/net/mvpp2/mrvl_ethdev.h | 1 - 2 files changed, 13 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index bfe496ff74..0c338c0c00 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -671,18 +671,6 @@ mrvl_dev_start(struct rte_eth_dev *dev) priv->uc_mc_flushed = 1; } - if (!priv->vlan_flushed) { - ret = pp2_ppio_flush_vlan(priv->ppio); - if (ret) { - MRVL_LOG(ERR, "Failed to flush vlan list"); - /* - * TODO - * once pp2_ppio_flush_vlan() is supported jump to out - * goto out; - */ - } - priv->vlan_flushed = 1; - } ret = mrvl_mtu_set(dev, dev->data->mtu); if (ret) MRVL_LOG(ERR, "Failed to set MTU to %d", dev->data->mtu); diff --git a/drivers/net/mvpp2/mrvl_ethdev.h b/drivers/net/mvpp2/mrvl_ethdev.h index db6632f5b6..eee5182ce8 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.h +++ b/drivers/net/mvpp2/mrvl_ethdev.h @@ -186,7 +186,6 @@ struct mrvl_priv { uint8_t bpool_bit; uint8_t rss_hf_tcp; uint8_t uc_mc_flushed; - uint8_t vlan_flushed; uint8_t isolated; uint8_t multiseg; -- 2.28.0
From: Liron Himi <lironh@marvell.com> CRC is being removed by HW before packet get write to the buffer, so CRC len should not be included in MRU validation Fixes: 79ec62028 ("net/mvpp2: update MTU and MRU related calculations") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Yuri Chipchev <yuric@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 0c338c0c00..eafe4f7ac9 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -441,8 +441,8 @@ mrvl_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) * when this feature has not been enabled/supported so far * (TODO check scattered_rx flag here once scattered RX is supported). */ - if (mru + MRVL_PKT_OFFS > mbuf_data_size) { - mru = mbuf_data_size - MRVL_PKT_OFFS; + if (mru - RTE_ETHER_CRC_LEN + MRVL_PKT_OFFS > mbuf_data_size) { + mru = mbuf_data_size + RTE_ETHER_CRC_LEN - MRVL_PKT_OFFS; mtu = MRVL_PP2_MRU_TO_MTU(mru); MRVL_LOG(WARNING, "MTU too big, max MTU possible limitted " "by current mbuf size: %u. Set MTU to %u, MRU to %u", -- 2.28.0
From: Liron Himi <lironh@marvell.com> Need to add CRC len to the frame-size to compare against max_rx_pkt_len which includes it. Fixes: 79ec62028 ("net/mvpp2: update MTU and MRU related calculations") Cc: stable@dpdk.org Signed-off-by: Liron Himi <lironh@marvell.com> Reviewed-by: Yuri Chipchev <yuric@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index eafe4f7ac9..6cd5acd337 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1699,7 +1699,8 @@ mrvl_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, return -EFAULT; } - frame_size = buf_size - RTE_PKTMBUF_HEADROOM - MRVL_PKT_EFFEC_OFFS; + frame_size = buf_size - RTE_PKTMBUF_HEADROOM - + MRVL_PKT_EFFEC_OFFS + RTE_ETHER_CRC_LEN; if (frame_size < max_rx_pkt_len) { MRVL_LOG(WARNING, "Mbuf size must be increased to %u bytes to hold up " -- 2.28.0