diff mbox series

[1/3] net: dsa: qca8k: reduce mgmt ethernet timeout

Message ID 20220618062300.28541-1-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [1/3] net: dsa: qca8k: reduce mgmt ethernet timeout | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter warning Series does not have a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Christian Marangi June 18, 2022, 6:22 a.m. UTC
The current mgmt ethernet timeout is set to 100ms. This value is too
big and would slow down any mdio command in case the mgmt ethernet
packet have some problems on the receiving part.
Reduce it to just 5ms to handle case when some operation are done on the
master port that would cause the mgmt ethernet to not work temporarily.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vladimir Oltean June 21, 2022, 12:33 p.m. UTC | #1
On Sat, Jun 18, 2022 at 08:22:58AM +0200, Christian Marangi wrote:
> The current mgmt ethernet timeout is set to 100ms. This value is too
> big and would slow down any mdio command in case the mgmt ethernet
> packet have some problems on the receiving part.
> Reduce it to just 5ms to handle case when some operation are done on the
> master port that would cause the mgmt ethernet to not work temporarily.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---

I think this could very well qualify as a regression and have a tag of:

Fixes: 5950c7c0a68c ("net: dsa: qca8k: add support for mgmt read/write in Ethernet packet")

if it was presented along with a situation where users could hit some
real life conditions where the Ethernet management interface isn't
functional.

>  drivers/net/dsa/qca8k.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
> index 04408e11402a..ec58d0e80a70 100644
> --- a/drivers/net/dsa/qca8k.h
> +++ b/drivers/net/dsa/qca8k.h
> @@ -15,7 +15,7 @@
>  
>  #define QCA8K_ETHERNET_MDIO_PRIORITY			7
>  #define QCA8K_ETHERNET_PHY_PRIORITY			6
> -#define QCA8K_ETHERNET_TIMEOUT				100
> +#define QCA8K_ETHERNET_TIMEOUT				5
>  
>  #define QCA8K_NUM_PORTS					7
>  #define QCA8K_NUM_CPU_PORTS				2
> -- 
> 2.36.1
>
Christian Marangi June 21, 2022, 2:48 p.m. UTC | #2
On Tue, Jun 21, 2022 at 03:33:35PM +0300, Vladimir Oltean wrote:
> On Sat, Jun 18, 2022 at 08:22:58AM +0200, Christian Marangi wrote:
> > The current mgmt ethernet timeout is set to 100ms. This value is too
> > big and would slow down any mdio command in case the mgmt ethernet
> > packet have some problems on the receiving part.
> > Reduce it to just 5ms to handle case when some operation are done on the
> > master port that would cause the mgmt ethernet to not work temporarily.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> 
> I think this could very well qualify as a regression and have a tag of:
> 
> Fixes: 5950c7c0a68c ("net: dsa: qca8k: add support for mgmt read/write in Ethernet packet")
> 
> if it was presented along with a situation where users could hit some
> real life conditions where the Ethernet management interface isn't
> functional.
>

It's really to handle corner case... In testing the MTU change (and
noticing the mgmt ethernet going "macheroni") I notice the timeout was
absurdly high. In a situation where something goes wrong at least you
can have access to it after some time.

Ok I will add the fixes tag.

> >  drivers/net/dsa/qca8k.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
> > index 04408e11402a..ec58d0e80a70 100644
> > --- a/drivers/net/dsa/qca8k.h
> > +++ b/drivers/net/dsa/qca8k.h
> > @@ -15,7 +15,7 @@
> >  
> >  #define QCA8K_ETHERNET_MDIO_PRIORITY			7
> >  #define QCA8K_ETHERNET_PHY_PRIORITY			6
> > -#define QCA8K_ETHERNET_TIMEOUT				100
> > +#define QCA8K_ETHERNET_TIMEOUT				5
> >  
> >  #define QCA8K_NUM_PORTS					7
> >  #define QCA8K_NUM_CPU_PORTS				2
> > -- 
> > 2.36.1
> >
diff mbox series

Patch

diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 04408e11402a..ec58d0e80a70 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -15,7 +15,7 @@ 
 
 #define QCA8K_ETHERNET_MDIO_PRIORITY			7
 #define QCA8K_ETHERNET_PHY_PRIORITY			6
-#define QCA8K_ETHERNET_TIMEOUT				100
+#define QCA8K_ETHERNET_TIMEOUT				5
 
 #define QCA8K_NUM_PORTS					7
 #define QCA8K_NUM_CPU_PORTS				2