On Mon, Oct 16, 2023 at 03:35:27PM +0000, Haiyang Zhang wrote:
>
>
> > -----Original Message-----
> > From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Sent: Monday, October 16, 2023 10:47 AM
> > To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx; patches@xxxxxxxxxxxxxxx; Simon Horman
> > <horms@xxxxxxxxxx>; Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx>;
> > Paolo Abeni <pabeni@xxxxxxxxxx>; Sasha Levin <sashal@xxxxxxxxxx>
> > Subject: Re: [PATCH 6.1 001/131] net: mana: Fix TX CQE error handling
> >
> > On Mon, Oct 16, 2023 at 02:35:15PM +0000, Haiyang Zhang wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > Sent: Monday, October 16, 2023 4:40 AM
> > > > To: stable@xxxxxxxxxxxxxxx
> > > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>;
> > > > patches@xxxxxxxxxxxxxxx; Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>;
> > Simon
> > > > Horman <horms@xxxxxxxxxx>; Shradha Gupta
> > > > <shradhagupta@xxxxxxxxxxxxxxxxxxx>; Paolo Abeni
> > <pabeni@xxxxxxxxxx>;
> > > > Sasha Levin <sashal@xxxxxxxxxx>
> > > > Subject: [PATCH 6.1 001/131] net: mana: Fix TX CQE error handling
> > > >
> > > > 6.1-stable review patch. If anyone has any objections, please let me know.
> > > >
> > > > ------------------
> > > >
> > > > From: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> > > >
> > > > [ Upstream commit b2b000069a4c307b09548dc2243f31f3ca0eac9c ]
> > > >
> > > > For an unknown TX CQE error type (probably from a newer hardware),
> > > > still free the SKB, update the queue tail, etc., otherwise the
> > > > accounting will be wrong.
> > > >
> > > > Also, TX errors can be triggered by injecting corrupted packets, so
> > > > replace the WARN_ONCE to ratelimited error logging.
> > > >
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure
> > Network
> > > > Adapter (MANA)")
> > > > Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> > > > Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
> > > > Reviewed-by: Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Paolo Abeni <pabeni@xxxxxxxxxx>
> > > > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> > > > ---
> > > > drivers/net/ethernet/microsoft/mana/mana_en.c | 16 ++++++++++------
> > > > 1 file changed, 10 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > > > b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > > > index 4f4204432aaa3..23ce26b8295dc 100644
> > > > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > > > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > > > @@ -1003,16 +1003,20 @@ static void mana_poll_tx_cq(struct mana_cq
> > > > *cq)
> > > > case CQE_TX_VPORT_IDX_OUT_OF_RANGE:
> > > > case CQE_TX_VPORT_DISABLED:
> > > > case CQE_TX_VLAN_TAGGING_VIOLATION:
> > > > - WARN_ONCE(1, "TX: CQE error %d: ignored.\n",
> > > > - cqe_oob->cqe_hdr.cqe_type);
> > > > + if (net_ratelimit())
> > > > + netdev_err(ndev, "TX: CQE error %d\n",
> > > > + cqe_oob->cqe_hdr.cqe_type);
> > > > +
> > > > break;
> > > >
> > > > default:
> > > > - /* If the CQE type is unexpected, log an error, assert,
> > > > - * and go through the error path.
> > > > + /* If the CQE type is unknown, log an error,
> > > > + * and still free the SKB, update tail, etc.
> > > > */
> > > > - WARN_ONCE(1, "TX: Unexpected CQE type %d: HW
> > > > BUG?\n",
> > > > - cqe_oob->cqe_hdr.cqe_type);
> > > > + if (net_ratelimit())
> > > > + netdev_err(ndev, "TX: unknown CQE
> > > > type %d\n",
> > > > + cqe_oob->cqe_hdr.cqe_type);
> > > > +
> > > > return;
> > >
> > > This should be changed to "break", because we should "still free the SKB,
> > update
> > > the queue tail, etc., otherwise the accounting will be wrong":
> >
> > Is that an issue in Linus's tree, or is this unique to the stable
> > backport?
>
> It's just a stable backporting issue.
>
> Linus's tree is fine:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b2b000069a4c307b09548dc2243f31f3ca0eac9c
Thanks, I've fixed this up now.
greg k-h
