Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow

[Jerin Jacob <jerinjacobk@gmail.com>]

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