wicked: allow interface configuration via Ethernet permanent address #2519
Conversation
| unsigned int value; | ||
| char name_buf[IF_NAMESIZE+1]; | ||
|
|
||
| + if (ni_string_empty(nnode->cdata)) { | ||
| + ni_debug_ifconfig("config is missing an interface name in <name> node"); | ||
| + goto error; | ||
| + } | ||
| + ifname = nnode->cdata; | ||
| + | ||
| if (ni_parse_uint(ifname, &value, 10) < 0) { |
There was a problem hiding this comment.
Since the interface "name" for the ifindex namespace is actually an integer, would it be more consistent with the other two branches to use the MAC address as the "name" for the ethernet namespace?
It looks like using the permanent-address field is suggested by the original docs, so what you have makes sense to me.
There was a problem hiding this comment.
I think I know what you mean, but am not 100% sure, so to clarify: Is what you're suggesting to store the MAC address in ifname when handling the ethernet namespace, similar to how the interface index is stored in ifname when handling the ifindex namespace?
If so, no, ifname needs to be the actual interface name in both cases. The diff is hiding the part of the code handling the ifindex namespace where the index temporarily stored in ifname is used to look up the actual interface name (edit: https://github.com/openSUSE/wicked/blob/a5762e9f372b9f8da3be4a7803f0a6d81c6dda6a/client/read-config.c#L356,L364). Unfortunately, the variable reuse doesn't help to make this clear either.
| + addrnode = xml_node_get_child(nnode, "permanent-address"); | ||
| + if (!addrnode || ni_string_empty(addrnode->cdata)) { | ||
| + ni_debug_ifconfig("cannot get interface name - " | ||
| + "config has no valid <ethernet> node"); |
There was a problem hiding this comment.
nit: isn't it a <name> node still, rather than an <ethernet> node?
| + if (ni_scandir("/sys/class/net", NULL, &all_ifnames) <= 0) | ||
| + goto error; |
There was a problem hiding this comment.
nit: add a debug message indicating the read failure?
|
I like @bcressey 's suggestions for changes - other than that this looks great! |
Add support for identifying Ethernet interfaces to configure via their
permanent/hardware/MAC address. For example, allow the following:
<interface>
<name namespace="ethernet">
<permanent-address>52:54:00:12:34:56</permanent-address>
</name>
<ipv4:dhcp>
<enabled>true</enabled>
</ipv4:dhcp>
</interface>
to configure the Ethernet interface with MAC address 52:54:00:12:34:56
via DHCP.
Signed-off-by: Markus Boehme <[email protected]>
markusboehme
force-pushed
the
feature/wicked-mac-based-config
branch
from
f2617af
to
a09b8b4
Compare
|
In the force push:
|
Issue number: #2293
Description of changes: Add support for identifying Ethernet interfaces to configure via their permanent/hardware/MAC address to wicked. For example, allow the following:
to configure the Ethernet interface with MAC address 52:54:00:12:34:56 via DHCP.
Note that:
Testing done: In a local VM, I successfully configured interfaces both via interface name and MAC address (at first by modifying
/etc/wicked/ifconfig/and restarting wicked components, later by just having netdog write the above XML snippet on boot).Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.