Ticket:
Reviewed By: Roopa, Nikhil G
Testing Done:
When using ifupdown2 on regular distribution (debian/ubuntu etc...) users may
not have the latest iproute2/bridge-utils version. Thus the output parsed from
the various subcommands executed by ifupdown2 might defer and cause exception
to be throwned.
We identified several cases:
/sbin/bridge -c -json vlan show
/sbin/brctl showmcqv4src bridge0
/sbin/brctl showstp bridge0
Julien Fortin [Tue, 4 Oct 2016 08:13:23 +0000 (10:13 +0200)]
addons: vrf: removing ifaceobj.name from exception message
Ticket: None
Reviewed By:
Testing Done:
The function _create_vrf_dev is surrounded by a try/catch as follow:
try:
vrf_table = self._create_vrf_dev(ifaceobj, vrf_table)
except Exception, e:
self.log_error('%s: %s' %(ifaceobj.name, str(e)), ifaceobj)
Thus we shouldn't include the ifaceobj.name in any error message passed to
log_error(). Since this function will raise an exception by default, it will
result in something like this:
error:br0.4002: red: red: create failed (cmd 'ip link add name red type vrf table 1001' failed: returned 1 ...[snip]
error:red: red: create failed (cmd 'ip link add name red type vrf table 1002' failed: returned 1 ...[snip]
after this patch:
error: red: create failed (cmd 'ip link add name red type vrf table 1002' failed: returned 1 ... [snip]
addons may provide a list of ifupdown scripts to ignore
Ticket:
Reviewed By: Roopa, Nikhil G
Testing Done:
ifupdown2's python addons are replacing some/most of the old ifupdown shell
scripts. When addon_scripts_support is set to 1, ifupdown2 will execute every
scripts present in /etc/network/if-$ACTION.d/ if a script exists with a name
identical to an ifupdown2 addon, this script won't be executed.
Sometimes an ifupdown2 addons doesn't have the same name as the ifupdown script
it's supposed to replace. Or maybe one addon is replacing several scripts.
For example: bond.py is replacing /etc/network/if-*.d/ifenslave
Now each addon is able to provide a list of script to ignore.
addons: mstpctl: warning user when using portadminedge attr on bridge vlan aware
Ticket: CM-12478
Reviewed By: Roopa, Nikhil G
Roopa: "For vlan aware bridge, we don't support the port attributes under the
bridge. It was deliberately done when vlan aware bridge config was introduced
because people wanted to get rid of the legacy under the bridge way of decla-
-ring port attributes. It was supported for a bit for evorack...in limited
form. we should probably just warn the user about this."
upperiface bringup is best effort, so ignore errors.
$ifquery -c -a
..[snip]...
auto br0
iface br0
bridge-vlan-aware yes [pass]
bridge-ports vx-14 vx-11 vx-10 vx-13 vx-12 [pass]
$ifdown vx-10
$ifdown vx-11
before patch:
$ifup vx-10
..[snip]...
info: running upperifaces (parent interfaces) if available ..
info: br0: running ops ...
error: br0: bridge port vx-11 does not exist
....
After patch:
$ifup vx-10
..[snip]...
info: running upperifaces (parent interfaces) if available ..
info: br0: running ops ...
....
ifupdownmain: dont catch link_up and link_down errors in ifupdownmain
Ticket: CM-12843
Reviewed By: julien, nikhil
Testing Done: Tested the failing test case in the bug
The scheduler already has some checking for errors from
link_up and link_down introduced by
commit 61c4d7244735 ("Suppress 'Network down' warnings when
link_master_slave feature is on").
The above fix was removed by a recent commit 707aeb737882 ("netlink ip link set up/down may silently fail, adding
try/except statements"). The scheduler does catch these errors.
so we will need to figure out why the scheduler is not catching it.
Right now this is failing an automation test. So the best fix
is to make sure the previous behaviour is restored.
addons: vrf: close sockets when vrf interface goes down
Ticket: CM-11393
Reviewed By: dsa, julien, nikhil
Testing Done: tested up and down of a vrf interface
$ifdown -v blue
..snip ..
info: executing /usr/lib/vrf/vrf-helper delete blue 1030
info: executing ip link del blue
info: executing /bin/ss -aK "dev == 54"
info: vrf: syncing table map to
/etc/iproute2/rt_tables.d/ifupdown2_vrf_map.conf
$ cat /etc/network/interfaces
auto tap0
iface tap0
#mtu 9000
mtu 1500
auto bridge
iface bridge
bridge-ports tap0
auto bridge.20
iface bridge.20
address 10.7.192.194/27
address-virtual 44:38:39:ff:00:20 10.7.192.193
$ ifreload -a
$ ip link show
bridge: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default
bridge.20@bridge: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default
bridge-20-v0@bridge.20: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default
$ #change mtu to 9000
$ ifreload -a
$ ip link show
bridge: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9000 qdisc noqueue state UP mode DEFAULT group default
bridge.20@bridge: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9000 qdisc noqueue state UP mode DEFAULT group default
bridge-20-v0@bridge.20: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9000 qdisc noqueue state UP mode DEFAULT group default
$
normalizing ip address(s) to IPNetwork format for string comparaison
Ticket: CM-12798
Reviewed By: Roopa, Nikhil G
Testing Done:
We are seeing some issue when using IP addresses with inner values padded with
zeros. Such as: 2a01:75e0:0000:09b0::1/64
The kernel will process the ip properly but when we query the kernel again (with
iproute2 or netlink) it returns 2a01:75e0:0:09b0::1/64
Since we are doing string comparaison we are seeing failures. We are now
converting all ip address to a standard format using IPNetwork or IPAddress obj
A customer discovered a corner case: the kernel is shrinking/reducing the ip
address fields containing 0s. For example if we configure and address such as
2a01:75e0:0000:09b0::1/64
Then if we query the kernel (using iproute2 or netlink) it will come back as:
2a01:75e0:0:09b0::1/64
Because of this issue we were seeing
root@r4u28:~# ifreload -a
error: bridge.200: cmd 'ip -force -batch - [addr add 10.50.103.193/26 dev bridge.200
addr add 2a01:75e0:0000:09b1::1/64 dev bridge.200
]' failed: returned 1 (RTNETLINK answers: File exists
Command failed -:1
)
Before adding an address to an iface we query it to know if the change is
necessary, since we only do a string comparaison between:
2a01:75e0:0000:09b1::1/64 and 2a01:75e0:0:09b1::1/64
it fails, ifupdown2 thinks that a change is needed. So we try to add the new
address via iproute2. iproute2 will fail because this address in a "shrinked"
format already exists.
This patch hot-fixes this problem by ignoring the error if it's an "exists"
error, we don't want to confuse the user.
A later fix will the real issue by normalizing all ip addr to the IPNetwork
format.
addons: vrf: be more friendly to older iproute2 versions
Ticket: CM-12049
Reviewed By: dsa, julien, nikhil
Testing Done: tested with and without /etc/iproute2/rt_tables.d/
OS does not have /etc/iproute2/rt_tables.d, on errors just info:
$ifreload -a -v
info: unable to save iproute2 vrf to table map ([Errno 2] No such file
or directory: '/etc/iproute2/rt_tables.d/ifupdown2_vrf_map.conf')
info: cannot find /etc/iproute2/rt_tables.d. pls check if your iproute2
version supports rt_tables.d
OS has /etc/iproute2/rt_tables.d, on errors warn:
$ifreload -a
warn: unable to save iproute2 vrf to table map ([Errno 2] No such file
Julien Fortin [Thu, 25 Aug 2016 21:57:21 +0000 (14:57 -0700)]
nlmanager: fixing error messages when receiving NLMSG_ERROR
Ticket: CM-12596
Reviewed By: Roopa, Daniel W
Testing Done:
before this patch:
$ cat /etc/network/interfaces
auto tapppppppppppp0.42
iface tapppppppppppp0.42
$
$ ifreload -a
warning: /etc/network/interfaces: line8: tapppppppppppp0.42: interface name too long
error: netlink: tapppppppppppp0: cannot create vlan 42: Operation failed with 'None' (RXed NLMSG_ERROR, pid 3346, seq 6, 108 bytes)
$
after:
$ ifreload -a
warning: /etc/network/interfaces: line8: tapppppppppppp0.42: interface name too long
error: netlink: tapppppppppppp0: cannot create vlan 42: Operation failed with 'Numerical result out of range'
$
Roopa Prabhu [Fri, 19 Aug 2016 18:09:57 +0000 (11:09 -0700)]
addons: bridge: fix running_vids value when cache is stale
Ticket: CM-12552
Reviewed By: julien, nikhil
Testing Done: tested with failing config with bridge-access 1
This is similar to the fix done in the below commit for pvid:
"5061730ea5bf ("addons: bridge: fix default pvid handling in cases where
cache is stale")"
easier steps to reproduce:
- have a vlan aware bridge with more than one ports
- add 'bridge-access 1' to one of the ports
- boot the box with the config
- check that the vlans are fine
- ifdown <interface_with_bridge_access_1>
- ifreload -a
- the interface with bridge_access 1 does
not have the pvid flag on vlan 1
This patch makes sure we assume the right running
vid and pvid value ie [1] and 1 if the
cache returns no values. vid = [1] and pvid = 1
are the kernel default/initial values for a port.
Roopa Prabhu [Sat, 13 Aug 2016 06:49:29 +0000 (23:49 -0700)]
addons: vrf: don't delete vrf map file at init if mgmt vrf already exists
Ticket: CM-12084
Reviewed By: julien, nikhil
Testing Done: tested with --allow=mgmt class
This fixes a problem where vrf map file was getting deleted
at boot during the second invocation of ifup. This is because
the code uses PERFMODE to check it is boot..but does
not realize that this may not be the first invocation of
ifup during the boot.
This patch adds additional checks for mgmt netdev to
make sure we delete the vrf map file only the first time.
My previous fix for the same bug did not fix the problem completly.
In this patch all running pvid calculation
is done before the pvid_to_del is calculated.
This makes sure the old pvid is deleted if it is not
the same as current pvid in all cases
Julien Fortin [Thu, 11 Aug 2016 01:05:04 +0000 (03:05 +0200)]
nlpacket: add new attribute: AttributeStringInterfaceName with length check
Ticket: CM-12302
Reviewed By: Daniel, Roopa, Nikhil G
Testing Done: ifupdown2 smoke test
With this pretty straight forward, we introduce a new Attribute in nlmanager,
so that we have single check and no code redundancy for ifname length. This
also prevent loosing time sending a netlink packet for an known error.
This can happen if the bridge vlans were
cached before the interface become a bridge port.
and when the interface becomes a bridge port
kernel adds a default pvid of 1. so this patch
assumes a default pvid is 1 if we did not find
anything in the cache. This will delete
the default pvid of 1 if the user has configured a
different pvid.
The message logged and the exception raised by nlmanager, when receiving
an NLMSG_ERROR packet from the kernel, wasn't super explicit plus if the
error wasn't a netlink related failure we didn't provide any information
Now we use strerror with the provided error code to generate a nice msg.
ifupdownaddons: iproute2: Cache vxlan remote ip addresses only if
statically configured
Ticket: CM-12277
Reviewed By: CCR-5059
Testing Done: ifreload with a VxlanClagClosConfig.py config
get_vxlan_peers gets remote ip addresses if they are statically configured
through ifupdown2 (vxlan-remoteip <address>). These are cached in
ifupdown2's link object so that they can be used in 'ip link set' (if link
exists already), 'ifquery -r' and 'ifquery -c'. ifupdown2 should ignore and
not cache these addresses if they are provisioned by vxrd. Check if vxrd is
running before caching remote ip addresses. This helps in optimizing time
taken to ifreload with 1k bridge/vxlan interfaces (vlan-aware bridge) in a
vxrd/vxsnd setup.
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 [Fri, 19 Aug 2016 18:09:57 +0000 (11:09 -0700)]
addons: bridge: fix running_vids value when cache is stale
Ticket: CM-12552
Reviewed By: julien, nikhil
Testing Done: tested with failing config with bridge-access 1
This is similar to the fix done in the below commit for pvid:
"5061730ea5bf ("addons: bridge: fix default pvid handling in cases where
cache is stale")"
easier steps to reproduce:
- have a vlan aware bridge with more than one ports
- add 'bridge-access 1' to one of the ports
- boot the box with the config
- check that the vlans are fine
- ifdown <interface_with_bridge_access_1>
- ifreload -a
- the interface with bridge_access 1 does
not have the pvid flag on vlan 1
This patch makes sure we assume the right running
vid and pvid value ie [1] and 1 if the
cache returns no values. vid = [1] and pvid = 1
are the kernel default/initial values for a port.
Roopa Prabhu [Sat, 13 Aug 2016 06:49:29 +0000 (23:49 -0700)]
addons: vrf: don't delete vrf map file at init if mgmt vrf already exists
Ticket: CM-12084
Reviewed By: julien, nikhil
Testing Done: tested with --allow=mgmt class
This fixes a problem where vrf map file was getting deleted
at boot during the second invocation of ifup. This is because
the code uses PERFMODE to check it is boot..but does
not realize that this may not be the first invocation of
ifup during the boot.
This patch adds additional checks for mgmt netdev to
make sure we delete the vrf map file only the first time.
My previous fix for the same bug did not fix the problem completly.
In this patch all running pvid calculation
is done before the pvid_to_del is calculated.
This makes sure the old pvid is deleted if it is not
the same as current pvid in all cases
Julien Fortin [Thu, 11 Aug 2016 01:05:04 +0000 (03:05 +0200)]
nlpacket: add new attribute: AttributeStringInterfaceName with length check
Ticket: CM-12302
Reviewed By: Daniel, Roopa, Nikhil G
Testing Done: ifupdown2 smoke test
With this pretty straight forward, we introduce a new Attribute in nlmanager,
so that we have single check and no code redundancy for ifname length. This
also prevent loosing time sending a netlink packet for an known error.
This can happen if the bridge vlans were
cached before the interface become a bridge port.
and when the interface becomes a bridge port
kernel adds a default pvid of 1. so this patch
assumes a default pvid is 1 if we did not find
anything in the cache. This will delete
the default pvid of 1 if the user has configured a
different pvid.
The message logged and the exception raised by nlmanager, when receiving
an NLMSG_ERROR packet from the kernel, wasn't super explicit plus if the
error wasn't a netlink related failure we didn't provide any information
Now we use strerror with the provided error code to generate a nice msg.
ifupdownaddons: iproute2: Cache vxlan remote ip addresses only if
statically configured
Ticket: CM-12277
Reviewed By: CCR-5059
Testing Done: ifreload with a VxlanClagClosConfig.py config
get_vxlan_peers gets remote ip addresses if they are statically configured
through ifupdown2 (vxlan-remoteip <address>). These are cached in
ifupdown2's link object so that they can be used in 'ip link set' (if link
exists already), 'ifquery -r' and 'ifquery -c'. ifupdown2 should ignore and
not cache these addresses if they are provisioned by vxrd. Check if vxrd is
running before caching remote ip addresses. This helps in optimizing time
taken to ifreload with 1k bridge/vxlan interfaces (vlan-aware bridge) in a
vxrd/vxsnd setup.
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.
The mstpctl addon and mstpctlutils file now contains a unified way to access
cached data (from mstpctl showportdetails json).
Some numbers:
- Without the patch ifreload -a on a scale vxlan config (1k vxlan) we used
to execute 5098 subcommands.
- With the patch "only" ~3000 (it should be 2k but clag ip link set protodown
needs to be move to netlink).
for ifquery -r -a: 11052 calls before patch, 1031 after.
This should improve time execution as well as CPU usage.
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 ...
projects / mirror_ifupdown2.git / log