From 474ed4064bfc10724448b1326368643d6e53fd27 Mon Sep 17 00:00:00 2001 From: "Kasiewicz, Marek" Date: Mon, 2 Dec 2024 12:49:33 +0000 Subject: [PATCH 1/4] Check if pointer is NULL before dereferencing it Signed-off-by: Kasiewicz, Marek --- lib/src/st2110/st_rx_video_session.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/src/st2110/st_rx_video_session.c b/lib/src/st2110/st_rx_video_session.c index 559a0807..8abf626e 100644 --- a/lib/src/st2110/st_rx_video_session.c +++ b/lib/src/st2110/st_rx_video_session.c @@ -1325,14 +1325,9 @@ static int rv_start_pcap(struct st_rx_video_session_impl* s, enum mtl_session_po err("%s(%d,%d), pcap dump already started\n", __func__, idx, s_port); return -EIO; } + snprintf(pcap->file_name, sizeof(pcap->file_name), "st22rx_s%dp%d_%u_XXXXXX.pcapng", + idx, s_port, max_dump_packets); - if (s->st22_info) { - snprintf(pcap->file_name, sizeof(pcap->file_name), "st22rx_s%dp%d_%u_XXXXXX.pcapng", - idx, s_port, max_dump_packets); - } else { - snprintf(pcap->file_name, sizeof(pcap->file_name), "st22rx_s%dp%d_%u_XXXXXX.pcapng", - idx, s_port, max_dump_packets); - } int fd = mt_mkstemps(pcap->file_name, strlen(".pcapng")); if (fd < 0) { err("%s(%d,%d), failed to create pcap file %s\n", __func__, idx, s_port, @@ -3337,7 +3332,8 @@ static int rv_migrate_dma(struct mtl_main_impl* impl, static void rv_stat(struct st_rx_video_sessions_mgr* mgr, struct st_rx_video_session_impl* s) { - int m_idx = mgr ? mgr->idx : 0, idx = s->idx; + int m_idx = mgr->idx; + int idx = s->idx; uint64_t cur_time_ns = mt_get_monotonic_time(); double time_sec = (double)(cur_time_ns - s->stat_last_time) / NS_PER_S; int frames_received = rte_atomic32_read(&s->stat_frames_received); @@ -3560,6 +3556,8 @@ static int rvs_pkt_rx_tasklet_start(void* priv) { static int rv_detach(struct mtl_main_impl* impl, struct st_rx_video_sessions_mgr* mgr, struct st_rx_video_session_impl* s) { + if (!mgr || !s) + return -EINVAL; s->attached = false; rv_stat(mgr, s); rv_uinit(impl, s); @@ -3776,6 +3774,9 @@ static int rv_sessions_stat(void* priv) { struct st_rx_video_sessions_mgr* mgr = priv; struct st_rx_video_session_impl* s; + if (!mgr) + return -EINVAL; + for (int j = 0; j < mgr->max_idx; j++) { s = rx_video_session_get_timeout(mgr, j, ST_SESSION_STAT_TIMEOUT_US); if (!s) continue; From 2fa5be583139a4c78678354e3a90b75fdbbd9522 Mon Sep 17 00:00:00 2001 From: "Kasiewicz, Marek" Date: Mon, 9 Dec 2024 08:46:48 +0000 Subject: [PATCH 2/4] Fix Improper use of negative value Signed-off-by: Kasiewicz, Marek --- lib/src/datapath/mt_shared_queue.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/datapath/mt_shared_queue.c b/lib/src/datapath/mt_shared_queue.c index e3e0ec45..29805a06 100644 --- a/lib/src/datapath/mt_shared_queue.c +++ b/lib/src/datapath/mt_shared_queue.c @@ -80,10 +80,11 @@ static int rsq_entry_free(struct mt_rsq_entry* entry) { mt_ring_dequeue_clean(entry->ring); rte_ring_free(entry->ring); } - if (entry->mcast_fd) { + if (entry->mcast_fd > 0) close(entry->mcast_fd); - entry->mcast_fd = -1; - } + + entry->mcast_fd = -1; + info("%s(%d), succ on q %u idx %d\n", __func__, rsqm->port, entry->queue_id, entry->idx); mt_rte_free(entry); From d80834cad21099fa67aeb0c56ff8c462dfb498cc Mon Sep 17 00:00:00 2001 From: "Kasiewicz, Marek" Date: Mon, 9 Dec 2024 08:55:43 +0000 Subject: [PATCH 3/4] Handle return val from send_response() and ioctl() Signed-off-by: Kasiewicz, Marek --- app/v4l2_to_ip/v4l2_to_ip.c | 6 +++- manager/mtl_instance.hpp | 64 +++++++++++++++++++++++++++---------- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/app/v4l2_to_ip/v4l2_to_ip.c b/app/v4l2_to_ip/v4l2_to_ip.c index a98a3303..bc5dc932 100644 --- a/app/v4l2_to_ip/v4l2_to_ip.c +++ b/app/v4l2_to_ip/v4l2_to_ip.c @@ -768,7 +768,11 @@ static void video_close(struct device* dev) { } static void video_log_status(struct device* dev) { - ioctl(dev->fd, VIDIOC_LOG_STATUS); + int ret; + ret = ioctl(dev->fd, VIDIOC_LOG_STATUS); + if (ret < 0) { + printf("Failed to log status: %s (%d).\n", strerror(errno), errno); + } } static int video_get_format(struct device* dev) { diff --git a/manager/mtl_instance.hpp b/manager/mtl_instance.hpp index 830860e8..5d9d1795 100644 --- a/manager/mtl_instance.hpp +++ b/manager/mtl_instance.hpp @@ -156,12 +156,16 @@ void mtl_instance::handle_message_get_lcore(mtl_lcore_message_t* lcore_msg) { uint16_t lcore_id = ntohs(lcore_msg->lcore); int ret = mtl_lcore::get_instance().get_lcore(lcore_id); if (ret < 0) { - send_response(ret); + if (send_response(ret) < 0) { + log(log_level::ERROR, "Failed to send response for get_lcore"); + } return; } lcore_ids.insert(lcore_id); log(log_level::INFO, "Added lcore " + std::to_string(lcore_id)); - send_response(0); + if (send_response(0) < 0) { + log(log_level::ERROR, "Failed to send response for get_lcore"); + } } void mtl_instance::handle_message_put_lcore(mtl_lcore_message_t* lcore_msg) { @@ -172,12 +176,16 @@ void mtl_instance::handle_message_put_lcore(mtl_lcore_message_t* lcore_msg) { uint16_t lcore_id = ntohs(lcore_msg->lcore); int ret = mtl_lcore::get_instance().put_lcore(lcore_id); if (ret < 0) { - send_response(ret); + if (send_response(ret) < 0) { + log(log_level::ERROR, "Failed to send response for put_lcore"); + } return; } lcore_ids.erase(lcore_id); log(log_level::INFO, "Removed lcore " + std::to_string(lcore_id)); - send_response(0); + if (send_response(0) < 0) { + log(log_level::ERROR, "Failed to send response for put_lcore"); + } } void mtl_instance::handle_message_register(mtl_register_message_t* register_msg) { @@ -190,7 +198,9 @@ void mtl_instance::handle_message_register(mtl_register_message_t* register_msg) auto interface = get_interface(ifindex); if (interface == nullptr) { log(log_level::ERROR, "Could not get interface " + std::to_string(ifindex)); - send_response(-1); + if (send_response(-1) < 0) { + log(log_level::ERROR, "Failed to send response for register"); + } return; } } @@ -198,7 +208,9 @@ void mtl_instance::handle_message_register(mtl_register_message_t* register_msg) log(log_level::INFO, "Registered."); is_registered = true; - send_response(0); + if (send_response(0) < 0) { + log(log_level::ERROR, "Failed to send response for register"); + } } std::shared_ptr mtl_instance::get_interface(const unsigned int ifindex) { @@ -265,11 +277,15 @@ void mtl_instance::handle_message_udp_dp_filter( auto interface = get_interface(ifindex); if (interface == nullptr) { log(log_level::ERROR, "Failed to get interface " + std::to_string(ifindex)); - send_response(-1); + if (send_response(-1) < 0) { + log(log_level::ERROR, "Failed to send response for udp_dp_filter"); + } return; } int ret = interface->update_udp_dp_filter(port, add); - send_response(ret); + if (send_response(ret) < 0) { + log(log_level::ERROR, "Failed to send response for udp_dp_filter"); + } } void mtl_instance::handle_message_if_get_queue(mtl_if_message_t* if_msg) { @@ -277,12 +293,16 @@ void mtl_instance::handle_message_if_get_queue(mtl_if_message_t* if_msg) { auto interface = get_interface(ifindex); if (interface == nullptr) { log(log_level::ERROR, "Failed to get interface " + std::to_string(ifindex)); - send_response(-1, MTL_MSG_TYPE_IF_QUEUE_ID); + if (send_response(-1, MTL_MSG_TYPE_IF_QUEUE_ID) < 0) { + log(log_level::ERROR, "Failed to send response for if_get_queue"); + } return; } int ret = interface->get_queue(); if (ret > 0) if_queue_ids[ifindex].insert(ret); - send_response(ret, MTL_MSG_TYPE_IF_QUEUE_ID); + if (send_response(ret, MTL_MSG_TYPE_IF_QUEUE_ID) < 0) { + log(log_level::ERROR, "Failed to send response for if_get_queue"); + } } void mtl_instance::handle_message_if_put_queue(mtl_if_message_t* if_msg) { @@ -290,13 +310,17 @@ void mtl_instance::handle_message_if_put_queue(mtl_if_message_t* if_msg) { auto interface = get_interface(ifindex); if (interface == nullptr) { log(log_level::ERROR, "Failed to get interface " + std::to_string(ifindex)); - send_response(-1); + if (send_response(-1) < 0) { + log(log_level::ERROR, "Failed to send response for if_put_queue"); + } return; } uint16_t queue_id = ntohs(if_msg->queue_id); int ret = interface->put_queue(queue_id); if (ret == 0) if_queue_ids[ifindex].erase(queue_id); - send_response(ret); + if (send_response(ret) < 0) { + log(log_level::ERROR, "Failed to send response for if_put_queue"); + } } void mtl_instance::handle_message_if_add_flow(mtl_if_message_t* if_msg) { @@ -304,14 +328,18 @@ void mtl_instance::handle_message_if_add_flow(mtl_if_message_t* if_msg) { auto interface = get_interface(ifindex); if (interface == nullptr) { log(log_level::ERROR, "Failed to get interface " + std::to_string(ifindex)); - send_response(-1); + if (send_response(-1) < 0) { + log(log_level::ERROR, "Failed to send response for if_add_flow"); + } return; } int ret = interface->add_flow(ntohs(if_msg->queue_id), ntohl(if_msg->flow_type), ntohl(if_msg->src_ip), ntohl(if_msg->dst_ip), ntohs(if_msg->src_port), ntohs(if_msg->dst_port)); if (ret > 0) if_flow_ids[ifindex].insert(ret); - send_response(ret, MTL_MSG_TYPE_IF_FLOW_ID); + if (send_response(ret, MTL_MSG_TYPE_IF_FLOW_ID) < 0) { + log(log_level::ERROR, "Failed to send response for if_add_flow"); + } } void mtl_instance::handle_message_if_del_flow(mtl_if_message_t* if_msg) { @@ -319,13 +347,17 @@ void mtl_instance::handle_message_if_del_flow(mtl_if_message_t* if_msg) { auto interface = get_interface(ifindex); if (interface == nullptr) { log(log_level::ERROR, "Failed to get interface " + std::to_string(ifindex)); - send_response(-1); + if (send_response(-1) < 0) { + log(log_level::ERROR, "Failed to send response for if_del_flow"); + } return; } unsigned int flow_id = ntohl(if_msg->flow_id); int ret = interface->del_flow(flow_id); if (ret == 0) if_flow_ids[ifindex].erase(flow_id); - send_response(ret); + if (send_response(ret) < 0) { + log(log_level::ERROR, "Failed to send response for if_del_flow"); + } } #endif \ No newline at end of file From a2620a617050082c61e138c4b1f2e2ddaa505bb6 Mon Sep 17 00:00:00 2001 From: "Kasiewicz, Marek" Date: Tue, 10 Dec 2024 10:27:38 +0000 Subject: [PATCH 4/4] Correct code style Signed-off-by: Kasiewicz, Marek --- lib/src/datapath/mt_shared_queue.c | 3 +-- lib/src/st2110/st_rx_video_session.c | 8 +++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/src/datapath/mt_shared_queue.c b/lib/src/datapath/mt_shared_queue.c index 29805a06..cb773098 100644 --- a/lib/src/datapath/mt_shared_queue.c +++ b/lib/src/datapath/mt_shared_queue.c @@ -80,8 +80,7 @@ static int rsq_entry_free(struct mt_rsq_entry* entry) { mt_ring_dequeue_clean(entry->ring); rte_ring_free(entry->ring); } - if (entry->mcast_fd > 0) - close(entry->mcast_fd); + if (entry->mcast_fd > 0) close(entry->mcast_fd); entry->mcast_fd = -1; diff --git a/lib/src/st2110/st_rx_video_session.c b/lib/src/st2110/st_rx_video_session.c index 8abf626e..685fc069 100644 --- a/lib/src/st2110/st_rx_video_session.c +++ b/lib/src/st2110/st_rx_video_session.c @@ -1326,7 +1326,7 @@ static int rv_start_pcap(struct st_rx_video_session_impl* s, enum mtl_session_po return -EIO; } snprintf(pcap->file_name, sizeof(pcap->file_name), "st22rx_s%dp%d_%u_XXXXXX.pcapng", - idx, s_port, max_dump_packets); + idx, s_port, max_dump_packets); int fd = mt_mkstemps(pcap->file_name, strlen(".pcapng")); if (fd < 0) { @@ -3556,8 +3556,7 @@ static int rvs_pkt_rx_tasklet_start(void* priv) { static int rv_detach(struct mtl_main_impl* impl, struct st_rx_video_sessions_mgr* mgr, struct st_rx_video_session_impl* s) { - if (!mgr || !s) - return -EINVAL; + if (!mgr || !s) return -EINVAL; s->attached = false; rv_stat(mgr, s); rv_uinit(impl, s); @@ -3774,8 +3773,7 @@ static int rv_sessions_stat(void* priv) { struct st_rx_video_sessions_mgr* mgr = priv; struct st_rx_video_session_impl* s; - if (!mgr) - return -EINVAL; + if (!mgr) return -EINVAL; for (int j = 0; j < mgr->max_idx; j++) { s = rx_video_session_get_timeout(mgr, j, ST_SESSION_STAT_TIMEOUT_US);