On Sun, Jul 5, 2020 at 12:56 AM Matan Azrad <ma...@mellanox.com> wrote: > > Hi all > > From: Jerin Jacob: > > On Fri, Jul 3, 2020 at 8:57 PM Thomas Monjalon <tho...@monjalon.net> > > wrote: > > > > > > 03/07/2020 17:08, Jerin Jacob: > > > > On Fri, Jul 3, 2020 at 8:25 PM Matan Azrad <ma...@mellanox.com> > > 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 ? >