Alex,
The "key" member in struct flow_miss refers to memory held by the "struct
upcall", hence the upcalls should be freed only after the flow misses are
processed by the main thread. How about this instead:
Free upcalls after they are used.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
ofproto/ofproto-dpif-upcall.c | 17 +++++++++++++----
ofproto/ofproto-dpif-upcall.h | 5 +++++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index d75c61b..c65b366 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -309,6 +309,7 @@ void
flow_miss_batch_destroy(struct flow_miss_batch *fmb)
{
struct flow_miss *miss, *next;
+ struct upcall *upcall, *next_upcall;
if (!fmb) {
return;
@@ -319,6 +320,11 @@ flow_miss_batch_destroy(struct flow_miss_batch *fmb)
miss_destroy(miss);
}
+ LIST_FOR_EACH_SAFE (upcall, next_upcall, list_node, &fmb->upcalls) {
+ list_remove(&upcall->list_node);
+ upcall_destroy(upcall);
+ }
+
hmap_destroy(&fmb->misses);
free(fmb);
}
@@ -599,7 +605,7 @@ handle_miss_upcalls(struct udpif *udpif, struct list
*upcalls)
struct dpif_op ops[FLOW_MISS_MAX_BATCH];
struct upcall *upcall, *next;
struct flow_miss_batch *fmb;
- size_t n_upcalls, n_ops, i;
+ size_t n_misses, n_ops, i;
struct flow_miss *miss;
unsigned int reval_seq;
bool fail_open;
@@ -626,11 +632,12 @@ handle_miss_upcalls(struct udpif *udpif, struct list
*upcalls)
fmb = xmalloc(sizeof *fmb);
atomic_read(&udpif->reval_seq, &fmb->reval_seq);
hmap_init(&fmb->misses);
- n_upcalls = 0;
+ list_init(&fmb->upcalls);
+ n_misses = 0;
LIST_FOR_EACH_SAFE (upcall, next, list_node, upcalls) {
struct dpif_upcall *dupcall = &upcall->dpif_upcall;
struct ofpbuf *packet = dupcall->packet;
- struct flow_miss *miss = &fmb->miss_buf[n_upcalls];
+ struct flow_miss *miss = &fmb->miss_buf[n_misses];
struct flow_miss *existing_miss;
struct ofproto_dpif *ofproto;
odp_port_t odp_in_port;
@@ -661,7 +668,7 @@ handle_miss_upcalls(struct udpif *udpif, struct list
*upcalls)
miss->stats.used = time_msec();
miss->stats.tcp_flags = 0;
- n_upcalls++;
+ n_misses++;
} else {
miss = existing_miss;
}
@@ -814,6 +821,8 @@ handle_miss_upcalls(struct udpif *udpif, struct list
*upcalls)
}
}
+ list_move(&fmb->upcalls, upcalls);
+
atomic_read(&udpif->reval_seq, &reval_seq);
if (reval_seq != fmb->reval_seq) {
COVERAGE_INC(fmb_queue_revalidated);
diff --git a/ofproto/ofproto-dpif-upcall.h b/ofproto/ofproto-dpif-upcall.h
index 9bd19ad..cd97e79 100644
--- a/ofproto/ofproto-dpif-upcall.h
+++ b/ofproto/ofproto-dpif-upcall.h
@@ -104,6 +104,11 @@ struct flow_miss_batch {
struct hmap misses;
unsigned int reval_seq;
+
+ /* Flow misses refer to the memory held by "struct upcall"s,
+ * so we need to keep track of the upcalls to be able to
+ * free them when done. */
+ struct list upcalls; /* Contains "struct upcall"s. */
};
struct flow_miss_batch *flow_miss_batch_next(struct udpif *);
--
On Sep 23, 2013, at 10:06 AM, Alex Wang <al...@nicira.com> wrote:
> This commit fixes a memory leak in ofproto-dpif-upcall module.
> The memory leak is caused by not freeing the 'struct upcall's.
>
> Signed-off-by: Alex Wang <al...@nicira.com>
> ---
> ofproto/ofproto-dpif-upcall.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index d75c61b..ec253e1 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -814,6 +814,12 @@ handle_miss_upcalls(struct udpif *udpif, struct list
> *upcalls)
> }
> }
>
> + /* Frees all remaining upcalls. */
> + LIST_FOR_EACH_SAFE (upcall, next, list_node, upcalls) {
> + list_remove(&upcall->list_node);
> + upcall_destroy(upcall);
> + }
> +
> atomic_read(&udpif->reval_seq, &reval_seq);
> if (reval_seq != fmb->reval_seq) {
> COVERAGE_INC(fmb_queue_revalidated);
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
