patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH] net/i40e: enable max frame size at port level
@ 2021-12-07  8:59 dapengx.yu
       [not found] ` <BYAPR11MB2711AB91F380189FB6AFCC52FE359@BYAPR11MB2711.namprd11.prod.outlook.com>
  2022-12-12 13:53 ` David Marchand
  0 siblings, 2 replies; 20+ messages in thread
From: dapengx.yu @ 2021-12-07  8:59 UTC (permalink / raw)
  To: Beilei Xing; +Cc: dev, Dapeng Yu, stable

From: Dapeng Yu <dapengx.yu@intel.com>

Currently max frame size is set at queue level, which makes the values
of the following counters wrong when a jumbo frame is received.

The expected value:
rx_good_bytes: 0
rx_errors: 1
rx_oversize_errors: 1

The actual value:
rx_good_bytes: 1626
rx_errors: 0
rx_oversize_errors: 0

This patch enables setting max frame size at port level, and makes the
values above right.

Cc: stable@dpdk.org

Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 41 ++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index c0bfff43ee..ef9b2b2414 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -386,6 +386,7 @@ 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,
@@ -1709,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);
 
@@ -2364,6 +2360,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;
 
@@ -2502,6 +2499,9 @@ 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);
+
 	return I40E_SUCCESS;
 
 tx_err:
@@ -2848,6 +2848,9 @@ 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)
 {
@@ -2914,8 +2917,6 @@ 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;
@@ -6719,6 +6720,7 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
 			if (!ret)
 				rte_eth_dev_callback_process(dev,
 					RTE_ETH_EVENT_INTR_LSC, NULL);
+
 			break;
 		default:
 			PMD_DRV_LOG(DEBUG, "Request %u is not supported yet",
@@ -12103,6 +12105,31 @@ 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;
+
+	do {
+		update_link_reg(hw, &link);
+		if (link.link_status)
+			break;
+
+		rte_delay_ms(CHECK_INTERVAL);
+	} while (--rep_cnt);
+
+	if (link.link_status) {
+		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.27.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] net/i40e: enable max frame size at port level
       [not found]   ` <BYAPR11MB27110434B9262C0E2BED3AD3FE3A9@BYAPR11MB2711.namprd11.prod.outlook.com>
@ 2022-02-21 10:42     ` Zhang, Peng1X
  2022-02-21 11:22       ` Zhang, Qi Z
  0 siblings, 1 reply; 20+ messages in thread
From: Zhang, Peng1X @ 2022-02-21 10:42 UTC (permalink / raw)
  To: dev, stable

> -----Original Message-----
> From: dapengx.yu@intel.com <dapengx.yu@intel.com>
> Sent: Tuesday, December 7, 2021 5:00 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Yu, DapengX <dapengx.yu@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/i40e: enable max frame size at port level
> 
> From: Dapeng Yu <dapengx.yu@intel.com>
> 
> Currently max frame size is set at queue level, which makes the values of the
> following counters wrong when a jumbo frame is received.
> 
> The expected value:
> rx_good_bytes: 0
> rx_errors: 1
> rx_oversize_errors: 1
> 
> The actual value:
> rx_good_bytes: 1626
> rx_errors: 0
> rx_oversize_errors: 0
> 
> This patch enables setting max frame size at port level, and makes the values
> above right.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> --
Tested-by: Peng Zhang <peng1x.zhang@intel.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] net/i40e: enable max frame size at port level
  2022-02-21 10:42     ` Zhang, Peng1X
@ 2022-02-21 11:22       ` Zhang, Qi Z
  2022-02-22  9:52         ` Kevin Traynor
  0 siblings, 1 reply; 20+ messages in thread
From: Zhang, Qi Z @ 2022-02-21 11:22 UTC (permalink / raw)
  To: Zhang, Peng1X, dev, stable



> -----Original Message-----
> From: Zhang, Peng1X <peng1x.zhang@intel.com>
> Sent: Monday, February 21, 2022 6:43 PM
> To: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] net/i40e: enable max frame size at port level
> 
> > -----Original Message-----
> > From: dapengx.yu@intel.com <dapengx.yu@intel.com>
> > Sent: Tuesday, December 7, 2021 5:00 PM
> > To: Xing, Beilei <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Yu, DapengX <dapengx.yu@intel.com>; stable@dpdk.org
> > Subject: [PATCH] net/i40e: enable max frame size at port level
> >
> > From: Dapeng Yu <dapengx.yu@intel.com>
> >
> > Currently max frame size is set at queue level, which makes the values
> > of the following counters wrong when a jumbo frame is received.
> >
> > The expected value:
> > rx_good_bytes: 0
> > rx_errors: 1
> > rx_oversize_errors: 1
> >
> > The actual value:
> > rx_good_bytes: 1626
> > rx_errors: 0
> > rx_oversize_errors: 0
> >
> > This patch enables setting max frame size at port level, and makes the
> > values above right.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> > --
> Tested-by: Peng Zhang <peng1x.zhang@intel.com>

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

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] net/i40e: enable max frame size at port level
  2022-02-21 11:22       ` Zhang, Qi Z
@ 2022-02-22  9:52         ` Kevin Traynor
  0 siblings, 0 replies; 20+ messages in thread
From: Kevin Traynor @ 2022-02-22  9:52 UTC (permalink / raw)
  To: Zhang, Qi Z, Zhang, Peng1X, dev, stable

