From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 015C6237 for ; Mon, 18 Sep 2017 13:11:19 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Sep 2017 04:11:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,412,1500966000"; d="scan'208";a="312871525" Received: from irsmsx109.ger.corp.intel.com ([163.33.3.23]) by fmsmga004.fm.intel.com with ESMTP; 18 Sep 2017 04:11:17 -0700 Received: from irsmsx111.ger.corp.intel.com (10.108.20.4) by IRSMSX109.ger.corp.intel.com (163.33.3.23) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 18 Sep 2017 12:11:16 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.75]) by irsmsx111.ger.corp.intel.com ([169.254.2.30]) with mapi id 14.03.0319.002; Mon, 18 Sep 2017 12:11:16 +0100 From: "Ananyev, Konstantin" To: "Richardson, Bruce" CC: Thomas Monjalon , "stephen@networkplumber.org" , "dev@dpdk.org" , Shahaf Shuler Thread-Topic: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to the new offloads API Thread-Index: AQHTJYU8DF/tpFi9okyg3kyCdEiysqKkwPgQgAEZ2gCAABLvoIAAIFYAgAATnACAAS25gIAAR4XggAryaYCAAB5GAIAAF+WAgAAUKND///a4gIAAilcAgACvEQCABnMXgIAAFgTA///zNQCAABH+MA== Date: Mon, 18 Sep 2017 11:11:16 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772584F24BFE7@irsmsx105.ger.corp.intel.com> References: <12544923.ZPp1eIik3W@xps> <2601191342CEEE43887BDE71AB9772584F24AA9D@irsmsx105.ger.corp.intel.com> <6120098.SpXJOblVKo@xps> <20170918103154.GA13172@bricha3-MOBL3.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772584F24BFBF@irsmsx105.ger.corp.intel.com> <20170918110456.GB15516@bricha3-MOBL3.ger.corp.intel.com> In-Reply-To: <20170918110456.GB15516@bricha3-MOBL3.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZDEwNzE5MGYtNmU2ZS00OGQ0LWIwZmQtOThjODBiNzhhODI5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkgzM1NNTU9idUYwSFh4VmdTQ2tDQW5mSlBBMHQxdW9CTzZnSUM0XC8zS0RFPSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to the new offloads API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Sep 2017 11:11:20 -0000 > -----Original Message----- > From: Richardson, Bruce > Sent: Monday, September 18, 2017 12:05 PM > To: Ananyev, Konstantin > Cc: Thomas Monjalon ; stephen@networkplumber.org; de= v@dpdk.org; Shahaf Shuler > Subject: Re: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to the ne= w offloads API >=20 > On Mon, Sep 18, 2017 at 11:57:03AM +0100, Ananyev, Konstantin wrote: > > > > > > > -----Original Message----- > > > From: Richardson, Bruce > > > Sent: Monday, September 18, 2017 11:32 AM > > > To: Thomas Monjalon > > > Cc: Ananyev, Konstantin ; stephen@netwo= rkplumber.org; dev@dpdk.org; Shahaf Shuler > > > > > > Subject: Re: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to th= e new offloads API > > > > > > On Thu, Sep 14, 2017 at 10:02:26AM +0200, Thomas Monjalon wrote: > > > > 13/09/2017 23:42, Ananyev, Konstantin: > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > > > 13/09/2017 14:56, Ananyev, Konstantin: > > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > > > Konstantin, I would like your opinion about the proposal below. > > > > > > It is about making on the fly configuration more generic. > > > > > > You say it is possible to configure VLAN on the fly, > > > > > > and I think we should make it possible for other offload featur= es. > > > > > > > > > > It would be a good thing, but I don't think it is possible for al= l offloads. > > > > > For some of them you still have to stop the queue(port) first. > > > > > > > > > > Also I am not sure what exactly do you propose? > > > > > Is that something like that: > > > > > - wipe existing offload bitfileds from rte_eth_rxmode (already do= ne by Shahaf) > > > > > - Instead of uint64_t offloads inside both rte_eth_rxmode and te= _eth_rxconf > > > > > Introduce new functions: > > > > > > > > > > int rte_eth_set_port_rx_offload(portid, uint64_t offload_mask); > > > > > int rte_eth_set_queue_rx_offload(portid, queueid, uint64_t offloa= d_mask); > > > Would be useful to have a valid mask here, to indicate what bits to u= se. > > > That way, you can adjust one bit without worrying about what other bi= ts > > > you may change in the process. There are probably apps out there that > > > just want to toggle a single bit on, and off, at runtime while ignori= ng > > > others. > > > Alternatively, we can have set/unset functions which enable/disable > > > offloads, based on the mask. > > > > My thought was that people would do: > > > > uint64_t offload =3D rte_eth_get_port_rx_offload(port); > > offload |=3D RX_OFFLOAD_X; > > offload &=3D ~RX_OFFLOAD_Y; > > rte_eth_set_port_rx_offload(port, offload); > > > > In that case, I think we don't really need a mask. > > > > > > > > > > > > > > > uint64_t rte_eth_get_port_rx_offload(portid); > > > > > uint64_t rte_eth_set_queue_rx_offload(portid, queueid); > > > s/set/get/ > > > > > > > > > > And add new fileds: > > > > > rx_offload_port_dynamic_capa > > > > > rx_offload_queue_dynamic_capa > > > > > inside rte_eth_dev_info. > > > > > > > > > > And it would be user responsibility to call set_port/queue_rx_off= load() > > > > > somewhere before dev_start() for static offloads. > > > > > ? > > > > > > > > Yes exactly. > > > > > > > > > If so, then it seems reasonable to me. > > > > > > > > Good, thank you > > > > > > > > > > > Sorry I'm a bit late to the review, but the above suggestion of separ= ate > > > APIs for enabling offloads, seems much better than passing in the fla= gs > > > in structures to the existing calls. From what I see all later revisi= ons > > > of this patchset still use the existing flags parameter to setup call= s > > > method. > > > > > > Some advantages that I see of the separate APIs: > > > * allows some settings to be set before start, and others afterwards, > > > with an appropriate return value if dynamic config not supported. > > > * we can get fine grained error reporting from these - the set calls = can > > > all return the mask indicating what offloads could not be applied - > > > zero means all ok, 1 means a problem with that setting. This may be > > > easier for the app to use than feature discovery in some cases. > > > * for those PMDs which support configuration at a per-queue level, it > > > can allow the user to specify the per-port settings as a default, a= nd > > > then override that value at the queue level, if you just want one q= ueue > > > different from the rest. > > > > I think we all in favor to have a separate API here. > > Though from the discussion we had at latest TB, I am not sure it is doa= ble > > in 17.11 timeframe. >=20 > Ok, so does that imply no change in this release, and that the existing > set is to be ignored? No, my understanding the current plan is to go forward with Shahaf patches, and then apply another one (new set/get API) on top of them. Konstantin