* [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
[parent not found: <BYAPR11MB2711AB91F380189FB6AFCC52FE359@BYAPR11MB2711.namprd11.prod.outlook.com>]
[parent not found: <BYAPR11MB27110434B9262C0E2BED3AD3FE3A9@BYAPR11MB2711.namprd11.prod.outlook.com>]
* 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).