[PATCH] net/i40e: rework maximum frame size configuration

[PATCH] net/i40e: rework maximum frame size configuration

[Simei Su]

This patch removes unnecessary link status check.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 47 +++++++++---------------------------------
 1 file changed, 10 insertions(+), 37 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d..e21e4d9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
 	const struct rte_eth_ethertype_filter *input,
@@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
-	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-	i40e_set_mac_max_frame(dev, max_frame_size);
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Fail to set mac config");
+		return ret;
+	}
 
 	return I40E_SUCCESS;
 
@@ -12123,40 +12130,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
 	return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t rep_cnt = MAX_REPEAT_TIME;
-	struct rte_eth_link link;
-	enum i40e_status_code status;
-	bool can_be_set = true;
-
-	/*
-	 * I40E_MEDIA_TYPE_BASET link up can be ignored
-	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-	 * is I40E_MEDIA_TYPE_UNKNOWN
-	 */
-	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-		do {
-			update_link_reg(hw, &link);
-			if (link.link_status)
-				break;
-			rte_delay_ms(CHECK_INTERVAL);
-		} while (--rep_cnt);
-		can_be_set = !!link.link_status;
-	}
-
-	if (can_be_set) {
-		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-		if (status != I40E_SUCCESS)
-			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
-	} else {
-		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
-	}
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
 #ifdef RTE_ETHDEV_DEBUG_RX
-- 
2.9.5

Re: [PATCH] net/i40e: rework maximum frame size configuration

[David Marchand]

On Mon, Jan 16, 2023 at 11:54 AM Simei Su <simei.su@intel.com> wrote:
>
> This patch removes unnecessary link status check.
>
> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> Cc: stable@dpdk.org
>
> Signed-off-by: Simei Su <simei.su@intel.com>

Thanks for looking into the issue.

This is rather close to what I had tried [1] along my original report,
but it failed in the CI.
Let's see how the validation of your patch goes.

1: https://patchwork.dpdk.org/project/dpdk/patch/20221212143715.29649-1-david.marchand@redhat.com/


-- 
David Marchand

RE: [PATCH] net/i40e: rework maximum frame size configuration

[Su, Simei]

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, January 16, 2023 7:19 PM
> To: Su, Simei <simei.su@intel.com>
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> Subject: Re: [PATCH] net/i40e: rework maximum frame size configuration
> 
> On Mon, Jan 16, 2023 at 11:54 AM Simei Su <simei.su@intel.com> wrote:
> >
> > This patch removes unnecessary link status check.
> >
> > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > level")
> > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > level")
> > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Simei Su <simei.su@intel.com>
> 
> Thanks for looking into the issue.
> 
> This is rather close to what I had tried [1] along my original report, but it failed
> in the CI.
> Let's see how the validation of your patch goes.
> 
> 1:
> https://patchwork.dpdk.org/project/dpdk/patch/20221212143715.29649-1-d
> avid.marchand@redhat.com/
> 

OK. We will find one environment to see why the unit test failed.

Thanks,
Simei

> 
> --
> David Marchand

Re: [PATCH] net/i40e: rework maximum frame size configuration

[David Marchand]

On Mon, Jan 16, 2023 at 1:15 PM Su, Simei <simei.su@intel.com> wrote:
>
> Hi David,
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Monday, January 16, 2023 7:19 PM
> > To: Su, Simei <simei.su@intel.com>
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> > <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> > Subject: Re: [PATCH] net/i40e: rework maximum frame size configuration
> >
> > On Mon, Jan 16, 2023 at 11:54 AM Simei Su <simei.su@intel.com> wrote:
> > >
> > > This patch removes unnecessary link status check.
> > >
> > > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > > level")
> > > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > > level")
> > > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Simei Su <simei.su@intel.com>
> >
> > Thanks for looking into the issue.
> >
> > This is rather close to what I had tried [1] along my original report, but it failed
> > in the CI.
> > Let's see how the validation of your patch goes.
> >
> > 1:
> > https://patchwork.dpdk.org/project/dpdk/patch/20221212143715.29649-1-d
> > avid.marchand@redhat.com/
> >
>
> OK. We will find one environment to see why the unit test failed.

Any update?


-- 
David Marchand

RE: [PATCH] net/i40e: rework maximum frame size configuration

[Su, Simei]

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, January 20, 2023 3:34 PM
> To: Su, Simei <simei.su@intel.com>
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> Subject: Re: [PATCH] net/i40e: rework maximum frame size configuration
> 
> On Mon, Jan 16, 2023 at 1:15 PM Su, Simei <simei.su@intel.com> wrote:
> >
> > Hi David,
> >
> > > -----Original Message-----
> > > From: David Marchand <david.marchand@redhat.com>
> > > Sent: Monday, January 16, 2023 7:19 PM
> > > To: Su, Simei <simei.su@intel.com>
> > > Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> > > <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> > > Subject: Re: [PATCH] net/i40e: rework maximum frame size
> > > configuration
> > >
> > > On Mon, Jan 16, 2023 at 11:54 AM Simei Su <simei.su@intel.com> wrote:
> > > >
> > > > This patch removes unnecessary link status check.
> > > >
> > > > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > > > level")
> > > > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > > > level")
> > > > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Simei Su <simei.su@intel.com>
> > >
> > > Thanks for looking into the issue.
> > >
> > > This is rather close to what I had tried [1] along my original
> > > report, but it failed in the CI.
> > > Let's see how the validation of your patch goes.
> > >
> > > 1:
> > >
> https://patchwork.dpdk.org/project/dpdk/patch/20221212143715.29649-1
> > > -d
> > > avid.marchand@redhat.com/
> > >
> >
> > OK. We will find one environment to see why the unit test failed.
> 
> Any update?

We can reproduce CI error, but "ifconfig interface up" needs to be done firstly
to reproduce it otherwise this error won't exist. The specific reason hasn't been
found currently. We will discuss it after Chinese New Year about how to handle it
more reasonably.

> 
> 
> --
> David Marchand

Re: [PATCH] net/i40e: rework maximum frame size configuration

[David Marchand]

On Fri, Jan 20, 2023 at 2:58 PM Su, Simei <simei.su@intel.com> wrote:
>
> Hi David,
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Friday, January 20, 2023 3:34 PM
> > To: Su, Simei <simei.su@intel.com>
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> > <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> > Subject: Re: [PATCH] net/i40e: rework maximum frame size configuration
> >
> > On Mon, Jan 16, 2023 at 1:15 PM Su, Simei <simei.su@intel.com> wrote:
> > >
> > > Hi David,
> > >
> > > > -----Original Message-----
> > > > From: David Marchand <david.marchand@redhat.com>
> > > > Sent: Monday, January 16, 2023 7:19 PM
> > > > To: Su, Simei <simei.su@intel.com>
> > > > Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> > > > <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> > > > Subject: Re: [PATCH] net/i40e: rework maximum frame size
> > > > configuration
> > > >
> > > > On Mon, Jan 16, 2023 at 11:54 AM Simei Su <simei.su@intel.com> wrote:
> > > > >
> > > > > This patch removes unnecessary link status check.
> > > > >
> > > > > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > > > > level")
> > > > > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > > > > level")
> > > > > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Simei Su <simei.su@intel.com>
> > > >
> > > > Thanks for looking into the issue.
> > > >
> > > > This is rather close to what I had tried [1] along my original
> > > > report, but it failed in the CI.
> > > > Let's see how the validation of your patch goes.
> > > >
> > > > 1:
> > > >
> > https://patchwork.dpdk.org/project/dpdk/patch/20221212143715.29649-1
> > > > -d
> > > > avid.marchand@redhat.com/
> > > >
> > >
> > > OK. We will find one environment to see why the unit test failed.
> >
> > Any update?
>
> We can reproduce CI error, but "ifconfig interface up" needs to be done firstly
> to reproduce it otherwise this error won't exist. The specific reason hasn't been
> found currently. We will discuss it after Chinese New Year about how to handle it
> more reasonably.

We have regressions in stable releases.
Please put priority when the team is back so that this topic is fixed in 23.03.


Thanks.

-- 
David Marchand

RE: [PATCH] net/i40e: rework maximum frame size configuration

[Su, Simei]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, January 20, 2023 10:47 PM
> To: Su, Simei <simei.su@intel.com>
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Subject: Re: [PATCH] net/i40e: rework maximum frame size configuration
> 
> On Fri, Jan 20, 2023 at 2:58 PM Su, Simei <simei.su@intel.com> wrote:
> >
> > Hi David,
> >
> > > -----Original Message-----
> > > From: David Marchand <david.marchand@redhat.com>
> > > Sent: Friday, January 20, 2023 3:34 PM
> > > To: Su, Simei <simei.su@intel.com>
> > > Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> > > <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> > > Subject: Re: [PATCH] net/i40e: rework maximum frame size
> > > configuration
> > >
> > > On Mon, Jan 16, 2023 at 1:15 PM Su, Simei <simei.su@intel.com> wrote:
> > > >
> > > > Hi David,
> > > >
> > > > > -----Original Message-----
> > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > Sent: Monday, January 16, 2023 7:19 PM
> > > > > To: Su, Simei <simei.su@intel.com>
> > > > > Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> > > > > <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> > > > > <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > > stable@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> > > > > Subject: Re: [PATCH] net/i40e: rework maximum frame size
> > > > > configuration
> > > > >
> > > > > On Mon, Jan 16, 2023 at 11:54 AM Simei Su <simei.su@intel.com>
> wrote:
> > > > > >
> > > > > > This patch removes unnecessary link status check.
> > > > > >
> > > > > > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at
> > > > > > port
> > > > > > level")
> > > > > > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at
> > > > > > port
> > > > > > level")
> > > > > > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > > > > > Cc: stable@dpdk.org
> > > > > >
> > > > > > Signed-off-by: Simei Su <simei.su@intel.com>
> > > > >
> > > > > Thanks for looking into the issue.
> > > > >
> > > > > This is rather close to what I had tried [1] along my original
> > > > > report, but it failed in the CI.
> > > > > Let's see how the validation of your patch goes.
> > > > >
> > > > > 1:
> > > > >
> > >
> https://patchwork.dpdk.org/project/dpdk/patch/20221212143715.29649-1
> > > > > -d
> > > > > avid.marchand@redhat.com/
> > > > >
> > > >
> > > > OK. We will find one environment to see why the unit test failed.
> > >
> > > Any update?
> >
> > We can reproduce CI error, but "ifconfig interface up" needs to be
> > done firstly to reproduce it otherwise this error won't exist. The
> > specific reason hasn't been found currently. We will discuss it after
> > Chinese New Year about how to handle it more reasonably.
> 
> We have regressions in stable releases.
> Please put priority when the team is back so that this topic is fixed in 23.03.
> 
> 
> Thanks.
> 
> --
> David Marchand

