Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RS-2320] Add QoS support in OpenStack code #9811

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

nelljerram
Copy link
Member

@nelljerram nelljerram commented Feb 7, 2025

Add QoS support in OpenStack code

This is the OpenStack-specific coding that matches up with #9744

Todos

Release Note

Calico for OpenStack now supports some Neutron QoS policy fields: the "max_kbps" and "max_burst_kbps" fields of bandwidth limit rules, and the "max_kpps" field of packet rate limit rules.  There are also new driver settings (cluster-wide) for imposing a maximum number of egress connections per Neutron port, and similarly for ingress connections.

@nelljerram nelljerram requested a review from a team as a code owner February 7, 2025 19:26
@marvin-tigera marvin-tigera added this to the Calico v3.30.0 milestone Feb 7, 2025
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Feb 7, 2025
Comment on lines +51 to +56
# Max connection options, in advance of max connection support being added
# properly to the Neutron API.
cfg.IntOpt('max_ingress_connections_per_port', default=0,
help="If non-zero, a maximum number of ingress connections to impose on each port."),
cfg.IntOpt('max_egress_connections_per_port', default=0,
help="If non-zero, a maximum number of egress connections to impose on each port."),
Copy link
Member

Choose a reason for hiding this comment

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

apologies for my lack of knowledge in this, but is there a need for a calico configuration (say, in FelixConfig) analogous to this? how exactly is this set by a user?

Copy link
Member Author

Choose a reason for hiding this comment

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

So this would be set in an /etc/neutron/neutron.conf file on the controller nodes, as:

...
[calico]
max_ingress_connections_per_port = 100
max_egress_connections_per_port = 100
...

I don't think it makes sense as a FelixConfig setting, because it's really just a stopgap for the Neutron DB currently missing these fields, and so the Neutron driver is the right place for temporary configuration to fill that gap.

If we think about a possible role for FelixConfig independently of OpenStack, then the question would be: given that we now support optional pod annotations for QoS, do we also want FelixConfig fields to set defaults for pods that don't have those annotations? It's certainly conceivable, but I haven't heard anyone saying that is actually required. If it was required, I assume it would be required equally for all of the possible QoS fields.

WDYT, does that all make sense?

Copy link
Member

Choose a reason for hiding this comment

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

I get it now, thanks @nelljerram! I agree it only makes sense on the neutron config, as far as I can tell there is no requirement to set defaults when unset. Thanks once again!

@nelljerram nelljerram merged commit 18cbece into projectcalico:master Feb 14, 2025
4 of 5 checks passed
@nelljerram nelljerram deleted the qos-openstack branch February 14, 2025 01:13
nelljerram added a commit to nelljerram/calico that referenced this pull request Feb 20, 2025
Sorry, these are more required plumbing changes that I missed in projectcalico#9811 and projectcalico#9856.

Also a key fix to query bandwidth limit and packet rate rules directly, instead of via the policy, as the policy object does not really have a 'rules' attribute.

After these, I've verified that I can execute the QoS workflow in https://docs.openstack.org/neutron/latest/admin/config-qos.html#user-workflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-pr-required Change is not yet documented release-note-required Change has user-facing impact (no matter how small)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants