[systemd-devel] [PATCH] udev: ifname matches given mac
Tom Gundersen
teg at jklm.no
Wed Jan 15 05:22:50 PST 2014
Hi Umut,
I'm not really convinced about this. I think we should be fairly
conservative about what names we consider persistent.
The way NamePolicy=mac currently works is that it will base the name
on the hardware mac address (i.e., not randomly generated by the
kernel, set by userspace etc), and only do that if we deem it to be
sane.
With your patch you also allow using the mac address set by us (but
note that if it is set by other userspace it is still not used). In
the case of MACPolicy=random, this is certainly the wrong thing to do,
and even in the case of MACAddress=persistent or
MACAddress=XX:XX:XX:XX:XX:XX, I'm not entirely convinced as these
values are not really "permanent", in the sense that they are part of
the configuration and not of the hardware.
Any other opinions on this?
Cheers,
Tom
On Wed, Jan 15, 2014 at 1:39 PM, Umut Tezduyar Lindskog
<umut.tezduyar at axis.com> wrote:
> This covers the case where NamePolicy has mac in it
> and MACAddress is given.
>
> Ex:
>
> NamePolicy=mac
> MACAddress=00:00:00:00:00:00
>
> Interface name becomes [en|wl]x000000000000
>
> Also cleanup rtnl_set_link_properties
> ---
> src/libsystemd/rtnl-util.c | 16 ++-------
> src/udev/net/link-config.c | 56 +++++++++++++++++++------------
> src/udev/net/link-config.h | 2 +-
> src/udev/udev-builtin-net_setup_link.c | 8 +++-
> 4 files changed, 44 insertions(+), 38 deletions(-)
>
> diff --git a/src/libsystemd/rtnl-util.c b/src/libsystemd/rtnl-util.c
> index dfc0050..cebda2e 100644
> --- a/src/libsystemd/rtnl-util.c
> +++ b/src/libsystemd/rtnl-util.c
> @@ -52,7 +52,6 @@ int rtnl_set_link_name(sd_rtnl *rtnl, int ifindex, const char *name) {
> int rtnl_set_link_properties(sd_rtnl *rtnl, int ifindex, const char *alias,
> const struct ether_addr *mac, unsigned mtu) {
> _cleanup_sd_rtnl_message_unref_ sd_rtnl_message *message = NULL;
> - bool need_update = false;
> int r;
>
> assert(rtnl);
> @@ -69,32 +68,23 @@ int rtnl_set_link_properties(sd_rtnl *rtnl, int ifindex, const char *alias,
> r = sd_rtnl_message_append_string(message, IFLA_IFALIAS, alias);
> if (r < 0)
> return r;
> -
> - need_update = true;
> -
> }
>
> if (mac) {
> r = sd_rtnl_message_append_ether_addr(message, IFLA_ADDRESS, mac);
> if (r < 0)
> return r;
> -
> - need_update = true;
> }
>
> if (mtu > 0) {
> r = sd_rtnl_message_append_u32(message, IFLA_MTU, mtu);
> if (r < 0)
> return r;
> -
> - need_update = true;
> }
>
> - if (need_update) {
> - r = sd_rtnl_call(rtnl, message, 0, NULL);
> - if (r < 0)
> - return r;
> - }
> + r = sd_rtnl_call(rtnl, message, 0, NULL);
> + if (r < 0)
> + return r;
>
> return 0;
> }
> diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
> index bd97cd8..e0a58ff 100644
> --- a/src/udev/net/link-config.c
> +++ b/src/udev/net/link-config.c
> @@ -342,7 +342,7 @@ static int get_mac(struct udev_device *device, bool want_random, struct ether_ad
> return 0;
> }
>
> -int link_config_apply(link_config_ctx *ctx, link_config *config, struct udev_device *device, const char **name) {
> +int link_config_apply(link_config_ctx *ctx, link_config *config, struct udev_device *device, const char **name, bool *free_name) {
> const char *old_name;
> const char *new_name = NULL;
> struct ether_addr generated_mac;
> @@ -378,6 +378,27 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, struct udev_dev
> return -ENODEV;
> }
>
> + switch (config->mac_policy) {
> + case MACPOLICY_PERSISTENT:
> + if (!mac_is_permanent(device)) {
> + r = get_mac(device, false, &generated_mac);
> + if (r < 0)
> + return r;
> + mac = &generated_mac;
> + }
> + break;
> + case MACPOLICY_RANDOM:
> + if (!mac_is_random(device)) {
> + r = get_mac(device, true, &generated_mac);
> + if (r < 0)
> + return r;
> + mac = &generated_mac;
> + }
> + break;
> + default:
> + mac = config->mac;
> + }
> +
> if (ctx->enable_name_policy && config->name_policy) {
> NamePolicy *policy;
>
> @@ -394,6 +415,18 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, struct udev_dev
> break;
> case NAMEPOLICY_MAC:
> new_name = udev_device_get_property_value(device, "ID_NET_NAME_MAC");
> + if (mac && new_name) {
> + char *mac_name = NULL;
> + mac_name = strdup(new_name);
> + if (!mac_name)
> + break;
> + snprintf(mac_name+strlen(mac_name)-(2*ETHER_ADDR_LEN), 2*ETHER_ADDR_LEN+1,
> + "%02x%02x%02x%02x%02x%02x",
> + mac->ether_addr_octet[0], mac->ether_addr_octet[1], mac->ether_addr_octet[2],
> + mac->ether_addr_octet[3], mac->ether_addr_octet[4], mac->ether_addr_octet[5]);
> + new_name = mac_name;
> + *free_name = true;
> + }
> break;
> default:
> break;
> @@ -408,27 +441,6 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, struct udev_dev
> else
> *name = NULL;
>
> - switch (config->mac_policy) {
> - case MACPOLICY_PERSISTENT:
> - if (!mac_is_permanent(device)) {
> - r = get_mac(device, false, &generated_mac);
> - if (r < 0)
> - return r;
> - mac = &generated_mac;
> - }
> - break;
> - case MACPOLICY_RANDOM:
> - if (!mac_is_random(device)) {
> - r = get_mac(device, true, &generated_mac);
> - if (r < 0)
> - return r;
> - mac = &generated_mac;
> - }
> - break;
> - default:
> - mac = config->mac;
> - }
> -
> r = rtnl_set_link_properties(ctx->rtnl, ifindex, config->alias, mac, config->mtu);
> if (r < 0) {
> log_warning("Could not set Alias, MACAddress or MTU on %s: %s", old_name, strerror(-r));
> diff --git a/src/udev/net/link-config.h b/src/udev/net/link-config.h
> index a55c6f5..460501c 100644
> --- a/src/udev/net/link-config.h
> +++ b/src/udev/net/link-config.h
> @@ -75,7 +75,7 @@ int link_config_load(link_config_ctx *ctx);
> bool link_config_should_reload(link_config_ctx *ctx);
>
> int link_config_get(link_config_ctx *ctx, struct udev_device *device, struct link_config **ret);
> -int link_config_apply(link_config_ctx *ctx, struct link_config *config, struct udev_device *device, const char **name);
> +int link_config_apply(link_config_ctx *ctx, struct link_config *config, struct udev_device *device, const char **name, bool *free_name);
>
> const char *name_policy_to_string(NamePolicy p) _const_;
> NamePolicy name_policy_from_string(const char *p) _pure_;
> diff --git a/src/udev/udev-builtin-net_setup_link.c b/src/udev/udev-builtin-net_setup_link.c
> index b7ba8c9..7c8346b 100644
> --- a/src/udev/udev-builtin-net_setup_link.c
> +++ b/src/udev/udev-builtin-net_setup_link.c
> @@ -27,6 +27,7 @@ static link_config_ctx *ctx = NULL;
>
> static int builtin_net_setup_link(struct udev_device *dev, int argc, char **argv, bool test) {
> const char *name;
> + bool free_name = false;
> link_config *link;
> int r;
>
> @@ -46,14 +47,17 @@ static int builtin_net_setup_link(struct udev_device *dev, int argc, char **argv
> }
> }
>
> - r = link_config_apply(ctx, link, dev, &name);
> + r = link_config_apply(ctx, link, dev, &name, &free_name);
> if (r < 0) {
> log_error("Could not apply link config to %s", udev_device_get_sysname(dev));
> return EXIT_FAILURE;
> }
>
> - if (name)
> + if (name) {
> udev_builtin_add_property(dev, test, "ID_NET_NAME", name);
> + if (free_name)
> + free(name);
> + }
>
> return EXIT_SUCCESS;
> }
> --
> 1.7.2.5
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list