-
Notifications
You must be signed in to change notification settings - Fork 3.4k
srv6: flow label control #35498
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
srv6: flow label control #35498
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for your contribution! A couple of comments.
975a975
to
332fe72
Compare
332fe72
to
35ea7cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the change looks good to me. Thanks!
/test |
35ea7cf
to
fc6834a
Compare
Going back to 16lsb implementation of flow label as per advice from Ahmed based on the ietf draft and intrustry best practices |
@ahsalam As I said out of band, I don't have any objection against this change, but could you elaborate more about the context here (for example, the link to the draft or vendor's docuemnt)? We need to leave the clear context since it's OSS. If we say only using the 16bit is the best practice of the industry, we need to show the reasonable evidence. Is this the one? https://www.ietf.org/archive/id/draft-filsfils-6man-structured-flow-label-02.html |
Hi @YutaroHayakawa @akaliwod Yes, this is the draft. This document defines the IPv6 Structured Flow Label. The document demonstrated the seamless change to [RFC6437] and explained the benefits of the solution and illustrated the use-cases. Instead of using FL as a single 20-bit entropy structure, this draft updates [[RFC6437] and defines the 20-bit FL field as a structure of two fields:
The draft explained the Seamless Migration from RFC6437. Also Cisco and Broadcom report that the norm for their products:
|
Thanks for your explanation! I think we don't have any risk for following that convention by default. 16bits still provides an enough ECMP/RSS entropy and we can always introduce a flag to switch the behavior if needed. |
/test |
Looks like the ci-e2e-upgrade is failing pretty consistently. Let me check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider this a bugfix and would thus apply label release-note/bug
and backport. @YutaroHayakawa What do you think?
@akaliwod Rebasing against the latest main branch might help improve CI status. Your branch is currently one week old, which in Cilium's case is quite old. |
Yep, agree. I put the label. |
This commit introduced IPv6 flow label control for SRv6 encapsulated packets. Currently IPv6 flow label is set to 0x0 value. IPv6 flow label is used to perform ECMP load-balancing in the core of the network. Setting flow label provides entropy and allows packet flows to be correctly load balanced. The commit uses bfp_get_hash_recalc helper function to populate ctx->hash field and uses the result of helper to set flow label based on 16 lsb of the hash. There is no configurable flag to control this behavior. Additional change in bpf/lib/common.h to use get_hash(ctx) helper function instead of get_hash_recalc. Working assumption is that hash calculation is performed once. Signed-off-by: Arkadiusz Kaliwoda (akaliwod) <akaliwod@cisco.com>
fc6834a
to
4057d61
Compare
/test |
This commit introduced IPv6 flow label control for SRv6 encapsulated packets. Currently IPv6 flow label is set to 0x0 value.
IPv6 flow label is used to perform ECMP load-balancing in the core of the network. Setting flow label provides entropy and allows packet flows to be correctly load balanced.
The commit introduces configurable flag that keeps the default behavior of flow label set to 0x0 value. As on option it enables TCP/UDP port based basic calculation of the flow label value that is set in the IPv6 header in case of SRv6 packet flow.
Fixes: #21961