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 F29C3A0524; Tue, 1 Jun 2021 16:17:31 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8582840DF7; Tue, 1 Jun 2021 16:17:31 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 6E22C40689 for ; Tue, 1 Jun 2021 16:17:30 +0200 (CEST) Received: from [192.168.45.100] (unknown [188.242.7.54]) (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 D5EEF7F4FD; Tue, 1 Jun 2021 17:17:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru D5EEF7F4FD DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1622557049; bh=Oo0qTUm8QBPF21KXZr19R10av/Oe1hYlzJlhTs8MgEk=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=eQxilDZR3mzF4IEz66r175aovPUs/9CIw1mqTbkKJHoRZDXnUZWuYA1qHZhCoo59o lbPycdr6FMrRYo2a/10ZTaPQVnZL4CmZybXV+cQYha8GBeXQ/LLXr5j8P8/MPIy/Ag YVcul7qUWiPPEjtayOU90c1RINAjpijafvL1UpKc= To: Stephen Hemminger , Ori Kam Cc: "dev@dpdk.org" , NBU-Contact-Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Ray Kinsella , Neil Horman References: <20210527082504.3495-1-ivan.malov@oktetlabs.ru> <20210530192805.71f5a8ef@hermes.local> From: Ivan Malov Message-ID: <949c8087-501f-5618-6fda-2b856ba0ec33@oktetlabs.ru> Date: Tue, 1 Jun 2021 17:17:24 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <20210530192805.71f5a8ef@hermes.local> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: add support for testpmd-compliant flow rule dumping 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" Hi Stephen, I agree that the API rte_flow_snprint() itself would look better if it provided the number of characters in its return value, like snprintf does. However, with respect to all internal helpers, this wouldn't be that clear and simple: one would have to update the buffer pointer and decrease the buffer size before each internal (smaller) helper invocation. That would make the code more cumbersome in many places. In v2, I will at least try to make the main API return the number of characters. Other than that, it can be discussed further. Thank you. On 31/05/2021 05:28, Stephen Hemminger wrote: > On Sun, 30 May 2021 07:27:32 +0000 > Ori Kam wrote: > >>> >>> DPDK applications (for example, OvS) or tests which use RTE flow API need to >>> log created or rejected flow rules to help to recognise what goes right or >>> wrong. From this standpoint, testpmd-compliant format is nice for the >>> purpose because it allows to copy-paste the flow rules and debug using >>> testpmd. >>> >>> Recognisable pattern items: >>> VOID, VF, PF, PHY_PORT, PORT_ID, ETH, VLAN, IPV4, IPV6, UDP, TCP, VXLAN, >>> NVGRE, GENEVE, MARK, PPPOES, PPPOED. >>> >>> Recognisable actions: >>> VOID, JUMP, MARK, FLAG, QUEUE, DROP, COUNT, RSS, PF, VF, PHY_PORT, >>> PORT_ID, OF_POP_VLAN, OF_PUSH_VLAN, OF_SET_VLAN_VID, >>> OF_SET_VLAN_PCP, VXLAN_ENCAP, VXLAN_DECAP. >>> >>> Recognisable RSS types (action RSS): >>> IPV4, FRAG_IPV4, NONFRAG_IPV4_TCP, NONFRAG_IPV4_UDP, >>> NONFRAG_IPV4_OTHER, IPV6, FRAG_IPV6, NONFRAG_IPV6_TCP, >>> NONFRAG_IPV6_UDP, NONFRAG_IPV6_OTHER, IPV6_EX, IPV6_TCP_EX, >>> IPV6_UDP_EX, L3_SRC_ONLY, L3_DST_ONLY, L4_SRC_ONLY, L4_DST_ONLY. >>> >>> Unrecognised parts of the flow specification are represented by tokens >>> "{unknown}" and "{unknown bits}". Interested parties are welcome to >>> extend this tool to recognise more items and actions. >>> >>> Signed-off-by: Ivan Malov >>> --- >>> lib/ethdev/meson.build | 1 + >>> lib/ethdev/rte_flow.h | 33 + >>> lib/ethdev/rte_flow_snprint.c | 1681 >>> +++++++++++++++++++++++++++++++++ >>> lib/ethdev/version.map | 3 + >>> 4 files changed, 1718 insertions(+) >>> create mode 100644 lib/ethdev/rte_flow_snprint.c >>> >>> diff --git a/lib/ethdev/meson.build b/lib/ethdev/meson.build index >>> 0205c853df..97bba4fa1b 100644 >>> --- a/lib/ethdev/meson.build >>> +++ b/lib/ethdev/meson.build >>> @@ -8,6 +8,7 @@ sources = files( >>> 'rte_class_eth.c', >>> 'rte_ethdev.c', >>> 'rte_flow.c', >>> + 'rte_flow_snprint.c', >>> 'rte_mtr.c', >>> 'rte_tm.c', >>> ) >>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index >>> 961a5884fe..cd5e9ef631 100644 >>> --- a/lib/ethdev/rte_flow.h >>> +++ b/lib/ethdev/rte_flow.h >>> @@ -4288,6 +4288,39 @@ rte_flow_tunnel_item_release(uint16_t port_id, >>> struct rte_flow_item *items, >>> uint32_t num_of_items, >>> struct rte_flow_error *error); >>> + >>> +/** >>> + * @warning >>> + * @b EXPERIMENTAL: this API may change without prior notice >>> + * >>> + * Dump testpmd-compliant textual representation of the flow rule. >>> + * Invoke this with zero-size buffer to learn the string size and >>> + * invoke this for the second time to actually dump the flow rule. >>> + * The buffer size on the second invocation = the string size + 1. >>> + * >>> + * @param[out] buf >>> + * Buffer to save the dump in, or NULL >>> + * @param buf_size >>> + * Buffer size, or 0 >>> + * @param[out] nb_chars_total >>> + * Resulting string size (excluding the terminating null byte) >>> + * @param[in] attr >>> + * Flow rule attributes. >>> + * @param[in] pattern >>> + * Pattern specification (list terminated by the END pattern item). >>> + * @param[in] actions >>> + * Associated actions (list terminated by the END action). >>> + * >>> + * @return >>> + * 0 on success, a negative errno value otherwise >>> + */ >>> +__rte_experimental >>> +int >>> +rte_flow_snprint(char *buf, size_t buf_size, size_t *nb_chars_total, >>> + const struct rte_flow_attr *attr, >>> + const struct rte_flow_item pattern[], >>> + const struct rte_flow_action actions[]); >>> + > > The code would be clearer and simpler if you adopted the same return value > as snprintf. Then lots of places could be just tail calls and the nb_chars_total > would be unnecessary. > -- Ivan M