From f32ad44e0b06d661e7eecf15ce61540ddea2b7c0 Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 30 Mar 2019 14:59:46 +0100 Subject: [PATCH] Refactor #n: go back to generic entry point, use if in apply{} --- doc/plan.org | 91 ++++++++++++++++++++++++------------ p4app/controller.py | 79 ++++++++++++++++++------------- p4src/checksums.p4 | 7 +-- p4src/headers.p4 | 6 +++ p4src/parsers.p4 | 5 ++ p4src/static-mapping.p4 | 101 ++++++++++++++++++++++++++++++++++------ 6 files changed, 210 insertions(+), 79 deletions(-) diff --git a/doc/plan.org b/doc/plan.org index 1cee8bc..7cb8842 100644 --- a/doc/plan.org +++ b/doc/plan.org @@ -144,11 +144,26 @@ | 2019-03-27 | | | | | switch cannot be used in actions! | | | | Refactor program to use multiple tables instead of switch | | +| | Ethernet frame check sequence error | | | | | | | 2019-03-28 | Meet Laurent #4 | | -| | - Router solicitation for finding router on startup! | | +| | - Router solicitation for finding router on startup | | | | - test.py for TDD | | | | - Parsing icmp6 is not enough - one layer deeper | | +| | - No LPM priorities | | +| | - if/switch action logic | | +| | - partial translation working to IPv4 | | +| | - Focus on checksumming work (again) | | +| | | | +| | | | +| | Notes: | | +| | - Later using ternary matching | | +| | - Document (nested) if's in action (in thesis) | | +| | - SW and HW Targets Tofino [Albert, Thomas] | | +| | - P414/P416 for Tofino? | | +| | - Barefoot support/question: Ticket/Support System | | +| | - Can try P416 compiler | | +| | - Next week Laurent not around: send email + Albert/Thomas/Edgar meeting | | | | | | | | | | | 2019-03-30 | NAT64 1:1 table ICMP, ICMPv6 working | | @@ -485,9 +500,15 @@ INFO:main:unhandled reassambled= should be 1 ***** DONE figure out switch() syntax -***** TODO Calculate ICMP checksum ***** TODO transform protocol specific: icmp6 -> icmp -**** TODO transform protocol specific: icmp -> icmp6 +****** DONE Implement double table, as there are no if's in actions +****** DONE Debug Ethernet frame check sequence error -> need to compute checksum + https://en.wikipedia.org/wiki/Frame_check_sequence + +According to Edgar this should not be seen anyway. +****** TODO Calculate ICMP checksum +****** TODO Check field lengths +***** TODO transform protocol specific: icmp -> icmp6 **** TODO Make switch answer IPv4 icmp echo request for **** TODO Add / check default route for v4 hosts *** TODO Get p4 VM / vagrant running @@ -1393,39 +1414,45 @@ I could work around this by using if(! .. .hit) { my_action(table_id) }, but it would not work with using default_action = ... **** No switch in actions, No conditional execution in actions +***** 3 possible solutions + - multi table (state as of 2019-03-28) + - switch/if in actions: with shadow tables + - switch/if in apply block -Imho, compiler should be able to unroll these to some degree. -#+BEGIN_SRC -../p4src/static-mapping.p4(60): error: SwitchStatement: switch statements not allowed in actions - switch(hdr.icmp6.type) { - ^^^^^^ -#+END_SRC +***** log + Imho, compiler should be able to unroll these to some degree. -#+BEGIN_SRC -../p4src/static-mapping.p4(57): error: MethodCallStatement: Conditional execution in actions is not supported on this target - hdr.icmp.setValid(); - ^^^^^^^^^^^^^^^^^^^ -../p4src/static-mapping.p4(70): error: MethodCallStatement: Conditional execution in actions is not supported on this target - hdr.icmp6.setInvalid(); - ^^^^^^^^^^^^^^^^^^^^^^ -../p4src/static-mapping.p4(73): error: MethodCallStatement: Conditional execution in actions is not supported on this target - hdr.icmp6_na_ns.setInvalid(); - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -../p4src/static-mapping.p4(74): error: MethodCallStatement: Conditional execution in actions is not supported on this target - hdr.icmp6_option_link_layer_addr.setInvalid(); - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Compilation Error -p4@ubuntu:~/master-thesis/p4app$ -#+END_SRC + #+BEGIN_SRC + ../p4src/static-mapping.p4(60): error: SwitchStatement: switch statements not allowed in actions + switch(hdr.icmp6.type) { + ^^^^^^ + #+END_SRC -Code: + #+BEGIN_SRC + ../p4src/static-mapping.p4(57): error: MethodCallStatement: Conditional execution in actions is not supported on this target + hdr.icmp.setValid(); + ^^^^^^^^^^^^^^^^^^^ + ../p4src/static-mapping.p4(70): error: MethodCallStatement: Conditional execution in actions is not supported on this target + hdr.icmp6.setInvalid(); + ^^^^^^^^^^^^^^^^^^^^^^ + ../p4src/static-mapping.p4(73): error: MethodCallStatement: Conditional execution in actions is not supported on this target + hdr.icmp6_na_ns.setInvalid(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + ../p4src/static-mapping.p4(74): error: MethodCallStatement: Conditional execution in actions is not supported on this target + hdr.icmp6_option_link_layer_addr.setInvalid(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + Compilation Error + p4@ubuntu:~/master-thesis/p4app$ + #+END_SRC -#+BEGIN_SRC - if(hdr.ipv6.next_header == PROTO_ICMP6) { - nat64_icmp6(); - } -#+END_SRC + Code: + + #+BEGIN_SRC + if(hdr.ipv6.next_header == PROTO_ICMP6) { + nat64_icmp6(); + } + #+END_SRC *** Implementation limitations **** No fragmentation support (yet) @@ -1443,6 +1470,8 @@ Only the destination network is matched for deciding on NAT64, as priority based double LPM is not supported. This limits a prefix to be used only in one network. *** References / Follow up +**** RFC 791 IPv4 https://tools.ietf.org/html/rfc791 +**** RFC 792 ICMP https://tools.ietf.org/html/rfc792 **** RFC 1017 ICMP checksum https://tools.ietf.org/html/rfc1071 **** RFC 2460 IPv6 (Checksum https://tools.ietf.org/html/rfc2460#section-8.1) **** RFC 3810 MLD2 https://tools.ietf.org/html/rfc3810 diff --git a/p4app/controller.py b/p4app/controller.py index 44d4066..bb4c4ad 100755 --- a/p4app/controller.py +++ b/p4app/controller.py @@ -284,45 +284,60 @@ class L2Controller(object): v6_src, v6_dst, v4_dst, v4_src, v4_dst, v6_src)) - self.controller.table_add("nat64_icmp6", - "nat64_icmp6_echo_request", - [str(v6_dst), - str(table_proto['ICMP6_ECHO_REQUEST']) - ], + self.controller.table_add("nat64", + "nat64_static", + [str(v6_dst)], + [str(v6_src.network_address), + str(v4_dst.network_address), + str(v6_dst.network_address)] + ) + self.controller.table_add("nat46", + "nat46_static", + [str(v4_dst)], [str(v6_src.network_address), str(v4_dst.network_address), str(v6_dst.network_address)] ) - self.controller.table_add("nat64_icmp6", - "nat64_icmp6_echo_reply", - [str(v6_dst), - str(table_proto['ICMP6_ECHO_REPLY']) - ], - [str(v6_src.network_address), - str(v4_dst.network_address), - str(v6_dst.network_address)] - ) + # self.controller.table_add("nat64_icmp6", + # "nat64_icmp6_echo_request", + # [str(v6_dst), + # str(table_proto['ICMP6_ECHO_REQUEST']) + # ], + # [str(v6_src.network_address), + # str(v4_dst.network_address), + # str(v6_dst.network_address)] + # ) - self.controller.table_add("nat46_icmp", - "nat46_icmp_echo_reply", - [str(v4_dst), - str(table_proto['ICMP_ECHO_REPLY']) - ], - [str(v6_src.network_address), - str(v4_dst.network_address), - str(v6_dst.network_address)] - ) + # self.controller.table_add("nat64_icmp6", + # "nat64_icmp6_echo_reply", + # [str(v6_dst), + # str(table_proto['ICMP6_ECHO_REPLY']) + # ], + # [str(v6_src.network_address), + # str(v4_dst.network_address), + # str(v6_dst.network_address)] + # ) - self.controller.table_add("nat46_icmp", - "nat46_icmp_echo_request", - [str(v4_dst), - str(table_proto['ICMP_ECHO_REQUEST']) - ], - [str(v6_src.network_address), - str(v4_dst.network_address), - str(v6_dst.network_address)] - ) + # self.controller.table_add("nat46_icmp", + # "nat46_icmp_echo_reply", + # [str(v4_dst), + # str(table_proto['ICMP_ECHO_REPLY']) + # ], + # [str(v6_src.network_address), + # str(v4_dst.network_address), + # str(v6_dst.network_address)] + # ) + + # self.controller.table_add("nat46_icmp", + # "nat46_icmp_echo_request", + # [str(v4_dst), + # str(table_proto['ICMP_ECHO_REQUEST']) + # ], + # [str(v6_src.network_address), + # str(v4_dst.network_address), + # str(v6_dst.network_address)] + # ) def config_hosts(self): diff --git a/p4src/checksums.p4 b/p4src/checksums.p4 index 72818cf..68119a9 100644 --- a/p4src/checksums.p4 +++ b/p4src/checksums.p4 @@ -21,7 +21,8 @@ control MyVerifyChecksum(inout headers hdr, inout metadata meta) { control MyComputeChecksum(inout headers hdr, inout metadata meta) { apply { - update_checksum_with_payload(meta.switch_task == TASK_CHECKSUM_ICMP6, + + update_checksum_with_payload(meta.chk_icmp6, { hdr.ipv6.src_addr, /* 128 */ hdr.ipv6.dst_addr, /* 128 */ @@ -36,7 +37,7 @@ control MyComputeChecksum(inout headers hdr, inout metadata meta) { ); /* checksumming for icmp6_na_ns_option */ - update_checksum_with_payload(meta.switch_task == TASK_CHECKSUM_ICMP6_NA, + update_checksum_with_payload(meta.icmp6_na_ns, { hdr.ipv6.src_addr, /* 128 */ hdr.ipv6.dst_addr, /* 128 */ @@ -60,7 +61,7 @@ control MyComputeChecksum(inout headers hdr, inout metadata meta) { HashAlgorithm.csum16 ); - update_checksum_with_payload(meta.switch_task == TASK_CHECKSUM_ICMP, + update_checksum_with_payload(meta.chk_icmp, { hdr.icmp.type, hdr.icmp.code diff --git a/p4src/headers.p4 b/p4src/headers.p4 index abc0920..4dc7480 100644 --- a/p4src/headers.p4 +++ b/p4src/headers.p4 @@ -179,6 +179,12 @@ struct metadata { port_t ingress_port; task_t task; task_t switch_task; + + /* migrate tasks to bool */ + bool chk_icmp6_na_ns; + bool chk_icmp6; + bool chk_icmp; + bit<16> tcp_length; bit<32> cast_length; table_t table_id; diff --git a/p4src/parsers.p4 b/p4src/parsers.p4 index 43b42a4..01dec7a 100644 --- a/p4src/parsers.p4 +++ b/p4src/parsers.p4 @@ -13,6 +13,11 @@ parser MyParser(packet_in packet, inout standard_metadata_t standard_metadata) { state start { + + meta.chk_icmp = false; + meta.chk_icmp6 = false; + meta.chk_icmp6_na_ns = false; + packet.extract(hdr.ethernet); transition select(hdr.ethernet.ethertype){ TYPE_IPV4: ipv4; diff --git a/p4src/static-mapping.p4 b/p4src/static-mapping.p4 index 6f2a259..22ce686 100644 --- a/p4src/static-mapping.p4 +++ b/p4src/static-mapping.p4 @@ -48,7 +48,6 @@ control MyIngress(inout headers hdr, /********************** NAT64 / NAT46 ACTIONS ***********************************/ - /* changes for icmp6 -> icmp */ action nat64_icmp6_generic() { @@ -58,6 +57,8 @@ control MyIngress(inout headers hdr, /* trigger checksumming */ meta.switch_task = TASK_CHECKSUM_ICMP; + meta.chk_icmp = true; + hdr.icmp6.setInvalid(); /* not needed, as we don't translate them (yet/ever) */ @@ -74,8 +75,10 @@ control MyIngress(inout headers hdr, hdr.ipv4.diff_serv = (bit<6>)0; // no ToS hdr.ipv4.ecn = (bit<2>)0; // unsupported - hdr.ipv4.ihl = (bit<4>) 5; // internet header length -- needs to be dynamic!? - hdr.ipv4.totalLen = (bit<16>) hdr.ipv6.payload_length + 5; // should probably also dynamic + /* 5 is ok as long as we don't use options / padding / + anything after the destination address */ + hdr.ipv4.ihl = (bit<4>) 5; // internet header length - static for us + hdr.ipv4.totalLen = (bit<16>) hdr.ipv6.payload_length + 5; // ok under above constraints hdr.ipv4.identification = (bit<16>) 0; // no support for fragments hdr.ipv4.flags = (bit<3>) 0; // DF bit and more fragments, unsupported ATM @@ -91,10 +94,6 @@ control MyIngress(inout headers hdr, hdr.ipv4.protocol = hdr.ipv6.next_header; - if(hdr.ipv6.next_header == PROTO_ICMP6) { - // nat64_icmp6(); - } - hdr.ipv6.setInvalid(); } @@ -108,12 +107,61 @@ control MyIngress(inout headers hdr, nat64_generic(src, dst); } + /* From https://tools.ietf.org/html/rfc792 (IPv4) + +Echo or Echo Reply Message + + 0 1 2 3 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Type | Code | Checksum | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Identifier | Sequence Number | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Data ... + +-+-+-+-+- + + + From https://tools.ietf.org/html/rfc4443#section-4.1 + +4.1. Echo Request Message + + 0 1 2 3 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Type | Code | Checksum | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Identifier | Sequence Number | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Data ... + +-+-+-+-+- + +4.2. Echo Reply Message + + 0 1 2 3 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Type | Code | Checksum | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Identifier | Sequence Number | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + | Data ... + +-+-+-+-+- + + + Type / code are different in ICMP4 and ICMP6! + + */ + + /* if replacing actions */ action nat64_icmp6_echo_request(ipv6_addr_t v6_src, ipv4_addr_t v4_dst, ipv6_addr_t nat64_prefix) { nat64_static(v6_src, v4_dst, nat64_prefix); nat64_icmp6_generic(); hdr.icmp.type = ICMP_ECHO_REQUEST; + + /* fix length, sequence number, etc */ } action nat64_icmp6_echo_reply(ipv6_addr_t v6_src, ipv4_addr_t v4_dst, ipv6_addr_t nat64_prefix) { @@ -162,6 +210,22 @@ control MyIngress(inout headers hdr, nat46_generic(src, dst); } + table nat64 { + key = { + hdr.ipv6.dst_addr: lpm; + } + actions = { /* FIXME: actions need to be updated */ + controller_debug; + nat64_generic; + nat64_icmp6_echo_reply; + nat64_icmp6_echo_request; + controller_debug_table_id; + NoAction; + } + size = NAT64_TABLE_SIZE; + default_action = controller_debug_table_id(TABLE_NAT64); + } + table nat64_icmp6 { key = { hdr.ipv6.dst_addr: lpm; @@ -378,17 +442,28 @@ control MyIngress(inout headers hdr, if(hdr.ipv6.isValid()) { icmp6.apply(); /* icmp6 echo, icmp6 ndp */ - if(nat64_icmp6.apply().hit) { + if(nat64.apply().hit) { /* generic nat64 done */ + if(hdr.icmp6.isValid()) { + nat64_icmp6_generic(); + + if(hdr.icmp6.type == ICMP6_ECHO_REPLY) { + hdr.icmp.type = ICMP_ECHO_REPLY; + } + if(hdr.icmp6.type == ICMP6_ECHO_REQUEST) { + hdr.icmp.type = ICMP_ECHO_REQUEST; + } + } + v4_networks.apply(); /* apply egress for IPv4 */ exit; /* no further v6 processing */ } - v6_networks.apply(); /* egress / routing */ + v6_networks.apply(); /* regular egress / routing */ } else if(hdr.ipv4.isValid()) { - if(nat46_icmp.apply().hit) { /* v4->v6 */ - v6_networks.apply(); /* Now apply v6 egress */ - exit; /* no further v4 processing */ - } + // if(nat46_icmp.apply().hit) { /* v4->v6 */ + // v6_networks.apply(); /* Now apply v6 egress */ + // exit; /* no further v4 processing */ + // } v4_networks.apply(); /* routing, egress */ } }