cvs commit: src/sys/dev/em if_em.c

[Yar Tikhiy]

On Mon, Jul 31, 2006 at 11:43:54AM +0900, Pyun YongHyeon wrote:
> On Sun, Jul 30, 2006 at 01:50:55PM +0400, Yar Tikhiy wrote:
>  > On Sun, Jul 30, 2006 at 03:48:07PM +0900, Pyun YongHyeon wrote:
>  > > On Sat, Jul 29, 2006 at 08:50:09PM +0000, Yar Tikhiy wrote:
>  > >  > On Thu, Jul 27, 2006 at 12:43:34AM +0000, Pyun YongHyeon wrote:
>  > >  > > yongari     2006-07-27 00:43:34 UTC
>  > >  > > 
>  > >  > >   FreeBSD src repository
>  > >  > > 
>  > >  > >   Modified files:
>  > >  > >     sys/dev/em           if_em.c 
>  > >  > >   Log:
>  > >  > >   Prepending an mbuf after loading a DMA map results in unexpected
>  > >  > >   result. So, modify mbuf chains before loading a DMA map.
>  > >  > >   
>  > >  > >   Revision  Changes    Path
>  > >  > >   1.122     +28 -31    src/sys/dev/em/if_em.c
>  > >  > 
>  > >  > Thanks a lot!  Do you think this can fix kern/72933?
>  > >  > 
>  > > 
>  > > I can't sure it helps as the submitter reported the same issue
>  > > on bge(4).
>  > 
>  > Sorry, my question wasn't accurate.  I believe that the original
>  > PR in fact described a different issue with similar symptoms; it
>  > was fixed quite a while ago.  Then Lev Shamardin posted a follow-up
>  > that he was still seeing the symptoms with em(4), which we (Gleb
>  > Smirnoff and yours truly) couldn't fully understand because we were
>  > missing the peculiarities of the interaction between DMA and mbufs
>  > you're so well aware of.
>  > 
>  > > Btw, I think the VLAN fixup code should use m_prepend(9)
>  > > insatead of M_PREEND(9) because we don't know whether a new mbuf
>  > > is allocated or not after M_PREPEND(9) call.
>  > 
>  > If a new mbuf should be allocated to satisfy DMA, m_prepend(9) is
> 
> No, prepending a new mbuf with m_prepend(9) is not necessary for DMA
> to work.
> 
>  > the function to use.  M_PREPEND(9) can use the free space at the
>  > beginning of the mbuf's data area if there is enough of it.  I'm
>  > unsure though whether we really need a new mbuf there.  em_encap()
>  > gets just a mbuf chain, which can be tweaked a little before starting
>  > the DMA magic on it.
>  > 
> 
> Sorry, I'm confused. I've misread VLAN fixup code. From my limited testing, 
> it seems that it works as expected.
> Previously em(4) may have failed VLAN tag insertion if M_PREPEND(9) added
> a new mbuf into existing mbuf chains. 

Your analysis sounds correct to me.  Previously the data from the
mbuf chain passed to em_encap() were "wired" to the DMA engine
(mapped through bus_dmamap_load*) first, and a VLAN header was
prepended then, which was plain wrong and worked sometimes by
accident, no matter M_PREPEND or m_prepend was used.  With the order
of the things being correct now, we can keep using M_PREPEND for
efficiency.  In the routing case, there is a fair chance that the
packet came from another VLAN and there is some free leading space
in the first mbuf.

-- 
Yar