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

[Matan Azrad <matan@mellanox.com>]

From: Jerin Jacob
> On Fri, Jul 3, 2020 at 8:25 PM Matan Azrad <matan@mellanox.com> wrote:
> >
> >
> > Hi Jerin
> 
> Hi Matan,
> 
> >
> > From: Jerin Jacob:
> > > On Fri, Jul 3, 2020 at 12:13 AM Jiawei Wang <jiaweiw@mellanox.com>
> wrote:
> > > >
> > > > When using full offload, all traffic will be handled by the HW,
> > > > and directed to the requested vf or wire, the control application
> > > > loses visibility on the traffic.
> > > > So there's a need for an action that will enable the control
> > > > application some visibility.
> > > >
> > > > The solution is introduced a new action that will sample the
> > > > incoming traffic and send a duplicated traffic in some predefined
> > > > ratio to the application, while the original packet will continue
> > > > to the target destination.
> > > >
> > > > The packets sampled equals is '1/ratio', if the ratio value be set
> > > > to
> > > > 1 , means that the packets would be completely mirrored. The
> > > > sample packet can be assigned with different set of actions from
> > > > the original
> > > packet.
> > > >
> > > > In order to support the sample packet in rte_flow, new rte_flow
> > > > action definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
> > > > rte_flow_action_sample will be introduced.
> > > >
> > > > Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
> > > > Acked-by: Ori Kam <orika@mellanox.com>
> > >
> > > 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;
> 
> But The VLAN and UDP related should be rte_flow candidates.(IMO)

Can you explain why?
Each one of them can be configured by rte_flow, so why do we need duplicate ways to configure the same thing.

What is the expected behavior if there are conflicts? Is it documented?

It is probably different discussion, but I think that rte_flow becomes more popular and it will be makes sense to concentrate all the traffic filtering/actions to the rte_flow.

 
> >
> > > With the above deprecation notice,
> > > Acked-by: Jerin Jacob <jerinj@marvell.com>