Improve fabric.p4 to reduce pipeline resources and refactor pipeconf impl
This patch affects both the P4 pipeline implementation and the
Java pipeconf.
P4 PIPELINE
- Less tables and smarter use of metadata to reduce inter-tables
dependencies and favor parallel execution of tables.
- Removed unused actions / renamed existing ones to make forwarding
behavior clearer (e.g. ingress_port_vlan table)
- Remove co-existence of simple and hansed table. Hashed should be the
default one, but implementations that do not support action profiles
might compile fabric.p4 to use the simple one.
- Use @name annotations for match fields to make control plane
independent of table implementation.
- Use @hidden to avoid showing actions and table on the p4info that
cannot be controlled at runtime.
- First attempt to support double VLAN cross-connect (xconnect table).
- New design has been tested with "fabric-refactoring" branch of
fabric-p4test:
github.com/opennetworkinglab/fabric-p4test/tree/fabric-refactoring
JAVA PIPECONF
This patch brings a major refactoring that reflects the experience
gathered in the past months of working on fabric.p4 and reasoning on its
pipeconf implementation. Indeed, the FlowObjective API is
under-specified and sometimes ambiguous which makes the process of
creating and maintaining a pipeliner implementation tedious. This
refactoring brings a simplified implementation by removing unused/
unnecessary functionalities and by recognizing commonality when possible
(e.g. by means of abstract and utility classes). It also makes design
patterns more explicit and consistent. Overall, the goal is to reduce
technical debt and to make it easier to support new features as we
evolve fabric.p4
Changes include:
- Changes in pipeliner/interpreter to reflect new pipeline design.
- By default translate objective treatment to PiAction. This favors
debuggability of flow rules in ONOS.
- Support new NextObjective’s NextTreatment class.
- Remove lots of unused/unnecessary code (e.g. async callback handling
for pending objective install status in pipeliner as current
implementation was always returning success)
- Gather commonality in abstract classes and simplify implementation
for objective translator (filtering, forwarding, next)
- New implementation of ForwardingFunctionTypes (FFT) that looks at
criterion instance values along with their types (to avoid relying on
case-specific if-else conditions to recognize variants of an FFT)
- Adaptive translation of NextObjective based on presence of simple or
hashed table.
- Support DENY FilteringObjective
Also:
- Fix onos-p4-gen-constants to avoid generating conflicting
PiMatchFieldId variable names.
- Install Graphviz tools in p4vm to generate p4c graphs
- Generate p4c graphs by default when compiling fabric.p4
- Use more compact Hex string when printing PI values
Change-Id: Ife79e44054dc5bc48833f95d0551a7370150eac5
diff --git a/pipelines/fabric/src/main/resources/include/int/int_main.p4 b/pipelines/fabric/src/main/resources/include/int/int_main.p4
index ef81cc9..c1368d8 100644
--- a/pipelines/fabric/src/main/resources/include/int/int_main.p4
+++ b/pipelines/fabric/src/main/resources/include/int/int_main.p4
@@ -45,11 +45,13 @@
table tb_set_source {
key = {
- standard_metadata.ingress_port: exact;
+ standard_metadata.ingress_port: exact @name("ig_port");
}
actions = {
int_set_source;
+ @defaultonly nop();
}
+ const default_action = nop();
counters = counter_set_source;
size = MAX_PORTS;
}
@@ -64,11 +66,13 @@
table tb_set_sink {
key = {
- standard_metadata.egress_spec: exact;
+ standard_metadata.egress_spec: exact @name("eg_spec");
}
actions = {
int_set_sink;
+ @defaultonly nop();
}
+ const default_action = nop();
counters = counter_set_sink;
size = MAX_PORTS;
}
diff --git a/pipelines/fabric/src/main/resources/include/int/int_report.p4 b/pipelines/fabric/src/main/resources/include/int/int_report.p4
index 9326375..8c48ba2 100644
--- a/pipelines/fabric/src/main/resources/include/int/int_report.p4
+++ b/pipelines/fabric/src/main/resources/include/int/int_report.p4
@@ -23,6 +23,7 @@
inout fabric_metadata_t fabric_metadata,
inout standard_metadata_t standard_metadata) {
+ @hidden
action add_report_fixed_header() {
/* Device should include its own INT metadata as embedded,
* we'll not use fabric_report_header for this purpose.
@@ -49,7 +50,7 @@
hdr.report_ethernet.setValid();
hdr.report_ethernet.dst_addr = mon_mac;
hdr.report_ethernet.src_addr = src_mac;
- hdr.report_ethernet.ether_type = ETHERTYPE_IPV4;
+ hdr.report_ethernet.eth_type = ETHERTYPE_IPV4;
//Report IPV4 Header
hdr.report_ipv4.setValid();
@@ -71,8 +72,8 @@
//Report UDP Header
hdr.report_udp.setValid();
- hdr.report_udp.src_port = 0;
- hdr.report_udp.dst_port = mon_port;
+ hdr.report_udp.sport = 0;
+ hdr.report_udp.dport = mon_port;
hdr.report_udp.len = (bit<16>) UDP_HEADER_LEN + (bit<16>) REPORT_FIXED_HEADER_LEN +
(bit<16>) ETH_HEADER_LEN + hdr.ipv4.total_len;
@@ -87,7 +88,9 @@
}
actions = {
do_report_encapsulation;
+ @defaultonly nop();
}
+ default_action = nop;
}
apply {
diff --git a/pipelines/fabric/src/main/resources/include/int/int_sink.p4 b/pipelines/fabric/src/main/resources/include/int/int_sink.p4
index 6c64e32..6531a17 100644
--- a/pipelines/fabric/src/main/resources/include/int/int_sink.p4
+++ b/pipelines/fabric/src/main/resources/include/int/int_sink.p4
@@ -22,11 +22,13 @@
inout parsed_headers_t hdr,
inout fabric_metadata_t fabric_metadata) {
+ @hidden
action restore_header () {
- hdr.udp.dst_port = hdr.intl4_tail.dest_port;
+ hdr.udp.dport = hdr.intl4_tail.dest_port;
hdr.ipv4.dscp = hdr.intl4_tail.dscp;
}
+ @hidden
action int_sink() {
// restore length fields of IPv4 header and UDP header
bit<16> len_bytes = (bit<16>) (hdr.intl4_shim.len_words << 5w2);
diff --git a/pipelines/fabric/src/main/resources/include/int/int_source.p4 b/pipelines/fabric/src/main/resources/include/int/int_source.p4
index 245fe7e..93288a4 100644
--- a/pipelines/fabric/src/main/resources/include/int/int_source.p4
+++ b/pipelines/fabric/src/main/resources/include/int/int_source.p4
@@ -26,6 +26,7 @@
direct_counter(CounterType.packets_and_bytes) counter_int_source;
+ @hidden
action int_source(bit<8> max_hop, bit<5> ins_cnt, bit<4> ins_mask0003, bit<4> ins_mask0407) {
// Insert INT shim header.
hdr.intl4_shim.setValid();
@@ -49,7 +50,7 @@
// Insert INT tail header.
hdr.intl4_tail.setValid();
hdr.intl4_tail.next_proto = hdr.ipv4.protocol;
- hdr.intl4_tail.dest_port = fabric_metadata.l4_dst_port;
+ hdr.intl4_tail.dest_port = fabric_metadata.l4_dport;
hdr.intl4_tail.dscp = hdr.ipv4.dscp;
// Update IP and UDP (if not valid we don't care) lens (in bytes).
hdr.ipv4.total_len = hdr.ipv4.total_len + INT_HEADER_LEN_BYTES;
@@ -64,15 +65,17 @@
table tb_int_source {
key = {
- hdr.ipv4.src_addr: ternary;
- hdr.ipv4.dst_addr: ternary;
- fabric_metadata.l4_src_port: ternary;
- fabric_metadata.l4_dst_port: ternary;
+ hdr.ipv4.src_addr: ternary @name("ipv4_src");
+ hdr.ipv4.dst_addr: ternary @name("ipv4_dst");
+ fabric_metadata.l4_sport: ternary @name("l4_sport");
+ fabric_metadata.l4_dport: ternary @name("l4_dport");
}
actions = {
int_source_dscp;
+ @defaultonly nop();
}
counters = counter_int_source;
+ const default_action = nop();
}
apply {
diff --git a/pipelines/fabric/src/main/resources/include/int/int_transit.p4 b/pipelines/fabric/src/main/resources/include/int/int_transit.p4
index 579fa07..b524f6f 100644
--- a/pipelines/fabric/src/main/resources/include/int/int_transit.p4
+++ b/pipelines/fabric/src/main/resources/include/int/int_transit.p4
@@ -36,22 +36,26 @@
_INT_METADATA_ACTIONS
#else
// Switch ID.
+ @hidden
action int_set_header_0() {
hdr.int_switch_id.setValid();
hdr.int_switch_id.switch_id = fmeta.int_meta.switch_id;
}
// Port IDs.
+ @hidden
action int_set_header_1() {
hdr.int_port_ids.setValid();
hdr.int_port_ids.ingress_port_id = (bit<16>) smeta.ingress_port;
hdr.int_port_ids.egress_port_id = (bit<16>) smeta.egress_port;
}
// Hop latency.
+ @hidden
action int_set_header_2() {
hdr.int_hop_latency.setValid();
hdr.int_hop_latency.hop_latency = (bit<32>) smeta.deq_timedelta;
}
// Queue occupancy.
+ @hidden
action int_set_header_3() {
hdr.int_q_occupancy.setValid();
// TODO: support queues in BMv2. ATM we assume only one.
@@ -59,16 +63,19 @@
hdr.int_q_occupancy.q_occupancy = (bit<24>) smeta.deq_qdepth;
}
// Ingress timestamp.
+ @hidden
action int_set_header_4() {
hdr.int_ingress_tstamp.setValid();
hdr.int_ingress_tstamp.ingress_tstamp = (bit<32>) smeta.enq_timestamp;
}
// Egress timestamp.
+ @hidden
action int_set_header_5() {
hdr.int_egress_tstamp.setValid();
hdr.int_egress_tstamp.egress_tstamp = (bit<32>) smeta.enq_timestamp + (bit<32>) smeta.deq_timedelta;
}
// Queue congestion.
+ @hidden
action int_set_header_6() {
hdr.int_q_congestion.setValid();
// TODO: support queue congestion.
@@ -76,6 +83,7 @@
hdr.int_q_congestion.q_congestion = 24w0;
}
// Egress port utilization.
+ @hidden
action int_set_header_7() {
hdr.int_egress_tx_util.setValid();
// TODO: implement tx utilization support in BMv2.
@@ -84,21 +92,25 @@
#endif // _INT_METADATA_ACTIONS
// Actions to keep track of the new metadata added.
+ @hidden
action add_1() {
fmeta.int_meta.new_words = fmeta.int_meta.new_words + 1;
fmeta.int_meta.new_bytes = fmeta.int_meta.new_bytes + 4;
}
+ @hidden
action add_2() {
fmeta.int_meta.new_words = fmeta.int_meta.new_words + 2;
fmeta.int_meta.new_bytes = fmeta.int_meta.new_bytes + 8;
}
+ @hidden
action add_3() {
fmeta.int_meta.new_words = fmeta.int_meta.new_words + 3;
fmeta.int_meta.new_bytes = fmeta.int_meta.new_bytes + 12;
}
+ @hidden
action add_4() {
fmeta.int_meta.new_words = fmeta.int_meta.new_words + 4;
fmeta.int_meta.new_bytes = fmeta.int_meta.new_bytes + 16;
@@ -106,78 +118,94 @@
// Action function for bits 0-3 combinations, 0 is msb, 3 is lsb.
// Each bit set indicates that corresponding INT header should be added.
+ @hidden
action int_set_header_0003_i0() {
}
+ @hidden
action int_set_header_0003_i1() {
int_set_header_3();
add_1();
}
+ @hidden
action int_set_header_0003_i2() {
int_set_header_2();
add_1();
}
+ @hidden
action int_set_header_0003_i3() {
int_set_header_3();
int_set_header_2();
add_2();
}
+ @hidden
action int_set_header_0003_i4() {
int_set_header_1();
add_1();
}
+ @hidden
action int_set_header_0003_i5() {
int_set_header_3();
int_set_header_1();
add_2();
}
+ @hidden
action int_set_header_0003_i6() {
int_set_header_2();
int_set_header_1();
add_2();
}
+ @hidden
action int_set_header_0003_i7() {
int_set_header_3();
int_set_header_2();
int_set_header_1();
add_3();
}
+ @hidden
action int_set_header_0003_i8() {
int_set_header_0();
add_1();
}
+ @hidden
action int_set_header_0003_i9() {
int_set_header_3();
int_set_header_0();
add_2();
}
+ @hidden
action int_set_header_0003_i10() {
int_set_header_2();
int_set_header_0();
add_2();
}
+ @hidden
action int_set_header_0003_i11() {
int_set_header_3();
int_set_header_2();
int_set_header_0();
add_3();
}
+ @hidden
action int_set_header_0003_i12() {
int_set_header_1();
int_set_header_0();
add_2();
}
+ @hidden
action int_set_header_0003_i13() {
int_set_header_3();
int_set_header_1();
int_set_header_0();
add_3();
}
+ @hidden
action int_set_header_0003_i14() {
int_set_header_2();
int_set_header_1();
int_set_header_0();
add_3();
}
+ @hidden
action int_set_header_0003_i15() {
int_set_header_3();
int_set_header_2();
@@ -187,78 +215,94 @@
}
// Action function for bits 4-7 combinations, 4 is msb, 7 is lsb.
+ @hidden
action int_set_header_0407_i0() {
}
+ @hidden
action int_set_header_0407_i1() {
int_set_header_7();
add_1();
}
+ @hidden
action int_set_header_0407_i2() {
int_set_header_6();
add_1();
}
+ @hidden
action int_set_header_0407_i3() {
int_set_header_7();
int_set_header_6();
add_2();
}
+ @hidden
action int_set_header_0407_i4() {
int_set_header_5();
add_1();
}
+ @hidden
action int_set_header_0407_i5() {
int_set_header_7();
int_set_header_5();
add_2();
}
+ @hidden
action int_set_header_0407_i6() {
int_set_header_6();
int_set_header_5();
add_2();
}
+ @hidden
action int_set_header_0407_i7() {
int_set_header_7();
int_set_header_6();
int_set_header_5();
add_3();
}
+ @hidden
action int_set_header_0407_i8() {
int_set_header_4();
add_1();
}
+ @hidden
action int_set_header_0407_i9() {
int_set_header_7();
int_set_header_4();
add_2();
}
+ @hidden
action int_set_header_0407_i10() {
int_set_header_6();
int_set_header_4();
add_2();
}
+ @hidden
action int_set_header_0407_i11() {
int_set_header_7();
int_set_header_6();
int_set_header_4();
add_3();
}
+ @hidden
action int_set_header_0407_i12() {
int_set_header_5();
int_set_header_4();
add_2();
}
+ @hidden
action int_set_header_0407_i13() {
int_set_header_7();
int_set_header_5();
int_set_header_4();
add_3();
}
+ @hidden
action int_set_header_0407_i14() {
int_set_header_6();
int_set_header_5();
int_set_header_4();
add_3();
}
+ @hidden
action int_set_header_0407_i15() {
int_set_header_7();
int_set_header_6();
@@ -272,17 +316,18 @@
// We don't really need a key here, however we add a dummy one as a
// workaround to ONOS inability to properly support default actions.
key = {
- hdr.int_header.isValid(): exact @name("hdr.int_header.is_valid");
+ hdr.int_header.isValid(): exact @name("int_is_valid");
}
actions = {
init_metadata;
@defaultonly nop;
}
- const default_action = nop;
+ const default_action = nop();
size = 1;
}
// Table to process instruction bits 0-3.
+ @hidden
table tb_int_inst_0003 {
key = {
hdr.int_header.instruction_mask_0003 : exact;
@@ -305,7 +350,6 @@
int_set_header_0003_i14;
int_set_header_0003_i15;
}
- size = 16;
const entries = {
(0x0) : int_set_header_0003_i0();
(0x1) : int_set_header_0003_i1();
@@ -327,6 +371,7 @@
}
// Table to process instruction bits 4-7.
+ @hidden
table tb_int_inst_0407 {
key = {
hdr.int_header.instruction_mask_0407 : exact;
@@ -349,7 +394,6 @@
int_set_header_0407_i14;
int_set_header_0407_i15;
}
- size = 16;
const entries = {
(0x0) : int_set_header_0407_i0();
(0x1) : int_set_header_0407_i1();