[v6] net/i40e: rework maximum frame size configuration
Checks
Commit Message
One issue is reported by David Marchand that error occurs in OVS due to
the fix patch in mentioned changes below. The detailed reproduce step
and result are in https://patchwork.dpdk.org/project/dpdk/patch/
20211207085946.121032-1-dapengx.yu@intel.com/.
This patch removes unnecessary link status check and directly sets mac
config in dev_start. Also, it sets the parameter "wait to complete" true
to wait for more time to make sure adminq command execute completed.
Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: stable@dpdk.org
Reported-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Simei Su <simei.su@intel.com>
---
v6:
* Refine commit log.
* Remove return error.
v5:
* Fix misspelling in commit log.
v4:
* Refine commit log.
* Avoid duplicate call to set parameter "wait to complete" true.
v3:
* Put link update before interrupt enable.
v2:
* Refine commit log.
* Add link update.
drivers/net/i40e/i40e_ethdev.c | 50 ++++++++----------------------------------
1 file changed, 9 insertions(+), 41 deletions(-)
Comments
> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Monday, February 20, 2023 4:00 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> david.marchand@redhat.com
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
> <simei.su@intel.com>; stable@dpdk.org
> Subject: [PATCH v6] net/i40e: rework maximum frame size configuration
>
> One issue is reported by David Marchand that error occurs in OVS due to the
> fix patch in mentioned changes below. The detailed reproduce step and
> result are in https://patchwork.dpdk.org/project/dpdk/patch/
> 20211207085946.121032-1-dapengx.yu@intel.com/.
>
> This patch removes unnecessary link status check and directly sets mac config
> in dev_start. Also, it sets the parameter "wait to complete" true to wait for
> more time to make sure adminq command execute completed.
>
> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> Cc: stable@dpdk.org
>
> Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Simei Su <simei.su@intel.com>
Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Applied to dpdk-next-net-intel.
Thanks
Qi
Qi,
On Mon, Feb 27, 2023 at 1:35 AM Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> > One issue is reported by David Marchand that error occurs in OVS due to the
> > fix patch in mentioned changes below. The detailed reproduce step and
> > result are in https://patchwork.dpdk.org/project/dpdk/patch/
> > 20211207085946.121032-1-dapengx.yu@intel.com/.
> >
> > This patch removes unnecessary link status check and directly sets mac config
> > in dev_start. Also, it sets the parameter "wait to complete" true to wait for
> > more time to make sure adminq command execute completed.
> >
> > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > Cc: stable@dpdk.org
> >
> > Reported-by: David Marchand <david.marchand@redhat.com>
> > Signed-off-by: Simei Su <simei.su@intel.com>
>
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
I was waiting for a ping... good thing I had a look at this thread.
I suggest splitting this in two parts before it reaches the main repo:
- put the reverts first (the reason is that 21.11 stable branch
already have them): this would end up the same as merging
https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-david.marchand@redhat.com/
that has been waiting since December,
- have the move of i40e_aq_set_mac_config from eth_i40e_dev_init to
i40e_dev_start + change in i40e_dev_link_update call in a second patch
(i.e. the following diff),
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index a982e42264..371f42233e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1710,11 +1710,6 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void
*init_params __rte_unused)
*/
i40e_add_tx_flow_control_drop_filter(pf);
- /* Set the max frame size to 0x2600 by default,
- * in case other drivers changed the default value.
- */
- i40e_aq_set_mac_config(hw, I40E_FRAME_SIZE_MAX, TRUE, false, 0, NULL);
-
/* initialize RSS rule list */
TAILQ_INIT(&pf->rss_config_list);
@@ -2332,6 +2327,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
uint32_t intr_vector = 0;
struct i40e_vsi *vsi;
uint16_t nb_rxq, nb_txq;
+ uint16_t max_frame_size;
hw->adapter_stopped = 0;
@@ -2452,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
PMD_DRV_LOG(WARNING, "Fail to set phy mask");
/* Call get_link_info aq command to enable/disable LSE */
- i40e_dev_link_update(dev, 0);
+ i40e_dev_link_update(dev, 1);
}
if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2470,6 +2466,13 @@ i40e_dev_start(struct rte_eth_dev *dev)
"please call hierarchy_commit() "
"before starting the port");
+ max_frame_size = dev->data->mtu ?
+ dev->data->mtu + I40E_ETH_OVERHEAD :
+ I40E_FRAME_SIZE_MAX;
+
+ /* Set the max frame size to HW*/
+ i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+
return I40E_SUCCESS;
tx_err:
This way, the new fix is easier to read too.
Thanks.
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, February 28, 2023 5:37 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Su, Simei <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Zhang, Yuying <yuying.zhang@intel.com>; dev@dpdk.org; Yang, Qiming
> <qiming.yang@intel.com>; stable@dpdk.org; Kevin Traynor
> <ktraynor@redhat.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
>
> Qi,
>
> On Mon, Feb 27, 2023 at 1:35 AM Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> > > One issue is reported by David Marchand that error occurs in OVS due
> > > to the fix patch in mentioned changes below. The detailed reproduce
> > > step and result are in
> > > https://patchwork.dpdk.org/project/dpdk/patch/
> > > 20211207085946.121032-1-dapengx.yu@intel.com/.
> > >
> > > This patch removes unnecessary link status check and directly sets
> > > mac config in dev_start. Also, it sets the parameter "wait to
> > > complete" true to wait for more time to make sure adminq command
> execute completed.
> > >
> > > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > > level")
> > > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > > level")
> > > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > > Cc: stable@dpdk.org
> > >
> > > Reported-by: David Marchand <david.marchand@redhat.com>
> > > Signed-off-by: Simei Su <simei.su@intel.com>
> >
> > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>
> I was waiting for a ping... good thing I had a look at this thread.
>
> I suggest splitting this in two parts before it reaches the main repo:
> - put the reverts first (the reason is that 21.11 stable branch already have
> them): this would end up the same as merging
> https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-
> david.marchand@redhat.com/
> that has been waiting since December,
> - have the move of i40e_aq_set_mac_config from eth_i40e_dev_init to
> i40e_dev_start + change in i40e_dev_link_update call in a second patch (i.e.
> the following diff),
OK, reverted in dpdk-next-net-intel.
And waiting for Simei's v7.
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index a982e42264..371f42233e 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1710,11 +1710,6 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void
> *init_params __rte_unused)
> */
> i40e_add_tx_flow_control_drop_filter(pf);
>
> - /* Set the max frame size to 0x2600 by default,
> - * in case other drivers changed the default value.
> - */
> - i40e_aq_set_mac_config(hw, I40E_FRAME_SIZE_MAX, TRUE, false, 0,
> NULL);
> -
> /* initialize RSS rule list */
> TAILQ_INIT(&pf->rss_config_list);
>
> @@ -2332,6 +2327,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
> uint32_t intr_vector = 0;
> struct i40e_vsi *vsi;
> uint16_t nb_rxq, nb_txq;
> + uint16_t max_frame_size;
>
> hw->adapter_stopped = 0;
>
> @@ -2452,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
> PMD_DRV_LOG(WARNING, "Fail to set phy mask");
>
> /* Call get_link_info aq command to enable/disable LSE */
> - i40e_dev_link_update(dev, 0);
> + i40e_dev_link_update(dev, 1);
> }
>
> if (dev->data->dev_conf.intr_conf.rxq == 0) { @@ -2470,6 +2466,13 @@
> i40e_dev_start(struct rte_eth_dev *dev)
> "please call hierarchy_commit() "
> "before starting the port");
>
> + max_frame_size = dev->data->mtu ?
> + dev->data->mtu + I40E_ETH_OVERHEAD :
> + I40E_FRAME_SIZE_MAX;
> +
> + /* Set the max frame size to HW*/
> + i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0,
> + NULL);
> +
> return I40E_SUCCESS;
>
> tx_err:
>
>
> This way, the new fix is easier to read too.
> Thanks.
>
>
> --
> David Marchand
On 27/02/2023 00:35, Zhang, Qi Z wrote:
>
Hi Simei/Qi/Yu
>
>> -----Original Message-----
>> From: Su, Simei <simei.su@intel.com>
>> Sent: Monday, February 20, 2023 4:00 PM
>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>> david.marchand@redhat.com
>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
>> <simei.su@intel.com>; stable@dpdk.org
>> Subject: [PATCH v6] net/i40e: rework maximum frame size configuration
>>
>> One issue is reported by David Marchand that error occurs in OVS due to the
>> fix patch in mentioned changes below. The detailed reproduce step and
>> result are in https://patchwork.dpdk.org/project/dpdk/patch/
>> 20211207085946.121032-1-dapengx.yu@intel.com/.
>>
>> This patch removes unnecessary link status check and directly sets mac config
>> in dev_start. Also, it sets the parameter "wait to complete" true to wait for
>> more time to make sure adminq command execute completed.
>>
>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
>> Cc: stable@dpdk.org
These patches caused an observable regression in multiple 20.11 and
21.11 LTS releases that was only caught a long time after releases.
Is there anything being added to LTS validation for regression testing
this issue, so we don't get caught again?
After reverting the original patch and 2 fixes, I'm a bit reluctant to
take more fixes without some form of regression testing in place.
thanks,
Kevin.
>>
>> Reported-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Simei Su <simei.su@intel.com>
>
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>
> Applied to dpdk-next-net-intel.
>
> Thanks
> Qi
>
On 22/03/2023 16:50, Kevin Traynor wrote:
> On 27/02/2023 00:35, Zhang, Qi Z wrote:
>>
>
> Hi Simei/Qi/Yu
>
Hi Yu,
>>
>>> -----Original Message-----
>>> From: Su, Simei <simei.su@intel.com>
>>> Sent: Monday, February 20, 2023 4:00 PM
>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
>>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>>> david.marchand@redhat.com
>>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
>>> <simei.su@intel.com>; stable@dpdk.org
>>> Subject: [PATCH v6] net/i40e: rework maximum frame size configuration
>>>
>>> One issue is reported by David Marchand that error occurs in OVS due to the
>>> fix patch in mentioned changes below. The detailed reproduce step and
>>> result are in https://patchwork.dpdk.org/project/dpdk/patch/
>>> 20211207085946.121032-1-dapengx.yu@intel.com/.
>>>
>>> This patch removes unnecessary link status check and directly sets mac config
>>> in dev_start. Also, it sets the parameter "wait to complete" true to wait for
>>> more time to make sure adminq command execute completed.
>>>
>
>>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
>>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
>>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
>>> Cc: stable@dpdk.org
>
> These patches caused an observable regression in multiple 20.11 and
> 21.11 LTS releases that was only caught a long time after releases.
>
> Is there anything being added to LTS validation for regression testing
> this issue, so we don't get caught again?
>
This is the issue I was talking about earlier during the release
meeting. Not sure if we were talking about the same patch.
I was asking if there are some regression tests added/can be added to
LTS validation that will be run during each LTS validation cycle so we
don't have any more regressions on it.
thanks,
Kevin.
> After reverting the original patch and 2 fixes, I'm a bit reluctant to
> take more fixes without some form of regression testing in place.
>
> thanks,
> Kevin.
>
>>>
>>> Reported-by: David Marchand <david.marchand@redhat.com>
>>> Signed-off-by: Simei Su <simei.su@intel.com>
>>
>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>
>> Applied to dpdk-next-net-intel.
>>
>> Thanks
>> Qi
>>
>
> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Thursday, March 23, 2023 10:51 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Su, Simei <simei.su@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
> david.marchand@redhat.com; Jiang, YuX <yux.jiang@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org;
> Luca Boccassi <bluca@debian.org>; Mcnamara, John
> <john.mcnamara@intel.com>
> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
>
> On 22/03/2023 16:50, Kevin Traynor wrote:
> > On 27/02/2023 00:35, Zhang, Qi Z wrote:
> >>
> >
> > Hi Simei/Qi/Yu
> >
>
> Hi Yu,
>
> >>
> >>> -----Original Message-----
> >>> From: Su, Simei <simei.su@intel.com>
> >>> Sent: Monday, February 20, 2023 4:00 PM
> >>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> >>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >>> david.marchand@redhat.com
> >>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
> >>> <simei.su@intel.com>; stable@dpdk.org
> >>> Subject: [PATCH v6] net/i40e: rework maximum frame size
> >>> configuration
> >>>
> >>> One issue is reported by David Marchand that error occurs in OVS due
> >>> to the fix patch in mentioned changes below. The detailed reproduce
> >>> step and result are in
> >>> https://patchwork.dpdk.org/project/dpdk/patch/
> >>> 20211207085946.121032-1-dapengx.yu@intel.com/.
> >>>
> >>> This patch removes unnecessary link status check and directly sets
> >>> mac config in dev_start. Also, it sets the parameter "wait to
> >>> complete" true to wait for more time to make sure adminq command
> execute completed.
> >>>
> >
> >>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> >>> level")
> >>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> >>> level")
> >>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> >>> Cc: stable@dpdk.org
> >
> > These patches caused an observable regression in multiple 20.11 and
> > 21.11 LTS releases that was only caught a long time after releases.
> >
> > Is there anything being added to LTS validation for regression testing
> > this issue, so we don't get caught again?
> >
>
> This is the issue I was talking about earlier during the release meeting. Not sure
> if we were talking about the same patch.
>
> I was asking if there are some regression tests added/can be added to LTS
> validation that will be run during each LTS validation cycle so we don't have any
> more regressions on it.
>
Hi Kevin,
Thanks for your comments.
Yes. We are adding additional case to cover more testing. For main branch, we have done the regression testing (including the additional case testing), they both work well.
We hope the two related patches can be backported to LTS branch, and the second patch just reworks for previous bug's fix.
Patch1: https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-david.marchand@redhat.com/ a8ca8ed net/i40e: revert link status check on device start
Pathc2: https://patchwork.dpdk.org/project/dpdk/patch/20230306121853.27547-1-simei.su@intel.com/ 82fcf20 net/i40e: fix maximum frame size configuration
Best regards,
Yu Jiang
> thanks,
> Kevin.
>
> > After reverting the original patch and 2 fixes, I'm a bit reluctant to
> > take more fixes without some form of regression testing in place.
> >
> > thanks,
> > Kevin.
> >
> >>>
> >>> Reported-by: David Marchand <david.marchand@redhat.com>
> >>> Signed-off-by: Simei Su <simei.su@intel.com>
> >>
> >> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>
> >> Applied to dpdk-next-net-intel.
> >>
> >> Thanks
> >> Qi
> >>
> >
On 24/03/2023 06:32, Jiang, YuX wrote:
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Thursday, March 23, 2023 10:51 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Su, Simei <simei.su@intel.com>; Xing,
>> Beilei <beilei.xing@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
>> david.marchand@redhat.com; Jiang, YuX <yux.jiang@intel.com>
>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org;
>> Luca Boccassi <bluca@debian.org>; Mcnamara, John
>> <john.mcnamara@intel.com>
>> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
>>
>> On 22/03/2023 16:50, Kevin Traynor wrote:
>>> On 27/02/2023 00:35, Zhang, Qi Z wrote:
>>>>
>>>
>>> Hi Simei/Qi/Yu
>>>
>>
>> Hi Yu,
>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Su, Simei <simei.su@intel.com>
>>>>> Sent: Monday, February 20, 2023 4:00 PM
>>>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
>>>>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>>>>> david.marchand@redhat.com
>>>>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
>>>>> <simei.su@intel.com>; stable@dpdk.org
>>>>> Subject: [PATCH v6] net/i40e: rework maximum frame size
>>>>> configuration
>>>>>
>>>>> One issue is reported by David Marchand that error occurs in OVS due
>>>>> to the fix patch in mentioned changes below. The detailed reproduce
>>>>> step and result are in
>>>>> https://patchwork.dpdk.org/project/dpdk/patch/
>>>>> 20211207085946.121032-1-dapengx.yu@intel.com/.
>>>>>
>>>>> This patch removes unnecessary link status check and directly sets
>>>>> mac config in dev_start. Also, it sets the parameter "wait to
>>>>> complete" true to wait for more time to make sure adminq command
>> execute completed.
>>>>>
>>>
>>>>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
>>>>> level")
>>>>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
>>>>> level")
>>>>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
>>>>> Cc: stable@dpdk.org
>>>
>>> These patches caused an observable regression in multiple 20.11 and
>>> 21.11 LTS releases that was only caught a long time after releases.
>>>
>>> Is there anything being added to LTS validation for regression testing
>>> this issue, so we don't get caught again?
>>>
>>
>> This is the issue I was talking about earlier during the release meeting. Not sure
>> if we were talking about the same patch.
>>
>> I was asking if there are some regression tests added/can be added to LTS
>> validation that will be run during each LTS validation cycle so we don't have any
>> more regressions on it.
>>
> Hi Kevin,
> Thanks for your comments.
> Yes. We are adding additional case to cover more testing. For main branch, we have done the regression testing (including the additional case testing), they both work well.
That's good to hear. Will the additional regression tests also be added
to the LTS validation tests when they are run?
> We hope the two related patches can be backported to LTS branch, and the second patch just reworks for previous bug's fix.
> Patch1: https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-david.marchand@redhat.com/ a8ca8ed net/i40e: revert link status check on device start
I have already reverted those 3 backports on 21.11 branch so this is not
needed.
> Pathc2: https://patchwork.dpdk.org/project/dpdk/patch/20230306121853.27547-1-simei.su@intel.com/ 82fcf20 net/i40e: fix maximum frame size configuration
>
That is the v7 of this v6 with revert and fix split, so same one being
discussed above.
> Best regards,
> Yu Jiang
>
>> thanks,
>> Kevin.
>>
>>> After reverting the original patch and 2 fixes, I'm a bit reluctant to
>>> take more fixes without some form of regression testing in place.
>>>
>>> thanks,
>>> Kevin.
>>>
>>>>>
>>>>> Reported-by: David Marchand <david.marchand@redhat.com>
>>>>> Signed-off-by: Simei Su <simei.su@intel.com>
>>>>
>>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>>>
>>>> Applied to dpdk-next-net-intel.
>>>>
>>>> Thanks
>>>> Qi
>>>>
>>>
>
> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Friday, March 24, 2023 5:40 PM
> To: Jiang, YuX <yux.jiang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Su,
> Simei <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> Yuying <yuying.zhang@intel.com>; david.marchand@redhat.com
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org;
> Luca Boccassi <bluca@debian.org>; Mcnamara, John
> <john.mcnamara@intel.com>
> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
>
> On 24/03/2023 06:32, Jiang, YuX wrote:
> >> -----Original Message-----
> >> From: Kevin Traynor <ktraynor@redhat.com>
> >> Sent: Thursday, March 23, 2023 10:51 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Su, Simei
> >> <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> >> Yuying <yuying.zhang@intel.com>; david.marchand@redhat.com; Jiang,
> >> YuX <yux.jiang@intel.com>
> >> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> >> stable@dpdk.org; Luca Boccassi <bluca@debian.org>; Mcnamara, John
> >> <john.mcnamara@intel.com>
> >> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size
> >> configuration
> >>
> >> On 22/03/2023 16:50, Kevin Traynor wrote:
> >>> On 27/02/2023 00:35, Zhang, Qi Z wrote:
> >>>>
> >>>
> >>> Hi Simei/Qi/Yu
> >>>
> >>
> >> Hi Yu,
> >>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Su, Simei <simei.su@intel.com>
> >>>>> Sent: Monday, February 20, 2023 4:00 PM
> >>>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> >>>>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >>>>> david.marchand@redhat.com
> >>>>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
> >>>>> <simei.su@intel.com>; stable@dpdk.org
> >>>>> Subject: [PATCH v6] net/i40e: rework maximum frame size
> >>>>> configuration
> >>>>>
> >>>>> One issue is reported by David Marchand that error occurs in OVS
> >>>>> due to the fix patch in mentioned changes below. The detailed
> >>>>> reproduce step and result are in
> >>>>> https://patchwork.dpdk.org/project/dpdk/patch/
> >>>>> 20211207085946.121032-1-dapengx.yu@intel.com/.
> >>>>>
> >>>>> This patch removes unnecessary link status check and directly sets
> >>>>> mac config in dev_start. Also, it sets the parameter "wait to
> >>>>> complete" true to wait for more time to make sure adminq command
> >> execute completed.
> >>>>>
> >>>
> >>>>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> >>>>> level")
> >>>>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> >>>>> level")
> >>>>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> >>>>> Cc: stable@dpdk.org
> >>>
> >>> These patches caused an observable regression in multiple 20.11 and
> >>> 21.11 LTS releases that was only caught a long time after releases.
> >>>
> >>> Is there anything being added to LTS validation for regression
> >>> testing this issue, so we don't get caught again?
> >>>
> >>
> >> This is the issue I was talking about earlier during the release
> >> meeting. Not sure if we were talking about the same patch.
> >>
> >> I was asking if there are some regression tests added/can be added to
> >> LTS validation that will be run during each LTS validation cycle so
> >> we don't have any more regressions on it.
> >>
> > Hi Kevin,
> > Thanks for your comments.
> > Yes. We are adding additional case to cover more testing. For main branch,
> we have done the regression testing (including the additional case testing), they
> both work well.
>
> That's good to hear. Will the additional regression tests also be added to the
> LTS validation tests when they are run?
>
Yes, we will test the additional case on each LTS version from now on.
Best regards,
Yu Jiang
> > We hope the two related patches can be backported to LTS branch, and the
> second patch just reworks for previous bug's fix.
> > Patch1:
> https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-
> david.marchand@redhat.com/ a8ca8ed net/i40e: revert link status check on
> device start
>
> I have already reverted those 3 backports on 21.11 branch so this is not needed.
>
> > Pathc2:
> https://patchwork.dpdk.org/project/dpdk/patch/20230306121853.27547-1-
> simei.su@intel.com/ 82fcf20 net/i40e: fix maximum frame size
> configuration
> >
>
> That is the v7 of this v6 with revert and fix split, so same one being discussed
> above.
>
> > Best regards,
> > Yu Jiang
> >
> >> thanks,
> >> Kevin.
> >>
> >>> After reverting the original patch and 2 fixes, I'm a bit reluctant
> >>> to take more fixes without some form of regression testing in place.
> >>>
> >>> thanks,
> >>> Kevin.
> >>>
> >>>>>
> >>>>> Reported-by: David Marchand <david.marchand@redhat.com>
> >>>>> Signed-off-by: Simei Su <simei.su@intel.com>
> >>>>
> >>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>>
> >>>> Applied to dpdk-next-net-intel.
> >>>>
> >>>> Thanks
> >>>> Qi
> >>>>
> >>>
> >
Hi Kevin,
> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Friday, March 24, 2023 5:40 PM
> To: Jiang, YuX <yux.jiang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Su,
> Simei <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> Yuying <yuying.zhang@intel.com>; david.marchand@redhat.com
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org;
> Luca Boccassi <bluca@debian.org>; Mcnamara, John
> <john.mcnamara@intel.com>
> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size configuration
>
> On 24/03/2023 06:32, Jiang, YuX wrote:
> >> -----Original Message-----
> >> From: Kevin Traynor <ktraynor@redhat.com>
> >> Sent: Thursday, March 23, 2023 10:51 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Su, Simei
> >> <simei.su@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> >> Yuying <yuying.zhang@intel.com>; david.marchand@redhat.com; Jiang,
> >> YuX <yux.jiang@intel.com>
> >> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> >> stable@dpdk.org; Luca Boccassi <bluca@debian.org>; Mcnamara, John
> >> <john.mcnamara@intel.com>
> >> Subject: Re: [PATCH v6] net/i40e: rework maximum frame size
> >> configuration
> >>
> >> On 22/03/2023 16:50, Kevin Traynor wrote:
> >>> On 27/02/2023 00:35, Zhang, Qi Z wrote:
> >>>>
> >>>
> >>> Hi Simei/Qi/Yu
> >>>
> >>
> >> Hi Yu,
> >>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Su, Simei <simei.su@intel.com>
> >>>>> Sent: Monday, February 20, 2023 4:00 PM
> >>>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> >>>>> <yuying.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >>>>> david.marchand@redhat.com
> >>>>> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Su, Simei
> >>>>> <simei.su@intel.com>; stable@dpdk.org
> >>>>> Subject: [PATCH v6] net/i40e: rework maximum frame size
> >>>>> configuration
> >>>>>
> >>>>> One issue is reported by David Marchand that error occurs in OVS
> >>>>> due to the fix patch in mentioned changes below. The detailed
> >>>>> reproduce step and result are in
> >>>>> https://patchwork.dpdk.org/project/dpdk/patch/
> >>>>> 20211207085946.121032-1-dapengx.yu@intel.com/.
> >>>>>
> >>>>> This patch removes unnecessary link status check and directly sets
> >>>>> mac config in dev_start. Also, it sets the parameter "wait to
> >>>>> complete" true to wait for more time to make sure adminq command
> >> execute completed.
> >>>>>
> >>>
> >>>>> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> >>>>> level")
> >>>>> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> >>>>> level")
> >>>>> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> >>>>> Cc: stable@dpdk.org
> >>>
> >>> These patches caused an observable regression in multiple 20.11 and
> >>> 21.11 LTS releases that was only caught a long time after releases.
> >>>
> >>> Is there anything being added to LTS validation for regression
> >>> testing this issue, so we don't get caught again?
> >>>
> >>
> >> This is the issue I was talking about earlier during the release
> >> meeting. Not sure if we were talking about the same patch.
> >>
> >> I was asking if there are some regression tests added/can be added to
> >> LTS validation that will be run during each LTS validation cycle so
> >> we don't have any more regressions on it.
> >>
> > Hi Kevin,
> > Thanks for your comments.
> > Yes. We are adding additional case to cover more testing. For main branch,
> we have done the regression testing (including the additional case testing),
> they both work well.
>
> That's good to hear. Will the additional regression tests also be added to the
> LTS validation tests when they are run?
>
> > We hope the two related patches can be backported to LTS branch, and the
> second patch just reworks for previous bug's fix.
> > Patch1:
> https://patchwork.dpdk.org/project/dpdk/patch/20221213091837.87953-1-d
> avid.marchand@redhat.com/ a8ca8ed net/i40e: revert link status check
> on device start
>
> I have already reverted those 3 backports on 21.11 branch so this is not
> needed.
>
> > Pathc2:
> https://patchwork.dpdk.org/project/dpdk/patch/20230306121853.27547-1-s
> imei.su@intel.com/ 82fcf20 net/i40e: fix maximum frame size
> configuration
> >
>
> That is the v7 of this v6 with revert and fix split, so same one being discussed
> above.
The fix split is to rework and simplify code in previous bug fix patch for unexpected packets received issue.
Best Regards,
Simei
>
> > Best regards,
> > Yu Jiang
> >
> >> thanks,
> >> Kevin.
> >>
> >>> After reverting the original patch and 2 fixes, I'm a bit reluctant
> >>> to take more fixes without some form of regression testing in place.
> >>>
> >>> thanks,
> >>> Kevin.
> >>>
> >>>>>
> >>>>> Reported-by: David Marchand <david.marchand@redhat.com>
> >>>>> Signed-off-by: Simei Su <simei.su@intel.com>
> >>>>
> >>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>>
> >>>> Applied to dpdk-next-net-intel.
> >>>>
> >>>> Thanks
> >>>> Qi
> >>>>
> >>>
> >
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
struct rte_ether_addr *mac_addr);
static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
static int i40e_ethertype_filter_convert(
const struct rte_eth_ethertype_filter *input,
@@ -2449,7 +2448,7 @@ i40e_dev_start(struct rte_eth_dev *dev)
PMD_DRV_LOG(WARNING, "Fail to set phy mask");
/* Call get_link_info aq command to enable/disable LSE */
- i40e_dev_link_update(dev, 0);
+ i40e_dev_link_update(dev, 1);
}
if (dev->data->dev_conf.intr_conf.rxq == 0) {
@@ -2467,8 +2466,12 @@ i40e_dev_start(struct rte_eth_dev *dev)
"please call hierarchy_commit() "
"before starting the port");
- max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
- i40e_set_mac_max_frame(dev, max_frame_size);
+ max_frame_size = dev->data->mtu ?
+ dev->data->mtu + I40E_ETH_OVERHEAD :
+ I40E_FRAME_SIZE_MAX;
+
+ /* Set the max frame size to HW*/
+ i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
return I40E_SUCCESS;
@@ -2809,9 +2812,6 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
return i40e_phy_conf_link(hw, abilities, speed, false);
}
-#define CHECK_INTERVAL 100 /* 100ms */
-#define MAX_REPEAT_TIME 10 /* 1s (10 * 100ms) in total */
-
static __rte_always_inline void
update_link_reg(struct i40e_hw *hw, struct rte_eth_link *link)
{
@@ -2878,6 +2878,8 @@ static __rte_always_inline void
update_link_aq(struct i40e_hw *hw, struct rte_eth_link *link,
bool enable_lse, int wait_to_complete)
{
+#define CHECK_INTERVAL 100 /* 100ms */
+#define MAX_REPEAT_TIME 10 /* 1s (10 * 100ms) in total */
uint32_t rep_cnt = MAX_REPEAT_TIME;
struct i40e_link_status link_status;
int status;
@@ -12123,40 +12125,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
return ret;
}
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- uint32_t rep_cnt = MAX_REPEAT_TIME;
- struct rte_eth_link link;
- enum i40e_status_code status;
- bool can_be_set = true;
-
- /*
- * I40E_MEDIA_TYPE_BASET link up can be ignored
- * I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
- * is I40E_MEDIA_TYPE_UNKNOWN
- */
- if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
- hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
- do {
- update_link_reg(hw, &link);
- if (link.link_status)
- break;
- rte_delay_ms(CHECK_INTERVAL);
- } while (--rep_cnt);
- can_be_set = !!link.link_status;
- }
-
- if (can_be_set) {
- status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
- if (status != I40E_SUCCESS)
- PMD_DRV_LOG(ERR, "Failed to set max frame size at port level");
- } else {
- PMD_DRV_LOG(ERR, "Set max frame size at port level not applicable on link down");
- }
-}
-
RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
#ifdef RTE_ETHDEV_DEBUG_RX