[ovs-dev] [v12 03/16] dpif-netdev: Add function pointer for netdev input.

[Ferriter, Cian]

Hi Ian,

Thanks for the review. My responses are inline.

> -----Original Message-----
> From: Stokes, Ian <ian.stokes at intel.com>
> Sent: Tuesday 1 June 2021 19:58
> To: Ferriter, Cian <cian.ferriter at intel.com>; ovs-dev at openvswitch.org; Van Haaren, Harry <harry.van.haaren at intel.com>
> Cc: i.maximets at ovn.org
> Subject: RE: [ovs-dev] [v12 03/16] dpif-netdev: Add function pointer for netdev input.
> 
> > This commit adds a function pointer to the pmd thread data structure,
> > giving the pmd thread flexibility in its dpif-input function choice.
> > This allows choosing of the implementation based on ISA capabilities
> > of the runtime CPU, leading to optimizations and higher performance.
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren at intel.com>
> 
> Thanks for the patch Harry/Cian, a few minor comments below.
> 
> > ---
> >  lib/dpif-netdev-private-thread.h | 12 ++++++++++++
> >  lib/dpif-netdev.c                |  7 ++++++-
> >  2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/dpif-netdev-private-thread.h b/lib/dpif-netdev-private-thread.h
> > index 5e5308b96..01a28a681 100644
> > --- a/lib/dpif-netdev-private-thread.h
> > +++ b/lib/dpif-netdev-private-thread.h
> > @@ -47,6 +47,13 @@ struct dp_netdev_pmd_thread_ctx {
> >      uint32_t emc_insert_min;
> >  };
> >
> > +/* Forward declaration for typedef. */
> > +struct dp_netdev_pmd_thread;
> > +
> > +typedef void (*dp_netdev_input_func)(struct dp_netdev_pmd_thread *pmd,
> > +                                     struct dp_packet_batch *packets,
> > +                                     odp_port_t port_no);
> > +
> >  /* PMD: Poll modes drivers.  PMD accesses devices via polling to eliminate
> >   * the performance overhead of interrupt processing.  Therefore netdev can
> >   * not implement rx-wait for these devices.  dpif-netdev needs to poll
> > @@ -101,6 +108,11 @@ struct dp_netdev_pmd_thread {
> >      /* Current context of the PMD thread. */
> >      struct dp_netdev_pmd_thread_ctx ctx;
> >
> > +    /* Function pointer to call for dp_netdev_input() functionality. */
> > +    dp_netdev_input_func netdev_input_func;
> 
> Probably white space to be added here to improve readability.
> 

Sure, I'll add this in the next version of the patch set.

> > +    /* Pointer for per-DPIF implementation scratch space. */
> > +    void *netdev_input_func_userdata;
> > +
> >      struct seq *reload_seq;
> >      uint64_t last_reload_seq;
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 88f37c505..bec984643 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -4234,8 +4234,9 @@ dp_netdev_process_rxq_port(struct
> > dp_netdev_pmd_thread *pmd,
> >                  }
> >              }
> >          }
> > +
> >          /* Process packet batch. */
> > -        dp_netdev_input(pmd, &batch, port_no);
> > +        pmd->netdev_input_func(pmd, &batch, port_no);
> >
> >          /* Assign processing cycles to rx queue. */
> >          cycles = cycle_timer_stop(&pmd->perf_stats, &timer);
> > @@ -6033,6 +6034,10 @@ dp_netdev_configure_pmd(struct
> > dp_netdev_pmd_thread *pmd, struct dp_netdev *dp,
> >      hmap_init(&pmd->tnl_port_cache);
> >      hmap_init(&pmd->send_port_cache);
> >      cmap_init(&pmd->tx_bonds);
> > +
> > +    /* Initialize the DPIF function pointer to the default scalar version. */
> > +    pmd->netdev_input_func = dp_netdev_input;
> > +
> 
> I like the approach above, this follows what we has previously been implemented for DPCLS.
> 
> It's good the existing scalar is the default as well so there is no change for existing users out of the box.
> 
> BR
> Ian

Yes, the above aims to be extensible for multiple difference implementations of the DPIF.

Thanks,
Cian

> >      /* init the 'flow_cache' since there is no
> >       * actual thread created for NON_PMD_CORE_ID. */
> >      if (core_id == NON_PMD_CORE_ID) {
> > --
> > 2.31.1
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev