[ovs-dev] [PATCH ovn] northd: avoid memory reallocation while building ACL and QoS rules
mmichels at redhat.com
Tue Jun 22 21:22:02 UTC 2021
On 6/22/21 1:14 PM, Dan Williams wrote:
> On Fri, 2021-06-18 at 21:49 +0200, Dumitru Ceara wrote:
>> On 6/4/21 10:00 PM, Dan Williams wrote:
>>> Inspried by:
>>> 3b6362d64e86b northd: Avoid memory reallocation while building lb
>>> Signed-off-by: Dan Williams <dcbw at redhat.com>
>>> NOTE: this is driven by visual inspection not perf data. But it
>>> shouldn't be worse than current code and should be better for
>>> large numbers of ACLs I think.
>> The changes look OK to me.
>> Acked-by: Dumitru Ceara <dceara at redhat.com>
>> However, I wonder how many such optimizations we can implement
>> affecting maintainability. Mark suggested an approach .
> I'm happy to drop my patch in favor of Mark's. I think mine is a subset
> of his.
Funny because I'm not even 100% behind my own approach.
>> CC-ing Ilya too, maybe he has some more suggestions, maybe there's a
>> to better use the OVS dynamic strings.
I did some brainstorming and came up with a test program:
my_crazy_printf(char *fmt1, char *fmt2, ...)
my_crazy_printf("%s=%d\n", "%s=%d\n", "howdy", 4, "byebye", 3);
On my system, the output is:
I came up with this as a test to see how feasible it is to have two
format strings in the same parameter list. The idea here is to translate
that into something like this:
/* I've omitted unimportant parameters */
ovn_lflow_add(match_fmt, actions_fmt, ...)
static struct ds match = DS_EMPTY_INITIALIZER;
static struct ds actions = DS_EMPTY_INITIALIZER;
struct va_list ap;
ds_put_format_valist(&match, match_fmt, ap);
struct va_list aq;
ds_put_format_valist(&match, actions_fmt, aq);
/* The rest of the function */
With this, the dynamic string handling is done entirely within
ovn_lflow_add(), meaning that the same buffers are reused. The problem
(aside from the fact that it's weird), is that ds_put_format_valist()
performs its own va_copy() operation of the passed-in va_list. This
means that the two ds_put_format_valist() operations operate on
identical va_lists. Pursuing this problem any further means essentially
re-implementing dynamic strings to allow for this unorthodox usage. It
feels like a dead end to me.
 Test code:
If you run this code, then the output is:
More information about the dev