[PATCH net-next v4 09/13] net: lan966x: add PCIe FDMA support

[Daniel Machon]

> On 5/9/26 10:27 AM, sashiko-bot at kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
> >> +static int lan966x_fdma_pci_napi_poll(struct napi_struct *napi, int weight)
> >> +{
> > [ ... ]
> >> +    /* Get all received skbs. */
> >> +    while (counter < weight) {
> >> +            if (!fdma_has_frames(fdma))
> >> +                    break;
> >> +            /* Order DONE read before DCB/frame reads below. */
> >> +            dma_rmb();
> >> +            counter++;
> >> +            switch (lan966x_fdma_pci_rx_check_frame(rx, &src_port)) {
> >> +            case FDMA_PASS:
> >> +                    break;
> >> +            case FDMA_ERROR:
> >> +                    fdma_dcb_advance(fdma);
> >> +                    goto allocate_new;
> >> +            }
> >> +            skb = lan966x_fdma_pci_rx_get_frame(rx, src_port);
> >> +            fdma_dcb_advance(fdma);
> >> +            if (!skb)
> >> +                    goto allocate_new;
> >
> > If napi_alloc_skb() fails due to memory pressure, does branching to
> > allocate_new prematurely break out of the processing loop? Since the hardware
> > interrupt was already cleared, and napi_complete_done() is called below,
> > will this cause the RX queue to stall permanently for the remaining
> > unprocessed frames? Should the code drop the packet and continue instead?
> 
> I went over the series and sashiko feedback, and generally speaking the
> series LGTM, but I think this one is real and should be addressed.

Thanks for reviewing. Before I respin, I'd like to sanity-check the design
expectation.

I had already looked at this and concluded it was fine — the reasoning being
that any leftover DCBs (frames) would be drained on the next IRQ when new
traffic arrives. Same pattern exists in all other Microchip FDMA drivers
(lan966x platform, sparx5/lan969x, ocelot) and has been running since their
introduction without a reported issue, AFAIK.

Is calling napi_complete_done() with DCBs still pending in the ring itself a
NAPI-contract violation — in that case this is a pattern that needs fixing
across all four drivers.

Happy to go either way — just want to make sure I understand the problem :-)

> 
> Also it would be very helpful if you could (for future series/revision)
> reply to the (wrong) sashiko comments individually explaining with a few
> words why the are off: decoding the context requires much more time for
> whoever has not wrote the code itself.

Ack.

> 
> I understand that dealing with AI feedback is a pain, but, paraphrasing
> a great aphorisms creator, I can assure you that ours (maintainers) pain
> is greater.

I can only imagine.

> 
> Thanks,
> 
> Paolo
> 

/Daniel