Message ID | 20210617161825.94741-4-cian.ferriter@intel.com |
---|---|
State | Changes Requested |
Headers | show |
Series | DPIF Framework + Optimizations | expand |
Hello, On Thu, Jun 17, 2021 at 05:18:16PM +0100, Cian Ferriter wrote: > From: Harry van Haaren <harry.van.haaren@intel.com> > > 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@intel.com> > > --- > > v13: > - Minor code refactor to address review comments. > --- > lib/dpif-netdev-private-thread.h | 13 +++++++++++++ > lib/dpif-netdev.c | 7 ++++++- > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/lib/dpif-netdev-private-thread.h b/lib/dpif-netdev-private-thread.h > index 5e5308b96..0d674ab83 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,12 @@ 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; > + > + /* Pointer for per-DPIF implementation scratch space. */ > + void *netdev_input_func_userdata; > + I see you need to switch the input function and the patch looks fine, but the default function doesn't require netdev_input_func_userdata. I think it would be better to add that in the next patch where it is actually required. fbl > struct seq *reload_seq; > uint64_t last_reload_seq; > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index e913f4efc..e6486417e 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -4231,8 +4231,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); > @@ -6029,6 +6030,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; > + > /* init the 'flow_cache' since there is no > * actual thread created for NON_PMD_CORE_ID. */ > if (core_id == NON_PMD_CORE_ID) { > -- > 2.32.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Flavio, Thanks for the review. My responses are inline. Cian > -----Original Message----- > From: Flavio Leitner <fbl@sysclose.org> > Sent: Friday 18 June 2021 13:53 > To: Ferriter, Cian <cian.ferriter@intel.com> > Cc: ovs-dev@openvswitch.org; i.maximets@ovn.org > Subject: Re: [ovs-dev] [v13 03/12] dpif-netdev: Add function pointer for netdev input. > > > Hello, > > On Thu, Jun 17, 2021 at 05:18:16PM +0100, Cian Ferriter wrote: > > From: Harry van Haaren <harry.van.haaren@intel.com> > > > > 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@intel.com> > > > > --- > > > > v13: > > - Minor code refactor to address review comments. > > --- > > lib/dpif-netdev-private-thread.h | 13 +++++++++++++ > > lib/dpif-netdev.c | 7 ++++++- > > 2 files changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/lib/dpif-netdev-private-thread.h b/lib/dpif-netdev-private-thread.h > > index 5e5308b96..0d674ab83 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,12 @@ 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; > > + > > + /* Pointer for per-DPIF implementation scratch space. */ > > + void *netdev_input_func_userdata; > > + > > I see you need to switch the input function and the patch looks fine, but > the default function doesn't require netdev_input_func_userdata. I think > it would be better to add that in the next patch where it is actually > required. > Good point, I'll move netdev_input_func_userdata to the next patch. > fbl > > > struct seq *reload_seq; > > uint64_t last_reload_seq; > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index e913f4efc..e6486417e 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -4231,8 +4231,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); > > @@ -6029,6 +6030,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; > > + > > /* init the 'flow_cache' since there is no > > * actual thread created for NON_PMD_CORE_ID. */ > > if (core_id == NON_PMD_CORE_ID) { > > -- > > 2.32.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > fbl
diff --git a/lib/dpif-netdev-private-thread.h b/lib/dpif-netdev-private-thread.h index 5e5308b96..0d674ab83 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,12 @@ 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; + + /* 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 e913f4efc..e6486417e 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -4231,8 +4231,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); @@ -6029,6 +6030,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; + /* init the 'flow_cache' since there is no * actual thread created for NON_PMD_CORE_ID. */ if (core_id == NON_PMD_CORE_ID) {