Thread (62 messages) 62 messages, 6 authors, 2020-11-05

Re: [dpdk-dev] [PATCH v4 3/5] net/ice: fix max mtu size packets with vlan tag cannot be received by default

From: Zhang, Qi Z <hidden>
Date: 2020-09-30 00:34:46

-----Original Message-----
From: Ananyev, Konstantin <redacted>
Sent: Wednesday, September 30, 2020 7:02 AM
To: Zhang, Qi Z <redacted>; Yang, SteveX
[off-list ref]; dev@dpdk.org
Cc: Zhao1, Wei <redacted>; Guo, Jia <redacted>; Yang,
Qiming [off-list ref]; Wu, Jingjing [off-list ref]; Xing,
Beilei [off-list ref]
Subject: RE: [PATCH v4 3/5] net/ice: fix max mtu size packets with vlan tag
cannot be received by default
quoted
quoted
-----Original Message-----
From: Yang, SteveX <redacted>
Sent: Monday, September 28, 2020 2:56 PM
To: dev@dpdk.org
Cc: Zhao1, Wei <redacted>; Guo, Jia <redacted>;
Yang, Qiming [off-list ref]; Zhang, Qi Z
[off-list ref]; Wu, Jingjing [off-list ref]; Xing,
Beilei [off-list ref]; Ananyev, Konstantin
[off-list ref]; Yang, SteveX [off-list ref]
Subject: [PATCH v4 3/5] net/ice: fix max mtu size packets with vlan
tag cannot be received by default

testpmd will initialize default max packet length to 1518 which
doesn't include vlan tag size in ether overheader. Once, send the
max mtu length packet with vlan tag, the max packet length will
exceed 1518 that will cause packets dropped directly from NIC hw side.

ice can support dual vlan tags that need more 8 bytes for max packet
size, so, configures the correct max packet size in dev_config ops.

Fixes: 50cc9d2a6e9d ("net/ice: fix max frame size")

Signed-off-by: SteveX Yang <redacted>
---
 drivers/net/ice/ice_ethdev.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
diff --git a/drivers/net/ice/ice_ethdev.c
b/drivers/net/ice/ice_ethdev.c index
cfd357b05..6b7098444 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -3146,6 +3146,7 @@ ice_dev_configure(struct rte_eth_dev *dev)
struct ice_adapter *ad =
ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+uint32_t frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
 int ret;

 /* Initialize to TRUE. If any of Rx queues doesn't meet the @@
-3157,6
+3158,16 @@ ice_dev_configure(struct rte_eth_dev *dev)
 if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;

+/**
+ * Considering QinQ packet, max frame size should be equal or
+ * larger than total size of MTU and Ether overhead.
+ */
quoted
+if (frame_size > dev->data->dev_conf.rxmode.max_rx_pkt_len) {

Why we need this check?
Can we just call ice_mtu_set directly
I think that without that check we can silently overwrite provided by user
dev_conf.rxmode.max_rx_pkt_len value.
OK, I see

But still have one question
dev->data->mtu is initialized to 1518 as default , but if application set dev_conf.rxmode.max_rx_pkt_len = 1000 in dev_configure.
does that mean we will still will set mtu to 1518, is this expected?

Should we just call ice_mtu_set(dev, dev_conf.rxmode.max_rx_pkt_len) here?


quoted
And please remove above comment, since ether overhead is already
considered in ice_mtu_set.
quoted
quoted
+ret = ice_mtu_set(dev, dev->data->mtu); if (ret != 0) return ret; }
+
 ret = ice_init_rss(pf);
 if (ret) {
 PMD_DRV_LOG(ERR, "Failed to enable rss for PF");
--
2.17.1
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help