2022-05-09 12:42:08

by Harini Katakam

Subject: [PATCH] net: macb: Increment rx bd head after allocating skb and buffer

In gem_rx_refill rx_prepared_head is incremented at the beginning of
the while loop preparing the skb and data buffers. If the skb or data
buffer allocation fails, this BD will be unusable BDs until the head
loops back to the same BD (and obviously buffer allocation succeeds).
In the unlikely event that there's a string of allocation failures,
there will be an equal number of unusable BDs and an inconsistent RX
BD chain. Hence increment the head at the end of the while loop to be
clean.

Signed-off-by: Harini Katakam <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Radhey Shyam Pandey <[email protected]>
---
drivers/net/ethernet/cadence/macb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 0b03305ad6a0..9c7d590c0188 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1215,7 +1215,6 @@ static void gem_rx_refill(struct macb_queue *queue)
/* Make hw descriptor updates visible to CPU */
rmb();

- queue->rx_prepared_head++;
desc = macb_rx_desc(queue, entry);

if (!queue->rx_skbuff[entry]) {
@@ -1254,6 +1253,7 @@ static void gem_rx_refill(struct macb_queue *queue)
dma_wmb();
desc->addr &= ~MACB_BIT(RX_USED);
}
+ queue->rx_prepared_head++;
}

/* Make descriptor updates visible to hardware */
--
2.17.1

2022-05-10 18:10:11

by Harini Katakam

Subject: Re: [PATCH] net: macb: Increment rx bd head after allocating skb and buffer

Hi Paolo,

On Tue, May 10, 2022 at 6:54 PM Paolo Abeni <[email protected]> wrote:
>
> Hello,
>
> On Mon, 2022-05-09 at 17:49 +0530, Harini Katakam wrote:
> > In gem_rx_refill rx_prepared_head is incremented at the beginning of
> > the while loop preparing the skb and data buffers. If the skb or data
> > buffer allocation fails, this BD will be unusable BDs until the head
> > loops back to the same BD (and obviously buffer allocation succeeds).
> > In the unlikely event that there's a string of allocation failures,
> > there will be an equal number of unusable BDs and an inconsistent RX
> > BD chain. Hence increment the head at the end of the while loop to be
> > clean.
> >
> > Signed-off-by: Harini Katakam <[email protected]>
> > Signed-off-by: Michal Simek <[email protected]>
> > Signed-off-by: Radhey Shyam Pandey <[email protected]>
>
> This looks like targeting the "net" tree, please repost adding a
> suitable Fixes tag.

Thanks for the review. This behavior would theoretically have been the
same since
GEM RX path handling was introduced ~9 yrs ago but I'm not sure since I cannot
reproduce. Probably this?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/cadence?id=4df95131ea803bcb94f472d465c73ed57015c470
Also, this patch can't be backported to stable branches so far back
since the driver
files have changed. It can also be queued for net-next instead.

Regards,
Harini

2022-05-10 20:45:30

by Paolo Abeni

Subject: Re: [PATCH] net: macb: Increment rx bd head after allocating skb and buffer

Hello,

On Mon, 2022-05-09 at 17:49 +0530, Harini Katakam wrote:
> In gem_rx_refill rx_prepared_head is incremented at the beginning of
> the while loop preparing the skb and data buffers. If the skb or data
> buffer allocation fails, this BD will be unusable BDs until the head
> loops back to the same BD (and obviously buffer allocation succeeds).
> In the unlikely event that there's a string of allocation failures,
> there will be an equal number of unusable BDs and an inconsistent RX
> BD chain. Hence increment the head at the end of the while loop to be
> clean.
>
> Signed-off-by: Harini Katakam <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>

This looks like targeting the "net" tree, please repost adding a
suitable Fixes tag.

Thanks,

Paolo