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 -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel