[ovs-dev] [PATCH v2] dpif-netdev: Delay packets' metadata initialization.
Chandran, Sugesh
sugesh.chandran at intel.com
Tue Feb 2 12:52:47 UTC 2016
Hi Daniele,
I feel this approach is fine to go ahead. At least it looks to me more readable than the earlier one.
Once again thank you for sending out the patch.
Acked!
Regards
_Sugesh
> -----Original Message-----
> From: Daniele Di Proietto [mailto:diproiettod at vmware.com]
> Sent: Tuesday, February 2, 2016 3:28 AM
> To: dev at openvswitch.org
> Cc: Daniele Di Proietto <diproiettod at vmware.com>; Chandran, Sugesh
> <sugesh.chandran at intel.com>
> Subject: [PATCH v2] dpif-netdev: Delay packets' metadata initialization.
>
> When a group of packets arrives from a port, we loop through them to
> initialize metadata and then we loop through them again to extract the
> flow and perform the exact match classification.
>
> This commit combines the two loops into one, and initializes packet->md
> in emc_processing() to improve performance.
>
> Since emc_processing() might also be called after recirculation (in
> which case the metadata is already valid), an extra parameter is added
> to support both cases.
>
> This commits also implements simple prefetching of packet metadata,
> to further improve performance.
>
> CC: Chandran, Sugesh <sugesh.chandran at intel.com>
> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
> Acked-by: Andy Zhou <azhou at ovn.org>
> ---
> lib/dpif-netdev.c | 61 +++++++++++++++++++++++++++++++++++++++----
> ------------
> lib/packets.h | 8 ++++++++
> 2 files changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 9abb948..f39b125 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -478,7 +478,9 @@ static void dp_netdev_execute_actions(struct
> dp_netdev_pmd_thread *pmd,
> const struct nlattr *actions,
> size_t actions_len);
> static void dp_netdev_input(struct dp_netdev_pmd_thread *,
> - struct dp_packet **, int cnt);
> + struct dp_packet **, int cnt, odp_port_t port_no);
> +static void dp_netdev_recirculate(struct dp_netdev_pmd_thread *,
> + struct dp_packet **, int cnt);
>
> static void dp_netdev_disable_upcall(struct dp_netdev *);
> static void dp_netdev_pmd_reload_done(struct dp_netdev_pmd_thread
> *pmd);
> @@ -2555,16 +2557,10 @@ dp_netdev_process_rxq_port(struct
> dp_netdev_pmd_thread *pmd,
> error = netdev_rxq_recv(rxq, packets, &cnt);
> cycles_count_end(pmd, PMD_CYCLES_POLLING);
> if (!error) {
> - int i;
> -
> *recirc_depth_get() = 0;
>
> - /* XXX: initialize md in netdev implementation. */
> - for (i = 0; i < cnt; i++) {
> - pkt_metadata_init(&packets[i]->md, port->port_no);
> - }
> cycles_count_start(pmd);
> - dp_netdev_input(pmd, packets, cnt);
> + dp_netdev_input(pmd, packets, cnt, port->port_no);
> cycles_count_end(pmd, PMD_CYCLES_PROCESSING);
> } else if (error != EAGAIN && error != EOPNOTSUPP) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> @@ -3284,17 +3280,21 @@ dp_netdev_queue_batches(struct dp_packet
> *pkt,
> }
>
> /* Try to process all ('cnt') the 'packets' using only the exact match cache
> - * 'flow_cache'. If a flow is not found for a packet 'packets[i]', the
> + * 'pmd->flow_cache'. If a flow is not found for a packet 'packets[i]', the
> * miniflow is copied into 'keys' and the packet pointer is moved at the
> * beginning of the 'packets' array.
> *
> * The function returns the number of packets that needs to be processed in
> the
> * 'packets' array (they have been moved to the beginning of the vector).
> + *
> + * If 'md_is_valid' is false, the metadata in 'packets' is not valid and must be
> + * initialized by this function using 'port_no'.
> */
> static inline size_t
> emc_processing(struct dp_netdev_pmd_thread *pmd, struct dp_packet
> **packets,
> size_t cnt, struct netdev_flow_key *keys,
> - struct packet_batch batches[], size_t *n_batches)
> + struct packet_batch batches[], size_t *n_batches,
> + bool md_is_valid, odp_port_t port_no)
> {
> struct emc_cache *flow_cache = &pmd->flow_cache;
> size_t i, n_missed = 0, n_dropped = 0;
> @@ -3309,10 +3309,14 @@ emc_processing(struct dp_netdev_pmd_thread
> *pmd, struct dp_packet **packets,
> }
>
> if (i != cnt - 1) {
> - /* Prefetch next packet data */
> + /* Prefetch next packet data and metadata. */
> OVS_PREFETCH(dp_packet_data(packets[i+1]));
> + pkt_metadata_prefetch_init(&packets[i+1]->md);
> }
>
> + if (!md_is_valid) {
> + pkt_metadata_init(&packets[i]->md, port_no);
> + }
> struct netdev_flow_key *key = &keys[n_missed];
> miniflow_extract(packets[i], &key->mf);
> key->len = 0; /* Not computed yet. */
> @@ -3473,9 +3477,16 @@ fast_path_processing(struct
> dp_netdev_pmd_thread *pmd,
> dp_netdev_count_packet(pmd, DP_STAT_LOST, lost_cnt);
> }
>
> +/* Packets enter the datapath from a port (or from recirculation) here.
> + *
> + * For performance reasons a caller may choose not to initialize the
> metadata
> + * in 'packets': in this case 'mdinit' is false and this function needs to
> + * initialize it using 'port_no'. If the metadata in 'packets' is already
> + * valid, 'md_is_valid' must be true and 'port_no' will be ignored. */
> static void
> -dp_netdev_input(struct dp_netdev_pmd_thread *pmd,
> - struct dp_packet **packets, int cnt)
> +dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
> + struct dp_packet **packets, int cnt,
> + bool md_is_valid, odp_port_t port_no)
> {
> #if !defined(__CHECKER__) && !defined(_WIN32)
> const size_t PKT_ARRAY_SIZE = cnt;
> @@ -3489,7 +3500,8 @@ dp_netdev_input(struct dp_netdev_pmd_thread
> *pmd,
> size_t newcnt, n_batches, i;
>
> n_batches = 0;
> - newcnt = emc_processing(pmd, packets, cnt, keys, batches, &n_batches);
> + newcnt = emc_processing(pmd, packets, cnt, keys, batches, &n_batches,
> + md_is_valid, port_no);
> if (OVS_UNLIKELY(newcnt)) {
> fast_path_processing(pmd, packets, newcnt, keys, batches,
> &n_batches);
> }
> @@ -3503,6 +3515,21 @@ dp_netdev_input(struct dp_netdev_pmd_thread
> *pmd,
> }
> }
>
> +static void
> +dp_netdev_input(struct dp_netdev_pmd_thread *pmd,
> + struct dp_packet **packets, int cnt,
> + odp_port_t port_no)
> +{
> + dp_netdev_input__(pmd, packets, cnt, false, port_no);
> +}
> +
> +static void
> +dp_netdev_recirculate(struct dp_netdev_pmd_thread *pmd,
> + struct dp_packet **packets, int cnt)
> +{
> + dp_netdev_input__(pmd, packets, cnt, true, 0);
> +}
> +
> struct dp_netdev_execute_aux {
> struct dp_netdev_pmd_thread *pmd;
> };
> @@ -3606,7 +3633,7 @@ dp_execute_cb(void *aux_, struct dp_packet
> **packets, int cnt,
> err = push_tnl_action(dp, a, packets, cnt);
> if (!err) {
> (*depth)++;
> - dp_netdev_input(pmd, packets, cnt);
> + dp_netdev_recirculate(pmd, packets, cnt);
> (*depth)--;
> } else {
> dp_netdev_drop_packets(tnl_pkt, cnt, !may_steal);
> @@ -3637,7 +3664,7 @@ dp_execute_cb(void *aux_, struct dp_packet
> **packets, int cnt,
> }
>
> (*depth)++;
> - dp_netdev_input(pmd, packets, cnt);
> + dp_netdev_recirculate(pmd, packets, cnt);
> (*depth)--;
> } else {
> dp_netdev_drop_packets(tnl_pkt, cnt, !may_steal);
> @@ -3695,7 +3722,7 @@ dp_execute_cb(void *aux_, struct dp_packet
> **packets, int cnt,
> }
>
> (*depth)++;
> - dp_netdev_input(pmd, packets, cnt);
> + dp_netdev_recirculate(pmd, packets, cnt);
> (*depth)--;
>
> return;
> diff --git a/lib/packets.h b/lib/packets.h
> index 834e8a4..f1445de 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -163,6 +163,14 @@ pkt_metadata_init(struct pkt_metadata *md,
> odp_port_t port)
> md->in_port.odp_port = port;
> }
>
> +/* This function prefetches the cachelines touched by pkt_metadata_init()
> + * For performance reasons the two functions should be kept in sync. */
> +static inline void
> +pkt_metadata_prefetch_init(struct pkt_metadata *md)
> +{
> + ovs_prefetch_range(md, offsetof(struct pkt_metadata, tunnel.ip_src));
> +}
> +
> bool dpid_from_string(const char *s, uint64_t *dpidp);
>
> #define ETH_ADDR_LEN 6
> --
> 2.1.4
More information about the dev
mailing list