Sorry for any inconvenience. OK, we will handle it as soon as possible when we are back
and fix it in 23.03.

Thanks,
Simei

[PATCH v2] net/i40e: rework maximum frame size configuration

[Simei Su]

This patch removes unnecessary link status check and adds link update.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
v2:
* Refine commit log.
* Add link update.

 drivers/net/i40e/i40e_ethdev.c | 54 +++++++++++-------------------------------
 1 file changed, 14 insertions(+), 40 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d..a3100ec 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
 	const struct rte_eth_ethertype_filter *input,
@@ -2467,8 +2466,18 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
-	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-	i40e_set_mac_max_frame(dev, max_frame_size);
+	i40e_dev_link_update(dev, 1);
+
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Fail to set mac config");
+		return ret;
+	}
 
 	return I40E_SUCCESS;
 
@@ -2809,9 +2818,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	return i40e_phy_conf_link(hw, abilities, speed, false);
 }
 
-#define CHECK_INTERVAL             100  /* 100ms */
-#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
-
 static __rte_always_inline void
 update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)
 {
@@ -2878,6 +2884,8 @@ static __rte_always_inline void
 update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
 	bool enable_lse, int wait_to_complete)
 {
+#define CHECK_INTERVAL             100  /* 100ms */
+#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
 	uint32_t rep_cnt = MAX_REPEAT_TIME;
 	struct i40e_link_status link_status;
 	int status;
@@ -12123,40 +12131,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
 	return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t rep_cnt = MAX_REPEAT_TIME;
-	struct rte_eth_link link;
-	enum i40e_status_code status;
-	bool can_be_set = true;
-
-	/*
-	 * I40E_MEDIA_TYPE_BASET link up can be ignored
-	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-	 * is I40E_MEDIA_TYPE_UNKNOWN
-	 */
-	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-		do {
-			update_link_reg(hw, &link);
-			if (link.link_status)
-				break;
-			rte_delay_ms(CHECK_INTERVAL);
-		} while (--rep_cnt);
-		can_be_set = !!link.link_status;
-	}
-
-	if (can_be_set) {
-		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-		if (status != I40E_SUCCESS)
-			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
-	} else {
-		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
-	}
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
 #ifdef RTE_ETHDEV_DEBUG_RX
-- 
2.9.5

答复: [[SPF Failed]][PATCH v2] net/i40e: rework maximum frame size configuration

[韩爽]

Why i40e_dev_link_update must be called with wait_to_complete when link interrupt is enabled ?
I think it's may have the same issue as ice_link_update, please check: https://patches.dpdk.org/project/dpdk/patch/1669207333-8769-1-git-send-email-hanshuang87@gmail.com/

-----邮件原件-----
发件人: Simei Su <simei.su@intel.com> 
发送时间: 2023年1月31日 14:52
收件人: beilei.xing@intel.com; yuying.zhang@intel.com; david.marchand@redhat.com
抄送: dev@dpdk.org; qi.z.zhang@intel.com; qiming.yang@intel.com; Simei Su <simei.su@intel.com>; stable@dpdk.org
主题: [[SPF Failed]][PATCH v2] net/i40e: rework maximum frame size configuration

该邮件SPF检测失败，发件IP不被发件域名信任，请谨慎甄别邮件，如有疑问，请蓝信搜索【网络安全部安全运营】反馈！


邮件原文如下：

This patch removes unnecessary link status check and adds link update.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
v2:
* Refine commit log.
* Add link update.

 drivers/net/i40e/i40e_ethdev.c | 54 +++++++++++-------------------------------
 1 file changed, 14 insertions(+), 40 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 7726a89d..a3100ec 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); -static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
 	const struct rte_eth_ethertype_filter *input, @@ -2467,8 +2466,18 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
-	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-	i40e_set_mac_max_frame(dev, max_frame_size);
+	i40e_dev_link_update(dev, 1);
+
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Fail to set mac config");
+		return ret;
+	}
 
 	return I40E_SUCCESS;
 
@@ -2809,9 +2818,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	return i40e_phy_conf_link(hw, abilities, speed, false);  }
 
-#define CHECK_INTERVAL             100  /* 100ms */
-#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
-
 static __rte_always_inline void
 update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)  { @@ -2878,6 +2884,8 @@ static __rte_always_inline void  update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
 	bool enable_lse, int wait_to_complete)  {
+#define CHECK_INTERVAL             100  /* 100ms */
+#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
 	uint32_t rep_cnt = MAX_REPEAT_TIME;
 	struct i40e_link_status link_status;
 	int status;
@@ -12123,40 +12131,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
 	return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size) -{
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t rep_cnt = MAX_REPEAT_TIME;
-	struct rte_eth_link link;
-	enum i40e_status_code status;
-	bool can_be_set = true;
-
-	/*
-	 * I40E_MEDIA_TYPE_BASET link up can be ignored
-	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-	 * is I40E_MEDIA_TYPE_UNKNOWN
-	 */
-	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-		do {
-			update_link_reg(hw, &link);
-			if (link.link_status)
-				break;
-			rte_delay_ms(CHECK_INTERVAL);
-		} while (--rep_cnt);
-		can_be_set = !!link.link_status;
-	}
-
-	if (can_be_set) {
-		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-		if (status != I40E_SUCCESS)
-			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
-	} else {
-		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
-	}
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);  RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);  #ifdef RTE_ETHDEV_DEBUG_RX
--
2.9.5

[PATCH v3] net/i40e: rework maximum frame size configuration

[Simei Su]

This patch removes unnecessary link status check and adds link update.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
v3:
* Put link update before interrupt enable.

v2:
* Refine commit log.
* Add link update.

 drivers/net/i40e/i40e_ethdev.c | 58 +++++++++++-------------------------------
 1 file changed, 15 insertions(+), 43 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d..f5a6cec 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
 	const struct rte_eth_ethertype_filter *input,
@@ -2447,11 +2446,11 @@ i40e_dev_start(struct rte_eth_dev *dev)
 					       I40E_AQ_EVENT_MEDIA_NA), NULL);
 		if (ret != I40E_SUCCESS)
 			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
-
-		/* Call get_link_info aq command to enable/disable LSE */
-		i40e_dev_link_update(dev, 0);
 	}
 
+	/* Call get_link_info aq command to enable/disable LSE */
+	i40e_dev_link_update(dev, 1);
+
 	if (dev->data->dev_conf.intr_conf.rxq == 0) {
 		rte_eal_alarm_set(I40E_ALARM_INTERVAL,
 				  i40e_dev_alarm_handler, dev);
@@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
-	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-	i40e_set_mac_max_frame(dev, max_frame_size);
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Fail to set mac config");
+		return ret;
+	}
 
 	return I40E_SUCCESS;
 
@@ -2809,9 +2816,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	return i40e_phy_conf_link(hw, abilities, speed, false);
 }
 
-#define CHECK_INTERVAL             100  /* 100ms */
-#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
-
 static __rte_always_inline void
 update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)
 {
@@ -2878,6 +2882,8 @@ static __rte_always_inline void
 update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
 	bool enable_lse, int wait_to_complete)
 {
+#define CHECK_INTERVAL             100  /* 100ms */
+#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
 	uint32_t rep_cnt = MAX_REPEAT_TIME;
 	struct i40e_link_status link_status;
 	int status;
@@ -12123,40 +12129,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
 	return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t rep_cnt = MAX_REPEAT_TIME;
-	struct rte_eth_link link;
-	enum i40e_status_code status;
-	bool can_be_set = true;
-
-	/*
-	 * I40E_MEDIA_TYPE_BASET link up can be ignored
-	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-	 * is I40E_MEDIA_TYPE_UNKNOWN
-	 */
-	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-		do {
-			update_link_reg(hw, &link);
-			if (link.link_status)
-				break;
-			rte_delay_ms(CHECK_INTERVAL);
-		} while (--rep_cnt);
-		can_be_set = !!link.link_status;
-	}
-
-	if (can_be_set) {
-		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-		if (status != I40E_SUCCESS)
-			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
-	} else {
-		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
-	}
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
 #ifdef RTE_ETHDEV_DEBUG_RX
-- 
2.9.5

RE: [PATCH v3] net/i40e: rework maximum frame size configuration

[Yang, Qiming]

Hi,

> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Tuesday, January 31, 2023 6:32 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; david.marchand@redhat.com
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Su, Simei <simei.su@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v3] net/i40e: rework maximum frame size configuration
> 
> This patch removes unnecessary link status check and adds link update.
I think this patch is not adding link update, just delete interrupt mode check before link update.
And why you change the wait to complete from 0 to 1?

