resctrlfs.c file contains mostly functions that interact in some way
with resctrl FS entries while functions inside resctrl_val.c deal with
measurements and benchmarking.
run_benchmark() is located in resctrlfs.c file even though it's
purpose is not interacting with the resctrl FS but to execute cache
checking logic.
Move run_benchmark() to resctrl_val.c just before resctrl_val() that
makes use of run_benchmark(). Make run_benchmark() static since it's
not used between multiple files anymore.
Remove return comment from kernel-doc since the function is type void.
Signed-off-by: Wieczor-Retman Maciej <maciej.wieczor-retman(a)intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen(a)linux.intel.com>
---
Changelog v3:
- Make run_benchmark() static and remove it from the header. (Reinette)
- Remove return void kernel-doc comment. (Ilpo)
- Added Ilpo's reviewed-by tag.
tools/testing/selftests/resctrl/resctrl.h | 1 -
tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++++++++
tools/testing/selftests/resctrl/resctrlfs.c | 52 -------------------
3 files changed, 50 insertions(+), 53 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 838d1a438f33..c6cca28810cd 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -89,7 +89,6 @@ int validate_bw_report_request(char *bw_report);
bool validate_resctrl_feature_request(const char *resctrl_val);
char *fgrep(FILE *inf, const char *str);
int taskset_benchmark(pid_t bm_pid, int cpu_no);
-void run_benchmark(int signum, siginfo_t *info, void *ucontext);
int write_schemata(char *ctrlgrp, char *schemata, int cpu_no,
char *resctrl_val);
int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index f0f6c5f6e98b..764acf5efa14 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -621,6 +621,56 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start)
return 0;
}
+/*
+ * run_benchmark - Run a specified benchmark or fill_buf (default benchmark)
+ * in specified signal. Direct benchmark stdio to /dev/null.
+ * @signum: signal number
+ * @info: signal info
+ * @ucontext: user context in signal handling
+ */
+static void run_benchmark(int signum, siginfo_t *info, void *ucontext)
+{
+ int operation, ret, memflush;
+ char **benchmark_cmd;
+ size_t span;
+ bool once;
+ FILE *fp;
+
+ benchmark_cmd = info->si_ptr;
+
+ /*
+ * Direct stdio of child to /dev/null, so that only parent writes to
+ * stdio (console)
+ */
+ fp = freopen("/dev/null", "w", stdout);
+ if (!fp)
+ PARENT_EXIT("Unable to direct benchmark status to /dev/null");
+
+ if (strcmp(benchmark_cmd[0], "fill_buf") == 0) {
+ /* Execute default fill_buf benchmark */
+ span = strtoul(benchmark_cmd[1], NULL, 10);
+ memflush = atoi(benchmark_cmd[2]);
+ operation = atoi(benchmark_cmd[3]);
+ if (!strcmp(benchmark_cmd[4], "true"))
+ once = true;
+ else if (!strcmp(benchmark_cmd[4], "false"))
+ once = false;
+ else
+ PARENT_EXIT("Invalid once parameter");
+
+ if (run_fill_buf(span, memflush, operation, once))
+ fprintf(stderr, "Error in running fill buffer\n");
+ } else {
+ /* Execute specified benchmark */
+ ret = execvp(benchmark_cmd[0], benchmark_cmd);
+ if (ret)
+ perror("wrong\n");
+ }
+
+ fclose(stdout);
+ PARENT_EXIT("Unable to run specified benchmark");
+}
+
/*
* resctrl_val: execute benchmark and measure memory bandwidth on
* the benchmark
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index b0b14a5bcbf5..2f1ca184ac40 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -291,58 +291,6 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no)
return 0;
}
-/*
- * run_benchmark - Run a specified benchmark or fill_buf (default benchmark)
- * in specified signal. Direct benchmark stdio to /dev/null.
- * @signum: signal number
- * @info: signal info
- * @ucontext: user context in signal handling
- *
- * Return: void
- */
-void run_benchmark(int signum, siginfo_t *info, void *ucontext)
-{
- int operation, ret, memflush;
- char **benchmark_cmd;
- size_t span;
- bool once;
- FILE *fp;
-
- benchmark_cmd = info->si_ptr;
-
- /*
- * Direct stdio of child to /dev/null, so that only parent writes to
- * stdio (console)
- */
- fp = freopen("/dev/null", "w", stdout);
- if (!fp)
- PARENT_EXIT("Unable to direct benchmark status to /dev/null");
-
- if (strcmp(benchmark_cmd[0], "fill_buf") == 0) {
- /* Execute default fill_buf benchmark */
- span = strtoul(benchmark_cmd[1], NULL, 10);
- memflush = atoi(benchmark_cmd[2]);
- operation = atoi(benchmark_cmd[3]);
- if (!strcmp(benchmark_cmd[4], "true"))
- once = true;
- else if (!strcmp(benchmark_cmd[4], "false"))
- once = false;
- else
- PARENT_EXIT("Invalid once parameter");
-
- if (run_fill_buf(span, memflush, operation, once))
- fprintf(stderr, "Error in running fill buffer\n");
- } else {
- /* Execute specified benchmark */
- ret = execvp(benchmark_cmd[0], benchmark_cmd);
- if (ret)
- perror("wrong\n");
- }
-
- fclose(stdout);
- PARENT_EXIT("Unable to run specified benchmark");
-}
-
/*
* create_grp - Create a group only if one doesn't exist
* @grp_name: Name of the group
--
2.42.0
Writing bitmasks to the schemata can fail when the bitmask doesn't
adhere to constraints defined by what a particular CPU supports.
Some example of constraints are max length or having contiguous bits.
The driver should properly return errors when any rule concerning
bitmask format is broken.
Resctrl FS returns error codes from fprintf() only when fclose() is
called. Current error checking scheme allows invalid bitmasks to be
written into schemata file and the selftest doesn't notice because the
fclose() error code isn't checked.
Substitute fopen(), flose() and fprintf() with open(), close() and
write() to avoid error code buffering between fprintf() and fclose().
Remove newline character from the schema string after writing it to
the schemata file so it prints correctly before function return.
Pass the string generated with strerror() to the "reason" buffer so
the error message is more verbose. Extend "reason" buffer so it can hold
longer messages.
Signed-off-by: Wieczor-Retman Maciej <maciej.wieczor-retman(a)intel.com>
---
Changelog v3:
- Rename fp to fd (Ilpo)
- Remove strlen, strcspn and just use the snprintf value instead (Ilpo)
Changelog v2:
- Rewrite patch message.
- Double "reason" buffer size to fit longer error explanation.
- Redo file interactions with syscalls instead of stdio functions.
tools/testing/selftests/resctrl/resctrlfs.c | 26 +++++++++++----------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index bd36ee206602..b0b14a5bcbf5 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -488,9 +488,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
*/
int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
{
- char controlgroup[1024], schema[1024], reason[64];
- int resource_id, ret = 0;
- FILE *fp;
+ char controlgroup[1024], schema[1024], reason[128];
+ int resource_id, fd, schema_len = -1, ret = 0;
if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) &&
strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) &&
@@ -518,27 +517,30 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) ||
!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
- sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata);
+ schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n",
+ "L3:", resource_id, '=', schemata);
if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) ||
!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)))
- sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata);
+ schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n",
+ "MB:", resource_id, '=', schemata);
- fp = fopen(controlgroup, "w");
- if (!fp) {
+ fd = open(controlgroup, O_WRONLY);
+ if (!fd) {
sprintf(reason, "Failed to open control group");
ret = -1;
goto out;
}
-
- if (fprintf(fp, "%s\n", schema) < 0) {
- sprintf(reason, "Failed to write schemata in control group");
- fclose(fp);
+ if (write(fd, schema, schema_len) < 0) {
+ snprintf(reason, sizeof(reason),
+ "write() failed : %s", strerror(errno));
+ close(fd);
ret = -1;
goto out;
}
- fclose(fp);
+ close(fd);
+ schema[schema_len - 1] = 0;
out:
ksft_print_msg("Write schema \"%s\" to resctrl FS%s%s\n",
--
2.42.0
Write_schemata() uses fprintf() to write a bitmask into a schemata file
inside resctrl FS. It checks fprintf() return value but it doesn't check
fclose() return value. Error codes from fprintf() such as write errors,
are buffered and flushed back to the user only after fclose() is executed
which means any invalid bitmask can be written into the schemata file.
Rewrite write_schemata() to use syscalls instead of stdio file
operations to avoid the buffering.
The resctrlfs.c file defines functions that interact with the resctrl FS
while resctrl_val.c file defines functions that perform measurements on
the cache. Run_benchmark() fits logically into the second file before
resctrl_val() function that uses it.
Move run_benchmark() from resctrlfs.c to resctrl_val.c and remove
redundant part of the kernel-doc comment. Make run_benchmark() static
and remove it from the header file.
Series is based on kselftest next branch.
Changelog v3:
- Use snprintf() return value instead of strlen() in write_schemata().
(Ilpo)
- Make run_benchmark() static and remove it from the header file.
(Reinette)
- Added Ilpo's reviewed-by tag to Patch 2/2.
- Patch messages and cover letter rewording.
Changelog v2:
- Change sprintf() to snprintf() in write_schemata().
- Redo write_schemata() with syscalls instead of stdio functions.
- Fix typos and missing dots in patch messages.
- Branch printf attribute patch to a separate series.
[v1] https://lore.kernel.org/all/cover.1692880423.git.maciej.wieczor-retman@inte…
[v2] https://lore.kernel.org/all/cover.1693213468.git.maciej.wieczor-retman@inte…
Wieczor-Retman Maciej (2):
selftests/resctrl: Fix schemata write error check
selftests/resctrl: Move run_benchmark() to a more fitting file
tools/testing/selftests/resctrl/resctrl.h | 1 -
tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++
tools/testing/selftests/resctrl/resctrlfs.c | 79 ++++---------------
3 files changed, 65 insertions(+), 65 deletions(-)
base-commit: 9b1db732866bee060b9bca9493e5ebf5e8874c48
--
2.42.0
The benchmark command handling (-b) in resctrl selftests is overly
complicated code. This series turns the benchmark command immutable to
preserve it for all selftests and improves benchmark command related
error handling.
This series also ends up removing the strcpy() calls which were pointed
out earlier.
v4:
- Correct off-by-one error in -b processing
- Reordered code in main() to make freeing span_str simpler (in new patch)
- Use consistent style for const char * const *
v3:
- Removed DEFAULT_SPAN_STR for real and the duplicated copy of defines
that made to v2 likely due to my incorrect conflict resolutions
v2:
- Added argument length check into patch 1/7
- Updated also -b line in help message.
- Document -b argument related "algorithm"
- Use asprintf() to convert defined constant int to string
- Improved changelog texts
- Added \n to ksft_exit_fail_msg() call messages.
- Print DEFAULT_SPAN with %u instead of %zu to avoid need to cast it
Ilpo Järvinen (8):
selftests/resctrl: Ensure the benchmark commands fits to its array
selftests/resctrl: Correct benchmark command help
selftests/resctrl: Remove bw_report and bm_type from main()
selftests/resctrl: Simplify span lifetime
selftests/resctrl: Reorder resctrl FS prep code and benchmark_cmd init
selftests/resctrl: Make benchmark command const and build it with
pointers
selftests/resctrl: Remove ben_count variable
selftests/resctrl: Cleanup benchmark argument parsing
tools/testing/selftests/resctrl/cache.c | 5 +-
tools/testing/selftests/resctrl/cat_test.c | 13 +--
tools/testing/selftests/resctrl/cmt_test.c | 34 ++++--
tools/testing/selftests/resctrl/mba_test.c | 4 +-
tools/testing/selftests/resctrl/mbm_test.c | 7 +-
tools/testing/selftests/resctrl/resctrl.h | 16 +--
.../testing/selftests/resctrl/resctrl_tests.c | 100 ++++++++----------
tools/testing/selftests/resctrl/resctrl_val.c | 10 +-
8 files changed, 104 insertions(+), 85 deletions(-)
--
2.30.2
All packets in the same flow (L3/L4 depending on multipath hash policy)
should be directed to the same target, but after [0]/[1] we see stray
packets directed towards other targets. This, for instance, causes RST
to be sent on TCP connections.
The first two patches solve the problem by ignoring route hints for
destinations that are part of multipath group, by using new SKB flags
for IPv4 and IPv6. The third patch is a selftest that tests the
scenario.
Thanks to Ido, for reviewing and suggesting a way forward in [2] and
also suggesting how to write a selftest for this.
v4->v5:
- Fixed review comments from Ido
v3->v4:
- Remove single path test
- Rebase to latest
v2->v3:
- Add NULL check for skb in fib6_select_path (Ido Schimmel)
- Use fib_tests.sh for selftest instead of the forwarding suite (Ido
Schimmel)
v1->v2:
- Update to commit messages describing the solution (Ido Schimmel)
- Use perf stat to count fib table lookups in selftest (Ido Schimmel)
Sriram Yagnaraman (3):
ipv4: ignore dst hint for multipath routes
ipv6: ignore dst hint for multipath routes
selftests: fib_tests: Add multipath list receive tests
include/linux/ipv6.h | 1 +
include/net/ip.h | 1 +
net/ipv4/ip_input.c | 3 +-
net/ipv4/route.c | 1 +
net/ipv6/ip6_input.c | 3 +-
net/ipv6/route.c | 3 +
tools/testing/selftests/net/fib_tests.sh | 155 ++++++++++++++++++++++-
7 files changed, 164 insertions(+), 3 deletions(-)
--
2.34.1