[net-next,v2,6/8] net: netconsole: cache userdata formatted string in netconsole_target
Commit Message
Store a formatted string for userdata that will be appended to netconsole
messages. The string has a capacity of 4KB, as calculated by the userdatum
entry length of 256 bytes and a max of 16 userdata entries.
Update the stored netconsole_target->userdata_complete string with the new
formatted userdata values when a userdatum is created, edited, or
removed. Each userdata entry contains a trailing newline, which will be
formatted as such in netconsole messages::
6.7.0-rc8-virtme,12,500,1646292204,-;test
release=foo
something=bar
6.7.0-rc8-virtme,12,500,1646292204,-;another test
release=foo
something=bar
Enforcement of MAX_USERDATA_ITEMS is done in userdatum_make_item;
update_userdata will not check for this case but will skip any userdata
children over the limit of MAX_USERDATA_ITEMs.
If a userdata entry/dir is created but no value is provided, that entry
will be skipped. This is in part because update_userdata() can't be
called in userdatum_make_item() since the item will not have been added
to the userdata config_group children yet. To preserve the experience of
adding an empty userdata that doesn't show up in the netconsole
messages, purposefully skip emtpy userdata items even when
update_userdata() can be called.
Co-developed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
drivers/net/netconsole.c | 63 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
Comments
Hi Matthew,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Wood/net-netconsole-cleanup-formatting-lints/20240127-072017
base: net-next/main
patch link: https://lore.kernel.org/r/20240126231348.281600-7-thepacketgeek%40gmail.com
patch subject: [PATCH net-next v2 6/8] net: netconsole: cache userdata formatted string in netconsole_target
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20240127/202401272022.r5Z4OtUg-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240127/202401272022.r5Z4OtUg-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401272022.r5Z4OtUg-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/net/netconsole.c: In function 'update_userdata':
>> drivers/net/netconsole.c:649:26: warning: variable 'ud' set but not used [-Wunused-but-set-variable]
649 | struct userdata *ud;
| ^~
vim +/ud +649 drivers/net/netconsole.c
644
645 static void update_userdata(struct netconsole_target *nt)
646 {
647 int complete_idx = 0, child_count = 0;
648 struct list_head *entry;
> 649 struct userdata *ud;
650
651 /* Clear the current string in case the last userdatum was deleted */
652 nt->userdata_length = 0;
653 nt->userdata_complete[0] = 0;
654
655 ud = to_userdata(&nt->userdata_group.cg_item);
656 list_for_each(entry, &nt->userdata_group.cg_children) {
657 struct userdatum *udm_item;
658 struct config_item *item;
659
660 if (child_count >= MAX_USERDATA_ITEMS)
661 break;
662 child_count++;
663
664 item = container_of(entry, struct config_item, ci_entry);
665 udm_item = to_userdatum(item);
666
667 /* Skip userdata with no value set */
668 if (strnlen(udm_item->value, MAX_USERDATA_VALUE_LENGTH) == 0)
669 continue;
670
671 /* This doesn't overflow userdata_complete since it will write
672 * one entry length (1/MAX_USERDATA_ITEMS long), entry count is
673 * checked to not exceed MAX items with child_count above
674 */
675 complete_idx += scnprintf(&nt->userdata_complete[complete_idx],
676 MAX_USERDATA_ENTRY_LENGTH, "%s=%s\n",
677 item->ci_name, udm_item->value);
678 }
679 nt->userdata_length = strnlen(nt->userdata_complete,
680 sizeof(nt->userdata_complete));
681 }
682
On Fri, Jan 26, 2024 at 03:13:41PM -0800, Matthew Wood wrote:
> Store a formatted string for userdata that will be appended to netconsole
> messages. The string has a capacity of 4KB, as calculated by the userdatum
> entry length of 256 bytes and a max of 16 userdata entries.
>
> Update the stored netconsole_target->userdata_complete string with the new
> formatted userdata values when a userdatum is created, edited, or
> removed. Each userdata entry contains a trailing newline, which will be
> formatted as such in netconsole messages::
>
> 6.7.0-rc8-virtme,12,500,1646292204,-;test
> release=foo
> something=bar
> 6.7.0-rc8-virtme,12,500,1646292204,-;another test
> release=foo
> something=bar
>
> Enforcement of MAX_USERDATA_ITEMS is done in userdatum_make_item;
> update_userdata will not check for this case but will skip any userdata
> children over the limit of MAX_USERDATA_ITEMs.
>
> If a userdata entry/dir is created but no value is provided, that entry
> will be skipped. This is in part because update_userdata() can't be
> called in userdatum_make_item() since the item will not have been added
> to the userdata config_group children yet. To preserve the experience of
> adding an empty userdata that doesn't show up in the netconsole
> messages, purposefully skip emtpy userdata items even when
nit: empty
> update_userdata() can be called.
>
> Co-developed-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
..
@@ -85,6 +85,8 @@ static struct console netconsole_ext;
* @list: Links this target into the target_list.
* @group: Links us into the configfs subsystem hierarchy.
* @userdata_group: Links to the userdata configfs hierarchy
+ * @userdata_complete: Cached, formatted string of append
+ * @userdata_length: String length of userdata_complete
* @enabled: On / off knob to enable / disable target.
* Visible from userspace (read-write).
* We maintain a strict 1:1 correspondence between this and
@@ -109,6 +111,8 @@ struct netconsole_target {
#ifdef CONFIG_NETCONSOLE_DYNAMIC
struct config_group group;
struct config_group userdata_group;
+ char userdata_complete[MAX_USERDATA_ENTRY_LENGTH * MAX_USERDATA_ITEMS];
+ size_t userdata_length;
#endif
bool enabled;
bool extended;
@@ -638,10 +642,50 @@ static ssize_t userdatum_value_show(struct config_item *item, char *buf)
return sysfs_emit(buf, "%s\n", &(to_userdatum(item)->value[0]));
}
+static void update_userdata(struct netconsole_target *nt)
+{
+ int complete_idx = 0, child_count = 0;
+ struct list_head *entry;
+ struct userdata *ud;
+
+ /* Clear the current string in case the last userdatum was deleted */
+ nt->userdata_length = 0;
+ nt->userdata_complete[0] = 0;
+
+ ud = to_userdata(&nt->userdata_group.cg_item);
+ list_for_each(entry, &nt->userdata_group.cg_children) {
+ struct userdatum *udm_item;
+ struct config_item *item;
+
+ if (child_count >= MAX_USERDATA_ITEMS)
+ break;
+ child_count++;
+
+ item = container_of(entry, struct config_item, ci_entry);
+ udm_item = to_userdatum(item);
+
+ /* Skip userdata with no value set */
+ if (strnlen(udm_item->value, MAX_USERDATA_VALUE_LENGTH) == 0)
+ continue;
+
+ /* This doesn't overflow userdata_complete since it will write
+ * one entry length (1/MAX_USERDATA_ITEMS long), entry count is
+ * checked to not exceed MAX items with child_count above
+ */
+ complete_idx += scnprintf(&nt->userdata_complete[complete_idx],
+ MAX_USERDATA_ENTRY_LENGTH, "%s=%s\n",
+ item->ci_name, udm_item->value);
+ }
+ nt->userdata_length = strnlen(nt->userdata_complete,
+ sizeof(nt->userdata_complete));
+}
+
static ssize_t userdatum_value_store(struct config_item *item, const char *buf,
size_t count)
{
struct userdatum *udm = to_userdatum(item);
+ struct netconsole_target *nt;
+ struct userdata *ud;
int ret;
if (count > MAX_USERDATA_VALUE_LENGTH)
@@ -654,6 +698,10 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf,
goto out_unlock;
trim_newline(udm->value, sizeof(udm->value));
+ ud = to_userdata(item->ci_parent);
+ nt = userdata_to_target(ud);
+ update_userdata(nt);
+
mutex_unlock(&dynamic_netconsole_mutex);
return count;
out_unlock:
@@ -708,12 +756,27 @@ static struct config_item *userdatum_make_item(struct config_group *group,
return &udm->item;
}
+static void userdatum_drop(struct config_group *group, struct config_item *item)
+{
+ struct netconsole_target *nt;
+ struct userdata *ud;
+
+ ud = to_userdata(&group->cg_item);
+ nt = userdata_to_target(ud);
+
+ mutex_lock(&dynamic_netconsole_mutex);
+ update_userdata(nt);
+ config_item_put(item);
+ mutex_unlock(&dynamic_netconsole_mutex);
+}
+
static struct configfs_attribute *userdata_attrs[] = {
NULL,
};
static struct configfs_group_operations userdata_ops = {
.make_item = userdatum_make_item,
+ .drop_item = userdatum_drop,
};
static struct config_item_type userdata_type = {