diff mbox series

[RFC] eal: increase the number of availble file descriptors for MP

Message ID 20240308185401.150651-1-stephen@networkplumber.org (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series [RFC] eal: increase the number of availble file descriptors for MP |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation fail ninja build failure
ci/Intel-compilation fail Compilation issues
ci/github-robot: build fail github build: failed
ci/iol-testing fail build patch failure

Commit Message

Stephen Hemminger March 8, 2024, 6:54 p.m. UTC 
The current limit of file descriptors is too low, it should have
been set to the maximum possible to send across an unix domain
socket.

This is an attempt to allow increasing it without breaking ABI.
But in the process it exposes what is broken about how symbol
versions are checked in check-symbol-maps.sh. That script is
broken in that it won't allow adding a backwards compatiable
version hook like this.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_proc.c | 118 ++++++++++++++++++++++++++-----
 lib/eal/common/meson.build       |   2 +
 lib/eal/include/rte_eal.h        |  13 +++-
 lib/eal/version.map              |   9 +++
 4 files changed, 125 insertions(+), 17 deletions(-)

Comments

David Marchand March 14, 2024, 2:40 p.m. UTC | #1 
On Fri, Mar 8, 2024 at 7:54 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The current limit of file descriptors is too low, it should have
> been set to the maximum possible to send across an unix domain
> socket.
>
> This is an attempt to allow increasing it without breaking ABI.
> But in the process it exposes what is broken about how symbol
> versions are checked in check-symbol-maps.sh. That script is
> broken in that it won't allow adding a backwards compatiable
> version hook like this.

- It could be enhanced maybe, but I see no problem with the script.

The versions for compat symbols in this patch are wrong.
We want to keep compat with ABI 24, not 23.
And next ABI will be 25.

- rte_mp_old_msg does not have to be exported as public in rte_eal.h.


- I think the patch is not complete:
 * rte_mp_action_register and rte_mp_request_async need versioning too,
 * because of the former point, handling of msg requests probably
needs to keep track of accepted length per registered callbacks,
diff mbox series

Patch

diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index d24093937c1d..c08113a8d9e0 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -27,6 +27,7 @@ 
 #include <rte_lcore.h>
 #include <rte_log.h>
 #include <rte_thread.h>
+#include <rte_function_versioning.h>
 
 #include "eal_memcfg.h"
 #include "eal_private.h"
@@ -796,7 +797,7 @@  mp_send(struct rte_mp_msg *msg, const char *peer, int type)
 }
 
 static int
