diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 9cb4dfc242f5..82c232f9ede2 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -39,6 +39,7 @@ #include #include #include +#include MODULE_AUTHOR("Matt Mackall "); MODULE_DESCRIPTION("Console driver for network interfaces"); @@ -85,6 +86,8 @@ static DEFINE_SPINLOCK(target_list_lock); /* This needs to be a mutex because netpoll_cleanup might sleep */ static DEFINE_MUTEX(target_cleanup_list_lock); +static struct workqueue_struct *netconsole_wq; + /* * Console driver for netconsoles. Register only consoles that have * an associated target of the same type. @@ -119,6 +122,12 @@ enum sysdata_feature { MAX_SYSDATA_ITEMS = 4, }; +enum target_state { + STATE_DISABLED, + STATE_ENABLED, + STATE_DEACTIVATED, +}; + /** * struct netconsole_target - Represents a configured netconsole target. * @list: Links this target into the target_list. @@ -130,12 +139,16 @@ enum sysdata_feature { * @sysdata_fields: Sysdata features enabled. * @msgcounter: Message sent counter. * @stats: Packet send stats for the target. Used for debugging. - * @enabled: On / off knob to enable / disable target. + * @state: State of the target. * Visible from userspace (read-write). - * We maintain a strict 1:1 correspondence between this and - * whether the corresponding netpoll is active or inactive. + * From a userspace perspective, the target is either enabled or + * disabled. Internally, although both STATE_DISABLED and + * STATE_DEACTIVATED correspond to inactive targets, the latter is + * due to automatic interface state changes and will try + * recover automatically, if the interface comes back + * online. * Also, other parameters of a target may be modified at - * runtime only when it is disabled (enabled == 0). + * runtime only when it is disabled (state != STATE_ENABLED). * @extended: Denotes whether console is extended or not. * @release: Denotes whether kernel release version should be prepended * to the message. Depends on extended console. @@ -149,6 +162,7 @@ enum sysdata_feature { * local_mac (read-only) * remote_mac (read-write) * @buf: The buffer used to send the full msg to the network stack + * @resume_wq: Workqueue to resume deactivated target */ struct netconsole_target { struct list_head list; @@ -165,12 +179,13 @@ struct netconsole_target { u32 msgcounter; #endif struct netconsole_target_stats stats; - bool enabled; + enum target_state state; bool extended; bool release; struct netpoll np; /* protected by target_list_lock */ char buf[MAX_PRINT_CHUNK]; + struct work_struct resume_wq; }; #ifdef CONFIG_NETCONSOLE_DYNAMIC @@ -207,6 +222,16 @@ static void netconsole_target_put(struct netconsole_target *nt) config_group_put(&nt->group); } +static void dynamic_netconsole_mutex_lock(void) +{ + mutex_lock(&dynamic_netconsole_mutex); +} + +static void dynamic_netconsole_mutex_unlock(void) +{ + mutex_unlock(&dynamic_netconsole_mutex); +} + #else /* !CONFIG_NETCONSOLE_DYNAMIC */ static int __init dynamic_netconsole_init(void) @@ -234,8 +259,87 @@ static void populate_configfs_item(struct netconsole_target *nt, int cmdline_count) { } + +static void dynamic_netconsole_mutex_lock(void) +{ +} + +static void dynamic_netconsole_mutex_unlock(void) +{ +} + #endif /* CONFIG_NETCONSOLE_DYNAMIC */ +/* Check if the target was bound by mac address. */ +static bool bound_by_mac(struct netconsole_target *nt) +{ + return is_valid_ether_addr(nt->np.dev_mac); +} + +/* Attempts to resume logging to a deactivated target. */ +static void resume_target(struct netconsole_target *nt) +{ + if (netpoll_setup(&nt->np)) { + /* netpoll fails setup once, do not try again. */ + nt->state = STATE_DISABLED; + return; + } + + nt->state = STATE_ENABLED; + pr_info("network logging resumed on interface %s\n", nt->np.dev_name); +} + +/* Checks if a deactivated target matches a device. */ +static bool deactivated_target_match(struct netconsole_target *nt, + struct net_device *ndev) +{ + if (nt->state != STATE_DEACTIVATED) + return false; + + if (bound_by_mac(nt)) + return !memcmp(nt->np.dev_mac, ndev->dev_addr, ETH_ALEN); + return !strncmp(nt->np.dev_name, ndev->name, IFNAMSIZ); +} + +/* Process work scheduled for target resume. */ +static void process_resume_target(struct work_struct *work) +{ + struct netconsole_target *nt; + unsigned long flags; + + nt = container_of(work, struct netconsole_target, resume_wq); + + dynamic_netconsole_mutex_lock(); + + spin_lock_irqsave(&target_list_lock, flags); + /* Check if target is still deactivated as it may have been disabled + * while resume was being scheduled. + */ + if (nt->state != STATE_DEACTIVATED) { + spin_unlock_irqrestore(&target_list_lock, flags); + goto out_unlock; + } + + /* resume_target is IRQ unsafe, remove target from + * target_list in order to resume it with IRQ enabled. + */ + list_del_init(&nt->list); + spin_unlock_irqrestore(&target_list_lock, flags); + + resume_target(nt); + + /* At this point the target is either enabled or disabled and + * was cleaned up before getting deactivated. Either way, add it + * back to target list. + */ + spin_lock_irqsave(&target_list_lock, flags); + list_add(&nt->list, &target_list); + spin_unlock_irqrestore(&target_list_lock, flags); + +out_unlock: + dynamic_netconsole_mutex_unlock(); +} + /* Allocate and initialize with defaults. * Note that these targets get their config_item fields zeroed-out. */ @@ -257,6 +361,8 @@ static struct netconsole_target *alloc_and_init(void) nt->np.local_port = 6665; nt->np.remote_port = 6666; eth_broadcast_addr(nt->np.remote_mac); + nt->state = STATE_DISABLED; + INIT_WORK(&nt->resume_wq, process_resume_target); return nt; } @@ -275,8 +381,10 @@ static void netconsole_process_cleanups_core(void) mutex_lock(&target_cleanup_list_lock); list_for_each_entry_safe(nt, tmp, &target_cleanup_list, list) { /* all entries in the cleanup_list needs to be disabled */ - WARN_ON_ONCE(nt->enabled); + WARN_ON_ONCE(nt->state == STATE_ENABLED); do_netpoll_cleanup(&nt->np); + if (bound_by_mac(nt)) + memset(&nt->np.dev_name, 0, IFNAMSIZ); /* moved the cleaned target to target_list. Need to hold both * locks */ @@ -398,7 +506,7 @@ static void trim_newline(char *s, size_t maxlen) static ssize_t enabled_show(struct config_item *item, char *buf) { - return sysfs_emit(buf, "%d\n", to_target(item)->enabled); + return sysfs_emit(buf, "%d\n", to_target(item)->state == STATE_ENABLED); } static ssize_t extended_show(struct config_item *item, char *buf) @@ -480,9 +588,9 @@ static ssize_t sysdata_cpu_nr_enabled_show(struct config_item *item, char *buf) struct netconsole_target *nt = to_target(item->ci_parent); bool cpu_nr_enabled; - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); cpu_nr_enabled = !!(nt->sysdata_fields & SYSDATA_CPU_NR); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return sysfs_emit(buf, "%d\n", cpu_nr_enabled); } @@ -494,9 +602,9 @@ static ssize_t sysdata_taskname_enabled_show(struct config_item *item, struct netconsole_target *nt = to_target(item->ci_parent); bool taskname_enabled; - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); taskname_enabled = !!(nt->sysdata_fields & SYSDATA_TASKNAME); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return sysfs_emit(buf, "%d\n", taskname_enabled); } @@ -507,9 +615,9 @@ static ssize_t sysdata_release_enabled_show(struct config_item *item, struct netconsole_target *nt = to_target(item->ci_parent); bool release_enabled; - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); release_enabled = !!(nt->sysdata_fields & SYSDATA_TASKNAME); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return sysfs_emit(buf, "%d\n", release_enabled); } @@ -547,9 +655,9 @@ static ssize_t sysdata_msgid_enabled_show(struct config_item *item, struct netconsole_target *nt = to_target(item->ci_parent); bool msgid_enabled; - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); msgid_enabled = !!(nt->sysdata_fields & SYSDATA_MSGID); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return sysfs_emit(buf, "%d\n", msgid_enabled); } @@ -565,19 +673,28 @@ static ssize_t enabled_store(struct config_item *item, const char *buf, size_t count) { struct netconsole_target *nt = to_target(item); + bool enabled, current_enabled; unsigned long flags; - bool enabled; ssize_t ret; - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); ret = kstrtobool(buf, &enabled); if (ret) goto out_unlock; + /* When the user explicitly enables or disables a target that is + * currently deactivated, reset its state to disabled. The DEACTIVATED + * state only tracks interface-driven deactivation and should _not_ + * persist when the user manually changes the target's enabled state. + */ + if (nt->state == STATE_DEACTIVATED) + nt->state = STATE_DISABLED; + ret = -EINVAL; - if (enabled == nt->enabled) { + current_enabled = nt->state == STATE_ENABLED; + if (enabled == current_enabled) { pr_info("network logging has already %s\n", - nt->enabled ? "started" : "stopped"); + current_enabled ? "started" : "stopped"); goto out_unlock; } @@ -610,16 +727,16 @@ static ssize_t enabled_store(struct config_item *item, if (ret) goto out_unlock; - nt->enabled = true; + nt->state = STATE_ENABLED; pr_info("network logging started\n"); } else { /* false */ /* We need to disable the netconsole before cleaning it up * otherwise we might end up in write_msg() with - * nt->np.dev == NULL and nt->enabled == true + * nt->np.dev == NULL and nt->state == STATE_ENABLED */ mutex_lock(&target_cleanup_list_lock); spin_lock_irqsave(&target_list_lock, flags); - nt->enabled = false; + nt->state = STATE_DISABLED; /* Remove the target from the list, while holding * target_list_lock */ @@ -636,7 +753,7 @@ static ssize_t enabled_store(struct config_item *item, /* Deferred cleanup */ netconsole_process_cleanups(); out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return ret; } @@ -647,8 +764,8 @@ static ssize_t release_store(struct config_item *item, const char *buf, bool release; ssize_t ret; - mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + dynamic_netconsole_mutex_lock(); + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); ret = -EINVAL; @@ -663,7 +780,7 @@ static ssize_t release_store(struct config_item *item, const char *buf, ret = strnlen(buf, count); out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return ret; } @@ -674,8 +791,8 @@ static ssize_t extended_store(struct config_item *item, const char *buf, bool extended; ssize_t ret; - mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + dynamic_netconsole_mutex_lock(); + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); ret = -EINVAL; @@ -689,7 +806,7 @@ static ssize_t extended_store(struct config_item *item, const char *buf, nt->extended = extended; ret = strnlen(buf, count); out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return ret; } @@ -698,18 +815,18 @@ static ssize_t dev_name_store(struct config_item *item, const char *buf, { struct netconsole_target *nt = to_target(item); - mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + dynamic_netconsole_mutex_lock(); + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return -EINVAL; } strscpy(nt->np.dev_name, buf, IFNAMSIZ); trim_newline(nt->np.dev_name, IFNAMSIZ); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return strnlen(buf, count); } @@ -719,8 +836,8 @@ static ssize_t local_port_store(struct config_item *item, const char *buf, struct netconsole_target *nt = to_target(item); ssize_t ret = -EINVAL; - mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + dynamic_netconsole_mutex_lock(); + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -731,7 +848,7 @@ static ssize_t local_port_store(struct config_item *item, const char *buf, goto out_unlock; ret = strnlen(buf, count); out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return ret; } @@ -741,8 +858,8 @@ static ssize_t remote_port_store(struct config_item *item, struct netconsole_target *nt = to_target(item); ssize_t ret = -EINVAL; - mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + dynamic_netconsole_mutex_lock(); + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -753,7 +870,7 @@ static ssize_t remote_port_store(struct config_item *item, goto out_unlock; ret = strnlen(buf, count); out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return ret; } @@ -764,8 +881,8 @@ static ssize_t local_ip_store(struct config_item *item, const char *buf, ssize_t ret = -EINVAL; int ipv6; - mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + dynamic_netconsole_mutex_lock(); + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -778,7 +895,7 @@ static ssize_t local_ip_store(struct config_item *item, const char *buf, ret = strnlen(buf, count); out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return ret; } @@ -789,8 +906,8 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf, ssize_t ret = -EINVAL; int ipv6; - mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + dynamic_netconsole_mutex_lock(); + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -803,7 +920,7 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf, ret = strnlen(buf, count); out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return ret; } @@ -824,8 +941,8 @@ static ssize_t remote_mac_store(struct config_item *item, const char *buf, u8 remote_mac[ETH_ALEN]; ssize_t ret = -EINVAL; - mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + dynamic_netconsole_mutex_lock(); + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -839,7 +956,7 @@ static ssize_t remote_mac_store(struct config_item *item, const char *buf, ret = strnlen(buf, count); out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); return ret; } @@ -960,7 +1077,7 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf, return -EMSGSIZE; mutex_lock(&netconsole_subsys.su_mutex); - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); ret = strscpy(udm->value, buf, sizeof(udm->value)); if (ret < 0) @@ -974,7 +1091,7 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf, goto out_unlock; ret = count; out_unlock: - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; } @@ -1002,7 +1119,7 @@ static ssize_t sysdata_msgid_enabled_store(struct config_item *item, return ret; mutex_lock(&netconsole_subsys.su_mutex); - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); curr = !!(nt->sysdata_fields & SYSDATA_MSGID); if (msgid_enabled == curr) goto unlock_ok; @@ -1014,7 +1131,7 @@ static ssize_t sysdata_msgid_enabled_store(struct config_item *item, unlock_ok: ret = strnlen(buf, count); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; } @@ -1031,7 +1148,7 @@ static ssize_t sysdata_release_enabled_store(struct config_item *item, return ret; mutex_lock(&netconsole_subsys.su_mutex); - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); curr = !!(nt->sysdata_fields & SYSDATA_RELEASE); if (release_enabled == curr) goto unlock_ok; @@ -1043,7 +1160,7 @@ static ssize_t sysdata_release_enabled_store(struct config_item *item, unlock_ok: ret = strnlen(buf, count); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; } @@ -1060,7 +1177,7 @@ static ssize_t sysdata_taskname_enabled_store(struct config_item *item, return ret; mutex_lock(&netconsole_subsys.su_mutex); - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); curr = !!(nt->sysdata_fields & SYSDATA_TASKNAME); if (taskname_enabled == curr) goto unlock_ok; @@ -1072,7 +1189,7 @@ static ssize_t sysdata_taskname_enabled_store(struct config_item *item, unlock_ok: ret = strnlen(buf, count); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; } @@ -1090,7 +1207,7 @@ static ssize_t sysdata_cpu_nr_enabled_store(struct config_item *item, return ret; mutex_lock(&netconsole_subsys.su_mutex); - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); curr = !!(nt->sysdata_fields & SYSDATA_CPU_NR); if (cpu_nr_enabled == curr) /* no change requested */ @@ -1106,7 +1223,7 @@ static ssize_t sysdata_cpu_nr_enabled_store(struct config_item *item, unlock_ok: ret = strnlen(buf, count); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; } @@ -1168,10 +1285,10 @@ static void userdatum_drop(struct config_group *group, struct config_item *item) ud = to_userdata(&group->cg_item); nt = userdata_to_target(ud); - mutex_lock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_lock(); update_userdata(nt); config_item_put(item); - mutex_unlock(&dynamic_netconsole_mutex); + dynamic_netconsole_mutex_unlock(); } static struct configfs_attribute *userdata_attrs[] = { @@ -1310,18 +1427,34 @@ static struct config_group *make_netconsole_target(struct config_group *group, static void drop_netconsole_target(struct config_group *group, struct config_item *item) { - unsigned long flags; struct netconsole_target *nt = to_target(item); + unsigned long flags; + + dynamic_netconsole_mutex_lock(); spin_lock_irqsave(&target_list_lock, flags); + /* Disable deactivated target to prevent races between resume attempt + * and target removal. + */ + if (nt->state == STATE_DEACTIVATED) + nt->state = STATE_DISABLED; list_del(&nt->list); spin_unlock_irqrestore(&target_list_lock, flags); + dynamic_netconsole_mutex_unlock(); + + /* Now that the target has been marked disabled no further work + * can be scheduled. Existing work will skip as targets are not + * deactivated anymore. Cancel any scheduled resume and wait for + * completion. + */ + cancel_work_sync(&nt->resume_wq); + /* * The target may have never been enabled, or was manually disabled * before being removed so netpoll may have already been cleaned up. */ - if (nt->enabled) + if (nt->state == STATE_ENABLED) netpoll_cleanup(&nt->np); config_item_put(&nt->group.cg_item); @@ -1418,13 +1551,14 @@ static int prepare_sysdata(struct netconsole_target *nt) static int netconsole_netdev_event(struct notifier_block *this, unsigned long event, void *ptr) { - unsigned long flags; - struct netconsole_target *nt, *tmp; struct net_device *dev = netdev_notifier_info_to_dev(ptr); + struct netconsole_target *nt, *tmp; bool stopped = false; + unsigned long flags; if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER || - event == NETDEV_RELEASE || event == NETDEV_JOIN)) + event == NETDEV_RELEASE || event == NETDEV_JOIN || + event == NETDEV_REGISTER)) goto done; mutex_lock(&target_cleanup_list_lock); @@ -1438,12 +1572,28 @@ static int netconsole_netdev_event(struct notifier_block *this, break; case NETDEV_RELEASE: case NETDEV_JOIN: + /* transition target to DISABLED instead of + * DEACTIVATED when (de)enslaving devices as + * their targets should not be automatically + * resumed when the interface is brought up. + */ + nt->state = STATE_DISABLED; + list_move(&nt->list, &target_cleanup_list); + stopped = true; + break; case NETDEV_UNREGISTER: - nt->enabled = false; + nt->state = STATE_DEACTIVATED; list_move(&nt->list, &target_cleanup_list); stopped = true; } } + if ((event == NETDEV_REGISTER || event == NETDEV_CHANGENAME) && + deactivated_target_match(nt, dev)) + /* Schedule resume on a workqueue as it will attempt + * to UP the device, which can't be done as part of this + * notifier. + */ + queue_work(netconsole_wq, &nt->resume_wq); netconsole_target_put(nt); } spin_unlock_irqrestore(&target_list_lock, flags); @@ -1720,7 +1870,8 @@ static void write_ext_msg(struct console *con, const char *msg, spin_lock_irqsave(&target_list_lock, flags); list_for_each_entry(nt, &target_list, list) - if (nt->extended && nt->enabled && netif_running(nt->np.dev)) + if (nt->extended && nt->state == STATE_ENABLED && + netif_running(nt->np.dev)) send_ext_msg_udp(nt, msg, len); spin_unlock_irqrestore(&target_list_lock, flags); } @@ -1740,7 +1891,8 @@ static void write_msg(struct console *con, const char *msg, unsigned int len) spin_lock_irqsave(&target_list_lock, flags); list_for_each_entry(nt, &target_list, list) { - if (!nt->extended && nt->enabled && netif_running(nt->np.dev)) { + if (!nt->extended && nt->state == STATE_ENABLED && + netif_running(nt->np.dev)) { /* * We nest this inside the for-each-target loop above * so that we're able to get as much logging out to @@ -1896,7 +2048,7 @@ static struct netconsole_target *alloc_param_target(char *target_config, */ goto fail; } else { - nt->enabled = true; + nt->state = STATE_ENABLED; } populate_configfs_item(nt, cmdline_count); @@ -1910,6 +2062,7 @@ static struct netconsole_target *alloc_param_target(char *target_config, /* Cleanup netpoll for given target (from boot/module param) and free it */ static void free_param_target(struct netconsole_target *nt) { + cancel_work_sync(&nt->resume_wq); netpoll_cleanup(&nt->np); #ifdef CONFIG_NETCONSOLE_DYNAMIC kfree(nt->userdata); @@ -1964,6 +2117,12 @@ static int __init init_netconsole(void) } } + netconsole_wq = alloc_workqueue("netconsole", WQ_UNBOUND, 0); + if (!netconsole_wq) { + err = -ENOMEM; + goto fail; + } + err = register_netdevice_notifier(&netconsole_netdev_notifier); if (err) goto fail; @@ -1986,6 +2145,8 @@ static int __init init_netconsole(void) fail: pr_err("cleaning up\n"); + if (netconsole_wq) + flush_workqueue(netconsole_wq); /* * Remove all targets and destroy them (only targets created * from the boot/module option exist here). Skipping the list @@ -1996,6 +2157,9 @@ static int __init init_netconsole(void) free_param_target(nt); } + if (netconsole_wq) + destroy_workqueue(netconsole_wq); + return err; } @@ -2009,6 +2173,7 @@ static void __exit cleanup_netconsole(void) unregister_console(&netconsole); dynamic_netconsole_exit(); unregister_netdevice_notifier(&netconsole_netdev_notifier); + flush_workqueue(netconsole_wq); /* * Targets created via configfs pin references on our module @@ -2022,6 +2187,8 @@ static void __exit cleanup_netconsole(void) list_del(&nt->list); free_param_target(nt); } + + destroy_workqueue(netconsole_wq); } /* diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile index f5c71d993750..3eba569b3366 100644 --- a/tools/testing/selftests/drivers/net/Makefile +++ b/tools/testing/selftests/drivers/net/Makefile @@ -19,6 +19,7 @@ TEST_PROGS := \ netcons_cmdline.sh \ netcons_fragmented_msg.sh \ netcons_overflow.sh \ + netcons_resume.sh \ netcons_sysdata.sh \ netcons_torture.sh \ netpoll_basic.py \ diff --git a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh index ae8abff4be40..b6093bcf2b06 100644 --- a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh +++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh @@ -203,19 +203,21 @@ function do_cleanup() { function cleanup_netcons() { # delete netconsole dynamic reconfiguration # do not fail if the target is already disabled - if [[ ! -d "${NETCONS_PATH}" ]] + local TARGET_PATH=${1:-${NETCONS_PATH}} + + if [[ ! -d "${TARGET_PATH}" ]] then # in some cases this is called before netcons path is created return fi - if [[ $(cat "${NETCONS_PATH}"/enabled) != 0 ]] + if [[ $(cat "${TARGET_PATH}"/enabled) != 0 ]] then - echo 0 > "${NETCONS_PATH}"/enabled || true + echo 0 > "${TARGET_PATH}"/enabled || true fi # Remove all the keys that got created during the selftest - find "${NETCONS_PATH}/userdata/" -mindepth 1 -type d -delete + find "${TARGET_PATH}/userdata/" -mindepth 1 -type d -delete # Remove the configfs entry - rmdir "${NETCONS_PATH}" + rmdir "${TARGET_PATH}" } function cleanup() { @@ -377,6 +379,29 @@ function check_netconsole_module() { fi } +function wait_target_state() { + local TARGET=${1} + local STATE=${2} + local TARGET_PATH="${NETCONS_CONFIGFS}"/"${TARGET}" + local ENABLED=0 + + if [ "${STATE}" == "enabled" ] + then + ENABLED=1 + fi + + if [ ! -d "$TARGET_PATH" ]; then + echo "FAIL: Target does not exist." >&2 + exit "${ksft_fail}" + fi + + local CHECK_CMD="grep \"$ENABLED\" \"$TARGET_PATH/enabled\"" + slowwait 2 sh -c "test -n \"\$($CHECK_CMD)\"" || { + echo "FAIL: ${TARGET} is not ${STATE}." >&2 + exit "${ksft_fail}" + } +} + # A wrapper to translate protocol version to udp version function wait_for_port() { local NAMESPACE=${1} diff --git a/tools/testing/selftests/drivers/net/netcons_resume.sh b/tools/testing/selftests/drivers/net/netcons_resume.sh new file mode 100755 index 000000000000..fc5e5e3ad3d4 --- /dev/null +++ b/tools/testing/selftests/drivers/net/netcons_resume.sh @@ -0,0 +1,124 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: GPL-2.0 + +# This test validates that netconsole is able to resume a target that was +# deactivated when its interface was removed when the interface is brought +# back up. +# +# The test configures a netconsole target and then removes netdevsim module to +# cause the interface to disappear. Targets are configured via cmdline to ensure +# targets bound by interface name and mac address can be resumed. +# The test verifies that the target moved to disabled state before adding +# netdevsim and the interface back. +# +# Finally, the test verifies that the target is re-enabled automatically and +# the message is received on the destination interface. +# +# Author: Andre Carvalho + +set -euo pipefail + +SCRIPTDIR=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")") + +source "${SCRIPTDIR}"/lib/sh/lib_netcons.sh + +SAVED_SRCMAC="" # to be populated later +SAVED_DSTMAC="" # to be populated later + +modprobe netdevsim 2> /dev/null || true +rmmod netconsole 2> /dev/null || true + +check_netconsole_module + +function cleanup() { + cleanup_netcons "${NETCONS_CONFIGFS}/cmdline0" + do_cleanup + rmmod netconsole +} + +function trigger_reactivation() { + # Add back low level module + modprobe netdevsim + # Recreate namespace and two interfaces + set_network + # Restore MACs + ip netns exec "${NAMESPACE}" ip link set "${DSTIF}" \ + address "${SAVED_DSTMAC}" + if [ "${BINDMODE}" == "mac" ]; then + ip link set dev "${SRCIF}" down + ip link set dev "${SRCIF}" address "${SAVED_SRCMAC}" + # Rename device in order to trigger target resume, as initial + # when device was recreated it didn't have correct mac address. + ip link set dev "${SRCIF}" name "${TARGET}" + fi +} + +function trigger_deactivation() { + # Start by storing mac addresses so we can be restored in reactivate + SAVED_DSTMAC=$(ip netns exec "${NAMESPACE}" \ + cat /sys/class/net/"$DSTIF"/address) + SAVED_SRCMAC=$(mac_get "${SRCIF}") + # Remove low level module + rmmod netdevsim +} + +trap cleanup EXIT + +# Run the test twice, with different cmdline parameters +for BINDMODE in "ifname" "mac" +do + echo "Running with bind mode: ${BINDMODE}" >&2 + # Set current loglevel to KERN_INFO(6), and default to KERN_NOTICE(5) + echo "6 5" > /proc/sys/kernel/printk + + # Create one namespace and two interfaces + set_network + + # Create the command line for netconsole, with the configuration from + # the function above + CMDLINE=$(create_cmdline_str "${BINDMODE}") + + # The content of kmsg will be save to the following file + OUTPUT_FILE="/tmp/${TARGET}-${BINDMODE}" + + # Load the module, with the cmdline set + modprobe netconsole "${CMDLINE}" + # Expose cmdline target in configfs + mkdir "${NETCONS_CONFIGFS}/cmdline0" + + # Target should be enabled + wait_target_state "cmdline0" "enabled" + + # Trigger deactivation by unloading netdevsim module. Target should be + # disabled. + trigger_deactivation + wait_target_state "cmdline0" "disabled" + + # Trigger reactivation by loading netdevsim, recreating the network and + # restoring mac addresses. Target should be re-enabled. + trigger_reactivation + wait_target_state "cmdline0" "enabled" + + # Listen for netconsole port inside the namespace and destination + # interface + listen_port_and_save_to "${OUTPUT_FILE}" & + # Wait for socat to start and listen to the port. + wait_local_port_listen "${NAMESPACE}" "${PORT}" udp + # Send the message + echo "${MSG}: ${TARGET}" > /dev/kmsg + # Wait until socat saves the file to disk + busywait "${BUSYWAIT_TIMEOUT}" test -s "${OUTPUT_FILE}" + # Make sure the message was received in the dst part + # and exit + validate_msg "${OUTPUT_FILE}" + + # kill socat in case it is still running + pkill_socat + # Cleanup & unload the module + cleanup + + echo "${BINDMODE} : Test passed" >&2 +done + +trap - EXIT +exit "${EXIT_STATUS}"