From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B315BA0350; Thu, 27 Jan 2022 10:35:11 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7464B4277A; Thu, 27 Jan 2022 10:35:11 +0100 (CET) Received: from mail-io1-f42.google.com (mail-io1-f42.google.com [209.85.166.42]) by mails.dpdk.org (Postfix) with ESMTP id A39334067C for ; Thu, 27 Jan 2022 10:35:10 +0100 (CET) Received: by mail-io1-f42.google.com with SMTP id h7so2843250iof.3 for ; Thu, 27 Jan 2022 01:35:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Dc0sT5iPHZ3xf5oBe3CP8SPj8xI0rtMbzfcutXWcrEU=; b=KihlSypKncM2ufhB4KZS+4hf2tq3N+1zs25Tlqzbb3ADe0shGHoj7FLNdG7xFOLp3G M5JfJrWdDOHXHf550N2qazC9UVx1BY6QBX2pgsvyigF1tZLdXqZa+rA0pY9ESxgCBluG 7iQ2J9wwU5dp28V+QNgNETCHhcgVfHdonQfJd/VdueHTqNOS5rp2um3gCcs5FNgC95D4 C/tGsXAXcdOctABco2OqHfnh560EoI/eqGWS5xrysancgQGncZW+XNw4Tv/m56dkH81+ MP87kSeHFGI1R6LpADYnjNsNfFhwUVwUDg/zHeYlwMIa1g1VHHta9YXcHgIYiU+cSDqX 2pHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Dc0sT5iPHZ3xf5oBe3CP8SPj8xI0rtMbzfcutXWcrEU=; b=LXKURvdMpKANz6eG4/78N8LkVB26U2PGzlIz2r45XvSIO3UlN/5EsePZHD88jYpkgY YlT0wFbQ2NiOuriL3K0C+kDg63xs95wglWf7acVMP5svuepL5J82xykrhRDV+bMvqOqI BuBGQTkqY/iQ2r3jhUTIfhsDLvwoyh0VC+u6nqocvQ9jrJf19C92qnsBOc3v1+XVdAeE SejjVb6+emJrNgyGs0e/QHz+m6+idCKlCLbr/nlqG32UwSvW37C5U5C9ug21Z2cD0+cx o98qLIluUTZkW4IV5enJ55RJ4CdvQkNUp5mosjQP2cg3lbJRz7sQ9n/ZM137p2SbWXg/ rPhg== X-Gm-Message-State: AOAM531jq6KVQXFJmJmI1qfciyuY9Yfkwv1GD0Axi57n4/uEicZ/mB7z wgwIs4qbGx3aHXc3f/Si0Xf7cSt8ZNExK50Nwe4= X-Google-Smtp-Source: ABdhPJyGj3nYB0+CEe41aZUN+A/pqB8RkQby8SCmOfy13DL8MMV7TkZo3wkpNfjwoURXIONkq1f4HgZHUxGmk1f6imw= X-Received: by 2002:a02:5d89:: with SMTP id w131mr1314049jaa.123.1643276109878; Thu, 27 Jan 2022 01:35:09 -0800 (PST) MIME-Version: 1.0 References: <20211006044835.3936226-1-akozyrev@nvidia.com> <20220118153027.3947448-1-akozyrev@nvidia.com> <20220118153027.3947448-2-akozyrev@nvidia.com> In-Reply-To: From: Jerin Jacob Date: Thu, 27 Jan 2022 15:04:43 +0530 Message-ID: Subject: Re: [PATCH v2 01/10] ethdev: introduce flow pre-configuration hints To: Alexander Kozyrev Cc: Ajit Khaparde , dpdk-dev , Ori Kam , "NBU-Contact-Thomas Monjalon (EXTERNAL)" , Ivan Malov , Andrew Rybchenko , Ferruh Yigit , "mohammad.abdul.awal@intel.com" , Qi Zhang , Jerin Jacob Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, Jan 27, 2022 at 3:32 AM Alexander Kozyrev wrote: > > On Tuesday, January 25, 2022 13:44 Jerin Jacob wrote: > > On Tue, Jan 25, 2022 at 6:58 AM Alexander Kozyrev > > wrote: > > > > > > On Monday, January 24, 2022 12:41 Ajit Khaparde > > wrote: > > > > On Mon, Jan 24, 2022 at 6:37 AM Jerin Jacob > > > > wrote: > > > > > > > > > > Ok, I'll adopt this wording in the v3. > > > > > > > > > + * > > > > > > + * @param port_id > > > > > > + * Port identifier of Ethernet device. > > > > > > + * @param[in] port_attr > > > > > > + * Port configuration attributes. > > > > > > + * @param[out] error > > > > > > + * Perform verbose error reporting if not NULL. > > > > > > + * PMDs initialize this structure in case of error only. > > > > > > + * > > > > > > + * @return > > > > > > + * 0 on success, a negative errno value otherwise and rte_errno is > > set. > > > > > > + */ > > > > > > +__rte_experimental > > > > > > +int > > > > > > +rte_flow_configure(uint16_t port_id, > > > > > > > > > > Should we couple, setting resource limit hint to configure function as > > > > > if we add future items in > > > > > configuration, we may pain to manage all state. Instead how about, > > > > > rte_flow_resource_reserve_hint_set()? > > > > +1 > > > Port attributes are the hints, PMD can safely ignore anything that is not > > supported/deemed unreasonable. > > > Having several functions to call instead of one configuration function seems > > like a burden to me. > > > > If we add a lot of features which has different state it will be > > difficult to manage. > > Since it is the slow path and OPTIONAL API. IMO, it should be fine to > > have a separate API for a specific purpose > > to have a clean interface. > > This approach contradicts to the DPDK way of configuring devices. > It you look at the rte_eth_dev_configure or rte_eth_rx_queue_setup API > you will see that the configuration is propagated via config structures. > I would like to conform to this approach with my new API as well. There is a subtle difference, those are mandatory APIs. i,e application must call those API to use the subsequent APIs. I am OK with introducing rte_flow_configure() for such use cases. Probably, we can add these parameters in rte_flow_configure() for the new features. And make it mandatory API for the next ABI to avoid application breakage. Also, please change git commit to the description for adding the configure state for rte_flow API. BTW: Your Queue patch[3/3] probably needs to add the nb_queue parameter to configure. So the driver knows, the number queue needed upfront like the ethdev API scheme. > > Another question is how to deal with interdependencies with separate hints? > There could be some resources that requires other resources to be present. > Or one resource shares the hardware registers with another one and needs to > be accounted for. That is not easy to do with separate function calls. I got the use case now. > > > > > > > > > > > > > > > > > > > > > > > > + const struct rte_flow_port_attr *port_attr, > > > > > > + struct rte_flow_error *error); > > > > > > > > > > I think, we should have _get function to get those limit numbers > > otherwise, > > > > > we can not write portable applications as the return value is kind of > > > > > boolean now if > > > > > don't define exact values for rte_errno for reasons. > > > > +1 > > > We had this discussion in RFC. The limits will vary from NIC to NIC and from > > system to > > > system, depending on hardware capabilities and amount of free memory > > for example. > > > It is easier to reject a configuration with a clear error description as we do > > for flow creation. > > > > In that case, we can return a "defined" return value or "defined" > > errno to capture this case so that > > the application can make forward progress to differentiate between API > > failed vs dont having enough resources > > and move on. > > I think you are right and it will be useful to provide some hardware capabilities. > I'll add something like rte_flow_info_get() to obtain available flow rule resources. Ack.