Currently we allocate and format a string in dp_netdev_flow_add in case miniflow_bits needs to be printed by dpctl/dump-flows/get-flow. However, this adds unneeded calls to realloc in the pmd hot path, while the resulting string may never be viewed.
Instead of keeping a pointer to an allocated string, now we just keep track of the 16 byte flow map. Signed-off-by: Mike Pattrick <[email protected]> --- lib/dpctl.c | 13 +++++++++++-- lib/dpif-netdev-private-flow.h | 2 +- lib/dpif-netdev.c | 15 +-------------- lib/dpif-netlink.c | 2 +- lib/dpif.h | 6 +++--- lib/flow.h | 1 + lib/netdev-offload-dpdk.c | 2 +- lib/netdev-offload-tc.c | 2 +- 8 files changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/dpctl.c b/lib/dpctl.c index 29041fa3e..6ff26fb1b 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -876,8 +876,17 @@ format_dpif_flow(struct ds *ds, const struct dpif_flow *f, struct hmap *ports, } ds_put_cstr(ds, ", actions:"); format_odp_actions(ds, f->actions, f->actions_len, ports); - if (dpctl_p->verbosity && f->attrs.dp_extra_info) { - ds_put_format(ds, ", dp-extra-info:%s", f->attrs.dp_extra_info); + if (dpctl_p->verbosity && !flowmap_is_empty(f->attrs.dp_extra_info)) { + size_t unit; + ds_put_cstr(ds, ", dp-extra-info:miniflow_bits("); + FLOWMAP_FOR_EACH_UNIT (unit) { + if (unit) { + ds_put_char(ds, ','); + } + ds_put_format(ds, "%d", + count_1bits(f->attrs.dp_extra_info.bits[unit])); + } + ds_put_char(ds, ')'); } } diff --git a/lib/dpif-netdev-private-flow.h b/lib/dpif-netdev-private-flow.h index 66016eb09..a99257251 100644 --- a/lib/dpif-netdev-private-flow.h +++ b/lib/dpif-netdev-private-flow.h @@ -123,7 +123,7 @@ struct dp_netdev_flow { struct packet_batch_per_flow *batch; /* Packet classification. */ - char *dp_extra_info; /* String to return in a flow dump/get. */ + struct flowmap dp_extra_info; struct dpcls_rule cr; /* In owning dp_netdev's 'cls'. */ /* 'cr' must be the last member. */ }; diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 720818e30..380274a3a 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -2380,7 +2380,6 @@ static void dp_netdev_flow_free(struct dp_netdev_flow *flow) { dp_netdev_actions_free(dp_netdev_flow_get_actions(flow)); - free(flow->dp_extra_info); free(flow); } @@ -4058,11 +4057,9 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd, odp_port_t orig_in_port) OVS_REQUIRES(pmd->flow_mutex) { - struct ds extra_info = DS_EMPTY_INITIALIZER; struct dp_netdev_flow *flow; struct netdev_flow_key mask; struct dpcls *cls; - size_t unit; /* Make sure in_port is exact matched before we read it. */ ovs_assert(match->wc.masks.in_port.odp_port == ODPP_NONE); @@ -4106,17 +4103,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd, cls = dp_netdev_pmd_find_dpcls(pmd, in_port); dpcls_insert(cls, &flow->cr, &mask); - ds_put_cstr(&extra_info, "miniflow_bits("); - FLOWMAP_FOR_EACH_UNIT (unit) { - if (unit) { - ds_put_char(&extra_info, ','); - } - ds_put_format(&extra_info, "%d", - count_1bits(flow->cr.mask->mf.map.bits[unit])); - } - ds_put_char(&extra_info, ')'); - flow->dp_extra_info = ds_steal_cstr(&extra_info); - ds_destroy(&extra_info); + flow->dp_extra_info = flow->cr.mask->mf.map; cmap_insert(&pmd->flow_table, CONST_CAST(struct cmap_node *, &flow->node), dp_netdev_flow_hash(&flow->ufid)); diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 71e35ccdd..a63c6ac8f 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -1768,7 +1768,7 @@ dpif_netlink_flow_to_dpif_flow(struct dpif_flow *dpif_flow, dpif_netlink_flow_get_stats(datapath_flow, &dpif_flow->stats); dpif_flow->attrs.offloaded = false; dpif_flow->attrs.dp_layer = "ovs"; - dpif_flow->attrs.dp_extra_info = NULL; + dpif_flow->attrs.dp_extra_info = FLOWMAP_EMPTY_MAP; } /* The design is such that all threads are working together on the first dump diff --git a/lib/dpif.h b/lib/dpif.h index 6cb4dae6d..6fd46ef94 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -515,9 +515,9 @@ struct dpif_flow_detailed_stats { }; struct dpif_flow_attrs { - bool offloaded; /* True if flow is offloaded to HW. */ - const char *dp_layer; /* DP layer the flow is handled in. */ - const char *dp_extra_info; /* Extra information provided by DP. */ + bool offloaded; /* True if flow is offloaded to HW. */ + const char *dp_layer; /* DP layer the flow is handled in. */ + struct flowmap dp_extra_info; /* Extra information provided by DP. */ }; struct dpif_flow_dump_types { diff --git a/lib/flow.h b/lib/flow.h index c647ad83c..5d8481add 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -283,6 +283,7 @@ struct flowmap { }; #define FLOWMAP_EMPTY_INITIALIZER { { 0 } } +static const struct flowmap FLOWMAP_EMPTY_MAP = FLOWMAP_EMPTY_INITIALIZER; static inline void flowmap_init(struct flowmap *); static inline bool flowmap_equal(struct flowmap, struct flowmap); diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index 94dc6a9b7..d18cbaa5c 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -2458,7 +2458,7 @@ netdev_offload_dpdk_flow_get(struct netdev *netdev, struct rte_flow_error error; int ret = 0; - attrs->dp_extra_info = NULL; + attrs->dp_extra_info = FLOWMAP_EMPTY_MAP; rte_flow_data = ufid_to_rte_flow_data_find(netdev, ufid, false); if (!rte_flow_data || !rte_flow_data->rte_flow || diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index 3f7068c8e..1d47eb624 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -600,7 +600,7 @@ parse_tc_flower_to_attrs(struct tc_flower *flower, flower->offloaded_state == TC_OFFLOADED_STATE_UNDEFINED); attrs->dp_layer = "tc"; - attrs->dp_extra_info = NULL; + attrs->dp_extra_info = FLOWMAP_EMPTY_MAP; } static int -- 2.27.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
