| Message ID | 20190822132004.31220-1-mfo@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [D] bnx2x: Disable multi-cos feature. | expand |
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) >
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) > > > >
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) >>> >> >> > >
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
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)