Page MenuHomeVyOS Platform
Create Task
Maniphest T2910

XML: generator should support override of variables
Closed, ResolvedPublicFEATURE REQUEST
Actions

Description

All our interfaces have a default MTU of 1500 bytes. Some interfaces (PPPoE, L2TPv3, MACsec) do not have a default MTU of 1500 bytes, but as we inherit the XML definition we "pull-in" this invalid value.

The default value is later exchanged during runtime of the script here:

This simply "feels wrong" to change the value in the handler script. It would be nice to add an XML statement into interfaces-pppoe.xml.in stating:

#include <include/interface-mtu-68-1500.xml.i>
<leafNode name="mtu">
  <defaultValue>1492</defaultValue>
</leafNode>

to override the default value with the correct one on this interface. This would mean "true inheritance" like in C++/Python.

Details

Version
-
Is it a breaking change?
Unspecified (possibly destroys the router)
Issue type
Internal change (not visible to end users)

Related Objects
Search...

Event Timeline

c-po created this task.Sep 20 2020, 6:48 PM
c-po triaged this task as Normal priority.
c-po added subscribers: dmbaturin, jestabro.
jack9603301 subscribed.Sep 20 2020, 7:00 PM
pasik subscribed.Sep 20 2020, 7:43 PM
c-po moved this task from Need Triage to Backlog on the VyOS 1.3 Equuleus board.Dec 26 2020, 3:52 PM
jestabro claimed this task.Jan 19 2021, 7:30 PM
jestabro added a project: VyOS 1.4 Sagitta.
jestabro moved this task from Backlog to In Progress on the VyOS 1.3 Equuleus board.
jestabro changed the task status from Open to Needs testing.EditedJan 19 2021, 8:32 PM

Preprocessing solution in testing here:
https://github.com/vyos/vyos-1x/compare/current...jestabro:override-default

in response to the following constraints:
(1) Agreed that maintaining defaults in the conf_mode scripts feels incorrect, and defeats the purpose of the tag 'defaultValue'.
(2) Overriding of the tag within the XML must be processed before 'build-command-templates' is invoked, as the check against the schema will fail if more than one 'defaultValue' tag exists for a given path.
(3) Overriding of the tag within the XML must be processed after the preprocessing of the *.xml.in files, for the obvious reason that all instances will not be collected until after that step.

Consequently, a script is provided to run at the outset of 'make interface_definitions' to process the XML to collect, replace, and remove the appropriate elements, with result the updated 'defaultValue' contents.

The script is a terse application of lxml's xpath capability. The link above includes the test case outlined in the description.

jestabro added a comment.EditedJan 20 2021, 9:55 PM

Two minor comments left out of the above:
(1) The script in question runs a check on all interface definitions, however, there is no measurable increase in compile time.
(2) lxml will escape characters '>', '<', however these are unescaped by build-command-templates, so the issue is mooted. One can see the single line changed in the XML by running the test case above; it is noted that some interface definitions already have a mixed use of, say, '>' and '&gt;'.

jestabro added a comment.Jan 21 2021, 2:38 PM

Will remove test case and rebase to create PR and merge for 1.4; will later backport to 1.3 pending any issue.

jestabro added a comment.Jan 21 2021, 3:13 PM

PR https://github.com/vyos/vyos-1x/pull/692

Restricted Repository Identity mentioned this in rVYOSONEX173e4599394c: Merge pull request #692 from jestabro/override-default.Jan 21 2021, 3:18 PM
jestabro mentioned this in rVYOSONEXdd89255dcc82: xml: T2910: add override script to Makefile.Jan 21 2021, 3:18 PM
jestabro mentioned this in rVYOSONEX3dfbd19e0f36: xml: T2910: add script to support override of defaultValue elements.
jestabro mentioned this in T3239: XML: override 'defaultValue' for mtu of certain interfaces; remove workarounds.Jan 21 2021, 11:56 PM
jestabro added a subtask: T3239: XML: override 'defaultValue' for mtu of certain interfaces; remove workarounds.
jestabro closed this task as Resolved.Jan 22 2021, 12:02 AM
jestabro changed the task status from Resolved to Unknown Status.
jestabro closed subtask T3239: XML: override 'defaultValue' for mtu of certain interfaces; remove workarounds as Resolved.
jestabro changed the status of subtask T3239: XML: override 'defaultValue' for mtu of certain interfaces; remove workarounds from Resolved to Unknown Status.
jestabro moved this task from Open to Finished on the VyOS 1.4 Sagitta board.Feb 3 2021, 5:39 PM
jestabro mentioned this in rVYOSONEXc5d56604c972: xml: T2910: add script to support override of defaultValue elements.Feb 3 2021, 6:42 PM
jestabro mentioned this in rVYOSONEXd38fdaa3a91b: xml: T2910: add override script to Makefile.
jestabro changed the task status from Unknown Status to Resolved.Feb 3 2021, 6:43 PM
jestabro changed the status of subtask T3239: XML: override 'defaultValue' for mtu of certain interfaces; remove workarounds from Unknown Status to Resolved.
jestabro moved this task from In Progress to Finished on the VyOS 1.3 Equuleus board.
erkin set Issue type to Internal change (not visible to end users).Aug 29 2021, 12:58 PM
erkin removed a subscriber: Global Notifications.
syncer edited projects, added VyOS 1.3 Equuleus (1.3.0); removed VyOS 1.3 Equuleus.Aug 29 2022, 12:26 PM
syncer moved this task from Need Triage to Finished on the VyOS 1.3 Equuleus (1.3.0) board.