-check_input(const struct rte_mp_msg *msg)
+check_input(const struct rte_mp_msg *msg, int max_fd)
 {
 	if (msg == NULL) {
 		EAL_LOG(ERR, "Msg cannot be NULL");
@@ -825,9 +826,8 @@  check_input(const struct rte_mp_msg *msg)
 		return -1;
 	}
 
-	if (msg->num_fds > RTE_MP_MAX_FD_NUM) {
-		EAL_LOG(ERR, "Cannot send more than %d FDs",
-			RTE_MP_MAX_FD_NUM);
+	if (msg->num_fds > max_fd) {
+		EAL_LOG(ERR, "Cannot send more than %d FDs", max_fd);
 		rte_errno = E2BIG;
 		return -1;
 	}
@@ -835,13 +835,13 @@  check_input(const struct rte_mp_msg *msg)
 	return 0;
 }
 
-int
-rte_mp_sendmsg(struct rte_mp_msg *msg)
+static int
+mp_sendmsg(struct rte_mp_msg *msg, int max_fd)
 {
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
 
-	if (check_input(msg) != 0)
+	if (check_input(msg, max_fd) != 0)
 		return -1;
 
 	if (internal_conf->no_shconf) {
@@ -854,6 +854,24 @@  rte_mp_sendmsg(struct rte_mp_msg *msg)
 	return mp_send(msg, NULL, MP_MSG);
 }
 
+int rte_mp_sendmsg_V23(struct rte_mp_old_msg *msg);
+int rte_mp_sendmsg_V24(struct rte_mp_msg *msg);
+
+int
+rte_mp_sendmsg_V23(struct rte_mp_old_msg *omsg)
+{
+	return mp_sendmsg((struct rte_mp_msg *)omsg, RTE_MP_MAX_OLD_FD_NUM);
+}
+VERSION_SYMBOL(rte_mp_sendmsg, _V23, 23);
+
+int
+rte_mp_sendmsg_V24(struct rte_mp_msg *msg)
+{
+	return mp_sendmsg(msg, RTE_MP_MAX_FD_NUM);
+}
+BIND_DEFAULT_SYMBOL(rte_mp_sendmsg, _V24, 24);
+MAP_STATIC_SYMBOL(int rte_mp_sendmsg(struct rte_mp_msg *msg), rte_mp_sendmsg_V24);
+
 static int
 mp_request_async(const char *dst, struct rte_mp_msg *req,
 		struct async_request_param *param, const struct timespec *ts)
@@ -988,9 +1006,9 @@  mp_request_sync(const char *dst, struct rte_mp_msg *req,
 	return 0;
 }
 
-int
-rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
-		const struct timespec *ts)
+static int
+__rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+		      const struct timespec *ts, int max_fd)
 {
 	int dir_fd, ret = -1;
 	DIR *mp_dir;
@@ -1005,7 +1023,7 @@  rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
 	reply->nb_received = 0;
 	reply->msgs = NULL;
 
-	if (check_input(req) != 0)
+	if (check_input(req, max_fd) != 0)
 		goto end;
 
 	if (internal_conf->no_shconf) {
@@ -1085,9 +1103,34 @@  rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
 	return ret;
 }
 
+int rte_mp_request_sync_V23(struct rte_mp_old_msg *req, struct rte_mp_reply *reply,
+			    const struct timespec *ts);
+int rte_mp_request_sync_V24(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+			    const struct timespec *ts);
+
+
+int
+rte_mp_request_sync_V23(struct rte_mp_old_msg *req, struct rte_mp_reply *reply,
+		    const struct timespec *ts)
+{
+	return __rte_mp_request_sync((struct rte_mp_msg *)req, reply, ts, RTE_MP_MAX_OLD_FD_NUM);
+}
+VERSION_SYMBOL(rte_mp_request_sync, _V23, 23);
+
 int
-rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
-		rte_mp_async_reply_t clb)
+rte_mp_request_sync_V24(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+			const struct timespec *ts)
+{
+	return __rte_mp_request_sync(req, reply, ts, RTE_MP_MAX_FD_NUM);
+}
+BIND_DEFAULT_SYMBOL(rte_mp_request_sync, _V24, 24);
+MAP_STATIC_SYMBOL(int rte_mp_request_sync(struct rte_mp_msg *req, \
+					  struct rte_mp_reply *reply, \
+					  const struct timespec *ts), rte_mp_request_sync_V24);
+
+static int
+__rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
+		       rte_mp_async_reply_t clb, int max_fd)
 {
 	struct rte_mp_msg *copy;
 	struct pending_request *dummy;
@@ -1104,7 +1147,7 @@  rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
 
 	EAL_LOG(DEBUG, "request: %s", req->name);
 
-	if (check_input(req) != 0)
+	if (check_input(req, max_fd) != 0)
 		return -1;
 
 	if (internal_conf->no_shconf) {
@@ -1237,14 +1280,38 @@  rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
 	return -1;
 }
 
+int rte_mp_request_async_V23(struct rte_mp_old_msg *req, const struct timespec *ts,
+			     rte_mp_async_reply_t clb);
+int rte_mp_request_async_V24(struct rte_mp_msg *req, const struct timespec *ts,
+			     rte_mp_async_reply_t clb);
+
 int
-rte_mp_reply(struct rte_mp_msg *msg, const char *peer)
+rte_mp_request_async_V23(struct rte_mp_old_msg *req, const struct timespec *ts,
+			 rte_mp_async_reply_t clb)
+{
+	return __rte_mp_request_async((struct rte_mp_msg *)req, ts, clb, RTE_MP_MAX_OLD_FD_NUM);
+}
+VERSION_SYMBOL(rte_mp_request_async, _V23, 23);
+
+int
+rte_mp_request_async_V24(struct rte_mp_msg *req, const struct timespec *ts,
+			 rte_mp_async_reply_t clb)
+{
+	return __rte_mp_request_async(req, ts, clb, RTE_MP_MAX_FD_NUM);
+}
+BIND_DEFAULT_SYMBOL(rte_mp_request_async, _V24, 24);
+MAP_STATIC_SYMBOL(int rte_mp_request_async(struct rte_mp_msg *req,	\
+					   const struct timespec *ts,	\
+					   rte_mp_async_reply_t clb), rte_mp_request_async_V24);
+
+static int
+mp_reply(struct rte_mp_msg *msg, const char *peer, int max_fd)
 {
 	EAL_LOG(DEBUG, "reply: %s", msg->name);
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
 
-	if (check_input(msg) != 0)
+	if (check_input(msg, max_fd) != 0)
 		return -1;
 
 	if (peer == NULL) {
@@ -1261,6 +1328,25 @@  rte_mp_reply(struct rte_mp_msg *msg, const char *peer)
 	return mp_send(msg, peer, MP_REP);
 }
 
+int rte_mp_reply_V23(struct rte_mp_old_msg *msg, const char *peer);
+int rte_mp_reply_V24(struct rte_mp_msg *msg, const char *peer);
+
+int
+rte_mp_reply_V23(struct rte_mp_old_msg *msg, const char *peer)
+{
+	return mp_reply((struct rte_mp_msg *)msg, peer, RTE_MP_MAX_OLD_FD_NUM);
+}
+VERSION_SYMBOL(rte_mp_reply, _V23, 23);
+
+int
+rte_mp_reply_V24(struct rte_mp_msg *msg, const char *peer)
+{
+	return mp_reply(msg, peer, RTE_MP_MAX_FD_NUM);
+}
+BIND_DEFAULT_SYMBOL(rte_mp_reply, _V24, 24);
+MAP_STATIC_SYMBOL(int rte_mp_reply(struct rte_mp_msg *msg, const char *peer), rte_mp_reply_V24);
+
+
 /* Internally, the status of the mp feature is represented as a three-state:
  * - "unknown" as long as no secondary process attached to a primary process
  *   and there was no call to rte_mp_disable yet,
diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
index 22a626ba6fc7..3faf0c20e798 100644
--- a/lib/eal/common/meson.build
+++ b/lib/eal/common/meson.build
@@ -3,6 +3,8 @@ 
 
 includes += include_directories('.')
 
+use_function_versioning = true
+
 cflags += [ '-DABI_VERSION="@0@"'.format(abi_version) ]
 
 sources += files(
diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index c2256f832e51..0d0761c50409 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -155,9 +155,10 @@  int rte_eal_primary_proc_alive(const char *config_file_path);
  */
 bool rte_mp_disable(void);
 
-#define RTE_MP_MAX_FD_NUM	8    /* The max amount of fds */
+#define RTE_MP_MAX_FD_NUM	253  /* The max number of fds (SCM_MAX_FD) */
 #define RTE_MP_MAX_NAME_LEN	64   /* The max length of action name */
 #define RTE_MP_MAX_PARAM_LEN	256  /* The max length of param */
+
 struct rte_mp_msg {
 	char name[RTE_MP_MAX_NAME_LEN];
 	int len_param;
@@ -166,6 +167,16 @@  struct rte_mp_msg {
 	int fds[RTE_MP_MAX_FD_NUM];
 };
 
+/* Legacy API version */
+#define RTE_MP_MAX_OLD_FD_NUM	8    /* The legacy limit on fds */
+struct rte_mp_old_msg {
+	char name[RTE_MP_MAX_NAME_LEN];
+	int len_param;
+	int num_fds;
+	uint8_t param[RTE_MP_MAX_PARAM_LEN];
+	int fds[RTE_MP_MAX_OLD_FD_NUM];
+};
+
 struct rte_mp_reply {
 	int nb_sent;
 	int nb_received;
diff --git a/lib/eal/version.map b/lib/eal/version.map
index c06ceaad5097..264ff2d0818b 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -344,6 +344,15 @@  DPDK_24 {
 	local: *;
 };
 
+DPDK_23 {
+	global:
+
+	rte_mp_reply;
+	rte_mp_request_async;
+	rte_mp_request_sync;
+	rte_mp_sendmsg;
+} DPDK_24;
+
 EXPERIMENTAL {
 	global: