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 53445A0524; Sun, 5 Jul 2020 03:21:23 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 681621DAFF; Sun, 5 Jul 2020 03:21:22 +0200 (CEST) Received: from mail-il1-f193.google.com (mail-il1-f193.google.com [209.85.166.193]) by dpdk.org (Postfix) with ESMTP id AC6991D70F for ; Sun, 5 Jul 2020 03:21:20 +0200 (CEST) Received: by mail-il1-f193.google.com with SMTP id s21so14988259ilk.5 for ; Sat, 04 Jul 2020 18:21:20 -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; bh=5WC5AmtNEN6nI8FxNu4OY2Ez3lCAYJ8M/PB5RmK+1/s=; b=JrsiNYEflumYVUeIwUmzEuQDWAW9d6QyBdEoF9dLFubqFu7mxkCEQ3q6EJC4HFIQNZ IF3EXJAmY3L1WPVhEepE09VZeSkqarecAmzguWoE4fTD2f2Gujdfs2QS/pmg3zuVTWyB 9KWI4cGH2VjQMsafbIS6hH3RkWuAp5XEismfHYl2cWaEfzB4D7TLfwTLHYY1j0QQE4ne zwEC1J1Qhd83nTjvTgQi0qH5Am+FKSHgo8gpPv672QNemfFdP/guFl4XJOL7NkX9Uv12 0lc/iied/wqdiuVey7uPKXUg869XM/kYRPxGTWxNvDJJwiQCJIE9mV+keK91pMLCMpNZ Voqw== 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; bh=5WC5AmtNEN6nI8FxNu4OY2Ez3lCAYJ8M/PB5RmK+1/s=; b=dQizN/F9F0ZQBA/pClqu0UvyqD+pW4HnroF4vmUTugtvJJy+xgYWTp/voYO9oVg/RV i2o6iqE2ALwX7yFFiJf1bswLSk2T0T6VlmAWTjx6V6uIdYHoEORsXSZOeAEPE9aZ4lQd gqXLfqkDGztFrQC9AS7AD8wL3JSmwYQrG3Wx/cIog5fyD43IXQ/fMjjGrET2Qgal8AYh 6Q/zf/E+3aH7aeJs6VchN6rPjXeJFndu5BjFPNrjEw33nT1OttPBmFGET0dA3kiQnQtu l99osaUckoEXDcsQPijgb+3TE2N6d3Ykb9MUh5ps3xi5gO743QP7z0R6oXl54hOMGV18 qFOg== X-Gm-Message-State: AOAM5311vMbJWd1AUJJUjjvShsvV6BR6PXYECHN5UUj7nObxqaTrwSXE DQTNNe9wWF7ie4mpzOee8tP2s2mLWmL0TOVpFIk= X-Google-Smtp-Source: ABdhPJzXmzg6SOyZgFI8zxDEodGSoiy0z+U8CJj41T9RGLzCWHsFoOR/gP+tEfrYQUsB33rI1ZZAna1J4vbpwVLvlys= X-Received: by 2002:a05:6e02:eb3:: with SMTP id u19mr25154068ilj.130.1593912079968; Sat, 04 Jul 2020 18:21:19 -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: Sun, 5 Jul 2020 06:51:03 +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" 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 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 developers. > > > > > > > > > > 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_flow > > > 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 b) Advance HW's will have CAM based flow filtering. IMO, CAM related stuff should go to rte_flow. This is to enable, The very basic PMD(without advanced features) should work with port level basic APIs(i.e without rte_flow) 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. > 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 needs to send the traffic to all queues and patterns is not just eth. > Multicast\mac related: flow create 0 ingress pattern eth dst is X /end actions queue 0/ end > (in case of legacy RSS queue action will be replaced by rss). > .... > > Everything here are flows. > > > > > > > > But The VLAN and UDP related should be rte_flow candidates.(IMO) > > > > > > Yes definitely, tunneling is better managed with rte_flow. > > Can you give reasons for your claim? > Why should Vlan\Tunnel be in rte_flow and promiscuous\Multicast\mac not? > > > > This is an important discussion, I Cc other ethdev maintainers. > > Agree, this is a good trigger to open this important discussion. > > > > Note: this is an ethdev patch, all ethdev maintainers should be Cc'ed. > > > Aren't you using --cc-cmd devtools/get-maintainer.sh ? >