[ovs-dev] [PATCH] dpif-netdev: Delay packets' metadata initialization.
Daniele Di Proietto
diproiettod at vmware.com
Tue Feb 2 03:28:19 UTC 2016
On 30/01/2016 17:30, "Andy Zhou" <azhou at ovn.org> wrote:
>
>
>On Thu, Jan 28, 2016 at 5:56 PM, Daniele Di Proietto
><diproiettod at vmware.com> wrote:
>
>When a group of packets arrives from a port, we loop through them to
>initialize metadata and them 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.
>
>Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>
>
>
>
>This patch provides significant performance boost for DPDK single core
>packet forwarding rate.
>I was able to verify line rate forwarding of 64 byte packets over a 10G
>NIC. Nice job!
>
>
>Acked-by: Andy Zhou <azhou at ovn.org>
>
Thanks for the review!
>
>
>In comments inline : s/and them/and then/.
>
fixed
>
>
>Unfortunately, the code reality dropped. May be it is a small price to
>pay for optimizing fast path code?
>The following incremental patch may help to regain some lost code
>readability. Please feel free to fold it in if you
>like.
I agree, it complicates the code a little bit, but I think
your incremental helps, so I rolled it in.
I sent a v2, since Sugesh had some comments also
>
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index c38ff2d..43836d2 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -478,8 +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,
>- bool mdinit, odp_port_t port_no);
>+ 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);
>@@ -2559,7 +2560,7 @@ dp_netdev_process_rxq_port(struct
>dp_netdev_pmd_thread *pmd,
> *recirc_depth_get() = 0;
>
> cycles_count_start(pmd);
>- dp_netdev_input(pmd, packets, cnt, false, port->port_no);
>+ 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);
>@@ -3286,14 +3287,14 @@ dp_netdev_queue_batches(struct dp_packet *pkt,
> * 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 'mdinit' is false, the metadata in 'packets' is not valid and must
>be
>+ * 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,
>- bool mdinit,odp_port_t port_no)
>+ bool md_is_valid, odp_port_t port_no)
> {
> struct emc_cache *flow_cache = &pmd->flow_cache;
> struct netdev_flow_key key;
>@@ -3314,7 +3315,7 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>struct dp_packet **packets,
> pkt_metadata_prefetch_init(&packets[i+1]->md);
> }
>
>- if (!mdinit) {
>+ if (!md_is_valid) {
> pkt_metadata_init(&packets[i]->md, port_no);
> }
> miniflow_extract(packets[i], &key.mf);
>@@ -3482,11 +3483,11 @@ fast_path_processing(struct dp_netdev_pmd_thread
>*pmd,
> * 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, 'mdinit' must be true and 'port_no' will be ignored. */
>+ * 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,
>- bool mdinit, odp_port_t port_no)
>+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;
>@@ -3501,7 +3502,7 @@ dp_netdev_input(struct dp_netdev_pmd_thread *pmd,
>
> n_batches = 0;
> newcnt = emc_processing(pmd, packets, cnt, keys, batches, &n_batches,
>- mdinit, port_no);
>+ md_is_valid, port_no);
> if (OVS_UNLIKELY(newcnt)) {
> fast_path_processing(pmd, packets, newcnt, keys, batches,
>&n_batches);
> }
>@@ -3515,6 +3516,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;
> };
>@@ -3618,7 +3634,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, true, 0);
>+ dp_netdev_recirculate(pmd, packets, cnt);
> (*depth)--;
> } else {
> dp_netdev_drop_packets(tnl_pkt, cnt, !may_steal);
>@@ -3649,7 +3665,7 @@ dp_execute_cb(void *aux_, struct dp_packet
>**packets, int cnt,
> }
>
> (*depth)++;
>- dp_netdev_input(pmd, packets, cnt, true, 0);
>+ dp_netdev_recirculate(pmd, packets, cnt);
> (*depth)--;
> } else {
> dp_netdev_drop_packets(tnl_pkt, cnt, !may_steal);
>@@ -3707,7 +3723,7 @@ dp_execute_cb(void *aux_, struct dp_packet
>**packets, int cnt,
> }
>
> (*depth)++;
>- dp_netdev_input(pmd, packets, cnt, true, 0);
>+ dp_netdev_recirculate(pmd, packets, cnt);
> (*depth)--;
>
> return;
>
>
>
>
More information about the dev
mailing list