[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dpdk-dev
Subject:    [dpdk-dev] Issue with non-scattered rx in ixgbe and i40e when mbuf private area size is odd
From:       thomas.monjalon () 6wind ! com (Thomas Monjalon)
Date:       2015-07-30 13:47:26
Message-ID: 1682615.eOc1hvvtSx () xps13
[Download RAW message or body]

2015-07-30 13:22, Olivier MATZ:
> On 07/30/2015 11:43 AM, Ananyev, Konstantin wrote:
> > From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> > > On 07/30/2015 11:00 AM, Ananyev, Konstantin wrote:
> > > > From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> > > > > On 07/29/2015 10:24 PM, Zhang, Helin wrote:
> > > > > > The similar situation in i40e, as explained by Konstantin.
> > > > > > As header split hasn't been supported by DPDK till now. It would be \
> > > > > > better to put the header address in RX descriptor to 0. But in the \
> > > > > > future, during header split enabling. We may need to pay extra attention \
> > > > > > to that. As at least x710 datasheet said
> > > > > specifically as below.
> > > > > > "The header address should be set by the software to an even number (word \
> > > > > > aligned address)". We may need to find a way to
> > > > > ensure that during mempool/mbuf allocation.
> > > > > 
> > > > > Indeed it would be good to force the priv_size to be aligned.
> > > > > 
> > > > > The priv_size could be aligned automatically in
> > > > > rte_pktmbuf_pool_create(). The only possible problem I could see
> > > > > is that it would break applications that access to the data buffer
> > > > > by doing (sizeof(mbuf) + sizeof(priv)), which is probably not the
> > > > > best thing to do (I didn't find any applications like this in dpdk).
> > > > 
> > > > 
> > > > Might be just make rte_pktmbuf_pool_create() fail if input priv_size % \
> > > > MIN_ALIGN != 0?
> > > 
> > > Hmm maybe it would break more applications: an odd priv_size is
> > > probably rare, but a priv_size that is not aligned to 8 bytes is
> > > maybe more common.
> > 
> > My thought was that rte_mempool_create() was just introduced in 2.1,
> > so if we add extra requirement for the input parameter now -
> > there would be no ABI breakage, and not many people started to use it already.
> > For me just seems a bit easier and more straightforward then silent alignment -
> > user would not have wrong assumptions here.
> > Though if you think that a silent alignment would be more convenient
> > for most users - I wouldn't insist.
> 
> 
> Yes, I agree on the principle, but it depends whether this fix
> is integrated for 2.1 or not.
> I think it may already be a bit late for that, especially as it
> is not a very critical bug.
> 
> Thomas, what do you think?

It is a fix.
Adding a doc comment, an assert and an alignment constraint or a new automatic
alignment in the not yet released function shouldn't hurt.
A patch would be welcome for 2.1. Thanks


[prev in list] [next in list] [prev in thread] [next in thread]