[ovs-dev] [PATCH v5 1/3] netdev-dpdk: Expose flow creation/destruction calls

[Ophir Munk]

> -----Original Message-----
> From: Ilya Maximets <i.maximets at samsung.com>
> Sent: Monday, March 4, 2019 4:30 PM
> To: Ophir Munk <ophirmu at mellanox.com>; ovs-dev at openvswitch.org
> Cc: Asaf Penso <asafp at mellanox.com>; Ian Stokes <ian.stokes at intel.com>;
> Shahaf Shuler <shahafs at mellanox.com>; Olga Shern
> <olgas at mellanox.com>; Kevin Traynor <ktraynor at redhat.com>; Roni Bar
> Yanai <roniba at mellanox.com>
> Subject: Re: [PATCH v5 1/3] netdev-dpdk: Expose flow creation/destruction
> calls
> 
> On 28.02.2019 17:33, Ophir Munk wrote:
> > From: Roni Bar Yanai <roniba at mellanox.com>
> >
> > Before offloading code was added to the netdev-dpdk.c file (MARK and
> > RSS actions) the only DPDK RTE calls in use were rte_flow_create() and
> > rte_flow_destroy(). In preparation for splitting the offloading code
> > from the netdev-dpdk.c file to a separate file, it is required to
> > embed these RTE calls into a global netdev-dpdk-* API so that they can
> > be called from the new file. An example for this requirement can be
> > seen in the handling of dev->mutex, which should be encapsulated
> > inside netdev-dpdk class (netdev-dpdk.c file), and should be unknown
> > to the outside callers. This commit embeds the rte_flow_create() call
> > inside the netdev_dpdk_flow_create() API and the rte_flow_destroy()
> > call inside the netdev_dpdk_rte_flow_destroy() API.
> >
> > Reviewed-by: Asaf Penso <asafp at mellanox.com>
> > Signed-off-by: Roni Bar Yanai <roniba at mellanox.com>
> > Signed-off-by: Ophir Munk <ophirmu at mellanox.com>
> > ---
> >  lib/netdev-dpdk.c | 44 ++++++++++++++++++++++++++++++++------------
> >  lib/netdev-dpdk.h | 17 +++++++++++++++++
> >  2 files changed, 49 insertions(+), 12 deletions(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > 4f9ca3b..d9a316c 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -4203,6 +4203,35 @@ unlock:
> >      return err;
> >  }
> >
> > +int
> > +netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
> > +                             struct rte_flow *rte_flow,
> > +                             struct rte_flow_error *error) {
> > +    struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> > +    int ret;
> > +
> > +    ovs_mutex_lock(&dev->mutex);
> > +    ret = rte_flow_destroy(dev->port_id, rte_flow, error);
> > +    ovs_mutex_unlock(&dev->mutex);
> > +    return ret;
> > +}
> > +
> > +struct rte_flow *
> > +netdev_dpdk_rte_flow_create(struct netdev *netdev,
> > +                            const struct rte_flow_attr *attr,
> > +                            const struct rte_flow_item *items,
> > +                            const struct rte_flow_action *actions,
> > +                            struct rte_flow_error *error) {
> > +    struct rte_flow *flow;
> > +    struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> > +
> > +    ovs_mutex_lock(&dev->mutex);
> > +    flow = rte_flow_create(dev->port_id, attr, items, actions, error);
> > +    ovs_mutex_unlock(&dev->mutex);
> > +    return flow;
> > +}
> >
> >  /* Find rte_flow with @ufid */
> >  static struct rte_flow *
> > @@ -4554,7 +4583,6 @@ netdev_dpdk_add_rte_flow_offload(struct
> netdev *netdev,
> >                                   size_t actions_len OVS_UNUSED,
> >                                   const ovs_u128 *ufid,
> >                                   struct offload_info *info) {
> > -    struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> >      const struct rte_flow_attr flow_attr = {
> >          .group = 0,
> >          .priority = 0,
> > @@ -4726,15 +4754,12 @@ netdev_dpdk_add_rte_flow_offload(struct
> netdev *netdev,
> >      mark.id = info->flow_mark;
> >      add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark);
> >
> > -    ovs_mutex_lock(&dev->mutex);
> >
> 
> 2 empty lines will be here after removing.
> And these 2 empty lines copied in patch #2 to the new file.
> 

Thanks for noticing that. V6 sent

> >      rss = add_flow_rss_action(&actions, netdev);
> >      add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL);
> >
> > -    flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items,
> > -                           actions.actions, &error);
> > -
> > -    ovs_mutex_unlock(&dev->mutex);
> > +    flow = netdev_dpdk_rte_flow_create(netdev,
> &flow_attr,patterns.items,
> > +                                       actions.actions, &error);
> >
> >      free(rss);
> >      if (!flow) {