From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 583B9A00C3; Thu, 16 Jun 2022 16:16:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 55D3940689; Thu, 16 Jun 2022 16:16:49 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 09C4C4003C for ; Thu, 16 Jun 2022 16:16:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1655389008; x=1686925008; h=from:to:subject:date:message-id; bh=6bJwvnljqJtXDh4lf8f15ZWIpzYmvQD5FpvNmglHnCU=; b=HgxtiXpPO6splxcXQh+Sbp2YsLaIGmp4xb6/eDYEGEsP7ZvCEozjL7va kpwZ+iLXQv7wyr+kyNSkHq1Z1rv+5qJ6cIGzBGcC6buxQhRhuU6PutJSN SHQXtYppKp/PToCn5LxI9N1rreSZ5h3CqwZNmSvdYT26/uinAC8TBNgAJ CxzN0XyilpD5FsUq8RCaman8XFZBWYIhfGfUwxOd57iCt6+TDw0SFz4VW fxVlc2h10WyQz8JCX5fY5vU+bKg5PzN0MQdhCBzaWTIgeZVGLAaBp6xqp AAxNVjIJxy7hlci2gIbIPABAXef5eAFif2y6kld3ZPcizXcxDRYjb6uQ8 w==; X-IronPort-AV: E=McAfee;i="6400,9594,10379"; a="343216368" X-IronPort-AV: E=Sophos;i="5.92,305,1650956400"; d="scan'208";a="343216368" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Jun 2022 07:16:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.92,305,1650956400"; d="scan'208";a="713373555" Received: from silpixa00400573.ir.intel.com (HELO silpixa00400573.ger.corp.intel.com) ([10.237.223.107]) by orsmga004.jf.intel.com with ESMTP; 16 Jun 2022 07:16:46 -0700 From: Cristian Dumitrescu To: dev@dpdk.org Subject: [PATCH] pipeline: rework optimization pattern for header generation Date: Thu, 16 Jun 2022 15:16:45 +0100 Message-Id: <20220616141645.30366-1-cristian.dumitrescu@intel.com> X-Mailer: git-send-email 2.17.1 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The P4 language requires marking a header as valid before any of the header fields are written as opposed to after the writes are done. Hence, the optimization of replacing the sequence of instructions to generate a header by reading it from the table action data with a single DMA internal instruction are reworked from "mov all + validate -> dma" to "validate + mov all -> dma". Signed-off-by: Cristian Dumitrescu --- examples/pipeline/examples/fib.spec | 2 +- examples/pipeline/examples/vxlan.spec | 8 +-- lib/pipeline/rte_swx_pipeline.c | 89 +++++++++++------------- lib/pipeline/rte_swx_pipeline_internal.h | 14 +++- 4 files changed, 57 insertions(+), 56 deletions(-) diff --git a/examples/pipeline/examples/fib.spec b/examples/pipeline/examples/fib.spec index 3f8c76610c..0c2d19c106 100644 --- a/examples/pipeline/examples/fib.spec +++ b/examples/pipeline/examples/fib.spec @@ -83,10 +83,10 @@ struct nexthop_action_args_t { action nexthop_action args instanceof nexthop_action_args_t { //Set Ethernet header. + validate h.ethernet mov h.ethernet.dst_addr t.ethernet_dst_addr mov h.ethernet.src_addr t.ethernet_src_addr mov h.ethernet.ethertype t.ethernet_ethertype - validate h.ethernet //Decrement the TTL and update the checksum within the IPv4 header. cksub h.ipv4.hdr_checksum h.ipv4.ttl diff --git a/examples/pipeline/examples/vxlan.spec b/examples/pipeline/examples/vxlan.spec index ac62ab2bec..4ab4309f16 100644 --- a/examples/pipeline/examples/vxlan.spec +++ b/examples/pipeline/examples/vxlan.spec @@ -115,12 +115,13 @@ struct vxlan_encap_args_t { action vxlan_encap args instanceof vxlan_encap_args_t { //Set the outer Ethernet header. + validate h.outer_ethernet mov h.outer_ethernet.dst_addr t.ethernet_dst_addr mov h.outer_ethernet.src_addr t.ethernet_src_addr mov h.outer_ethernet.ethertype t.ethernet_ethertype - validate h.outer_ethernet //Set the outer IPv4 header. + validate h.outer_ipv4 mov h.outer_ipv4.ver_ihl t.ipv4_ver_ihl mov h.outer_ipv4.diffserv t.ipv4_diffserv mov h.outer_ipv4.total_len t.ipv4_total_len @@ -131,21 +132,20 @@ action vxlan_encap args instanceof vxlan_encap_args_t { mov h.outer_ipv4.hdr_checksum t.ipv4_hdr_checksum mov h.outer_ipv4.src_addr t.ipv4_src_addr mov h.outer_ipv4.dst_addr t.ipv4_dst_addr - validate h.outer_ipv4 //Set the outer UDP header. + validate h.outer_udp mov h.outer_udp.src_port t.udp_src_port mov h.outer_udp.dst_port t.udp_dst_port mov h.outer_udp.length t.udp_length mov h.outer_udp.checksum t.udp_checksum - validate h.outer_udp //Set the outer VXLAN header. + validate h.outer_vxlan mov h.outer_vxlan.flags t.vxlan_flags mov h.outer_vxlan.reserved t.vxlan_reserved mov h.outer_vxlan.vni t.vxlan_vni mov h.outer_vxlan.reserved2 t.vxlan_reserved2 - validate h.outer_vxlan //Set the output port. mov m.port_out t.port_out diff --git a/lib/pipeline/rte_swx_pipeline.c b/lib/pipeline/rte_swx_pipeline.c index 066356684e..50805ba821 100644 --- a/lib/pipeline/rte_swx_pipeline.c +++ b/lib/pipeline/rte_swx_pipeline.c @@ -2284,6 +2284,7 @@ instr_hdr_validate_translate(struct rte_swx_pipeline *p, instr->type = INSTR_HDR_VALIDATE; instr->valid.header_id = h->id; + instr->valid.struct_id = h->struct_id; return 0; } @@ -6754,7 +6755,7 @@ action_arg_src_mov_count(struct action *a, uint32_t n_instructions); static int -instr_pattern_mov_all_validate_search(struct rte_swx_pipeline *p, +instr_pattern_validate_mov_all_search(struct rte_swx_pipeline *p, struct action *a, struct instruction *instr, struct instruction_data *data, @@ -6771,51 +6772,42 @@ instr_pattern_mov_all_validate_search(struct rte_swx_pipeline *p, if (!a || !a->st) return 0; - /* First instruction: MOV_HM. */ - if (data[0].invalid || (instr[0].type != INSTR_MOV_HM)) + /* First instruction: HDR_VALIDATE. Second instruction: MOV_HM. */ + if (data[0].invalid || + (instr[0].type != INSTR_HDR_VALIDATE) || + (n_instr < 2) || + data[1].invalid || + (instr[1].type != INSTR_MOV_HM) || + instr[1].mov.src.struct_id) return 0; - h = header_find_by_struct_id(p, instr[0].mov.dst.struct_id); - if (!h || h->st->var_size) + h = header_find_by_struct_id(p, instr[0].valid.struct_id); + if (!h || + h->st->var_size || + (n_instr < 1 + h->st->n_fields)) return 0; for (src_field_id = 0; src_field_id < a->st->n_fields; src_field_id++) - if (instr[0].mov.src.offset == a->st->fields[src_field_id].offset / 8) + if (instr[1].mov.src.offset == a->st->fields[src_field_id].offset / 8) break; - if (src_field_id == a->st->n_fields) + if (src_field_id + h->st->n_fields > a->st->n_fields) return 0; - if (instr[0].mov.dst.offset || - (instr[0].mov.dst.n_bits != h->st->fields[0].n_bits) || - instr[0].mov.src.struct_id || - (instr[0].mov.src.n_bits != a->st->fields[src_field_id].n_bits) || - (instr[0].mov.dst.n_bits != instr[0].mov.src.n_bits)) - return 0; - - if ((n_instr < h->st->n_fields + 1) || - (a->st->n_fields < src_field_id + h->st->n_fields + 1)) - return 0; - - /* Subsequent instructions: MOV_HM. */ - for (i = 1; i < h->st->n_fields; i++) - if (data[i].invalid || - data[i].n_users || - (instr[i].type != INSTR_MOV_HM) || - (instr[i].mov.dst.struct_id != h->struct_id) || - (instr[i].mov.dst.offset != h->st->fields[i].offset / 8) || - (instr[i].mov.dst.n_bits != h->st->fields[i].n_bits) || - instr[i].mov.src.struct_id || - (instr[i].mov.src.offset != a->st->fields[src_field_id + i].offset / 8) || - (instr[i].mov.src.n_bits != a->st->fields[src_field_id + i].n_bits) || - (instr[i].mov.dst.n_bits != instr[i].mov.src.n_bits)) + /* Second and subsequent instructions: MOV_HM. */ + for (i = 0; i < h->st->n_fields; i++) + if (data[1 + i].invalid || + data[1 + i].n_users || + (instr[1 + i].type != INSTR_MOV_HM) || + (instr[1 + i].mov.dst.struct_id != h->struct_id) || + (instr[1 + i].mov.dst.offset != h->st->fields[i].offset / 8) || + (instr[1 + i].mov.dst.n_bits != h->st->fields[i].n_bits) || + instr[1 + i].mov.src.struct_id || + (instr[1 + i].mov.src.offset != a->st->fields[src_field_id + i].offset / 8) || + (instr[1 + i].mov.src.n_bits != a->st->fields[src_field_id + i].n_bits) || + (instr[1 + i].mov.dst.n_bits != instr[1 + i].mov.src.n_bits)) return 0; - /* Last instruction: HDR_VALIDATE. */ - if ((instr[i].type != INSTR_HDR_VALIDATE) || - (instr[i].valid.header_id != h->id)) - return 0; - /* Check that none of the action args that are used as source for this * DMA transfer are not used as source in any other mov instruction. */ @@ -6831,12 +6823,12 @@ instr_pattern_mov_all_validate_search(struct rte_swx_pipeline *p, return 0; } - *n_pattern_instr = 1 + i; + *n_pattern_instr = 1 + h->st->n_fields; return 1; } static void -instr_pattern_mov_all_validate_replace(struct rte_swx_pipeline *p, +instr_pattern_validate_mov_all_replace(struct rte_swx_pipeline *p, struct action *a, struct instruction *instr, struct instruction_data *data, @@ -6846,19 +6838,16 @@ instr_pattern_mov_all_validate_replace(struct rte_swx_pipeline *p, uint32_t src_field_id, src_offset, i; /* Read from the instructions before they are modified. */ - h = header_find_by_struct_id(p, instr[0].mov.dst.struct_id); + h = header_find_by_struct_id(p, instr[1].mov.dst.struct_id); if (!h) return; + src_offset = instr[1].mov.src.offset; + for (src_field_id = 0; src_field_id < a->st->n_fields; src_field_id++) - if (instr[0].mov.src.offset == a->st->fields[src_field_id].offset / 8) + if (src_offset == a->st->fields[src_field_id].offset / 8) break; - if (src_field_id == a->st->n_fields) - return; - - src_offset = instr[0].mov.src.offset; - /* Modify the instructions. */ instr[0].type = INSTR_DMA_HT; instr[0].dma.dst.header_id[0] = h->id; @@ -6875,7 +6864,7 @@ instr_pattern_mov_all_validate_replace(struct rte_swx_pipeline *p, } static uint32_t -instr_pattern_mov_all_validate_optimize(struct rte_swx_pipeline *p, +instr_pattern_validate_mov_all_optimize(struct rte_swx_pipeline *p, struct action *a, struct instruction *instructions, struct instruction_data *instruction_data, @@ -6892,8 +6881,8 @@ instr_pattern_mov_all_validate_optimize(struct rte_swx_pipeline *p, uint32_t n_instr = 0; int detected; - /* Mov all + validate. */ - detected = instr_pattern_mov_all_validate_search(p, + /* Validate + mov all. */ + detected = instr_pattern_validate_mov_all_search(p, a, instr, data, @@ -6903,7 +6892,7 @@ instr_pattern_mov_all_validate_optimize(struct rte_swx_pipeline *p, n_instructions, &n_instr); if (detected) { - instr_pattern_mov_all_validate_replace(p, a, instr, data, n_instr); + instr_pattern_validate_mov_all_replace(p, a, instr, data, n_instr); i += n_instr; continue; } @@ -7020,8 +7009,8 @@ instr_optimize(struct rte_swx_pipeline *p, instruction_data, n_instructions); - /* Mov all + validate. */ - n_instructions = instr_pattern_mov_all_validate_optimize(p, + /* Validate + mov all. */ + n_instructions = instr_pattern_validate_mov_all_optimize(p, a, instructions, instruction_data, diff --git a/lib/pipeline/rte_swx_pipeline_internal.h b/lib/pipeline/rte_swx_pipeline_internal.h index 5feee8eff6..a35635efb7 100644 --- a/lib/pipeline/rte_swx_pipeline_internal.h +++ b/lib/pipeline/rte_swx_pipeline_internal.h @@ -632,6 +632,7 @@ struct instr_io { struct instr_hdr_validity { uint8_t header_id; + uint8_t struct_id; }; struct instr_table { @@ -2228,11 +2229,22 @@ __instr_hdr_validate_exec(struct rte_swx_pipeline *p __rte_unused, const struct instruction *ip) { uint32_t header_id = ip->valid.header_id; + uint32_t struct_id = ip->valid.struct_id; + uint64_t valid_headers = t->valid_headers; + struct header_runtime *h = &t->headers[header_id]; TRACE("[Thread %2u] validate header %u\n", p->thread_id, header_id); + /* If this header is already valid, then its associated t->structs[] element is also valid + * and therefore it should not be modified. It could point to the packet buffer (in case of + * extracted header) and setting it to the default location (h->ptr0) would be incorrect. + */ + if (MASK64_BIT_GET(valid_headers, header_id)) + return; + /* Headers. */ - t->valid_headers = MASK64_BIT_SET(t->valid_headers, header_id); + t->structs[struct_id] = h->ptr0; + t->valid_headers = MASK64_BIT_SET(valid_headers, header_id); } /* -- 2.17.1