On Mon, 20.04.15 15:27, Tom Gundersen (tome...@kemper.freedesktop.org) wrote:
+ <varlistentry>
+ <term><varname>LearnPacketIntvSec,=</varname></term>
+ <listitem>
+ <para>Specifies the number of seconds between instances where the
bonding
+ driver sends learning packets to each slaves peer switch.
+ The valid range is 1 - 0x7fffffff; the default value is 1. This
Option
+ has effect only in balance-tlb and balance-alb modes.</para>
+ </listitem>
+ </varlistentry>
We usually don't abbreviate options unnecessarily. Please use
"Interval" instead of "Intv".
+ <varlistentry>
+ <term><varname>FailOverMac=</varname></term>
+ <listitem>
+ <para>Specifies whether active-backup mode should set all slaves to
+ the same MAC address at enslavement or, when enabled, perform
special handling of the
+ bond's MAC address in accordance with the selected policy. The
default policy is none.
+ Possible values are
+ <literal>none</literal>,
+ <literal>active</literal>,
+ <literal>follow</literal>
+ </para>
+ </listitem>
+ </varlistentry>
The option MACAddress= is currently spelt with an uppercase MAC. In
fact most options containing acronyms use uppercase naming. hence
FailOverMAC=.
That said, shouldn't this be FailOverMACMode= or FailOverMACPolicy= or so?
+
+ <varlistentry>
+ <term><varname>ArpValidate=</varname></term>
+ <listitem>
+ <para>Specifies whether or not ARP probes and replies should be
+ validated in any mode that supports arp monitoring, or whether
+ non-ARP traffic should be filtered (disregarded) for link
+ monitoring purposes. Possible values are
+ <literal>none</literal>,
+ <literal>active</literal>,
+ <literal>backup</literal>,
+ <literal>all</literal>
+ </para>
+ </listitem>
+ </varlistentry>
Uppercase "ARP" please, see above. ARPValidate=
+ <varlistentry>
+ <term><varname>ArpIntervalSec=</varname></term>
+ <listitem>
+ <para>Specifies the ARP link monitoring frequency in milliseconds.
+ A value of 0 disables ARP monitoring. The default value is 0.
+ </para>
+ </listitem>
+ </varlistentry>
Good that Interval is spelt out this time. But s/Arp/ARP/ please.
+ <varlistentry>
+ <term><varname>ArpIpTargets=</varname></term>
+ <listitem>
+ <para>Specifies the IP addresses to use as ARP monitoring peers when
+ ArpIntervalSec is greater than 0. These are the targets of the ARP
request
+ sent to determine the health of the link to the targets.
+ Specify these values in ipv4 dotted decimal format. At least one IP
+ address must be given for ARP monitoring to function. The
+ maximum number of targets that can be specified is 16. The
+ default value is no IP addresses.
+ </para>
+ </listitem>
+ </varlistentry>
s/Arp/ARP/
s/Ip/IP/
Maybe append "Address"?
+
+ <varlistentry>
+ <term><varname>ArpAllTargets=</varname></term>
+ <listitem>
+ <para>Specifies the quantity of ArpIpTargets that must be reachable
+ in order for the ARP monitor to consider a slave as being up.
+ This option affects only active-backup mode for slaves with
+ ArpValidate enabled. Possible values are
+ <literal>any</literal>,
+ <literal>all</literal>
+ </para>
+ </listitem>
+ </varlistentry>
similar...
+
+ <varlistentry>
+ <term><varname>PrimaryReselect=</varname></term>
+ <listitem>
+ <para>Specifies the reselection policy for the primary slave. This
+ affects how the primary slave is chosen to become the active slave
+ when failure of the active slave or recovery of the primary slave
+ occurs. This option is designed to prevent flip-flopping between
+ the primary slave and other slaves. Possible values are
+ <literal>always</literal>,
+ <literal>better</literal>,
+ <literal>failure</literal>
+ </para>
+ </listitem>
+ </varlistentry>
PrimaryReselectPolicy= or so?
+
+ <varlistentry>
+ <term><varname>ResendIGMP=</varname></term>
+ <listitem>
+ <para>Specifies the number of IGMP membership reports to be issued
after
+ a failover event. One membership report is issued immediately after
+ the failover, subsequent packets are sent in each 200ms interval.
+ The valid range is (0 - 255). Defaults to 1. A value of 0
+ prevents the IGMP membership report from being issued in response
+ to the failover event.
+ </para>
+ </listitem>
+ </varlistentry>
I like the that the "IGMP" name is upper case!
+ <varlistentry>
+ <term><varname>NumGratuitousARP=</varname></term>
+ <listitem>
+ <para>Specify the number of peer notifications (gratuitous ARPs and
+ unsolicited IPv6 Neighbor Advertisements) to be issued after a
+ failover event. As soon as the link is up on the new slave
+ a peer notification is sent on the bonding device and each
+ VLAN sub-device. This is repeated at each link monitor interval
+ (ArpIntervalSec or MIIMonitorSec, whichever is active) if the
number is
+ greater than 1. The valid range is (0 - 255). Default value is 1.
+ These options affect only the active-backup mode
+ </para>
+ </listitem>
+ </varlistentry>
"Num"? Nah, please no unnecessary abbreviations.
The last sentence lacks a full stop.
+ if (b->arp_interval != 0) {
+ r = sd_rtnl_message_append_u32(m, IFLA_BOND_ARP_INTERVAL,
b->arp_interval / USEC_PER_MSEC);
+ if (r < 0) {
+ log_netdev_error(netdev,
+ "Could not append IFLA_BOND_ARP_INTERVAL
attribute: %s",
+ strerror(-r));
+ return r;
+ }
+ }
I'd really prefer if we wouldn't add new code using strerror().
Maybe it's time to introduce log_netdev_error_errno(), that is the
combination of what log_error_errno() and log_netdev_error() do? We
already have the anem for log_network_xyz()...
Lennart
