Skip to content

Commit

Permalink
Add major version number to P4Runtime protobuf packages (#356)
Browse files Browse the repository at this point in the history
* Add major version number to P4Runtime protobuf packages

We follow best practices for protobuf cloud services and add the major
version number to the package path. This change affects many C++ source
files in this project and I didn't find a good way to make the diff
smaller by using namespace "tricks". I did introduce namespace aliases
(p4rt = ::p4::v1 and p4config = ::p4::config::v1) to make future
potential changes easier.

Fixes #294

This commit also split p4/p4types.proto into p4/v1/p4data.proto and
p4/config/v1/p4types.proto, thus breaking the circular import in golang
between the p4 and p4.config packages.

Fixes #350

* Use p4v1 and p4configv1 namespace aliases

Instead of p4rt and p4config. This is based on a discussion with
reviewer @wmohsin about future-proofness, best practices and name
collisions.

Even though the fact that we were only using namespace aliases in
implementation files mean that there should be no name conflicts at link
time (namespace aliases are only visible in their translation unit and
do not affect the full-qualified name of linker symbols), we decided
that in the end it probably was simpler to just include the major
version number in the namespace alias. A change in major version number
should be a rare and disruptive event, and keeping the diff small when
updating the implementation may not be a valid concern. There will be
many things to figure out when & if we introduce P4Runtime v2 and it is
tough to predict how it will happen and which steps we will follow to
roll out the new version.
  • Loading branch information
antoninbas committed May 30, 2018
1 parent 8588f60 commit af06c17
Show file tree
Hide file tree
Showing 31 changed files with 869 additions and 791 deletions.
77 changes: 43 additions & 34 deletions proto/Makefile.am
Expand Up @@ -18,9 +18,10 @@ PROTOFLAGS = -I$(abs_srcdir) -I$(abs_builddir)

# Absolute paths are needed here for 'make distcheck' to work properly
protos = \
$(abs_srcdir)/p4/p4types.proto \
$(abs_srcdir)/p4/p4runtime.proto \
$(abs_srcdir)/p4/config/p4info.proto \
$(abs_srcdir)/p4/v1/p4data.proto \
$(abs_srcdir)/p4/v1/p4runtime.proto \
$(abs_srcdir)/p4/config/v1/p4info.proto \
$(abs_srcdir)/p4/config/v1/p4types.proto \
$(abs_srcdir)/google/rpc/status.proto \
$(abs_srcdir)/google/rpc/code.proto \
$(abs_srcdir)/p4/tmp/p4config.proto \
Expand All @@ -29,21 +30,24 @@ $(abs_srcdir)/gnmi/gnmi.proto
# Somehow, using an absolute path above prevents me from using EXTRA_DIST =
# $(protos)
EXTRA_DIST = \
p4/p4types.proto \
p4/p4runtime.proto \
p4/config/p4info.proto \
p4/v1/p4data.proto \
p4/v1/p4runtime.proto \
p4/config/v1/p4info.proto \
p4/config/v1/p4types.proto \
google/rpc/status.proto \
google/rpc/code.proto \
p4/tmp/p4config.proto \
gnmi/gnmi.proto

proto_cpp_files = \
cpp_out/p4/p4types.pb.cc \
cpp_out/p4/p4types.pb.h \
cpp_out/p4/p4runtime.pb.cc \
cpp_out/p4/p4runtime.pb.h \
cpp_out/p4/config/p4info.pb.cc \
cpp_out/p4/config/p4info.pb.h \
cpp_out/p4/v1/p4data.pb.cc \
cpp_out/p4/v1/p4data.pb.h \
cpp_out/p4/v1/p4runtime.pb.cc \
cpp_out/p4/v1/p4runtime.pb.h \
cpp_out/p4/config/v1/p4info.pb.cc \
cpp_out/p4/config/v1/p4info.pb.h \
cpp_out/p4/config/v1/p4types.pb.cc \
cpp_out/p4/config/v1/p4types.pb.h \
cpp_out/google/rpc/status.pb.cc \
cpp_out/google/rpc/status.pb.h \
cpp_out/google/rpc/code.pb.cc \
Expand All @@ -54,12 +58,14 @@ cpp_out/gnmi/gnmi.pb.cc \
cpp_out/gnmi/gnmi.pb.h

proto_grpc_files = \
grpc_out/p4/p4types.grpc.pb.cc \
grpc_out/p4/p4types.grpc.pb.h \
grpc_out/p4/p4runtime.grpc.pb.cc \
grpc_out/p4/p4runtime.grpc.pb.h \
grpc_out/p4/config/p4info.grpc.pb.cc \
grpc_out/p4/config/p4info.grpc.pb.h \
grpc_out/p4/v1/p4data.grpc.pb.cc \
grpc_out/p4/v1/p4data.grpc.pb.h \
grpc_out/p4/v1/p4runtime.grpc.pb.cc \
grpc_out/p4/v1/p4runtime.grpc.pb.h \
grpc_out/p4/config/v1/p4info.grpc.pb.cc \
grpc_out/p4/config/v1/p4info.grpc.pb.h \
grpc_out/p4/config/v1/p4types.grpc.pb.cc \
grpc_out/p4/config/v1/p4types.grpc.pb.h \
grpc_out/google/rpc/status.grpc.pb.cc \
grpc_out/google/rpc/status.grpc.pb.h \
grpc_out/google/rpc/code.grpc.pb.cc \
Expand All @@ -69,17 +75,19 @@ grpc_out/p4/tmp/p4config.grpc.pb.h \
grpc_out/gnmi/gnmi.grpc.pb.cc \
grpc_out/gnmi/gnmi.grpc.pb.h

includep4dir = $(includedir)/p4/
includep4dir = $(includedir)/p4/v1/
nodist_includep4_HEADERS = \
cpp_out/p4/p4types.pb.h \
grpc_out/p4/p4types.grpc.pb.h \
cpp_out/p4/p4runtime.pb.h \
grpc_out/p4/p4runtime.grpc.pb.h
cpp_out/p4/v1/p4data.pb.h \
grpc_out/p4/v1/p4data.grpc.pb.h \
cpp_out/p4/v1/p4runtime.pb.h \
grpc_out/p4/v1/p4runtime.grpc.pb.h

includep4configdir = $(includedir)/p4/config/
includep4configdir = $(includedir)/p4/config/v1/
nodist_includep4config_HEADERS = \
cpp_out/p4/config/p4info.pb.h \
grpc_out/p4/config/p4info.grpc.pb.h
cpp_out/p4/config/v1/p4info.pb.h \
grpc_out/p4/config/v1/p4info.grpc.pb.h \
cpp_out/p4/config/v1/p4types.pb.h \
grpc_out/p4/config/v1/p4types.grpc.pb.h

includep4tmpdir = $(includedir)/p4/tmp/
nodist_includep4tmp_HEADERS = \
Expand All @@ -105,16 +113,17 @@ AM_CPPFLAGS = -Icpp_out -Igrpc_out \
BUILT_SOURCES = $(proto_cpp_files) $(proto_grpc_files)

if HAVE_GRPC_PY_PLUGIN
p4pydir = $(pythondir)/p4
p4pydir = $(pythondir)/p4/v1
nodist_p4py_PYTHON = \
py_out/p4/p4types_pb2.py \
py_out/p4/p4runtime_pb2.py \
py_out/p4/__init__.py
py_out/p4/v1/p4data_pb2.py \
py_out/p4/v1/p4runtime_pb2.py \
py_out/p4/v1/__init__.py

p4configpydir = $(pythondir)/p4/config
p4configpydir = $(pythondir)/p4/config/v1
nodist_p4configpy_PYTHON = \
py_out/p4/config/p4info_pb2.py \
py_out/p4/config/__init__.py
py_out/p4/config/v1/p4info_pb2.py \
py_out/p4/config/v1/p4types_pb2.py \
py_out/p4/config/v1/__init__.py

# this one is temporary
p4tmppydir = $(pythondir)/p4/tmp
Expand Down Expand Up @@ -163,7 +172,7 @@ if HAVE_GRPC_PY_PLUGIN
# plugin inserts code into the proto-generated files). But maybe I am just using
# an old version of the Python plugin.
$(PROTOC) $^ --python_out $(builddir)/py_out $(PROTOFLAGS) --grpc_out $(builddir)/py_out --plugin=protoc-gen-grpc=$(GRPC_PY_PLUGIN)
@touch $(builddir)/py_out/p4/__init__.py $(builddir)/py_out/p4/config/__init__.py $(builddir)/py_out/p4/tmp/__init__.py
@touch $(builddir)/py_out/p4/v1/__init__.py $(builddir)/py_out/p4/config/v1/__init__.py $(builddir)/py_out/p4/tmp/__init__.py
@touch $(builddir)/py_out/google/__init__.py $(builddir)/py_out/google/rpc/__init__.py $(builddir)/py_out/gnmi/__init__.py
endif
@mv -f proto_files.tmp $@
Expand Down
4 changes: 2 additions & 2 deletions proto/PI/proto/util.h
Expand Up @@ -21,7 +21,7 @@
#ifndef PI_PROTO_UTIL_H_
#define PI_PROTO_UTIL_H_

#include <p4/config/p4info.pb.h>
#include <p4/config/v1/p4info.pb.h>

#include <cstdint>

Expand All @@ -35,7 +35,7 @@ using p4_id_t = uint32_t;

constexpr p4_id_t invalid_id() { return 0; }

p4::config::P4Ids::Prefix resource_type_from_id(p4_id_t p4_id);
p4::config::v1::P4Ids::Prefix resource_type_from_id(p4_id_t p4_id);

} // namespace util

Expand Down

0 comments on commit af06c17

Please sign in to comment.