On Thu, Dec 08, 2011 at 17:46 +0100, Mike Belopuhov wrote:
> there are some m_freem's missing in the error code paths.
> i guess that's because freebsd code is a bit different
> and we didn't look closely at it.
>
> ok?
>
> @@ -2601,6 +2604,8 @@ no_split:
> error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->pmap,
> mp, BUS_DMA_NOWAIT);
> if (error) {
> + if (mh)
> + m_freem(mh);
> m_freem(mp);
> return (error);
> }
in fact, if we're failing here, mh is already dma-loaded,
so we have to unload it first and clear the m_head pointer.
also i forgot to note that split mbufs are not currently
used, so there's no change in the behavior and these bugs
are basically hidden. nevertheless i think it makes sense
to fix them.
diff --git dev/pci/if_ix.c dev/pci/if_ix.c
index d3c268e..35ecb34 100644
--- dev/pci/if_ix.c
+++ dev/pci/if_ix.c
@@ -2546,7 +2546,7 @@ ixgbe_get_buf(struct rx_ring *rxr, int i)
{
struct ix_softc *sc = rxr->sc;
struct ixgbe_rx_buf *rxbuf;
- struct mbuf *mh, *mp;
+ struct mbuf *mp, *mh = NULL;
int error;
union ixgbe_adv_rx_desc *rxdesc;
size_t dsize = sizeof(union ixgbe_adv_rx_desc);
@@ -2570,8 +2570,10 @@ ixgbe_get_buf(struct rx_ring *rxr, int i)
goto no_split;
mh = m_gethdr(M_DONTWAIT, MT_DATA);
- if (mh == NULL)
+ if (mh == NULL) {
+ m_freem(mp);
return (ENOBUFS);
+ }
mh->m_pkthdr.len = mh->m_len = MHLEN;
mh->m_len = MHLEN;
@@ -2581,6 +2583,7 @@ ixgbe_get_buf(struct rx_ring *rxr, int i)
error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->hmap,
mh, BUS_DMA_NOWAIT);
if (error) {
+ m_freem(mp);
m_freem(mh);
return (error);
}
@@ -2600,6 +2603,11 @@ no_split:
error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->pmap,
mp, BUS_DMA_NOWAIT);
if (error) {
+ if (mh) {
+ bus_dmamap_unload(rxr->rxdma.dma_tag, rxbuf->hmap);
+ rxbuf->m_head = NULL;
+ m_freem(mh);
+ }
m_freem(mp);
return (error);
}