On 21/02/2022 11:22, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Zhang, Peng1X <peng1x.zhang@intel.com>
>> Sent: Monday, February 21, 2022 6:43 PM
>> To: dev@dpdk.org; stable@dpdk.org
>> Subject: RE: [PATCH] net/i40e: enable max frame size at port level
>>
>>> -----Original Message-----
>>> From: dapengx.yu@intel.com <dapengx.yu@intel.com>
>>> Sent: Tuesday, December 7, 2021 5:00 PM
>>> To: Xing, Beilei <beilei.xing@intel.com>
>>> Cc: dev@dpdk.org; Yu, DapengX <dapengx.yu@intel.com>; stable@dpdk.org
>>> Subject: [PATCH] net/i40e: enable max frame size at port level
>>>
>>> From: Dapeng Yu <dapengx.yu@intel.com>
>>>
>>> Currently max frame size is set at queue level, which makes the values
>>> of the following counters wrong when a jumbo frame is received.
>>>
>>> The expected value:
>>> rx_good_bytes: 0
>>> rx_errors: 1
>>> rx_oversize_errors: 1
>>>
>>> The actual value:
>>> rx_good_bytes: 1626
>>> rx_errors: 0
>>> rx_oversize_errors: 0
>>>
>>> This patch enables setting max frame size at port level, and makes the
>>> values above right.
>>>
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
>>> --
>> Tested-by: Peng Zhang <peng1x.zhang@intel.com>
> 
> Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Applied to dpdk-next-net-intel.
> 

Please add a Fixes: tag so we can know which LTS it is relevant for.

> Thanks
> Qi
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] net/i40e: enable max frame size at port level
  2021-12-07  8:59 [PATCH] net/i40e: enable max frame size at port level dapengx.yu
       [not found] ` <BYAPR11MB2711AB91F380189FB6AFCC52FE359@BYAPR11MB2711.namprd11.prod.outlook.com>
@ 2022-12-12 13:53 ` David Marchand
  2022-12-12 13:58   ` David Marchand
  1 sibling, 1 reply; 20+ messages in thread
From: David Marchand @ 2022-12-12 13:53 UTC (permalink / raw)
  To: dapengx.yu
  Cc: Beilei Xing, dev, stable, Qi Zhang, Thomas Monjalon,
	Kevin Traynor, Luca Boccassi, Ferruh Yigit, Timothy Redaelli,
	Christophe Fontaine

On Tue, Dec 7, 2021 at 10:02 AM <dapengx.yu@intel.com> wrote:
>
> From: Dapeng Yu <dapengx.yu@intel.com>
>
> Currently max frame size is set at queue level, which makes the values
> of the following counters wrong when a jumbo frame is received.
>
> The expected value:
> rx_good_bytes: 0
> rx_errors: 1
> rx_oversize_errors: 1
>
> The actual value:
> rx_good_bytes: 1626
> rx_errors: 0
> rx_oversize_errors: 0
>
> This patch enables setting max frame size at port level, and makes the
> values above right.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>

I am looking again at this patch that triggered regressions in stable
branches, when used with OVS.


> ---
>  drivers/net/i40e/i40e_ethdev.c | 41 ++++++++++++++++++++++++++++------
>  1 file changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index c0bfff43ee..ef9b2b2414 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -386,6 +386,7 @@ 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,
> @@ -1709,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);
> -

If I understand correctly, the max framesize (9728) was always applied
in dev_init, before this change.


>         /* initialize RSS rule list */
>         TAILQ_INIT(&pf->rss_config_list);
>
> @@ -2364,6 +2360,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;
>
> @@ -2502,6 +2499,9 @@ 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);
> +

And now this change tries to configure, at dev_start step, the max
frame size that the application actually requested.
Which seems ok, afaiu.


>         return I40E_SUCCESS;
>
>  tx_err:
> @@ -2848,6 +2848,9 @@ 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)
>  {
> @@ -2914,8 +2917,6 @@ 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;
> @@ -6719,6 +6720,7 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
>                         if (!ret)
>                                 rte_eth_dev_callback_process(dev,
>                                         RTE_ETH_EVENT_INTR_LSC, NULL);
> +
>                         break;
>                 default:
>                         PMD_DRV_LOG(DEBUG, "Request %u is not supported yet",
> @@ -12103,6 +12105,31 @@ 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;
> +
> +       do {
> +               update_link_reg(hw, &link);
> +               if (link.link_status)
> +                       break;
> +
> +               rte_delay_ms(CHECK_INTERVAL);
> +       } while (--rep_cnt);
> +
> +       if (link.link_status) {
> +               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");
> +       }

But here, applying the configuration when the link is up only, looks
very wrong to me.

If the link is down, then this configuration will never get applied.
And what is worse, the application has no indication of it since no
error code is propagated.

So the application thinks the port started, but it will never get "big" packets.


Like for example:
- unplug cable,
- start testpmd and configure mtu to 9000,
# dpdk-testpmd -l 0,2,3 --in-memory -a 0000:05:00.1 -- -i --disable-link-check
...
Configuring Port 0 (socket 0)
i40e_set_mac_max_frame(): Set max frame size at port level not
applicable on link down
Port 0: 3C:FD:FE:A0:A4:C5
Done

testpmd> port stop 0
Stopping ports...
Done

testpmd> port config mtu 0 9000

testpmd> port start 0
i40e_set_mac_max_frame(): Set max frame size at port level not
applicable on link down
Port 0: 3C:FD:FE:A0:A4:C5
Done

- plug back cable, and send "big" packets, like for example ping with
-s 8000 from other side of the cable,
- no "big" packet is received on the dpdk port, and its rx_errors
xstats keeps increasing,


Looking at the original change, why did you not ignore link status state?
I would just move "i40e_aq_set_mac_config(hw, I40E_FRAME_SIZE_MAX,
TRUE, false, 0, NULL);" in dev_start.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] net/i40e: enable max frame size at port level
  2022-12-12 13:53 ` David Marchand
@ 2022-12-12 13:58   ` David Marchand
  2022-12-12 14:37     ` [PATCH] net/i40e: drop link check when configuring frame size David Marchand
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2022-12-12 13:58 UTC (permalink / raw)
  To: Jie Wang, Yuan, DukaiX, Wenxuan Wu, Yuying Zhang
  Cc: Beilei Xing, dev, Dapeng Yu, stable, Qi Zhang, Thomas Monjalon,
	Kevin Traynor, Luca Boccassi, Ferruh Yigit, Timothy Redaelli,
	Christophe Fontaine

