| Message ID | 20261001034309.400119-2-rosemarie@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v3,1/2] tutorial: Add developer performance benchmark. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
I just have a minor change request. This patch guards only the node count but the same overflow applies to backends which use the second octet. Using -b > 255 produces the same invalid address. You accounted for the backends in ovn-benchmark.py. Since this is a similar issue with a similar solution could you fix it here as well? On Wed, Sep 30, 2026 at 11:43 PM Rosemarie O'Riorden via dev < ovs-dev@openvswitch.org> wrote: > ovn-lb-benchmark.py embedded the node index directly in the last > octet of VIP addresses (42.42.42.{i}). When more than 255 nodes > are requested, this produces addresses like 42.42.42.256 which are > not valid IPv4 -- octets must be in the range 0-255. > > Spread the node index across the third and fourth octets > (42.42.{i >> 8}.{i & 0xff}) to support up to 65535 nodes with > valid addresses. > > This is the same design as ovn-benchmark.py. > > Fixes: 4eefb6359203 ("tutorial: Add scripts to simulate node-port ovn-k8s > services.") > Assisted-by: Claude Sonnet 4.5, Claude Code > Signed-off-by: Rosemarie O'Riorden <rosemarie@redhat.com> > --- > This patch was added in v2 and has not been changed in v3. > --- > tutorial/ovn-lb-benchmark.py | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/tutorial/ovn-lb-benchmark.py b/tutorial/ovn-lb-benchmark.py > index 658beb94c..f7075174c 100755 > --- a/tutorial/ovn-lb-benchmark.py > +++ b/tutorial/ovn-lb-benchmark.py > @@ -20,6 +20,11 @@ def die(msg): > sys.exit(1) > > > +def ip_node(i): > + """Convert node index to two IP octets, supporting up to 65535 > nodes.""" > + return f'{i >> 8}.{i & 0xff}' > + > + > def create_topology(idl, n): > vlog.info('Creating topology') > txn = ovs.db.idl.Transaction(idl) > @@ -121,7 +126,7 @@ def add_chassis_template_vars(idl, n, n_vips, > n_backends): > txn = ovs.db.idl.Transaction(idl) > tv = txn.insert(idl.tables['Chassis_Template_Var']) > tv.chassis = f'chassis-{i}' > - tv.setkey('variables', 'vip', f'42.42.42.{i}') > + tv.setkey('variables', 'vip', f'42.42.{ip_node(i)}') > > for j in range(n_vips): > port = j + 1 > @@ -154,7 +159,8 @@ def add_explicit_lbs(idl, n, n_vips, n_backends): > > lb = txn.insert(idl.tables['Load_Balancer']) > lb.name = f'lb-{j}-{i}' > - lb.setkey('vips', f'42.42.42.{i}:{port}', > f'{",".join(backends)}') > + lb.setkey('vips', f'42.42.{ip_node(i)}:{port}', > + f'{",".join(backends)}') > lb.protocol = 'tcp' > lr.addvalue('load_balancer', lb.uuid) > ls.addvalue('load_balancer', lb.uuid) > @@ -223,6 +229,11 @@ def main(argv): > ) > parser.set_defaults(template=False) > args = parser.parse_args() > + > + if args.nodes > 65535: > + sys.stderr.write('Error: maximum supported node count is 65535\n') > + sys.exit(1) > + > run(args.remote, args.nodes, args.vips, args.backends, args.template) > > > -- > 2.55.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Jacob Tanenbaum
diff --git a/tutorial/ovn-lb-benchmark.py b/tutorial/ovn-lb-benchmark.py index 658beb94c..f7075174c 100755 --- a/tutorial/ovn-lb-benchmark.py +++ b/tutorial/ovn-lb-benchmark.py @@ -20,6 +20,11 @@ def die(msg): sys.exit(1) +def ip_node(i): + """Convert node index to two IP octets, supporting up to 65535 nodes.""" + return f'{i >> 8}.{i & 0xff}' + + def create_topology(idl, n): vlog.info('Creating topology') txn = ovs.db.idl.Transaction(idl) @@ -121,7 +126,7 @@ def add_chassis_template_vars(idl, n, n_vips, n_backends): txn = ovs.db.idl.Transaction(idl) tv = txn.insert(idl.tables['Chassis_Template_Var']) tv.chassis = f'chassis-{i}' - tv.setkey('variables', 'vip', f'42.42.42.{i}') + tv.setkey('variables', 'vip', f'42.42.{ip_node(i)}') for j in range(n_vips): port = j + 1 @@ -154,7 +159,8 @@ def add_explicit_lbs(idl, n, n_vips, n_backends): lb = txn.insert(idl.tables['Load_Balancer']) lb.name = f'lb-{j}-{i}' - lb.setkey('vips', f'42.42.42.{i}:{port}', f'{",".join(backends)}') + lb.setkey('vips', f'42.42.{ip_node(i)}:{port}', + f'{",".join(backends)}') lb.protocol = 'tcp' lr.addvalue('load_balancer', lb.uuid) ls.addvalue('load_balancer', lb.uuid) @@ -223,6 +229,11 @@ def main(argv): ) parser.set_defaults(template=False) args = parser.parse_args() + + if args.nodes > 65535: + sys.stderr.write('Error: maximum supported node count is 65535\n') + sys.exit(1) + run(args.remote, args.nodes, args.vips, args.backends, args.template)
ovn-lb-benchmark.py embedded the node index directly in the last octet of VIP addresses (42.42.42.{i}). When more than 255 nodes are requested, this produces addresses like 42.42.42.256 which are not valid IPv4 -- octets must be in the range 0-255. Spread the node index across the third and fourth octets (42.42.{i >> 8}.{i & 0xff}) to support up to 65535 nodes with valid addresses. This is the same design as ovn-benchmark.py. Fixes: 4eefb6359203 ("tutorial: Add scripts to simulate node-port ovn-k8s services.") Assisted-by: Claude Sonnet 4.5, Claude Code Signed-off-by: Rosemarie O'Riorden <rosemarie@redhat.com> --- This patch was added in v2 and has not been changed in v3. --- tutorial/ovn-lb-benchmark.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)