From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BE66FA00C5; Mon, 6 Jul 2020 10:37:23 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4D4321DA4F; Mon, 6 Jul 2020 10:37:23 +0200 (CEST) Received: from mail-il1-f195.google.com (mail-il1-f195.google.com [209.85.166.195]) by dpdk.org (Postfix) with ESMTP id 7EA731D723 for ; Mon, 6 Jul 2020 10:37:21 +0200 (CEST) Received: by mail-il1-f195.google.com with SMTP id r12so24972727ilh.4 for ; Mon, 06 Jul 2020 01:37:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=7C+ts7w1kD06hwAus4+H+bcyZL7yLg+4G4axr6cXmSo=; b=PyyOdTM2vuIbhdKLxjv3b5WZIw3dUMmpJyGy8uNJ5pKTVlBnglkSKJhdHw9fqYXhk9 D8bRiilxdO7EgUfARCxj65IBHn7w6PTJRg9pRR8PU/pUUc68ZlHXMtQSPoxxQljBJLY+ O+zoolZgNxXPPJrXtjYX91QwhbYSBXtwZhzF0hyK2dX2KSW2EtlLCntS+x4mLC5qEqG9 2imd9g90CzWbieTm6e4WEg+AP7090MoMBRvBwU9GmHCTJLn/QpnmujPLwE0GVawS/Z/j G5OElk22l5HIo1gC7tsjxFgQ1Zt1CR6Ae1bzAZCMNucgh9VMlXTV4XDdZgosuFzMkl9g nPAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=7C+ts7w1kD06hwAus4+H+bcyZL7yLg+4G4axr6cXmSo=; b=ZQXVquV9oUHgaSgUAGE5EC9uI60KV0y5ALnaSKnO70+7tmoXZF0w02CMF/1m0uLb4G nDqXNz37y/LZoehArl0qJ25atWyICl/bENTlu1cr/lhggYG2x5LrMzp6GVtyuOKwmBLi XtdgzBMDnbNt5ss2bc8pIeBb+X2Q0jWt2hk4O7YK7zWMk0DqgDREBk0yRlExhto5xmUk NoiPwtWMOmq5aBN5Xgw7dJ41wF58l/RDnkXzknsMVormXfFNq91Hj1YNAK3BVF3w7IpZ LELMrUWpN5FaG9dx+6bA8+sTe5Ml2gjTrO+VED7pIuofAEQabhe/Z3/hUJDQE3IXRwlY Tijw== X-Gm-Message-State: AOAM533XJlFqQd48zeG/MNmKUOQddI6aJekyN8fTv85uR/i5erk8y/9T PTemlo+ONU49eKfulY6dzEagpu44Z9i1kdi8pCs= X-Google-Smtp-Source: ABdhPJyvRB5qj4griFExOSl7VFKpucgDBRt6vc5i8psicuqAadCR9p86oReTdKov8CxxT2Ri1uPG67ggtEd902IHSQw= X-Received: by 2002:a92:d206:: with SMTP id y6mr8497961ily.162.1594024640637; Mon, 06 Jul 2020 01:37:20 -0700 (PDT) MIME-Version: 1.0 References: <1593102379-400132-1-git-send-email-jiaweiw@mellanox.com> <2018114.U5Vea0xDAA@thomas> In-Reply-To: From: Jerin Jacob Date: Mon, 6 Jul 2020 14:07:04 +0530 Message-ID: To: Matan Azrad Cc: Thomas Monjalon , "Jiawei(Jonny) Wang" , Ori Kam , Slava Ovsiienko , dpdk-dev , Raslan Darawsheh , "ian.stokes@intel.com" , "fbl@redhat.com" , Ferruh Yigit , Andrew Rybchenko Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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 Sun, Jul 5, 2020 at 10:22 AM Matan Azrad wrote: > > > > From: Jerin Jacob: > > On Sun, Jul 5, 2020 at 12:56 AM Matan Azrad wrote: > > > > > > Hi all > > > > > > From: Jerin Jacob: > > > > On Fri, Jul 3, 2020 at 8:57 PM Thomas Monjalon > > > > wrote: > > > > > > > > > > 03/07/2020 17:08, Jerin Jacob: > > > > > > On Fri, Jul 3, 2020 at 8:25 PM Matan Azrad > > > > wrote: > > > > > > > From: Jerin Jacob: > > > > > > > > When adding overlapping API(rte_eth_mirror_rule_set()) in > > > > > > > > the same library(ethdev). > > > > > > > > Please depreciate the old API. > > > > > > > > We should not have two separate paths for the same function > > > > > > > > in the same ethdev library. It is pain for app and driver d= evelopers. > > > > > > > > > > > > > > What are about all the other rte_flow parallel configuration > > > > > > > APIs in > > > > ethdev: > > > > > > > promiscuous_enable; > > > > > > > promiscuous_disable; > > > > > > > allmulticast_enable; > > > > > > > allmulticast_disable; > > > > > > > mac_addr_remove; > > > > > > > mac_addr_add; > > > > > > > mac_addr_set; > > > > > > > set_mc_addr_list; > > > > > > > vlan_filter_set; > > > > > > > vlan_tpid_set; > > > > > > > vlan_strip_queue_set; > > > > > > > vlan_offload_set; > > > > > > > vlan_pvid_set; > > > > > > > udp_tunnel_port_add; > > > > > > > udp_tunnel_port_del; > > > > > > > ... > > > > > > > > > > > > > > These APIs can be replaced easily by rte_flow API. > > > > > > > Do you think we need to deprecate all? > > > > > > > > > > > > I think, basic stuff like below can have separate API. > > > > > > 1) promiscuous_enable; > > > > > > 2) promiscuous_disable; > > > > > > 3) allmulticast_enable; > > > > > > 4) allmulticast_disable; > > > > > > 5) mac_addr_remove; > > > > > > 6) mac_addr_add; > > > > > > 7) mac_addr_set; > > > > > > 8) set_mc_addr_list; > > > > > > > > > > "Basic" is not a precise definition :) > > > > > > > > Yep. > > > > > > > > > I would say port-level configuration should remain out of rte_flo= w > > > > > API. > > > > > > Thomas, Can you explain what is port-level? > > > Everything in rte_flow is per port... > > > > > > Also, can you give reasons for your claim? > > > > > > > +1. > > > > In addition that, I would say anything needs to configured at "per-= flow" > > > > granularity use rte_flow. > > > > > > Jerin, What do you mean "per-flow" ? > > > > In Terms of NIC HW features, Typical HW will have > > a) Basic "port" level configuration like > > - enable/disable promiscuous > > What is "port level", everything in rte_flow is also per port... > > > b) Advance HW's will have CAM based flow filtering. IMO, CAM related st= uff > > should go to rte_flow. > > It is HW internal, I don't think all HWs use the same logic here. > Since rte_flow is generic for all filtering methods, It is good candidate= API for all HWs. > > > This is to enable, The very basic PMD(without advanced features) shoul= d > > work with port level basic APIs(i.e without rte_flow) > > What is "basic"? Do you mean simple match and simple action? > As I said, Also rte_flow is port level API - so "port level" is not good = term here. > > As you said " When adding overlapping API(rte_eth_mirror_rule_set()) in t= he same library(ethdev). Please depreciate the old API." > > Different APIs to do the same thing is not good, especially in packet fil= tering: > What should we do if we have conflicts? > For example: legacy filtering APIs say to receive packet A and rte_flow s= ays to drop it. > > Don't you think it complicates more the user API understanding, also the = PMD implementations? > > > > I have seen promiscuous, mac address handling is part of basic NIC HW(i= .e > > NICs without advanced CAM filters). > > That's my reasoning for the split. > > As I said, the nic HW specific implementation should not affect the API. > I don't think we need to split API and to complicate the user because of = it. > > IMO, It is better to have one generic API for packet filtering: > It is clearer, simpler, generic and classic. > The user and PMD need to understand only one filtering API and that=E2=80= =99s it (no need to combine between multiple filtering APIs). > > I know this is big change but we can do it in modular way. > It reminds me the big change that was done in Rx\Tx offload configuration= s: > So, when offload became more popular we modularly forced users to replace= the offload API. > Also here, flow filtering becomes popular so maybe this is the time(20.08= -20.11) to force changes in the old APIs. > > > > Everything in traffic filtering\actions is per flow, for example: > > > Promiscuous: flow create 0 ingress pattern eth / end actions queue > > > index 0 / end > > > > IMO, it is not an accurate representation of promiscuous enable. It nee= ds to > > send the traffic to all queues and patterns is not just eth. > > Yes, if legacy RSS is configured we need to replace the above queue actio= n by rss action as I wrote before.(did you read it just below?) > > So, we can add legacy RSS APIs to the list above... I meant, If promiscuous enable, then what would be the pattern, Should be limit just to "eth". I leave up to ethdev maintainer to decide. Is promiscuous part of rte_flow API or not? I dnt have a very strong objection. For me, VLAN(rte_vlan_*) and MIRROR(rte_eth_mirror_rule_set()) are most worrisome as each PMD need to duplicate that work as both are CAM based API.