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 286DAA0C4C; Mon, 4 Oct 2021 13:08:20 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A1A0C41321; Mon, 4 Oct 2021 13:08:19 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 0676141313 for ; Mon, 4 Oct 2021 13:08:19 +0200 (CEST) Received: from [100.65.5.102] (unknown [5.144.122.214]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 960E97F596; Mon, 4 Oct 2021 14:08:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 960E97F596 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1633345698; bh=1FeUjCyUFx9DQU9IJel38cxGMsOBVioyYf62EEFl+f0=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=NhAy1mwVO9qxtqfLaA1mEBjkODlBIoqWZ9X1OnGQM51TffIJlx+mVfcZ9y8OYmGdG OXg+cZNqN2TmNq5edJXZmiuz0EQqBCbpQmzVYAanmF4ru3ZNqDTav1VFlmkIgvLrT7 XNLrPwXwp5fJNOmdVz6luzSh+9h0xt3wa1uLnUJc= To: Andrew Rybchenko , Ori Kam , Xiaoyun Li , NBU-Contact-Thomas Monjalon , Ferruh Yigit Cc: "dev@dpdk.org" References: <20210907125157.3843-1-ivan.malov@oktetlabs.ru> <20211001134716.1608857-1-andrew.rybchenko@oktetlabs.ru> <20211001134716.1608857-2-andrew.rybchenko@oktetlabs.ru> <5022fd56-4493-aa45-74d9-4feea9ee1fd7@oktetlabs.ru> <18b7250a-3e69-056e-2b29-bc827c0a94bc@oktetlabs.ru> From: Ivan Malov Message-ID: <660161a9-91ae-89e1-5c7a-23eaffe16ef0@oktetlabs.ru> Date: Mon, 4 Oct 2021 14:08:07 +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: <18b7250a-3e69-056e-2b29-bc827c0a94bc@oktetlabs.ru> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v1 01/12] ethdev: add ethdev item to flow API 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 Andrew, Ori, On 04/10/2021 13:47, Andrew Rybchenko wrote: > On 10/4/21 3:00 AM, Ivan Malov wrote: >> Hi Ori, >> >> On 04/10/2021 00:09, Ori Kam wrote: >>> Hi Ivan, >>> >>>> -----Original Message----- >>>> From: Ivan Malov >>>> Subject: Re: [PATCH v1 01/12] ethdev: add ethdev item to flow API >>>> >>>> Hi Ori, >>>> >>>> On 03/10/2021 14:52, Ori Kam wrote: >>>>> Hi Andrew and Ivan, >>>>> >>>>>> -----Original Message----- >>>>>> From: Andrew Rybchenko >>>>>> Sent: Friday, October 1, 2021 4:47 PM >>>>>> Subject: [PATCH v1 01/12] ethdev: add ethdev item to flow API >>>>>> >>>>>> From: Ivan Malov >>>>>> >>>>>> For use with "transfer" flows. Supposed to match traffic transmitted >>>>>> by the DPDK application via the specified ethdev, at e-switch level. >>>>>> >>>>>> Must not be combined with attributes "ingress" / "egress". >>>>>> >>>>>> Signed-off-by: Ivan Malov >>>>>> Signed-off-by: Andrew Rybchenko >>>>>> --- >>>>> >>>>> [Snip] >>>>> >>>>>>    /** Generate flow_action[] entry. */ diff --git >>>>>> a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index >>>>>> 7b1ed7f110..880502098e 100644 >>>>>> --- a/lib/ethdev/rte_flow.h >>>>>> +++ b/lib/ethdev/rte_flow.h >>>>>> @@ -574,6 +574,15 @@ enum rte_flow_item_type { >>>>>>         * @see struct rte_flow_item_conntrack. >>>>>>         */ >>>>>>        RTE_FLOW_ITEM_TYPE_CONNTRACK, >>>>>> + >>>>>> +    /** >>>>>> +     * [META] >>>>>> +     * >>>>>> +     * Matches traffic at e-switch going from (sent by) the given >>>>>> ethdev. >>>>>> +     * >>>>>> +     * @see struct rte_flow_item_ethdev >>>>>> +     */ >>>>>> +    RTE_FLOW_ITEM_TYPE_ETHDEV, >>>>>>    }; >>>>>> >>>>>>    /** >>>>>> @@ -1799,6 +1808,24 @@ static const struct rte_flow_item_conntrack >>>>>> rte_flow_item_conntrack_mask = {  };  #endif >>>>>> >>>>>> +/** >>>>>> + * @warning >>>>>> + * @b EXPERIMENTAL: this structure may change without prior notice >>>>>> + * >>>>>> + * Provides an ethdev ID for use with items which are as follows: >>>>>> + * RTE_FLOW_ITEM_TYPE_ETHDEV. >>>>>> + */ >>>>>> +struct rte_flow_item_ethdev { >>>>>> +    uint16_t id; /**< Ethdev ID */ >>>>> >>>>> True for all above uses, >>>>> should this be renamed to port? >>>> >>>> I'd not rename it to "port". The very idea of this series is to >>>> disambiguate >>>> things. This structure is shared between primitives ETHDEV and >>>> ESWITCH_PORT. If we go for "port", then in conjunction with ESWITCH_PORT >>>> the structure name may trick readers into thinking that the ID in >>>> question is >>>> the own ID of the e-switch port itself. But in fact this ID is an >>>> ethdev ID which >>>> is associated with the e-switch port. >>>> >>>> Should you wish to elaborate on your concerns with regard to naming, >>>> please >>>> do so. I'm all ears. >>>> >>> Fully understand and agree that the idea is to clear the ambiguaty. >>> My concern is that we don't use ethdev id, from application ethdev has >>> only >>> ports, so what is the id? (if we keep this, we should document that >>> the id is the >>> port) >>> What about ETHDEV_PORT and ESWITCH_PORT? >> >> I understand that, technically, the only ports which the application can >> really interface with are ethdevs. So, terms "ethdev" and "port" may >> appear synonymous to the application - you are right on that. But, given >> the fact that we have some primitives like PHY_PORT and the likes, which >> also have "PORT" in their names, I'd rather go for "ethdev" as more >> precise term. >> >> But let me assure you: I'm not saying that my opinion should prevail. >> I'm giving more thoughts to this in the background. Maybe Andrew can >> join this conversation as well. > > As far as I can see ethdev API uses 'port_id' everywhere to > refer to ethdev port by its number. So, I suggest > > struct rte_flow_item_ethdev { > uint16_t port_id; /**< ethdev port ID */ > }; > > Basically I agree with Ori, that just "id" is a bit confusing > even when it is a member of the _ethdev structure, but I'd > prepend "port_" a field name to sync with ethdev API which > uses port_id. So, we have ethdev->port_id. Ack > > Andrew. > -- Ivan M