On 29 Mar 2023, at 13:42, 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.
One small comment inline. //Eelco > Add similar change for recv_wait. > > Signed-off-by: Chris Mi <[email protected]> > Reviewed-by: Roi Dayan <[email protected]> > --- > 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 ebe7b5cb1..4e56922f8 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; > @@ -3005,7 +3010,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; > @@ -3056,7 +3060,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; > @@ -3130,13 +3133,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(&dpif->upcall_lock); > #ifdef _WIN32 > error = dpif_netlink_recv_windows(dpif, handler_id, upcall, buf); > #else > @@ -3147,6 +3149,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(&dpif->upcall_lock); > + handler = &dpif->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(&dpif->upcall_lock); > > return error; > @@ -3211,6 +3239,7 @@ dpif_netlink_recv_wait(struct dpif *dpif_, uint32_t > handler_id) > } else { > dpif_netlink_recv_wait_vport_dispatch(dpif, handler_id); > } > + netdev_offload_recv_wait(handler_id); > #endif Should this netdev_offload_recv_wait(handler_id) not be called outside of the '#ifdef _WIN32' as it should not depend on Windows not being the implementation? > fat_rwlock_unlock(&dpif->upcall_lock); > } > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index cd57fdbd9..0b3ef30a0 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 = &dupcall->flow; > } > > if (dupcall->mru) { > -- > 2.26.3 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
