[D] bnx2x: Disable multi-cos feature.
diff mbox series

Message ID 20190822132004.31220-1-mfo@canonical.com
State New
Headers show
Series
  • [D] bnx2x: Disable multi-cos feature.
Related show

Commit Message

Mauricio Faria de Oliveira Aug. 22, 2019, 1:20 p.m. UTC
From: Sudarsana Reddy Kalluru <skalluru@marvell.com>

BugLink: https://bugs.launchpad.net/bugs/1840789

Commit 3968d38917eb ("bnx2x: Fix Multi-Cos.") which enabled multi-cos
feature after prolonged time in driver added some regression causing
numerous issues (sudden reboots, tx timeout etc.) reported by customers.
We plan to backout this commit and submit proper fix once we have root
cause of issues reported with this feature enabled.

Fixes: 3968d38917eb ("bnx2x: Fix Multi-Cos.")
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(backported from commit d1f0b5dce8fda09a7f5f04c1878f181d548e42f5)
[mfo: backport: essentially revert the mentioned commit;
 upstream more recently removed the fallback() function
 and moved to netdev_pick_tx(),
 the parentheses are left in as done in this commit.]
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Stefan Bader Aug. 27, 2019, 2:35 p.m. UTC | #1
On 22.08.19 15:20, Mauricio Faria de Oliveira wrote:
> From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1840789
> 
> Commit 3968d38917eb ("bnx2x: Fix Multi-Cos.") which enabled multi-cos
> feature after prolonged time in driver added some regression causing
> numerous issues (sudden reboots, tx timeout etc.) reported by customers.
> We plan to backout this commit and submit proper fix once we have root
> cause of issues reported with this feature enabled.
> 
> Fixes: 3968d38917eb ("bnx2x: Fix Multi-Cos.")
> Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Signed-off-by: Manish Chopra <manishc@marvell.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (backported from commit d1f0b5dce8fda09a7f5f04c1878f181d548e42f5)
> [mfo: backport: essentially revert the mentioned commit;
>  upstream more recently removed the fallback() function
>  and moved to netdev_pick_tx(),
>  the parentheses are left in as done in this commit.]
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
> ---

Is this really a backport for D? The change looks very much like the upstream
change to me...

-Stefan

>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 78a01880931c..9c5b932c61dd 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -1932,8 +1932,7 @@ u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb,
>  	}
>  
>  	/* select a non-FCoE queue */
> -	return fallback(dev, skb, NULL) %
> -	       (BNX2X_NUM_ETH_QUEUES(bp) * bp->max_cos);
> +	return fallback(dev, skb, NULL) % (BNX2X_NUM_ETH_QUEUES(bp));
>  }
>  
>  void bnx2x_set_num_queues(struct bnx2x *bp)
>
Mauricio Faria de Oliveira Aug. 27, 2019, 2:42 p.m. UTC | #2
On Tue, Aug 27, 2019 at 11:35 AM Stefan Bader
<stefan.bader@canonical.com> wrote:
>
> On 22.08.19 15:20, Mauricio Faria de Oliveira wrote:
> > From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1840789
> >
> > Commit 3968d38917eb ("bnx2x: Fix Multi-Cos.") which enabled multi-cos
> > feature after prolonged time in driver added some regression causing
> > numerous issues (sudden reboots, tx timeout etc.) reported by customers.
> > We plan to backout this commit and submit proper fix once we have root
> > cause of issues reported with this feature enabled.
> >
> > Fixes: 3968d38917eb ("bnx2x: Fix Multi-Cos.")
> > Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > (backported from commit d1f0b5dce8fda09a7f5f04c1878f181d548e42f5)
> > [mfo: backport: essentially revert the mentioned commit;
> >  upstream more recently removed the fallback() function
> >  and moved to netdev_pick_tx(),
> >  the parentheses are left in as done in this commit.]
> > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
> > ---
>
> Is this really a backport for D? The change looks very much like the upstream
> change to me...
>

Yes, D needs s/netdev_pick_tx/fallback/

> -Stefan
>
> >  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > index 78a01880931c..9c5b932c61dd 100644
> > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> > @@ -1932,8 +1932,7 @@ u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb,
> >       }
> >
> >       /* select a non-FCoE queue */
> > -     return fallback(dev, skb, NULL) %
> > -            (BNX2X_NUM_ETH_QUEUES(bp) * bp->max_cos);
> > +     return fallback(dev, skb, NULL) % (BNX2X_NUM_ETH_QUEUES(bp));
> >  }
> >
> >  void bnx2x_set_num_queues(struct bnx2x *bp)
> >
>
>
Stefan Bader Aug. 27, 2019, 3:27 p.m. UTC | #3
On 27.08.19 16:42, Mauricio Faria de Oliveira wrote:
> On Tue, Aug 27, 2019 at 11:35 AM Stefan Bader
> <stefan.bader@canonical.com> wrote:
>>
>> On 22.08.19 15:20, Mauricio Faria de Oliveira wrote:
>>> From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1840789
>>>
>>> Commit 3968d38917eb ("bnx2x: Fix Multi-Cos.") which enabled multi-cos
>>> feature after prolonged time in driver added some regression causing
>>> numerous issues (sudden reboots, tx timeout etc.) reported by customers.
>>> We plan to backout this commit and submit proper fix once we have root
>>> cause of issues reported with this feature enabled.
>>>
>>> Fixes: 3968d38917eb ("bnx2x: Fix Multi-Cos.")
>>> Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
>>> Signed-off-by: Manish Chopra <manishc@marvell.com>
>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>> (backported from commit d1f0b5dce8fda09a7f5f04c1878f181d548e42f5)
>>> [mfo: backport: essentially revert the mentioned commit;
>>>  upstream more recently removed the fallback() function
>>>  and moved to netdev_pick_tx(),
>>>  the parentheses are left in as done in this commit.]
>>> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
>>> ---
>>
>> Is this really a backport for D? The change looks very much like the upstream
>> change to me...
>>
> 
> Yes, D needs s/netdev_pick_tx/fallback/

Ah ok, missed that in the patch.

Acked-by: Stefan Bader <stefan.bader@canonical.com>

> 
>> -Stefan
>>
>>>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
>>> index 78a01880931c..9c5b932c61dd 100644
>>> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
>>> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
>>> @@ -1932,8 +1932,7 @@ u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb,
>>>       }
>>>
>>>       /* select a non-FCoE queue */
>>> -     return fallback(dev, skb, NULL) %
>>> -            (BNX2X_NUM_ETH_QUEUES(bp) * bp->max_cos);
>>> +     return fallback(dev, skb, NULL) % (BNX2X_NUM_ETH_QUEUES(bp));
>>>  }
>>>
>>>  void bnx2x_set_num_queues(struct bnx2x *bp)
>>>
>>
>>
> 
>
Mauricio Faria de Oliveira Sept. 2, 2019, 11:21 a.m. UTC | #4
Please hold / don't apply this patch for now.

The reporter hit an apparently unrelated Oops in 3 of 40 nodes,
and it hasn't been possible yet to determine whether this patch
is at all related or at fault, due to timing/deployment matters
preventing a methodical approach to revert to a original kernel.

Since the patch is recent even in the mainline kernel, holding
it up for a bit seemed to be the most prudent action for LTSes
and thus drop the patch which would be required on Disco too.

We'll be following up on this as possible on the reporter's end.

Thanks,
Mauricio

Patch
diff mbox series

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 78a01880931c..9c5b932c61dd 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -1932,8 +1932,7 @@  u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb,
 	}
 
 	/* select a non-FCoE queue */
-	return fallback(dev, skb, NULL) %
-	       (BNX2X_NUM_ETH_QUEUES(bp) * bp->max_cos);
+	return fallback(dev, skb, NULL) % (BNX2X_NUM_ETH_QUEUES(bp));
 }
 
 void bnx2x_set_num_queues(struct bnx2x *bp)