Re: [dpdk-dev] rte_mbuf.next in 2nd cacheline

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 15 Jun 2015 18:34:13 +0000
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:

> 
> 
> > -----Original Message-----
> > From: Richardson, Bruce
> > Sent: Monday, June 15, 2015 5:23 PM
> > To: Ananyev, Konstantin
> > Cc: Olivier MATZ; dev@dpdk.org; Damjan Marion (damarion)
> > Subject: Re: [dpdk-dev] rte_mbuf.next in 2nd cacheline
> > 
> > On Mon, Jun 15, 2015 at 05:10:44PM +0100, Ananyev, Konstantin wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Richardson, Bruce
> > > > Sent: Monday, June 15, 2015 5:02 PM
> > > > To: Ananyev, Konstantin
> > > > Cc: Olivier MATZ; dev@dpdk.org; Damjan Marion (damarion)
> > > > Subject: Re: [dpdk-dev] rte_mbuf.next in 2nd cacheline
> > > >
> > > > On Mon, Jun 15, 2015 at 04:59:55PM +0100, Ananyev, Konstantin wrote:
> > > > >
> > > > >
> > > > >
> > > > > As I can see, vector TX is the only one that calls __rte_pktmbuf_prefree_seg() directly.
> > > > > All others use rte_pktmbuf_free_seg(), that does ' m->next = NULL' anyway.
> > > > > For vector TX - yes, need to verify that it would not introduce a slowdown.
> > > > > Konstantin
> > > > >
> > > >
> > > > But if the function is only directly called from one place, and that doesn't
> > > > have a problem, why would we bother making any change at all?
> > >
> > >
> > > For future usages?
> > > But sure, if you believe that we can safely remove 'm->next = NULL' at RX path,
> > > without any changes in the __rte_pktmbuf_prefree_seg() -
> > > that seems fine to me.
> > > Konstantin
> > >
> > 
> > If we find it's not safe, we can add in the change to __rte_pktmbuf_prefree_seg
> > as you suggest.
> > 
> > One other question: based on this, do you think it's safe to also remove the
> > assignment to NULL from the pktmbuf_alloc function? I suspect it should be safe, and
> > that should help any traffic-generator type applications that use that function
> > extensively.
> 
> So it will be setup to NULL, either by:
> - mbuf constructor.
> - TX full-path free descriptors code.
> - upper layer code that uses 'next' pointer explicitly.
> ?
> 
> I can't come-up with some breakage scenario off-hand.
> But that means that we have to should avoid resetting tx_offload in pktmbuf_alloc() too, right?
> Otherwise there probably wouldn't be any real gain.
> Konstantin
> 
> > 
> > /Bruce
> 
> 

The issue is when mbuf is used once by something and it sets it to non-NULL
packet is sent then freed.
then the free packet is picked up by one of the drivers using "fast path" Rx code.

IMHO the "fast path" Rx code has to go away. All the special allocation code
should call rte_pktmuf_reset and then only set the fields that are special
for the received packet.  The compiler should be able to remove the redundant
stores, and it would prevent bugs I am seeing where some fields are not set
in some drivers.