diff mbox series

[ovs-dev,v13,03/12] dpif-netdev: Add function pointer for netdev input.

Message ID 20210617161825.94741-4-cian.ferriter@intel.com
State Changes Requested
Headers show
Series DPIF Framework + Optimizations | expand

Commit Message

Ferriter, Cian June 17, 2021, 4:18 p.m. UTC 
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(-)

Comments

Flavio Leitner June 18, 2021, 12:53 p.m. UTC | #1 
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
Ferriter, Cian June 21, 2021, 4:10 p.m. UTC | #2 
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 mbox series

Patch

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) {