Page MenuHomeVyOS Platform

Instantiating Interfaces without risk of creation
Closed, ResolvedPublic

Description

There is currently no good way to access the underlying information of an interface (mac address, oper_state): the current pattern assumes that the interface is correctly working and that it is safe to call to `Interface("interface_name") (or whatever subclass) and from then call the underlying function (get_mac, get_oper_state, etc.)

A recently patch allowed for the logging to be disabled, but there is still a risk for the interface to be created if something is not like expected (like the user deciding to remove the interface using Linux command ip link).

The code should provide a option when creating the interface which will skip the interface creation code def _create(..).
Something like EthernetIf("eth0", create=False) should prevent the interface creation.

All operational command should then be able to safely be instantiated with `InterfaceNameIf("name", create=False, debug=False) with the default being true

Where this pattern is used, when possible Interface should be replaced by the right Interface type too (EthernetIf, DummyIf, ...)

Details

Difficulty level
Unknown (require assessment)
Version
-
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Unspecified (possibly destroys the router)

Event Timeline

thomas-mangin created this object in space S1 VyOS Public.
pasik added a subscriber: pasik.Apr 1 2020, 7:00 AM
thomas-mangin closed this task as Resolved.Apr 7 2020, 3:02 PM
thomas-mangin claimed this task.
jjakob added a subscriber: jjakob.EditedApr 22 2020, 10:57 AM

There are places in the code where operational commands are still ran without create=False, for example:
https://github.com/vyos/vyos-1x/blob/675f400bacb03ae93be928e7270f89205d1036b9/src/conf_mode/interfaces-bonding.py#L242

and I'm sure other places too, a quick grep for all interface types should show all lines where it's used, and can then be checked.

This may possibly be the cause of some bugs?

Wouldn't it be better to default to create=False? there are much less places where a create=True is needed, I think just once in every interface's conf_mode apply() function. Whereas operational commands are ran in much more places.

thomas-mangin added a comment.EditedApr 22 2020, 3:34 PM
This may possibly be the cause of some bugs?

If the interface exists, it is perfectly harmless: it was the previous behaviour. The example where it can cause issue when a interface name is used and does not exists as it will create it. So really op_mode commands.

You are however right that best practice should be to have "create=False" until the apply section.