facebook / fboss

Facebook Open Switching System Software for controlling network switches.
Other
860 stars 295 forks source link

i2c_smbus_* not defined #68

Closed bluecmd closed 6 years ago

bluecmd commented 6 years ago

Hi,

I'm attempting to compile fboss. I'm using Fedora 28.

[ 33%] Building CXX object CMakeFiles/fboss_agent.dir/fboss/agent/IPv6Handler.cpp.o
/home/bluecmd/fboss/fboss/agent/I2c.cpp: In member function ‘int facebook::fboss::I2cDevice::readBlock(int, uint8_t*)’:
/home/bluecmd/fboss/fboss/agent/I2c.cpp:42:13: error: ‘i2c_smbus_read_i2c_block_data’ was not declared in this scope
   auto rc = i2c_smbus_read_i2c_block_data(fd(), offset, I2C_BLOCK_SIZE,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/bluecmd/fboss/fboss/agent/I2c.cpp:42:13: note: suggested alternative: ‘i2c_smbus_ioctl_data’
   auto rc = i2c_smbus_read_i2c_block_data(fd(), offset, I2C_BLOCK_SIZE,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             i2c_smbus_ioctl_data
/home/bluecmd/fboss/fboss/agent/I2c.cpp: In member function ‘uint16_t facebook::fboss::I2cDevice::readWord(int)’:
/home/bluecmd/fboss/fboss/agent/I2c.cpp:51:15: error: ‘i2c_smbus_read_word_data’ was not declared in this scope
   auto data = i2c_smbus_read_word_data(fd(), offset);
               ^~~~~~~~~~~~~~~~~~~~~~~~
/home/bluecmd/fboss/fboss/agent/I2c.cpp:51:15: note: suggested alternative: ‘i2c_smbus_ioctl_data’
   auto data = i2c_smbus_read_word_data(fd(), offset);
               ^~~~~~~~~~~~~~~~~~~~~~~~
               i2c_smbus_ioctl_data
/home/bluecmd/fboss/fboss/agent/I2c.cpp: In member function ‘uint8_t facebook::fboss::I2cDevice::readByte(int)’:
/home/bluecmd/fboss/fboss/agent/I2c.cpp:59:15: error: ‘i2c_smbus_read_byte_data’ was not declared in this scope
   auto data = i2c_smbus_read_byte_data(fd(), offset);
               ^~~~~~~~~~~~~~~~~~~~~~~~
/home/bluecmd/fboss/fboss/agent/I2c.cpp:59:15: note: suggested alternative: ‘i2c_smbus_ioctl_data’
   auto data = i2c_smbus_read_byte_data(fd(), offset);
               ^~~~~~~~~~~~~~~~~~~~~~~~
               i2c_smbus_ioctl_data
/home/bluecmd/fboss/fboss/agent/I2c.cpp: In member function ‘void facebook::fboss::I2cDevice::read(int, int, uint8_t*)’:
/home/bluecmd/fboss/fboss/agent/I2c.cpp:71:10: error: ‘i2c_smbus_read_i2c_block_data’ was not declared in this scope
     rc = i2c_smbus_read_i2c_block_data(fd(), offset + i,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/bluecmd/fboss/fboss/agent/I2c.cpp:71:10: note: suggested alternative: ‘i2c_smbus_ioctl_data’
     rc = i2c_smbus_read_i2c_block_data(fd(), offset + i,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          i2c_smbus_ioctl_data
make[2]: *** [CMakeFiles/fboss_agent.dir/build.make:1090: CMakeFiles/fboss_agent.dir/fboss/agent/I2c.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:793: CMakeFiles/fboss_agent.dir/all] Error 2

I fixed this locally by installing libi2c-devel and patching like this:

diff --git a/fboss/agent/I2c.cpp b/fboss/agent/I2c.cpp
index d266afa..841f510 100644
--- a/fboss/agent/I2c.cpp
+++ b/fboss/agent/I2c.cpp
@@ -17,6 +17,7 @@ extern "C" {
 #include <sys/ioctl.h>
 #include <fcntl.h>
 #include <linux/i2c-dev.h>
+#include <i2c/smbus.h>
 }

 namespace facebook { namespace fboss {

I'm a bit confused why nobody seem to have reported this over the years, maybe it works differently on non-Fedora platforms.

capveg commented 6 years ago

[durh -- ignore my first comment] Hi,

Thanks for the interest. FBOSS can in theory compile against a fedora based distribution, but no one has tried it before. It's not surprising that there would be some minor header incompatibilities - thanks for pointing this one out. As hopefully you can see from the build instructions and the travis builds, this builds cleanly as is for ubuntu and Debian systems.

If you want to submit a patch for this or any other fedora compatibility tweaks, that would be much appreciated.

bluecmd commented 6 years ago

Yes, I kind of figured that Fedora was a bit of a stretch (pun intended) ;-).

FWIW, I tested on my Debian machine which is "testing/buster", and it appears that these functions are defined in the above file even here (after running getdeps.sh).

bluecmd@x:~/fboss$ lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux testing (buster)
Release:    testing
Codename:   buster
bluecmd@x:~/fboss$ grep i2c_smbus_read_i2c_block_data -R /usr/include/
/usr/include/i2c/smbus.h:extern __s32 i2c_smbus_read_i2c_block_data(int file, __u8 command, __u8 length,
bluecmd@x:/usr/include$ grep smbus.h -R .
./i2c/smbus.h:    smbus.h - SMBus level access helper functions
bluecmd@x:~/fboss$ grep smbus.h -R .

I'll see if I can find a Ubuntu 16.04 machine to test on as well, but it seems weird that the "linux/i2c-dev" which I guess is a kernel API would in some distros contain userspace library functions (libi2c-dev). Maybe libi2c-dev was spun out after Ubuntu 16.04?

capveg commented 6 years ago

Probably the easiest thing to do is download Open Network Linux and use the docker based build environment that's included with it. If you look at the build steps used in the .travis.yml build script, it should give you a feel for how to do this.

bluecmd commented 6 years ago

I ended up doing just that, and it worked. Weird, but at least the workaround is here if it's needed in the future.