Hi Rosemarie,
On 10/23/26 11:09 PM, Rosemarie O'Riorden wrote:
> On 10/23/26 3:30 PM, Dumitru Ceara wrote:
>> Hi Rosemarie,
>>
>> On 10/23/26 7:15 AM, Rosemarie O'Riorden wrote:
>>> This commit adds a test that developers can run to check if the changes
>>> they've made cause a regression in the performance of ovn-controller or
>>> ovn-northd.
>>>
>>> Run from inside the sandbox and compare two commits' runtime and peak
>>> memory usage for each process.
>>>
>>> This test creates a topology consisting of logical switches and routers,
>>> ACLs, port groups, NAT, DHCP, DNS, QoS, routing policies, and load
>>> balancers. The user can also provide their own database file for the
>>> benchmark.
>>>
>>> * To run the test with defaults (200 nodes, track northd and controller):
>>> ./ovn-benchmark.sh
>>> * To run with 50 nodes: ./ovn-benchmark.sh 50
>>> * To run with 30 nodes and only track ovn-northd:
>>> ./ovn-benchmark.sh 30 northd
>>> * To run with Valgrind Massif tracking allocations:
>>> ./ovn-benchmark.sh --valgrind
>>> * To run with a custom db file: ./ovn-benchmark.sh -f file.db
>>> * To run with debug info printed: ./ovn-benchmark.sh --debug
>>> * To see detailed usage instructions: ./ovn-benchmark.sh --help
>>>
>>> Each "node" is a router-switch pair with associated features:
>>> - 1 gateway router (with NAT, static routes, routing policies)
>>> - 1 logical switch (with 9 ports by default, DHCP, DNS, QoS)
>>> - 5 load balancers (each with 5 backends)
>>> - ACLs, port groups, and address sets (shared across all nodes)
>>>
>>> For example, 200 nodes creates:
>>> - 200 routers + 200 switches
>>> - 1,800 logical switch ports (9 per switch)
>>> - 1,000 load balancers (5 per node)
>>> - 1,200 NAT rules, 1,200 static routes, 800 routing policies
>>> - Plus ACLs, DHCP/DNS records, QoS rules, etc.
>>>
>>> ovn-benchmark.py creates the topology and ovn-benchmark.sh is the
>>> wrapper that tracks peak memory and execution time. ovn-benchmark.py was
>>> built off of ovn-lb-benchmark.py as a starting point.
>>>
>>> VmPeak is tracked by default, but Valgrind can be used instead for more
>>> precise results.
>>>
>>> Reported-at: https://redhat.atlassian.net/browse/FDP-978
>>> Assisted-by: Claude Sonnet 4.5, Claude Code
>>> Signed-off-by: Rosemarie O'Riorden <[email protected]>
>>> ---
>>> BTW:
>>> A CI component for this patch, which would add a GH action that runs the
>>> benchmark on the base upstream vs with the patch applied, and compares
>>> the results to test for regressions, is coming in the future.
>>>
>>> +
>>> +def create_topology(idl, n, ports_per_switch, batch_size):
>>> + """Create the basic topology with routers, switches, and ports."""
>>> + vlog.info('Creating topology')
>>> + txn = ovs.db.idl.Transaction(idl)
>>> + lbg = txn.insert(idl.tables['Load_Balancer_Group'])
>>> + lbg.name = 'lbg'
>>
>> We never add load balancers through this LB group. That's something
>> that would be nice to test for too. But I don't want to block this
>> patch set any longer. Would you mind sending a follow up for that?
>
> Yup, I can address this and the asan/ubsan in follow-ups.
>
Ack, looking forward to that.
[...]
>>> +
>>> + kill "$pid"
>>> + while [ -e "/proc/$pid" ]; do sleep 0.1; done
>>
>> This may end up looping indefinitely if for some reason "$pid" refuses
>> to terminate, e.g., ignores SIGTERM.
>>
>> I think we'll need a sort of "kill_insist()" or similar function that
>> would force a SIGKILL to be sent if the process doesn't end in time, e.g.:
>>
>> kill_insist() {
>> local pid=$1
>> local timeout_ms=${2:-30000}
>>
>> env kill -0 "$pid" 2> /dev/null && \
>> env kill --timeout "$timeout_ms" KILL "$pid"
>> }
>>
>> We could use this function inside cleanup() too where we're currently
>> cleaning up any leftover valgrind processes.
>>
>> We need "env" because the bash builtin kill doesn't support --timeout.
>>
>> The rest looks good to me so, aside from the load balancer group comment
>> and the check/note about valgrind not working with ubsan/asan (discussed
>> in the other thread) which can be potential follow ups, I'm considering
>> applying this patch to main with the following incremental change
>> squashed in. Please let me know what you think about it.
>
> This is a good edit, I'm fine with it being squashed into my patch.
> Thank you so much!
>
Cool, I squashed this in and pushed the patch to main.
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
