Re: [PATCH net-next v13 2/5] net: rnpgbe: Add basic TX packet transmission support

[Yibo Dong]

On Fri, Sep 25, 2026 at 05:07:05PM +0100, Simon Horman wrote:
> On Mon, Sep 21, 2026 at 10:20:45AM +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> > Thank you for your contribution! Sashiko AI review found 5 potential
> > issue(s) to consider.
> > 
> > Critical: 0 · High: 0 · Medium: 2 · Low: 3
> > 
> > If you believe none of the issues are legitimate, please reply to each
> > issue of Medium severity or higher and explain why the AI is wrong.
> > If any of them are legitimate and you plan to respin, please let patchwork
> > know by including "pw-bot: cr" as a separate line at the end of your reply
> > (one such reply per series is enough).
> 
> Hi Dong,
> 
> There is an expectation that the patch author responds to these
> AI-generated reviews as described above.
> 
> In order to move things along I'll provide some feedback on this one
> (but not the other's for this patchset).
> 

Thanks for the heads‑up and your feedback.
I will fix the issues and send out a revised version for this patchset.
Starting from the next submission, I will respond promptly to AI‑generated reviews.

> > 
> > [Severity: Medium]
> > Should the return value of dma_set_mask_and_coherent() still be checked
> > here? The previous code aborted probe and printed "No usable DMA
> > configuration, aborting", and dma_set_mask_and_coherent() is a plain static
> > inline int with no __must_check, so a failure is now completely silent.
> > 
> > If dma_supported() rejects a 56-bit mask, neither the streaming nor the
> > coherent mask is changed, the device keeps the PCI core's 32-bit default,
> > and probe continues through pci_request_mem_regions() to register_netdev().
> > 
> > The dma_get_mask() test above keeps highmem frags away by not setting
> > NETIF_F_HIGHDMA, but it does not cover the linear-data mapping in
> > rnpgbe_tx_map():
> > 
> > 	dma = dma_map_single(tx_ring->dev, skb->data, size,
> > 			     DMA_TO_DEVICE);
> > 	...
> > 	if (dma_mapping_error(tx_ring->dev, dma))
> > 		goto err_unmap;
> > 
> > nor the descriptor allocation in rnpgbe_setup_tx_resources():
> > 
> > 	tx_ring->desc = dma_alloc_coherent(dev, tx_ring->size, &tx_ring->dma,
> > 					   GFP_KERNEL);
> > 
> > So every frame whose linear data sits above 4GB would be dropped with only
> > the private per-ring dropped counter moving and no kernel message.
> > 
> > The commit message says only:
> > 
> >   "Configure streaming and coherent DMA masks for the hardware's 56-bit
> >   DMA addressing capability."
> > 
> > Could it also state that a DMA configuration failure is now non-fatal and
> > unreported, or could the check and the error path be restored?
> > 
> > The call is still unchecked at the end of the series, so no later patch
> > covers this.
> 
> I wonder if this is a side-effect of the development of this patch-set.
> It does look like a regression that should be addressed in this patch.
> 
I left the return value unchecked because Documentation/core-api/dma-api-howto.rst
states that dma_set_mask_and_coherent() does not fail for masks wider than 32 bits.
And feedback for other patches point this out:
https://lore.kernel.org/netdev/20260814123330.4b8056f5@xxxxxxxxxx/
So maybe uncheck the return of dma_set_mask_and_coherent here is ok? Or
I should check the return?
> -- 
> pw-bot: changes-requested
>