> 
> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Simei Su <simei.su@intel.com>
> ---
> v3:
> * Put link update before interrupt enable.
> 
> v2:
> * Refine commit log.
> * Add link update.
> 
>  drivers/net/i40e/i40e_ethdev.c | 58 +++++++++++-------------------------------
>  1 file changed, 15 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7726a89d..f5a6cec 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
>  				      struct rte_ether_addr *mac_addr);
> 
>  static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); -static
> void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
> 
>  static int i40e_ethertype_filter_convert(
>  	const struct rte_eth_ethertype_filter *input, @@ -2447,11 +2446,11
> @@ i40e_dev_start(struct rte_eth_dev *dev)
>  					       I40E_AQ_EVENT_MEDIA_NA),
> NULL);
>  		if (ret != I40E_SUCCESS)
>  			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
> -
> -		/* Call get_link_info aq command to enable/disable LSE */
> -		i40e_dev_link_update(dev, 0);
>  	}
> 
> +	/* Call get_link_info aq command to enable/disable LSE */
> +	i40e_dev_link_update(dev, 1);
> +
>  	if (dev->data->dev_conf.intr_conf.rxq == 0) {
>  		rte_eal_alarm_set(I40E_ALARM_INTERVAL,
>  				  i40e_dev_alarm_handler, dev);
> @@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
>  			    "please call hierarchy_commit() "
>  			    "before starting the port");
> 
> -	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
> -	i40e_set_mac_max_frame(dev, max_frame_size);
> +	max_frame_size = dev->data->mtu ?
> +		dev->data->mtu + I40E_ETH_OVERHEAD :
> +		I40E_FRAME_SIZE_MAX;
> +
> +	/* Set the max frame size to HW*/
> +	ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0,
> NULL);
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "Fail to set mac config");
> +		return ret;
> +	}
> 
>  	return I40E_SUCCESS;
> 
> @@ -2809,9 +2816,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
>  	return i40e_phy_conf_link(hw, abilities, speed, false);  }
> 
> -#define CHECK_INTERVAL             100  /* 100ms */
> -#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
> -
>  static __rte_always_inline void
>  update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)  { @@ -
> 2878,6 +2882,8 @@ static __rte_always_inline void  update_link_aq(struct
> i40e_hw *hw, struct rte_eth_link *link,
>  	bool enable_lse, int wait_to_complete)  {
> +#define CHECK_INTERVAL             100  /* 100ms */
> +#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
>  	uint32_t rep_cnt = MAX_REPEAT_TIME;
>  	struct i40e_link_status link_status;
>  	int status;
> @@ -12123,40 +12129,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf
> *pf)
>  	return ret;
>  }
> 
> -static void
> -i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size) -{
> -	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> -	uint32_t rep_cnt = MAX_REPEAT_TIME;
> -	struct rte_eth_link link;
> -	enum i40e_status_code status;
> -	bool can_be_set = true;
> -
> -	/*
> -	 * I40E_MEDIA_TYPE_BASET link up can be ignored
> -	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
> -	 * is I40E_MEDIA_TYPE_UNKNOWN
> -	 */
> -	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
> -	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
> -		do {
> -			update_link_reg(hw, &link);
> -			if (link.link_status)
> -				break;
> -			rte_delay_ms(CHECK_INTERVAL);
> -		} while (--rep_cnt);
> -		can_be_set = !!link.link_status;
> -	}
> -
> -	if (can_be_set) {
> -		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false,
> NULL);
> -		if (status != I40E_SUCCESS)
> -			PMD_DRV_LOG(ERR, "Failed to set max frame size at
> port level");
> -	} else {
> -		PMD_DRV_LOG(ERR, "Set max frame size at port level not
> applicable on link down");
> -	}
> -}
> -
>  RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
> RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);  #ifdef
> RTE_ETHDEV_DEBUG_RX
> --
> 2.9.5

[PATCH v4] net/i40e: rework maximum frame size configuration

[Simei Su]

This patch reverts mentionned changes below to remove unnecessary link
status check and only moves max frame size configuration to dev_start.
Also, it sets the parameter "wait to complete" true to wait for complete
right after setting link up.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
v4:
* Refine commit log.
* Avoid duplicate call to set parameter "wait to complete" true.

v3:
* Put link update before interrupt enable.

v2:
* Refine commit log.
* Add link update.

 drivers/net/i40e/i40e_ethdev.c | 54 ++++++++++--------------------------------
 1 file changed, 13 insertions(+), 41 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d..5d57bb9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
 	const struct rte_eth_ethertype_filter *input,
@@ -2449,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
 
 		/* Call get_link_info aq command to enable/disable LSE */
-		i40e_dev_link_update(dev, 0);
+		i40e_dev_link_update(dev, 1);
 	}
 
 	if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
-	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-	i40e_set_mac_max_frame(dev, max_frame_size);
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Fail to set mac config");
+		return ret;
+	}
 
 	return I40E_SUCCESS;
 
@@ -2809,9 +2816,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	return i40e_phy_conf_link(hw, abilities, speed, false);
 }
 
-#define CHECK_INTERVAL             100  /* 100ms */
-#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
-
 static __rte_always_inline void
 update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)
 {
@@ -2878,6 +2882,8 @@ static __rte_always_inline void
 update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
 	bool enable_lse, int wait_to_complete)
 {
+#define CHECK_INTERVAL             100  /* 100ms */
+#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
 	uint32_t rep_cnt = MAX_REPEAT_TIME;
 	struct i40e_link_status link_status;
 	int status;
@@ -12123,40 +12129,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
 	return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t rep_cnt = MAX_REPEAT_TIME;
-	struct rte_eth_link link;
-	enum i40e_status_code status;
-	bool can_be_set = true;
-
-	/*
-	 * I40E_MEDIA_TYPE_BASET link up can be ignored
-	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-	 * is I40E_MEDIA_TYPE_UNKNOWN
-	 */
-	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-		do {
-			update_link_reg(hw, &link);
-			if (link.link_status)
-				break;
-			rte_delay_ms(CHECK_INTERVAL);
-		} while (--rep_cnt);
-		can_be_set = !!link.link_status;
-	}
-
-	if (can_be_set) {
-		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-		if (status != I40E_SUCCESS)
-			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
-	} else {
-		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
-	}
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
 #ifdef RTE_ETHDEV_DEBUG_RX
-- 
2.9.5

[PATCH v5] net/i40e: rework maximum frame size configuration

[Simei Su]

This patch reverts mentioned changes below to remove unnecessary link
status check and only moves max frame size configuration to dev_start.
Also, it sets the parameter "wait to complete" true to wait for complete
right after setting link up.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
v5:
* Fix misspelling in commit log.

v4:
* Refine commit log.
* Avoid duplicate call to set parameter "wait to complete" true.

v3:
* Put link update before interrupt enable.

v2:
* Refine commit log.
* Add link update.

 drivers/net/i40e/i40e_ethdev.c | 54 ++++++++++--------------------------------
 1 file changed, 13 insertions(+), 41 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d..5d57bb9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
 	const struct rte_eth_ethertype_filter *input,
@@ -2449,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
 
 		/* Call get_link_info aq command to enable/disable LSE */
-		i40e_dev_link_update(dev, 0);
+		i40e_dev_link_update(dev, 1);
 	}
 
 	if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
-	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-	i40e_set_mac_max_frame(dev, max_frame_size);
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Fail to set mac config");
+		return ret;
+	}
 
 	return I40E_SUCCESS;
 
@@ -2809,9 +2816,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	return i40e_phy_conf_link(hw, abilities, speed, false);
 }
 
-#define CHECK_INTERVAL             100  /* 100ms */
-#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
-
 static __rte_always_inline void
 update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)
 {
@@ -2878,6 +2882,8 @@ static __rte_always_inline void
 update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
 	bool enable_lse, int wait_to_complete)
 {
+#define CHECK_INTERVAL             100  /* 100ms */
+#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
 	uint32_t rep_cnt = MAX_REPEAT_TIME;
 	struct i40e_link_status link_status;
 	int status;
@@ -12123,40 +12129,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
 	return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t rep_cnt = MAX_REPEAT_TIME;
-	struct rte_eth_link link;
-	enum i40e_status_code status;
-	bool can_be_set = true;
-
-	/*
-	 * I40E_MEDIA_TYPE_BASET link up can be ignored
-	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-	 * is I40E_MEDIA_TYPE_UNKNOWN
-	 */
-	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-		do {
-			update_link_reg(hw, &link);
-			if (link.link_status)
-				break;
-			rte_delay_ms(CHECK_INTERVAL);
-		} while (--rep_cnt);
-		can_be_set = !!link.link_status;
-	}
-
-	if (can_be_set) {
-		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-		if (status != I40E_SUCCESS)
-			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
-	} else {
-		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
-	}
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
 #ifdef RTE_ETHDEV_DEBUG_RX
-- 
2.9.5

Re: [PATCH v5] net/i40e: rework maximum frame size configuration

[David Marchand]

On Thu, Feb 2, 2023 at 1:37 PM Simei Su <simei.su@intel.com> wrote:
>
> This patch reverts mentioned changes below to remove unnecessary link
> status check and only moves max frame size configuration to dev_start.
> Also, it sets the parameter "wait to complete" true to wait for complete
> right after setting link up.

Why is the change on link status needed?
Is it necessary?

>
> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> Cc: stable@dpdk.org
>
> Signed-off-by: Simei Su <simei.su@intel.com>

I would have preferred you reply to my original report.
At least, I'd like you add some credit with my name in the commitlog.


For the record, the differences with my v1 are:

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5635dd03cf..5d57bb9a0e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2327,6 +2327,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
        uint32_t intr_vector = 0;
        struct i40e_vsi *vsi;
        uint16_t nb_rxq, nb_txq;
+       uint16_t max_frame_size;

        hw->adapter_stopped = 0;

@@ -2447,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
                        PMD_DRV_LOG(WARNING, "Fail to set phy mask");

                /* Call get_link_info aq command to enable/disable LSE */
-               i40e_dev_link_update(dev, 0);
+               i40e_dev_link_update(dev, 1);
        }

        if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2465,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
                            "please call hierarchy_commit() "
                            "before starting the port");

-       i40e_aq_set_mac_config(hw, dev->data->mtu + I40E_ETH_OVERHEAD, TRUE,
-               false, 0, NULL);
+       max_frame_size = dev->data->mtu ?
+               dev->data->mtu + I40E_ETH_OVERHEAD :
+               I40E_FRAME_SIZE_MAX;
+
+       /* Set the max frame size to HW*/
+       ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+       if (ret) {
+               PMD_DRV_LOG(ERR, "Fail to set mac config");
+               return ret;
+       }

        return I40E_SUCCESS;


Qi, don't apply this fix yet.
I'll generate some binaries internally to have Red Hat QE run their tests.


Thanks.

-- 
David Marchand

Re: [PATCH v4] net/i40e: rework maximum frame size configuration

[Kevin Traynor]

On 02/02/2023 12:30, Simei Su wrote:
> This patch reverts mentionned changes below to remove unnecessary link
> status check and only moves max frame size configuration to dev_start.
> Also, it sets the parameter "wait to complete" true to wait for complete
> right after setting link up.
> 

I think it would be better for fixes tracking and stable branches to put 
reverts into a single or multiple patches in a series. Then put whatever 
new functionality or new fix for the code prior to these patches that 
are being reverted into a separate patch.

Otherwise fixes for this new functionality will be seen as a fix for 
patches that were reverted etc.

Also, for 21.11 I have already reverted these patches, so I won't be 
able to take a combined revert/new fixes patch without manual editing or 
getting a 21.11 branch rebased patch.

> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Simei Su <simei.su@intel.com>
> ---
> v4:
> * Refine commit log.
> * Avoid duplicate call to set parameter "wait to complete" true.
> 
> v3:
> * Put link update before interrupt enable.
> 
> v2:
> * Refine commit log.
> * Add link update.
> 
>   drivers/net/i40e/i40e_ethdev.c | 54 ++++++++++--------------------------------
>   1 file changed, 13 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7726a89d..5d57bb9 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
>   				      struct rte_ether_addr *mac_addr);
>   
>   static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
> -static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
>   
>   static int i40e_ethertype_filter_convert(
>   	const struct rte_eth_ethertype_filter *input,
> @@ -2449,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
>   			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
>   
>   		/* Call get_link_info aq command to enable/disable LSE */
> -		i40e_dev_link_update(dev, 0);
> +		i40e_dev_link_update(dev, 1);
>   	}
>   
>   	if (dev->data->dev_conf.intr_conf.rxq == 0) {
> @@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
>   			    "please call hierarchy_commit() "
>   			    "before starting the port");
>   
> -	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
> -	i40e_set_mac_max_frame(dev, max_frame_size);
> +	max_frame_size = dev->data->mtu ?
> +		dev->data->mtu + I40E_ETH_OVERHEAD :
> +		I40E_FRAME_SIZE_MAX;
> +
> +	/* Set the max frame size to HW*/
> +	ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "Fail to set mac config");
> +		return ret;
> +	}
>   
>   	return I40E_SUCCESS;
>   
> @@ -2809,9 +2816,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
>   	return i40e_phy_conf_link(hw, abilities, speed, false);
>   }
>   
> -#define CHECK_INTERVAL             100  /* 100ms */
> -#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
> -
>   static __rte_always_inline void
>   update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)
>   {
> @@ -2878,6 +2882,8 @@ static __rte_always_inline void
>   update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
>   	bool enable_lse, int wait_to_complete)
>   {
> +#define CHECK_INTERVAL             100  /* 100ms */
> +#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
>   	uint32_t rep_cnt = MAX_REPEAT_TIME;
>   	struct i40e_link_status link_status;
>   	int status;
> @@ -12123,40 +12129,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
>   	return ret;
>   }
>   
> -static void
> -i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
> -{
> -	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> -	uint32_t rep_cnt = MAX_REPEAT_TIME;
> -	struct rte_eth_link link;
> -	enum i40e_status_code status;
> -	bool can_be_set = true;
> -
> -	/*
> -	 * I40E_MEDIA_TYPE_BASET link up can be ignored
> -	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
> -	 * is I40E_MEDIA_TYPE_UNKNOWN
> -	 */
> -	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
> -	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
> -		do {
> -			update_link_reg(hw, &link);
> -			if (link.link_status)
> -				break;
> -			rte_delay_ms(CHECK_INTERVAL);
> -		} while (--rep_cnt);
> -		can_be_set = !!link.link_status;
> -	}
> -
> -	if (can_be_set) {
> -		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
> -		if (status != I40E_SUCCESS)
> -			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
> -	} else {
> -		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
> -	}
> -}
> -
>   RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
>   RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
>   #ifdef RTE_ETHDEV_DEBUG_RX

Re: [PATCH v5] net/i40e: rework maximum frame size configuration

[David Marchand]

On Thu, Feb 2, 2023 at 1:37 PM Simei Su <simei.su@intel.com> wrote:
> @@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
>                             "please call hierarchy_commit() "
>                             "before starting the port");
>
> -       max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
> -       i40e_set_mac_max_frame(dev, max_frame_size);
> +       max_frame_size = dev->data->mtu ?
> +               dev->data->mtu + I40E_ETH_OVERHEAD :
> +               I40E_FRAME_SIZE_MAX;
> +
> +       /* Set the max frame size to HW*/
> +       ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
> +       if (ret) {
> +               PMD_DRV_LOG(ERR, "Fail to set mac config");
> +               return ret;
> +       }

Reading this patch again.

Returning here seems incorrect as we leave rx/tx queue in started state.
Don't we need to jump to tx_err label on error?

>
>         return I40E_SUCCESS;
>

-- 
David Marchand

Re: [PATCH v5] net/i40e: rework maximum frame size configuration

[David Marchand]

On Thu, Feb 2, 2023 at 2:24 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Thu, Feb 2, 2023 at 1:37 PM Simei Su <simei.su@intel.com> wrote:
> > @@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
> >                             "please call hierarchy_commit() "
> >                             "before starting the port");
> >
> > -       max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
> > -       i40e_set_mac_max_frame(dev, max_frame_size);
> > +       max_frame_size = dev->data->mtu ?
> > +               dev->data->mtu + I40E_ETH_OVERHEAD :
> > +               I40E_FRAME_SIZE_MAX;
> > +
> > +       /* Set the max frame size to HW*/
> > +       ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
> > +       if (ret) {
> > +               PMD_DRV_LOG(ERR, "Fail to set mac config");
> > +               return ret;
> > +       }
>
> Reading this patch again.
>
> Returning here seems incorrect as we leave rx/tx queue in started state.
> Don't we need to jump to tx_err label on error?

There is probably an issue with interrupt handler still being registered too.
Qi, i40e maintainers, please review this patch carefully, and ping me
when it is ready so that we can test it at RH.


Thanks.
-- 
David Marchand

RE: [PATCH v5] net/i40e: rework maximum frame size configuration

[Su, Simei]

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, February 2, 2023 8:56 PM
> To: Su, Simei <simei.su@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; dev@dpdk.org; Yang, Qiming
> <qiming.yang@intel.com>; stable@dpdk.org
> Subject: Re: [PATCH v5] net/i40e: rework maximum frame size configuration
> 
> On Thu, Feb 2, 2023 at 1:37 PM Simei Su <simei.su@intel.com> wrote:
> >
> > This patch reverts mentioned changes below to remove unnecessary link
> > status check and only moves max frame size configuration to dev_start.
> > Also, it sets the parameter "wait to complete" true to wait for
> > complete right after setting link up.
> 
> Why is the change on link status needed?
> Is it necessary?

Indeed, it doesn't change the link status, it only involves update, waiting for it to complete.
Sorry not to describe correctly about setting the " wait to complete" true.

> 
> >
> > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > level")
> > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > level")
> > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Simei Su <simei.su@intel.com>
> 
> I would have preferred you reply to my original report.
> At least, I'd like you add some credit with my name in the commitlog.

Sorry, I will notice it in next version.

Thanks,
Simei

> 
> 
> For the record, the differences with my v1 are:
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5635dd03cf..5d57bb9a0e 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2327,6 +2327,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
>         uint32_t intr_vector = 0;
>         struct i40e_vsi *vsi;
>         uint16_t nb_rxq, nb_txq;
> +       uint16_t max_frame_size;
> 
>         hw->adapter_stopped = 0;
> 
> @@ -2447,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
>                         PMD_DRV_LOG(WARNING, "Fail to set phy
> mask");
> 
>                 /* Call get_link_info aq command to enable/disable LSE
> */
> -               i40e_dev_link_update(dev, 0);
> +               i40e_dev_link_update(dev, 1);
>         }
> 
>         if (dev->data->dev_conf.intr_conf.rxq == 0) { @@ -2465,8 +2466,16
> @@ i40e_dev_start(struct rte_eth_dev *dev)
>                             "please call hierarchy_commit() "
>                             "before starting the port");
> 
> -       i40e_aq_set_mac_config(hw, dev->data->mtu +
> I40E_ETH_OVERHEAD, TRUE,
> -               false, 0, NULL);
> +       max_frame_size = dev->data->mtu ?
> +               dev->data->mtu + I40E_ETH_OVERHEAD :
> +               I40E_FRAME_SIZE_MAX;
> +
> +       /* Set the max frame size to HW*/
> +       ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0,
> NULL);
> +       if (ret) {
> +               PMD_DRV_LOG(ERR, "Fail to set mac config");
> +               return ret;
> +       }
> 
>         return I40E_SUCCESS;
> 
> 
> Qi, don't apply this fix yet.
> I'll generate some binaries internally to have Red Hat QE run their tests.
> 
> 
> Thanks.
> 
> --
> David Marchand

RE: [PATCH v5] net/i40e: rework maximum frame size configuration

[Su, Simei]

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, February 2, 2023 9:24 PM
> To: Su, Simei <simei.su@intel.com>
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; dev@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> stable@dpdk.org
> Subject: Re: [PATCH v5] net/i40e: rework maximum frame size configuration
> 
> On Thu, Feb 2, 2023 at 1:37 PM Simei Su <simei.su@intel.com> wrote:
> > @@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
> >                             "please call hierarchy_commit() "
> >                             "before starting the port");
> >
> > -       max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
> > -       i40e_set_mac_max_frame(dev, max_frame_size);
> > +       max_frame_size = dev->data->mtu ?
> > +               dev->data->mtu + I40E_ETH_OVERHEAD :
> > +               I40E_FRAME_SIZE_MAX;
> > +
> > +       /* Set the max frame size to HW*/
> > +       ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0,
> NULL);
> > +       if (ret) {
> > +               PMD_DRV_LOG(ERR, "Fail to set mac config");
> > +               return ret;
> > +       }
> 
> Reading this patch again.
> 
> Returning here seems incorrect as we leave rx/tx queue in started state.
> Don't we need to jump to tx_err label on error?

Yes, it's my fault to return here incorrectly. I will modify it in next version.

Thanks,
Simei

> 
> >
> >         return I40E_SUCCESS;
> >
> 
> --
> David Marchand

RE: [PATCH v5] net/i40e: rework maximum frame size configuration

[Yang, Qiming]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, February 2, 2023 9:49 PM
> To: Su, Simei <simei.su@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> stable@dpdk.org
> Subject: Re: [PATCH v5] net/i40e: rework maximum frame size configuration
> 
> On Thu, Feb 2, 2023 at 2:24 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Thu, Feb 2, 2023 at 1:37 PM Simei Su <simei.su@intel.com> wrote:
> > > @@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
> > >                             "please call hierarchy_commit() "
> > >                             "before starting the port");
> > >
> > > -       max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
> > > -       i40e_set_mac_max_frame(dev, max_frame_size);
> > > +       max_frame_size = dev->data->mtu ?
> > > +               dev->data->mtu + I40E_ETH_OVERHEAD :
> > > +               I40E_FRAME_SIZE_MAX;
> > > +
> > > +       /* Set the max frame size to HW*/
> > > +       ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0,
> NULL);
> > > +       if (ret) {
> > > +               PMD_DRV_LOG(ERR, "Fail to set mac config");
> > > +               return ret;
> > > +       }
> >
> > Reading this patch again.
> >
> > Returning here seems incorrect as we leave rx/tx queue in started state.
> > Don't we need to jump to tx_err label on error?
> 
> There is probably an issue with interrupt handler still being registered too.
> Qi, i40e maintainers, please review this patch carefully, and ping me when it
> is ready so that we can test it at RH.
> 

This change will not break interrupt handler, the second parameter is waiting to complete. Just waiting more time to make sure adminq command execute completed. So that subsequent commands(MTU set) can be executed.
And if you have other issues report at RH system, please report it.

> 
> Thanks.
> --
> David Marchand

Re: [PATCH v5] net/i40e: rework maximum frame size configuration

[David Marchand]

