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 6E297A0C43; Wed, 28 Jul 2021 14:27:01 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E610140E64; Wed, 28 Jul 2021 14:27:00 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 435D640142 for ; Wed, 28 Jul 2021 14:27:00 +0200 (CEST) Received: from [192.168.100.116] (unknown [37.139.99.76]) (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 87A017F529; Wed, 28 Jul 2021 15:26:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 87A017F529 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1627475219; bh=9YmXBVQH0DSYb6IthH1+wPjAm383JZC23oUEBC+b40k=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=JunT1BmTzgGW4Sy4FaWoSmyDKZCUL/NFJ+c1LcmG/RGxwL92dflm0PbrEhtV6xQFt diK9qyrUfD8z4IW45F3TsiwQk5ucyrDz9qMqoad77k/3eRtPij7sCvuyOtdgouOy5f 3FqueqvL7IzFuJRvwHSCYGkFX7yuKthv/cJohnTE= To: Dmitry Kozlyuk , "dev@dpdk.org" Cc: David Marchand References: <20210727073121.895620-1-dkozlyuk@nvidia.com> From: Andrew Rybchenko Message-ID: <15ebcccf-d317-8ade-22f2-1a1acd24ea96@oktetlabs.ru> Date: Wed, 28 Jul 2021 15:26:58 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 0/4] net/mlx5: keep indirect actions across port 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 7/28/21 2:18 PM, Dmitry Kozlyuk wrote: > Hi Andrew, > >> From: Andrew Rybchenko >> On 7/27/21 10:31 AM, Dmitry Kozlyuk wrote: >>> It was unspecified what happens to indirect actions when a port is >>> stopped, possibly reconfigured, and started again. MLX5 PMD, the first >>> one to use indirect actions, intended to keep them across such a >>> sequence, but the implementation was buggy. Patches 1-3 fix the PMD >>> behavior, patch 4 adds common specification with rationale. >> >> I'm sorry, but it looks very inconsistent. If flow rules are not preserved across >> restart, indirect actions should not be preserved as well. We need very strong >> reasons to introduce the inconsistency. > > Indirect actions really don't need to behave like flow rules. They are just objects owned by the port and they can exist while it exists. Consider a counter: stopping and starting the port doesn't logically affect its state. MLX5 PMD destroys flow rules on port stop for internal reasons and documents this behavior, but ethdev API doesn't require it either. It all sounds bad. All these gray areas just make it hard for DPDK applications to switch from one HW to another. Any rules must not be motivated because of some PMD internal reasons. We should not adjust ethdev rules to fit some PMD behaviour. ethdev rules should be motivated by common sense and convenience from applications point of view. For example, it is strange to preserve indirect RSS action with queues specified across device reconfiguration when queues count may change. I'd say that reconfiguration must drop all indirect actions. However, just stop/start could preserve both indirect actions and flow rues since it could be more convenient from application point of view. If application really wants to remove all flow rules, it can call rte_flow_flush(). The strong reason to flush indirect actions and flow rules across restart is possible actions or rules restore failure on start. However, may be it is sufficient to document that start should really fail, if it can't restore everything and application should retry after rte_flow_flush() taking it into account. >> If we finally accept it, I think it would be very useful to care about PMDs which >> cannot preserve it in HW across restart from the very beginning and save it in >> ethdev layer and restore on start automatically (i.e. do not force all such PMDs >> to care about the restore internally and basically duplicate the code). > > Or keeping indirect actions can be an advertised PMD capability. > Given Ori's comments to patch 4, I think the common spec needs more work. > For this patchset that fixes MLX5 we can have the behavior documented for PMD and not require it from all the drivers. Are you going to drop 4th patch? In general documenting PMD behaviour specifics in its documentation is a wrong direction since it does not help DPDK applications to be portable across different HW.