From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shahaf Shuler Subject: Re: [PATCH 4/4] ethdev: add helpers to move to the new offloads API Date: Mon, 4 Sep 2017 14:02:54 +0000 Message-ID: References: <810c1d26724f82f0d9fc9d6684dc4b1c62fd5f62.1504508375.git.shahafs@mellanox.com> <2601191342CEEE43887BDE71AB9772584F24602F@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: "Ananyev, Konstantin" , Thomas Monjalon Return-path: Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-he1eur01on0081.outbound.protection.outlook.com [104.47.0.81]) by dpdk.org (Postfix) with ESMTP id 63069378E for ; Mon, 4 Sep 2017 16:02:57 +0200 (CEST) In-Reply-To: <2601191342CEEE43887BDE71AB9772584F24602F@irsmsx105.ger.corp.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Konstantin, Monday, September 4, 2017 4:25 PM, Ananyev, Konstantin: >=20 > Hi Shahaf, >=20 > > } > > > > +/** > > + * A conversion function from rxmode offloads API to rte_eth_rxq_conf > > + * offloads API. > > + */ > > +static void > > +rte_eth_convert_rxmode_offloads(struct rte_eth_rxmode *rxmode, > > + struct rte_eth_rxq_conf *rxq_conf) { > > + if (rxmode->header_split =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_HEADER_SPLIT; > > + if (rxmode->hw_ip_checksum =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_CHECKSUM; > > + if (rxmode->hw_vlan_filter =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_VLAN_FILTER; >=20 > Thinking on it a bit more: > VLAN_FILTER is definitely one per device, as it would affect VFs also. > At least that's what we have for Intel devices (ixgbe, i40e) right now. This is vendor specific. For Mellanox this is one per device (regardless if= it is a vf/pf). > For Intel devices VLAN_STRIP is also per device and will also be applied= to all > corresponding VFs. Again - vendor specific. For Mellanox is per queue. > In fact, right now it is possible to query/change these 3 vlan offload fl= ags on > the fly (after dev_start) on port basis by > rte_eth_dev_(get|set)_vlan_offload API. > So, I think at least these 3 flags need to be remained on a port basis. Am not sure I agree.=20 Why, for example, block from application the option to set some queues with= vlan strip and some without if device allows? Also how will we decide which offloads should stay per port and which are a= llowed to move per queue? this much depends on the underlying PMD. Looks like i missed that part on ethdev, and if Rx offload will be per queu= e I will need to change it also. > In fact, why can't we have both per port and per queue RX offload: > - dev_configure() will accept RX_OFFLOAD_* flags and apply them on a port > basis. > - rx_queue_setup() will also accept RX_OFFLOAD_* flags and apply them on > a queue basis. > - if particular RX_OFFLOAD flag for that device couldn't be setup on a qu= eue > basis - > rx_queue_setup() will return an error. Why not taking the per port configuration as a sub-case of per queue config= uration? For per-port offloads as long as the same configuration applies the queue s= etup succeeds. > - rte_eth_rxq_info can be extended to provide information which > RX_OFFLOADs > can be configured on a per queue basis. I am OK with the info suggestion.=20 > BTW - in that case we probably wouldn't need ignore flag inside rx_conf > anymore. >=20 >=20 > > + if (rxmode->hw_vlan_strip =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_VLAN_STRIP; > > + if (rxmode->hw_vlan_extend =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_VLAN_EXTEND; > > + if (rxmode->jumbo_frame =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_JUMBO_FRAME; >=20 > There are some extra checks for that flag inside rte_eth_dev_configure(). > If we going so support it per queue - then it probably need to be updated= . >=20 > > + if (rxmode->hw_strip_crc =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_CRC_STRIP; > > + if (rxmode->enable_scatter =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_SCATTER; > > + if (rxmode->enable_lro =3D=3D 1) > > + rxq_conf->offloads |=3D DEV_RX_OFFLOAD_TCP_LRO; } > > + >=20 > Konstantin