TA events implementaion by okt-osadakov · Pull Request #56 · oktetlabs/test-environment

okt-osadakov · 2024-10-21T14:35:40Z

This PR is used to add asynchronous TA events. Such events can be processed inside any RCF client (e.g: TEST). RCF command can be interrupted based on the process result.

List of TAPI functions added in this PR:

tapi_ta_events_subscribe: register callback to handle specified TA events from exact TA;
tapi_ta_events_unsubscribe: unregister one of the registered handlers.

Additionally we have a function to trigger TA event:

rcf_ta_events_trigger_event: this function is useful to debug purpose.

Testing done:

../test-environment/suites/selftest/run.sh \
    --cfg=localhost \
    --tester-run=ts/tapi/ta_events
...
Platform linux build - pass
Simple RCF consistency check succeeded
--->>> Starting Logger...done
--->>> Starting RCF...done
--->>> Starting Configurator...done
--->>> Start Tester
Starting package ts
Starting test prologue                    pass
Starting package tapi
Starting package ta_events
Starting test sample                      pass
Starting test trigger                     pass
Starting test trigger                     pass
Done package ta_events pass
Done package tapi pass
Done package ts pass
--->>> Shutdown Configurator...done
--->>> Flush Logs
--->>> Shutdown RCF...done
--->>> Shutdown Logger...done
--->>> Logs conversion...done

Run (total)                               4
  Passed, as expected                     3
  Failed, as expected                     0
  Passed unexpectedly                     1
  Failed unexpectedly                     0
  Aborted (no useful result)              0
  New (expected result is not known)      0
Not Run (total)                         154
  Skipped, as expected                    0
  Skipped unexpectedly                    0

rcf_free_usrreq is a pair function for rcf_alloc_usrreq. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

TA event is used to notify TEST or any RCF client about unpredictable event on TA side. tapi_ta_events_cb is used to handle these events. Based on the result, we can continue the client's work or terminate it. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

TE_RCF should have an information about RCF clients that subscribe to TA events to be able to forward TA events. These messages are used to notify TE_RCF about these RCF clients. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

TE_RCF should have a table with unique RCF clients to be able to forward TA events to suitable clients. Without this table we will send TA events to all RCF clients. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

…f interest Configuration tree is used to restore this information after agent restart. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

This function is useful to debug TA events or raise TA event from test. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

Here we add TA events messages with common prefix "SID ${RCF_TA_EVENTS_SID} " between TE_RCF and TA: - trigger event command: "ta_events trigger_event ${event_name} ${value}" - trigger event reply: "${rc} trigger_event" Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

Here we add TA events messages with common prefix "SID ${RCF_TA_EVENTS_SID} " between TE_RCF and TA: - event reply: "${rc} event ${rcf_clients} ${event_name} ${value}" Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

reply from TA with TA event contains list of RCF clients. This patch is used to split this list into separate RCF clients. And configure RCF message to forward TA event to exact TEST or RCF client. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

Here we add to RCF API additional handler for TA event RCF messages (RCF messages with with TA events opcode and special EVENT type). This handler returns status code and non-zero status code causes termination the related command. Note: rcf_trpoll contains more complex logic and cannot be completed instantly. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

This test is used to validate random order of subscribe/unsubscribe operations. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

This test contains two independent TA events: - good: that shouldn't stop RPC call; - bad: that must stop RPC call. Additionally here we check the number of TA events handled in this test. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

ol-andrewr · 2024-10-25T11:28:47Z

+#endif
+
+/** Event callback to process TA events. */
+typedef bool (*tapi_ta_events_cb)(const char *ta, char *name, char *value);


Please, define it without * before type name. It would allow to use it as:
static tapi_ta_events_cb my_func;
static bool
my_func(const char *ta, char *name, char *value)

to highlight that function matches the proto and fail build proto changes and current instance it lost.

ol-andrewr · 2024-10-25T11:29:12Z

+typedef bool (*tapi_ta_events_cb)(const char *ta, char *name, char *value);
+
+/** TA events handle */
+typedef unsigned tapi_ta_events_handle;


No plain unsigned, please, 'unsigned int'

ol-andrewr · 2024-10-25T11:30:38Z

+ * Register new handler, that will be executed to process TA events.
+ *
+ * @param ta         Agent from which we want to receive TA events
+ * @param events     Comma separated list of events to receive


I dislike comma-separated strings. What is the motivation to use such format? Can we use an array or vector instead?

