> Hi Ian,
>
> Thanks for the review. My responses are inline.
>
> > -----Original Message-----
> > From: Stokes, Ian <[email protected]>
> > Sent: Tuesday 1 June 2021 19:58
> > To: Ferriter, Cian <[email protected]>; [email protected]; Van
> Haaren, Harry <[email protected]>
> > Cc: [email protected]
> > 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 <[email protected]>
> >
> > 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.
+1, that's key in this work for sure.
Thanks
Ian
>
> 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
> > > [email protected]
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
