bpf: Refactor meta, ipsec/hostdev_ingress#10766
Conversation
|
test-me-please |
| __u32 magic = ctx_load_meta(ctx, 0); | ||
|
|
||
| if (magic == MARK_MAGIC_TO_PROXY) { | ||
| if ((magic & MARK_MAGIC_HOST_MASK) == MARK_MAGIC_ENCRYPT) { |
There was a problem hiding this comment.
@jrfastab would we be safe to also use 0xFFFF to mask here? At the moment it's only masking on 0x0F00 so any 0xXEXX value would match.
I refrained from making any changes like this in this PR since it's intended only as a cleanup, no functional changes.
There was a problem hiding this comment.
I think this should be OK I can't think of any reason why this wouldn't work.
There was a problem hiding this comment.
followup question for extra points, how confident are you that the CI would pick up a regression if I made this change? ;-)
| bpf_load $HOST_DEV1 "" "ingress" bpf_hostdev_ingress.c bpf_hostdev_ingress.o to-host $CALLS_MAP | ||
| # bpf_ipsec.o is also needed by proxy redirects, so we load it unconditionally | ||
| bpf_load $HOST_DEV2 "" "ingress" bpf_ipsec.c bpf_ipsec.o from-netdev $CALLS_MAP | ||
| bpf_load $HOST_DEV2 "" "ingress" bpf_hostdev_ingress.c bpf_hostdev_ingress.o to-host $CALLS_MAP |
There was a problem hiding this comment.
We can follow up to avoid the double-compile here (it's already compiled twice in master today).
qmonnet
left a comment
There was a problem hiding this comment.
Looks good, one nit on patch 2. For patch 1, what about updating set_identity_cb() and set_encrypt_key_cb() as well?
4416490 to
0cca91a
Compare
0cca91a to
38aa4db
Compare
|
test-me-please |
|
@qmonnet New patch in the middle for your suggestions on identity/encrypt. I patched up the other nit as well. |
|
Full green CI, just need to rebase. |
The rest of the functions that interact with the skb->cb refer to it as 'meta', rename this function to be consistent so it's clear that they're changing the same fields. Signed-off-by: Joe Stringer <joe@cilium.io>
* set_identity() -> set_identity_mark() * set_identity_cb() -> set_identity_meta() * set_encrypt_key() -> set_encrypt_key_mark() * set_encrypt_key_cb() -> set_encrypt_key_meta() Signed-off-by: Joe Stringer <joe@cilium.io>
bpf_ipsec.c had a superset of the functionality, handling encryption as well as proxying, but bpf_hostdev_ingress.c had the better name. Combine the two for great justice. Signed-off-by: Joe Stringer <joe@cilium.io>
38aa4db to
794c03f
Compare
|
test-me-please |
|
test-me-please |
A couple of non-functional datapath changes:
bpf_clear_cb()->bpf_clear_meta()to be consistent with other meta (cb) functions