Hi David,
> -----Original Message-----
> From: David Marchand <david.march...@redhat.com>
> Sent: Thursday, February 2, 2023 8:56 PM
> To: Su, Simei <simei...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
> Cc: Xing, Beilei <beilei.x...@intel.com>; Zhang, Yuying
> <yuying.zh...@intel.com>; dev@dpdk.org; Yang, Qiming
> <qiming.y...@intel.com>; sta...@dpdk.org
> Subject: Re: [PATCH v5] net/i40e: rework maximum frame size configuration
>
> On Thu, Feb 2, 2023 at 1:37 PM Simei Su <simei...@intel.com> wrote:
> >
> > This patch reverts mentioned changes below to remove unnecessary link
> > status check and only moves max frame size configuration to dev_start.
> > Also, it sets the parameter "wait to complete" true to wait for
> > complete right after setting link up.
>
> Why is the change on link status needed?
> Is it necessary?
Indeed, it doesn't change the link status, it only involves update, waiting for
it to complete.
Sorry not to describe correctly about setting the " wait to complete" true.
>
> >
> > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > level")
> > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > level")
> > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Simei Su <simei...@intel.com>
>
> I would have preferred you reply to my original report.
> At least, I'd like you add some credit with my name in the commitlog.
Sorry, I will notice it in next version.
Thanks,
Simei
>
>
> For the record, the differences with my v1 are:
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5635dd03cf..5d57bb9a0e 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2327,6 +2327,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
> uint32_t intr_vector = 0;
> struct i40e_vsi *vsi;
> uint16_t nb_rxq, nb_txq;
> + uint16_t max_frame_size;
>
> hw->adapter_stopped = 0;
>
> @@ -2447,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
> PMD_DRV_LOG(WARNING, "Fail to set phy
> mask");
>
> /* Call get_link_info aq command to enable/disable LSE
> */
> - i40e_dev_link_update(dev, 0);
> + i40e_dev_link_update(dev, 1);
> }
>
> if (dev->data->dev_conf.intr_conf.rxq == 0) { @@ -2465,8 +2466,16
> @@ i40e_dev_start(struct rte_eth_dev *dev)
> "please call hierarchy_commit() "
> "before starting the port");
>
> - i40e_aq_set_mac_config(hw, dev->data->mtu +
> I40E_ETH_OVERHEAD, TRUE,
> - false, 0, NULL);
> + max_frame_size = dev->data->mtu ?
> + dev->data->mtu + I40E_ETH_OVERHEAD :
> + I40E_FRAME_SIZE_MAX;
> +
> + /* Set the max frame size to HW*/
> + ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0,
> NULL);
> + if (ret) {
> + PMD_DRV_LOG(ERR, "Fail to set mac config");
> + return ret;
> + }
>
> return I40E_SUCCESS;
>
>
> Qi, don't apply this fix yet.
> I'll generate some binaries internally to have Red Hat QE run their tests.
>
>
> Thanks.
>
> --
> David Marchand
