RE: [PATCH 6.1 001/131] net: mana: Fix TX CQE error handling

Haiyang Zhang
2023-10-16T17:55:16Z

> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Monday, October 16, 2023 1:02 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: stable@vger.kernel.org; patches@lists.linux.dev; Simon Horman
> <horms@kernel.org>; Shradha Gupta <shradhagupta@linux.microsoft.com>;
> Paolo Abeni <pabeni@redhat.com>; Sasha Levin <sashal@kernel.org>
> Subject: Re: [PATCH 6.1 001/131] net: mana: Fix TX CQE error handling
>
> On Mon, Oct 16, 2023 at 03:35:27PM +0000, Haiyang Zhang wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Sent: Monday, October 16, 2023 10:47 AM
> > > To: Haiyang Zhang <haiyangz@microsoft.com>
> > > Cc: stable@vger.kernel.org; patches@lists.linux.dev; Simon Horman
> > > <horms@kernel.org>; Shradha Gupta
> <shradhagupta@linux.microsoft.com>;
> > > Paolo Abeni <pabeni@redhat.com>; Sasha Levin <sashal@kernel.org>
> > > 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@linuxfoundation.org>
> > > > > Sent: Monday, October 16, 2023 4:40 AM
> > > > > To: stable@vger.kernel.org
> > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> > > > > patches@lists.linux.dev; Haiyang Zhang <haiyangz@microsoft.com>;
> > > Simon
> > > > > Horman <horms@kernel.org>; Shradha Gupta
> > > > > <shradhagupta@linux.microsoft.com>; Paolo Abeni
> > > <pabeni@redhat.com>;
> > > > > Sasha Levin <sashal@kernel.org>
> > > > > 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@microsoft.com>
> > > > >
> > > > > [ 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@vger.kernel.org
> > > > > Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure
> > > Network
> > > > > Adapter (MANA)")
> > > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > > Reviewed-by: Simon Horman <horms@kernel.org>
> > > > > Reviewed-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > > > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > > > > ---
> > > > > 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.k/
> ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%
> 2Fcommit%2F%3Fid%3Db2b000069a4c307b09548dc2243f31f3ca0eac9c&d
> ata=05%7C01%7Chaiyangz%40microsoft.com%7Cea8eae85691b47b1674b0
> 8dbce69a49a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63
> 8330725380724334%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
> C%7C&sdata=rOwj5G7r%2BjkBUWImNrdcjqXneCjq4TG3EiLFYfKKGGE%3D&r
> eserved=0
>
> Thanks, I've fixed this up now.

Thank you, Greg!

- Haiyang