On Fri, Feb 3, 2023 at 9:39 AM Yang, Qiming <qiming.yang@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Thursday, February 2, 2023 9:49 PM
> > To: Su, Simei <simei.su@intel.com>; Zhang, Yuying
> > <yuying.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> > stable@dpdk.org
> > Subject: Re: [PATCH v5] net/i40e: rework maximum frame size configuration
> >
> > On Thu, Feb 2, 2023 at 2:24 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > On Thu, Feb 2, 2023 at 1:37 PM Simei Su <simei.su@intel.com> wrote:
> > > > @@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
> > > >                             "please call hierarchy_commit() "
> > > >                             "before starting the port");
> > > >
> > > > -       max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
> > > > -       i40e_set_mac_max_frame(dev, max_frame_size);
> > > > +       max_frame_size = dev->data->mtu ?
> > > > +               dev->data->mtu + I40E_ETH_OVERHEAD :
> > > > +               I40E_FRAME_SIZE_MAX;
> > > > +
> > > > +       /* Set the max frame size to HW*/
> > > > +       ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0,
> > NULL);
> > > > +       if (ret) {
> > > > +               PMD_DRV_LOG(ERR, "Fail to set mac config");
> > > > +               return ret;
> > > > +       }
> > >
> > > Reading this patch again.
> > >
> > > Returning here seems incorrect as we leave rx/tx queue in started state.
> > > Don't we need to jump to tx_err label on error?
> >
> > There is probably an issue with interrupt handler still being registered too.
> > Qi, i40e maintainers, please review this patch carefully, and ping me when it
> > is ready so that we can test it at RH.
> >
>
> This change will not break interrupt handler, the second parameter is waiting to complete. Just waiting more time to make sure adminq command execute completed. So that subsequent commands(MTU set) can be executed.
> And if you have other issues report at RH system, please report it.

If i40e_aq_set_mac_config() fails and we return directly, an interrupt
handler is left registered, don't you think it is an issue?

In any case, I will wait for the next revision before asking for tests
on my side.
Thanks.


-- 
David Marchand

[PATCH v6] net/i40e: rework maximum frame size configuration

[Simei Su]

One issue is reported by David Marchand that error occurs in OVS due to
the fix patch in mentioned changes below. The detailed reproduce step
and result are in https://patchwork.dpdk.org/project/dpdk/patch/
20211207085946.121032-1-dapengx.yu@intel.com/.

This patch removes unnecessary link status check and directly sets mac
config in dev_start. Also, it sets the parameter "wait to complete" true
to wait for more time to make sure adminq command execute completed.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org

Reported-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Simei Su <simei.su@intel.com>
---
v6:
* Refine commit log.
* Remove return error.

v5:
* Fix misspelling in commit log.

v4:
* Refine commit log.
* Avoid duplicate call to set parameter "wait to complete" true.

v3:
* Put link update before interrupt enable.

v2:
* Refine commit log.
* Add link update.

 drivers/net/i40e/i40e_ethdev.c | 50 ++++++++----------------------------------
 1 file changed, 9 insertions(+), 41 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d..30c904c 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
 	const struct rte_eth_ethertype_filter *input,
@@ -2449,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
 
 		/* Call get_link_info aq command to enable/disable LSE */
-		i40e_dev_link_update(dev, 0);
+		i40e_dev_link_update(dev, 1);
 	}
 
 	if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2467,8 +2466,12 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
-	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-	i40e_set_mac_max_frame(dev, max_frame_size);
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
 
 	return I40E_SUCCESS;
 
@@ -2809,9 +2812,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	return i40e_phy_conf_link(hw, abilities, speed, false);
 }
 
-#define CHECK_INTERVAL             100  /* 100ms */
-#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
-
 static __rte_always_inline void
 update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)
 {
@@ -2878,6 +2878,8 @@ static __rte_always_inline void
 update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
 	bool enable_lse, int wait_to_complete)
 {
+#define CHECK_INTERVAL             100  /* 100ms */
+#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
 	uint32_t rep_cnt = MAX_REPEAT_TIME;
 	struct i40e_link_status link_status;
 	int status;
@@ -12123,40 +12125,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
 	return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t rep_cnt = MAX_REPEAT_TIME;
-	struct rte_eth_link link;
-	enum i40e_status_code status;
-	bool can_be_set = true;
-
-	/*
-	 * I40E_MEDIA_TYPE_BASET link up can be ignored
-	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-	 * is I40E_MEDIA_TYPE_UNKNOWN
-	 */
-	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-		do {
-			update_link_reg(hw, &link);
-			if (link.link_status)
-				break;
-			rte_delay_ms(CHECK_INTERVAL);
-		} while (--rep_cnt);
-		can_be_set = !!link.link_status;
-	}
-
-	if (can_be_set) {
-		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-		if (status != I40E_SUCCESS)
-			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
-	} else {
-		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
-	}
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
 #ifdef RTE_ETHDEV_DEBUG_RX
-- 
2.9.5

RE: [PATCH v6] net/i40e: rework maximum frame size configuration

[Zhang, Qi Z]

> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Monday, February 20, 2023 4:00 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> david.marchand@redhat.com
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
> <simei.su@intel.com>; stable@dpdk.org
> Subject: [PATCH v6] net/i40e: rework maximum frame size configuration
> 
> One issue is reported by David Marchand that error occurs in OVS due to the
> fix patch in mentioned changes below. The detailed reproduce step and
> result are in https://patchwork.dpdk.org/project/dpdk/patch/
> 20211207085946.121032-1-dapengx.yu@intel.com/.
> 
> This patch removes unnecessary link status check and directly sets mac config
> in dev_start. Also, it sets the parameter "wait to complete" true to wait for
> more time to make sure adminq command execute completed.
> 
> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> Cc: stable@dpdk.org
> 
> Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Simei Su <simei.su@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [PATCH v6] net/i40e: rework maximum frame size configuration

[David Marchand]

Qi,

On Mon, Feb 27, 2023 at 1:35 AM Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> > One issue is reported by David Marchand that error occurs in OVS due to the
> > fix patch in mentioned changes below. The detailed reproduce step and
> > result are in https://patchwork.dpdk.org/project/dpdk/patch/
> > 20211207085946.121032-1-dapengx.yu@intel.com/.
> >
> > This patch removes unnecessary link status check and directly sets mac config
> > in dev_start. Also, it sets the parameter "wait to complete" true to wait for
> > more time to make sure adminq command execute completed.
> >
> > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > Cc: stable@dpdk.org
> >
> > Reported-by: David Marchand <david.marchand@redhat.com>
> > Signed-off-by: Simei Su <simei.su@intel.com>
>
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>

I was waiting for a ping... good thing I had a look at this thread.

I suggest splitting this in two parts before it reaches the main repo:
- put the reverts first (the reason is that 21.11 stable branch
already have them): this would end up the same as merging
https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-david.marchand@redhat.com/
that has been waiting since December,
- have the move of i40e_aq_set_mac_config from eth_i40e_dev_init to
i40e_dev_start + change in i40e_dev_link_update call in a second patch
(i.e. the following diff),

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index a982e42264..371f42233e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1710,11 +1710,6 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void
*init_params __rte_unused)
         */
        i40e_add_tx_flow_control_drop_filter(pf);

-       /* Set the max frame size to 0x2600 by default,
-        * in case other drivers changed the default value.
-        */
-       i40e_aq_set_mac_config(hw, I40E_FRAME_SIZE_MAX, TRUE, false, 0, NULL);
-
        /* initialize RSS rule list */
        TAILQ_INIT(&pf->rss_config_list);

@@ -2332,6 +2327,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
        uint32_t intr_vector = 0;
        struct i40e_vsi *vsi;
        uint16_t nb_rxq, nb_txq;
+       uint16_t max_frame_size;

        hw->adapter_stopped = 0;

@@ -2452,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
                        PMD_DRV_LOG(WARNING, "Fail to set phy mask");

                /* Call get_link_info aq command to enable/disable LSE */
-               i40e_dev_link_update(dev, 0);
+               i40e_dev_link_update(dev, 1);
        }

        if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2470,6 +2466,13 @@ i40e_dev_start(struct rte_eth_dev *dev)
                            "please call hierarchy_commit() "
                            "before starting the port");

+       max_frame_size = dev->data->mtu ?
+               dev->data->mtu + I40E_ETH_OVERHEAD :
+               I40E_FRAME_SIZE_MAX;
+
+       /* Set the max frame size to HW*/
+       i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+
        return I40E_SUCCESS;

 tx_err:


This way, the new fix is easier to read too.
Thanks.


-- 
David Marchand

RE: [PATCH v6] net/i40e: rework maximum frame size configuration

[Zhang, Qi Z]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, February 28, 2023 5:37 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Su, Simei <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Zhang, Yuying <yuying.zhang@intel.com>; dev@dpdk.org; Yang, Qiming
> <qiming.yang@intel.com>; stable@dpdk.org; Kevin Traynor
> <ktraynor@redhat.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
> 
> Qi,
> 
> On Mon, Feb 27, 2023 at 1:35 AM Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> > > One issue is reported by David Marchand that error occurs in OVS due
> > > to the fix patch in mentioned changes below. The detailed reproduce
> > > step and result are in
> > > https://patchwork.dpdk.org/project/dpdk/patch/
> > > 20211207085946.121032-1-dapengx.yu@intel.com/.
> > >
> > > This patch removes unnecessary link status check and directly sets
> > > mac config in dev_start. Also, it sets the parameter "wait to
> > > complete" true to wait for more time to make sure adminq command
> execute completed.
> > >
> > > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > > level")
> > > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > > level")
> > > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > > Cc: stable@dpdk.org
> > >
> > > Reported-by: David Marchand <david.marchand@redhat.com>
> > > Signed-off-by: Simei Su <simei.su@intel.com>
> >
> > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> I was waiting for a ping... good thing I had a look at this thread.
> 
> I suggest splitting this in two parts before it reaches the main repo:
> - put the reverts first (the reason is that 21.11 stable branch already have
> them): this would end up the same as merging
> https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-
> david.marchand@redhat.com/
> that has been waiting since December,
> - have the move of i40e_aq_set_mac_config from eth_i40e_dev_init to
> i40e_dev_start + change in i40e_dev_link_update call in a second patch (i.e.
> the following diff),

OK, reverted in dpdk-next-net-intel.

And waiting for Simei's v7.

> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index a982e42264..371f42233e 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1710,11 +1710,6 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void
> *init_params __rte_unused)
>          */
>         i40e_add_tx_flow_control_drop_filter(pf);
> 
> -       /* Set the max frame size to 0x2600 by default,
> -        * in case other drivers changed the default value.
> -        */
> -       i40e_aq_set_mac_config(hw, I40E_FRAME_SIZE_MAX, TRUE, false, 0,
> NULL);
> -
>         /* initialize RSS rule list */
>         TAILQ_INIT(&pf->rss_config_list);
> 
> @@ -2332,6 +2327,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
>         uint32_t intr_vector = 0;
>         struct i40e_vsi *vsi;
>         uint16_t nb_rxq, nb_txq;
> +       uint16_t max_frame_size;
> 
>         hw->adapter_stopped = 0;
> 
> @@ -2452,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
>                         PMD_DRV_LOG(WARNING, "Fail to set phy mask");
> 
>                 /* Call get_link_info aq command to enable/disable LSE */
> -               i40e_dev_link_update(dev, 0);
> +               i40e_dev_link_update(dev, 1);
>         }
> 
>         if (dev->data->dev_conf.intr_conf.rxq == 0) { @@ -2470,6 +2466,13 @@
> i40e_dev_start(struct rte_eth_dev *dev)
>                             "please call hierarchy_commit() "
>                             "before starting the port");
> 
> +       max_frame_size = dev->data->mtu ?
> +               dev->data->mtu + I40E_ETH_OVERHEAD :
> +               I40E_FRAME_SIZE_MAX;
> +
> +       /* Set the max frame size to HW*/
> +       i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0,
> + NULL);
> +
>         return I40E_SUCCESS;
> 
>  tx_err:
> 
> 
> This way, the new fix is easier to read too.
> Thanks.
> 
> 
> --
> David Marchand

[PATCH v7] net/i40e: fix max frame size configuration

[Simei Su]

This patch sets max frame size at port level rather than queue level
to avoid unexpected packets received by port.

Fixes: 34fe803c051f ("net/i40e: don't check link status on device start")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
v7:
* Split v6 patch into two parts:
  - David Marchand's revert patch first:
    https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-david.marchand@redhat.com/
  - issue fix patch
* Refine commit title and commit log.

v6:
* Refine commit log.
* Remove return error.

v5:
* Fix misspelling in commit log.

v4:
* Refine commit log.
* Avoid duplicate call to set parameter "wait to complete" true.

v3:
* Put link update before interrupt enable.

v2:
* Refine commit log.
* Add link update.

 drivers/net/i40e/i40e_ethdev.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index a982e42..371f422 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1710,11 +1710,6 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	 */
 	i40e_add_tx_flow_control_drop_filter(pf);
 
-	/* Set the max frame size to 0x2600 by default,
-	 * in case other drivers changed the default value.
-	 */
-	i40e_aq_set_mac_config(hw, I40E_FRAME_SIZE_MAX, TRUE, false, 0, NULL);
-
 	/* initialize RSS rule list */
 	TAILQ_INIT(&pf->rss_config_list);
 
@@ -2332,6 +2327,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
 	uint32_t intr_vector = 0;
 	struct i40e_vsi *vsi;
 	uint16_t nb_rxq, nb_txq;
+	uint16_t max_frame_size;
 
 	hw->adapter_stopped = 0;
 
@@ -2452,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
 
 		/* Call get_link_info aq command to enable/disable LSE */
-		i40e_dev_link_update(dev, 0);
+		i40e_dev_link_update(dev, 1);
 	}
 
 	if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2470,6 +2466,13 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+
 	return I40E_SUCCESS;
 
 tx_err:
-- 
2.9.5

RE: [PATCH v7] net/i40e: fix max frame size configuration

[Zhang, Qi Z]

> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Monday, March 6, 2023 8:19 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> david.marchand@redhat.com; Su, Simei <simei.su@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v7] net/i40e: fix max frame size configuration
> 
> This patch sets max frame size at port level rather than queue level to avoid
> unexpected packets received by port.
> 
> Fixes: 34fe803c051f ("net/i40e: don't check link status on device start")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Simei Su <simei.su@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [PATCH v6] net/i40e: rework maximum frame size configuration

[Kevin Traynor]

On 27/02/2023 00:35, Zhang, Qi Z wrote:
> 

Hi Simei/Qi/Yu

> 
>> -----Original Message-----
>> From: Su, Simei <simei.su@intel.com>
>> Sent: Monday, February 20, 2023 4:00 PM
>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>> david.marchand@redhat.com
>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
>> <simei.su@intel.com>; stable@dpdk.org
>> Subject: [PATCH v6] net/i40e: rework maximum frame size configuration
>>
>> One issue is reported by David Marchand that error occurs in OVS due to the
>> fix patch in mentioned changes below. The detailed reproduce step and
>> result are in https://patchwork.dpdk.org/project/dpdk/patch/
>> 20211207085946.121032-1-dapengx.yu@intel.com/.
>>
>> This patch removes unnecessary link status check and directly sets mac config
>> in dev_start. Also, it sets the parameter "wait to complete" true to wait for
>> more time to make sure adminq command execute completed.
>>

>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
>> Cc: stable@dpdk.org

These patches caused an observable regression in multiple 20.11 and 
21.11 LTS releases that was only caught a long time after releases.

Is there anything being added to LTS validation for regression testing 
this issue, so we don't get caught again?

After reverting the original patch and 2 fixes, I'm a bit reluctant to 
take more fixes without some form of regression testing in place.

thanks,
Kevin.

>>
>> Reported-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Simei Su <simei.su@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Applied to dpdk-next-net-intel.
> 
> Thanks
> Qi
> 

Re: [PATCH v6] net/i40e: rework maximum frame size configuration

[Kevin Traynor]

On 22/03/2023 16:50, Kevin Traynor wrote:
> On 27/02/2023 00:35, Zhang, Qi Z wrote:
>>
> 
> Hi Simei/Qi/Yu
> 

Hi Yu,

>>
>>> -----Original Message-----
>>> From: Su, Simei <simei.su@intel.com>
>>> Sent: Monday, February 20, 2023 4:00 PM
>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
>>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>>> david.marchand@redhat.com
>>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
>>> <simei.su@intel.com>; stable@dpdk.org
>>> Subject: [PATCH v6] net/i40e: rework maximum frame size configuration
>>>
>>> One issue is reported by David Marchand that error occurs in OVS due to the
>>> fix patch in mentioned changes below. The detailed reproduce step and
>>> result are in https://patchwork.dpdk.org/project/dpdk/patch/
>>> 20211207085946.121032-1-dapengx.yu@intel.com/.
>>>
>>> This patch removes unnecessary link status check and directly sets mac config
>>> in dev_start. Also, it sets the parameter "wait to complete" true to wait for
>>> more time to make sure adminq command execute completed.
>>>
> 
>>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
>>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
>>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
>>> Cc: stable@dpdk.org
> 
> These patches caused an observable regression in multiple 20.11 and
> 21.11 LTS releases that was only caught a long time after releases.
> 
> Is there anything being added to LTS validation for regression testing
> this issue, so we don't get caught again?
> 

This is the issue I was talking about earlier during the release 
meeting. Not sure if we were talking about the same patch.

I was asking if there are some regression tests added/can be added to 
LTS validation that will be run during each LTS validation cycle so we 
don't have any more regressions on it.

thanks,
Kevin.

> After reverting the original patch and 2 fixes, I'm a bit reluctant to
> take more fixes without some form of regression testing in place.
> 
> thanks,
> Kevin.
> 
>>>
>>> Reported-by: David Marchand <david.marchand@redhat.com>
>>> Signed-off-by: Simei Su <simei.su@intel.com>
>>
>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>
>> Applied to dpdk-next-net-intel.
>>
>> Thanks
>> Qi
>>
> 

RE: [PATCH v6] net/i40e: rework maximum frame size configuration

[Jiang, YuX]

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Thursday, March 23, 2023 10:51 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Su, Simei <simei.su@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
> david.marchand@redhat.com; Jiang, YuX <yux.jiang@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org;
> Luca Boccassi <bluca@debian.org>; Mcnamara, John
> <john.mcnamara@intel.com>
> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
> 
> On 22/03/2023 16:50, Kevin Traynor wrote:
> > On 27/02/2023 00:35, Zhang, Qi Z wrote:
> >>
> >
> > Hi Simei/Qi/Yu
> >
> 
> Hi Yu,
> 
> >>
> >>> -----Original Message-----
> >>> From: Su, Simei <simei.su@intel.com>
> >>> Sent: Monday, February 20, 2023 4:00 PM
> >>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> >>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >>> david.marchand@redhat.com
> >>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
> >>> <simei.su@intel.com>; stable@dpdk.org
> >>> Subject: [PATCH v6] net/i40e: rework maximum frame size
> >>> configuration
> >>>
> >>> One issue is reported by David Marchand that error occurs in OVS due
> >>> to the fix patch in mentioned changes below. The detailed reproduce
> >>> step and result are in
> >>> https://patchwork.dpdk.org/project/dpdk/patch/
> >>> 20211207085946.121032-1-dapengx.yu@intel.com/.
> >>>
> >>> This patch removes unnecessary link status check and directly sets
> >>> mac config in dev_start. Also, it sets the parameter "wait to
> >>> complete" true to wait for more time to make sure adminq command
> execute completed.
> >>>
> >
> >>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> >>> level")
> >>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> >>> level")
> >>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> >>> Cc: stable@dpdk.org
> >
> > These patches caused an observable regression in multiple 20.11 and
> > 21.11 LTS releases that was only caught a long time after releases.
> >
> > Is there anything being added to LTS validation for regression testing
> > this issue, so we don't get caught again?
> >
> 
> This is the issue I was talking about earlier during the release meeting. Not sure
> if we were talking about the same patch.
> 
> I was asking if there are some regression tests added/can be added to LTS
> validation that will be run during each LTS validation cycle so we don't have any
> more regressions on it.
> 
Hi Kevin, 
Thanks for your comments.
Yes. We are adding additional case to cover more testing. For main branch, we have done the regression testing (including the additional case testing), they both work well.
We hope the two related patches can be backported to LTS branch, and the second patch just reworks for previous bug's fix.
	Patch1: https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-david.marchand@redhat.com/	a8ca8ed net/i40e: revert link status check on device start
	Pathc2: https://patchwork.dpdk.org/project/dpdk/patch/20230306121853.27547-1-simei.su@intel.com/	82fcf20 net/i40e: fix maximum frame size configuration

Best regards,
Yu Jiang

> thanks,
> Kevin.
> 
> > After reverting the original patch and 2 fixes, I'm a bit reluctant to
> > take more fixes without some form of regression testing in place.
> >
> > thanks,
> > Kevin.
> >
> >>>
> >>> Reported-by: David Marchand <david.marchand@redhat.com>
> >>> Signed-off-by: Simei Su <simei.su@intel.com>
> >>
> >> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>
> >> Applied to dpdk-next-net-intel.
> >>
> >> Thanks
> >> Qi
> >>
> >

