Skip to content

ROX-28861: Make Berserker connection load more configurable#40

Merged
JoukoVirtanen merged 8 commits intomainfrom
jv-make-num-ports-per-address-configurable
Apr 9, 2025
Merged

ROX-28861: Make Berserker connection load more configurable#40
JoukoVirtanen merged 8 commits intomainfrom
jv-make-num-ports-per-address-configurable

Conversation

@JoukoVirtanen
Copy link
Contributor

@JoukoVirtanen JoukoVirtanen commented Apr 1, 2025

For the network load Berserker cycles through IP addresses and ports. Currently for every one hundred ports it increments the IP address once. This PR makes it so that the number of ports per IP address is not hard coded, but can be configured.

Also makes it so that the exact starting IP address can be set, instead of starting from x.x.0.2, and makes it so that the full range of IP addresses can be incremented through instead of just cycling through the last two octets.

Testing

Summary

The image produced by this PR was used in kube-burner. Stackrox was deployed in the same cluster and the network graph was checked. Connections between pods and external IPs generated by berserker were observed.

Details

Starting from the root of the berserker repository

Build and push the berserker image with a client and server.

make
cd scripts/network
docker build .

docker image ls | head
REPOSITORY                                                                  TAG                                                  IMAGE ID       CREATED         SIZE
<none>                                                                      <none>                                               a03934ab4c9e   2 minutes ago   410MB

docker tag a03934ab4c9e quay.io/jvirtane/berserker-connection-load-1.0-77-g80d8771450-2
docker push quay.io/jvirtane/berserker-connection-load-1.0-77-g80d8771450-2

The image was then used in a kube-burner configuration file. Here stackrox/stackrox#14790

A GKE cluster was created. Stackrox was deployed there.

Starting from the root of the stacrox/actions repository.

cd release/start-kube-burner

export KUBECONFIG=/home/jvirtane/scripts/artifact/kubeconfig
export KUBE_BURNER_CONFIG_DIR=/home/jvirtane/go/src/github.com/stackrox/backup/stackrox/scripts/release-tools/kube-burner-configs
export BENCHMARK_OPERATOR_DIR=/home/jvirtane/software/benchmark-operator

./start-kube-burner.sh

The network graph UI was checked and the following was seen.

Screenshot from 2025-04-01 13-28-55

For each deployment in the cluster-density-1 namespace about 50 connections to different external IPs was observed. This is what is expected as each pod makes 100 connections and for each IP there are two ports.

fn get_local_addr_port(
&self,
addr: Ipv4Address,
index: usize,
Copy link
Contributor Author

@JoukoVirtanen JoukoVirtanen Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usize can be u64 or u32 depending upon the system. In practice it is always u64. Setting it to u64 seems a bit clearer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I agree that it's clearer now that we have numerous additional casts elsewhere in the code base

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only new cast that we need as a result of this change is

let index = i as u64;

The other casts would have been needed anyway.

// 254 (a2 octet) * 254 (a3 octet) * ports_per_address (port)
// gives us maximum 6451600 connections that could be opened
let local_port = 49152 + (index % 100) as u16;
let local_port = 49152 + (index % ports_per_addr as u64) as u16;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I set ports_per_addr to u16, but I am always converting it to u64. Seems a bit strange.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a little strange, but upcasting at least means we can guarantee that we're not losing precision. It's probably fine as-is.

// 254 (a2 octet) * 254 (a3 octet) * ports_per_address (port)
// gives us maximum 6451600 connections that could be opened
let local_port = 49152 + (index % 100) as u16;
let local_port = 49152 + (index % ports_per_addr as u64) as u16;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a little strange, but upcasting at least means we can guarantee that we're not losing precision. It's probably fine as-is.

fn get_local_addr_port(
&self,
addr: Ipv4Address,
index: usize,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I agree that it's clearer now that we have numerous additional casts elsewhere in the code base

@JoukoVirtanen JoukoVirtanen force-pushed the jv-make-num-ports-per-address-configurable branch from 26eb849 to 80d8771 Compare April 1, 2025 17:05
@JoukoVirtanen JoukoVirtanen marked this pull request as ready for review April 1, 2025 20:34
/// Map socket index to a local port and address. The address octets are
/// incremented every conns_per_addr sockets, whithin this interval the local port
/// is incremented.
fn get_local_addr_port(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self was not used, so I removed it and moved this function outside of the class.

@JoukoVirtanen JoukoVirtanen changed the title Make num ports per address configurable ROX-28861: Make Berserker connection load more configurable Apr 7, 2025
@JoukoVirtanen JoukoVirtanen force-pushed the jv-make-num-ports-per-address-configurable branch from e0585d4 to 7171d1b Compare April 8, 2025 14:59
@JoukoVirtanen JoukoVirtanen merged commit 04b198a into main Apr 9, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants