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 9F37EA0524; Tue, 1 Jun 2021 17:10:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1DC7640689; Tue, 1 Jun 2021 17:10:24 +0200 (CEST) Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) by mails.dpdk.org (Postfix) with ESMTP id 7E5D140041 for ; Tue, 1 Jun 2021 17:10:23 +0200 (CEST) Received: by mail-pf1-f179.google.com with SMTP id y15so11596747pfn.13 for ; Tue, 01 Jun 2021 08:10:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=EgrrDp309oWSNL8xxl25esk7zI5k40JGZLM8YlI2OpI=; b=igX7B2x80TXBxtDtGE+12bu31GntEHEwPjxq+0oioJNT9mN0lq/ACzIvxZFSKUhfVf mK2sSznRjWy5RtrBJF2RF9BjZ+ms/bRL/3ST81KerWW3bWjvmSp/JjlWcVefh1G2IpAh 6tcD2ggidPlF8KCzuDw/4yMh727TLPyqwXvhfbnMwiN1aCl4At2g9CEKrDdebF6B8gdO 7X0MqLbiZxf7/xnp5RXt0tojSkNjxg3gS/gTbLSKXChex9NGEUBxw7FhyKFeDJ2JAEur YxP1RhQaszlgoL6BVFSk2z1LZDCZDPfPv3A+zqA8LsNc12vheMnGu0Pw2JAqnIct859l GJ1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=EgrrDp309oWSNL8xxl25esk7zI5k40JGZLM8YlI2OpI=; b=oL6HUBXRLfDQoXKthww/y0gYD09GQ4pPY7VCLWC5KQfRRqEIfN+dNgH7hO1txa0mHy cB4i2qETk9jYlayjxnuSrDsYAfySJkBCmPE8YbGMcWMUivW1+agBxP5SsWEd1h9sbJ5G tR0D0n4iP7G+LNNF6uUSQpQ4f18xulUrIIDEpmLh2vhL5fKzA+Ebs24Y5idSrRTobNJH 4rvXV3z8hhBRVHyo8kpXV+Nd6waPaWqHxAMAn6Egpvgpy1HM2HH+s7WhYEiF3Fsw1oOE jU83vUWgp2Ak7uek5JFyKddA1XEarRCg/deWCDNtGfN4ClLkZWjqX3vblbIQoL/oMQ45 7Y7Q== X-Gm-Message-State: AOAM533j/QDD3mQF0riDC2AejfFtR5qhpHcjq0t9pxhqcMrSnKWyeu/F +0iRLLcwOcqSvPpWH44Ih+N2wQ== X-Google-Smtp-Source: ABdhPJxdoL8Fah/HAreRgEuKMZRoGUEIGr9kPq11cuhyUCcReUe3ud5YefYNqvi7N+o936OX2mN/WA== X-Received: by 2002:a63:2cd4:: with SMTP id s203mr15808069pgs.417.1622560222542; Tue, 01 Jun 2021 08:10:22 -0700 (PDT) Received: from hermes.local (76-14-218-44.or.wavecable.com. [76.14.218.44]) by smtp.gmail.com with ESMTPSA id o7sm14991749pgs.45.2021.06.01.08.10.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Jun 2021 08:10:22 -0700 (PDT) Date: Tue, 1 Jun 2021 08:10:19 -0700 From: Stephen Hemminger To: Ivan Malov Cc: Ori Kam , "dev@dpdk.org" , NBU-Contact-Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Ray Kinsella , Neil Horman Message-ID: <20210601081019.0099bf27@hermes.local> In-Reply-To: <949c8087-501f-5618-6fda-2b856ba0ec33@oktetlabs.ru> References: <20210527082504.3495-1-ivan.malov@oktetlabs.ru> <20210530192805.71f5a8ef@hermes.local> <949c8087-501f-5618-6fda-2b856ba0ec33@oktetlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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" On Tue, 1 Jun 2021 17:17:24 +0300 Ivan Malov wrote: > 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. > > > One other thing. Code for this kind of thing grows like a weed. It would be good to change from if/else/switch to a more table driven approach.