networkd: add missing bonding options #10542
Conversation
yuwata
added
the
reviewed/needs-rework 🔨
|
Targeted most of the comments, besides the one style remark. Rebased as well. Thanks for the review! Since it might get lost in the comment above were there any thoughts about the use of clang-format so far? |
|
@yuwata any more comments on this? |
toanju
force-pushed
the
bond-options
branch
3 times, most recently
from
949578f
to
faa0d99
Compare
yuwata
removed
the
reviewed/needs-rework 🔨
yuwata
added
the
good-to-merge/waiting-for-ci 👍
|
thanks for the reviews! |
evverx
removed
the
good-to-merge/waiting-for-ci 👍
yuwata
added
the
reviewed/needs-rework 🔨
Add support for bonding options system prio, port key and actor system mac. These options exist in the linux kernel since 4.2 (torvalds/linux@171a42c38c6e1) Details: https://www.kernel.org/doc/Documentation/networking/bonding.txt
yuwata
added
good-to-merge/waiting-for-ci 👍
|
CI failures seem to be unrelated. |
| @@ -1268,6 +1268,27 @@ | |||
| </listitem> | |||
| </varlistentry> | |||
|
|
|||
| <varlistentry> | |||
| <term><varname>AdActorSysPrio=</varname></term> | |||
There was a problem hiding this comment.
Please, let's not needlessly abbreviate. Unless some abbreviation is very well established let's write things out, it makes things more self-explanatory for readers, and we generally do so across the tree.
Hence, AdActorSystemPriority maybe?
There was a problem hiding this comment.
In principle the names are the same as they are in the sysfs (/sys/class/net/$IFNAME/bonding), besides the CamelCase adaptation. This one is originally ad_actor_sys_prio. The same names are accepted from iproute2 (see ip link help bond). None the less I would not mind changing them.
There was a problem hiding this comment.
I'd really prefer the mor verbose naming in this case
There was a problem hiding this comment.
Great! Updates are pushed.
| <varlistentry> | ||
| <term><varname>AdActorSysPrio=</varname></term> | ||
| <listitem> | ||
| <para>Specifies the 802.3ad system priority. Ranges [1-65535].</para> |
There was a problem hiding this comment.
how come the word "actor" isn't mentioned in the description, but in the setting name?
|
heya, sorry, i was a bit late with my review. added a couple of notes above. any chance you could address those in a quick follow-up PR, @toanju? |
|
yep, I'll do so |
|
Addressed the comments in #10653. Just give me a final conclusion on the AdActorSystemPriority vs AdActorSysPrio. Thanks! |
DUID/IAID — systemd#2818, systemd#2890, systemd#3156, Scope – systemd#6449, bond options — systemd#10542, option 119: sd_network_get_domains/sd_network_get_search_domains, /proc/cmdline parsing – 426c1d3, wait states — systemd#14536.
Add support for bonding options system prio, port key and actor system mac.
These options exist in the linux kernel since 4.2
(torvalds/linux@171a42c38c6e1)
Details:
https://www.kernel.org/doc/Documentation/networking/bonding.txt