From d2c30b127f5c663760d1c4ff4573cf0aab2b4d78 Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Tue, 13 Apr 2021 12:49:11 +0200 Subject: [PATCH 01/13] first proposal --- p4src/include/control/hasher.p4 | 10 +++++++++- p4src/include/header.p4 | 10 ++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index 8ae8f714f..c8cd7dd1c 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -13,7 +13,15 @@ control Hasher( apply { if (fabric_md.acl_lkp.is_ipv4) { - fabric_md.bridged.base.flow_hash = ipv4_hasher.get(fabric_md.acl_lkp); + gtp_flow_t to_hash; + to_hash.ipv4_src = fabric_md.acl_lkp.ipv4_src; + to_hash.ipv4_dst = fabric_md.acl_lkp.ipv4_dst; + to_hash.ip_proto = fabric_md.acl_lkp.ip_proto; + to_hash.l4_sport = fabric_md.acl_lkp.l4_sport; + to_hash.l4_dport = fabric_md.acl_lkp.l4_dport; + to_hash.teid = fabric_md.bridged.spgw.gtpu_teid; + // compute hash from GTP flow + fabric_md.bridged.base.flow_hash = ipv4_hasher.get(to_hash); } // FIXME: remove ipv6 support or test it // https://github.com/stratum/fabric-tna/pull/227 diff --git a/p4src/include/header.p4 b/p4src/include/header.p4 index 0341b6da9..41a1f1584 100644 --- a/p4src/include/header.p4 +++ b/p4src/include/header.p4 @@ -298,6 +298,16 @@ struct acl_lookup_t { l4_port_t l4_dport; } +// Used for ECMP hashing. +struct gtp_flow_t { + bit<32> ipv4_src; + bit<32> ipv4_dst; + bit<8> ip_proto; + l4_port_t l4_sport; + l4_port_t l4_dport; + teid_t teid; +} + // Ingress pipeline-only metadata @flexible @pa_auto_init_metadata From d5227b27d1cf46884bb633d65f4963d036b2e026 Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Wed, 14 Apr 2021 14:40:08 +0200 Subject: [PATCH 02/13] Use outer IPv4 header for GTP flows; refactoring --- p4src/include/control/hasher.p4 | 42 ++++++++++++++++++++++++++------- p4src/include/header.p4 | 10 -------- ptf/tests/ptf/fabric_test.py | 4 ++++ 3 files changed, 38 insertions(+), 18 deletions(-) diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index c8cd7dd1c..960a5e94e 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -4,6 +4,16 @@ #include "../define.p4" #include "../header.p4" +// Used for ECMP hashing. +struct flow_t { + bit<32> ipv4_src; + bit<32> ipv4_dst; + bit<8> ip_proto; + l4_port_t l4_sport; + l4_port_t l4_dport; + teid_t gtpu_teid; +} + control Hasher( in parsed_headers_t hdr, inout fabric_ingress_metadata_t fabric_md) { @@ -13,14 +23,30 @@ control Hasher( apply { if (fabric_md.acl_lkp.is_ipv4) { - gtp_flow_t to_hash; - to_hash.ipv4_src = fabric_md.acl_lkp.ipv4_src; - to_hash.ipv4_dst = fabric_md.acl_lkp.ipv4_dst; - to_hash.ip_proto = fabric_md.acl_lkp.ip_proto; - to_hash.l4_sport = fabric_md.acl_lkp.l4_sport; - to_hash.l4_dport = fabric_md.acl_lkp.l4_dport; - to_hash.teid = fabric_md.bridged.spgw.gtpu_teid; - // compute hash from GTP flow + flow_t to_hash; + if (hdr.gtpu.isValid()) { + // for GTP-encapsulated IPv4 packet use outer IPv4 header for hashing + to_hash.gtpu_teid = fabric_md.bridged.spgw.gtpu_teid; + to_hash.ipv4_src = hdr.ipv4.src_addr; + to_hash.ipv4_dst = hdr.ipv4.dst_addr; + to_hash.ip_proto = hdr.ipv4.protocol; + // avoid the impact of the PHV overlay + to_hash.l4_sport = 0; + to_hash.l4_dport = 0; + // this should always be true for the GTP-encapsulated packets + if (hdr.udp.isValid()) { + to_hash.l4_sport = hdr.udp.sport; + to_hash.l4_dport = hdr.udp.dport; + } + } else { + to_hash.gtpu_teid = 0; + to_hash.ipv4_src = fabric_md.acl_lkp.ipv4_src; + to_hash.ipv4_dst = fabric_md.acl_lkp.ipv4_dst; + to_hash.ip_proto = fabric_md.acl_lkp.ip_proto; + to_hash.l4_sport = fabric_md.acl_lkp.l4_sport; + to_hash.l4_dport = fabric_md.acl_lkp.l4_dport; + } + // compute hash for a flow fabric_md.bridged.base.flow_hash = ipv4_hasher.get(to_hash); } // FIXME: remove ipv6 support or test it diff --git a/p4src/include/header.p4 b/p4src/include/header.p4 index 41a1f1584..0341b6da9 100644 --- a/p4src/include/header.p4 +++ b/p4src/include/header.p4 @@ -298,16 +298,6 @@ struct acl_lookup_t { l4_port_t l4_dport; } -// Used for ECMP hashing. -struct gtp_flow_t { - bit<32> ipv4_src; - bit<32> ipv4_dst; - bit<8> ip_proto; - l4_port_t l4_sport; - l4_port_t l4_dport; - teid_t teid; -} - // Ingress pipeline-only metadata @flexible @pa_auto_init_metadata diff --git a/ptf/tests/ptf/fabric_test.py b/ptf/tests/ptf/fabric_test.py index c820caca7..55d7b0994 100644 --- a/ptf/tests/ptf/fabric_test.py +++ b/ptf/tests/ptf/fabric_test.py @@ -3455,6 +3455,10 @@ def runUplinkIntDropTest( if expect_int_report: self.verify_packet(exp_int_report_pkt_masked, self.port3) self.verify_no_other_packets() + # block for a while to avoid Bloom filter to drop INT reports + # This is needed, because flow_hash is calculated from the GTP flow + # and it does not change during this test case. + time.sleep(2) def runDownlinkIntDropTest( self, From d0201a3aaa2ef553f847a747339b2bff6dd77f52 Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Thu, 15 Apr 2021 15:59:36 +0200 Subject: [PATCH 03/13] Use hash from inner headers for INT; add PTF test to verify GTP-based load balancing --- p4src/include/control/hasher.p4 | 24 +++---- p4src/include/control/int.p4 | 10 +-- p4src/include/control/next.p4 | 2 +- p4src/include/header.p4 | 3 +- ptf/tests/ptf/fabric.ptf/test.py | 109 +++++++++++++++++++++++++++++++ ptf/tests/ptf/fabric_test.py | 4 -- 6 files changed, 129 insertions(+), 23 deletions(-) diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index 960a5e94e..223e24aaa 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -18,15 +18,22 @@ control Hasher( in parsed_headers_t hdr, inout fabric_ingress_metadata_t fabric_md) { + Hash(HashAlgorithm_t.CRC32) int_hasher; Hash(HashAlgorithm_t.CRC32) ipv4_hasher; Hash(HashAlgorithm_t.CRC32) non_ip_hasher; apply { if (fabric_md.acl_lkp.is_ipv4) { - flow_t to_hash; + // we always need to calculate hash from inner headers for the INT reporter + fabric_md.bridged.base.int_hash = int_hasher.get(fabric_md.acl_lkp); + + // if not a GTP flow, use hash calculated from inner headers + fabric_md.flow_hash = fabric_md.bridged.base.int_hash; +#ifdef WITH_SPGW if (hdr.gtpu.isValid()) { + flow_t to_hash; // for GTP-encapsulated IPv4 packet use outer IPv4 header for hashing - to_hash.gtpu_teid = fabric_md.bridged.spgw.gtpu_teid; + to_hash.gtpu_teid = hdr.gtpu.teid; to_hash.ipv4_src = hdr.ipv4.src_addr; to_hash.ipv4_dst = hdr.ipv4.dst_addr; to_hash.ip_proto = hdr.ipv4.protocol; @@ -38,16 +45,9 @@ control Hasher( to_hash.l4_sport = hdr.udp.sport; to_hash.l4_dport = hdr.udp.dport; } - } else { - to_hash.gtpu_teid = 0; - to_hash.ipv4_src = fabric_md.acl_lkp.ipv4_src; - to_hash.ipv4_dst = fabric_md.acl_lkp.ipv4_dst; - to_hash.ip_proto = fabric_md.acl_lkp.ip_proto; - to_hash.l4_sport = fabric_md.acl_lkp.l4_sport; - to_hash.l4_dport = fabric_md.acl_lkp.l4_dport; + fabric_md.flow_hash = ipv4_hasher.get(to_hash); } - // compute hash for a flow - fabric_md.bridged.base.flow_hash = ipv4_hasher.get(to_hash); +#endif // WITH_SPGW } // FIXME: remove ipv6 support or test it // https://github.com/stratum/fabric-tna/pull/227 @@ -62,7 +62,7 @@ control Hasher( // } else { // Not an IP packet - fabric_md.bridged.base.flow_hash = non_ip_hasher.get({ + fabric_md.flow_hash = non_ip_hasher.get({ hdr.ethernet.dst_addr, hdr.ethernet.src_addr, hdr.eth_type.value diff --git a/p4src/include/control/int.p4 b/p4src/include/control/int.p4 index 5deaead24..bb5c869aa 100644 --- a/p4src/include/control/int.p4 +++ b/p4src/include/control/int.p4 @@ -62,11 +62,11 @@ control FlowReportFilter( fabric_md.bridged.base.ig_port, eg_intr_md.egress_port, fabric_md.int_md.hop_latency, - fabric_md.bridged.base.flow_hash, + fabric_md.bridged.base.int_hash, fabric_md.int_md.timestamp }); - flag = filter_get_and_set1.execute(fabric_md.bridged.base.flow_hash[31:16]); - flag = flag | filter_get_and_set2.execute(fabric_md.bridged.base.flow_hash[15:0]); + flag = filter_get_and_set1.execute(fabric_md.bridged.base.int_hash[31:16]); + flag = flag | filter_get_and_set2.execute(fabric_md.bridged.base.int_hash[15:0]); // Generate report only when ALL register actions detect a change. if (flag == 1) { eg_dprsr_md.mirror_type = (bit<3>)FabricMirrorType_t.INVALID; @@ -187,7 +187,7 @@ control IntIngress ( fabric_md.int_mirror_md.ip_eth_type = fabric_md.bridged.base.ip_eth_type; fabric_md.int_mirror_md.eg_port = (bit<16>)ig_tm_md.ucast_egress_port; fabric_md.int_mirror_md.queue_id = (bit<8>)ig_tm_md.qid; - fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.base.flow_hash; + fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.base.int_hash; ig_dprsr_md.drop_ctl = 1; #ifdef WITH_DEBUG drop_report_counter.count(); @@ -406,7 +406,7 @@ control IntEgress ( fabric_md.int_mirror_md.ig_tstamp = fabric_md.bridged.base.ig_tstamp[31:0]; fabric_md.int_mirror_md.eg_tstamp = eg_prsr_md.global_tstamp[31:0]; fabric_md.int_mirror_md.ip_eth_type = fabric_md.bridged.base.ip_eth_type; - fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.base.flow_hash; + fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.base.int_hash; // fabric_md.int_mirror_md.vlan_stripped set by egress_vlan table // fabric_md.int_mirror_md.strip_gtpu will be initialized by the parser } diff --git a/p4src/include/control/next.p4 b/p4src/include/control/next.p4 index e02737a53..a9c52b4b5 100644 --- a/p4src/include/control/next.p4 +++ b/p4src/include/control/next.p4 @@ -130,7 +130,7 @@ control Next (inout parsed_headers_t hdr, table hashed { key = { fabric_md.next_id : exact @name("next_id"); - fabric_md.bridged.base.flow_hash : selector; + fabric_md.flow_hash : selector; } actions = { output_hashed; diff --git a/p4src/include/header.p4 b/p4src/include/header.p4 index 0341b6da9..6e5a3e1ec 100644 --- a/p4src/include/header.p4 +++ b/p4src/include/header.p4 @@ -262,7 +262,7 @@ struct bridged_metadata_base_t { bit<8> mpls_ttl; bit<48> ig_tstamp; bit<16> ip_eth_type; - flow_hash_t flow_hash; + flow_hash_t int_hash; #ifdef WITH_DOUBLE_VLAN_TERMINATION @padding bit<7> _pad1; bool push_double_vlan; @@ -303,6 +303,7 @@ struct acl_lookup_t { @pa_auto_init_metadata struct fabric_ingress_metadata_t { bridged_metadata_t bridged; + flow_hash_t flow_hash; acl_lookup_t acl_lkp; bit<32> routing_ipv4_dst; // Outermost bool skip_forwarding; diff --git a/ptf/tests/ptf/fabric.ptf/test.py b/ptf/tests/ptf/fabric.ptf/test.py index 8c803039e..d2874d206 100644 --- a/ptf/tests/ptf/fabric.ptf/test.py +++ b/ptf/tests/ptf/fabric.ptf/test.py @@ -1024,6 +1024,115 @@ def runTest(self): self.verify_no_other_packets() +@group("spgw") +class FabricGTPUnicastGroupTestAllPortTEID(FabricTest): + + @tvsetup + @autocleanup + def GTPUnicastGroupTestAllPortTEID(self, pkt_type): + # In this test we check that packets are forwarded to all ports when we + # change one of the 5-tuple header values and we have an ECMP-like + # distribution. + # In this case TEID for GTP-encapsulated packets + vlan_id = 10 + self.set_ingress_port_vlan(self.port1, False, 0, vlan_id) + self.set_forwarding_type( + self.port1, + SWITCH_MAC, + ethertype=ETH_TYPE_IPV4, + fwd_type=FORWARDING_TYPE_UNICAST_IPV4, + ) + self.add_forwarding_routing_v4_entry(S1U_SGW_IPV4, 24, 300) + grp_id = 66 + mbrs = [ + (self.port2, SWITCH_MAC, HOST2_MAC), + (self.port3, SWITCH_MAC, HOST3_MAC), + ] + self.add_next_routing_group(300, grp_id, mbrs) + self.set_egress_vlan(self.port2, vlan_id, False) + self.set_egress_vlan(self.port3, vlan_id, False) + # teid_toport list is used to learn the teid that causes the packet + # to be forwarded for each port + teid_toport = [None, None] + for i in range(50): + test_teid = i + pkt_from1 = getattr(testutils, "simple_%s_packet" % pkt_type)( + eth_src=HOST1_MAC, + eth_dst=SWITCH_MAC, + ip_src=HOST1_IPV4, + ip_dst=HOST2_IPV4, + ip_ttl=64, + ) + pkt_from1 = pkt_add_gtp( + pkt_from1, + out_ipv4_src=S1U_ENB_IPV4, + out_ipv4_dst=S1U_SGW_IPV4, + teid=test_teid, + ) + + exp_pkt_to2 = pkt_from1.copy() + exp_pkt_to2[Ether].src = SWITCH_MAC + exp_pkt_to2[Ether].dst = HOST2_MAC + exp_pkt_to2[IP].ttl = 63 + + exp_pkt_to3 = pkt_from1.copy() + exp_pkt_to3[Ether].src = SWITCH_MAC + exp_pkt_to3[Ether].dst = HOST3_MAC + exp_pkt_to3[IP].ttl = 63 + + self.send_packet(self.port1, pkt_from1) + out_port_indx = self.verify_any_packet_any_port( + [exp_pkt_to2, exp_pkt_to3], [self.port2, self.port3] + ) + teid_toport[out_port_indx] = test_teid + + inner_pkt = getattr(testutils, "simple_%s_packet" % pkt_type)( + eth_src=HOST1_MAC, + eth_dst=SWITCH_MAC, + ip_src=HOST1_IPV4, + ip_dst=HOST2_IPV4, + ip_ttl=64, + ) + + pkt_toport2 = pkt_add_gtp( + inner_pkt, + out_ipv4_src=S1U_ENB_IPV4, + out_ipv4_dst=S1U_SGW_IPV4, + teid=teid_toport[0], + ) + + pkt_toport3 = pkt_add_gtp( + inner_pkt, + out_ipv4_src=S1U_ENB_IPV4, + out_ipv4_dst=S1U_SGW_IPV4, + teid=teid_toport[1], + ) + + exp_pkt_to2 = pkt_toport2.copy() + exp_pkt_to2[Ether].src = SWITCH_MAC + exp_pkt_to2[Ether].dst = HOST2_MAC + exp_pkt_to2[IP].ttl = 63 + + exp_pkt_to3 = pkt_toport3.copy() + exp_pkt_to3[Ether].src = SWITCH_MAC + exp_pkt_to3[Ether].dst = HOST3_MAC + exp_pkt_to3[IP].ttl = 63 + + self.send_packet(self.port1, pkt_toport2) + self.send_packet(self.port1, pkt_toport3) + # In this assertion we are verifying: + # 1) all ports of the same group are used almost once + # 2) consistency of the forwarding decision, i.e. packets with the + # same 5-tuple fields are always forwarded out of the same port + self.verify_each_packet_on_each_port( + [exp_pkt_to2, exp_pkt_to3], [self.port2, self.port3] + ) + + def runTest(self): + for pkt_type in ["tcp", "udp", "icmp"]: + self.GTPUnicastGroupTestAllPortTEID(pkt_type) + + @group("spgw") class FabricSpgwDownlinkTest(SpgwSimpleTest): @tvsetup diff --git a/ptf/tests/ptf/fabric_test.py b/ptf/tests/ptf/fabric_test.py index 55d7b0994..c820caca7 100644 --- a/ptf/tests/ptf/fabric_test.py +++ b/ptf/tests/ptf/fabric_test.py @@ -3455,10 +3455,6 @@ def runUplinkIntDropTest( if expect_int_report: self.verify_packet(exp_int_report_pkt_masked, self.port3) self.verify_no_other_packets() - # block for a while to avoid Bloom filter to drop INT reports - # This is needed, because flow_hash is calculated from the GTP flow - # and it does not change during this test case. - time.sleep(2) def runDownlinkIntDropTest( self, From edcdcb0ec415c499703d4d338765bd8d6d35c8e3 Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Thu, 15 Apr 2021 17:36:22 +0200 Subject: [PATCH 04/13] improve naming --- p4src/include/control/hasher.p4 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index 223e24aaa..dd602bd3b 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -5,7 +5,7 @@ #include "../header.p4" // Used for ECMP hashing. -struct flow_t { +struct gtp_flow_t { bit<32> ipv4_src; bit<32> ipv4_dst; bit<8> ip_proto; @@ -18,20 +18,20 @@ control Hasher( in parsed_headers_t hdr, inout fabric_ingress_metadata_t fabric_md) { - Hash(HashAlgorithm_t.CRC32) int_hasher; - Hash(HashAlgorithm_t.CRC32) ipv4_hasher; + Hash(HashAlgorithm_t.CRC32) ip_hasher; + Hash(HashAlgorithm_t.CRC32) gtp_flow_hasher; Hash(HashAlgorithm_t.CRC32) non_ip_hasher; apply { if (fabric_md.acl_lkp.is_ipv4) { // we always need to calculate hash from inner headers for the INT reporter - fabric_md.bridged.base.int_hash = int_hasher.get(fabric_md.acl_lkp); + fabric_md.bridged.base.int_hash = ip_hasher.get(fabric_md.acl_lkp); // if not a GTP flow, use hash calculated from inner headers fabric_md.flow_hash = fabric_md.bridged.base.int_hash; #ifdef WITH_SPGW if (hdr.gtpu.isValid()) { - flow_t to_hash; + gtp_flow_t to_hash; // for GTP-encapsulated IPv4 packet use outer IPv4 header for hashing to_hash.gtpu_teid = hdr.gtpu.teid; to_hash.ipv4_src = hdr.ipv4.src_addr; @@ -45,7 +45,7 @@ control Hasher( to_hash.l4_sport = hdr.udp.sport; to_hash.l4_dport = hdr.udp.dport; } - fabric_md.flow_hash = ipv4_hasher.get(to_hash); + fabric_md.flow_hash = gtp_flow_hasher.get(to_hash); } #endif // WITH_SPGW } From 2749aea0ac7f57678b4c298ccf37139f569d9133 Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Fri, 16 Apr 2021 17:36:37 +0200 Subject: [PATCH 05/13] next version of hasher --- p4src/include/control/hasher.p4 | 25 ++--- ptf/tests/ptf/fabric.ptf/test.py | 177 +++++++++++++++++++++++++++---- 2 files changed, 172 insertions(+), 30 deletions(-) diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index dd602bd3b..af898b5ae 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -9,8 +9,6 @@ struct gtp_flow_t { bit<32> ipv4_src; bit<32> ipv4_dst; bit<8> ip_proto; - l4_port_t l4_sport; - l4_port_t l4_dport; teid_t gtpu_teid; } @@ -30,21 +28,24 @@ control Hasher( // if not a GTP flow, use hash calculated from inner headers fabric_md.flow_hash = fabric_md.bridged.base.int_hash; #ifdef WITH_SPGW + gtp_flow_t to_hash; + bool calc_gtp_hash = false; if (hdr.gtpu.isValid()) { - gtp_flow_t to_hash; // for GTP-encapsulated IPv4 packet use outer IPv4 header for hashing - to_hash.gtpu_teid = hdr.gtpu.teid; to_hash.ipv4_src = hdr.ipv4.src_addr; to_hash.ipv4_dst = hdr.ipv4.dst_addr; to_hash.ip_proto = hdr.ipv4.protocol; - // avoid the impact of the PHV overlay - to_hash.l4_sport = 0; - to_hash.l4_dport = 0; - // this should always be true for the GTP-encapsulated packets - if (hdr.udp.isValid()) { - to_hash.l4_sport = hdr.udp.sport; - to_hash.l4_dport = hdr.udp.dport; - } + to_hash.gtpu_teid = hdr.gtpu.teid; + calc_gtp_hash = true; + } else if (fabric_md.bridged.spgw.needs_gtpu_encap) { + to_hash.ipv4_src = fabric_md.bridged.spgw.gtpu_tunnel_sip; + to_hash.ipv4_dst = fabric_md.bridged.spgw.gtpu_tunnel_dip; + // we assume UDP protocol + to_hash.ip_proto = PROTO_UDP; + to_hash.gtpu_teid = fabric_md.bridged.spgw.gtpu_teid; + calc_gtp_hash = true; + } + if (calc_gtp_hash) { fabric_md.flow_hash = gtp_flow_hasher.get(to_hash); } #endif // WITH_SPGW diff --git a/ptf/tests/ptf/fabric.ptf/test.py b/ptf/tests/ptf/fabric.ptf/test.py index d2874d206..a5c9b4755 100644 --- a/ptf/tests/ptf/fabric.ptf/test.py +++ b/ptf/tests/ptf/fabric.ptf/test.py @@ -1026,6 +1026,10 @@ def runTest(self): @group("spgw") class FabricGTPUnicastGroupTestAllPortTEID(FabricTest): + """ + This test case verifies if the GTP encapsulated traffic + is distributed over next hops by the fabric. + """ @tvsetup @autocleanup @@ -1051,20 +1055,23 @@ def GTPUnicastGroupTestAllPortTEID(self, pkt_type): self.add_next_routing_group(300, grp_id, mbrs) self.set_egress_vlan(self.port2, vlan_id, False) self.set_egress_vlan(self.port3, vlan_id, False) + + pkt = getattr(testutils, "simple_%s_packet" % pkt_type)( + eth_src=HOST1_MAC, + eth_dst=SWITCH_MAC, + ip_src=HOST1_IPV4, + ip_dst=HOST2_IPV4, + ip_ttl=64, + ) + # teid_toport list is used to learn the teid that causes the packet # to be forwarded for each port teid_toport = [None, None] for i in range(50): test_teid = i - pkt_from1 = getattr(testutils, "simple_%s_packet" % pkt_type)( - eth_src=HOST1_MAC, - eth_dst=SWITCH_MAC, - ip_src=HOST1_IPV4, - ip_dst=HOST2_IPV4, - ip_ttl=64, - ) + pkt_from1 = pkt_add_gtp( - pkt_from1, + pkt, out_ipv4_src=S1U_ENB_IPV4, out_ipv4_dst=S1U_SGW_IPV4, teid=test_teid, @@ -1086,23 +1093,15 @@ def GTPUnicastGroupTestAllPortTEID(self, pkt_type): ) teid_toport[out_port_indx] = test_teid - inner_pkt = getattr(testutils, "simple_%s_packet" % pkt_type)( - eth_src=HOST1_MAC, - eth_dst=SWITCH_MAC, - ip_src=HOST1_IPV4, - ip_dst=HOST2_IPV4, - ip_ttl=64, - ) - pkt_toport2 = pkt_add_gtp( - inner_pkt, + pkt, out_ipv4_src=S1U_ENB_IPV4, out_ipv4_dst=S1U_SGW_IPV4, teid=teid_toport[0], ) pkt_toport3 = pkt_add_gtp( - inner_pkt, + pkt, out_ipv4_src=S1U_ENB_IPV4, out_ipv4_dst=S1U_SGW_IPV4, teid=teid_toport[1], @@ -1133,6 +1132,148 @@ def runTest(self): self.GTPUnicastGroupTestAllPortTEID(pkt_type) +@group("spgw") +class FabricSpgwDownlinkLoadBalancingTest(SpgwSimpleTest): + """ + This test case verifies if from PDN to UE (downlink) is + distrubted over next hops. + """ + + @tvsetup + @autocleanup + def doRunTest(self, pkt_type): + vlan_id = 10 + self.set_ingress_port_vlan(self.port1, False, 0, vlan_id) + self.set_forwarding_type( + self.port1, + SWITCH_MAC, + ethertype=ETH_TYPE_IPV4, + fwd_type=FORWARDING_TYPE_UNICAST_IPV4, + ) + self.add_forwarding_routing_v4_entry(S1U_ENB_IPV4, 24, 300) + grp_id = 66 + + # used for this test only + S1U_ENB_NEXTHOP1_MAC = "00:00:00:00:00:ee" + S1U_ENB_NEXTHOP2_MAC = "00:00:00:00:00:ff" + mbrs = [ + (self.port2, SWITCH_MAC, S1U_ENB_NEXTHOP1_MAC), + (self.port3, SWITCH_MAC, S1U_ENB_NEXTHOP2_MAC), + ] + self.add_next_routing_group(300, grp_id, mbrs) + self.set_egress_vlan(self.port2, vlan_id, False) + self.set_egress_vlan(self.port3, vlan_id, False) + + # ue_ipv4_toport list is used to learn the ue_ipv4 address for a given packet. + ue_ipv4_toport = [None, None] + # teid_toport list is used to learn the teid + # assigned by SPGW for a downlink packet. + # This teid causes the packet to be forwarded for each port. + teid_toport = [None, None] + for i in range(50): + ue_ipv4 = "10.0.0." + str(i) + far_id = i + test_teid = i*3 + + self.setup_downlink( + s1u_sgw_addr=S1U_SGW_IPV4, + s1u_enb_addr=S1U_ENB_IPV4, + teid=test_teid, + ue_addr=ue_ipv4, + ctr_id=DOWNLINK_PDR_CTR_IDX, + far_id=far_id, + ) + + pkt_from1 = getattr(testutils, "simple_%s_packet" % pkt_type)( + eth_src=HOST1_MAC, + eth_dst=SWITCH_MAC, + ip_src=UE2_IPV4, + ip_dst=ue_ipv4, + ip_ttl=64, + ) + + exp_pkt_to2 = pkt_from1.copy() + exp_pkt_to2[IP].ttl = 63 + exp_pkt_to2 = pkt_add_gtp( + exp_pkt_to2, + out_ipv4_src=S1U_SGW_IPV4, + out_ipv4_dst=S1U_ENB_IPV4, + teid=test_teid, + ) + exp_pkt_to2[Ether].src = SWITCH_MAC + exp_pkt_to2[Ether].dst = S1U_ENB_NEXTHOP1_MAC + + exp_pkt_to3 = pkt_from1.copy() + exp_pkt_to3[IP].ttl = 63 + exp_pkt_to3 = pkt_add_gtp( + exp_pkt_to3, + out_ipv4_src=S1U_SGW_IPV4, + out_ipv4_dst=S1U_ENB_IPV4, + teid=test_teid, + ) + exp_pkt_to3[Ether].src = SWITCH_MAC + exp_pkt_to3[Ether].dst = S1U_ENB_NEXTHOP2_MAC + + self.send_packet(self.port1, pkt_from1) + out_port_indx = self.verify_any_packet_any_port( + [exp_pkt_to2, exp_pkt_to3], [self.port2, self.port3] + ) + ue_ipv4_toport[out_port_indx] = ue_ipv4 + teid_toport[out_port_indx] = test_teid + + pkt_toport2 = getattr(testutils, "simple_%s_packet" % pkt_type)( + eth_src=HOST1_MAC, + eth_dst=SWITCH_MAC, + ip_src=UE2_IPV4, + ip_dst=ue_ipv4_toport[0], + ip_ttl=64, + ) + + pkt_toport3 = getattr(testutils, "simple_%s_packet" % pkt_type)( + eth_src=HOST1_MAC, + eth_dst=SWITCH_MAC, + ip_src=UE2_IPV4, + ip_dst=ue_ipv4_toport[1], + ip_ttl=64, + ) + + exp_pkt_to2 = pkt_toport2.copy() + exp_pkt_to2[IP].ttl = 63 + exp_pkt_to2 = pkt_add_gtp( + exp_pkt_to2, + out_ipv4_src=S1U_SGW_IPV4, + out_ipv4_dst=S1U_ENB_IPV4, + teid=teid_toport[0], + ) + exp_pkt_to2[Ether].src = SWITCH_MAC + exp_pkt_to2[Ether].dst = S1U_ENB_NEXTHOP1_MAC + + exp_pkt_to3 = pkt_toport3.copy() + exp_pkt_to3[IP].ttl = 63 + exp_pkt_to3 = pkt_add_gtp( + exp_pkt_to3, + out_ipv4_src=S1U_SGW_IPV4, + out_ipv4_dst=S1U_ENB_IPV4, + teid=teid_toport[1], + ) + exp_pkt_to3[Ether].src = SWITCH_MAC + exp_pkt_to3[Ether].dst = S1U_ENB_NEXTHOP2_MAC + + self.send_packet(self.port1, pkt_toport2) + self.send_packet(self.port1, pkt_toport3) + # In this assertion we are verifying: + # 1) all ports of the same group are used almost once + # 2) consistency of the forwarding decision, i.e. packets with the + # same 5-tuple fields are always forwarded out of the same port + self.verify_each_packet_on_each_port( + [exp_pkt_to2, exp_pkt_to3], [self.port2, self.port3] + ) + + def runTest(self): + for pkt_type in ["tcp", "udp", "icmp"]: + self.doRunTest(pkt_type) + + @group("spgw") class FabricSpgwDownlinkTest(SpgwSimpleTest): @tvsetup From c24d91f86dab5b43fb32193034704ea9ace0a8b7 Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Fri, 16 Apr 2021 18:05:13 +0200 Subject: [PATCH 06/13] fix indent --- p4src/include/control/hasher.p4 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index af898b5ae..b3cf8939c 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -38,12 +38,12 @@ control Hasher( to_hash.gtpu_teid = hdr.gtpu.teid; calc_gtp_hash = true; } else if (fabric_md.bridged.spgw.needs_gtpu_encap) { - to_hash.ipv4_src = fabric_md.bridged.spgw.gtpu_tunnel_sip; - to_hash.ipv4_dst = fabric_md.bridged.spgw.gtpu_tunnel_dip; - // we assume UDP protocol - to_hash.ip_proto = PROTO_UDP; - to_hash.gtpu_teid = fabric_md.bridged.spgw.gtpu_teid; - calc_gtp_hash = true; + to_hash.ipv4_src = fabric_md.bridged.spgw.gtpu_tunnel_sip; + to_hash.ipv4_dst = fabric_md.bridged.spgw.gtpu_tunnel_dip; + // we assume UDP protocol + to_hash.ip_proto = PROTO_UDP; + to_hash.gtpu_teid = fabric_md.bridged.spgw.gtpu_teid; + calc_gtp_hash = true; } if (calc_gtp_hash) { fabric_md.flow_hash = gtp_flow_hasher.get(to_hash); From 40d6b24b2b8526f88f3b6b64b0ce4f3c876c581a Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Mon, 19 Apr 2021 18:13:00 +0200 Subject: [PATCH 07/13] refactor hasher --- .jenkins/pr_verify.sh | 6 ++++- p4src/include/control/hasher.p4 | 44 ++++++++++++++++++++------------ p4src/include/control/int.p4 | 10 ++++---- p4src/include/control/next.p4 | 2 +- p4src/include/header.p4 | 4 +-- ptf/tests/ptf/fabric.ptf/test.py | 22 ++++++++-------- ptf/tests/ptf/fabric_test.py | 2 +- 7 files changed, 51 insertions(+), 39 deletions(-) diff --git a/.jenkins/pr_verify.sh b/.jenkins/pr_verify.sh index 74c887244..b4b4f07ab 100644 --- a/.jenkins/pr_verify.sh +++ b/.jenkins/pr_verify.sh @@ -20,7 +20,11 @@ echo "Build all profiles using SDE ${SDE_P4C_DOCKER_IMG}..." # Pull first to avoid pulling multiple times in parallel by the make jobs docker pull "${SDE_P4C_DOCKER_IMG}" # Jenkins uses 8 cores 15G VM -make -j8 all +# We commented out 'all' target, because we exceeded 45 min limit on Jenkins. +# TODO: revert once the PTF tests execution time is optimized +# make -j8 all +make -j8 fabric-int +make -j8 fabric-spgw-int echo "Build and verify Java pipeconf" make constants pipeconf MVN_FLAGS="-Pci-verify -Pcoverage" diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index b3cf8939c..73c46a856 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -8,7 +8,6 @@ struct gtp_flow_t { bit<32> ipv4_src; bit<32> ipv4_dst; - bit<8> ip_proto; teid_t gtpu_teid; } @@ -21,34 +20,40 @@ control Hasher( Hash(HashAlgorithm_t.CRC32) non_ip_hasher; apply { + gtp_flow_t to_hash; + flow_hash_t inner_hash; + bool calc_gtp_hash = false; + + // checks if inner header is IPv4 if (fabric_md.acl_lkp.is_ipv4) { - // we always need to calculate hash from inner headers for the INT reporter - fabric_md.bridged.base.int_hash = ip_hasher.get(fabric_md.acl_lkp); + // we always need to calculate hash from the inner IPv4 header for the INT reporter. + inner_hash = ip_hasher.get(fabric_md.acl_lkp); - // if not a GTP flow, use hash calculated from inner headers - fabric_md.flow_hash = fabric_md.bridged.base.int_hash; -#ifdef WITH_SPGW - gtp_flow_t to_hash; - bool calc_gtp_hash = false; + // use inner hash by default + fabric_md.ecmp_hash = inner_hash; + + // if an outer GTP header exists, use it to perform GTP-aware ECMP if (hdr.gtpu.isValid()) { - // for GTP-encapsulated IPv4 packet use outer IPv4 header for hashing to_hash.ipv4_src = hdr.ipv4.src_addr; to_hash.ipv4_dst = hdr.ipv4.dst_addr; - to_hash.ip_proto = hdr.ipv4.protocol; to_hash.gtpu_teid = hdr.gtpu.teid; calc_gtp_hash = true; - } else if (fabric_md.bridged.spgw.needs_gtpu_encap) { + } + +#ifdef WITH_SPGW + // enable GTP-aware ECMP for downlink packets. + if (fabric_md.bridged.spgw.needs_gtpu_encap) { to_hash.ipv4_src = fabric_md.bridged.spgw.gtpu_tunnel_sip; to_hash.ipv4_dst = fabric_md.bridged.spgw.gtpu_tunnel_dip; - // we assume UDP protocol - to_hash.ip_proto = PROTO_UDP; to_hash.gtpu_teid = fabric_md.bridged.spgw.gtpu_teid; calc_gtp_hash = true; } +#endif // WITH_SPGW + if (calc_gtp_hash) { - fabric_md.flow_hash = gtp_flow_hasher.get(to_hash); + fabric_md.ecmp_hash = gtp_flow_hasher.get(to_hash); } -#endif // WITH_SPGW + } // FIXME: remove ipv6 support or test it // https://github.com/stratum/fabric-tna/pull/227 @@ -62,12 +67,17 @@ control Hasher( // }); // } else { - // Not an IP packet - fabric_md.flow_hash = non_ip_hasher.get({ + // Inner header is not an IPv4 packet + inner_hash = non_ip_hasher.get({ hdr.ethernet.dst_addr, hdr.ethernet.src_addr, hdr.eth_type.value }); } + +#ifdef WITH_INT + fabric_md.bridged.int_bmd.inner_hash = inner_hash; +#endif // WITH_INT + } } diff --git a/p4src/include/control/int.p4 b/p4src/include/control/int.p4 index bb5c869aa..81d6b3846 100644 --- a/p4src/include/control/int.p4 +++ b/p4src/include/control/int.p4 @@ -62,11 +62,11 @@ control FlowReportFilter( fabric_md.bridged.base.ig_port, eg_intr_md.egress_port, fabric_md.int_md.hop_latency, - fabric_md.bridged.base.int_hash, + fabric_md.bridged.int_bmd.inner_hash, fabric_md.int_md.timestamp }); - flag = filter_get_and_set1.execute(fabric_md.bridged.base.int_hash[31:16]); - flag = flag | filter_get_and_set2.execute(fabric_md.bridged.base.int_hash[15:0]); + flag = filter_get_and_set1.execute(fabric_md.bridged.int_bmd.inner_hash[31:16]); + flag = flag | filter_get_and_set2.execute(fabric_md.bridged.int_bmd.inner_hash[15:0]); // Generate report only when ALL register actions detect a change. if (flag == 1) { eg_dprsr_md.mirror_type = (bit<3>)FabricMirrorType_t.INVALID; @@ -187,7 +187,7 @@ control IntIngress ( fabric_md.int_mirror_md.ip_eth_type = fabric_md.bridged.base.ip_eth_type; fabric_md.int_mirror_md.eg_port = (bit<16>)ig_tm_md.ucast_egress_port; fabric_md.int_mirror_md.queue_id = (bit<8>)ig_tm_md.qid; - fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.base.int_hash; + fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.int_bmd.inner_hash; ig_dprsr_md.drop_ctl = 1; #ifdef WITH_DEBUG drop_report_counter.count(); @@ -406,7 +406,7 @@ control IntEgress ( fabric_md.int_mirror_md.ig_tstamp = fabric_md.bridged.base.ig_tstamp[31:0]; fabric_md.int_mirror_md.eg_tstamp = eg_prsr_md.global_tstamp[31:0]; fabric_md.int_mirror_md.ip_eth_type = fabric_md.bridged.base.ip_eth_type; - fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.base.int_hash; + fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.int_bmd.inner_hash; // fabric_md.int_mirror_md.vlan_stripped set by egress_vlan table // fabric_md.int_mirror_md.strip_gtpu will be initialized by the parser } diff --git a/p4src/include/control/next.p4 b/p4src/include/control/next.p4 index a9c52b4b5..6da1c6d0c 100644 --- a/p4src/include/control/next.p4 +++ b/p4src/include/control/next.p4 @@ -130,7 +130,7 @@ control Next (inout parsed_headers_t hdr, table hashed { key = { fabric_md.next_id : exact @name("next_id"); - fabric_md.flow_hash : selector; + fabric_md.ecmp_hash : selector; } actions = { output_hashed; diff --git a/p4src/include/header.p4 b/p4src/include/header.p4 index 6e5a3e1ec..c340d5a86 100644 --- a/p4src/include/header.p4 +++ b/p4src/include/header.p4 @@ -237,6 +237,7 @@ struct int_bridged_metadata_t { IntReportType_t report_type; @padding bit<6> _pad1; MirrorId_t mirror_session_id; + flow_hash_t inner_hash; } struct int_metadata_t { @@ -262,7 +263,6 @@ struct bridged_metadata_base_t { bit<8> mpls_ttl; bit<48> ig_tstamp; bit<16> ip_eth_type; - flow_hash_t int_hash; #ifdef WITH_DOUBLE_VLAN_TERMINATION @padding bit<7> _pad1; bool push_double_vlan; @@ -303,7 +303,7 @@ struct acl_lookup_t { @pa_auto_init_metadata struct fabric_ingress_metadata_t { bridged_metadata_t bridged; - flow_hash_t flow_hash; + flow_hash_t ecmp_hash; acl_lookup_t acl_lkp; bit<32> routing_ipv4_dst; // Outermost bool skip_forwarding; diff --git a/ptf/tests/ptf/fabric.ptf/test.py b/ptf/tests/ptf/fabric.ptf/test.py index a5c9b4755..d84d05e50 100644 --- a/ptf/tests/ptf/fabric.ptf/test.py +++ b/ptf/tests/ptf/fabric.ptf/test.py @@ -1024,20 +1024,19 @@ def runTest(self): self.verify_no_other_packets() -@group("spgw") -class FabricGTPUnicastGroupTestAllPortTEID(FabricTest): +class FabricGTPUnicastECMPbasedOnTEID(FabricTest): """ This test case verifies if the GTP encapsulated traffic - is distributed over next hops by the fabric. + is distributed over next hops by hashing on the TEID. """ @tvsetup @autocleanup - def GTPUnicastGroupTestAllPortTEID(self, pkt_type): + def doRunTest(self, pkt_type): # In this test we check that packets are forwarded to all ports when we - # change one of the 5-tuple header values and we have an ECMP-like + # change one of the values used for hash calculcation and we have an ECMP-like # distribution. - # In this case TEID for GTP-encapsulated packets + # In this case, we change TEID for GTP-encapsulated packets vlan_id = 10 self.set_ingress_port_vlan(self.port1, False, 0, vlan_id) self.set_forwarding_type( @@ -1122,21 +1121,21 @@ def GTPUnicastGroupTestAllPortTEID(self, pkt_type): # In this assertion we are verifying: # 1) all ports of the same group are used almost once # 2) consistency of the forwarding decision, i.e. packets with the - # same 5-tuple fields are always forwarded out of the same port + # same hashed fields are always forwarded out of the same port self.verify_each_packet_on_each_port( [exp_pkt_to2, exp_pkt_to3], [self.port2, self.port3] ) def runTest(self): for pkt_type in ["tcp", "udp", "icmp"]: - self.GTPUnicastGroupTestAllPortTEID(pkt_type) + self.doRunTest(pkt_type) @group("spgw") -class FabricSpgwDownlinkLoadBalancingTest(SpgwSimpleTest): +class FabricSpgwDownlinkECMPTest(SpgwSimpleTest): """ - This test case verifies if from PDN to UE (downlink) is - distrubted over next hops. + This test case verifies if traffic from PDN to UEs (downlink) served by the same + base station is distributed over next hops using GTP-aware load balancing. """ @tvsetup @@ -1168,7 +1167,6 @@ def doRunTest(self, pkt_type): ue_ipv4_toport = [None, None] # teid_toport list is used to learn the teid # assigned by SPGW for a downlink packet. - # This teid causes the packet to be forwarded for each port. teid_toport = [None, None] for i in range(50): ue_ipv4 = "10.0.0." + str(i) diff --git a/ptf/tests/ptf/fabric_test.py b/ptf/tests/ptf/fabric_test.py index c820caca7..cba38b38f 100644 --- a/ptf/tests/ptf/fabric_test.py +++ b/ptf/tests/ptf/fabric_test.py @@ -217,7 +217,7 @@ if testutils.test_param_get("profile") == "fabric-spgw-int": BMD_BYTES = 43 elif testutils.test_param_get("profile") == "fabric-spgw": - BMD_BYTES = 40 + BMD_BYTES = 36 elif testutils.test_param_get("profile") == "fabric-int": BMD_BYTES = 24 else: From 37cf734250f2abb35f7c18f2be6132a514bc14b1 Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Mon, 19 Apr 2021 18:26:14 +0200 Subject: [PATCH 08/13] move local variables to inner if block --- p4src/include/control/hasher.p4 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index 73c46a856..d59c28d2d 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -20,12 +20,13 @@ control Hasher( Hash(HashAlgorithm_t.CRC32) non_ip_hasher; apply { - gtp_flow_t to_hash; flow_hash_t inner_hash; - bool calc_gtp_hash = false; // checks if inner header is IPv4 if (fabric_md.acl_lkp.is_ipv4) { + gtp_flow_t to_hash; + bool calc_gtp_hash = false; + // we always need to calculate hash from the inner IPv4 header for the INT reporter. inner_hash = ip_hasher.get(fabric_md.acl_lkp); From b2cea67e821a1394b8b33054933842c9e70e720b Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Mon, 19 Apr 2021 18:58:43 +0200 Subject: [PATCH 09/13] use global var for pkt types list --- ptf/tests/ptf/fabric.ptf/test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ptf/tests/ptf/fabric.ptf/test.py b/ptf/tests/ptf/fabric.ptf/test.py index d84d05e50..8d937ada6 100644 --- a/ptf/tests/ptf/fabric.ptf/test.py +++ b/ptf/tests/ptf/fabric.ptf/test.py @@ -1127,7 +1127,7 @@ def doRunTest(self, pkt_type): ) def runTest(self): - for pkt_type in ["tcp", "udp", "icmp"]: + for pkt_type in PKT_TYPES_UNDER_TEST: self.doRunTest(pkt_type) @@ -1268,7 +1268,7 @@ def doRunTest(self, pkt_type): ) def runTest(self): - for pkt_type in ["tcp", "udp", "icmp"]: + for pkt_type in PKT_TYPES_UNDER_TEST: self.doRunTest(pkt_type) From b32d415a78d5288d04721833cc55e8352e955265 Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Mon, 19 Apr 2021 22:29:28 +0200 Subject: [PATCH 10/13] move inner_hash back to base bridged metadata --- p4src/include/control/hasher.p4 | 16 ++++------------ p4src/include/control/int.p4 | 10 +++++----- p4src/include/header.p4 | 2 +- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/p4src/include/control/hasher.p4 b/p4src/include/control/hasher.p4 index d59c28d2d..0599a0b1d 100644 --- a/p4src/include/control/hasher.p4 +++ b/p4src/include/control/hasher.p4 @@ -20,18 +20,15 @@ control Hasher( Hash(HashAlgorithm_t.CRC32) non_ip_hasher; apply { - flow_hash_t inner_hash; - - // checks if inner header is IPv4 if (fabric_md.acl_lkp.is_ipv4) { gtp_flow_t to_hash; bool calc_gtp_hash = false; // we always need to calculate hash from the inner IPv4 header for the INT reporter. - inner_hash = ip_hasher.get(fabric_md.acl_lkp); + fabric_md.bridged.base.inner_hash = ip_hasher.get(fabric_md.acl_lkp); // use inner hash by default - fabric_md.ecmp_hash = inner_hash; + fabric_md.ecmp_hash = fabric_md.bridged.base.inner_hash; // if an outer GTP header exists, use it to perform GTP-aware ECMP if (hdr.gtpu.isValid()) { @@ -68,17 +65,12 @@ control Hasher( // }); // } else { - // Inner header is not an IPv4 packet - inner_hash = non_ip_hasher.get({ + // Not an IP packet + fabric_md.bridged.base.inner_hash = non_ip_hasher.get({ hdr.ethernet.dst_addr, hdr.ethernet.src_addr, hdr.eth_type.value }); } - -#ifdef WITH_INT - fabric_md.bridged.int_bmd.inner_hash = inner_hash; -#endif // WITH_INT - } } diff --git a/p4src/include/control/int.p4 b/p4src/include/control/int.p4 index 81d6b3846..f26a265fd 100644 --- a/p4src/include/control/int.p4 +++ b/p4src/include/control/int.p4 @@ -62,11 +62,11 @@ control FlowReportFilter( fabric_md.bridged.base.ig_port, eg_intr_md.egress_port, fabric_md.int_md.hop_latency, - fabric_md.bridged.int_bmd.inner_hash, + fabric_md.bridged.base.inner_hash, fabric_md.int_md.timestamp }); - flag = filter_get_and_set1.execute(fabric_md.bridged.int_bmd.inner_hash[31:16]); - flag = flag | filter_get_and_set2.execute(fabric_md.bridged.int_bmd.inner_hash[15:0]); + flag = filter_get_and_set1.execute(fabric_md.bridged.base.inner_hash[31:16]); + flag = flag | filter_get_and_set2.execute(fabric_md.bridged.base.inner_hash[15:0]); // Generate report only when ALL register actions detect a change. if (flag == 1) { eg_dprsr_md.mirror_type = (bit<3>)FabricMirrorType_t.INVALID; @@ -187,7 +187,7 @@ control IntIngress ( fabric_md.int_mirror_md.ip_eth_type = fabric_md.bridged.base.ip_eth_type; fabric_md.int_mirror_md.eg_port = (bit<16>)ig_tm_md.ucast_egress_port; fabric_md.int_mirror_md.queue_id = (bit<8>)ig_tm_md.qid; - fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.int_bmd.inner_hash; + fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.base.inner_hash; ig_dprsr_md.drop_ctl = 1; #ifdef WITH_DEBUG drop_report_counter.count(); @@ -406,7 +406,7 @@ control IntEgress ( fabric_md.int_mirror_md.ig_tstamp = fabric_md.bridged.base.ig_tstamp[31:0]; fabric_md.int_mirror_md.eg_tstamp = eg_prsr_md.global_tstamp[31:0]; fabric_md.int_mirror_md.ip_eth_type = fabric_md.bridged.base.ip_eth_type; - fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.int_bmd.inner_hash; + fabric_md.int_mirror_md.flow_hash = fabric_md.bridged.base.inner_hash; // fabric_md.int_mirror_md.vlan_stripped set by egress_vlan table // fabric_md.int_mirror_md.strip_gtpu will be initialized by the parser } diff --git a/p4src/include/header.p4 b/p4src/include/header.p4 index c340d5a86..85dfc3a57 100644 --- a/p4src/include/header.p4 +++ b/p4src/include/header.p4 @@ -237,7 +237,6 @@ struct int_bridged_metadata_t { IntReportType_t report_type; @padding bit<6> _pad1; MirrorId_t mirror_session_id; - flow_hash_t inner_hash; } struct int_metadata_t { @@ -252,6 +251,7 @@ struct int_metadata_t { // See: https://community.intel.com/t5/Intel-Connectivity-Research/Compiler-stuck-when-compiling-P4-code/m-p/1258087 // @flexible struct bridged_metadata_base_t { + flow_hash_t inner_hash; mpls_label_t mpls_label; @padding bit<11> _pad0; PortId_t ig_port; From 31a8a9dc798ad5885e9d6d7e24bb7dd9f4be300d Mon Sep 17 00:00:00 2001 From: Tomasz Osinski Date: Mon, 19 Apr 2021 23:01:29 +0200 Subject: [PATCH 11/13] increase BMD_BYTES for fabric-spgw profile --- ptf/tests/ptf/fabric_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ptf/tests/ptf/fabric_test.py b/ptf/tests/ptf/fabric_test.py index cba38b38f..c820caca7 100644 --- a/ptf/tests/ptf/fabric_test.py +++ b/ptf/tests/ptf/fabric_test.py @@ -217,7 +217,7 @@ if testutils.test_param_get("profile") == "fabric-spgw-int": BMD_BYTES = 43 elif testutils.test_param_get("profile") == "fabric-spgw": - BMD_BYTES = 36 + BMD_BYTES = 40 elif testutils.test_param_get("profile") == "fabric-int": BMD_BYTES = 24 else: From 3cc3769f04d4acf6a1c06ef4ac1a46f772cad743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Osi=C5=84ski?= Date: Tue, 20 Apr 2021 16:01:46 +0200 Subject: [PATCH 12/13] Update ptf/tests/ptf/fabric.ptf/test.py Co-authored-by: Carmelo Cascone --- ptf/tests/ptf/fabric.ptf/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ptf/tests/ptf/fabric.ptf/test.py b/ptf/tests/ptf/fabric.ptf/test.py index 8d937ada6..1b61919b3 100644 --- a/ptf/tests/ptf/fabric.ptf/test.py +++ b/ptf/tests/ptf/fabric.ptf/test.py @@ -1024,7 +1024,7 @@ def runTest(self): self.verify_no_other_packets() -class FabricGTPUnicastECMPbasedOnTEID(FabricTest): +class FabricGtpUnicastEcmpBasedOnTeid(FabricTest): """ This test case verifies if the GTP encapsulated traffic is distributed over next hops by hashing on the TEID. From 9942947981f908f935bb187c9105c8e0b8815916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Osi=C5=84ski?= Date: Tue, 20 Apr 2021 16:01:55 +0200 Subject: [PATCH 13/13] Update ptf/tests/ptf/fabric.ptf/test.py Co-authored-by: Carmelo Cascone --- ptf/tests/ptf/fabric.ptf/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ptf/tests/ptf/fabric.ptf/test.py b/ptf/tests/ptf/fabric.ptf/test.py index 1b61919b3..6b4eb37a9 100644 --- a/ptf/tests/ptf/fabric.ptf/test.py +++ b/ptf/tests/ptf/fabric.ptf/test.py @@ -1132,7 +1132,7 @@ def runTest(self): @group("spgw") -class FabricSpgwDownlinkECMPTest(SpgwSimpleTest): +class FabricSpgwDownlinkEcmpTest(SpgwSimpleTest): """ This test case verifies if traffic from PDN to UEs (downlink) served by the same base station is distributed over next hops using GTP-aware load balancing.