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

statd: add interface type to operational data #91

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

rical
Copy link
Contributor

@rical rical commented Jul 31, 2023

An iana or infix interface type is derived from "ip link" data. Such as loopback -> iana-if-type:softwareLoopback.

This patch adds support for the following types:
loopback, ethernetCsmacd, veth, l2vlan and bridge.

Example data:

{
  "ietf-interfaces:interfaces": {
    "interface": [
      {
        "name": "lo",
        "type": "iana-if-type:softwareLoopback",
        "oper-status": "unknown",
        "if-index": 1
      },
      {
        "name": "e0",
        "type": "iana-if-type:ethernetCsmacd",
        "oper-status": "up",
        "if-index": 2
      },
      {
        "name": "e0.100",
        "type": "iana-if-type:l2vlan",
        "oper-status": "down",
        "if-index": 3
      },
      {
        "name": "br0",
        "type": "iana-if-type:bridge",
        "oper-status": "down",
        "if-index": 4
      },
      {
        "name": "veth1",
        "type": "infix-if-type:veth",
        "oper-status": "down",
        "if-index": 5
      }
    ]
  }
}

@troglobit
Copy link
Contributor

@wkz what do you think, should this perhaps be derived from the interface type in the running datastore instead of duplicated in operational?

@rical
Copy link
Contributor Author

rical commented Aug 2, 2023

@wkz what do you think, should this perhaps be derived from the interface type in the running datastore instead of duplicated in operational?

Isn't the running datastore based on configuration data? I thought the idea with operational data was to get it directly from the source i.e. the running kernel.

@troglobit
Copy link
Contributor

Yes mostly, and that's the crux of it. The interface type is derived from a) physical interfaces that exist at first boot (factory-config), like ethernetCsmacd for all ethN/eN interfaces, but also loopback. Other types are inferred from the name when creating them, e.g. veth pairs. See ifchange_cand_infer_veth().

I'm just concerned we start bookkeeping this core knowledge in more than one place, so curious what @wkz thinks about it.

@wkz
Copy link
Contributor

wkz commented Aug 8, 2023

I agree that it is double bookkeeping, but I think it's worth it.

Random thoughts in no particular order:

  • Knowing that the operational state matches the kernel's view as closely as possible is very useful from a debugging perspective. It can give us insights into discrepancies between what the user configured and what has been applied, without needing to get shell access on the box.
  • One day, we might have interfaces that are not part of the configuration - that are dynamically created by some daemon, for example. What do we do at that point?
  • If we find a rouge interface on the box that isn't in the config, what type do we report then?

An iana or infix interface type is derived from "ip link" data.
Such as loopback -> iana-if-type:softwareLoopback.

This patch adds support for the following types:
loopback, ethernetCsmacd, veth, l2vlan and bridge.

Signed-off-by: Richard Alpe <richard@bit42.se>
@troglobit
Copy link
Contributor

Good points all around, and I agree. I'm just worried about duplication and keeping it all in sync. I guess we should put things like this in a checklist or auto test to ensure we don't drift apart in the various backends.

Also, see https://github.com/addiva-elektronik/infix-planning/issues/34

@wkz wkz merged commit 7e10274 into kernelkit:main Aug 8, 2023
1 check passed
@troglobit troglobit added this to the Infix v23.08 milestone Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants