mgmtd: don't use libyang definitions in FE messages

mgmtd frontend is a public API so it's a bad practice to use some external library definitions in message fields. Replace them with our own definitions and translate them into internal ones during message processing. Also, the frontend must always return the response in the format specified in the request. Remove unnecessary response format checks. Signed-off-by: Igor Ryzhov <[email protected]>
idryzhov · Jan 28, 2024 · 20afefd · 20afefd
1 parent f576e2d
commit 20afefd
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 55 deletions.
diff --git a/lib/mgmt_fe_client.c b/lib/mgmt_fe_client.c
@@ -308,10 +308,9 @@ int mgmt_fe_send_regnotify_req(struct mgmt_fe_client *client,
 /*
  * Send get-data request.
  */
-int mgmt_fe_send_get_data_req(struct mgmt_fe_client *client,
-			      uint64_t session_id, uint64_t req_id,
-			      uint8_t datastore, LYD_FORMAT result_type,
-			      uint8_t flags, const char *xpath)
+int mgmt_fe_send_get_data_req(struct mgmt_fe_client *client, uint64_t session_id,
+			      uint64_t req_id, uint8_t datastore,
+			      uint8_t format, uint8_t flags, const char *xpath)
 {
 	struct mgmt_msg_get_data *msg;
 	size_t xplen = strlen(xpath);
@@ -322,7 +321,7 @@ int mgmt_fe_send_get_data_req(struct mgmt_fe_client *client,
 	msg->refer_id = session_id;
 	msg->req_id = req_id;
 	msg->code = MGMT_MSG_CODE_GET_DATA;
-	msg->result_type = result_type;
+	msg->format = format;
 	msg->flags = flags;
 	msg->datastore = datastore;
 	strlcpy(msg->xpath, xpath, xplen + 1);

diff --git a/lib/mgmt_fe_client.h b/lib/mgmt_fe_client.h
@@ -382,8 +382,8 @@ extern int mgmt_fe_send_regnotify_req(struct mgmt_fe_client *client,
  * datastore
  *    Datastore for getting data.
  *
- * result_type
- *    The LYD_FORMAT of the result.
+ * format
+ *    The format of the result.
  *
  * flags
  *    Flags to control the behavior of the request.
@@ -396,7 +396,7 @@ extern int mgmt_fe_send_regnotify_req(struct mgmt_fe_client *client,
  */
 extern int mgmt_fe_send_get_data_req(struct mgmt_fe_client *client,
 				     uint64_t session_id, uint64_t req_id,
-				     uint8_t datastore, LYD_FORMAT result_type,
+				     uint8_t datastore, uint8_t format,
 				     uint8_t flags, const char *xpath);
 
 /*

diff --git a/lib/mgmt_msg_native.h b/lib/mgmt_msg_native.h
@@ -160,6 +160,13 @@ DECLARE_MTYPE(MSG_NATIVE_GET_DATA);
 #define MGMT_MSG_DATASTORE_RUNNING     2
 #define MGMT_MSG_DATASTORE_OPERATIONAL 3
 
+/*
+ * Formats
+ */
+#define MGMT_MSG_FORMAT_XML    0
+#define MGMT_MSG_FORMAT_JSON   1
+#define MGMT_MSG_FORMAT_BINARY 2 /* non-standard libyang internal format */
+
 /**
  * struct mgmt_msg_header - Header common to all native messages.
  *
@@ -223,7 +230,7 @@ _Static_assert(sizeof(struct mgmt_msg_get_tree) ==
  * struct mgmt_msg_tree_data - Message carrying tree data.
  *
  * @partial_error: If the full result could not be returned do to this error.
- * @result_type: ``LYD_FORMAT`` for format of the @result value.
+ * @result_type: ``LYD_FORMAT`` for format of the @result value. Not used on frontend.
  * @more: if this is a partial return and there will be more coming.
  * @result: The tree data in @result_type format.
  *
@@ -262,13 +269,13 @@ _Static_assert(sizeof(struct mgmt_msg_tree_data) ==
 /**
  * struct mgmt_msg_get_data - frontend get-data request.
  *
- * @result_type: ``LYD_FORMAT`` for the returned result.
+ * @format: format of the returned result.
  * @flags: combination of ``GET_DATA_FLAG_*`` flags.
  * @xpath: the query for the data to return.
  */
 struct mgmt_msg_get_data {
 	struct mgmt_msg_header;
-	uint8_t result_type;
+	uint8_t format;
 	uint8_t flags;
 	uint8_t datastore;
 	uint8_t resv2[5];

diff --git a/lib/vty.c b/lib/vty.c
@@ -3673,6 +3673,7 @@ static int vty_mgmt_get_data_result_notified(
 	return 0;
 }
 
+/*
 static ssize_t vty_mgmt_libyang_print(void *user_data, const void *buf,
 				      size_t count)
 {
@@ -3785,7 +3786,7 @@ static uint vty_out_yang_errors(struct vty *vty, LYD_FORMAT format)
 
 	return count;
 }
-
+*/
 
 static int vty_mgmt_get_tree_result_notified(
 	struct mgmt_fe_client *client, uintptr_t user_data, uint64_t client_id,
@@ -3794,9 +3795,6 @@ static int vty_mgmt_get_tree_result_notified(
 	size_t len, int partial_error)
 {
 	struct vty *vty;
-	struct lyd_node *dnode;
-	int ret = CMD_SUCCESS;
-	LY_ERR err;
 
 	vty = (struct vty *)session_ctx;
 
@@ -3805,37 +3803,13 @@ static int vty_mgmt_get_tree_result_notified(
 			    partial_error ? "partially " : "", client_id,
 			    req_id);
 
-	assert(result_type == LYD_LYB ||
-	       result_type == vty->mgmt_req_pending_data);
-
-	if (vty->mgmt_req_pending_data == LYD_XML && partial_error)
+	if (vty->mgmt_req_pending_data == MGMT_MSG_FORMAT_XML && partial_error)
 		vty_out(vty,
 			"<!-- some errors occurred gathering results -->\n");
 
-	if (result_type == LYD_LYB) {
-		/*
-		 * parse binary into tree and print in the specified format
-		 */
-		result_type = vty->mgmt_req_pending_data;
-
-		err = lyd_parse_data_mem(ly_native_ctx, result, LYD_LYB, 0, 0,
-					 &dnode);
-		if (!err)
-			err = lyd_print_clb(vty_mgmt_libyang_print, vty, dnode,
-					    result_type, LYD_PRINT_WITHSIBLINGS);
-		lyd_free_all(dnode);
+	vty_out(vty, "%.*s\n", (int)len - 1, (const char *)result);
 
-		if (vty_out_yang_errors(vty, result_type) || err)
-			ret = CMD_WARNING;
-	} else {
-		/*
-		 * Print the in-format result
-		 */
-		assert(result_type == LYD_XML || result_type == LYD_JSON);
-		vty_out(vty, "%.*s\n", (int)len - 1, (const char *)result);
-	}
-
-	vty_mgmt_resume_response(vty, ret);
+	vty_mgmt_resume_response(vty, CMD_SUCCESS);
 
 	return 0;
 }
@@ -4114,16 +4088,13 @@ int vty_mgmt_send_get_req(struct vty *vty, bool is_config,
 }
 
 int vty_mgmt_send_get_data_req(struct vty *vty, uint8_t datastore,
-			       LYD_FORMAT result_type, uint8_t flags,
-			       const char *xpath)
+			       uint8_t format, uint8_t flags, const char *xpath)
 {
-	LYD_FORMAT intern_format = result_type;
-
 	vty->mgmt_req_id++;
 
 	if (mgmt_fe_send_get_data_req(mgmt_fe_client, vty->mgmt_session_id,
-				      vty->mgmt_req_id, datastore,
-				      intern_format, flags, xpath)) {
+				      vty->mgmt_req_id, datastore, format,
+				      flags, xpath)) {
 		zlog_err("Failed to send GET-DATA to MGMTD session-id: %" PRIu64
 			 " req-id %" PRIu64 ".",
 			 vty->mgmt_session_id, vty->mgmt_req_id);
@@ -4132,7 +4103,7 @@ int vty_mgmt_send_get_data_req(struct vty *vty, uint8_t datastore,
 	}
 
 	vty->mgmt_req_pending_cmd = "MESSAGE_GET_DATA_REQ";
-	vty->mgmt_req_pending_data = result_type;
+	vty->mgmt_req_pending_data = format;
 
 	return 0;
 }

diff --git a/lib/vty.h b/lib/vty.h
@@ -421,7 +421,7 @@ extern int vty_mgmt_send_get_req(struct vty *vty, bool is_config,
 				 Mgmtd__DatastoreId datastore,
 				 const char **xpath_list, int num_req);
 extern int vty_mgmt_send_get_data_req(struct vty *vty, uint8_t datastore,
-				      LYD_FORMAT result_type, uint8_t flags,
+				      uint8_t format, uint8_t flags,
 				      const char *xpath);
 extern int vty_mgmt_send_lockds_req(struct vty *vty, Mgmtd__DatastoreId ds_id,
 				    bool lock, bool scok);

diff --git a/mgmtd/mgmt_fe_adapter.c b/mgmtd/mgmt_fe_adapter.c
@@ -1095,7 +1095,6 @@ static int fe_adapter_send_tree_data(struct mgmt_fe_session_ctx *session,
 	msg->req_id = req_id;
 	msg->code = MGMT_MSG_CODE_TREE_DATA;
 	msg->partial_error = partial_error;
-	msg->result_type = result_type;
 
 	ret = yang_print_tree_append(&buf, tree, result_type,
 				     (wd_options | LYD_PRINT_WITHSIBLINGS));
@@ -1139,6 +1138,7 @@ static void fe_adapter_handle_get_data(struct mgmt_fe_session_ctx *session,
 	char *xpath_resolved = NULL;
 	uint64_t req_id = msg->req_id;
 	Mgmtd__DatastoreId ds_id;
+	uint8_t result_type;
 	uint64_t clients;
 	bool simple_xpath;
 	LY_ERR err;
@@ -1182,6 +1182,24 @@ static void fe_adapter_handle_get_data(struct mgmt_fe_session_ctx *session,
 		goto done;
 	}
 
+	switch (msg->format) {
+	case MGMT_MSG_FORMAT_XML:
+		result_type = LYD_XML;
+		break;
+	case MGMT_MSG_FORMAT_JSON:
+		result_type = LYD_JSON;
+		break;
+	case MGMT_MSG_FORMAT_BINARY:
+		result_type = LYD_LYB;
+		break;
+	default:
+		fe_adapter_send_error(session, req_id, false, -EINVAL,
+				      "Unsupported format %" PRIu8
+				      " requested from session-id: %" PRIu64,
+				      msg->format, session->session_id);
+		goto done;
+	}
+
 	err = yang_resolve_snode_xpath(ly_native_ctx, msg->xpath, &snodes,
 				       &simple_xpath);
 	if (err) {
@@ -1199,8 +1217,8 @@ static void fe_adapter_handle_get_data(struct mgmt_fe_session_ctx *session,
 				     msg->xpath, session->txn_id,
 				     session->session_id);
 
-		fe_adapter_send_tree_data(session, req_id, false,
-					  msg->result_type, 0, NULL, 0);
+		fe_adapter_send_tree_data(session, req_id, false, result_type,
+					  0, NULL, 0);
 		goto done;
 	}
 
@@ -1219,7 +1237,7 @@ static void fe_adapter_handle_get_data(struct mgmt_fe_session_ctx *session,
 
 	/* Create a GET-TREE request under the transaction */
 	ret = mgmt_txn_send_get_tree_oper(session->txn_id, req_id, clients,
-					  ds_id, msg->result_type, msg->flags,
+					  ds_id, result_type, msg->flags,
 					  simple_xpath, msg->xpath);
 	if (ret) {
 		/* destroy the just created txn */

diff --git a/mgmtd/mgmt_vty.c b/mgmtd/mgmt_vty.c
@@ -275,7 +275,8 @@ DEFPY(show_mgmt_get_data, show_mgmt_get_data_cmd,
       "JSON output format\n"
       "XML output format\n")
 {
-	LYD_FORMAT format = (fmt && fmt[0] == 'x') ? LYD_XML : LYD_JSON;
+	uint8_t format = (fmt && fmt[0] == 'x') ? MGMT_MSG_FORMAT_XML
+						: MGMT_MSG_FORMAT_JSON;
 	int plen = strlen(path);
 	char *xpath = NULL;
 	uint8_t flags = content ? GET_DATA_FLAG_CONFIG : GET_DATA_FLAG_STATE;