Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate temporary P4DeviceConfig message #462

Merged
merged 1 commit into from
Apr 11, 2019
Merged

Deprecate temporary P4DeviceConfig message #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions proto/demo_grpc/pi_server_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/

#include <PI/frontends/proto/device_mgr.h>
#include <PI/frontends/proto/logging.h>

#include <PI/proto/pi_server.h>

Expand Down Expand Up @@ -46,6 +47,28 @@ int main(int argc, char** argv) {
PIGrpcServerForceShutdown(1); // 1 second deadline
};

{
using pi::fe::proto::LogWriterIface;
using pi::fe::proto::LoggerConfig;
class P4RuntimeLogger : public LogWriterIface {
void write(Severity severity, const char *msg) override {
auto severity_map = [&severity]() -> const char * {
switch (severity) {
case Severity::TRACE : return "trace";
case Severity::DEBUG: return "debug";
case Severity::INFO: return "info";
case Severity::WARN: return "warn";
case Severity::ERROR: return "error";
case Severity::CRITICAL: return "critical";
}
return "unknown severity";
};
std::cout << "[P4Runtime] [" << severity_map() << "] " << msg << "\n";
}
};
LoggerConfig::set_writer(std::make_shared<P4RuntimeLogger>());
}

PIGrpcServerRunAddr(server_address);

// TODO(antonin): use sigaction?
Expand Down
26 changes: 16 additions & 10 deletions proto/frontend/src/device_mgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,11 +425,14 @@ class DeviceMgrImp {
RETURN_OK_STATUS();

p4::tmp::P4DeviceConfig p4_device_config;
if (!p4_device_config.ParseFromString(config.p4_device_config())) {
RETURN_ERROR_STATUS(
Code::INVALID_ARGUMENT,
"Invalid 'p4_device_config', not an instance of "
"p4::tmp::P4DeviceConfig");
const std::string *device_data = nullptr;
bool uses_legacy_p4_device_config = false;
if (p4_device_config.ParseFromString(config.p4_device_config())) {
device_data = &p4_device_config.device_data();
uses_legacy_p4_device_config = true;
Logger::get()->warn("p4::tmp::P4DeviceConfig is deprecated");
} else {
device_data = &config.p4_device_config();
}

auto lock = unique_lock();
Expand Down Expand Up @@ -459,7 +462,8 @@ class DeviceMgrImp {

// This is for legacy support of bmv2
if (action == SetConfigRequest::VERIFY_AND_COMMIT &&
p4_device_config.device_data().empty()) {
uses_legacy_p4_device_config &&
device_data->empty()) {
if (pi_is_device_assigned(device_id)) remove_device();
assert(!pi_is_device_assigned(device_id));
auto assign_options = make_assign_options();
Expand All @@ -478,8 +482,11 @@ class DeviceMgrImp {
if (action == SetConfigRequest::VERIFY_AND_SAVE ||
action == SetConfigRequest::VERIFY_AND_COMMIT ||
action == SetConfigRequest::RECONCILE_AND_COMMIT) {
if (pi_is_device_assigned(device_id) && p4_device_config.reassign())
if (uses_legacy_p4_device_config &&
pi_is_device_assigned(device_id) &&
p4_device_config.reassign()) {
remove_device();
}
if (!pi_is_device_assigned(device_id)) {
auto assign_options = make_assign_options();
pi_status = pi_assign_device(device_id, NULL, assign_options.data());
Expand All @@ -506,10 +513,9 @@ class DeviceMgrImp {
if (action == SetConfigRequest::VERIFY_AND_SAVE ||
action == SetConfigRequest::VERIFY_AND_COMMIT ||
action == SetConfigRequest::RECONCILE_AND_COMMIT) {
const auto &device_data = p4_device_config.device_data();
pi_status = pi_update_device_start(device_id, p4info_tmp,
device_data.data(),
device_data.size());
device_data->data(),
device_data->size());
if (pi_status != PI_STATUS_SUCCESS) {
pi_destroy_config(p4info_tmp);
RETURN_ERROR_STATUS(Code::UNKNOWN,
Expand Down
3 changes: 1 addition & 2 deletions proto/ptf/base_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
from p4.v1 import p4runtime_pb2
from p4.v1 import p4runtime_pb2_grpc
from p4.config.v1 import p4info_pb2
from p4.tmp import p4config_pb2
import google.protobuf.text_format

# See https://gist.github.com/carymrobbins/8940382
Expand Down Expand Up @@ -446,7 +445,7 @@ def set_match_key(self, table_entry, t_name, mk):
def set_action(self, action, a_name, params):
action_id = self.get_action_id(a_name)
if action_id is None:
self.fail("Failed to get id of action '{}' - perhaps the action name is misspelled?".format(a_name)
self.fail("Failed to get id of action '{}' - perhaps the action name is misspelled?").format(a_name)
action.action_id = action_id
for p_name, v in params:
param = action.params.add()
Expand Down
13 changes: 6 additions & 7 deletions proto/ptf/bmv2/gen_bmv2_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
import sys
import tempfile

from p4.tmp import p4config_pb2

def check_compiler_exec(path):
try:
with open(os.devnull, 'w') as devnull:
Expand Down Expand Up @@ -86,11 +84,12 @@ def main():
print "Error when writing to", args.out_p4info
sys.exit(1)

with open(args.out_bin, 'wf') as f_out:
with open(out_json, 'r') as f_json:
device_config = p4config_pb2.P4DeviceConfig()
device_config.device_data = f_json.read()
f_out.write(device_config.SerializeToString())
try:
shutil.copyfile(out_json, args.out_bin)
except:
print "Error when writing to", args.out_bin
sys.exit(1)

shutil.rmtree(tmp_dir)

if __name__ == '__main__':
Expand Down
11 changes: 2 additions & 9 deletions proto/tests/test_proto_fe.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@
#include <tuple>
#include <vector>

#include "p4/tmp/p4config.pb.h"

#include "PI/frontends/cpp/tables.h"
#include "PI/frontends/proto/device_mgr.h"
#include "PI/int/pi_int.h"
Expand Down Expand Up @@ -213,9 +211,7 @@ class DeviceMgrTest : public ::testing::Test {
p4v1::ForwardingPipelineConfig config;
config.set_allocated_p4info(&p4info_proto);
config.mutable_cookie()->set_cookie(cookie);
p4::tmp::P4DeviceConfig dummy_device_config_;
dummy_device_config_.set_device_data("This is a dummy device config");
dummy_device_config_.SerializeToString(&dummy_device_config);
dummy_device_config = "This is a dummy device config";
config.set_p4_device_config(dummy_device_config);
EXPECT_CALL(*mock, action_prof_api_support())
.WillRepeatedly(Return(action_prof_api_choice));
Expand Down Expand Up @@ -353,13 +349,10 @@ TEST_F(DeviceMgrTest, PipelineConfigGet) {
}

TEST_F(DeviceMgrTest, PipelineConfigGetLarge) {
std::string large_device_config;
std::string large_device_config(32768, 'a');
{
p4v1::ForwardingPipelineConfig config;
config.mutable_p4info()->CopyFrom(p4info_proto);
p4::tmp::P4DeviceConfig large_device_config_;
large_device_config_.set_device_data(std::string(32768, 'a'));
large_device_config_.SerializeToString(&large_device_config);
config.set_p4_device_config(large_device_config);
EXPECT_CALL(*mock, table_idle_timeout_config_set(
pi_p4info_table_id_from_name(p4info, "IdleTimeoutTable"), _));
Expand Down