From 4f984b86ee1badc21f2ae291bb2e1031e06bc37c Mon Sep 17 00:00:00 2001 From: Han Wang Date: Thu, 11 Jan 2018 17:46:48 -0800 Subject: [PATCH] proposed fix for #1135 --- frontends/p4/fromv1.0/converters.cpp | 56 +++++++++++----- frontends/p4/fromv1.0/programStructure.cpp | 6 ++ testdata/p4_14_samples/double_apply.p4 | 45 +++++++++++++ .../double_apply-first.p4 | 65 +++++++++++++++++++ .../double_apply-frontend.p4 | 65 +++++++++++++++++++ .../double_apply-midend.p4 | 60 +++++++++++++++++ .../p4_14_samples_outputs/double_apply.p4 | 63 ++++++++++++++++++ .../double_apply.p4-stderr | 0 8 files changed, 343 insertions(+), 17 deletions(-) create mode 100644 testdata/p4_14_samples/double_apply.p4 create mode 100644 testdata/p4_14_samples_outputs/double_apply-first.p4 create mode 100644 testdata/p4_14_samples_outputs/double_apply-frontend.p4 create mode 100644 testdata/p4_14_samples_outputs/double_apply-midend.p4 create mode 100644 testdata/p4_14_samples_outputs/double_apply.p4 create mode 100644 testdata/p4_14_samples_outputs/double_apply.p4-stderr diff --git a/frontends/p4/fromv1.0/converters.cpp b/frontends/p4/fromv1.0/converters.cpp index 55005b8dece..1cfbbe285e9 100644 --- a/frontends/p4/fromv1.0/converters.cpp +++ b/frontends/p4/fromv1.0/converters.cpp @@ -590,23 +590,7 @@ class ComputeCallGraph : public Inspector { public: explicit ComputeCallGraph(ProgramStructure* structure) : structure(structure) { CHECK_NULL(structure); setName("ComputeCallGraph"); } - void postorder(const IR::Apply* apply) override { - LOG3("Scanning " << apply->name); - auto tbl = structure->tables.get(apply->name.name); - if (tbl == nullptr) - ::error("Could not find table %1%", apply->name); - auto parent = findContext(); - BUG_CHECK(parent != nullptr, "%1%: Apply not within a control block?", apply); - auto ctrl = get(structure->tableMapping, tbl); - if (ctrl != nullptr && ctrl != parent) { - auto previous = get(structure->tableInvocation, tbl); - ::error("Table %1% invoked from two different controls: %2% and %3%", - tbl, apply, previous); - } - LOG3("Invoking " << tbl << " in " << parent->name); - structure->tableMapping.emplace(tbl, parent); - structure->tableInvocation.emplace(tbl, apply); - } + void postorder(const IR::V1Parser* parser) override { LOG3("Scanning parser " << parser->name); structure->parsers.add(parser->name); @@ -716,6 +700,43 @@ class ComputeCallGraph : public Inspector { } }; +/// Table call graph should be built after the control call graph is built. +/// In the case that the program contains an unused control block, the +/// table invocation in the unused control block should not be considered. +class ComputeTableCallGraph : public Inspector { + ProgramStructure *structure; + + public: + explicit ComputeTableCallGraph(ProgramStructure *structure) : structure(structure) { + CHECK_NULL(structure); + setName("ComputeCallGraph"); + } + + void postorder(const IR::Apply *apply) override { + LOG3("Scanning " << apply->name); + auto tbl = structure->tables.get(apply->name.name); + if (tbl == nullptr) + ::error("Could not find table %1%", apply->name); + auto parent = findContext(); + BUG_CHECK(parent != nullptr, "%1%: Apply not within a control block?", apply); + + auto ctrl = get(structure->tableMapping, tbl); + + // skip checking control block that is unused. + if (!structure->calledControls.isCallee(parent->name)) + return; + + if (ctrl != nullptr && ctrl != parent) { + auto previous = get(structure->tableInvocation, tbl); + ::error("Table %1% invoked from two different controls: %2% and %3%", + tbl, apply, previous); + } + LOG3("Invoking " << tbl << " in " << parent->name); + structure->tableMapping.emplace(tbl, parent); + structure->tableInvocation.emplace(tbl, apply); + } +}; + class Rewriter : public Transform { ProgramStructure* structure; public: @@ -1059,6 +1080,7 @@ Converter::Converter() { // Convert passes.emplace_back(new DiscoverStructure(&structure)); passes.emplace_back(new ComputeCallGraph(&structure)); + passes.emplace_back(new ComputeTableCallGraph(&structure)); passes.emplace_back(new Rewriter(&structure)); passes.emplace_back(new FixExtracts(&structure)); passes.emplace_back(new RemoveAnnotatedFields); diff --git a/frontends/p4/fromv1.0/programStructure.cpp b/frontends/p4/fromv1.0/programStructure.cpp index 733fb8cc9c6..2d347cf6ed7 100644 --- a/frontends/p4/fromv1.0/programStructure.cpp +++ b/frontends/p4/fromv1.0/programStructure.cpp @@ -1940,6 +1940,12 @@ void ProgramStructure::createControls() { for (auto c : controlsToDo) { auto ct = controls.get(c); + /// do not convert control block if it is not invoked + /// by other control block and it is not ingress or egress. + if (!calledControls.isCallee(c) && + ct != controls.get(v1model.ingress.name) && + ct != controls.get(v1model.egress.name)) + continue; auto ctrl = convertControl(ct, controls.get(ct)); if (ctrl == nullptr) return; diff --git a/testdata/p4_14_samples/double_apply.p4 b/testdata/p4_14_samples/double_apply.p4 new file mode 100644 index 00000000000..031d52acf02 --- /dev/null +++ b/testdata/p4_14_samples/double_apply.p4 @@ -0,0 +1,45 @@ +/* +Copyright 2013-present Barefoot Networks, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +header_type h { + fields { + b : 1; + } +} + +metadata h m; + +parser start { + return ingress; +} + +action x() {} + +table t { + actions { x; } +} + +control c { + apply(t); +} + +control e { + apply(t); +} + +control ingress { + c(); +} diff --git a/testdata/p4_14_samples_outputs/double_apply-first.p4 b/testdata/p4_14_samples_outputs/double_apply-first.p4 new file mode 100644 index 00000000000..0476d288b7f --- /dev/null +++ b/testdata/p4_14_samples_outputs/double_apply-first.p4 @@ -0,0 +1,65 @@ +#include +#include + +struct h { + bit<1> b; +} + +struct metadata { + @name(".m") + h m; +} + +struct headers { +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".start") state start { + transition accept; + } +} + +control c(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".x") action x() { + } + @name(".t") table t { + actions = { + x(); + @defaultonly NoAction(); + } + default_action = NoAction(); + } + apply { + t.apply(); + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".c") c() c_0; + apply { + c_0.apply(hdr, meta, standard_metadata); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + apply { + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + } +} + +control verifyChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; + diff --git a/testdata/p4_14_samples_outputs/double_apply-frontend.p4 b/testdata/p4_14_samples_outputs/double_apply-frontend.p4 new file mode 100644 index 00000000000..044ccd6491d --- /dev/null +++ b/testdata/p4_14_samples_outputs/double_apply-frontend.p4 @@ -0,0 +1,65 @@ +#include +#include + +struct h { + bit<1> b; +} + +struct metadata { + @name(".m") + h m; +} + +struct headers { +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".start") state start { + transition accept; + } +} + +control c(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".x") action x_0() { + } + @name(".t") table t_0 { + actions = { + x_0(); + @defaultonly NoAction(); + } + default_action = NoAction(); + } + apply { + t_0.apply(); + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".c") c() c_1; + apply { + c_1.apply(hdr, meta, standard_metadata); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + apply { + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + } +} + +control verifyChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; + diff --git a/testdata/p4_14_samples_outputs/double_apply-midend.p4 b/testdata/p4_14_samples_outputs/double_apply-midend.p4 new file mode 100644 index 00000000000..382af5d8a24 --- /dev/null +++ b/testdata/p4_14_samples_outputs/double_apply-midend.p4 @@ -0,0 +1,60 @@ +#include +#include + +struct h { + bit<1> b; +} + +struct metadata { + @name(".m") + h m; +} + +struct headers { +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".start") state start { + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("NoAction") action NoAction_0() { + } + @name(".x") action _x() { + } + @name(".t") table _t_0 { + actions = { + _x(); + @defaultonly NoAction_0(); + } + default_action = NoAction_0(); + } + apply { + _t_0.apply(); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + apply { + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + } +} + +control verifyChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; + diff --git a/testdata/p4_14_samples_outputs/double_apply.p4 b/testdata/p4_14_samples_outputs/double_apply.p4 new file mode 100644 index 00000000000..717306d5eb7 --- /dev/null +++ b/testdata/p4_14_samples_outputs/double_apply.p4 @@ -0,0 +1,63 @@ +#include +#include + +struct h { + bit<1> b; +} + +struct metadata { + @name(".m") + h m; +} + +struct headers { +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".start") state start { + transition accept; + } +} + +control c(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".x") action x() { + } + @name(".t") table t { + actions = { + x; + } + } + apply { + t.apply(); + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".c") c() c_0; + apply { + c_0.apply(hdr, meta, standard_metadata); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + apply { + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + } +} + +control verifyChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; + diff --git a/testdata/p4_14_samples_outputs/double_apply.p4-stderr b/testdata/p4_14_samples_outputs/double_apply.p4-stderr new file mode 100644 index 00000000000..e69de29bb2d