On Mon, Dec 12, 2022 at 2:53 PM David Marchand
<david.marchand@redhat.com> wrote:
> On Tue, Dec 7, 2021 at 10:02 AM <dapengx.yu@intel.com> wrote:
> >
> > From: Dapeng Yu <dapengx.yu@intel.com>

I got a bounce on his mail address, so adding people who
submitted/reviewed later fixes for this change.

Please guys, have a look.


> >
> > Currently max frame size is set at queue level, which makes the values
> > of the following counters wrong when a jumbo frame is received.
> >
> > The expected value:
> > rx_good_bytes: 0
> > rx_errors: 1
> > rx_oversize_errors: 1
> >
> > The actual value:
> > rx_good_bytes: 1626
> > rx_errors: 0
> > rx_oversize_errors: 0
> >
> > This patch enables setting max frame size at port level, and makes the
> > values above right.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
>
> I am looking again at this patch that triggered regressions in stable
> branches, when used with OVS.
>
>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 41 ++++++++++++++++++++++++++++------
> >  1 file changed, 34 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> > index c0bfff43ee..ef9b2b2414 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -386,6 +386,7 @@ 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,
> > @@ -1709,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);
> > -
>
> If I understand correctly, the max framesize (9728) was always applied
> in dev_init, before this change.
>
>
> >         /* initialize RSS rule list */
> >         TAILQ_INIT(&pf->rss_config_list);
> >
> > @@ -2364,6 +2360,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;
> >
> > @@ -2502,6 +2499,9 @@ 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);
> > +
>
> And now this change tries to configure, at dev_start step, the max
> frame size that the application actually requested.
> Which seems ok, afaiu.
>
>
> >         return I40E_SUCCESS;
> >
> >  tx_err:
> > @@ -2848,6 +2848,9 @@ 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)
> >  {
> > @@ -2914,8 +2917,6 @@ 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;
> > @@ -6719,6 +6720,7 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
> >                         if (!ret)
> >                                 rte_eth_dev_callback_process(dev,
> >                                         RTE_ETH_EVENT_INTR_LSC, NULL);
> > +
> >                         break;
> >                 default:
> >                         PMD_DRV_LOG(DEBUG, "Request %u is not supported yet",
> > @@ -12103,6 +12105,31 @@ 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;
> > +
> > +       do {
> > +               update_link_reg(hw, &link);
> > +               if (link.link_status)
> > +                       break;
> > +
> > +               rte_delay_ms(CHECK_INTERVAL);
> > +       } while (--rep_cnt);
> > +
> > +       if (link.link_status) {
> > +               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");
> > +       }
>
> But here, applying the configuration when the link is up only, looks
> very wrong to me.
>
> If the link is down, then this configuration will never get applied.
> And what is worse, the application has no indication of it since no
> error code is propagated.
>
> So the application thinks the port started, but it will never get "big" packets.
>
>
> Like for example:
> - unplug cable,
> - start testpmd and configure mtu to 9000,
> # dpdk-testpmd -l 0,2,3 --in-memory -a 0000:05:00.1 -- -i --disable-link-check
> ...
> Configuring Port 0 (socket 0)
> i40e_set_mac_max_frame(): Set max frame size at port level not
> applicable on link down
> Port 0: 3C:FD:FE:A0:A4:C5
> Done
>
> testpmd> port stop 0
> Stopping ports...
> Done
>
> testpmd> port config mtu 0 9000
>
> testpmd> port start 0
> i40e_set_mac_max_frame(): Set max frame size at port level not
> applicable on link down
> Port 0: 3C:FD:FE:A0:A4:C5
> Done
>
> - plug back cable, and send "big" packets, like for example ping with
> -s 8000 from other side of the cable,
> - no "big" packet is received on the dpdk port, and its rx_errors
> xstats keeps increasing,
>
>
> Looking at the original change, why did you not ignore link status state?
> I would just move "i40e_aq_set_mac_config(hw, I40E_FRAME_SIZE_MAX,
> TRUE, false, 0, NULL);" in dev_start.
>
>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] net/i40e: drop link check when configuring frame size
  2022-12-12 13:58   ` David Marchand
@ 2022-12-12 14:37     ` David Marchand
  2022-12-13  9:18       ` [PATCH v2] net/i40e: don't check link status on device start David Marchand
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2022-12-12 14:37 UTC (permalink / raw)
  To: dev
  Cc: stable, Yuying Zhang, Beilei Xing, Dapeng Yu, Qi Zhang,
	Wenxuan Wu, Jie Wang

This is a tentative fix, reverting mentionned changes below and only
moving max frame size configuration to dev_start.

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: David Marchand <david.marchand@redhat.com>
---
 drivers/net/i40e/i40e_ethdev.c | 46 +++-------------------------------
 1 file changed, 4 insertions(+), 42 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d99..5635dd03cf 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,
@@ -2328,7 +2327,6 @@ 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;
 
@@ -2467,8 +2465,8 @@ 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_aq_set_mac_config(hw, dev->data->mtu + I40E_ETH_OVERHEAD, TRUE,
+		false, 0, NULL);
 
 	return I40E_SUCCESS;
 
@@ -2809,9 +2807,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 +2873,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;
@@ -6738,7 +6735,6 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
 			if (!ret)
 				rte_eth_dev_callback_process(dev,
 					RTE_ETH_EVENT_INTR_LSC, NULL);
-
 			break;
 		default:
 			PMD_DRV_LOG(DEBUG, "Request %u is not supported yet",
@@ -12123,40 +12119,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.38.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2] net/i40e: don't check link status on device start
  2022-12-12 14:37     ` [PATCH] net/i40e: drop link check when configuring frame size David Marchand
