diff mbox series

[v2] net/i40e: don't check link status on device start

Message ID 20221213091837.87953-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series [v2] net/i40e: don't check link status on device start |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

David Marchand Dec. 13, 2022, 9:18 a.m. UTC 
  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(-)

Comments

David Marchand Jan. 3, 2023, 2:02 p.m. UTC | #1 
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 Jan. 9, 2023, 9:21 a.m. UTC | #2 
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 Jan. 13, 2023, 1:33 p.m. UTC | #3 
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.
Zhang, Helin Jan. 13, 2023, 1:39 p.m. UTC | #4 
> -----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
David Marchand Jan. 13, 2023, 1:46 p.m. UTC | #5 
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.
Zhang, Helin Jan. 13, 2023, 1:50 p.m. UTC | #6 
> -----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
David Marchand Jan. 13, 2023, 1:53 p.m. UTC | #7 
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.
Simei Su Jan. 16, 2023, 11:02 a.m. UTC | #8 
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
Thomas Monjalon Feb. 7, 2023, 11:31 a.m. UTC | #9 
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.
Simei Su Feb. 7, 2023, 2:05 p.m. UTC | #10 
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
Simei Su March 6, 2023, 6:53 a.m. UTC | #11 
> -----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
Qi Zhang March 6, 2023, 11:05 a.m. UTC | #12 
> -----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
diff mbox series

Patch

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