From f7f5478eba431d6d38fbd84b3e552ce412e3ced4 Mon Sep 17 00:00:00 2001 From: "Anand H. Krishnan" Date: Mon, 20 Apr 2015 12:41:11 +0530 Subject: [PATCH] Free the defer data in case of errors To make sure that we flush all the packets that are queued in a flow entry, we run a defer function. If for any reason this defer was not scheduled (because the function was called with no hold queue), the defer data has to be freed. Change-Id: Ideabe252ff6c56c6e01ccbc3a27044bac09933f3 Closes-BUG: #1436798 --- dp-core/vr_flow.c | 52 +++++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/dp-core/vr_flow.c b/dp-core/vr_flow.c index 254181d4b..ae1c87e95 100644 --- a/dp-core/vr_flow.c +++ b/dp-core/vr_flow.c @@ -99,7 +99,7 @@ vr_valid_link_local_port(struct vrouter *router, int family, if (proto == VR_IP_PROTO_UDP) tmp += (router->vr_link_local_ports_size * 8 / 2); - data = router->vr_link_local_ports[(tmp /8)]; + data = router->vr_link_local_ports[(tmp / 8)]; if (data & (1 << (tmp % 8))) return true; @@ -110,7 +110,6 @@ static void vr_clear_link_local_port(struct vrouter *router, int family, int proto, int port) { - unsigned char *data; unsigned int tmp; @@ -128,15 +127,16 @@ vr_clear_link_local_port(struct vrouter *router, int family, if (proto == VR_IP_PROTO_UDP) tmp += (router->vr_link_local_ports_size * 8 / 2); - data = &router->vr_link_local_ports[(tmp /8)]; + data = &router->vr_link_local_ports[(tmp / 8)]; *data &= (~(1 << (tmp % 8))); + + return; } static void vr_set_link_local_port(struct vrouter *router, int family, int proto, int port) { - unsigned char *data; unsigned int tmp; @@ -154,8 +154,10 @@ vr_set_link_local_port(struct vrouter *router, int family, if (proto == VR_IP_PROTO_UDP) tmp += (router->vr_link_local_ports_size * 8 / 2); - data = &router->vr_link_local_ports[tmp /8]; + data = &router->vr_link_local_ports[tmp / 8]; *data |= (1 << (tmp % 8)); + + return; } static void @@ -193,7 +195,13 @@ vr_reset_flow_entry(struct vrouter *router, struct vr_flow_entry *fe, unsigned int index) { memset(&fe->fe_stats, 0, sizeof(fe->fe_stats)); - memset(&fe->fe_hold_list, 0, sizeof(fe->fe_hold_list));; + + if (fe->fe_hold_list) { + vr_printf("vrouter: Potential memory leak @ %s:%d\n", + __FILE__, __LINE__); + } + fe->fe_hold_list = NULL; + fe->fe_key.key_len = 0; fe->fe_type = VP_TYPE_NULL; memset(&fe->fe_key, 0, sizeof(fe->fe_key)); @@ -314,6 +322,7 @@ vr_flow_queue_free_defer(struct vr_flow_md *flmd, struct vr_flow_queue *vfq) vdd->vdd_data = (void *)vfq; vr_defer(flmd->flmd_router, vr_flow_queue_free, (void *)vdd); + flmd->flmd_defer_data = NULL; return; } @@ -341,7 +350,7 @@ vr_find_free_entry(struct vrouter *router, struct vr_flow *key, uint8_t type, } index++; } - + if (!fe) { index = hash % vr_oflow_entries; for (i = 0; i < vr_oflow_entries; i++) { @@ -525,7 +534,7 @@ vr_flow_set_forwarding_md(struct vrouter *router, struct vr_flow_entry *fe, } static flow_result_t -vr_flow_action(struct vrouter *router, struct vr_flow_entry *fe, +vr_flow_action(struct vrouter *router, struct vr_flow_entry *fe, unsigned int index, struct vr_packet *pkt, struct vr_forwarding_md *fmd) { @@ -656,7 +665,7 @@ vr_do_flow_action(struct vrouter *router, struct vr_flow_entry *fe, fe->fe_stats.flow_bytes_oflow++; new_stats = __sync_add_and_fetch(&fe->fe_stats.flow_packets, 1); - if (!new_stats) + if (!new_stats) fe->fe_stats.flow_packets_oflow++; if (fe->fe_action == VR_FLOW_ACTION_HOLD) { @@ -752,8 +761,8 @@ vr_flow_lookup(struct vrouter *router, struct vr_flow *key, flow_e->fe_vrf = fmd->fmd_dvrf; /* mark as hold */ vr_flow_entry_set_hold(router, flow_e); - } - + } + return vr_do_flow_action(router, flow_e, fe_index, pkt, fmd); } @@ -829,7 +838,7 @@ vr_flush_flow_queue(struct vrouter *router, struct vr_flow_entry *fe, continue; pnode->pl_packet = NULL; - /* + /* * this is only a security check and not a catch all check. one note * of caution. please do not access pkt->vp_if till the if block is * succesfully bypassed @@ -884,7 +893,7 @@ vr_flow_flush(void *arg) struct vrouter *router; struct vr_flow_entry *fe; struct vr_forwarding_md fmd; - struct vr_flow_md *flmd = + struct vr_flow_md *flmd = (struct vr_flow_md *)arg; router = flmd->flmd_router; @@ -905,6 +914,11 @@ vr_flow_flush(void *arg) } exit_flush: + if (flmd->flmd_defer_data) { + vr_put_defer_data(flmd->flmd_defer_data); + flmd->flmd_defer_data = NULL; + } + vr_free(flmd); return; @@ -918,7 +932,7 @@ vr_flow_set_mirror(struct vrouter *router, vr_flow_req *req, if (!(req->fr_flags & VR_FLOW_FLAG_MIRROR) && (fe->fe_flags & VR_FLOW_FLAG_MIRROR)) { - vr_flow_reset_mirror(router, fe, req->fr_index); + vr_flow_reset_mirror(router, fe, req->fr_index); return; } @@ -1049,7 +1063,7 @@ vr_flow_req_is_invalid(struct vrouter *router, vr_flow_req *req, return -EINVAL; } - /* + /* * for delete, we need not validate nh_index from incoming request */ if (req->fr_flags & VR_FLOW_FLAG_ACTIVE) { @@ -1158,7 +1172,7 @@ vr_flow_set(struct vrouter *router, vr_flow_req *req) ((req->fr_action != fe->fe_action) || !(req->fr_flags & VR_FLOW_FLAG_ACTIVE))) __sync_fetch_and_add(&infop->vfti_action_count, 1); - /* + /* * for delete, absence of the requested flow entry is caustic. so * handle that case first */ @@ -1200,7 +1214,7 @@ vr_flow_set(struct vrouter *router, vr_flow_req *req) } fe->fe_vrf = req->fr_flow_vrf; - if (req->fr_flags & VR_FLOW_FLAG_VRFT) + if (req->fr_flags & VR_FLOW_FLAG_VRFT) fe->fe_dvrf = req->fr_flow_dvrf; if (fe->fe_type == VP_TYPE_IP) { @@ -1478,6 +1492,8 @@ vr_link_local_ports_reset(struct vrouter *router) memset(router->vr_link_local_ports, 0, router->vr_link_local_ports_size); } + + return; } static void @@ -1488,6 +1504,8 @@ vr_link_local_ports_exit(struct vrouter *router) router->vr_link_local_ports = NULL; router->vr_link_local_ports_size = 0; } + + return; } static int