Re: [ovs-dev] [PATCH v28 6/8] dpif-netlink: Add netdev offload recv in normal recv upcalls

2024-03-25 Thread Chris Mi via dev 

On 6/24/2023 4:18 AM, Ilya Maximets wrote:

On 6/19/23 07:05, Chris Mi wrote:

In thread handler 0, add netdev offload recv in normal recv upcalls.
To avoid starvation, introduce a flag to alternate the order of
receiving normal upcalls and offload upcalls based on that flag.

Add similar change for recv_wait.

Signed-off-by: Chris Mi 
Reviewed-by: Roi Dayan 
---
  lib/dpif-netlink.c| 41 ++-
  ofproto/ofproto-dpif-upcall.c | 15 +
  2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 60bd39643..6e7b644e8 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -201,6 +201,11 @@ struct dpif_handler {
  struct nl_sock *sock; /* Each handler thread holds one netlink
   socket. */
  
+/* The receive handler thread deals with both normal and offload receive

+ * upcalls. To avoid starvation, the below flag is used to alternate the
+ * processing order. */
+bool recv_offload_first;
+
  #ifdef _WIN32
  /* Pool of sockets. */
  struct dpif_windows_vport_sock *vport_sock_pool;
@@ -3010,7 +3015,6 @@ dpif_netlink_recv_windows(struct dpif_netlink *dpif, 
uint32_t handler_id,
  static int
  dpif_netlink_recv_cpu_dispatch(struct dpif_netlink *dpif, uint32_t handler_id,
 struct dpif_upcall *upcall, struct ofpbuf *buf)
-OVS_REQ_RDLOCK(dpif->upcall_lock)
  {
  struct dpif_handler *handler;
  int read_tries = 0;
@@ -3061,7 +3065,6 @@ dpif_netlink_recv_vport_dispatch(struct dpif_netlink 
*dpif,
   uint32_t handler_id,
   struct dpif_upcall *upcall,
   struct ofpbuf *buf)
-OVS_REQ_RDLOCK(dpif->upcall_lock)
  {
  struct dpif_handler *handler;
  int read_tries = 0;
@@ -3135,13 +3138,12 @@ dpif_netlink_recv_vport_dispatch(struct dpif_netlink 
*dpif,
  #endif
  
  static int

-dpif_netlink_recv(struct dpif *dpif_, uint32_t handler_id,
-  struct dpif_upcall *upcall, struct ofpbuf *buf)
+dpif_netlink_recv__(struct dpif_netlink *dpif, uint32_t handler_id,
+struct dpif_upcall *upcall, struct ofpbuf *buf)
+OVS_REQ_RDLOCK(dpif->upcall_lock)
  {
-struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
  int error;
  
-fat_rwlock_rdlock(>upcall_lock);

  #ifdef _WIN32
  error = dpif_netlink_recv_windows(dpif, handler_id, upcall, buf);
  #else
@@ -3152,6 +3154,32 @@ dpif_netlink_recv(struct dpif *dpif_, uint32_t 
handler_id,
   handler_id, upcall, buf);
  }
  #endif
+
+return error;
+}
+
+static int
+dpif_netlink_recv(struct dpif *dpif_, uint32_t handler_id,
+  struct dpif_upcall *upcall, struct ofpbuf *buf)
+{
+struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+struct dpif_handler *handler;
+int error;
+
+fat_rwlock_rdlock(>upcall_lock);
+handler = >handlers[handler_id];
+if (handler->recv_offload_first) {
+error = netdev_offload_recv(upcall, buf, handler_id);
+if (error == EAGAIN) {
+error = dpif_netlink_recv__(dpif, handler_id, upcall, buf);
+}
+} else {
+error = dpif_netlink_recv__(dpif, handler_id, upcall, buf);
+if (error == EAGAIN) {
+error = netdev_offload_recv(upcall, buf, handler_id);
+}
+}
+handler->recv_offload_first = !handler->recv_offload_first;
  fat_rwlock_unlock(>upcall_lock);
  
  return error;

@@ -3217,6 +3245,7 @@ dpif_netlink_recv_wait(struct dpif *dpif_, uint32_t 
handler_id)
  dpif_netlink_recv_wait_vport_dispatch(dpif, handler_id);
  }
  #endif
+netdev_offload_recv_wait(handler_id);
  fat_rwlock_unlock(>upcall_lock);
  }
  
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c

index 04b583f81..c1fad9a8f 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -855,10 +855,17 @@ recv_upcalls(struct handler *handler)
  break;
  }
  
-upcall->fitness = odp_flow_key_to_flow(dupcall->key, dupcall->key_len,

-   flow, NULL);
-if (upcall->fitness == ODP_FIT_ERROR) {
-goto free_dupcall;
+/* If key and key_len are available, use them to construct flow.
+ * Otherwise, use upcall->flow. */
+if (dupcall->key && dupcall->key_len) {
+upcall->fitness = odp_flow_key_to_flow(dupcall->key,
+   dupcall->key_len,
+   flow, NULL);
+if (upcall->fitness == ODP_FIT_ERROR) {
+goto free_dupcall;
+}
+} else {
+flow = >flow;


Might make sense to set upcall->fitness to ODP_FIT_PERFECT here.
It might not be used today,

Re: [ovs-dev] [PATCH v28 6/8] dpif-netlink: Add netdev offload recv in normal recv upcalls

2023-06-23 Thread Ilya Maximets 
On 6/19/23 07:05, Chris Mi wrote:
> In thread handler 0, add netdev offload recv in normal recv upcalls.
> To avoid starvation, introduce a flag to alternate the order of
> receiving normal upcalls and offload upcalls based on that flag.
> 
> Add similar change for recv_wait.
> 
> Signed-off-by: Chris Mi 
> Reviewed-by: Roi Dayan 
> ---
>  lib/dpif-netlink.c| 41 ++-
>  ofproto/ofproto-dpif-upcall.c | 15 +
>  2 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 60bd39643..6e7b644e8 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -201,6 +201,11 @@ struct dpif_handler {
>  struct nl_sock *sock; /* Each handler thread holds one netlink
>   socket. */
>  
> +/* The receive handler thread deals with both normal and offload receive
> + * upcalls. To avoid starvation, the below flag is used to alternate the
> + * processing order. */
> +bool recv_offload_first;
> +
>  #ifdef _WIN32
>  /* Pool of sockets. */
>  struct dpif_windows_vport_sock *vport_sock_pool;
> @@ -3010,7 +3015,6 @@ dpif_netlink_recv_windows(struct dpif_netlink *dpif, 
> uint32_t handler_id,
>  static int
>  dpif_netlink_recv_cpu_dispatch(struct dpif_netlink *dpif, uint32_t 
> handler_id,
> struct dpif_upcall *upcall, struct ofpbuf 
> *buf)
> -OVS_REQ_RDLOCK(dpif->upcall_lock)
>  {
>  struct dpif_handler *handler;
>  int read_tries = 0;
> @@ -3061,7 +3065,6 @@ dpif_netlink_recv_vport_dispatch(struct dpif_netlink 
> *dpif,
>   uint32_t handler_id,
>   struct dpif_upcall *upcall,
>   struct ofpbuf *buf)
> -OVS_REQ_RDLOCK(dpif->upcall_lock)
>  {
>  struct dpif_handler *handler;
>  int read_tries = 0;
> @@ -3135,13 +3138,12 @@ dpif_netlink_recv_vport_dispatch(struct dpif_netlink 
> *dpif,
>  #endif
>  
>  static int
> -dpif_netlink_recv(struct dpif *dpif_, uint32_t handler_id,
> -  struct dpif_upcall *upcall, struct ofpbuf *buf)
> +dpif_netlink_recv__(struct dpif_netlink *dpif, uint32_t handler_id,
> +struct dpif_upcall *upcall, struct ofpbuf *buf)
> +OVS_REQ_RDLOCK(dpif->upcall_lock)
>  {
> -struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
>  int error;
>  
> -fat_rwlock_rdlock(>upcall_lock);
>  #ifdef _WIN32
>  error = dpif_netlink_recv_windows(dpif, handler_id, upcall, buf);
>  #else
> @@ -3152,6 +3154,32 @@ dpif_netlink_recv(struct dpif *dpif_, uint32_t 
> handler_id,
>   handler_id, upcall, buf);
>  }
>  #endif
> +
> +return error;
> +}
> +
> +static int
> +dpif_netlink_recv(struct dpif *dpif_, uint32_t handler_id,
> +  struct dpif_upcall *upcall, struct ofpbuf *buf)
> +{
> +struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> +struct dpif_handler *handler;
> +int error;
> +
> +fat_rwlock_rdlock(>upcall_lock);
> +handler = >handlers[handler_id];
> +if (handler->recv_offload_first) {
> +error = netdev_offload_recv(upcall, buf, handler_id);
> +if (error == EAGAIN) {
> +error = dpif_netlink_recv__(dpif, handler_id, upcall, buf);
> +}
> +} else {
> +error = dpif_netlink_recv__(dpif, handler_id, upcall, buf);
> +if (error == EAGAIN) {
> +error = netdev_offload_recv(upcall, buf, handler_id);
> +}
> +}
> +handler->recv_offload_first = !handler->recv_offload_first;
>  fat_rwlock_unlock(>upcall_lock);
>  
>  return error;
> @@ -3217,6 +3245,7 @@ dpif_netlink_recv_wait(struct dpif *dpif_, uint32_t 
> handler_id)
>  dpif_netlink_recv_wait_vport_dispatch(dpif, handler_id);
>  }
>  #endif
> +netdev_offload_recv_wait(handler_id);
>  fat_rwlock_unlock(>upcall_lock);
>  }
>  
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 04b583f81..c1fad9a8f 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -855,10 +855,17 @@ recv_upcalls(struct handler *handler)
>  break;
>  }
>  
> -upcall->fitness = odp_flow_key_to_flow(dupcall->key, 
> dupcall->key_len,
> -   flow, NULL);
> -if (upcall->fitness == ODP_FIT_ERROR) {
> -goto free_dupcall;
> +/* If key and key_len are available, use them to construct flow.
> + * Otherwise, use upcall->flow. */
> +if (dupcall->key && dupcall->key_len) {
> +upcall->fitness = odp_flow_key_to_flow(dupcall->key,
> +   dupcall->key_len,
> +   flow, NULL);
> +if (upcall->fitness == ODP_FIT_ERROR) {
> +goto

Re: [ovs-dev] [PATCH v28 6/8] dpif-netlink: Add netdev offload recv in normal recv upcalls

2023-06-19 Thread Eelco Chaudron 



On 19 Jun 2023, at 7:05, Chris Mi wrote:

> In thread handler 0, add netdev offload recv in normal recv upcalls.
> To avoid starvation, introduce a flag to alternate the order of
> receiving normal upcalls and offload upcalls based on that flag.
>
> Add similar change for recv_wait.
>
> Signed-off-by: Chris Mi 
> Reviewed-by: Roi Dayan 

You forgot to include my ACK on v27. So here it is again:

Acked-by: Eelco Chaudron 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v28 6/8] dpif-netlink: Add netdev offload recv in normal recv upcalls

2023-06-18 Thread Chris Mi via dev 
In thread handler 0, add netdev offload recv in normal recv upcalls.
To avoid starvation, introduce a flag to alternate the order of
receiving normal upcalls and offload upcalls based on that flag.

Add similar change for recv_wait.

Signed-off-by: Chris Mi 
Reviewed-by: Roi Dayan 
---
 lib/dpif-netlink.c| 41 ++-
 ofproto/ofproto-dpif-upcall.c | 15 +
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 60bd39643..6e7b644e8 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -201,6 +201,11 @@ struct dpif_handler {
 struct nl_sock *sock; /* Each handler thread holds one netlink
  socket. */
 
+/* The receive handler thread deals with both normal and offload receive
+ * upcalls. To avoid starvation, the below flag is used to alternate the
+ * processing order. */
+bool recv_offload_first;
+
 #ifdef _WIN32
 /* Pool of sockets. */
 struct dpif_windows_vport_sock *vport_sock_pool;
@@ -3010,7 +3015,6 @@ dpif_netlink_recv_windows(struct dpif_netlink *dpif, 
uint32_t handler_id,
 static int
 dpif_netlink_recv_cpu_dispatch(struct dpif_netlink *dpif, uint32_t handler_id,
struct dpif_upcall *upcall, struct ofpbuf *buf)
-OVS_REQ_RDLOCK(dpif->upcall_lock)
 {
 struct dpif_handler *handler;
 int read_tries = 0;
@@ -3061,7 +3065,6 @@ dpif_netlink_recv_vport_dispatch(struct dpif_netlink 
*dpif,
  uint32_t handler_id,
  struct dpif_upcall *upcall,
  struct ofpbuf *buf)
-OVS_REQ_RDLOCK(dpif->upcall_lock)
 {
 struct dpif_handler *handler;
 int read_tries = 0;
@@ -3135,13 +3138,12 @@ dpif_netlink_recv_vport_dispatch(struct dpif_netlink 
*dpif,
 #endif
 
 static int
-dpif_netlink_recv(struct dpif *dpif_, uint32_t handler_id,
-  struct dpif_upcall *upcall, struct ofpbuf *buf)
+dpif_netlink_recv__(struct dpif_netlink *dpif, uint32_t handler_id,
+struct dpif_upcall *upcall, struct ofpbuf *buf)
+OVS_REQ_RDLOCK(dpif->upcall_lock)
 {
-struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
 int error;
 
-fat_rwlock_rdlock(>upcall_lock);
 #ifdef _WIN32
 error = dpif_netlink_recv_windows(dpif, handler_id, upcall, buf);
 #else
@@ -3152,6 +3154,32 @@ dpif_netlink_recv(struct dpif *dpif_, uint32_t 
handler_id,
  handler_id, upcall, buf);
 }
 #endif
+
+return error;
+}
+
+static int
+dpif_netlink_recv(struct dpif *dpif_, uint32_t handler_id,
+  struct dpif_upcall *upcall, struct ofpbuf *buf)
+{
+struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+struct dpif_handler *handler;
+int error;
+
+fat_rwlock_rdlock(>upcall_lock);
+handler = >handlers[handler_id];
+if (handler->recv_offload_first) {
+error = netdev_offload_recv(upcall, buf, handler_id);
+if (error == EAGAIN) {
+error = dpif_netlink_recv__(dpif, handler_id, upcall, buf);
+}
+} else {
+error = dpif_netlink_recv__(dpif, handler_id, upcall, buf);
+if (error == EAGAIN) {
+error = netdev_offload_recv(upcall, buf, handler_id);
+}
+}
+handler->recv_offload_first = !handler->recv_offload_first;
 fat_rwlock_unlock(>upcall_lock);
 
 return error;
@@ -3217,6 +3245,7 @@ dpif_netlink_recv_wait(struct dpif *dpif_, uint32_t 
handler_id)
 dpif_netlink_recv_wait_vport_dispatch(dpif, handler_id);
 }
 #endif
+netdev_offload_recv_wait(handler_id);
 fat_rwlock_unlock(>upcall_lock);
 }
 
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 04b583f81..c1fad9a8f 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -855,10 +855,17 @@ recv_upcalls(struct handler *handler)
 break;
 }
 
-upcall->fitness = odp_flow_key_to_flow(dupcall->key, dupcall->key_len,
-   flow, NULL);
-if (upcall->fitness == ODP_FIT_ERROR) {
-goto free_dupcall;
+/* If key and key_len are available, use them to construct flow.
+ * Otherwise, use upcall->flow. */
+if (dupcall->key && dupcall->key_len) {
+upcall->fitness = odp_flow_key_to_flow(dupcall->key,
+   dupcall->key_len,
+   flow, NULL);
+if (upcall->fitness == ODP_FIT_ERROR) {
+goto free_dupcall;
+}
+} else {
+flow = >flow;
 }
 
 if (dupcall->mru) {
-- 
2.26.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev