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.

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

gRPC build failure #436

Closed
hesingh opened this issue Jan 18, 2019 · 9 comments

Comments

@hesingh
Copy link

commented Jan 18, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2019

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

This comment has been minimized.

Copy link
Author

commented Jan 19, 2019

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

This comment has been minimized.

Copy link
Author

commented Jan 19, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jan 19, 2019

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

This comment has been minimized.

Copy link
Author

commented Jan 19, 2019

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

This comment has been minimized.

Copy link
Author

commented Jan 19, 2019

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
@BenRLewis

This comment has been minimized.

Copy link

commented Feb 22, 2019

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

This comment has been minimized.

Copy link
Author

commented Feb 23, 2019

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

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

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.

@antoninbas antoninbas closed this Jun 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.