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

[David Marchand <david.marchand@redhat.com>]

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

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

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

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


For the record, the differences with my v1 are:

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

        hw->adapter_stopped = 0;

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

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

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

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

        return I40E_SUCCESS;


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


Thanks.

-- 
David Marchand