Re: [PATCH v6] net/i40e: rework maximum frame size configuration

[Kevin Traynor]

On 24/03/2023 06:32, Jiang, YuX wrote:
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Thursday, March 23, 2023 10:51 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Su, Simei <simei.su@intel.com>; Xing,
>> Beilei <beilei.xing@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
>> david.marchand@redhat.com; Jiang, YuX <yux.jiang@intel.com>
>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org;
>> Luca Boccassi <bluca@debian.org>; Mcnamara, John
>> <john.mcnamara@intel.com>
>> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
>>
>> On 22/03/2023 16:50, Kevin Traynor wrote:
>>> On 27/02/2023 00:35, Zhang, Qi Z wrote:
>>>>
>>>
>>> Hi Simei/Qi/Yu
>>>
>>
>> Hi Yu,
>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Su, Simei <simei.su@intel.com>
>>>>> Sent: Monday, February 20, 2023 4:00 PM
>>>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
>>>>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>>>>> david.marchand@redhat.com
>>>>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
>>>>> <simei.su@intel.com>; stable@dpdk.org
>>>>> Subject: [PATCH v6] net/i40e: rework maximum frame size
>>>>> configuration
>>>>>
>>>>> One issue is reported by David Marchand that error occurs in OVS due
>>>>> to the fix patch in mentioned changes below. The detailed reproduce
>>>>> step and result are in
>>>>> https://patchwork.dpdk.org/project/dpdk/patch/
>>>>> 20211207085946.121032-1-dapengx.yu@intel.com/.
>>>>>
>>>>> This patch removes unnecessary link status check and directly sets
>>>>> mac config in dev_start. Also, it sets the parameter "wait to
>>>>> complete" true to wait for more time to make sure adminq command
>> execute completed.
>>>>>
>>>
>>>>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
>>>>> level")
>>>>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
>>>>> level")
>>>>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
>>>>> Cc: stable@dpdk.org
>>>
>>> These patches caused an observable regression in multiple 20.11 and
>>> 21.11 LTS releases that was only caught a long time after releases.
>>>
>>> Is there anything being added to LTS validation for regression testing
>>> this issue, so we don't get caught again?
>>>
>>
>> This is the issue I was talking about earlier during the release meeting. Not sure
>> if we were talking about the same patch.
>>
>> I was asking if there are some regression tests added/can be added to LTS
>> validation that will be run during each LTS validation cycle so we don't have any
>> more regressions on it.
>>
> Hi Kevin,
> Thanks for your comments.
> Yes. We are adding additional case to cover more testing. For main branch, we have done the regression testing (including the additional case testing), they both work well.

That's good to hear. Will the additional regression tests also be added 
to the LTS validation tests when they are run?

> We hope the two related patches can be backported to LTS branch, and the second patch just reworks for previous bug's fix.
> 	Patch1: https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-david.marchand@redhat.com/	a8ca8ed net/i40e: revert link status check on device start

I have already reverted those 3 backports on 21.11 branch so this is not 
needed.

> 	Pathc2: https://patchwork.dpdk.org/project/dpdk/patch/20230306121853.27547-1-simei.su@intel.com/	82fcf20 net/i40e: fix maximum frame size configuration
> 

That is the v7 of this v6 with revert and fix split, so same one being 
discussed above.

> Best regards,
> Yu Jiang
> 
>> thanks,
>> Kevin.
>>
>>> After reverting the original patch and 2 fixes, I'm a bit reluctant to
>>> take more fixes without some form of regression testing in place.
>>>
>>> thanks,
>>> Kevin.
>>>
>>>>>
>>>>> Reported-by: David Marchand <david.marchand@redhat.com>
>>>>> Signed-off-by: Simei Su <simei.su@intel.com>
>>>>
>>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>>>
>>>> Applied to dpdk-next-net-intel.
>>>>
>>>> Thanks
>>>> Qi
>>>>
>>>
> 

RE: [PATCH v6] net/i40e: rework maximum frame size configuration

[Jiang, YuX]

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Friday, March 24, 2023 5:40 PM
> To: Jiang, YuX <yux.jiang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Su,
> Simei <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> Yuying <yuying.zhang@intel.com>; david.marchand@redhat.com
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org;
> Luca Boccassi <bluca@debian.org>; Mcnamara, John
> <john.mcnamara@intel.com>
> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
> 
> On 24/03/2023 06:32, Jiang, YuX wrote:
> >> -----Original Message-----
> >> From: Kevin Traynor <ktraynor@redhat.com>
> >> Sent: Thursday, March 23, 2023 10:51 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Su, Simei
> >> <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> >> Yuying <yuying.zhang@intel.com>; david.marchand@redhat.com; Jiang,
> >> YuX <yux.jiang@intel.com>
> >> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> >> stable@dpdk.org; Luca Boccassi <bluca@debian.org>; Mcnamara, John
> >> <john.mcnamara@intel.com>
> >> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size
> >> configuration
> >>
> >> On 22/03/2023 16:50, Kevin Traynor wrote:
> >>> On 27/02/2023 00:35, Zhang, Qi Z wrote:
> >>>>
> >>>
> >>> Hi Simei/Qi/Yu
> >>>
> >>
> >> Hi Yu,
> >>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Su, Simei <simei.su@intel.com>
> >>>>> Sent: Monday, February 20, 2023 4:00 PM
> >>>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> >>>>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >>>>> david.marchand@redhat.com
> >>>>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
> >>>>> <simei.su@intel.com>; stable@dpdk.org
> >>>>> Subject: [PATCH v6] net/i40e: rework maximum frame size
> >>>>> configuration
> >>>>>
> >>>>> One issue is reported by David Marchand that error occurs in OVS
> >>>>> due to the fix patch in mentioned changes below. The detailed
> >>>>> reproduce step and result are in
> >>>>> https://patchwork.dpdk.org/project/dpdk/patch/
> >>>>> 20211207085946.121032-1-dapengx.yu@intel.com/.
> >>>>>
> >>>>> This patch removes unnecessary link status check and directly sets
> >>>>> mac config in dev_start. Also, it sets the parameter "wait to
> >>>>> complete" true to wait for more time to make sure adminq command
> >> execute completed.
> >>>>>
> >>>
> >>>>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> >>>>> level")
> >>>>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> >>>>> level")
> >>>>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> >>>>> Cc: stable@dpdk.org
> >>>
> >>> These patches caused an observable regression in multiple 20.11 and
> >>> 21.11 LTS releases that was only caught a long time after releases.
> >>>
> >>> Is there anything being added to LTS validation for regression
> >>> testing this issue, so we don't get caught again?
> >>>
> >>
> >> This is the issue I was talking about earlier during the release
> >> meeting. Not sure if we were talking about the same patch.
> >>
> >> I was asking if there are some regression tests added/can be added to
> >> LTS validation that will be run during each LTS validation cycle so
> >> we don't have any more regressions on it.
> >>
> > Hi Kevin,
> > Thanks for your comments.
> > Yes. We are adding additional case to cover more testing. For main branch,
> we have done the regression testing (including the additional case testing), they
> both work well.
> 
> That's good to hear. Will the additional regression tests also be added to the
> LTS validation tests when they are run?
> 
Yes, we will test the additional case on each LTS version from now on.

Best regards,
Yu Jiang

> > We hope the two related patches can be backported to LTS branch, and the
> second patch just reworks for previous bug's fix.
> > 	Patch1:
> https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-
> david.marchand@redhat.com/	a8ca8ed net/i40e: revert link status check on
> device start
> 
> I have already reverted those 3 backports on 21.11 branch so this is not needed.
> 
> > 	Pathc2:
> https://patchwork.dpdk.org/project/dpdk/patch/20230306121853.27547-1-
> simei.su@intel.com/	82fcf20 net/i40e: fix maximum frame size
> configuration
> >
> 
> That is the v7 of this v6 with revert and fix split, so same one being discussed
> above.
> 
> > Best regards,
> > Yu Jiang
> >
> >> thanks,
> >> Kevin.
> >>
> >>> After reverting the original patch and 2 fixes, I'm a bit reluctant
> >>> to take more fixes without some form of regression testing in place.
> >>>
> >>> thanks,
> >>> Kevin.
> >>>
> >>>>>
> >>>>> Reported-by: David Marchand <david.marchand@redhat.com>
> >>>>> Signed-off-by: Simei Su <simei.su@intel.com>
> >>>>
> >>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>>
> >>>> Applied to dpdk-next-net-intel.
> >>>>
> >>>> Thanks
> >>>> Qi
> >>>>
> >>>
> >

RE: [PATCH v6] net/i40e: rework maximum frame size configuration

[Su, Simei]

