Roopa Prabhu [Mon, 8 Aug 2016 22:46:13 +0000 (15:46 -0700)]
addons: address: never reset mtu on lo implicity
we should leave the mtu on lo to the default mtu
if user has not requested to change it. lo has
a larger mtu because it does not really depend on a
physical mtu.
vrf slave brings up the master if master is not up yet.
Today this is done only when ALL (auto) option is set
just as an optimization. because you dont want to bring
up the master in cases where user just wants to
bring up the vrf slave. eg ifup -v eth0.
This does not work so well, when user uses
--allow classes to bring up vrf master and slaves
together (eg mgmt vrf).
This patch removes the ALL check when bringing
up master and replaces it with an ALL or
CLASS check. ie make sure vrf master belongs to the
same class as you when CLASS is specified.
Roopa Prabhu [Fri, 5 Aug 2016 18:28:28 +0000 (11:28 -0700)]
ifupdown: move IFACE_CLASS ifupdownmain flag to ifupdownflags.CLASS
This patch moves ifupdownmain flag IFACE_CLASS to
ifupdownflags.CLASS. ifupdownflags.CLASS is set to true
if the user is asking for a class of interfaces.
example: ifreload --allow=mgmt
By moving it to ifupdownflags, we make it visible
to add modules.
Roopa Prabhu [Wed, 3 Aug 2016 22:35:04 +0000 (15:35 -0700)]
addons: bridge: fix bridge vid error on ifaces with multiple stanzas
Ticket: CM-12151
Reviewed By: julien, nikhil
Testing Done: Tested bridge vid errors with multiple iface stanzas for
ports
This patch just uses the HAS_SIBLINGS and OLDEST_SIBLINGS
flags on an iface to make sure we flag the error
on the last interface ie oldest of the siblings. all interfaces
with multiple iface objects have HAS_SIBLINGS set and the last iface
has OLDEST_SIBLINGS set.
The other way to fix this for the user would be to set ifaceobj_squash=1
in /etc/network/ifupdown2/ifupdown2.conf
Roopa Prabhu [Wed, 3 Aug 2016 19:22:14 +0000 (12:22 -0700)]
addons: bridge: reset vlan_filtering when user moves from vlan aware to
unaware
Ticket: CM-12070
Reviewed By: julien, nikhil
Testing Done: Tested with config in the bug
The bug also talks about removing the vlans during this
transition. If the vlans are removed from the interfaces file,
ifupdown2 will delete them on an ifreload. In any case, once moved to
vlan unaware bridge settings, kernel and switchd
don't look at vlans any more.
Julien Fortin [Mon, 1 Aug 2016 07:49:08 +0000 (09:49 +0200)]
ifupdownaddons: mstpctl: treeportprio value is cached. It prevents ifupdown2 from executing mstpctl settreeportprio commands when the running value is the same as the one previously cached
Ticket: CM-11773
Reviewed By: Roopa, Nikhil G
Testing Done: smoke + hand testings :
$ cat /etc/network/interfaces
auto br2
iface br2
bridge-vlan-aware yes
bridge-vids 100
bridge-pvid 1
bridge-ports swp1 swp4 swp5
bridge-stp on
$ ifreload -a -v
[...]
info: netlink: set link swp1 up
info: netlink: set link swp4 up
info: netlink: set link swp5 up
info: br2: applying mstp configuration specific to ports
info: br2: processing mstp config for port swp1
info: executing /sbin/mstpctl showportdetail br2 json
info: executing /sbin/mstpctl settreeportprio br2 swp1 0 128
info: br2: processing mstp config for port swp4
info: executing /sbin/mstpctl settreeportprio br2 swp4 0 128
info: br2: processing mstp config for port swp5
info: executing /sbin/mstpctl settreeportprio br2 swp5 0 128
info: netlink: set link br2 up
[...]
$
/sbin/mstpctl settreeportprio br2 swpX 0 128 is executed for every port.
ifupdown2 tries to set this attr to 128 (the default value). This is a
problem at scale. We shouldn't try to set this value if the running value
is already 128.
With the attached patch:
$ ifreload -a
[...]
info: netlink: set link swp1 up
info: netlink: set link swp4 up
info: netlink: set link swp5 up
info: executing /sbin/mstpctl showportdetail br2 json
info: br2: applying mstp configuration specific to ports
info: br2: processing mstp config for port swp1
info: br2: processing mstp config for port swp4
info: br2: processing mstp config for port swp5
info: netlink: set link br2 up
[...]
$
It's possible to combine a keyword with a range from validrange. example:
validrange: 10-50
validvals: <intrface-range-list>
value: swp1=21 swp2=42 ...
addons: addressvirtual: enslave macvlans on vrf slaves to the vrf master
Ticket: CM-11803
Reviewed By: dsa, scotte, wkok, nikhil, julien
Testing Done: tested config of address-virtual lines on vrf slaves
This patch does the following:
- addressvirtual: enslaves macvlans created on vrf slaves
to the vrf master
- vrf: when looking for stale slaves on vrf master, skip
macvlan devices. This code does basic checking right now
and can be improved to include more cases.
addons: bridge: fix 'ifquery -c' 'bridge vid error' on vlan aware bridge port
Ticket: CM-11811
Reviewed By: roopa, julien
Testing Done: used same configuration from ticket
For a VLAN aware bridge, if 'bridge-pvid' is not configured,
auto bridge
iface bridge
bridge-ports swp7 swp8
bridge-vids 1-200
bridge-vlan-aware yes
'1' is would be running pvid and [2-200] would be running vids.
While doing _query_check() we compare configred versus running
vids excluding running 'pvid'. Configured vids here is [1-200]
and running is [2-200], but instead of excluding running 'pvid'
we used to exclued configured 'pvid' which is None in this case.
Comparison used to fail because of this.
This patch excludes running 'pvid' instead of configured 'pvid'
during _query_check()
This patch also fixes 'ifquery -c' error for bridge-pvid if
configured under a port of vlan aware bridge
This patch also avoids printing 'bridge-pvid' on 'ifquery -c'
if bridge-pvid is not configured under vlan aware bridge port
addons: bridge: fix 'ifquery -c' 'bridge vid error' on vlan aware bridge port
Ticket: CM-11811
Reviewed By: roopa, julien
Testing Done: used same configuration from ticket
For a VLAN aware bridge, if 'bridge-pvid' is not configured,
auto bridge
iface bridge
bridge-ports swp7 swp8
bridge-vids 1-200
bridge-vlan-aware yes
'1' is would be running pvid and [2-200] would be running vids.
While doing _query_check() we compare configred versus running
vids excluding running 'pvid'. Configured vids here is [1-200]
and running is [2-200], but instead of excluding running 'pvid'
we used to exclued configured 'pvid' which is None in this case.
Comparison used to fail because of this.
This patch excludes running 'pvid' instead of configured 'pvid'
during _query_check()
This patch also fixes 'ifquery -c' error for bridge-pvid if
configured under a port of vlan aware bridge
ethtool module should really care only about
physical interfaces. so, this patch makes ethtool
module ignore all logical interfaces, ie interfaces
with link_kind set.
addons: usercmds: adding environment variables passed by the ifupdown2 to user scripts
Ticket: None
Reviewed By: Roopa
Testing Done: with a dummy script I printed the value passed by ifupdown
After the changes I did the same with ifupdown2 and checksd if the values
matched correctly.
ifupdown: networkinterfaces: not importing template engine if no mako keyword found
Ticket: CM-11807
Reviewed By: Roopa, Daniel
Testing Done:
Loading the default template engine (mako) is adding an extra overhead: 100ms
We also import the template engine even if we are dealing with a json input
The overhead is noticable when using NCLU.
With this change we are now important the template engine when a keyword is
found in the configuration file.
ifupdown: fixes to make addon scripts work via special config options
ifupdown2 can invoke scripts under /etc/network/if-*.d/*
in the required order with the required environment variables.
This patch includes fixes to execute these scripts.
The following attributes in /etc/network/ifupdown2/ifupdown.conf
can influence the execution behaviour of python-addon modules
and /etc/network/if-*.d/*
addons: addressvirtual: allowing address-virtual attr for vrf slave interfaces
Ticket: CM-11745
Reviewed By: Roopa, Nikhil G
Testing Done: configuration provided in the bug
User was attempting to configure a traditional bridge under a VRF with VRR.
When issuing ifreload -a, the configuration failed to apply with an error.
Applying an address-virtual keyword to an interface with upper interfaces
or parent interfaces was not allowed. But now we are allowing the use of
this keyword only for vrf slaves.
iproute2 was parsing the '/sbin/bridge -c vlan show' output with the assumption that
pvid line '711 PVID Egress Untagged' appears before all the vland ids.
Something like this:
port vlan ids
swp1 711 PVID Egress Untagged
600
700-710
712-900
Sam Tannous [Fri, 8 Jul 2016 00:16:22 +0000 (17:16 -0700)]
ifupdown2 defaults for link attributes are not applied
Ticket: CM-11718
Reviewed By: CCR-4931
Testing Done: Tested complete regression suite on hardnode in 3.0.
This patch fixes a problem in the ethtool addon module where a single iface stanza was
configured for a link-speed (1G) other then the default (10G). The link-speed config is then
removed or commented out but the link-speed is not restored to its default value (10G) because of
incorrect logic.
Nikhil [Wed, 29 Jun 2016 23:38:50 +0000 (16:38 -0700)]
addons: bridge: ifquery -c fix for port attributes
Ticket: CM-11195
Reviewed By: roopa, julien
Testing Done: used the configuration mentioned in bug description
Signed-off-by: Nikhil <nikhil@cumulusnetworks.com>
running values for bridge-portmcrouter, bridge-portmcfl, and bridge-portprios
were accessed using invalid keys.
Julien Fortin [Sun, 26 Jun 2016 23:24:18 +0000 (00:24 +0100)]
nlmanager: nlpacket: throw an error if the user of nlmanager tries to encode an attribute that we haven't added an encode() method for
+ adding one byte attribute class for protodown operations.
Ticket: CM-11581
Reviewed By: CCR-4721
Testing Done: Smoke + custom interface file with clag bond that requires protodown.
- nlmanager will now throw an exception if a user is trying to use the default
attribute class when in fact he should use a more specific attribute class.
- The protodown implementation needed to use a one byte attribute to set the protodown state
Scott Emery [Thu, 23 Jun 2016 21:58:14 +0000 (14:58 -0700)]
ifupdown2: Disable IPv6 duplicate address detection on VRR interfaces
Ticket: CM-11511
Reviewed By: CCR-4890
Testing Done: Used "address-virtual" keyword to create VRR interface with IPv6
address. Checked sysctl and dad failures.
In a VRR setup, both switches are programmed with the same IPv6 address and that
address is active on both switches. This causes the IPv6 duplicate address
detection to kick in and diable the use of one of the address on one of the
switches. This patch causes duplicate address detection to be disabled on VRR
interfaces by setting the net.ipv6.conf.<ifname>.accept_dad and
net.ipv6.conf.<ifname>.dad_transmits sysctl's to 0. The only IPv6 addresses
which are defined on these interfaces are the virtual addresses and the link
local address (which is unused). No other operational IPv6 addresses should ever
be assigned to these interfaces. Instead, operational IPv6 addresses should be
assigned to the "base", or lower, interface of the VRR interface.
Julien Fortin [Tue, 21 Jun 2016 14:17:09 +0000 (15:17 +0100)]
addons: vlan: ifquery will try to get the vlan-id from the ifacename if vlan-id attr is missing
Ticket: CM-11485
Reviewed By: Roopa
Testing Done: config from the github issue
First reported here https://github.com/CumulusNetworks/ifupdown2/issues/10
ifquery just like ifupdown should try to get the vlan-id from the iface name
if vlan-id attr is not specified.
Nikhil [Wed, 25 May 2016 23:16:22 +0000 (16:16 -0700)]
addons: vrf: Fix to kill current ssh on 'sudo ifreload -a' to enable mgmt VRF
Ticket: CM-11080
Reviewed By: roopa, dsa, dave olson, daniel, julien
Testing Done: yes, with mgmt VRF configured
This patch parses '/usr/bin/pstree -Aps <pid>' output to find
the pid of current ssh session, and send 'sudo ifreload -a'
to background before killing itself.
Nikhil [Mon, 23 May 2016 16:06:10 +0000 (09:06 -0700)]
addons: bridge: Fix for ifquery -c bridge pvid error to a valid config
Ticket: CM-8623
Reviewed By: Roopa Prabhu, Julien Fortin
Testing Done: yes, using the below mentioned config.
This patch fixes 'ifquery -c' error, bridge pvid error, which is an
unrelated error for the following valid config.
This patch also removes 'bridge-pvid', if configured, from 'bridge-vids' set.
In the below example, 510 is removed from the set bridge-vids 510-550.
The new bridge-vids set is 511-550.
auto swp1
iface swp1 inet6 dhcp
address 14.0.0.5/30
down ip addr flush dev swp1
auto Bridge1
iface Bridge1
address 0.0.0.0/0
down ip addr flush dev Bridge1
bridge-vlan-aware yes
bridge-ports swp1
bridge-pvid 510
bridge-vids 510-550
bridge-stp on
mstpctl-portnetwork swp1=yes
ifreload always deletes and adds back a host IP address
if that address is specified without a "/" and without a "netmask" config.
The problem is fixed so that _inet_address_convert_to_cidr() handles a missing
"/" and a missing "netmask" config. The host addresses is no longer added to a
list of addresses we will del/add. The patch was simply to add a case for the
missing "/" and a missing "netmask".
ifupdown2.TestMakoJson test was failing because the json values werent trimmed
Because of a trailing whitespace, a list of iface had an empty value, ifupdown2
was throwing an exception.
If called with close_fds=True the subprocess module will try to close every fd
from 3 to MAXFD before executing the specified command. This is done in Python
not even with a C-implementation which truly affecting performances.
This patch aims to better handle the file descriptor used by ifupdown2. Either
by closing them after use or by setting the close-on-exec flag for the file
descriptor, which causes the file descriptor to be automatically
(and atomically) closed when any of the exec-family functions succeed.
With the actual patch all tests are passing, I can't think of any future issue
but if any a possible future modification might be to use the parameter
'preexec_fn', which allows us to set function which will be executed in the
child process before executing the command line. We can always manually close
any remaining open file descriptors with something like:
>>> os.listdir('/proc/self/fd/')
['0', '1', '2', ‘3’, etc..]
>>> for fd in os.listdir('/proc/self/fd/')
>>> if int(fd) > 2:
>>> os.close(fd)
This patch is also totally re-organising the use of subprocesses. By removing
all subprocess code redundancy.
Roopa Prabhu [Sat, 14 May 2016 05:44:50 +0000 (22:44 -0700)]
addons: vrf: vrf_slave: up vrf dev only when all interfaces are being
brought up
Ticket: CM-10954, CM-10953
Reviewed By: julien
Testing Done: ran ifupdown2 smoke and vrf ifup/ifdown testing
- vrf master if not around is brought up when the first slave is brought up
by design (because we want the slaves to be
enslaved to a vrf master before addresses are configured).
and master is not brought up by first because interfaces
are brought up down to top in the dependency tree.
- This patch makes sure a slave brings up a vrf master only
when all interfaces are specified. When an individual interface
is brought up, skip master bring up and throw an error.
- Since the addon modules also need to use the ALL and WITH_DEPENDS
flags, this patch moves them to the existing global ifupdownflags
class
- vrf module uses the ifupdownflags.ALL flag to not bring up the master
when only the slave is being brought up
example:
ifup <vrf> # brings up the vrf device
ifup <vrf> --with-depends # brings up the vrf dev and
# its slaves
ifup <vrf_slave> # if vrf master is not around,
# an error is thrown
ifup <vrf_slave> --with-depends # will still not bring up the vrf master
ifdown <vrf> # deletes vrf dev and flushes the
# addresses on vrf slaves
ifup <vrf> # brings up vrf dev and does not
# up the adresses on the slaves
ifup <vrf> --with-depends # will bring up vrf and reapply config
# on slaves (including addresses)
Roopa Prabhu [Tue, 10 May 2016 01:48:52 +0000 (18:48 -0700)]
addons: address: force reapply addresses during dhcp to static trasition
Ticket: CM-10857
Reviewed By: julien, nikhil
Testing Done: Tested moving from a dhcp to static addr method for eth0
before the patch:
- when moving from dhcp to static address method, after releasing the dhcp
address, it takes a few seconds for the dhcp address to go away. When we
query addresses to configure, there is a chance that we may end up
getting the dhcp address for a few seconds. Which is ok until when the
static address to be configured is same as the dhcp address.
In which case we end up not configuring the static address because we
think it is already configured (per the running list).
After this patch:
- when moving from dhcp to static address method, we always re-apply the
address config avoiding the chance of not configuring an address due to a
dhcp address sticking around.
Roopa Prabhu [Mon, 9 May 2016 20:50:56 +0000 (13:50 -0700)]
addons: address: fix call to dhclient is_running
Ticket: CM-10857
Reviewed By: julien, nikhil
Testing Done: Tested moving from a static to dhcp addr method for eth0
the check to see if dhclient is running on an interface was failing.
which caused the following problem:
- interface when moved from dhcp to static addr_method was
not killing the existing dhcp process
- and a subsequent move from static to dhcp addr_method quit starting
dhcp because a dhclient process was already running
- but the original dhcp ip was already removed because of the
static address config which leaves eth0 in a weird state
After this patch:
- move from dhcp to static addr_method will release any
existing dhcp leases on an interface
- a subsequent move from static to dhcp will request a new
dhcp lease
Roopa Prabhu [Sun, 8 May 2016 02:32:23 +0000 (19:32 -0700)]
addons: mstpctl: call disable_ipv6 only if mstpctl_ports is specified
Ticket:
Reviewed By: trivial
Testing Done: tested sanity and bridge bringup and reload
bridge module already disables ipv6 on ports when bridge-ports is
specified. Making this change because i saw redundant calls to
disable_ipv6 on bridge ports by modules bridge and mstpctl
when bridge-ports is specified. It is slowing down ifreload
Roopa Prabhu [Sun, 8 May 2016 01:27:05 +0000 (18:27 -0700)]
ifupdownmain: fix error check when an iface is removed but still exists as a dependency
Ticket: CM-8095
Reviewed By: julien, nikhil
Testing Done: Tested ifupdown2 sanity and unclean removal of interfaces from the
interfaces file
example: for the below interfaces file,
{noformat}
auto bond2
iface bond2 inet static
address 100.0.0.2/16
bond-slaves swp3 swp4
auto bridge
iface bridge
bridge-vlan-aware yes
bridge-ports swp1 swp2 bond2
bridge-vids 12 505
bridge-stp on
auto swp1
iface swp1
mtu 9000
{noformat}
when iface sections for bond2 and swp1 are removed from the interfaces file
but left as dependents of bridge, on an ifreload:
prior to this patch:
===================
- ifreload would throw a warning for both bond2 and swp1
warning: bond2: misconfig ? removed but still exists as a dependency of
['bridge']
warning: swp1: misconfig ? removed but still exists as a dependency of
['bridge']
after the patch:
================
- ifreload will now correctly throw a warning only for bond2
warning: bond2: misconfig ? removed but still exists as a dependency of
['bridge']