@ 2022-12-13  9:18       ` David Marchand
  2023-01-03 14:02         ` David Marchand
  2023-03-06  6:53         ` Su, Simei
  0 siblings, 2 replies; 20+ messages in thread
From: David Marchand @ 2022-12-13  9:18 UTC (permalink / raw)
  To: dev
  Cc: stable, Yuying Zhang, Beilei Xing, Qi Zhang, Dapeng Yu,
	Wenxuan Wu, Jie Wang

The mentioned changes broke existing applications when the link status
of i40e ports is down at the time the port is started.
Revert those changes, the original issue will need a different fix.

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: David Marchand <david.marchand@redhat.com>
---
Changes since v1:
- since the CI reports a failure on v1, simplified the fix by only
  reverting commits,

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

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d99..a982e42264 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,
@@ -1711,6 +1710,11 @@ 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);
 
@@ -2328,7 +2332,6 @@ 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;
 
@@ -2467,9 +2470,6 @@ 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);
-
 	return I40E_SUCCESS;
 
 tx_err:
@@ -2809,9 +2809,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 +2875,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;
@@ -6738,7 +6737,6 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
 			if (!ret)
 				rte_eth_dev_callback_process(dev,
 					RTE_ETH_EVENT_INTR_LSC, NULL);
-
 			break;
 		default:
 			PMD_DRV_LOG(DEBUG, "Request %u is not supported yet",
@@ -12123,40 +12121,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.38.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] net/i40e: don't check link status on device start
  2022-12-13  9:18       ` [PATCH v2] net/i40e: don't check link status on device start David Marchand
@ 2023-01-03 14:02         ` David Marchand
  2023-01-09  9:21           ` David Marchand
  2023-03-06  6:53         ` Su, Simei
  1 sibling, 1 reply; 20+ messages in thread
From: David Marchand @ 2023-01-03 14:02 UTC (permalink / raw)
  To: Yuying Zhang, Beilei Xing
  Cc: dev, stable, Qi Zhang, Dapeng Yu, Wenxuan Wu, Jie Wang

Hi i40e maintainers,

On Tue, Dec 13, 2022 at 10:19 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> The mentioned changes broke existing applications when the link status
> of i40e ports is down at the time the port is started.
> Revert those changes, the original issue will need a different fix.
>
> 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: David Marchand <david.marchand@redhat.com>

This issue was reported only by our QE, but I suspect that real
deployments will get affected.
Please review.

Thanks.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] net/i40e: don't check link status on device start
  2023-01-03 14:02         ` David Marchand
@ 2023-01-09  9:21           ` David Marchand
  2023-01-13 13:33             ` David Marchand
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2023-01-09  9:21 UTC (permalink / raw)
  To: Yuying Zhang, Beilei Xing
  Cc: dev, stable, Qi Zhang, Dapeng Yu, Wenxuan Wu, Jie Wang, Mcnamara, John

On Tue, Jan 3, 2023 at 3:02 PM David Marchand <david.marchand@redhat.com> wrote:
>
> Hi i40e maintainers,
>
> On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > The mentioned changes broke existing applications when the link status
> > of i40e ports is down at the time the port is started.
> > Revert those changes, the original issue will need a different fix.
> >
> > 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: David Marchand <david.marchand@redhat.com>
>
> This issue was reported only by our QE, but I suspect that real
> deployments will get affected.
> Please review.

Ping.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] net/i40e: don't check link status on device start
  2023-01-09  9:21           ` David Marchand
@ 2023-01-13 13:33             ` David Marchand
  2023-01-13 13:39               ` Zhang, Helin
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2023-01-13 13:33 UTC (permalink / raw)
  To: Yuying Zhang, Beilei Xing, Mcnamara, John
  Cc: dev, stable, Qi Zhang, Dapeng Yu, Wenxuan Wu

Hello i40e maintainers, John,

On Mon, Jan 9, 2023 at 10:21 AM David Marchand
<david.marchand@redhat.com> wrote:
> On Tue, Jan 3, 2023 at 3:02 PM David Marchand <david.marchand@redhat.com> wrote:
> > Hi i40e maintainers,
> >
> > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > The mentioned changes broke existing applications when the link status
> > > of i40e ports is down at the time the port is started.
> > > Revert those changes, the original issue will need a different fix.
> > >
> > > 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: David Marchand <david.marchand@redhat.com>
> >
> > This issue was reported only by our QE, but I suspect that real
> > deployments will get affected.
> > Please review.
>
> Ping.

Ping.

Note: I removed Jie Wang from Cc list since I get bounces.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v2] net/i40e: don't check link status on device start
  2023-01-13 13:33             ` David Marchand
@ 2023-01-13 13:39               ` Zhang, Helin
  2023-01-13 13:46                 ` David Marchand
  0 siblings, 1 reply; 20+ messages in thread
From: Zhang, Helin @ 2023-01-13 13:39 UTC (permalink / raw)
  To: David Marchand, Zhang, Yuying, Xing, Beilei, Mcnamara, John
  Cc: dev, stable, Zhang, Qi Z, Dapeng Yu, Wenxuan Wu



> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, January 13, 2023 9:33 PM
> To: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Dapeng Yu <dapengx.yu@intel.com>; Wenxuan Wu
> <wenxuanx.wu@intel.com>
> Subject: Re: [PATCH v2] net/i40e: don't check link status on device start
> 
> Hello i40e maintainers, John,
> 
> On Mon, Jan 9, 2023 at 10:21 AM David Marchand
> <david.marchand@redhat.com> wrote:
> > On Tue, Jan 3, 2023 at 3:02 PM David Marchand
> <david.marchand@redhat.com> wrote:
> > > Hi i40e maintainers,
> > >
> > > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > > <david.marchand@redhat.com> wrote:
> > > >
> > > > The mentioned changes broke existing applications when the link
> > > > status of i40e ports is down at the time the port is started.
> > > > Revert those changes, the original issue will need a different fix.
Hi David

Does it break all the application or just a specific application?
We may need to understand the issue you met, and try to fix it later.
Thank you very much for reporting it out!

Regards,
Helin