Hi Kevin,

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Friday, March 24, 2023 5:40 PM
> To: Jiang, YuX <yux.jiang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Su,
> Simei <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> Yuying <yuying.zhang@intel.com>; david.marchand@redhat.com
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org;
> Luca Boccassi <bluca@debian.org>; Mcnamara, John
> <john.mcnamara@intel.com>
> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
> 
> On 24/03/2023 06:32, Jiang, YuX wrote:
> >> -----Original Message-----
> >> From: Kevin Traynor <ktraynor@redhat.com>
> >> Sent: Thursday, March 23, 2023 10:51 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Su, Simei
> >> <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> >> Yuying <yuying.zhang@intel.com>; david.marchand@redhat.com; Jiang,
> >> YuX <yux.jiang@intel.com>
> >> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> >> stable@dpdk.org; Luca Boccassi <bluca@debian.org>; Mcnamara, John
> >> <john.mcnamara@intel.com>
> >> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size
> >> configuration
> >>
> >> On 22/03/2023 16:50, Kevin Traynor wrote:
> >>> On 27/02/2023 00:35, Zhang, Qi Z wrote:
> >>>>
> >>>
> >>> Hi Simei/Qi/Yu
> >>>
> >>
> >> Hi Yu,
> >>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Su, Simei <simei.su@intel.com>
> >>>>> Sent: Monday, February 20, 2023 4:00 PM
> >>>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> >>>>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >>>>> david.marchand@redhat.com
> >>>>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
> >>>>> <simei.su@intel.com>; stable@dpdk.org
> >>>>> Subject: [PATCH v6] net/i40e: rework maximum frame size
> >>>>> configuration
> >>>>>
> >>>>> One issue is reported by David Marchand that error occurs in OVS
> >>>>> due to the fix patch in mentioned changes below. The detailed
> >>>>> reproduce step and result are in
> >>>>> https://patchwork.dpdk.org/project/dpdk/patch/
> >>>>> 20211207085946.121032-1-dapengx.yu@intel.com/.
> >>>>>
> >>>>> This patch removes unnecessary link status check and directly sets
> >>>>> mac config in dev_start. Also, it sets the parameter "wait to
> >>>>> complete" true to wait for more time to make sure adminq command
> >> execute completed.
> >>>>>
> >>>
> >>>>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> >>>>> level")
> >>>>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> >>>>> level")
> >>>>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> >>>>> Cc: stable@dpdk.org
> >>>
> >>> These patches caused an observable regression in multiple 20.11 and
> >>> 21.11 LTS releases that was only caught a long time after releases.
> >>>
> >>> Is there anything being added to LTS validation for regression
> >>> testing this issue, so we don't get caught again?
> >>>
> >>
> >> This is the issue I was talking about earlier during the release
> >> meeting. Not sure if we were talking about the same patch.
> >>
> >> I was asking if there are some regression tests added/can be added to
> >> LTS validation that will be run during each LTS validation cycle so
> >> we don't have any more regressions on it.
> >>
> > Hi Kevin,
> > Thanks for your comments.
> > Yes. We are adding additional case to cover more testing. For main branch,
> we have done the regression testing (including the additional case testing),
> they both work well.
> 
> That's good to hear. Will the additional regression tests also be added to the
> LTS validation tests when they are run?
> 
> > We hope the two related patches can be backported to LTS branch, and the
> second patch just reworks for previous bug's fix.
> > 	Patch1:
> https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-d
> avid.marchand@redhat.com/	a8ca8ed net/i40e: revert link status check
> on device start
> 
> I have already reverted those 3 backports on 21.11 branch so this is not
> needed.
> 
> > 	Pathc2:
> https://patchwork.dpdk.org/project/dpdk/patch/20230306121853.27547-1-s
> imei.su@intel.com/	82fcf20 net/i40e: fix maximum frame size
> configuration
> >
> 
> That is the v7 of this v6 with revert and fix split, so same one being discussed
> above.

The fix split is to rework and simplify code in previous bug fix patch for unexpected packets received issue.

Best Regards,
Simei

> 
> > Best regards,
> > Yu Jiang
> >
> >> thanks,
> >> Kevin.
> >>
> >>> After reverting the original patch and 2 fixes, I'm a bit reluctant
> >>> to take more fixes without some form of regression testing in place.
> >>>
> >>> thanks,
> >>> Kevin.
> >>>
> >>>>>
> >>>>> Reported-by: David Marchand <david.marchand@redhat.com>
> >>>>> Signed-off-by: Simei Su <simei.su@intel.com>
> >>>>
> >>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>>
> >>>> Applied to dpdk-next-net-intel.
> >>>>
> >>>> Thanks
> >>>> Qi
> >>>>
> >>>
> >

RE: [PATCH v6] net/i40e: rework maximum frame size configuration

[Su, Simei]

Sorry for sending this email incorrectly, please ignore this mail.

Thanks,
Simei

> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Monday, February 20, 2023 3:27 PM
> To: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Cc: Su, Simei <simei.su@intel.com>; stable@dpdk.org
> Subject: [PATCH v6] net/i40e: rework maximum frame size configuration
> 
> The issue reported by David is that error occurs in OVS due to the fix patch in
> mentionned changes below. The detailed reproduce step and result is in
> https://patchwork.dpdk.org/project/dpdk/patch/
> 20211207085946.121032-1-dapengx.yu@intel.com/.
> 
> This patch removes unnecessary link status check and directly sets mac config
> in dev_start. Also, it sets the parameter "wait to complete" true to wait for
> more time to make sure adminq command execute completed.
> 
> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Simei Su <simei.su@intel.com>
> ---
> v6:
> * Refine commit log.
> * Remove return error.
> 
> v5:
> * Fix misspelling in commit log.
> 
> v4:
> * Refine commit log.
> * Avoid duplicate call to set parameter "wait to complete" true.
> 
> v3:
> * Put link update before interrupt enable.
> 
> v2:
> * Refine commit log.
> * Add link update.
> 
>  drivers/net/i40e/i40e_ethdev.c | 50 ++++++++----------------------------------
>  1 file changed, 9 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7726a89d..30c904c 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
>  				      struct rte_ether_addr *mac_addr);
> 
>  static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); -static
> void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
> 
>  static int i40e_ethertype_filter_convert(
>  	const struct rte_eth_ethertype_filter *input, @@ -2449,7 +2448,7 @@
> i40e_dev_start(struct rte_eth_dev *dev)
>  			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
> 
>  		/* Call get_link_info aq command to enable/disable LSE */
> -		i40e_dev_link_update(dev, 0);
> +		i40e_dev_link_update(dev, 1);
>  	}
> 
>  	if (dev->data->dev_conf.intr_conf.rxq == 0) { @@ -2467,8 +2466,12 @@
> i40e_dev_start(struct rte_eth_dev *dev)
>  			    "please call hierarchy_commit() "
>  			    "before starting the port");
> 
> -	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
> -	i40e_set_mac_max_frame(dev, max_frame_size);
> +	max_frame_size = dev->data->mtu ?
> +		dev->data->mtu + I40E_ETH_OVERHEAD :
> +		I40E_FRAME_SIZE_MAX;
> +
> +	/* Set the max frame size to HW*/
> +	i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
> 
>  	return I40E_SUCCESS;
> 
> @@ -2809,9 +2812,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
>  	return i40e_phy_conf_link(hw, abilities, speed, false);  }
> 
> -#define CHECK_INTERVAL             100  /* 100ms */
> -#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
> -
>  static __rte_always_inline void
>  update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)  { @@
> -2878,6 +2878,8 @@ static __rte_always_inline void  update_link_aq(struct
> i40e_hw *hw, struct rte_eth_link *link,
>  	bool enable_lse, int wait_to_complete)  {
> +#define CHECK_INTERVAL             100  /* 100ms */
> +#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total
> */
>  	uint32_t rep_cnt = MAX_REPEAT_TIME;
>  	struct i40e_link_status link_status;
>  	int status;
> @@ -12123,40 +12125,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf
> *pf)
>  	return ret;
>  }
> 
> -static void
> -i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size) -{
> -	struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> -	uint32_t rep_cnt = MAX_REPEAT_TIME;
> -	struct rte_eth_link link;
> -	enum i40e_status_code status;
> -	bool can_be_set = true;
> -
> -	/*
> -	 * I40E_MEDIA_TYPE_BASET link up can be ignored
> -	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
> -	 * is I40E_MEDIA_TYPE_UNKNOWN
> -	 */
> -	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
> -	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
> -		do {
> -			update_link_reg(hw, &link);
> -			if (link.link_status)
> -				break;
> -			rte_delay_ms(CHECK_INTERVAL);
> -		} while (--rep_cnt);
> -		can_be_set = !!link.link_status;
> -	}
> -
> -	if (can_be_set) {
> -		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
> -		if (status != I40E_SUCCESS)
> -			PMD_DRV_LOG(ERR, "Failed to set max frame size at port
> level");
> -	} else {
> -		PMD_DRV_LOG(ERR, "Set max frame size at port level not
> applicable on link down");
> -	}
> -}
> -
>  RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
> RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);  #ifdef
> RTE_ETHDEV_DEBUG_RX
> --
> 2.9.5

[PATCH v6] net/i40e: rework maximum frame size configuration

[Simei Su]

The issue reported by David is that error occurs in OVS due to the fix
patch in mentionned changes below. The detailed reproduce step and result
is in https://patchwork.dpdk.org/project/dpdk/patch/
20211207085946.121032-1-dapengx.yu@intel.com/.

This patch removes unnecessary link status check and directly sets mac
config in dev_start. Also, it sets the parameter "wait to complete" true
to wait for more time to make sure adminq command execute completed.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
v6:
* Refine commit log.
* Remove return error.

v5:
* Fix misspelling in commit log.

v4:
* Refine commit log.
* Avoid duplicate call to set parameter "wait to complete" true.

v3:
* Put link update before interrupt enable.

v2:
* Refine commit log.
* Add link update.

 drivers/net/i40e/i40e_ethdev.c | 50 ++++++++----------------------------------
 1 file changed, 9 insertions(+), 41 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d..30c904c 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
 	const struct rte_eth_ethertype_filter *input,
@@ -2449,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
 
 		/* Call get_link_info aq command to enable/disable LSE */
-		i40e_dev_link_update(dev, 0);
+		i40e_dev_link_update(dev, 1);
 	}
 
 	if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2467,8 +2466,12 @@ i40e_dev_start(struct rte_eth_dev *dev)
 			    "please call hierarchy_commit() "
 			    "before starting the port");
 
-	max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-	i40e_set_mac_max_frame(dev, max_frame_size);
+	max_frame_size = dev->data->mtu ?
+		dev->data->mtu + I40E_ETH_OVERHEAD :
+		I40E_FRAME_SIZE_MAX;
+
+	/* Set the max frame size to HW*/
+	i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
 
 	return I40E_SUCCESS;
 
@@ -2809,9 +2812,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	return i40e_phy_conf_link(hw, abilities, speed, false);
 }
 
-#define CHECK_INTERVAL             100  /* 100ms */
-#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
-
 static __rte_always_inline void
 update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)
 {
@@ -2878,6 +2878,8 @@ static __rte_always_inline void
 update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
 	bool enable_lse, int wait_to_complete)
 {
+#define CHECK_INTERVAL             100  /* 100ms */
+#define MAX_REPEAT_TIME            10  /* 1s (10 * 100ms) in total */
 	uint32_t rep_cnt = MAX_REPEAT_TIME;
 	struct i40e_link_status link_status;
 	int status;
@@ -12123,40 +12125,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
 	return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t rep_cnt = MAX_REPEAT_TIME;
-	struct rte_eth_link link;
-	enum i40e_status_code status;
-	bool can_be_set = true;
-
-	/*
-	 * I40E_MEDIA_TYPE_BASET link up can be ignored
-	 * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-	 * is I40E_MEDIA_TYPE_UNKNOWN
-	 */
-	if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-	    hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-		do {
-			update_link_reg(hw, &link);
-			if (link.link_status)
-				break;
-			rte_delay_ms(CHECK_INTERVAL);
-		} while (--rep_cnt);
-		can_be_set = !!link.link_status;
-	}
-
-	if (can_be_set) {
-		status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-		if (status != I40E_SUCCESS)
-			PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
-	} else {
-		PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
-	}
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
 #ifdef RTE_ETHDEV_DEBUG_RX
-- 
2.9.5