Re: [PATCH] 3c59x: fix missing dma_mapping_error check
From: Neil Horman Date: Wed, 3 Jan 2018 05:42:04 -0500 > On Tue, Jan 02, 2018 at 09:48:27PM -0500, David Miller wrote: >> And for the RX cases, it allows the RX ring to deplete to empty which >> tends to hang most chips. You need to make the DMA failure detection >> early and recycle the RX buffer back to the chip instead of passing >> it up to the stack. >> > Strictly speaking, I think we're ok here, because the dirty_rx counter > creates a > contiguous area to refill, and we will just pick up where we left off on the > next napi poll. If you continually fail the mappings, even NAPI poll, eventually the RX ring will empty. I don't think we're ok here.
Re: [PATCH] 3c59x: fix missing dma_mapping_error check
On Tue, Jan 02, 2018 at 09:48:27PM -0500, David Miller wrote: > From: Neil Horman > Date: Fri, 29 Dec 2017 11:40:10 -0500 > > > @@ -2067,6 +2072,9 @@ vortex_start_xmit(struct sk_buff *skb, struct > > net_device *dev) > > int len = (skb->len + 3) & ~3; > > vp->tx_skb_dma = pci_map_single(VORTEX_PCI(vp), skb->data, len, > > PCI_DMA_TODEVICE); > > + if (dma_mapping_error(&VORTEX_PCI(vp)->dev, vp->tx_skb_dma)) > > + return NETDEV_TX_OK; > > + > > This leaks the SKB, right? > Crud, I think you're right, I'll respin this to address today. > And for the RX cases, it allows the RX ring to deplete to empty which > tends to hang most chips. You need to make the DMA failure detection > early and recycle the RX buffer back to the chip instead of passing > it up to the stack. > Strictly speaking, I think we're ok here, because the dirty_rx counter creates a contiguous area to refill, and we will just pick up where we left off on the next napi poll. That said, it can still depelete if we get several consecutive dma_mapping_errors in a sufficiently long period of time that the ring doesn't ever refill, so yeah, recycling will be better. That will take a bigger rewrite of this code. I'll post something asap. Neil
Re: [PATCH] 3c59x: fix missing dma_mapping_error check
From: Neil Horman Date: Fri, 29 Dec 2017 11:40:10 -0500 > @@ -2067,6 +2072,9 @@ vortex_start_xmit(struct sk_buff *skb, struct > net_device *dev) > int len = (skb->len + 3) & ~3; > vp->tx_skb_dma = pci_map_single(VORTEX_PCI(vp), skb->data, len, > PCI_DMA_TODEVICE); > + if (dma_mapping_error(&VORTEX_PCI(vp)->dev, vp->tx_skb_dma)) > + return NETDEV_TX_OK; > + This leaks the SKB, right? And for the RX cases, it allows the RX ring to deplete to empty which tends to hang most chips. You need to make the DMA failure detection early and recycle the RX buffer back to the chip instead of passing it up to the stack.
