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. 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