> > > >
> > > > 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: David Marchand <david.marchand@redhat.com>
> > >
> > > This issue was reported only by our QE, but I suspect that real
> > > deployments will get affected.
> > > Please review.
> >
> > Ping.
> 
> Ping.
> 
> Note: I removed Jie Wang from Cc list since I get bounces.
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] net/i40e: don't check link status on device start
  2023-01-13 13:39               ` Zhang, Helin
@ 2023-01-13 13:46                 ` David Marchand
  2023-01-13 13:50                   ` Zhang, Helin
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2023-01-13 13:46 UTC (permalink / raw)
  To: Zhang, Helin
  Cc: Zhang, Yuying, Xing, Beilei, Mcnamara, John, dev, stable, Zhang,
	Qi Z, Dapeng Yu, Wenxuan Wu

On Fri, Jan 13, 2023 at 2:39 PM Zhang, Helin <helin.zhang@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Friday, January 13, 2023 9:33 PM
> > To: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Mcnamara, John <john.mcnamara@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > Dapeng Yu <dapengx.yu@intel.com>; Wenxuan Wu
> > <wenxuanx.wu@intel.com>
> > Subject: Re: [PATCH v2] net/i40e: don't check link status on device start
> >
> > Hello i40e maintainers, John,
> >
> > On Mon, Jan 9, 2023 at 10:21 AM David Marchand
> > <david.marchand@redhat.com> wrote:
> > > On Tue, Jan 3, 2023 at 3:02 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > > > Hi i40e maintainers,
> > > >
> > > > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > > > <david.marchand@redhat.com> wrote:
> > > > >
> > > > > The mentioned changes broke existing applications when the link
> > > > > status of i40e ports is down at the time the port is started.
> > > > > Revert those changes, the original issue will need a different fix.
> Hi David
>
> Does it break all the application or just a specific application?

I don't see how it would not affect all applications seeing how the
original patch is dumb.

> We may need to understand the issue you met, and try to fix it later.

Just unplug the cable or fake a link down on your i40e port, start
your application or port, then plug the cable back.
The max frame size will never get applied to hw.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v2] net/i40e: don't check link status on device start
  2023-01-13 13:46                 ` David Marchand
@ 2023-01-13 13:50                   ` Zhang, Helin
  2023-01-13 13:53                     ` David Marchand
  0 siblings, 1 reply; 20+ messages in thread
From: Zhang, Helin @ 2023-01-13 13:50 UTC (permalink / raw)
  To: David Marchand
  Cc: Zhang, Yuying, Xing, Beilei, Mcnamara, John, dev, stable, Zhang,
	Qi Z, Dapeng Yu, Wenxuan Wu



> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, January 13, 2023 9:47 PM
> To: Zhang, Helin <helin.zhang@intel.com>
> Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Dapeng Yu <dapengx.yu@intel.com>; Wenxuan Wu
> <wenxuanx.wu@intel.com>
> Subject: Re: [PATCH v2] net/i40e: don't check link status on device start
> 
> On Fri, Jan 13, 2023 at 2:39 PM Zhang, Helin <helin.zhang@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: David Marchand <david.marchand@redhat.com>
> > > Sent: Friday, January 13, 2023 9:33 PM
> > > To: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>; Mcnamara, John <john.mcnamara@intel.com>
> > > Cc: dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Dapeng Yu <dapengx.yu@intel.com>; Wenxuan
> Wu
> > > <wenxuanx.wu@intel.com>
> > > Subject: Re: [PATCH v2] net/i40e: don't check link status on device
> > > start
> > >
> > > Hello i40e maintainers, John,
> > >
> > > On Mon, Jan 9, 2023 at 10:21 AM David Marchand
> > > <david.marchand@redhat.com> wrote:
> > > > On Tue, Jan 3, 2023 at 3:02 PM David Marchand
> > > <david.marchand@redhat.com> wrote:
> > > > > Hi i40e maintainers,
> > > > >
> > > > > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > > > > <david.marchand@redhat.com> wrote:
> > > > > >
> > > > > > The mentioned changes broke existing applications when the
> > > > > > link status of i40e ports is down at the time the port is started.
> > > > > > Revert those changes, the original issue will need a different fix.
> > Hi David
> >
> > Does it break all the application or just a specific application?
> 
> I don't see how it would not affect all applications seeing how the original
> patch is dumb.
> 
> > We may need to understand the issue you met, and try to fix it later.
> 
> Just unplug the cable or fake a link down on your i40e port, start your
> application or port, then plug the cable back.
> The max frame size will never get applied to hw.
Got it, I will forward to a right expert to check. Thank you very much for reaching out to us!

Regards,
Helin
> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] net/i40e: don't check link status on device start
  2023-01-13 13:50                   ` Zhang, Helin
@ 2023-01-13 13:53                     ` David Marchand
  2023-01-16 11:02                       ` Su, Simei
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2023-01-13 13:53 UTC (permalink / raw)
  To: Zhang, Helin
  Cc: Zhang, Yuying, Xing, Beilei, Mcnamara, John, dev, stable, Zhang,
	Qi Z, Dapeng Yu, Wenxuan Wu

On Fri, Jan 13, 2023 at 2:51 PM Zhang, Helin <helin.zhang@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Friday, January 13, 2023 9:47 PM
> > To: Zhang, Helin <helin.zhang@intel.com>
> > Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > Dapeng Yu <dapengx.yu@intel.com>; Wenxuan Wu
> > <wenxuanx.wu@intel.com>
> > Subject: Re: [PATCH v2] net/i40e: don't check link status on device start
> >
> > On Fri, Jan 13, 2023 at 2:39 PM Zhang, Helin <helin.zhang@intel.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: David Marchand <david.marchand@redhat.com>
> > > > Sent: Friday, January 13, 2023 9:33 PM
> > > > To: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > > > <beilei.xing@intel.com>; Mcnamara, John <john.mcnamara@intel.com>
> > > > Cc: dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>; Dapeng Yu <dapengx.yu@intel.com>; Wenxuan
> > Wu
> > > > <wenxuanx.wu@intel.com>
> > > > Subject: Re: [PATCH v2] net/i40e: don't check link status on device
> > > > start
> > > >
> > > > Hello i40e maintainers, John,
> > > >
> > > > On Mon, Jan 9, 2023 at 10:21 AM David Marchand
> > > > <david.marchand@redhat.com> wrote:
> > > > > On Tue, Jan 3, 2023 at 3:02 PM David Marchand
> > > > <david.marchand@redhat.com> wrote:
> > > > > > Hi i40e maintainers,
> > > > > >
> > > > > > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > > > > > <david.marchand@redhat.com> wrote:
> > > > > > >
> > > > > > > The mentioned changes broke existing applications when the
> > > > > > > link status of i40e ports is down at the time the port is started.
> > > > > > > Revert those changes, the original issue will need a different fix.
> > > Hi David
> > >
> > > Does it break all the application or just a specific application?
> >
> > I don't see how it would not affect all applications seeing how the original
> > patch is dumb.
> >
> > > We may need to understand the issue you met, and try to fix it later.
> >
> > Just unplug the cable or fake a link down on your i40e port, start your
> > application or port, then plug the cable back.
> > The max frame size will never get applied to hw.
> Got it, I will forward to a right expert to check. Thank you very much for reaching out to us!

I hope I get a reply _soon_.
Or I will just apply those reverts.


Thanks.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v2] net/i40e: don't check link status on device start
  2023-01-13 13:53                     ` David Marchand
@ 2023-01-16 11:02                       ` Su, Simei
  2023-02-07 11:31                         ` Thomas Monjalon
  0 siblings, 1 reply; 20+ messages in thread
From: Su, Simei @ 2023-01-16 11:02 UTC (permalink / raw)
  To: David Marchand, Zhang, Helin
  Cc: Zhang, Yuying, Xing, Beilei, Mcnamara, John, dev, stable, Zhang,
	Qi Z, Dapeng Yu, Wenxuan Wu

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, January 13, 2023 9:53 PM
> To: Zhang, Helin <helin.zhang@intel.com>
> Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Dapeng
> Yu <dapengx.yu@intel.com>; Wenxuan Wu <wenxuanx.wu@intel.com>
> Subject: Re: [PATCH v2] net/i40e: don't check link status on device start
> 
> On Fri, Jan 13, 2023 at 2:51 PM Zhang, Helin <helin.zhang@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: David Marchand <david.marchand@redhat.com>
> > > Sent: Friday, January 13, 2023 9:47 PM
> > > To: Zhang, Helin <helin.zhang@intel.com>
> > > Cc: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > > dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > > Dapeng Yu <dapengx.yu@intel.com>; Wenxuan Wu
> <wenxuanx.wu@intel.com>
> > > Subject: Re: [PATCH v2] net/i40e: don't check link status on device
> > > start
> > >
> > > On Fri, Jan 13, 2023 at 2:39 PM Zhang, Helin <helin.zhang@intel.com>
> wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > Sent: Friday, January 13, 2023 9:33 PM
> > > > > To: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> > > > > <beilei.xing@intel.com>; Mcnamara, John
> > > > > <john.mcnamara@intel.com>
> > > > > Cc: dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z
> > > > > <qi.z.zhang@intel.com>; Dapeng Yu <dapengx.yu@intel.com>;
> > > > > Wenxuan
> > > Wu
> > > > > <wenxuanx.wu@intel.com>
> > > > > Subject: Re: [PATCH v2] net/i40e: don't check link status on
> > > > > device start
> > > > >
> > > > > Hello i40e maintainers, John,
> > > > >
> > > > > On Mon, Jan 9, 2023 at 10:21 AM David Marchand
> > > > > <david.marchand@redhat.com> wrote:
> > > > > > On Tue, Jan 3, 2023 at 3:02 PM David Marchand
> > > > > <david.marchand@redhat.com> wrote:
> > > > > > > Hi i40e maintainers,
> > > > > > >
> > > > > > > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > > > > > > <david.marchand@redhat.com> wrote:
> > > > > > > >
> > > > > > > > The mentioned changes broke existing applications when the
> > > > > > > > link status of i40e ports is down at the time the port is started.
> > > > > > > > Revert those changes, the original issue will need a different fix.
> > > > Hi David
> > > >
> > > > Does it break all the application or just a specific application?
> > >
> > > I don't see how it would not affect all applications seeing how the
> > > original patch is dumb.
> > >
> > > > We may need to understand the issue you met, and try to fix it later.
> > >
> > > Just unplug the cable or fake a link down on your i40e port, start
> > > your application or port, then plug the cable back.
> > > The max frame size will never get applied to hw.
> > Got it, I will forward to a right expert to check. Thank you very much for
> reaching out to us!
> 
> I hope I get a reply _soon_.
> Or I will just apply those reverts.

If applying those reverts, some issues still exist on our side. I sent one patch to patchwork:
https://patchwork.dpdk.org/project/dpdk/patch/20230116105318.19412-1-simei.su@intel.com/.
You can try this patch to see whether it can solve the issue on your side.
At the same time, on our side, we need to do regression test further to check if this patch will affect other
cases, the regression takes some time.

Thanks,
Simei

