On 11/16/22 22:45, Mark Michelson wrote: > Acked-by: Mark Michelson <[email protected]> > > I have some python-related nits below, but I don't think any of them is > strong enough to prevent this from being merged. >
I went ahead and rewrote most of the benchmark in python. It was too messy with half of it in bash. It should be a bit better now. > On 11/4/22 18:11, Dumitru Ceara wrote: >> In a sandbox run: >> >> $ ./ovn-lb-benchmark.sh <N> <VIPS> <BACKENDS> <USE_TEMPLATES> >> >> to simulate an ovn-k8s-like topology with N nodes, VIPS NodePort services >> applied to all nodes. Each service has BACKENDS backends. >> >> If USE_TEMPLATES is "yes" then the configuration will be optimized to use >> Chassis_Template_Vars. Otherwise it will create N LBs per service, one >> for every node. >> >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> tutorial/automake.mk | 4 + >> tutorial/ovn-gen-lb-template-vars.py | 116 >> ++++++++++++++++++++++++++++++++++ >> tutorial/ovn-lb-benchmark.sh | 110 >> ++++++++++++++++++++++++++++++++ >> 3 files changed, 229 insertions(+), 1 deletion(-) >> create mode 100755 tutorial/ovn-gen-lb-template-vars.py >> create mode 100755 tutorial/ovn-lb-benchmark.sh >> >> diff --git a/tutorial/automake.mk b/tutorial/automake.mk >> index 046962c000..171da8de66 100644 >> --- a/tutorial/automake.mk >> +++ b/tutorial/automake.mk >> @@ -1,6 +1,8 @@ >> EXTRA_DIST += \ >> tutorial/ovs-sandbox \ >> - tutorial/ovn-setup.sh >> + tutorial/ovn-setup.sh \ >> + tutorial/ovn-lb-benchmark.sh \ >> + tutorial/ovn-gen-lb-template-vars.py >> sandbox: all >> cd $(srcdir)/tutorial && MAKE=$(MAKE) >> HAVE_OPENSSL=$(HAVE_OPENSSL) \ >> ./ovs-sandbox -b $(abs_builddir) --ovs-src $(ovs_srcdir) >> --ovs-build $(ovs_builddir) $(SANDBOXFLAGS) >> diff --git a/tutorial/ovn-gen-lb-template-vars.py >> b/tutorial/ovn-gen-lb-template-vars.py >> new file mode 100755 >> index 0000000000..fe7e6b93f6 >> --- /dev/null >> +++ b/tutorial/ovn-gen-lb-template-vars.py >> @@ -0,0 +1,116 @@ >> +import getopt >> +import os >> +import re >> +import sys >> +import uuid >> + >> +import ovs.db.idl >> +import ovs.db.schema >> +import ovs.db.types >> +import ovs.ovsuuid >> +import ovs.poller >> +import ovs.stream >> +import ovs.util >> +import ovs.vlog >> +from ovs.db import data >> +from ovs.db import error >> +from ovs.db.idl import _row_to_uuid as row_to_uuid >> +from ovs.fatal_signal import signal_alarm > > The following imports are not used: > * os > * re > * uuid > * ovs.db.schema > * ovs.db.types > * ovs.ovsuuid > * from ovs.db import data > * from ovs.db.idl import _row_to_uuid as row_to_uuid > Ack, I removed them in v3. >> + >> +vlog = ovs.vlog.Vlog("template-lb-stress") >> +vlog.set_levels_from_string("console:info") >> +vlog.init(None) >> + >> +SCHEMA = '../ovn-nb.ovsschema' >> + >> + >> +def add_chassis_template_vars(idl, n, n_vips, n_backends): >> + for i in range(1, n + 1): >> + print(f'ADDING LBs for node {i}') >> + txn = ovs.db.idl.Transaction(idl) > > Would it make sense to create the transaction outside this for loop and > then commit just one big transaction at the end? > It actually takes really long for the transaction json to be built if we create a single huge transaction (e.g., in the case of 250 nodes with 10K LBs). It's way faster to just do 250 smaller transactions. >> + tv = txn.insert(idl.tables["Chassis_Template_Var"]) >> + tv.chassis = f'chassis-{i}' >> + tv.setkey('variables', 'vip', f'42.42.42.{i}') >> + >> + for j in range(1, n_vips + 1): >> + backends = '' >> + for k in range(0, n_backends): >> + j1 = j // 250 >> + j2 = j % 250 > > The j1 and j2 calculations can be moved out of this loop since they are > constant across all values of k. > > If you do that, then you can rewrite this inner loop as a list > comprehension: > > backends = [f'42.{k}.{j1}.{j2}:{j}' for k in range(n_backends)] > > (note that when using range(), you can omit the '0' as the first > parameter and just put the upper bound) > > Then when you need to stringify backends, use > > ','.join(backends) > Done. >> + backends = f'42.{k}.{j1}.{j2}:{j},{backends}' >> + tv.setkey('variables', f'backends{j}', backends) >> + status = txn.commit_block() >> + sys.stdout.write( >> + f'commit status = >> {ovs.db.idl.Transaction.status_to_string(status)}\n' >> + ) >> + >> + >> +def run(remote, n, n_vips, n_backends): >> + schema_helper = ovs.db.idl.SchemaHelper(SCHEMA) >> + schema_helper.register_all() >> + idl = ovs.db.idl.Idl(remote, schema_helper, leader_only=False) >> + >> + seqno = 0 >> + >> + error, stream = ovs.stream.Stream.open_block( >> + ovs.stream.Stream.open(remote), 2000 >> + ) >> + if error: >> + sys.stderr.write(f'failed to connect to \"{remote}\"') >> + sys.exit(1) >> + >> + if not stream: >> + sys.stderr.write(f'failed to connect to \"{remote}\"') >> + sys.exit(1) >> + rpc = ovs.jsonrpc.Connection(stream) >> + >> + while idl.change_seqno == seqno and not idl.run(): >> + rpc.run() >> + >> + poller = ovs.poller.Poller() >> + idl.wait(poller) >> + rpc.wait(poller) >> + poller.block() >> + >> + add_chassis_template_vars(idl, n, n_vips, n_backends) >> + >> + >> +def main(argv): >> + try: >> + options, args = getopt.gnu_getopt( >> + argv[1:], 'n:v:b:r:', ['vips', 'backends', 'remote'] >> + ) >> + except getopt.GetoptError as geo: >> + sys.stderr.write(f'{ovs.util.PROGRAM_NAME}: {geo.msg}\n') >> + sys.exit(1) >> + >> + n = None >> + vips = None >> + backends = None >> + remote = None >> + for key, value in options: >> + if key == '-n': >> + n = int(value) >> + elif key in ['-v', '--vips']: >> + vips = int(value) >> + elif key in ['-b', '--backends']: >> + backends = int(value) >> + elif key in ['-r', '--remote']: >> + remote = value >> + else: >> + sys.stderr.write(f'{ovs.util.PROGRAM_NAME}: unknown input >> args') >> + sys.exit(1) >> + >> + if not n or not vips or not backends: >> + sys.stderr.write(f'{ovs.util.PROGRAM_NAME}: invalid input args') >> + sys.exit(1) > > I'm of the opinion that argparse is streets ahead of getopt. argparse > allows for: > * automatic type-checking of inputs > * automatic usage and help text generation > * automatic detection of unknown/missing options > > I also think it's easier to read and write, and less error-prone than > getopt. > > As I said at the top of the email, none of the findings here are > actionable enough to require an update to the series. However, don't be > surprised if I end up submitting a change later that migrates from > getopt to argparse :) > It's way better indeed. I moved to argparse. >> + >> + run(remote, n, vips, backends) >> + >> + >> +if __name__ == '__main__': >> + try: >> + main(sys.argv) >> + except error.Error as e: >> + sys.stderr.write(f'{e}\n') >> + sys.exit(1) >> diff --git a/tutorial/ovn-lb-benchmark.sh b/tutorial/ovn-lb-benchmark.sh >> new file mode 100755 >> index 0000000000..6e8129ab97 >> --- /dev/null >> +++ b/tutorial/ovn-lb-benchmark.sh >> @@ -0,0 +1,110 @@ >> +#!/bin/bash >> + >> +nrtr=$1 >> +nlb=$2 >> +nbackends=$3 >> +use_template=$4 >> + >> +echo "ROUTERS : $nrtr" >> +echo "LBS : $nlb" >> +echo "BACKENDS PER LB: $nbackends" >> +echo "USE TEMPLATE : ${use_template}" >> + >> +export OVN_NB_DAEMON=$(ovn-nbctl --detach) >> +export OVN_SB_DAEMON=$(ovn-sbctl --detach) >> +trap "killall -9 ovn-nbctl; killall -9 ovn-sbctl" EXIT >> + >> +lbg=$(ovn-nbctl create load_balancer_group name=lbg) >> + >> +ovn-nbctl ls-add join >> +ovn-nbctl lr-add cluster >> +ovn-nbctl lrp-add cluster rcj 00:00:00:00:00:01 10.0.0.1/8 >> +ovn-nbctl lsp-add join sjc \ >> + -- lsp-set-type sjc router \ >> + -- lsp-set-addresses sjc router \ >> + -- lsp-set-options sjc router-port=rcj >> + >> +for i in $(seq $nrtr); do >> + ch="chassis-$i" >> + gwr=lr-$i >> + echo "ROUTER $gwr" >> + gwr2join=lr2j-$i >> + join2gwr=j2lr-$i >> + >> + ovn-nbctl lr-add $gwr \ >> + -- set logical_router $gwr load_balancer_group=$lbg \ >> + -- set logical_router $gwr options:chassis=$ch >> + ovn-nbctl lrp-add $gwr $gwr2join 00:00:00:00:00:01 10.0.0.1/8 >> + ovn-nbctl lsp-add join $join2gwr \ >> + -- lsp-set-type $join2gwr router \ >> + -- lsp-set-addresses $join2gwr router \ >> + -- lsp-set-options $join2gwr router-port=$gwr2join >> + >> + s=ls-$i >> + echo "SWITCH $s" >> + s2cluster=s2c-$s >> + cluster2s=c2s-$s >> + ovn-nbctl ls-add $s \ >> + -- set logical_switch $s load_balancer_group=$lbg >> + ovn-nbctl lsp-add $s $s2cluster \ >> + -- lsp-set-type $s2cluster router \ >> + -- lsp-set-addresses $s2cluster router \ >> + -- lsp-set-options $s2cluster router-port=$cluster2s >> + ovn-nbctl lrp-add cluster $cluster2s 00:00:00:00:00:01 10.0.0.1/8 >> + ovn-nbctl lrp-set-gateway-chassis $cluster2s $ch 1 >> + >> + lsp=lsp-$i >> + echo "LSP $lsp" >> + ovn-nbctl lsp-add $s $lsp >> +done >> + >> +# Bind a port from the first LS locally. >> +ovs-vsctl add-port br-int lsp-1 \ >> + -- set interface lsp-1 external_ids:iface-id=lsp-1 >> + >> +# Add NodePort-like LBs using templates. >> +function add_template_lbs() { >> + for l in $(seq $nlb); do >> + lb=lb-$l >> + echo "LB $lb" >> + ovn-nbctl --template lb-add $lb "^vip:$l" "^backends$l" tcp >> + lb_uuid=$(ovn-nbctl --columns _uuid --bare find load_balancer >> name=$lb) >> + ovn-nbctl add load_balancer_group $lbg load_balancer $lb_uuid >> + done >> + >> + # Generate LBs chassis_template_vars. >> + python ovn-gen-lb-template-vars.py -n $nrtr -v $nlb -b $nbackends \ >> + -r unix:$PWD/sandbox/nb1.ovsdb >> +} >> + >> +# Add NodePort-like LBs without using templates. >> +function add_non_template_lbs() { >> + for i in $(seq $nrtr); do >> + echo ITERATION $i >> + gwr=lr-$i >> + s=ls-$i >> + for l in $(seq $nlb); do >> + lb=lb-$l-$i >> + echo LB $lb >> + backends="" >> + for k in $(seq $nbackends); do >> + l1=$(expr $l / 250) >> + l2=$(expr $l % 250) >> + backends="42.$k.$l1.$l2:$l,$backends" >> + done >> + lb_uuid=$(ovn-nbctl create load_balancer name=$lb \ >> + protocol=tcp >> vips:"\"42.42.42.$i:$l\""="\"$backends\"") >> + ovn-nbctl add logical_switch $s load_balancer ${lb_uuid} >> + ovn-nbctl add logical_router $gwr load_balancer ${lb_uuid} >> + done >> + done >> +} >> + >> +if [ "${use_template}" = "yes" ]; then >> + add_template_lbs >> +else >> + add_non_template_lbs >> +fi >> + >> +ovs-appctl -t $PWD/sandbox/nb1 ovsdb-server/compact >> +ovs-appctl -t $PWD/sandbox/sb1 ovsdb-server/compact >> > Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
