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 DDED0A0C4B; Fri, 15 Oct 2021 11:36:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CB594411CB; Fri, 15 Oct 2021 11:36:11 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id AF13940692 for ; Fri, 15 Oct 2021 11:36:10 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 5FF307F5F5; Fri, 15 Oct 2021 12:36:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 5FF307F5F5 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1634290570; bh=MVL5wriy2ppS36il597XQKMD2KvKFEVFqnhVXRxuYpM=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=e4kUHHT+ZJzvb8yJ6W3XOFMH42D6pzmIQQYktrggPX16lziReRwse2WyTnj2sURVx H78X7ZFRi1Cz7BG6UXI/nvG7F3OkkW+wsVhp5+eKiqUzpOpUjGAw70X6lrDT1ZeVEp 1+RTqS5lScuouFmUTf5T9CFSSF95E6fDt1P1kGUM= To: Dmitry Kozlyuk , "dev@dpdk.org" , Ori Kam , Raslan Darawsheh Cc: NBU-Contact-Thomas Monjalon , Ferruh Yigit References: <20211005005216.2427489-1-dkozlyuk@nvidia.com> <20211005005216.2427489-3-dkozlyuk@nvidia.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <6a22dfbe-79b4-0ceb-3331-61761a3777bd@oktetlabs.ru> Date: Fri, 15 Oct 2021 12:36:10 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 2/5] ethdev: add capability to keep shared objects on restart 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 Sender: "dev" On 10/15/21 12:04 PM, Dmitry Kozlyuk wrote: >> [...] >>>>> From: Dmitry Kozlyuk >>>>> >>>>> rte_flow_action_handle_create() did not mention what happens with an >>>>> indirect action when a device is stopped, possibly reconfigured, and >>>>> started again. It is natural for some indirect actions to be >>>>> persistent, like counters and meters; keeping others just saves >>>>> application time and complexity. However, not all PMDs can support it. >>>>> It is proposed to add a device capability to indicate if indirect >>>>> actions are kept across the above sequence or implicitly destroyed. >>>>> >>>>> In the future, indirect actions may not be the only type of objects >>>>> shared between flow rules. The capability bit intends to cover all >>>>> possible types of such objects, hence its name. >>>>> >>>>> It may happen that in the future a PMD acquires support for a type >>>>> of shared objects that it cannot keep across a restart. It is >>>>> undesirable to stop advertising the capability so that applications >>>>> that don't use objects of the problematic type can still take >> advantage of it. >>>>> This is why PMDs are allowed to keep only a subset of shared objects >>>>> provided that the vendor mandatorily documents it. >>>>> >>>>> If the device is being reconfigured in a way that is incompatible >>>>> with an existing shared objects, PMD is required to report an error. >>>>> This is mandatory, because flow API does not supply users with >>>>> capabilities, so this is the only way for a user to learn that >>>>> configuration is invalid. For example, if queue count changes and >>>>> RSS indirect action specifies queues that are going away, the user >>>>> must update the action before removing the queues or remove the >>>>> action and all flow rules that were using it. >>>>> >>>>> Signed-off-by: Dmitry Kozlyuk >>>>> --- >>>>> [...] >>>> >>>> Current pain point is that capability bits may be insufficient and a >>>> programmatic way is desired to check which types of objects can be >>>> kept across restart, instead of documenting the limitations. >>>> >>>> I support one of previous Ori's suggestions and want to clarify it [1]: >>>> >>>> Ori: "Another way is to assume that if the action was created before >>>> port start it will be kept after port stop." >>>> Andrew: "It does not sound like a solution. May be I simply don't >>>> know target usecase." >>>> >>>> What Ori suggests (offline discussion summary): Suppose an >>>> application wants to check whether a shared object (indirect action) >>>> or a flow rule of a particular kind. It calls >>>> rte_flow_action_handle_create() or >>>> rte_flow_create() before rte_eth_dev_start(). If it succeeds, 1) it >>>> means objects of this type can be kept across restart, 2) it's a >>>> normal object created that will work after the port is started. This >>>> is logical, because if the PMD can keep some kind of objects when the >>>> port is stopped, it is likely to be able to create them when the port >>>> is not started. It is subject to discussion if "object kind" means >>>> only "type" or "type + transfer bit" combination; for mlx5 PMD it >>>> doesn't matter. One minor drawback is that applications can only do >>>> the test when the port is stopped, but it seems likely that the test >>>> really needs to be done at startup anyway. >>>> >>>> If this is acceptable: >>>> 1. Capability bits are not needed anymore. >>>> 2. ethdev patches can be accepted in RC1, present behavior is >>>> undefined anyway. >>>> 3. PMD patches will need update that can be done by RC2. >>> >>> Andrew, what do you think? >>> If you agree, do we need to include transfer bit into "kind"? >>> I'd like to conclude before RC1 and can update the docs quickly. >>> >>> I've seen the proposition to advertise capability to create flow rules >>> before device start as a flag. >>> I don't think it conflicts with Ori's suggestion because the flag >>> doesn't imply that _any_ rule can be created, neither does it say >>> about indirect actions. >>> On the other hand, if PMD can create a flow object (rule, etc.) when >>> the device is not started, it is logical to assume that after the >>> device is stopped it can move existing flow objects to the same state >>> as when the device was not started, then restore when it is started >>> again. >> >> Dmitry, thanks for the explanations. Ori's idea makes sense to me now. The >> problem is to document it properly. We must define rules to check it. >> Which bits in the check request matter and how application should make a >> choice of rule to try. > > This is a generalization of the last question about the transfer bit. > I call the bits that matter a "kind". As I see it: > > rule kind = seq. of item types + seq. of action types > indirect action kind = action type > > As Ori mentioned, for mlx5 PMD transfer bit doesn't affect object persistence. > If you or other PMD maintainers think it may be relevant, no problem, > because PMDs like mlx5 will just ignore it when checking. Then it will be: > > rule kind = seq. of item types + seq. of action types + attr. transfer bit > indirect action kind = action type + indirect action conf. transfer bit > > Including the transfer bit seems to be a safe choice from DPDK point of view, > but obviously it can force applications to do up to twice as many checks. > > The application needs to construct a valid flow configuration > that is (1) valid and (2) has the kind the application is interested in. > It is worth noting that these checks are not about resource consumption, > i.e. it is sufficient to test an indirect RSS with one queue > to be confident that indirect RSS with any number of queues are preserved. > >> Which status code should be returned by the PMD to >> clearly say that addition in started state is not supported and, >> therefore, preserving across restart is not supported. > > I suggest a new DPDK-specific value in rte_errno.h. > >> Must the device be configured before an attempt to check it? > > Yes, because flow objects created by these checks are as good as any others > and AFAIK no PMD supports rte_flow calls before configuration is done. > >> Should transfer and non-transfer rules/indirect actions be checked separately? > > See above. > Please, try to put it into the patch for documentation and I'll review the result. All my above questions should be answered in the documentation.