[ovs-dev] [optimize 06/13] dpif-linux: Only ask datapath to echo back results when they will be used.

[Ethan Jackson]

Looks good to me.

Ethan

On Tue, Sep 27, 2011 at 16:27, Ben Pfaff <blp at nicira.com> wrote:
> A fair number of datapath flow operations optionally report back results
> to the requester based on whether NLM_F_ECHO is set in the request.  When
> userspace isn't going to use those results anyway, it wastes memory to
> store them and a system call to retrieve them.
>
> This commit omits the NLM_F_ECHO bit in cases where the caller isn't going
> to use the results.
>
> (NLM_F_ECHO has no effect on operations whose entire purpose is to retrieve
> data, e.g. "get" and "dump" operations, so we need not bother to set it
> for those.)
>
> This improves "ovs-benchmark rate" results in my testing by about 4%.
> ---
>  lib/dpif-linux.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
> index 97b8144..84d224e 100644
> --- a/lib/dpif-linux.c
> +++ b/lib/dpif-linux.c
> @@ -125,7 +125,7 @@ static int dpif_linux_flow_from_ofpbuf(struct dpif_linux_flow *,
>                                        const struct ofpbuf *);
>  static void dpif_linux_flow_to_ofpbuf(const struct dpif_linux_flow *,
>                                       struct ofpbuf *);
> -static int dpif_linux_flow_transact(const struct dpif_linux_flow *request,
> +static int dpif_linux_flow_transact(struct dpif_linux_flow *request,
>                                     struct dpif_linux_flow *reply,
>                                     struct ofpbuf **bufp);
>  static void dpif_linux_flow_get_stats(const struct dpif_linux_flow *,
> @@ -1734,7 +1734,7 @@ dpif_linux_flow_to_ofpbuf(const struct dpif_linux_flow *flow,
>     struct ovs_header *ovs_header;
>
>     nl_msg_put_genlmsghdr(buf, 0, ovs_flow_family,
> -                          NLM_F_REQUEST | NLM_F_ECHO | flow->nlmsg_flags,
> +                          NLM_F_REQUEST | flow->nlmsg_flags,
>                           flow->cmd, 1);
>
>     ovs_header = ofpbuf_put_uninit(buf, sizeof *ovs_header);
> @@ -1775,7 +1775,7 @@ dpif_linux_flow_init(struct dpif_linux_flow *flow)
>  * stored in '*reply' and '*bufp'.  The caller must free '*bufp' when the reply
>  * is no longer needed ('reply' will contain pointers into '*bufp'). */
>  static int
> -dpif_linux_flow_transact(const struct dpif_linux_flow *request,
> +dpif_linux_flow_transact(struct dpif_linux_flow *request,
>                          struct dpif_linux_flow *reply, struct ofpbuf **bufp)
>  {
>     struct ofpbuf *request_buf;
> @@ -1783,6 +1783,10 @@ dpif_linux_flow_transact(const struct dpif_linux_flow *request,
>
>     assert((reply != NULL) == (bufp != NULL));
>
> +    if (reply) {
> +        request->nlmsg_flags |= NLM_F_ECHO;
> +    }
> +
>     request_buf = ofpbuf_new(1024);
>     dpif_linux_flow_to_ofpbuf(request, request_buf);
>     error = nl_sock_transact(genl_sock, request_buf, bufp);
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>