p4lang / PI

An implementation framework for a P4Runtime server
Apache License 2.0
165 stars 107 forks source link

gRPC build failure #436

Closed hesingh closed 5 years ago

hesingh commented 5 years ago

I used Ubuntu 18.04 and latest g++, gcc, and clang.

I used the gRPC download and build instructions in the PI README.md file today, but gRPC fails to build. I had to change its code as shown below in diffs to build completely. gRPC, for several files does not use a 'break;' in switch case statements so that their code is brief by falling through. But with -Wall, the compiler fails with fall through error.

Do we have a liaison to gRPC team to send these errors to, or should we just a new version of gRPC?

diff --git a/Makefile b/Makefile
index 6a8037fca4..e85721c8ba 100644
--- a/Makefile
+++ b/Makefile
@@ -336,7 +336,7 @@ CXXFLAGS += -std=c++11
 else
 CXXFLAGS += -std=c++0x
 endif
-CPPFLAGS += -g -Wall -Wextra -Werror -Wno-long-long -Wno-unused-parameter -DOSATOMIC_USE_INLINED=1
+CPPFLAGS += -g -Wall -Wextra -Werror -Wno-long-long -Wno-unused-parameter -Wno-implicit-fallthrough -DOSATOMIC_USE_INLINED=1
 LDFLAGS += -g

 CPPFLAGS += $(CPPFLAGS_$(CONFIG))
diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c
index 88335ecd0b..56614d191b 1006<44
--- a/src/core/ext/transport/cronet/transport/cronet_transport.c
+++ b/src/core/ext/transport/cronet/transport/cronet_transport.c
@@ -664,7 +664,7 @@ static void create_grpc_frame(grpc_slice_buffer *write_slice_buffer,
   uint8_t *p = (uint8_t *)write_buffer;
   /* Append 5 byte header */
   /* Compressed flag */
-  *p++ = (flags & GRPC_WRITE_INTERNAL_COMPRESS) ? 1 : 0;
+  *p++ = (uint8_t)((flags & GRPC_WRITE_INTERNAL_COMPRESS) ? 1 : 0);
   /* Message length */
   *p++ = (uint8_t)(length >> 24);
   *p++ = (uint8_t)(length >> 16);
jafingerhut commented 5 years ago

I have a bash script linked below that applies several source code patches that I copied from an Ubuntu gRPC package, which I found helped enable gRPC 1.3.2 to compile on Ubuntu 18.04.

https://github.com/jafingerhut/p4-guide/blob/master/bin/install-p4dev-p4runtime.sh

Search for "Apply patch" in that script to see where it is done, if you are curious.

I haven't had success trying to use a newer version of gRPC on Ubuntu 18.04, but didn't spend more than an hour or three experimenting with options there.

hesingh commented 5 years ago

Excellent, thanks.

We do have to update the PI README.md to perhaps use a slightly higher gRPC version which includes the patches you are talking about once someone has figured out what version it is. My code changes also built and installed gRPC fine. I have to now use gRPC on my machine and see how it goes. I have used the CLI with PI, but have to check if it uses any gRPC and if not, I have to see what PI tool to test gRPC with. Otherwise, I will use simple C++ client/server code to test gRPC.

hesingh commented 5 years ago

Until we figure out which gRPC to use, I changed the README.md that would help. I also changed the README.md to say, the CLI infra uses Thrift. To newbies, this seems helpful because the newbie has to look for other tools to test PI using gRPC. I don't see the README.md mention any tool to test PI using gRPC either. So I add text related to that as well.

diff --git a/README.md b/README.md
index f0afc7d..d0d6b31 100644
--- a/README.md
+++ b/README.md
@@ -5,6 +5,13 @@
 **This repository has submodules; after cloning it you should run `git submodule
   update --init --recursive`.**

+## Install script
+
+To use one script that installs all that PI needs and also intalls PI,
+see the URL to the script below:
+
+https://github.com/jafingerhut/p4-guide/blob/master/bin/install-p4dev-p4runtime.sh
+
 ## Dependencies

 ### Base dependencies
@@ -119,6 +126,8 @@ p4runtime_proto_repositories()

 ## PI CLI

+PI CLI uses Thrift to communicate between client and server.
+
 For now the PI CLI supports an experimental version of `table_add` and
 `table_delete`. Because these two functions have been implemented in the bmv2 PI
 implementation, you can test the PI CLI with the bmv2 `simple_switch`. Assuming
@@ -132,6 +141,12 @@ bmv2 is installed on your system, build the PI and the CLI with `./configure
     PI CLI> table_dump ipv4_lpm
     PI CLI> table_delete ipv4_lpm <handle returned by table_add>

+## gRPC Testing using protobuf .proto definitions.
+
+See the README.md in following path from the root PI directory.
+
+./proto/p4runtime/proto/README.md
+
 ## Contributing

 All contributed code must pass the style checker, which can be run with
jafingerhut commented 5 years ago

I personally do not mind the link to my script in the README, but it would also be very appropriate to add a copy of the script to this repository, if the maintainers of this repo think it is a useful thing to include.

hesingh commented 5 years ago

I haven't had success trying to use a newer version of gRPC on Ubuntu 18.04, but didn't spend more than an hour or three experimenting with options there.

I am not there yet using gRPC with PI. So I used a test C++ example in grpc and tested grpc to work on my Ubuntu 18.04 machine. These are the notes related to the example.

https://grpc.io/docs/tutorials/basic/c.html

hesingh commented 5 years ago

I found demo_gprc in the PI repo and could build and run it. The build built a different server than what is mentioned in PI/proto/demo_gprc/README.md. So this README.md changed. See new diffs below for both README.md files.

diff --git a/README.md b/README.md
index f0afc7d..b45b0c6 100644
--- a/README.md
+++ b/README.md
@@ -5,6 +5,13 @@
 **This repository has submodules; after cloning it you should run `git submodule
   update --init --recursive`.**

+## Install script
+
+To use one script that installs all that PI needs and also intalls PI,
+see the URL to the script below:
+
+https://github.com/jafingerhut/p4-guide/blob/master/bin/install-p4dev-p4runtime.sh
+
 ## Dependencies

 ### Base dependencies
@@ -119,6 +126,8 @@ p4runtime_proto_repositories()

 ## PI CLI

+PI CLI uses Thrift to communicate between client and server.
+
 For now the PI CLI supports an experimental version of `table_add` and
 `table_delete`. Because these two functions have been implemented in the bmv2 PI
 implementation, you can test the PI CLI with the bmv2 `simple_switch`. Assuming
@@ -132,6 +141,11 @@ bmv2 is installed on your system, build the PI and the CLI with `./configure
     PI CLI> table_dump ipv4_lpm
     PI CLI> table_delete ipv4_lpm <handle returned by table_add>

+## gRPC Testing
+
+See proto/demo_grpc/README.md which describes the demo.
+
+
 ## Contributing

 All contributed code must pass the style checker, which can be run with
diff --git a/proto/demo_grpc/README.md b/proto/demo_grpc/README.md
index 26af06c..4886dce 100644
--- a/proto/demo_grpc/README.md
+++ b/proto/demo_grpc/README.md
@@ -29,7 +29,7 @@ calls (using the PI C++ frontend).

 To run the demo, you will need 3 terminal instances:
 - `sudo python 1sw_demo.py --cpu-port veth250`
-- `sudo ./pi_grpc_server`
+- `sudo ./pi_server_dummy`
 - `sudo ./controller -c simple_router.json -p simple_router.p4info.txt`

 Note that the demo assumes that you have a veth250 / veth251 veth pair on your
b3n-l commented 5 years ago

My workaround solution has been to use gcc-6 which allows all the dependencies for PI to compile on 18.04. You can set the environment variable CC to the version you wish to use e.g CC=“gcc-6”

You can install gcc-6 using apt without impacting the rest of the system. 7 remains the default version.

hesingh commented 5 years ago

I downloaded latest gRPC today (02/22/2019). I believe it's version 1.18.x. I also downloaded latest PI and its make check fails for few tests. Clearly, we need to step gingerly before upgrading to newer version of gRPC.

PASS: test_p4info_convert
PASS: test_proto_fe_packet_io
PASS: test_proto_fe_set_pipeline_config
../test-driver: line 107: 45202 Segmentation fault      (core dumped) "$@" > $log_file 2>&1
FAIL: test_server_no_pipeline_config
../test-driver: line 107: 45222 Segmentation fault      (core dumped) "$@" > $log_file 2>&1
FAIL: test_server_gnmi
PASS: test_proto_fe_digest
../test-driver: line 107: 45245 Segmentation fault      (core dumped) "$@" > $log_file 2>&1
FAIL: test_server_arbitration
../test-driver: line 107: 45251 Segmentation fault      (core dumped) "$@" > $log_file 2>&1
FAIL: test_pi_server
PASS: test_proto_fe
PASS: test_task_queue
============================================================================
Testsuite summary for PI-proto 0.1
============================================================================
# TOTAL: 10
# PASS:  6
# SKIP:  0
# XFAIL: 0
# FAIL:  4
# XPASS: 0
# ERROR: 0
============================================================================
See tests/test-suite.log
============================================================================
Makefile:938: recipe for target 'test-suite.log' failed
make[5]: *** [test-suite.log] Error 1
make[5]: Leaving directory '/home/hemant/Downloads/pi/proto/tests'
Makefile:1044: recipe for target 'check-TESTS' failed
make[4]: *** [check-TESTS] Error 2
make[4]: Leaving directory '/home/hemant/Downloads/pi/proto/tests'
Makefile:1180: recipe for target 'check-am' failed
make[3]: *** [check-am] Error 2
make[3]: Leaving directory '/home/hemant/Downloads/pi/proto/tests'
Makefile:1497: recipe for target 'check-recursive' failed
make[2]: *** [check-recursive] Error 1
make[2]: Leaving directory '/home/hemant/Downloads/pi/proto'
Makefile:1786: recipe for target 'check' failed
make[1]: *** [check] Error 2
make[1]: Leaving directory '/home/hemant/Downloads/pi/proto'
Makefile:459: recipe for target 'check-recursive' failed
make: *** [check-recursive] Error 1
hemant@ubuntu:~/Downloads/pi$ 
antoninbas commented 5 years ago

Closing this. Some changes were made to make sure the code works with more recent versions of Protobuf / gRPC, but the officially supported versions (and the ones used for CI) are Protobuf 3.2.0 and gRPC 1.3.2.