From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
by dpdk.org (Postfix) with ESMTP id A1EB07E62
for <dev@dpdk.org>; Fri, 24 Oct 2014 17:49:52 +0200 (CEST)
Received: from orsmga001.jf.intel.com ([10.7.209.18])
by orsmga103.jf.intel.com with ESMTP; 24 Oct 2014 08:56:53 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.04,780,1406617200"; d="scan'208";a="595338633"
Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153])
by orsmga001.jf.intel.com with ESMTP; 24 Oct 2014 08:58:20 -0700
Received: from irsmsx152.ger.corp.intel.com (163.33.192.66) by
IRSMSX101.ger.corp.intel.com (163.33.3.153) with Microsoft SMTP Server (TLS)
id 14.3.195.1; Fri, 24 Oct 2014 16:58:20 +0100
Received: from irsmsx105.ger.corp.intel.com ([169.254.7.56]) by
IRSMSX152.ger.corp.intel.com ([169.254.6.125]) with mapi id 14.03.0195.001;
Fri, 24 Oct 2014 16:58:20 +0100
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Richardson, Bruce" <bruce.richardson@intel.com>
Thread-Topic: [dpdk-dev] [PATCH 2/3] pmd: RX function need keep
EXTERNAL_MBUF flag
Thread-Index: AQHP72II3cEQXzazwEuf9kL3IfWzwJw/Dd9AgAAQcQCAADSqAIAAEg+Q
Date: Fri, 24 Oct 2014 15:58:19 +0000
Message-ID: <2601191342CEEE43887BDE71AB9772582139ECF1@IRSMSX105.ger.corp.intel.com>
References: <1414138209-24431-1-git-send-email-changchun.ouyang@intel.com>
<1414138209-24431-3-git-send-email-changchun.ouyang@intel.com>
<2601191342CEEE43887BDE71AB9772582139EBAF@IRSMSX105.ger.corp.intel.com>
<20141024123458.GA7648@BRICHA3-MOBL> <20141024154328.GC7648@BRICHA3-MOBL>
In-Reply-To: <20141024154328.GC7648@BRICHA3-MOBL>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [163.33.239.180]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 2/3] pmd: RX function need keep EXTERNAL_MBUF
flag
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 24 Oct 2014 15:49:53 -0000
> -----Original Message-----
> From: Richardson, Bruce
> Sent: Friday, October 24, 2014 4:43 PM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/3] pmd: RX function need keep EXTERNAL_M=
BUF flag
>=20
> On Fri, Oct 24, 2014 at 01:34:58PM +0100, Bruce Richardson wrote:
> > On Fri, Oct 24, 2014 at 10:46:06AM +0000, Ananyev, Konstantin wrote:
> > > Hi Changchun,
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang Changch=
un
> > > > Sent: Friday, October 24, 2014 9:10 AM
> > > > To: dev@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH 2/3] pmd: RX function need keep EXTERNAL=
_MBUF flag
> > > >
> > > > Every pmd RX function need keep the EXTERNAL_MBUF flag
> > > > in mbuf.ol_flags, and can't overwrite it when filling ol_flags from
> > > > descriptor to mbuf, otherwise, it probably cause to crash when free=
ing a mbuf
> > > > and trying to freeing its attached external buffer, say, from guest=
space.
> > > >
> > >
> > > Don't really like the idea to put:
> > > mb->ol_flags =3D pkt_flags | (mb->ol_flags & EXTERNAL_MBUF);
> > > in each and every PMD from now on...
> > >
> > > From other side, it is probably not very good that RX functions updat=
e whole ol_flags, not only RX related part.
> > > Wonder can we reserve low 32bits of ol_flags for RX, and high 32bits =
for TX and generic stuff.
> > > So our ol_flags will look something like that:
> > >
> > > union {
> > > uint64_t ol_raw_flags;
> > > struct {
> > > uint32_t rx;
> > > uint32_t gen_tx;
> > > } ol_flags
> > > };
> > >
> > > And make all PMD RX functions to operate on rx part of the flags only=
:
> > > mb->ol_flags.rx =3D pkt_flags;
> > > ?
> > >
> > > Konstantin
> > >
> > I would tend to agree with this. Changchun, did you get to assess the
> > performance impact of making this change to the PMDs? I suspect that ma=
king
> > the changes to each PMD would impact performance, while Konstantin's
> > suggestion should eliminate that impact.
> > The downside there is that we are limiting the flexibility we have in
> > expanding beyond 32 RX flags and 24 TX flags. :-(
> >
> > /Bruce
> >
>=20
> How about switching things about in terms of the flag. Instead of having =
to
> manage a flag across the baord to indicate if an mbuf is pointing to
> external memory, I think we should use the flag to indicate that an mbuf =
is
> attached to the memory space of another mbuf.
>=20
> My reasons for suggesting this are:
> 1. Mbufs pointing to externally managed memory are not really the problem=
to
> be dealt with on free, since they can be handled the same as mbufs with t=
he
> data pointer pointing internally, it's mbufs attached to other mbufs whic=
h
> are - so that's what we need to track using a flag.
> 2. Setting the flag to indicate an indirect mbuf should have no impact on
> the driver, as an mbuf that has just been allocated from mempool cannot b=
e
> an indirect one.
> 3. The only place we would need to worry about such a flag is in the atta=
ch,
> detach and free mbuf functions - and on free we would simply need to repl=
ace
> the existing check for "md !=3D m" with a new check for the new flag. It =
would
> be a contained change.
>=20
Sounds good to me.
That's' definitely much better than my proposal.
Plus, if we'll stop to rely on:
md =3D RTE_MBUF_FROM_BADDR(m->buf_addr);
if (unlikely (md !=3D m)) {
That will allow us to set buf_addr to some other valid offset inside mbuf
and that fix an old problem with mbufs extra metadata (userdata) stored in =
the packet's headroom.=20
Konstantin
> Thoughts?
> /Bruce