diff mbox series

[v1,5/5] net/i40e: fix max mtu size packets with vlan tag cannot be received by default

Message ID 20200916055212.25019-6-stevex.yang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series fix default max mtu size when device configured |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS

Commit Message

Steve Yang Sept. 16, 2020, 5:52 a.m. UTC 
  testpmd will initialize default max packet length to 1518 which does'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. But for i40e/i40evf,
they should support dual vlan tags that need more 8 bytes for max packet
size, so, configure the correct max packet size in dev_config ops.

Fixes: ff8282f4bbcd ("net/i40e: consider QinQ when setting MTU")

Signed-off-by: SteveX Yang <stevex.yang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c    | 10 ++++++++++
 drivers/net/i40e/i40e_ethdev_vf.c | 11 +++++++++++
 2 files changed, 21 insertions(+)

Comments

Ananyev, Konstantin Sept. 16, 2020, 2:41 p.m. UTC | #1 
> testpmd will initialize default max packet length to 1518 which does'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. But for i40e/i40evf,
> they should support dual vlan tags that need more 8 bytes for max packet
> size, so, configure the correct max packet size in dev_config ops.
> 
> Fixes: ff8282f4bbcd ("net/i40e: consider QinQ when setting MTU")
> 
> Signed-off-by: SteveX Yang <stevex.yang@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c    | 10 ++++++++++
>  drivers/net/i40e/i40e_ethdev_vf.c | 11 +++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 841447228..787ff61c0 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1917,6 +1917,7 @@ i40e_dev_configure(struct rte_eth_dev *dev)
>  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	enum rte_eth_rx_mq_mode mq_mode = dev->data->dev_conf.rxmode.mq_mode;
>  	int i, ret;
> +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> 
>  	ret = i40e_dev_sync_phy_type(hw);
>  	if (ret)
> @@ -1930,6 +1931,15 @@ i40e_dev_configure(struct rte_eth_dev *dev)
>  	ad->tx_simple_allowed = true;
>  	ad->tx_vec_allowed = true;
> 
> +	/* Considering QinQ packet, max frame size should be MTU and
> +	 * corresponding ether overhead.
> +	 */
> +	if (dev->data->mtu == RTE_ETHER_MTU &&
> +		rxmode->max_rx_pkt_len == RTE_ETHER_MAX_LEN) {

Wonder why that particular max_rx_pkt_len and mtu values are important?
Shouldn't we always do here same calculations as we do in i40e_dev_mtu_set()?


> +		rxmode->max_rx_pkt_len = RTE_ETHER_MTU + I40E_ETH_OVERHEAD;
> +		rxmode->offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> +	}
> +
>  	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
>  		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index b755350cd..7410563db 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1669,6 +1669,7 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
>  		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
>  	uint16_t num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
>  				dev->data->nb_tx_queues);
> +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> 
>  	/* Initialize to TRUE. If any of Rx queues doesn't meet the bulk
>  	 * allocation or vector Rx preconditions we will reset it.
> @@ -1681,6 +1682,16 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
>  	dev->data->dev_conf.intr_conf.lsc =
>  		!!(dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC);
> 
> +
> +	/* Considering QinQ packet, max frame size should be MTU and
> +	 * corresponding ether overhead.
> +	 */
> +	if (dev->data->mtu == RTE_ETHER_MTU &&
> +		rxmode->max_rx_pkt_len == RTE_ETHER_MAX_LEN) {
> +		rxmode->max_rx_pkt_len = RTE_ETHER_MTU + I40E_ETH_OVERHEAD;
> +		rxmode->offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> +	}
> +
>  	if (num_queue_pairs > vf->vsi_res->num_queue_pairs) {
>  		struct i40e_hw *hw;
>  		int ret;
> --
> 2.17.1
Ananyev, Konstantin Sept. 17, 2020, 12:18 p.m. UTC | #2 
> > Subject: RE: [dpdk-dev] [PATCH v1 5/5] net/i40e: fix max mtu size packets
> > with vlan tag cannot be received by default
> >
> > > testpmd will initialize default max packet length to 1518 which does'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. But for
> > > i40e/i40evf, they should support dual vlan tags that need more 8 bytes
> > > for max packet size, so, configure the correct max packet size in dev_config
> > ops.
> > >
> > > Fixes: ff8282f4bbcd ("net/i40e: consider QinQ when setting MTU")
> > >
> > > Signed-off-by: SteveX Yang <stevex.yang@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_ethdev.c    | 10 ++++++++++
> > >  drivers/net/i40e/i40e_ethdev_vf.c | 11 +++++++++++
> > >  2 files changed, 21 insertions(+)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 841447228..787ff61c0 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -1917,6 +1917,7 @@ i40e_dev_configure(struct rte_eth_dev *dev)
> > >  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > >  	enum rte_eth_rx_mq_mode mq_mode = dev->data-
> > >dev_conf.rxmode.mq_mode;
> > >  	int i, ret;
> > > +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> > >
> > >  	ret = i40e_dev_sync_phy_type(hw);
> > >  	if (ret)
> > > @@ -1930,6 +1931,15 @@ i40e_dev_configure(struct rte_eth_dev *dev)
> > >  	ad->tx_simple_allowed = true;
> > >  	ad->tx_vec_allowed = true;
> > >
> > > +	/* Considering QinQ packet, max frame size should be MTU and
> > > +	 * corresponding ether overhead.
> > > +	 */
> > > +	if (dev->data->mtu == RTE_ETHER_MTU &&
> > > +		rxmode->max_rx_pkt_len == RTE_ETHER_MAX_LEN) {
> >
> > Wonder why that particular max_rx_pkt_len and mtu values are important?
> > Shouldn't we always do here same calculations as we do in
> > i40e_dev_mtu_set()?
> 
> The combination of RTE_ETHER_MTU (1500) & RTE_ETHER_MAX_LEN (1518) is
> the generical default value from test-pmd or other apps. the RTE_ETHER_MAX_LEN
> doesn't include VLAN tag(s) size, hence, only need adjust frame size to hold real mtu
> size packet for the particular condition.

Ok, but user can overwrite default values in dev_configure.
What would happen if user would set rxmode.max_rx_pkt_len to RTE_ETHER_MAX_LEN + 1
or RTE_ETHER_MAX_LEN - 1?

> 
> >
> >
> > > +		rxmode->max_rx_pkt_len = RTE_ETHER_MTU +
> > I40E_ETH_OVERHEAD;
> > > +		rxmode->offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> > > +	}
> > > +
> > >  	if (dev->data->dev_conf.rxmode.mq_mode &
> > ETH_MQ_RX_RSS_FLAG)
> > >  		dev->data->dev_conf.rxmode.offloads |=
> > DEV_RX_OFFLOAD_RSS_HASH;
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > > b/drivers/net/i40e/i40e_ethdev_vf.c
> > > index b755350cd..7410563db 100644
> > > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > > @@ -1669,6 +1669,7 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
> > >  		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > >  	uint16_t num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
> > >  				dev->data->nb_tx_queues);
> > > +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> > >
> > >  	/* Initialize to TRUE. If any of Rx queues doesn't meet the bulk
> > >  	 * allocation or vector Rx preconditions we will reset it.
> > > @@ -1681,6 +1682,16 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
> > >  	dev->data->dev_conf.intr_conf.lsc =
> > >  		!!(dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC);
> > >
> > > +
> > > +	/* Considering QinQ packet, max frame size should be MTU and
> > > +	 * corresponding ether overhead.
> > > +	 */
> > > +	if (dev->data->mtu == RTE_ETHER_MTU &&
> > > +		rxmode->max_rx_pkt_len == RTE_ETHER_MAX_LEN) {
> > > +		rxmode->max_rx_pkt_len = RTE_ETHER_MTU +
> > I40E_ETH_OVERHEAD;
> > > +		rxmode->offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> > > +	}
> > > +
> > >  	if (num_queue_pairs > vf->vsi_res->num_queue_pairs) {
> > >  		struct i40e_hw *hw;
> > >  		int ret;
> > > --
> > > 2.17.1
diff mbox series

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 841447228..787ff61c0 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1917,6 +1917,7 @@  i40e_dev_configure(struct rte_eth_dev *dev)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	enum rte_eth_rx_mq_mode mq_mode = dev->data->dev_conf.rxmode.mq_mode;
 	int i, ret;
+	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 
 	ret = i40e_dev_sync_phy_type(hw);
 	if (ret)
@@ -1930,6 +1931,15 @@  i40e_dev_configure(struct rte_eth_dev *dev)
 	ad->tx_simple_allowed = true;
 	ad->tx_vec_allowed = true;
 
+	/* Considering QinQ packet, max frame size should be MTU and
+	 * corresponding ether overhead.
+	 */
+	if (dev->data->mtu == RTE_ETHER_MTU &&
+		rxmode->max_rx_pkt_len == RTE_ETHER_MAX_LEN) {
+		rxmode->max_rx_pkt_len = RTE_ETHER_MTU + I40E_ETH_OVERHEAD;
+		rxmode->offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+	}
+
 	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
 		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index b755350cd..7410563db 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1669,6 +1669,7 @@  i40evf_dev_configure(struct rte_eth_dev *dev)
 		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	uint16_t num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
 				dev->data->nb_tx_queues);
+	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 
 	/* Initialize to TRUE. If any of Rx queues doesn't meet the bulk
 	 * allocation or vector Rx preconditions we will reset it.
@@ -1681,6 +1682,16 @@  i40evf_dev_configure(struct rte_eth_dev *dev)
 	dev->data->dev_conf.intr_conf.lsc =
 		!!(dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC);
 
+
+	/* Considering QinQ packet, max frame size should be MTU and
+	 * corresponding ether overhead.
+	 */
+	if (dev->data->mtu == RTE_ETHER_MTU &&
+		rxmode->max_rx_pkt_len == RTE_ETHER_MAX_LEN) {
+		rxmode->max_rx_pkt_len = RTE_ETHER_MTU + I40E_ETH_OVERHEAD;
+		rxmode->offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+	}
+
 	if (num_queue_pairs > vf->vsi_res->num_queue_pairs) {
 		struct i40e_hw *hw;
 		int ret;