From 1ad3b040baa7146246b53f0b5095c6e111acc761 Mon Sep 17 00:00:00 2001 From: Devansh-567 Date: Sun, 18 Oct 2026 16:15:19 +0530 Subject: [PATCH] Fix leak of serialized p4info in RPC server state sync Signed-off-by: Devansh-567 --- src/pi_rpc_server.c | 3 + tests/Makefile.am | 17 +++- tests/main.c | 4 + tests/test_rpc_server.c | 173 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 tests/test_rpc_server.c diff --git a/src/pi_rpc_server.c b/src/pi_rpc_server.c index eb89a0e3..2f7acf6d 100644 --- a/src/pi_rpc_server.c +++ b/src/pi_rpc_server.c @@ -188,6 +188,9 @@ static void __pi_init(char *req) { if (num_devices > 0) { assert(p4info_tmp); + for (size_t idx = 0; idx < num_devices; idx++) { + pi_free_serialized_config(p4info_tmp[idx].json); + } free(p4info_tmp); } diff --git a/tests/Makefile.am b/tests/Makefile.am index a873446d..c1eff5c8 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -24,7 +24,8 @@ test_bmv2_json_reader \ test_read_file \ test_getnetv \ test_p4info \ -test_frontends_generic +test_frontends_generic \ +test_rpc_server common_source = main.c utils.c utils.h @@ -43,6 +44,19 @@ test_p4info_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_P4INFO test_frontends_generic_SOURCES = $(common_source) frontends/generic/test.c test_frontends_generic_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_FRONTENDS_GENERIC +test_rpc_server_SOURCES = $(common_source) test_rpc_server.c +test_rpc_server_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_RPC_SERVER +# this test starts the RPC server, which needs nanomsg +test_rpc_server_LDADD = \ +$(top_builddir)/src/libpi.la \ +$(top_builddir)/src/libpifegeneric.la \ +$(top_builddir)/targets/dummy/libpi_dummy.la \ +$(top_builddir)/src/libpip4info.la \ +$(top_builddir)/third_party/unity/libunity.la \ +$(top_builddir)/third_party/cJSON/libpicjson.la \ +$(top_builddir)/lib/libpitoolkit.la \ +-lnanomsg -lpthread + test_all_SOURCES = $(common_source) \ test_bmv2_json_reader.c \ test_read_file.c \ @@ -72,6 +86,7 @@ test_read_file \ test_getnetv \ test_p4info \ test_frontends_generic \ +test_rpc_server \ test_all EXTRA_DIST = \ diff --git a/tests/main.c b/tests/main.c index 02722837..e68ea984 100644 --- a/tests/main.c +++ b/tests/main.c @@ -30,6 +30,7 @@ extern void test_read_file(); extern void test_getnetv(); extern void test_p4info(); extern void test_frontends_generic(); +extern void test_rpc_server(); static void run() { #ifdef TEST_BMV2_JSON_READER @@ -47,6 +48,9 @@ static void run() { #ifdef TEST_FRONTENDS_GENERIC test_frontends_generic(); #endif +#ifdef TEST_RPC_SERVER + test_rpc_server(); +#endif } int main(int argc, const char *argv[]) { diff --git a/tests/test_rpc_server.c b/tests/test_rpc_server.c new file mode 100644 index 00000000..a8034b83 --- /dev/null +++ b/tests/test_rpc_server.c @@ -0,0 +1,173 @@ +/* SPDX-FileCopyrightText: 2026 The P4 Language Consortium & Devansh Singh + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "PI/int/rpc_common.h" +#include "PI/int/serialize.h" +#include "PI/pi.h" + +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "unity/unity_fixture.h" + +// pi_rpc_server_run usually runs in the pi_rpc_server process, we run it in a +// thread and talk to it with a raw nanomsg client + +extern pi_status_t pi_rpc_server_run(const pi_remote_addr_t *remote_addr); + +// the smallest config accepted by the native JSON reader +static const char *native_json_config = + "{" + " \"actions\": [" + " {\"name\": \"noop\", \"id\": 16777217, \"params\": []}" + " ]," + " \"tables\": [{" + " \"name\": \"t\"," + " \"id\": 33554433," + " \"match_fields\": []," + " \"actions\": [{\"id\": 16777217, \"scope\": 0}]," + " \"max_size\": 1024," + " \"is_const\": false" + " }]," + " \"act_profs\": []," + " \"counters\": []," + " \"direct_counters\": []," + " \"meters\": []," + " \"direct_meters\": []," + " \"digests\": []" + "}"; + +static char rpc_addr[256]; +static char *ipc_path; + +static void *server_run(void *arg) { + pi_rpc_server_run((const pi_remote_addr_t *)arg); + return NULL; +} + +static void send_request(int s, uint32_t req_id, pi_rpc_type_t type, + const char *body, size_t body_size) { + size_t s_ = sizeof(req_hdr_t) + body_size; + char *req = nn_allocmsg(s_, 0); + assert(req); + char *req_ = req; + req_ += emit_uint32(req_, req_id); + req_ += emit_uint32(req_, type); + if (body_size > 0) memcpy(req_, body, body_size); + int rc = nn_send(s, &req, NN_MSG, 0); + assert((size_t)rc == s_); +} + +static char *recv_reply(int s, uint32_t exp_req_id, pi_status_t *status) { + char *rep = NULL; + int bytes = nn_recv(s, &rep, NN_MSG, 0); + assert(bytes > 0); + const char *rep_ = rep; + uint32_t req_id; + uint32_t status_; + rep_ += retrieve_uint32(rep_, &req_id); + rep_ += retrieve_uint32(rep_, &status_); + assert(req_id == exp_req_id); + *status = (pi_status_t)status_; + return rep; +} + +TEST_GROUP(RpcServer); + +TEST_SETUP(RpcServer) { + snprintf(rpc_addr, sizeof(rpc_addr), "ipc:///tmp/pi_rpc_server_test_%d.ipc", + (int)getpid()); + ipc_path = rpc_addr + strlen("ipc://"); +} + +TEST_TEAR_DOWN(RpcServer) { unlink(ipc_path); } + +TEST(RpcServer, StateSync) { + TEST_ASSERT_EQUAL(PI_STATUS_SUCCESS, pi_init(256, NULL)); + + pi_remote_addr_t server_addr = {rpc_addr, NULL}; + // the server loop cannot be stopped, the thread stays blocked in nn_recv + // until the process exits + pthread_t server_thread; + int rc = pthread_create(&server_thread, NULL, server_run, &server_addr); + TEST_ASSERT_EQUAL(0, rc); + usleep(100000); + + int s = nn_socket(AF_SP, NN_REQ); + TEST_ASSERT_TRUE(s >= 0); + TEST_ASSERT_TRUE(nn_connect(s, rpc_addr) >= 0); + + pi_status_t status; + char *rep; + + // no device assigned yet + send_request(s, 0, PI_RPC_INIT, NULL, 0); + rep = recv_reply(s, 0, &status); + TEST_ASSERT_EQUAL(PI_STATUS_SUCCESS, status); + uint32_t num_devices; + retrieve_uint32(rep + sizeof(rep_hdr_t), &num_devices); + nn_freemsg(rep); + TEST_ASSERT_EQUAL_UINT32(0u, num_devices); + + // assign one device with a valid config + char req_body[8 + 1024]; + char *req_body_ = req_body; + req_body_ += emit_dev_id(req_body_, 1u); + memcpy(req_body_, native_json_config, strlen(native_json_config) + 1); + req_body_ += strlen(native_json_config) + 1; + req_body_ += emit_uint32(req_body_, 0u); // no extras + send_request(s, 1, PI_RPC_ASSIGN_DEVICE, req_body, req_body_ - req_body); + rep = recv_reply(s, 1, &status); + nn_freemsg(rep); + TEST_ASSERT_EQUAL(PI_STATUS_SUCCESS, status); + + // a new client gets the current state, with the p4info of every device + send_request(s, 2, PI_RPC_INIT, NULL, 0); + rep = recv_reply(s, 2, &status); + TEST_ASSERT_EQUAL(PI_STATUS_SUCCESS, status); + const char *rep_ = rep + sizeof(rep_hdr_t); + rep_ += retrieve_uint32(rep_, &num_devices); + TEST_ASSERT_EQUAL_UINT32(1u, num_devices); + pi_dev_id_t dev_id; + uint32_t version; + rep_ += retrieve_dev_id(rep_, &dev_id); + rep_ += retrieve_uint32(rep_, &version); + (void)version; + TEST_ASSERT_EQUAL(1, (int)dev_id); + const char *json = rep_; + pi_p4info_t *p4info = NULL; + // the client parses the config of every device with the native JSON reader + TEST_ASSERT_EQUAL(PI_STATUS_SUCCESS, + pi_add_config(json, PI_CONFIG_TYPE_NATIVE_JSON, &p4info)); + TEST_ASSERT_EQUAL(PI_STATUS_SUCCESS, pi_destroy_config(p4info)); + nn_freemsg(rep); + + nn_close(s); + TEST_ASSERT_EQUAL(PI_STATUS_SUCCESS, pi_destroy()); +} + +TEST_GROUP_RUNNER(RpcServer) { + RUN_TEST_CASE(RpcServer, StateSync); +} + +void test_rpc_server() { RUN_TEST_GROUP(RpcServer); }