I have found the root cause of this memory issue, and it appears to be a
bug in the OVS Meter code. See below from ofp-util.c.
void
ofputil_append_meter_config(struct ovs_list *replies,
const struct ofputil_meter_config *mc)
{
struct ofpbuf *msg = ofpbuf_from_list(list_back(replies));
size_t start_ofs = msg->size;
struct ofp13_meter_config *reply = ofpbuf_put_uninit(msg, sizeof
*reply); <--- establishing pointer into Meter response data
reply->flags = htons(mc->flags);
reply->meter_id = htonl(mc->meter_id);
ofputil_put_bands(mc->n_bands, mc->bands, msg); <--- this may result
in the Meter response data (msg->base) allocation being moved
reply->length = htons(msg->size - start_ofs); <--- this pointer
will still be into the old allocation
ofpmp_postappend(replies, start_ofs);
}
I have watched this play out. Once I get past the 41st Meter entry, the
allocation must expand and the realloc() calls happen for each added
meter. Eventually, the allocation has to be moved. The length value for
the Meter entry that is being added to the message does not get written.
Thus I get a response like this from dump-meters:
...
meter=52 pktps bands=
type=drop rate=52
meter=53 pktps bands=
type=drop rate=53
meter=54 pktps bands=
type=drop rate=54
meter=55 pktps bands=
type=drop rate=55
meter=56 pktps bands=
type=drop rate=56
***decode error: OFPBRC_BAD_LEN***
From: Gavin Remaley/SEL
To: discuss@openvswitch.org
Date: 07/13/2016 04:42 PM
Subject: Re: Meter config memory allocation problem
I see the code is trying to do some reallocating to grow the memory space
as needed. I need to investigate further. Symptoms suggest we are not
controlling the memory beyond 1000 bytes for some reason...
From: Gavin Remaley/SEL
To: discuss@openvswitch.org
Date: 07/13/2016 01:31 PM
Subject: Meter config memory allocation problem
We are using OVS v2.4.0 and seem to have discovered a memory allocation
issue.
When we have more than 41 meters (each with a single Band) defined, we
regularly get corruption in meters 42 and beyond within a Meter
Configuration Response to a request with meter_id=ALL.
ofpmp_init() is used as part of the message handling in ofproto.c
(handle_meter_request). It calls ofpraw_alloc_stats_reply, allocating
1000 "tail" bytes. Since each Meter requires 24 bytes in the message and
24 * 41 = 984, this seems to be the issue we are encountering. It looks
like Meters 42 and beyond occupy memory that we do not own.
Is this function being used properly with a Meter Configuration request?
Why the hard-coded 1000 bytes of space?
We need to fix this ASAP. We can do so, but an official patch would be
better.
Thanks.
_______________________________________________
discuss mailing list
discuss@openvswitch.org
http://openvswitch.org/mailman/listinfo/discuss