ol-andrewr · 2024-10-25T11:32:01Z

+/**
+ * Register new handler, that will be executed to process TA events.
+ *
+ * @param ta         Agent from which we want to receive TA events


If I'm not mistaken coding standard requires full stop here if parameter description starts from capital letter.
Similar cases below.

ol-andrewr · 2024-10-25T11:33:53Z

+extern te_errno tapi_ta_events_subscribe(const char *ta, const char *events,
+                                         tapi_ta_events_cb      callback,
+                                         tapi_ta_events_handle *handle);


alignment of parameters does not look consistent. It should be either intended for each parameter or no indent at all.

ol-andrewr · 2024-10-25T11:58:38Z

 #include <libxml/xmlmemory.h>
 #include <libxml/parser.h>
 #include <libxml/xinclude.h>



I don't understand what is TE_RCF in the patch summary. IMHO, summary should look like:

engine/rcf: register client for TA events

ol-andrewr · 2024-10-25T12:15:19Z

+/** RCF client to receive TA events */
+typedef struct ta_events_client {
+    int     refcnt; /**< Number of active TA events patterns for this client */
+    char   *name;   /**< Unique name for this client (${pid}_${tid}) */


Can't ipc_server_client to the job?

ol-andrewr · 2024-10-25T12:16:35Z

 #include "cs_common.h"
 #include "logger_api.h"
 #include "comm_agent.h"
 #include "rcf_ch_api.h"


summary is obviously too long

ol-andrewr · 2024-10-25T12:18:20Z

+
+    buf[0] = '\0';
+
+    SLIST_FOREACH (param, &ta_events_params, links)


unnecessary space after SLIST_FOREACH

ol-andrewr · 2024-10-25T12:22:21Z

         is set, current value is adjusted too.
         Name: empty
+
+    - oid: "/agent/ta_events"


IMHO it is weird. RCF should care about it itself. It restarts the agent and it may subscribe to events using RCF protocol just after restart. Usage of CS here just makes the picture more and more complicated.
If you which to expose the information (make it clear which events are subscribed to), you may do it (read-only volatile).

ol-arteman reviewed Oct 21, 2024

View reviewed changes

Comment thread lib/tapi_ta_events/tapi_ta_events.c Outdated

okt-osadakov force-pushed the user/ark-oleg/ta_events branch 3 times, most recently from a54a613 to c22696a Compare October 21, 2024 18:09

okt-osadakov added 13 commits October 22, 2024 16:10

engine/rcf: implement function to release allocated user request

c811c70

rcf_free_usrreq is a pair function for rcf_alloc_usrreq. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

engine/rcf: register RCF client for TA events inside TE_RCF

85464be

TE_RCF should have a table with unique RCF clients to be able to forward TA events to suitable clients. Without this table we will send TA events to all RCF clients. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

lib/rcfpch: add /agent/ta_events node with information about events o…

ef2b288

…f interest Configuration tree is used to restore this information after agent restart. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

lib/tapi_ta_events: add TA events configuration tree

1367be6

Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

lib/rcfapi: add RCF function to trigger TA event

ac1c31e

This function is useful to debug TA events or raise TA event from test. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

lib/rcfpch: support TA event generation

45e212c

Here we add TA events messages with common prefix "SID ${RCF_TA_EVENTS_SID} " between TE_RCF and TA: - event reply: "${rc} event ${rcf_clients} ${event_name} ${value}" Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

selftest: add new trivial test to validate TAPI for TA events

9674b6a

This test is used to validate random order of subscribe/unsubscribe operations. Signed-off-by: Oleg Sadakov <Oleg.Sadakov@arknetworks.am>

okt-osadakov force-pushed the user/ark-oleg/ta_events branch from c22696a to 2af1aae Compare October 22, 2024 12:11

ol-andrewr requested changes Oct 25, 2024

View reviewed changes

ol-andrewr self-assigned this Dec 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TA events implementaion#56

TA events implementaion#56
okt-osadakov wants to merge 13 commits into
oktetlabs:mainfrom
okt-osadakov:user/ark-oleg/ta_events

okt-osadakov commented Oct 21, 2024 •

edited

Loading

Uh oh!

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

ol-andrewr Oct 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		buf[0] = '\0';

		SLIST_FOREACH (param, &ta_events_params, links)

Conversation

okt-osadakov commented Oct 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

okt-osadakov commented Oct 21, 2024 •

edited

Loading