> 
> 
> Thanks.
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] net/i40e: don't check link status on device start
  2023-01-16 11:02                       ` Su, Simei
@ 2023-02-07 11:31                         ` Thomas Monjalon
  2023-02-07 14:05                           ` Su, Simei
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Monjalon @ 2023-02-07 11:31 UTC (permalink / raw)
  To: Zhang, Helin, Su, Simei
  Cc: David Marchand, dev, Zhang, Yuying, Xing, Beilei, Mcnamara, John,
	stable, Zhang, Qi Z, Dapeng Yu, Wenxuan Wu

16/01/2023 12:02, Su, Simei:
> From: David Marchand <david.marchand@redhat.com>
> > On Fri, Jan 13, 2023 at 2:51 PM Zhang, Helin <helin.zhang@intel.com> wrote:
> > > From: David Marchand <david.marchand@redhat.com>
> > > > On Fri, Jan 13, 2023 at 2:39 PM Zhang, Helin <helin.zhang@intel.com>
> > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > > Hello i40e maintainers, John,
> > > > > >
> > > > > > On Mon, Jan 9, 2023 at 10:21 AM David Marchand
> > > > > > <david.marchand@redhat.com> wrote:
> > > > > > > On Tue, Jan 3, 2023 at 3:02 PM David Marchand
> > > > > > <david.marchand@redhat.com> wrote:
> > > > > > > > Hi i40e maintainers,
> > > > > > > >
> > > > > > > > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > > > > > > > <david.marchand@redhat.com> wrote:
> > > > > > > > >
> > > > > > > > > The mentioned changes broke existing applications when the
> > > > > > > > > link status of i40e ports is down at the time the port is started.
> > > > > > > > > Revert those changes, the original issue will need a different fix.
> > > > > Hi David
> > > > >
> > > > > Does it break all the application or just a specific application?
> > > >
> > > > I don't see how it would not affect all applications seeing how the
> > > > original patch is dumb.
> > > >
> > > > > We may need to understand the issue you met, and try to fix it later.
> > > >
> > > > Just unplug the cable or fake a link down on your i40e port, start
> > > > your application or port, then plug the cable back.
> > > > The max frame size will never get applied to hw.
> > > 
> > > Got it, I will forward to a right expert to check. Thank you very much for
> > reaching out to us!
> > 
> > I hope I get a reply _soon_.
> > Or I will just apply those reverts.
> 
> If applying those reverts, some issues still exist on our side. I sent one patch to patchwork:
> https://patchwork.dpdk.org/project/dpdk/patch/20230116105318.19412-1-simei.su@intel.com/.
> You can try this patch to see whether it can solve the issue on your side.
> At the same time, on our side, we need to do regression test further to check if this patch will affect other
> cases, the regression takes some time.
> 
> Thanks,
> Simei

Simei, do you have any update in this thread?
Last reply was 3 weeks ago. I hope regression test don't take so much time.




^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v2] net/i40e: don't check link status on device start
  2023-02-07 11:31                         ` Thomas Monjalon
@ 2023-02-07 14:05                           ` Su, Simei
  0 siblings, 0 replies; 20+ messages in thread
From: Su, Simei @ 2023-02-07 14:05 UTC (permalink / raw)
  To: Thomas Monjalon, Zhang, Helin
  Cc: David Marchand, dev, Zhang, Yuying, Xing, Beilei, Mcnamara, John,
	stable, Zhang, Qi Z, Dapeng Yu, Wenxuan Wu

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, February 7, 2023 7:32 PM
> To: Zhang, Helin <helin.zhang@intel.com>; Su, Simei <simei.su@intel.com>
> Cc: David Marchand <david.marchand@redhat.com>; dev@dpdk.org; Zhang,
> Yuying <yuying.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Mcnamara, John <john.mcnamara@intel.com>; stable@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Dapeng Yu <dapengx.yu@intel.com>; Wenxuan Wu
> <wenxuanx.wu@intel.com>
> Subject: Re: [PATCH v2] net/i40e: don't check link status on device start
> 
> 16/01/2023 12:02, Su, Simei:
> > From: David Marchand <david.marchand@redhat.com>
> > > On Fri, Jan 13, 2023 at 2:51 PM Zhang, Helin <helin.zhang@intel.com>
> wrote:
> > > > From: David Marchand <david.marchand@redhat.com>
> > > > > On Fri, Jan 13, 2023 at 2:39 PM Zhang, Helin
> > > > > <helin.zhang@intel.com>
> > > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > > > Hello i40e maintainers, John,
> > > > > > >
> > > > > > > On Mon, Jan 9, 2023 at 10:21 AM David Marchand
> > > > > > > <david.marchand@redhat.com> wrote:
> > > > > > > > On Tue, Jan 3, 2023 at 3:02 PM David Marchand
> > > > > > > <david.marchand@redhat.com> wrote:
> > > > > > > > > Hi i40e maintainers,
> > > > > > > > >
> > > > > > > > > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > > > > > > > > <david.marchand@redhat.com> wrote:
> > > > > > > > > >
> > > > > > > > > > The mentioned changes broke existing applications when
> > > > > > > > > > the link status of i40e ports is down at the time the port is
> started.
> > > > > > > > > > Revert those changes, the original issue will need a different
> fix.
> > > > > > Hi David
> > > > > >
> > > > > > Does it break all the application or just a specific application?
> > > > >
> > > > > I don't see how it would not affect all applications seeing how
> > > > > the original patch is dumb.
> > > > >
> > > > > > We may need to understand the issue you met, and try to fix it later.
> > > > >
> > > > > Just unplug the cable or fake a link down on your i40e port,
> > > > > start your application or port, then plug the cable back.
> > > > > The max frame size will never get applied to hw.
> > > >
> > > > Got it, I will forward to a right expert to check. Thank you very
> > > > much for
> > > reaching out to us!
> > >
> > > I hope I get a reply _soon_.
> > > Or I will just apply those reverts.
> >
> > If applying those reverts, some issues still exist on our side. I sent one patch
> to patchwork:
> >
> https://patchwork.dpdk.org/project/dpdk/patch/20230116105318.19412-1-si
> mei.su@intel.com/.
> > You can try this patch to see whether it can solve the issue on your side.
> > At the same time, on our side, we need to do regression test further
> > to check if this patch will affect other cases, the regression takes some time.
> >
> > Thanks,
> > Simei
> 
> Simei, do you have any update in this thread?
> Last reply was 3 weeks ago. I hope regression test don't take so much time.
> 
> 
The latest patch for this issue is https://patchwork.dpdk.org/project/dpdk/patch/20230202123632.56730-1-simei.su@intel.com/ which needs to be reworked further.

Thanks,
Simei


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v2] net/i40e: don't check link status on device start
  2022-12-13  9:18       ` [PATCH v2] net/i40e: don't check link status on device start David Marchand
  2023-01-03 14:02         ` David Marchand
@ 2023-03-06  6:53         ` Su, Simei
  2023-03-06 11:05           ` Zhang, Qi Z
  1 sibling, 1 reply; 20+ messages in thread
From: Su, Simei @ 2023-03-06  6:53 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable, Zhang, Yuying, Xing, Beilei, Zhang, Qi Z



> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, December 13, 2022 5:19 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Dapeng Yu
> <dapengx.yu@intel.com>; Wenxuan Wu <wenxuanx.wu@intel.com>; Wang,
> Jie1X <jie1x.wang@intel.com>
> Subject: [PATCH v2] net/i40e: don't check link status on device start
> 
> The mentioned changes broke existing applications when the link status of
> i40e ports is down at the time the port is started.
> Revert those changes, the original issue will need a different fix.
> 
> 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: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v1:
> - since the CI reports a failure on v1, simplified the fix by only
>   reverting commits,
> 
> ---
>  drivers/net/i40e/i40e_ethdev.c | 50 +++++-----------------------------
>  1 file changed, 7 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7726a89d99..a982e42264 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, @@ -1711,6 +1710,11 @@
> 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);
> 
> @@ -2328,7 +2332,6 @@ 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;
> 
> @@ -2467,9 +2470,6 @@ 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);
> -
>  	return I40E_SUCCESS;
> 
>  tx_err:
> @@ -2809,9 +2809,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 +2875,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;
> @@ -6738,7 +6737,6 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
>  			if (!ret)
>  				rte_eth_dev_callback_process(dev,
>  					RTE_ETH_EVENT_INTR_LSC, NULL);
> -
>  			break;
>  		default:
>  			PMD_DRV_LOG(DEBUG, "Request %u is not supported yet", @@
> -12123,40 +12121,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.38.1

Acked-by: Simei Su <simei.su@intel.com>

Thanks,
Simei 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v2] net/i40e: don't check link status on device start
  2023-03-06  6:53         ` Su, Simei
