Peng He <[email protected]> writes:
> by using ipf_list's key instead of first frags' metadata can reduce
> quite a lot of cache access as by the time calling ipf_ctx_eq, ipf_list
> is cache-hot.
>
> Signed-off-by: Peng He <[email protected]>
> ---
Is there a reason not to just fold this into 1/3? It's strange to
introduce something and immediately re-write it in the same series.
I don't see a reason not to fold this into 1/3. The performance
optimization isn't difficult to understand, and doesn't seem awkward.
> lib/ipf.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/lib/ipf.c b/lib/ipf.c
> index 1fd3d8d30..26de7bbcf 100644
> --- a/lib/ipf.c
> +++ b/lib/ipf.c
> @@ -1047,14 +1047,17 @@ ipf_send_frags_in_list(struct ipf *ipf, struct
> ipf_list *ipf_list,
> }
>
> static bool
> -ipf_ctx_eq(struct ipf_list *ipf_list, struct ipf_ctx *ctx)
> +ipf_ctx_eq(struct ipf_list *ipf_list, struct ipf_ctx *ctx,
> + struct dp_packet *pkt)
> {
> - struct dp_packet *pkt =
> - ipf_list->frag_list[ipf_list->last_sent_idx + 1].pkt;
> -
> - if (pkt->md.recirc_id != ctx->recirc_id ||
> + /* NOTE: not using the first pkt's metadata, use ipf_list instead,
> + * using pkt's metadata causes too much cache miss,
> + * using ipf_list instead drops ipf_postprocessing cpu usage
> + * from 4 percent to 2 percent. ^_^.
> + */
Maybe this note is better in the commit log. We can't actually be sure
about each system's underlying memory architecture (for example, does
this hold true even on ARM?).
> + if (ipf_list->key.recirc_id != ctx->recirc_id ||
> pkt->md.in_port.odp_port != ctx->in_port.odp_port ||
> - pkt->md.ct_zone != ctx->zone) {
> + ipf_list->key.zone != ctx->zone) {
> return false;
> }
> return true;
> @@ -1077,7 +1080,8 @@ ipf_send_completed_frags(struct ipf *ipf, struct
> dp_packet_batch *pb,
>
> LIST_FOR_EACH_SAFE (ipf_list, next, list_node, &ipf->frag_complete_list)
> {
>
> - if (ctx && !ipf_ctx_eq(ipf_list, ctx)) {
> + if (ctx && !ipf_ctx_eq(ipf_list, ctx, \
> + ipf_list->frag_list[ipf_list->last_sent_idx + 1].pkt)) {
> continue;
> }
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev