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 B9BAB1B027 for ; Wed, 13 Sep 2017 23:42:27 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Sep 2017 14:42:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,389,1500966000"; d="scan'208";a="1014168935" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by orsmga003.jf.intel.com with ESMTP; 13 Sep 2017 14:42:25 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.75]) by IRSMSX101.ger.corp.intel.com ([169.254.1.22]) with mapi id 14.03.0319.002; Wed, 13 Sep 2017 22:42:24 +0100 From: "Ananyev, Konstantin" To: Thomas Monjalon , "stephen@networkplumber.org" CC: "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///a4gIAAilcA Date: Wed, 13 Sep 2017 21:42:24 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772584F24AA9D@irsmsx105.ger.corp.intel.com> References: <1868308.cPa78Soq0s@xps> <2601191342CEEE43887BDE71AB9772584F24A738@irsmsx105.ger.corp.intel.com> <12544923.ZPp1eIik3W@xps> In-Reply-To: <12544923.ZPp1eIik3W@xps> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiM2VjZjI5MjctZGJiMi00MDIyLTllOGYtN2MwMmY1ZmViYTZlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjlEN3ZhRnRucEdENDhPZDkrdnJnMHdxM3V4VytiQlVUWTkyYzRXSnRkZGc9In0= 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: Wed, 13 Sep 2017 21:42:28 -0000 > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Wednesday, September 13, 2017 2:21 PM > To: Ananyev, Konstantin ; stephen@networkpl= umber.org > Cc: dev@dpdk.org; Shahaf Shuler > Subject: Re: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to the ne= w offloads API >=20 > 13/09/2017 14:56, Ananyev, Konstantin: > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > 13/09/2017 13:16, Shahaf Shuler: > > > > Wednesday, September 13, 2017 12:28 PM, Thomas Monjalon: > > > > > I still think we must streamline ethdev API instead of complexify= ing. > > > > > We should drop the big "configure everything" and configure offlo= ads one by > > > > > one, and per queue (the finer grain). > > > > > > > > The issue is, that there is some functionality which cannot be achi= eved when configuring offload per queue. > > > > For example - vlan filter on intel NICs. The PF can set it even wit= hout creating a single queue, in order to enable it for the VFs. > > > > > > As it is a device-specific - not documented - side effect, > > > I won't consider it. > > > > Hmm, are you saying that if there are gaps in our documentation it ok t= o break things? >=20 > If it is not documented, we did not explicitly agree on it. > How an application knows that setting a PF settings will have > effect on related VFs? >=20 > > Once again - you suggest to break existing functionality without provid= ing any > > alternative way to support it. >=20 > It is not a functionality, it is a side effect. I wouldn't agree with that. DPDK does support PF for these devices. It is responsibility of PF to provide to user ability to configure and cont= rol it's VF(s). =20 > What happens if a VF changes this settings? error? Depends on particular offload and HW. For ixgbe and igb for most cases VF is simply not physically capable to cha= nge these things. I think, that in some places error is returned, in other such request is si= lently ignored. > Is this error documented? >=20 > > Surely I will NACK such proposal. >=20 > Nothing to nack, I agree with v3 which doesn't break ixgbe VLAN settings. Ok then. >=20 > 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 features. It would be a good thing, but I don't think it is possible for all offloads= . For some of them you still have to stop the queue(port) first.=20 Also I am not sure what exactly do you propose? Is that something like that: - wipe existing offload bitfileds from rte_eth_rxmode (already done by Shah= af) - Instead of uint64_t offloads inside both rte_eth_rxmode and te_eth_rxcon= f 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 offload_mask); uint64_t rte_eth_get_port_rx_offload(portid); uint64_t rte_eth_set_queue_rx_offload(portid, queueid); 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_offload() somewhere before dev_start() for static offloads. ? If so, then it seems reasonable to me. Konstantin >=20 > > > However I understand it may be better to be able to configure > > > per-port offloads with a dedicated per-port function. > > > I agree with the approach of the v3 of this series. > > > > > > Let me give my overview of offloads: > > > > > > We have simple offloads which are configured by just setting a flag. > > > The same flag can be set per-port or per-queue. > > > This offload can be set before starting or on the fly. > > > We currently have no generic way to set it on the fly. > > > > > > We have also more complicate offloads which require more configuratio= n. > > > They are set with the rte_flow API. > > > They can be per-port, per-queue, on the fly or not (AFAIK). > > > > > > I think we must discuss "on the fly" capability. > > > It requires probably to set up simple offloads (flags) with a dedicat= ed > > > function instead of using "configure" and "queue_setup" functions. > > > This new capability can be implemented in a different series. > > > > > > Opinions?