Skip to contentSkip to conversation

configfs: Create config_item from netconsole

DISCUSSION Breno Leitao 2 participants 3 messages Sep 28, 2023 – Oct 2, 2023

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Right now there is a limitation in netconsole, where it is impossible to disable or modify the target created from the command line parameter. (netconsole=...).

"netconsole" cmdline parameter sets the remote IP, and if the remote IP changes, the machine needs to be rebooted (with the new remote IP set in the command line parameter).

I am planning to reuse the dynamic netconsole mechanism for the 'command line' target, i.e., create a cmdline configfs_item for the "command line" target, so, the user can modify the "command line" target in configfs in runtime. Something as :

echo 0 > /sys/kernel/config/netconsole/cmdline/enabled echo <new-IP> > /sys/kernel/config/netconsole/cmdline/remote_ip echo 1 > /sys/kernel/config/netconsole/cmdline/enabled

I didn't find a configfs API to register a configfs_item into a config_group. Basically the make_item() callbacks are called once the inode is created by the user at mkdir(2) time, but now I need to create it at the driver initialization.

Should I create a configfs_register_item() to solve this problem?

Thanks!

#1 lore raw
Quoted 8 lines · Breno Leitao

Right now there is a limitation in netconsole, where it is impossible to disable or modify the target created from the command line parameter. (netconsole=...).

"netconsole" cmdline parameter sets the remote IP, and if the remote IP changes, the machine needs to be rebooted (with the new remote IP set in the command line parameter).

Quoted 8 lines

I am planning to reuse the dynamic netconsole mechanism for the 'command line' target, i.e., create a cmdline configfs_item for the "command line" target, so, the user can modify the "command line" target in configfs in runtime. Something as :

echo 0 > /sys/kernel/config/netconsole/cmdline/enabled echo <new-IP> > /sys/kernel/config/netconsole/cmdline/remote_ip echo 1 > /sys/kernel/config/netconsole/cmdline/enabled

Note that the netconsole= command line parameter can specify more than one network console, split by semicolons. Anything you do here has to be responsive to this. So you can't create a single cmdline entry.

Quoted 6 lines

I didn't find a configfs API to register a configfs_item into a config_group. Basically the make_item() callbacks are called once the inode is created by the user at mkdir(2) time, but now I need to create it at the driver initialization.

Should I create a configfs_register_item() to solve this problem?

It's an express philosophy of configfs that all lifetimes are controlled by userspace, which is why we don't have such a facility. If hch wants to change this, I defer to his judgement. But I don't think it is necessary.

What I would do instead is check whether a mkdir(2) call is trying to reference a command line entry. If so, attach it to the existing entry rather than creating a new one.

Currently, alloc_param_target() initializes netconsole_target.item to zeros. The item is never used by parameter-created targets. Step one would be to give it a name. So in init_netconsole(), right after alloc_param_target(), initialize nt->item just like we do in make_netconsole_target(). So something like:

```
+ #ifdef CONFIG_NETCONOLE_DYNAMIC
+       char target_name[16];
+       int target_count = 0;
+ #endif
	while ((target_config = strsep(&input, ";"))) {
			nt = alloc_param_target(target_config);
			if (IS_ERR(nt)) {
				err = PTR_ERR(nt);
				goto fail;
			}
+ #ifdef CONFIG_NETCONSOLE_DYNAMIC
+                       snprintf(target_name, 16, "cmdline", target_count);
+                       config_item_init_type_name(&nt->item, target_name,
+                                                  &netconsole_target_type);
+                       target_count++;
+ #endif
+
			/* Dump existing printks when we register */
			if (nt->extended) {
				extended = true;
```

Then, later in make_netconsole_target(), rather than blindly inserting the new netconsole_target in the list, you can check if the name is already present and use that. Here's some ugly pseudocode:

```
	spin_lock_irqsave(&target_list_lock, flags);
	list_for_each_entry(tmp, &target_list, list) {
		if (!strcmp(tmp->item.name, nt->item.name)) {
			existing = tmp;
			break;
		}
	}
	if (existing) {
		to_free = nt;
		nt = existing;
	} else {
		list_add(&nt->list, &target_list);
	}
	spin_unlock_irqrestore(&target_list_lock, flags);

	if (to_free)
		kfree(to_free);

	return &nt->item;
}
```

In this fashion, each console created on the command line will get a name of cmdline0, cmdline1, etc. They will not be part of the configfs tree. If the user comes along later and says mkdir /sys/kernel/config/netconsole/cmdline1, the existing cmdline1 console will be attached to the configfs tree. The user is then free to disable and reconfigure the device.

Note that this behavior cannot be triggered for netconsoles already created by configfs. If I try to mkdir /sys/kernel/config/netconsole/mydev twice, the second command will get -EEXISTS in filesystem code long before it reaches the netconsole code. Only when someone makes a config item that matches the console names can we traverse this code. Thus, matching the name is safe.

There would, of course, be some other corner cases to handle. Do we allow dynamic names that look like command-line names if no command-line parameter exists? Does rmdir /sys/kernel/config/netconsole/cmdline0 actually delete the command-line console entry, or does it return -EBUSY? And so on.

Thanks,
Joel

Signature · 5 lines
"When ideas fail, words come in very handy." 
         - Goethe

			http://www.jlbec.org/
			[email protected]
#2 lore raw

Hello Joel,

Quoted 8 lines · Joel Becker
Quoted 1 line · Breno Leitao

Should I create a configfs_register_item() to solve this problem?

It's an express philosophy of configfs that all lifetimes are controlled by userspace, which is why we don't have such a facility. If hch wants to change this, I defer to his judgement. But I don't think it is necessary.

I am happy the suggestion you gave, so, no need to change this philosophy.

Quoted 6 lines

In this fashion, each console created on the command line will get a name of cmdline0, cmdline1, etc. They will not be part of the configfs tree. If the user comes along later and says mkdir /sys/kernel/config/netconsole/cmdline1, the existing cmdline1 console will be attached to the configfs tree. The user is then free to disable and reconfigure the device.

I am happy with this approach.

Quoted 3 lines

There would, of course, be some other corner cases to handle. Do we allow dynamic names that look like command-line names if no command-line parameter exists?

I'd say so, I think cmdline targets shouldn't be special if there are not netconsole targets defined at boot time.

Quoted 3 lines

Does rmdir /sys/kernel/config/netconsole/cmdline0 actually delete the command-line console entry, or does it return -EBUSY? And so on.

I've looked at the code and tested, it cmdline0could be removed as any other target entry. On top of that, the user can create a new cmdline0 entry, and, it will be set with the default values for the attributes.

I am planning to send a patch based on what you suggested, and we can continue the discuss there.

Thanks for the _very_ detailed feedback!
Breno