The bridge_get_all_ifaces() function is rather odd. It creates an shash
index over the "struct iface"s within a bridge, but there's already an
index over them (the 'iface_by_name' hmap in struct bridge) that the
iface_lookup() function searches. The only value it adds is to put the
names of bond fake ifaces into the index, but that's hardly worth it. We
can just search the existing hash table as needed, instead.
The bridge_fetch_dp_ifaces() function is also odd. It fetches the entire
mapping from port number to name from the dpif again, although this has
already been done twice already. We can just merge this in with the second
iteration.
This commit makes both of those changes.
This is more convenient for looking up a bridge by name. That makes
reconfiguration a little bit simpler, because there is no longer a need to
build a temporary index of existing bridges. I don't see any downsides.
The main reason that iterate_and_prune_ifaces() existed was because it was
somewhat inconvenient to iterate across all of the interfaces, especially
if anything needed to be deleted. Now that we've switched from arrays to
lists and hmaps, it's a bit easier, and certainly it's easier to read code
when there aren't any callbacks involved, so inline what this was doing.
This was the only remaining caller of iterate_and_prune_ifaces() so this
removes that function as well as the callback.
Users will the ability to manually set aggregation keys on a
per-slave basis in order to use some of the more advanced LACP
features. Most notably, LACP controlled active-backup bonding
requires fine grained aggregation key configuration.
In some extremely advanced situations, one may want to force
non-bondable slaves to advertise themselves as bondable. This
patch adds that capability.
Also includes some minor code cleanup.
These new semantics are less efficient in the case where the flood_vlans
actually changed, but that should be very rare.
There are no advantages to this change on its own, but upcoming commits
will add multiple layers between the code supplying the flood_vlans and
actually calling mac_learning_set_flood_vlans(). Consistency in this
multilayered interface seems valuable, and the rest of it does not transfer
ownership from the caller to the callee.
This improves the abstraction behind ofproto and connmgr.
Some of this could even go into fail_open, but I'm not sure that it would
make anything easier to understand.
A bond slave has a pointer to its iface's netdev, so we don't want it to
keep that pointer after the bridge closes the netdev.
This is becoming a bit of a mess so perhaps we need reference counting for
netdevs (although Jesse didn't like the idea when I proposed it before).
Until now, ovs-vswitchd has been unable to configure IP addresses and
routes for bridges whose Bridge records lack a Port and an Interface
record for the bridge's local port (e.g. OFPP_LOCAL, the port with the
same name as the bridge itself). When such a bridge was reconfigured,
ovs-vswitchd would output a log message that worried people.
This commit fixes the internal limitation that led to the message being
printed.
Bug #5385.
Mirrors that output to ports will be destroyed when their output ports are
destroyed, but mirrors that output to VLANs ("RSPAN" mirrors) don't get
automatically destroyed like this and we need to take care of them in a
separate loop.
Extremely advanced users may want fine grained control over the
LACP port and system IDs of a bond. This would be extremely
unusual for the average user, so this patch puts the configuration
parameters in other_config of the relevant tables.
When LACP negotiations are unsuccessful, OVS falls back to standard
balance-slb bonding. In some cases, users may want to require
successful LACP negotiations for any slaves to be enabled at all.
This patch implements a new "strict" mode which disables all slaves
when LACP negotiations are unsuccessful.
The logic here did not make sense. A packet arriving on a port is mirrored
if the port is a mirroring source port AND (not OR) the packet is in one of
the VLANs that is mirrored.
This test has been here since the mirroring code was introduced. It seems
to me that it was never correct.
Setting the 'mac' in the Interface record for a bridge's local port has
always been ineffective, but the log message was suppressed because of a
check at too high of a level. This commit fixes the problem. It also
fixes the wording of the log message, which has been obsolete since the
introduction of the database.
Finally, it seems better to check for the local port before checking for a
multicast address, so this reverses the order of the checks.
Whether or not a given slave is current with its LACP protocol
messages can be very interesting to a controller. If an interface
is not current, it usually indicates a connectivity problem or
misconfiguration of some sort.
In future patches, lacp status will need to be written to the
database in a rate limited manner. It doesn't make sense to run
two parallel rate limiters. This patch renames the CFM rate
limiter to something more generic.
This code was trying to check for priorities greater than UINT16_MAX and
reset them, but it assigned the value to a uint16_t before it checked it,
which of course hid the problem.
Fixes the following GCC warning:
vswitchd/bridge.c:3034: warning: comparison is always false due to limited
range of data type
Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
The bonding code only needs to know whether a given slave may be
enabled, and whether LACP has been negotiated on the bond. Instead
of passing in the LACP handle and letting the bond query this
information. This patch passes in the information directly.
For BM_STABLE bonds, instead of choosing the sort key in the
qsort() comparator, this patch makes it a configuration setting of
each slave. This will help wrest LACP out of the bonding code
further in future patches.
Before this patch, the bonding code had taken over responsibility
for running the LACP module. However, the bonding code only needs
the LACP module for some basic status queries. LACP and bonding
are actually logically parallel modules and do not really have a
parent child relationship. Furthermore, we need to be able to run
LACP on non-bonded interfaces which the existing approach
prevented. This patch gives control of the LACP module back to the
bridge.
Changes in the bonding mode can cause drastic changes in flow
assignments to slaves. This commit causes all flows in a bridge
to be revalidated when bond_reconfigure() changes its bonding mode.
This approach is a bit aggressive, but bond reconfiguration
shouldn't happen often.
By omitting columns that ovs-vswitchd does not use at all, and omitting
alerts for columns that ovs-vswitchd writes to but does not read, we can
save CPU time and bandwidth.
CFM configuration requires the ofproto_run function to have been
executed at least once in order to guarantee that the relevant
ports exist.
Bug #5303.
The following commit will need to iterate over a set of "struct
dst"s, obtaining the iface for each. It could look them up using
the hash table that indexes over dp_ifidx, but it's easier if we
simply store the iface pointer directly.
This removes over 1000 lines of code from bridge.c and will make it
easier to moving the bonding implementation into ofproto as part of
future development.
The code that enables and disables bond slaves was a bit of a mess:
* Disabling a slave could recursively enable a different slave.
* Processing a flow could enable a slave.
This commit gets rid of both of those properties, which made it difficult
to reason about the code paths along which slaves would be enabled and
disabled.
Bug #5121.
There's no reason that I can see to maintain this information in struct
port and struct iface. It's redundant, since the lacp implementation
maintains the same information.
