libifconfig: Add functionality for querying SFP modules

[freqlabs]

Summary

This work is based on sbin/ifconfig/sfp.c

Test Plan

I have wrapped these additions in my WIP lua bindings for libifconfig here: https://gitlab.com/freqlabs/lua-libifconfig

Here is a short script to demonstrate some of the information provided by libifconfig:

#!/usr/libexec/flua

ifcfg = require('ifconfig').open()
ucl = require('ucl')

modules = ifcfg:foreach_iface(function(_, iface, mods)
    local name = iface:name()
    local info = ifcfg:get_sfp_info(name)
    if not info then
        return mods
    end
    mods[name] = {
        info = info,
        vendor_info = ifcfg:get_sfp_vendor_info(name),
        status = ifcfg:get_sfp_status(name),
        dump = tostring(ifcfg:get_sfp_dump(name)),
    }
    return mods
end, {})

print(ucl.to_json(modules))

The output (pretty-printed with json2yaml so the hex dumps I do in the Lua library look nicer):

ix1:
  status:
    channels:
    - tx_bias:
        raw: 3057
        mA: 6.114
      rx_power:
        raw: 1
        dBm: -40
        mW: 1.0e-4
    temperature: 39.425781
    voltage: 3.2927
  dump: |
    SFF8472 DUMP (0xA0 0..127 range):
    03 04 07 10 00 00 00 00 00 00 00 06 67 00 00 00
    08 03 00 1E 41 56 41 47 4F 20 20 20 20 20 20 20
    20 20 20 20 00 00 17 6A 41 46 42 52 2D 37 30 39
    53 4D 5A 20 20 20 20 20 47 34 2E 31 03 52 00 B4
    00 1A 00 00 41 44 31 37 30 33 33 30 42 32 44 20
    20 20 20 20 31 37 30 31 32 30 20 20 68 F0 03 EB
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  vendor_info:
    serial_number: AD170330B2D
    date: 2017-01-20
    name: AVAGO
    part_number: AFBR-709SMZ
  info:
    sfp_id:
      string: SFP or SFP+
      value: 3
      description: Transceiver identifier
    sfp_eth_10g:
      string: 10G Base-SR
      value: 16
      description: 10G Ethernet/IB compliance
    sfp_conn:
      string: LC
      value: 7
      description: Connector type
ix0:
  dump: |
    SFF8472 DUMP (0xA0 0..127 range):
    03 04 21 01 00 00 00 41 84 80 55 00 67 00 00 00
    00 00 05 00 44 45 4C 4C 20 20 20 20 20 20 20 20
    20 20 20 20 00 3C 18 A0 4C 35 36 51 46 30 33 30
    2D 53 44 2D 52 20 20 20 42 20 20 20 01 00 00 EB
    00 00 00 00 31 36 32 30 33 39 30 36 31 20 20 20
    20 20 20 20 31 36 30 37 32 31 20 20 00 00 00 1D
    0F 10 00 A3 84 00 00 FF FF FF FF FF FF FF FF FF
    FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF
  vendor_info:
    serial_number: '162039061'
    date: 2016-07-21
    name: DELL
    part_number: L56QF030-SD-R
  info:
    sfp_cab_tech:
      string: Passive Cable
      value: 4
      description: Channel/cable technology
    sfp_id:
      string: SFP or SFP+
      value: 3
      description: Transceiver identifier
    sfp_eth_10g:
      string: 1X Copper Passive
      value: 1
      description: 10G Ethernet/IB compliance
    sfp_fc_media:
      string: Twin Axial Pair (TW)
      value: 128
      description: Fibre Channel transmission media
    sfp_fc_len:
      string: short distance
      value: 64
      description: Fibre Channel link length
    sfp_conn:
      string: Copper pigtail
      value: 33
      description: Connector type

I will test this with various QSFP modules at some point, too.

[freqlabs]

Added documentation comments for public interface
Reorganized slightly to isolate boilerplate to the bottom of the header

[freqlabs]

"four channel" -> "four channels"

[melifaro]

What is the plan of switching ifconfig to use it? SFF standards are evolving, the code is not trivial, we regularly need to update either defines or the actual code. Also, I guess QSFP-DD / OSFP might require separate handling. I'd really prefer not to have 2 different kind of headers and 2 code implementations for that in base.

lib/libifconfig/libifconfig_sfp.c

59 ↗

