From 74bc8f26ecfd99bad9dcb6948d038b5efe8dfa41 Mon Sep 17 00:00:00 2001 From: "Anand H. Krishnan" Date: Fri, 9 Sep 2016 19:20:27 +0530 Subject: [PATCH] Fix memory allocation for interface request In order to pass per lcore queue input error statistics to the application that does vif query, we allocate only VR_MAX_CPUS worth of memory, but we try to copy vr_num_cpus worth of data. In the case of vr_num_cpus > VR_MAX_CPUS (64), we will hit a snag. As a fix, allocate memory for vr_num_cpus. Change-Id: Ifb1060aac20011b8d51e1b31063a363fe268fd3d Closes-Bug: #1621816 --- dp-core/vr_interface.c | 16 ++++++++++++++-- dp-core/vr_sandesh.c | 1 + include/vr_interface.h | 1 + include/vrouter.h | 1 + utils/vif.c | 21 +++++++++------------ 5 files changed, 26 insertions(+), 14 deletions(-) diff --git a/dp-core/vr_interface.c b/dp-core/vr_interface.c index 0f485e087..2afaefa2f 100644 --- a/dp-core/vr_interface.c +++ b/dp-core/vr_interface.c @@ -1988,7 +1988,7 @@ __vr_interface_make_req(vr_interface_req *req, struct vr_interface *intf, req->vifr_oerrors = 0; /* queue counters */ req->vifr_queue_ipackets = 0; - for (i = 0; i < VR_MAX_CPUS; i++) + for (i = 0; i < vr_num_cpus; i++) req->vifr_queue_ierrors_to_lcore[i] = 0; req->vifr_queue_ierrors = 0; req->vifr_queue_opackets = 0; @@ -2072,6 +2072,18 @@ __vr_interface_make_req(vr_interface_req *req, struct vr_interface *intf, return; } +unsigned int +vr_interface_req_get_size(void *req_p) +{ + unsigned int size = 4 * sizeof(vr_interface_req); + vr_interface_req *req = (vr_interface_req *)req_p; + + if (req->vifr_queue_ierrors_to_lcore) + size += (vr_num_cpus * sizeof(int64_t)); + + return size; +} + static int vr_interface_make_req(vr_interface_req *req, struct vr_interface *vif, unsigned int core) @@ -2116,7 +2128,7 @@ vr_interface_req_get(void) req->vifr_name = vr_zalloc(VR_INTERFACE_NAME_LEN, VR_INTERFACE_REQ_NAME_OBJECT); - req->vifr_queue_ierrors_to_lcore = vr_zalloc(VR_MAX_CPUS * sizeof(uint64_t), + req->vifr_queue_ierrors_to_lcore = vr_zalloc(vr_num_cpus * sizeof(uint64_t), VR_INTERFACE_REQ_TO_LCORE_ERRORS_OBJECT); if (req->vifr_queue_ierrors_to_lcore) req->vifr_queue_ierrors_to_lcore_size = 0; diff --git a/dp-core/vr_sandesh.c b/dp-core/vr_sandesh.c index 17f1308f8..c74004418 100644 --- a/dp-core/vr_sandesh.c +++ b/dp-core/vr_sandesh.c @@ -16,6 +16,7 @@ struct sandesh_object_md sandesh_md[] = { }, [VR_INTERFACE_OBJECT_ID] = { .obj_len = 4 * sizeof(vr_interface_req), + .obj_get_size = vr_interface_req_get_size, .obj_type_string = "vr_interface_req", }, [VR_NEXTHOP_OBJECT_ID] = { diff --git a/include/vr_interface.h b/include/vr_interface.h index 3c920fbf0..e54359d0c 100644 --- a/include/vr_interface.h +++ b/include/vr_interface.h @@ -311,4 +311,5 @@ extern void vr_set_vif_ptr(struct net_device *dev, void *vif); #endif extern fat_flow_port_mask_t vif_fat_flow_lookup(struct vr_interface *, uint8_t, uint16_t, uint16_t); +extern unsigned int vr_interface_req_get_size(void *); #endif /* __VR_INTERFACE_H__ */ diff --git a/include/vrouter.h b/include/vrouter.h index 2ce15892f..4df1d553c 100644 --- a/include/vrouter.h +++ b/include/vrouter.h @@ -13,6 +13,7 @@ extern "C" { #include "sandesh.h" #include "vr_types.h" #include "vr_flow.h" +#include "vr_interface.h" #include "vr_nexthop.h" #include "vr_route.h" #include "vr_response.h" diff --git a/utils/vif.c b/utils/vif.c index 89bde9bb7..4d0a51cd2 100644 --- a/utils/vif.c +++ b/utils/vif.c @@ -632,6 +632,13 @@ rate_process(vr_interface_req *req, vr_interface_req *prev_req) temp_prev_req_ptr = prev_req->vifr_queue_ierrors_to_lcore; *prev_req = *req; prev_req->vifr_queue_ierrors_to_lcore = temp_prev_req_ptr; + if (!prev_req->vifr_queue_ierrors_to_lcore) { + prev_req->vifr_queue_ierrors_to_lcore = + malloc(req->vifr_queue_ierrors_to_lcore_size * sizeof(uint64_t)); + if (!prev_req->vifr_queue_ierrors_to_lcore) + return; + } + memcpy(prev_req->vifr_queue_ierrors_to_lcore, req->vifr_queue_ierrors_to_lcore, req->vifr_queue_ierrors_to_lcore_size * sizeof(uint64_t)); @@ -640,7 +647,8 @@ rate_process(vr_interface_req *req, vr_interface_req *prev_req) } rate_req_temp = *req; - rate_req_temp.vifr_queue_ierrors_to_lcore = calloc(VR_MAX_CPUS, sizeof(uint64_t)); + rate_req_temp.vifr_queue_ierrors_to_lcore = + calloc(req->vifr_queue_ierrors_to_lcore_size, sizeof(uint64_t)); if (!rate_req_temp.vifr_queue_ierrors_to_lcore) { fprintf(stderr, "Fail, memory allocation. (%s:%d).", __FILE__ , __LINE__); @@ -1503,17 +1511,6 @@ main(int argc, char *argv[]) vr_intf_op(cl, vr_op); } else { - for (i = 0; i < VR_MAX_INTERFACES; i++) { - - prev_req[i].vifr_queue_ierrors_to_lcore = - (calloc(VR_MAX_CPUS, sizeof(uint64_t))); - - if (!(prev_req[i].vifr_queue_ierrors_to_lcore)) { - fprintf(stderr, "Fail, memory allocation. (%s:%d).", __FILE__ , __LINE__); - exit(1); - } - } - fcntl(STDIN_FILENO, F_SETFL, fcntl(STDIN_FILENO, F_GETFL) | O_NONBLOCK); /* * tc[get/set]attr functions are for changing terminal behavior.