@ 2023-03-06 11:05           ` Zhang, Qi Z
  0 siblings, 0 replies; 20+ messages in thread
From: Zhang, Qi Z @ 2023-03-06 11:05 UTC (permalink / raw)
  To: Su, Simei, David Marchand, dev; +Cc: stable, Zhang, Yuying, Xing, Beilei



> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Monday, March 6, 2023 2:54 PM
> To: David Marchand <david.marchand@redhat.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: RE: [PATCH v2] net/i40e: don't check link status on device start
> 
> 
> 
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Tuesday, December 13, 2022 5:19 PM
> > To: dev@dpdk.org
> > Cc: stable@dpdk.org; Zhang, Yuying <yuying.zhang@intel.com>; Xing,
> > Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > Dapeng Yu <dapengx.yu@intel.com>; Wenxuan Wu
> <wenxuanx.wu@intel.com>;
> > Wang, Jie1X <jie1x.wang@intel.com>
> > Subject: [PATCH v2] net/i40e: don't check link status on device start
> >
> > The mentioned changes broke existing applications when the link status
> > of i40e ports is down at the time the port is started.
> > Revert those changes, the original issue will need a different fix.
> >
> > 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: David Marchand <david.marchand@redhat.com>
> > ---
> > Changes since v1:
> > - since the CI reports a failure on v1, simplified the fix by only
> >   reverting commits,
> >
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 50
> > +++++-----------------------------
> >  1 file changed, 7 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 7726a89d99..a982e42264
> 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, @@ -1711,6 +1710,11
> @@
> > 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);
> >
> > @@ -2328,7 +2332,6 @@ 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;
> >
> > @@ -2467,9 +2470,6 @@ 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);
> > -
> >  	return I40E_SUCCESS;
> >
> >  tx_err:
> > @@ -2809,9 +2809,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 +2875,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;
> > @@ -6738,7 +6737,6 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev
> *dev)
> >  			if (!ret)
> >  				rte_eth_dev_callback_process(dev,
> >  					RTE_ETH_EVENT_INTR_LSC, NULL);
> > -
> >  			break;
> >  		default:
> >  			PMD_DRV_LOG(DEBUG, "Request %u is not
> supported yet", @@
> > -12123,40 +12121,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.38.1
> 
> Acked-by: Simei Su <simei.su@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
> 
> Thanks,
> Simei


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-03-06 11:05 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-07  8:59 [PATCH] net/i40e: enable max frame size at port level dapengx.yu
     [not found] ` <BYAPR11MB2711AB91F380189FB6AFCC52FE359@BYAPR11MB2711.namprd11.prod.outlook.com>
     [not found]   ` <BYAPR11MB27110434B9262C0E2BED3AD3FE3A9@BYAPR11MB2711.namprd11.prod.outlook.com>
2022-02-21 10:42     ` Zhang, Peng1X
2022-02-21 11:22       ` Zhang, Qi Z
2022-02-22  9:52         ` Kevin Traynor
2022-12-12 13:53 ` David Marchand
2022-12-12 13:58   ` David Marchand
2022-12-12 14:37     ` [PATCH] net/i40e: drop link check when configuring frame size David Marchand
2022-12-13  9:18       ` [PATCH v2] net/i40e: don't check link status on device start David Marchand
2023-01-03 14:02         ` David Marchand
2023-01-09  9:21           ` David Marchand
2023-01-13 13:33             ` David Marchand
2023-01-13 13:39               ` Zhang, Helin
2023-01-13 13:46                 ` David Marchand
2023-01-13 13:50                   ` Zhang, Helin
2023-01-13 13:53                     ` David Marchand
2023-01-16 11:02                       ` Su, Simei
2023-02-07 11:31                         ` Thomas Monjalon
2023-02-07 14:05                           ` Su, Simei
2023-03-06  6:53         ` Su, Simei
2023-03-06 11:05           ` Zhang, Qi Z

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).