* [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02
@ 2022-09-14 13:19 Dylan Jhong
2022-09-15 8:58 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Dylan Jhong @ 2022-09-14 13:19 UTC (permalink / raw)
To: ltp; +Cc: alankao, minachou, dminus, x5710999x
When CONFIG_SMP is not selected, CONFIG_RPS will not be enabled. Causes the
kernel to not create rx queues in sysfs[*1] when creating tun devices.
uevent02 will check if the rx queue exists, so we have to dynamically adjust
the number of uevents to pass this testcase.
[*1]: https://github.com/torvalds/linux/blob/3245cb65fd91cd514801bf91f5a3066d562f0ac4/net/core/net-sysfs.c#L1109
Signed-off-by: Dylan Jhong <dylan@andestech.com>
---
testcases/kernel/uevents/uevent02.c | 146 ++++++++++++++--------------
1 file changed, 73 insertions(+), 73 deletions(-)
diff --git a/testcases/kernel/uevents/uevent02.c b/testcases/kernel/uevents/uevent02.c
index ce0cf757d..059320f1c 100644
--- a/testcases/kernel/uevents/uevent02.c
+++ b/testcases/kernel/uevents/uevent02.c
@@ -19,11 +19,71 @@
#include <linux/if_tun.h>
#include "tst_test.h"
+#include "tst_private.h"
#include "uevent.h"
#define TUN_PATH "/dev/net/tun"
+#define MAX_UEVENT 7
+
+struct uevent_desc add = {
+ .msg = "add@/devices/virtual/net/ltp-tun0",
+ .value_cnt = 4,
+ .values = (const char*[]) {
+ "ACTION=add",
+ "DEVPATH=/devices/virtual/net/ltp-tun0",
+ "SUBSYSTEM=net",
+ "INTERFACE=ltp-tun0",
+ }
+};
+struct uevent_desc add_rx = {
+ .msg = "add@/devices/virtual/net/ltp-tun0/queues/rx-0",
+ .value_cnt = 3,
+ .values = (const char*[]) {
+ "ACTION=add",
+ "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
+ "SUBSYSTEM=queues",
+ }
+};
+struct uevent_desc add_tx = {
+ .msg = "add@/devices/virtual/net/ltp-tun0/queues/tx-0",
+ .value_cnt = 3,
+ .values = (const char*[]) {
+ "ACTION=add",
+ "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
+ "SUBSYSTEM=queues",
+ }
+};
+struct uevent_desc rem_rx = {
+ .msg = "remove@/devices/virtual/net/ltp-tun0/queues/rx-0",
+ .value_cnt = 3,
+ .values = (const char*[]) {
+ "ACTION=remove",
+ "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
+ "SUBSYSTEM=queues",
+ }
+};
+struct uevent_desc rem_tx = {
+ .msg = "remove@/devices/virtual/net/ltp-tun0/queues/tx-0",
+ .value_cnt = 3,
+ .values = (const char*[]) {
+ "ACTION=remove",
+ "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
+ "SUBSYSTEM=queues",
+ }
+};
+struct uevent_desc rem = {
+ .msg = "remove@/devices/virtual/net/ltp-tun0",
+ .value_cnt = 4,
+ .values = (const char*[]) {
+ "ACTION=remove",
+ "DEVPATH=/devices/virtual/net/ltp-tun0",
+ "SUBSYSTEM=net",
+ "INTERFACE=ltp-tun0",
+ }
+};
+
static void generate_tun_uevents(void)
{
int fd = SAFE_OPEN(TUN_PATH, O_RDWR);
@@ -42,79 +102,19 @@ static void generate_tun_uevents(void)
static void verify_uevent(void)
{
- int pid, fd;
-
- struct uevent_desc add = {
- .msg = "add@/devices/virtual/net/ltp-tun0",
- .value_cnt = 4,
- .values = (const char*[]) {
- "ACTION=add",
- "DEVPATH=/devices/virtual/net/ltp-tun0",
- "SUBSYSTEM=net",
- "INTERFACE=ltp-tun0",
- }
- };
-
- struct uevent_desc add_rx = {
- .msg = "add@/devices/virtual/net/ltp-tun0/queues/rx-0",
- .value_cnt = 3,
- .values = (const char*[]) {
- "ACTION=add",
- "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
- "SUBSYSTEM=queues",
- }
- };
-
- struct uevent_desc add_tx = {
- .msg = "add@/devices/virtual/net/ltp-tun0/queues/tx-0",
- .value_cnt = 3,
- .values = (const char*[]) {
- "ACTION=add",
- "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
- "SUBSYSTEM=queues",
- }
- };
-
- struct uevent_desc rem_rx = {
- .msg = "remove@/devices/virtual/net/ltp-tun0/queues/rx-0",
- .value_cnt = 3,
- .values = (const char*[]) {
- "ACTION=remove",
- "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
- "SUBSYSTEM=queues",
- }
- };
-
- struct uevent_desc rem_tx = {
- .msg = "remove@/devices/virtual/net/ltp-tun0/queues/tx-0",
- .value_cnt = 3,
- .values = (const char*[]) {
- "ACTION=remove",
- "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
- "SUBSYSTEM=queues",
- }
- };
-
- struct uevent_desc rem = {
- .msg = "remove@/devices/virtual/net/ltp-tun0",
- .value_cnt = 4,
- .values = (const char*[]) {
- "ACTION=remove",
- "DEVPATH=/devices/virtual/net/ltp-tun0",
- "SUBSYSTEM=net",
- "INTERFACE=ltp-tun0",
- }
- };
-
- const struct uevent_desc *const uevents[] = {
- &add,
- &add_rx,
- &add_tx,
- &rem_rx,
- &rem_tx,
- &rem,
- NULL
- };
+ const struct uevent_desc *uevents[MAX_UEVENT];
+ int pid, fd, i = 0;
+ int has_RPS = tst_kconfig_get("CONFIG_RPS");
+
+ uevents[i++] = &add;
+ if (has_RPS)
+ uevents[i++] = &add_rx;
+ uevents[i++] = &add_tx;
+ if (has_RPS)
+ uevents[i++] = &rem_rx;
+ uevents[i++] = &rem_tx;
+ uevents[i++] = &rem;
+ uevents[i++] = NULL;
pid = SAFE_FORK();
if (!pid) {
--
2.34.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02
2022-09-14 13:19 [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02 Dylan Jhong
@ 2022-09-15 8:58 ` Cyril Hrubis
2022-09-15 10:03 ` Dylan Jhong
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2022-09-15 8:58 UTC (permalink / raw)
To: Dylan Jhong; +Cc: minachou, x5710999x, dminus, alankao, ltp
Hi!
> Signed-off-by: Dylan Jhong <dylan@andestech.com>
> ---
> testcases/kernel/uevents/uevent02.c | 146 ++++++++++++++--------------
> 1 file changed, 73 insertions(+), 73 deletions(-)
>
> diff --git a/testcases/kernel/uevents/uevent02.c b/testcases/kernel/uevents/uevent02.c
> index ce0cf757d..059320f1c 100644
> --- a/testcases/kernel/uevents/uevent02.c
> +++ b/testcases/kernel/uevents/uevent02.c
> @@ -19,11 +19,71 @@
> #include <linux/if_tun.h>
>
> #include "tst_test.h"
> +#include "tst_private.h"
This header is called private for a reason, the tst_kconfig_get() is not
meant to be used from tests, you are supposed to call tst_config_read()
as it's done in shmget02.c for example.
> #include "uevent.h"
>
> #define TUN_PATH "/dev/net/tun"
>
> +#define MAX_UEVENT 7
> +
> +struct uevent_desc add = {
> + .msg = "add@/devices/virtual/net/ltp-tun0",
> + .value_cnt = 4,
> + .values = (const char*[]) {
> + "ACTION=add",
> + "DEVPATH=/devices/virtual/net/ltp-tun0",
> + "SUBSYSTEM=net",
> + "INTERFACE=ltp-tun0",
> + }
> +};
> +struct uevent_desc add_rx = {
> + .msg = "add@/devices/virtual/net/ltp-tun0/queues/rx-0",
> + .value_cnt = 3,
> + .values = (const char*[]) {
> + "ACTION=add",
> + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> + "SUBSYSTEM=queues",
> + }
> +};
> +struct uevent_desc add_tx = {
> + .msg = "add@/devices/virtual/net/ltp-tun0/queues/tx-0",
> + .value_cnt = 3,
> + .values = (const char*[]) {
> + "ACTION=add",
> + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> + "SUBSYSTEM=queues",
> + }
> +};
> +struct uevent_desc rem_rx = {
> + .msg = "remove@/devices/virtual/net/ltp-tun0/queues/rx-0",
> + .value_cnt = 3,
> + .values = (const char*[]) {
> + "ACTION=remove",
> + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> + "SUBSYSTEM=queues",
> + }
> +};
> +struct uevent_desc rem_tx = {
> + .msg = "remove@/devices/virtual/net/ltp-tun0/queues/tx-0",
> + .value_cnt = 3,
> + .values = (const char*[]) {
> + "ACTION=remove",
> + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> + "SUBSYSTEM=queues",
> + }
> +};
> +struct uevent_desc rem = {
> + .msg = "remove@/devices/virtual/net/ltp-tun0",
> + .value_cnt = 4,
> + .values = (const char*[]) {
> + "ACTION=remove",
> + "DEVPATH=/devices/virtual/net/ltp-tun0",
> + "SUBSYSTEM=net",
> + "INTERFACE=ltp-tun0",
> + }
> +};
Why do we have to move these outside of the function? I do not see a
single reason to do so.
> + const struct uevent_desc *uevents[MAX_UEVENT];
> + int pid, fd, i = 0;
> + int has_RPS = tst_kconfig_get("CONFIG_RPS");
Getting the flag should be done once in the test setup, otherwise kernel
config will be parsed in each iteration of the test.
> + uevents[i++] = &add;
> + if (has_RPS)
> + uevents[i++] = &add_rx;
> + uevents[i++] = &add_tx;
> + if (has_RPS)
> + uevents[i++] = &rem_rx;
> + uevents[i++] = &rem_tx;
> + uevents[i++] = &rem;
> + uevents[i++] = NULL;
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02
2022-09-15 8:58 ` Cyril Hrubis
@ 2022-09-15 10:03 ` Dylan Jhong
2022-09-15 10:09 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Dylan Jhong @ 2022-09-15 10:03 UTC (permalink / raw)
To: Cyril Hrubis
Cc: Mina Hui-Min Chou(((((((), x5710999x@gmail.com,
Charles Ci-Jyun Wu(((((((), Alan Quey-Liang Kao(((((((),
ltp@lists.linux.it
On Thu, Sep 15, 2022 at 04:58:56PM +0800, Cyril Hrubis wrote:
Hi Cyril,
Thanks for reviewing this patch.
> Hi!
> > Signed-off-by: Dylan Jhong <dylan@andestech.com>
> > ---
> > testcases/kernel/uevents/uevent02.c | 146 ++++++++++++++--------------
> > 1 file changed, 73 insertions(+), 73 deletions(-)
> >
> > diff --git a/testcases/kernel/uevents/uevent02.c b/testcases/kernel/uevents/uevent02.c
> > index ce0cf757d..059320f1c 100644
> > --- a/testcases/kernel/uevents/uevent02.c
> > +++ b/testcases/kernel/uevents/uevent02.c
> > @@ -19,11 +19,71 @@
> > #include <linux/if_tun.h>
> >
> > #include "tst_test.h"
> > +#include "tst_private.h"
>
> This header is called private for a reason, the tst_kconfig_get() is not
> meant to be used from tests, you are supposed to call tst_config_read()
> as it's done in shmget02.c for example.
>
Thanks. I will fix this in v2 patch.
> > #include "uevent.h"
> >
> > #define TUN_PATH "/dev/net/tun"
> >
> > +#define MAX_UEVENT 7
> > +
> > +struct uevent_desc add = {
> > + .msg = "add@/devices/virtual/net/ltp-tun0",
> > + .value_cnt = 4,
> > + .values = (const char*[]) {
> > + "ACTION=add",
> > + "DEVPATH=/devices/virtual/net/ltp-tun0",
> > + "SUBSYSTEM=net",
> > + "INTERFACE=ltp-tun0",
> > + }
> > +};
> > +struct uevent_desc add_rx = {
> > + .msg = "add@/devices/virtual/net/ltp-tun0/queues/rx-0",
> > + .value_cnt = 3,
> > + .values = (const char*[]) {
> > + "ACTION=add",
> > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> > + "SUBSYSTEM=queues",
> > + }
> > +};
> > +struct uevent_desc add_tx = {
> > + .msg = "add@/devices/virtual/net/ltp-tun0/queues/tx-0",
> > + .value_cnt = 3,
> > + .values = (const char*[]) {
> > + "ACTION=add",
> > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> > + "SUBSYSTEM=queues",
> > + }
> > +};
> > +struct uevent_desc rem_rx = {
> > + .msg = "remove@/devices/virtual/net/ltp-tun0/queues/rx-0",
> > + .value_cnt = 3,
> > + .values = (const char*[]) {
> > + "ACTION=remove",
> > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> > + "SUBSYSTEM=queues",
> > + }
> > +};
> > +struct uevent_desc rem_tx = {
> > + .msg = "remove@/devices/virtual/net/ltp-tun0/queues/tx-0",
> > + .value_cnt = 3,
> > + .values = (const char*[]) {
> > + "ACTION=remove",
> > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> > + "SUBSYSTEM=queues",
> > + }
> > +};
> > +struct uevent_desc rem = {
> > + .msg = "remove@/devices/virtual/net/ltp-tun0",
> > + .value_cnt = 4,
> > + .values = (const char*[]) {
> > + "ACTION=remove",
> > + "DEVPATH=/devices/virtual/net/ltp-tun0",
> > + "SUBSYSTEM=net",
> > + "INTERFACE=ltp-tun0",
> > + }
> > +};
>
> Why do we have to move these outside of the function? I do not see a
> single reason to do so.
>
I think separating the declaration of uevents and dynamic adding uevents
will make the program easier to read.
This part is open to discussion, I'm just giving my thoughts.
if everyone think it's a bad idea, I'll change it back in v2 patch.
declaration
----------------------------
struct uevent_desc rem_tx = {}
struct uevent_desc add_rx = {}
struct uevent_desc add_tx = {}
.....
dynamically adding uevent:
--------------------------
uevents[i++] = &add;
if (has_RPS)
uevents[i++] = &add_rx;
uevents[i++] = &add_tx;
if (has_RPS)
uevents[i++] = &rem_rx;
uevents[i++] = &rem_tx;
uevents[i++] = &rem;
uevents[i++] = NULL;
> > + const struct uevent_desc *uevents[MAX_UEVENT];
> > + int pid, fd, i = 0;
> > + int has_RPS = tst_kconfig_get("CONFIG_RPS");
>
> Getting the flag should be done once in the test setup, otherwise kernel
> config will be parsed in each iteration of the test.
>
If we parse the kernel configuration during test setup, we will block all
images without CONFIG_RPS from executing this testcase. This is not my
intention, in this patch I designed to continue executing uevent02 even
without CONFIG_RPS. Just adjusting uevents can pass this test. So I think
parsing the kernel config in test_all() should be the correct way.
Best,
Dylan
> > + uevents[i++] = &add;
> > + if (has_RPS)
> > + uevents[i++] = &add_rx;
> > + uevents[i++] = &add_tx;
> > + if (has_RPS)
> > + uevents[i++] = &rem_rx;
> > + uevents[i++] = &rem_tx;
> > + uevents[i++] = &rem;
> > + uevents[i++] = NULL;
>
>
>
> --
> Cyril Hrubis
> chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02
2022-09-15 10:03 ` Dylan Jhong
@ 2022-09-15 10:09 ` Cyril Hrubis
2022-09-15 10:32 ` Dylan Jhong
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2022-09-15 10:09 UTC (permalink / raw)
To: Dylan Jhong
Cc: Mina Hui-Min Chou(((((((), x5710999x@gmail.com,
Charles Ci-Jyun Wu(((((((), Alan Quey-Liang Kao(((((((),
ltp@lists.linux.it
Hi!
> > > #include "uevent.h"
> > >
> > > #define TUN_PATH "/dev/net/tun"
> > >
> > > +#define MAX_UEVENT 7
> > > +
> > > +struct uevent_desc add = {
> > > + .msg = "add@/devices/virtual/net/ltp-tun0",
> > > + .value_cnt = 4,
> > > + .values = (const char*[]) {
> > > + "ACTION=add",
> > > + "DEVPATH=/devices/virtual/net/ltp-tun0",
> > > + "SUBSYSTEM=net",
> > > + "INTERFACE=ltp-tun0",
> > > + }
> > > +};
> > > +struct uevent_desc add_rx = {
> > > + .msg = "add@/devices/virtual/net/ltp-tun0/queues/rx-0",
> > > + .value_cnt = 3,
> > > + .values = (const char*[]) {
> > > + "ACTION=add",
> > > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> > > + "SUBSYSTEM=queues",
> > > + }
> > > +};
> > > +struct uevent_desc add_tx = {
> > > + .msg = "add@/devices/virtual/net/ltp-tun0/queues/tx-0",
> > > + .value_cnt = 3,
> > > + .values = (const char*[]) {
> > > + "ACTION=add",
> > > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> > > + "SUBSYSTEM=queues",
> > > + }
> > > +};
> > > +struct uevent_desc rem_rx = {
> > > + .msg = "remove@/devices/virtual/net/ltp-tun0/queues/rx-0",
> > > + .value_cnt = 3,
> > > + .values = (const char*[]) {
> > > + "ACTION=remove",
> > > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> > > + "SUBSYSTEM=queues",
> > > + }
> > > +};
> > > +struct uevent_desc rem_tx = {
> > > + .msg = "remove@/devices/virtual/net/ltp-tun0/queues/tx-0",
> > > + .value_cnt = 3,
> > > + .values = (const char*[]) {
> > > + "ACTION=remove",
> > > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> > > + "SUBSYSTEM=queues",
> > > + }
> > > +};
> > > +struct uevent_desc rem = {
> > > + .msg = "remove@/devices/virtual/net/ltp-tun0",
> > > + .value_cnt = 4,
> > > + .values = (const char*[]) {
> > > + "ACTION=remove",
> > > + "DEVPATH=/devices/virtual/net/ltp-tun0",
> > > + "SUBSYSTEM=net",
> > > + "INTERFACE=ltp-tun0",
> > > + }
> > > +};
> >
> > Why do we have to move these outside of the function? I do not see a
> > single reason to do so.
> >
>
> I think separating the declaration of uevents and dynamic adding uevents
> will make the program easier to read.
> This part is open to discussion, I'm just giving my thoughts.
> if everyone think it's a bad idea, I'll change it back in v2 patch.
>
> declaration
> ----------------------------
> struct uevent_desc rem_tx = {}
> struct uevent_desc add_rx = {}
> struct uevent_desc add_tx = {}
> .....
>
> dynamically adding uevent:
> --------------------------
> uevents[i++] = &add;
> if (has_RPS)
> uevents[i++] = &add_rx;
> uevents[i++] = &add_tx;
> if (has_RPS)
> uevents[i++] = &rem_rx;
> uevents[i++] = &rem_tx;
> uevents[i++] = &rem;
> uevents[i++] = NULL;
>
> > > + const struct uevent_desc *uevents[MAX_UEVENT];
> > > + int pid, fd, i = 0;
> > > + int has_RPS = tst_kconfig_get("CONFIG_RPS");
> >
> > Getting the flag should be done once in the test setup, otherwise kernel
> > config will be parsed in each iteration of the test.
> >
>
> If we parse the kernel configuration during test setup, we will block all
> images without CONFIG_RPS from executing this testcase. This is not my
> intention, in this patch I designed to continue executing uevent02 even
> without CONFIG_RPS. Just adjusting uevents can pass this test. So I think
> parsing the kernel config in test_all() should be the correct way.
I do not think that we understand each other here. What I proposed is to
call the function that parses kernel config in the test setup and
initialize the has_RPS flag there so that it's not called on each
interation of the run() function (e.g. when test is passed -i 100 on
command line).
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02
2022-09-15 10:09 ` Cyril Hrubis
@ 2022-09-15 10:32 ` Dylan Jhong
2022-09-15 11:06 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Dylan Jhong @ 2022-09-15 10:32 UTC (permalink / raw)
To: Cyril Hrubis
Cc: Mina Hui-Min Chou(((((((), x5710999x@gmail.com,
Charles Ci-Jyun Wu(((((((), Alan Quey-Liang Kao(((((((),
ltp@lists.linux.it
Hi Cyril,
Oh, i got it!!! Sorry for the misunderstanding.
I thought this was talking about adding the .needs_kconfigs property.
I will add setup() function in v2 patch.
And about the adjustment of uevent declaration, any comments that I can
do in v2 patch?
Best,
Dylan
On Thu, Sep 15, 2022 at 06:09:07PM +0800, Cyril Hrubis wrote:
> Hi!
> > > > #include "uevent.h"
> > > >
> > > > #define TUN_PATH "/dev/net/tun"
> > > >
> > > > +#define MAX_UEVENT 7
> > > > +
> > > > +struct uevent_desc add = {
> > > > + .msg = "add@/devices/virtual/net/ltp-tun0",
> > > > + .value_cnt = 4,
> > > > + .values = (const char*[]) {
> > > > + "ACTION=add",
> > > > + "DEVPATH=/devices/virtual/net/ltp-tun0",
> > > > + "SUBSYSTEM=net",
> > > > + "INTERFACE=ltp-tun0",
> > > > + }
> > > > +};
> > > > +struct uevent_desc add_rx = {
> > > > + .msg = "add@/devices/virtual/net/ltp-tun0/queues/rx-0",
> > > > + .value_cnt = 3,
> > > > + .values = (const char*[]) {
> > > > + "ACTION=add",
> > > > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> > > > + "SUBSYSTEM=queues",
> > > > + }
> > > > +};
> > > > +struct uevent_desc add_tx = {
> > > > + .msg = "add@/devices/virtual/net/ltp-tun0/queues/tx-0",
> > > > + .value_cnt = 3,
> > > > + .values = (const char*[]) {
> > > > + "ACTION=add",
> > > > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> > > > + "SUBSYSTEM=queues",
> > > > + }
> > > > +};
> > > > +struct uevent_desc rem_rx = {
> > > > + .msg = "remove@/devices/virtual/net/ltp-tun0/queues/rx-0",
> > > > + .value_cnt = 3,
> > > > + .values = (const char*[]) {
> > > > + "ACTION=remove",
> > > > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/rx-0",
> > > > + "SUBSYSTEM=queues",
> > > > + }
> > > > +};
> > > > +struct uevent_desc rem_tx = {
> > > > + .msg = "remove@/devices/virtual/net/ltp-tun0/queues/tx-0",
> > > > + .value_cnt = 3,
> > > > + .values = (const char*[]) {
> > > > + "ACTION=remove",
> > > > + "DEVPATH=/devices/virtual/net/ltp-tun0/queues/tx-0",
> > > > + "SUBSYSTEM=queues",
> > > > + }
> > > > +};
> > > > +struct uevent_desc rem = {
> > > > + .msg = "remove@/devices/virtual/net/ltp-tun0",
> > > > + .value_cnt = 4,
> > > > + .values = (const char*[]) {
> > > > + "ACTION=remove",
> > > > + "DEVPATH=/devices/virtual/net/ltp-tun0",
> > > > + "SUBSYSTEM=net",
> > > > + "INTERFACE=ltp-tun0",
> > > > + }
> > > > +};
> > >
> > > Why do we have to move these outside of the function? I do not see a
> > > single reason to do so.
> > >
> >
> > I think separating the declaration of uevents and dynamic adding uevents
> > will make the program easier to read.
> > This part is open to discussion, I'm just giving my thoughts.
> > if everyone think it's a bad idea, I'll change it back in v2 patch.
> >
> > declaration
> > ----------------------------
> > struct uevent_desc rem_tx = {}
> > struct uevent_desc add_rx = {}
> > struct uevent_desc add_tx = {}
> > .....
> >
> > dynamically adding uevent:
> > --------------------------
> > uevents[i++] = &add;
> > if (has_RPS)
> > uevents[i++] = &add_rx;
> > uevents[i++] = &add_tx;
> > if (has_RPS)
> > uevents[i++] = &rem_rx;
> > uevents[i++] = &rem_tx;
> > uevents[i++] = &rem;
> > uevents[i++] = NULL;
> >
> > > > + const struct uevent_desc *uevents[MAX_UEVENT];
> > > > + int pid, fd, i = 0;
> > > > + int has_RPS = tst_kconfig_get("CONFIG_RPS");
> > >
> > > Getting the flag should be done once in the test setup, otherwise kernel
> > > config will be parsed in each iteration of the test.
> > >
> >
> > If we parse the kernel configuration during test setup, we will block all
> > images without CONFIG_RPS from executing this testcase. This is not my
> > intention, in this patch I designed to continue executing uevent02 even
> > without CONFIG_RPS. Just adjusting uevents can pass this test. So I think
> > parsing the kernel config in test_all() should be the correct way.
>
> I do not think that we understand each other here. What I proposed is to
> call the function that parses kernel config in the test setup and
> initialize the has_RPS flag there so that it's not called on each
> interation of the run() function (e.g. when test is passed -i 100 on
> command line).
>
> --
> Cyril Hrubis
> chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02
2022-09-15 10:32 ` Dylan Jhong
@ 2022-09-15 11:06 ` Cyril Hrubis
0 siblings, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2022-09-15 11:06 UTC (permalink / raw)
To: Dylan Jhong
Cc: Mina Hui-Min Chou(((((((), x5710999x@gmail.com,
Charles Ci-Jyun Wu(((((((), Alan Quey-Liang Kao(((((((),
ltp@lists.linux.it
Hi!
> Oh, i got it!!! Sorry for the misunderstanding.
> I thought this was talking about adding the .needs_kconfigs property.
> I will add setup() function in v2 patch.
>
> And about the adjustment of uevent declaration, any comments that I can
> do in v2 patch?
I would avoid any unnecessary changes.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-15 11:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-14 13:19 [LTP] [PATCH] kernel/uevent: Adjust the number of uevents dynamically in uevent02 Dylan Jhong
2022-09-15 8:58 ` Cyril Hrubis
2022-09-15 10:03 ` Dylan Jhong
2022-09-15 10:09 ` Cyril Hrubis
2022-09-15 10:32 ` Dylan Jhong
2022-09-15 11:06 ` Cyril Hrubis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.