(On Diff #73866)

This boilerplate seem also to be required only because of approach taken in .h

504 ↗

(On Diff #73866)

Can this be put in a separate file (if this code existence is really necessary) ?

lib/libifconfig/libifconfig_sfp.h

69 ↗

(On Diff #73866)

It is not always 4, 400G-FR8 / 400G-LR8 have 8 lanes.

112 ↗

(On Diff #73866)

s/WSFP/QSFP/

264 ↗

(On Diff #73866)

Sorry, do we really need these magic?
We can do a lot of stuff with preprocessor, but do we really need to?
This will complicate life of various tooling and IMHO this is harder to read for humans.

[melifaro]

Also: thank you for working on this! Access to to all of the sfp attributes in structured form is a long-waited stuff.

[freqlabs]

My current plan is to implement as much of the generally useful functionality of ifconfig in libifconfig as I can get away with. I think it would be good to start replacing bits of ifconfig with libifconfig, but I don't have an idea what a good timeline for that is, since I don't know if there is some reason it hasn't been started already.

lib/libifconfig/libifconfig_sfp.c

504 ↗

(On Diff #73866)

Sure, both of the macro sections could be hidden away in separate files. I can also add comments with instructions how to make changes. If not for shortcomings of the C preprocessor, most of the macro code here could be mechanically generated as well.

lib/libifconfig/libifconfig_sfp.h

69 ↗

(On Diff #73866)

This is how it is done in ifconfig, but that doesn't have an API that would break by adding more channel slots. Do you think allocating space as required is a better approach then?

264 ↗

(On Diff #73866)

I can get rid of the macros, it just adds a lot more redundant code to maintain in expanded form. The tooling is an important factor though.

I might play around with generating all this with flua instead, that way tooling will have the expanded code to grok and people will have something even easier to maintain.

[kp]

In D25494#563540, @freqlabs wrote:

My current plan is to implement as much of the generally useful functionality of ifconfig in libifconfig as I can get away with. I think it would be good to start replacing bits of ifconfig with libifconfig, but I don't have an idea what a good timeline for that is, since I don't know if there is some reason it hasn't been started already.

The main reason it hasn't been done yet is because no one has done the work. It's somewhere on my todo list, but that list tends to mostly just grow. It's probably on many peoples long-term todo list at this point.

I'd encourage anyone (and you especially) to just do it.

I'd like to see a man page for libifconfig at some point too, but I don't think that's blocking for ifconfig to start using the lib. There's no documentation for the ifconfig internals now either, so it's not as if we'd be worse off if we have ifconfig start using libifconfig before there's a man page for it.

[freqlabs]

Here's a direction I could take things. The boilerplate code is generated from a table in this Lua script (it could just as easily be pulled in from a separate json or yaml file by the script). This consolidates all of the relevant information into one place to ensure all the necessary code gets changed when updates are made.

The generated files are nicely formatted and documented, but they get output in the objdir instead of landing in the tree since the generator is driven by the Makefile. I wonder if it would be more or less desirable to commit the generated file to the tree instead.

The template.lua library may be introduced separately and that may not be where it gets placed, I just put it somewhere for now.

These are the files generated:

libifconfig_sfp_enums.c31 KBDownload

libifconfig_sfp_enums.h18 KBDownload

libifconfig_sfp_enums_internal.h3 KBDownload

[freqlabs]

• Split out the templates for each file so they feel a bit closer to C
• Utilize src.lua.mk in the Makefile
• Use a suffix rule to transform the templates into C

[freqlabs]

And here are what the generated files look like now.

libifconfig_sfp_tables.c32 KBDownload

libifconfig_sfp_tables.h17 KBDownload

libifconfig_sfp_tables_internal.h3 KBDownload

[freqlabs]

• Make (struct ifconfig_sfp_status).power a pointer to an array of length (struct ifconfig_sfp_status).channels so that more than 4 channels may be supported in the future without changing the struct layout.
• Add a corresponding function to free the above allocation.
• Fix missing comments.

Generated code:

libifconfig_sfp_tables.c32 KBDownload

libifconfig_sfp_tables.h18 KBDownload

libifconfig_sfp_tables_internal.h3 KBDownload

[melifaro]

Sorry for the late reply and thank you for addressing the comments!
It all looks good to me.
Though, can we switch ifconfig to actually use this library code?

[freqlabs]

• Use libifconfig for sfp status in ifconfig

In doing so I noticed we have been incorrectly displaying TX channel bias current as power in mW.
QSFP modules don't report TX power, only bias current, and SFP modules report both. I opted to make the output consistent and show TX bias current for both. The status struct in libifconfig has also changed to accommodate this realization, and I added a few more helper functions to convert the units reported by the modules into something more useful.

I also opted to adopt hexdump(3) from libutil for the I2C hex dumps. The only difference in output is an extra space in the indentation before each line. This could conceivably be eliminated by adding a flag in hexdump(3), but I didn't take that liberty.

[melifaro]

Thank you for making ifconfig(8) conversion!
LGTM, please see some minor comments inline.

sbin/ifconfig/sfp.c

108 ↗

(On Diff #74990)

Isn't this something we may want to abstract in the API either as a call or ifconfig_sfp_info_strings member?

tools/lua/template.lua

1 ↗

(On Diff #74990)

Is it needed for the sfp change?

[freqlabs]

Fixed 32-bit build

[freqlabs]

Will add an ifconfig_sfp_class function to select the description string from the appropriate field to describe the module's Ethernet specification compliance.

sbin/ifconfig/sfp.c

108 ↗

(On Diff #74990)

Certainly, I will do something about this.

tools/lua/template.lua

1 ↗

(On Diff #74990)

Yes, this is the template library used in sfp.lua to generate the C sources.

[freqlabs]

Add ifconfig_sfp_id_is_qsfp and ifconfig_sfp_physical_spec helpers