From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 9E249A00E6
	for <public@inbox.dpdk.org>; Tue,  9 Jul 2019 16:00:08 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 7FA6A37A2;
	Tue,  9 Jul 2019 16:00:07 +0200 (CEST)
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by dpdk.org (Postfix) with ESMTP id 3043B160;
 Tue,  9 Jul 2019 16:00:04 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga003.fm.intel.com ([10.253.24.29])
 by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 09 Jul 2019 07:00:03 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.63,470,1557212400"; d="scan'208";a="173569232"
Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203])
 by FMSMGA003.fm.intel.com with ESMTP; 09 Jul 2019 07:00:02 -0700
Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by
 FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS)
 id 14.3.439.0; Tue, 9 Jul 2019 07:00:02 -0700
Received: from shsmsx106.ccr.corp.intel.com ([169.254.10.240]) by
 SHSMSX105.ccr.corp.intel.com ([169.254.11.232]) with mapi id 14.03.0439.000;
 Tue, 9 Jul 2019 22:00:01 +0800
From: "Wang, Xiao W" <xiao.w.wang@intel.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>, "Singh, Jasvinder"
 <jasvinder.singh@intel.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Thread-Topic: [PATCH] net/softnic: fix pipeline time calculation
Thread-Index: AQHVCyi60AUum/nqs0uYdsR6yzvfvaaE4pSAgANih0CAOaGVAIAAtHTQ
Date: Tue, 9 Jul 2019 14:00:00 +0000
Message-ID: <B7F2E978279D1D49A3034B7786DACF407AF621C5@SHSMSX106.ccr.corp.intel.com>
References: <20190515135904.81415-1-xiao.w.wang@intel.com>
 <54CBAA185211B4429112C315DA58FF6D3FD15433@IRSMSX101.ger.corp.intel.com>
 <B7F2E978279D1D49A3034B7786DACF407AF322CD@SHSMSX106.ccr.corp.intel.com>
 <3EB4FA525960D640B5BDFFD6A3D891268E8EBDB3@IRSMSX108.ger.corp.intel.com>
In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891268E8EBDB3@IRSMSX108.ger.corp.intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-ctpclassification: CTP_NT
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzkyNjU0OTMtYmZiZS00NzJkLWFlZDQtMmI5MGUwNzFhNzkzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibUlwTXhuQnI3UEZXMVJiTndnUndidUlDcjBhK25WYmdsSHVuYmZ2d3NLcGV0ZElGTzlUc1NsTVNSZU93Y0RGKyJ9
dlp-product: dlpe-windows
dlp-version: 11.2.0.6
dlp-reaction: no-action
x-originating-ip: [10.239.127.40]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH] net/softnic: fix pipeline time calculation
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>



> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Tuesday, July 9, 2019 6:32 PM
> To: Wang, Xiao W <xiao.w.wang@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] net/softnic: fix pipeline time calculation
>=20
>=20
>=20
> > -----Original Message-----
> > From: Wang, Xiao W
> > Sent: Sunday, June 2, 2019 11:46 AM
> > To: Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> > stable@dpdk.org
> > Subject: RE: [PATCH] net/softnic: fix pipeline time calculation
> >
> >
> >
> > > -----Original Message-----
> > > From: Singh, Jasvinder
> > > Sent: Friday, May 31, 2019 10:46 PM
> > > To: Wang, Xiao W <xiao.w.wang@intel.com>
> > > Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com=
>;
> > > stable@dpdk.org
> > > Subject: RE: [PATCH] net/softnic: fix pipeline time calculation
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wang, Xiao W
> > > > Sent: Wednesday, May 15, 2019 2:59 PM
> > > > To: Singh, Jasvinder <jasvinder.singh@intel.com>
> > > > Cc: dev@dpdk.org; Dumitrescu, Cristian
> > <cristian.dumitrescu@intel.com>;
> > > > Wang, Xiao W <xiao.w.wang@intel.com>; stable@dpdk.org
> > > > Subject: [PATCH] net/softnic: fix pipeline time calculation
> > > >
> > > > When a new pipeline is added to a thread, the "time_next_min" value
> > may
> > > > need update, otherwise this pipeline won't get served timely.
> > > >
> > > > Fixes: 70709c78fda6 ("net/softnic: add command to enable/disable
> > pipeline")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > > ---
> > > >  drivers/net/softnic/rte_eth_softnic_thread.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/net/softnic/rte_eth_softnic_thread.c
> > > > b/drivers/net/softnic/rte_eth_softnic_thread.c
> > > > index 855408e98..2b482117d 100644
> > > > --- a/drivers/net/softnic/rte_eth_softnic_thread.c
> > > > +++ b/drivers/net/softnic/rte_eth_softnic_thread.c
> > > > @@ -337,6 +337,9 @@ softnic_thread_pipeline_enable(struct
> > > pmd_internals
> > > > *softnic,
> > > >  		tdp->timer_period =3D (rte_get_tsc_hz() * p-
> > >timer_period_ms)
> > > /
> > > > 1000;
> > > >  		tdp->time_next =3D rte_get_tsc_cycles() + tdp->timer_period;
> > > >
> > > > +		if (tdp->time_next < td->time_next_min)
> > > > +			td->time_next_min =3D tdp->time_next;
> > > > +
> > > >  		td->n_pipelines++;
> > > >
> > > >  		/* Pipeline */
> > > > @@ -522,6 +525,9 @@ thread_msg_handle_pipeline_enable(struct
> > > > softnic_thread_data *t,
> > > >  		(rte_get_tsc_hz() * req->pipeline_enable.timer_period_ms)
> > /
> > > > 1000;
> > > >  	p->time_next =3D rte_get_tsc_cycles() + p->timer_period;
> > > >
> > > > +	if (p->time_next < t->time_next_min)
> > > > +		t->time_next_min =3D p->time_next;
> > > > +
> > > >  	t->n_pipelines++;
> > > >
> > > >  	/* Response */
> > > > --
> > > > 2.15.1
> > >
> > >
> > > Hi Wang,
> > >
> > > Timer values for pipelines and thread level message handlers are alre=
ady
> > > adjusted in runtime function rte_pmd_softnic_run_internal(). In runti=
me
> > > function, the values of t->time_next_min is updated as well. IMO, abo=
ve
> > > changes not needed. Could you help with the case where timer
> > adjustments in
> > > runtime not working?
> >
> > Hi Jasvinder,
> >
> > the values of t->time_next_min is updated only when the pipeline messag=
e
> > and thread message get handled, but not when the pipeline is added to
> that
> > thread. E.g. when a thread t->time_next_min is ~100ms later, and a new
> > pipeline is added to that thread with timer_period_ms parameter set to
> > 10ms, then this pipeline's control message will not be served until 100=
ms
> > later.
> >
> > BRs,
> > Xiao
> >
> > >
> > > Thanks,
> > > Jasvinder
> > >
>=20
> NAK
>=20
> There are no bugs/issues fixed by this patch, but there are side effects
> introduced that maybe you did not anticipate.
>=20
> - Yes, the first message handler for a newly added pipeline might be slig=
htly
> delayed, but this is harmless.

As the above example shows, the new pipeline handling may get ~90ms delayed=
, and
maybe the user wants this newly added pipeline to be real-time and 90ms del=
ay is too long
for that application.

>=20
> - For a given thread, we periodically iterate through all pipelines the c=
urrent
> thread is running and check if there are any pending messages for each
> pipeline (function rte_pmd_softnic_run_internal). We also update the
> deadline for the next message handling session for the thread (thread-
> >time_next_min), which should only be changed by the thread (existing

Yeah it's about (thead->time_next_min), if current time passes it, then we =
would
Iteratively check each pipepine, only when pipeline's time_next also passes=
 it, we
handle this pipeline.

If the thread is running, the softnic_thread_data should only be changed by=
 the
thread itself. And this patch doesn't break this.

> code); if this is changed by the pipeline message handler, then there is =
the
> risk that some pipelines will run their message handler again very soon a=
fter,
> as the deadline had been brought earlier. Makes sense?

The THREAD_REQ_PIPELINE_ENABLE message is handled in thread_msg_handle(),
not in pipeline_msg_handle().

Thanks for your comments.

-Xiao