On 4/10/24 7:29 PM, Brad Cowie wrote:
On Sat, 6 Apr 2024 at 09:01, Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
How about the other fields (flags and dir) in the "struct nf_conntrack_zone" and
would it be useful to have values other than the default?
Good question, it would probably be useful to make these configurable
as well. My reason for only adding ct zone id was to avoid changing
the size of bpf_ct_opts (NF_BPF_CT_OPTS_SZ).
I would be interested in some opinions here on if it's acceptable to
increase the size of bpf_ct_opts, if so, should I also add back some
reserved options to the struct for future use?
I think the reserved[2] was there for the padding reason.
It should be the first time there is a __sz increase. May be worth to explore
how it should work.
The opts_len check will need to check == old_size or == new_size. Only use the
new fields if it is new_size.
There is
enum {
NF_BPF_CT_OPTS_SZ = 12,
};
This enum probably needs to update with the new size also. NF_BPF_CT_OPTS_SZ
should be under CO-RE and its enum value will be updated with the running kernel.
The bpf prog has its own struct bpf_ct_opts during compilation (from vmlinux.h
or defined a local one), so may be the bpf prog can do something like this:
#include "vmlinux.h"
struct bpf_ct_opts___newer {
s32 netns_id;
s32 error;
u8 l4proto;
u8 dir;
u8 reserved[2];
u32 new_field; /* for example */
} __attribute__((preserve_access_index));
SEC("tc")
int run_in_older_kernel(struct __sk_buff *ctx)
{
struct bpf_ct_opts___newer opts = {};
/* min of the running kernel opts size or the
* local ___newer opts size
*/
bpf_skb_ct_lookup(ctx, &tup, sizeof(tup.ipv4), &opts,
min(NF_BPF_CT_OPTS_SZ, sizeof(opts));
}
Can it actually test an alloc and lookup of a non default zone id?
Yes, I have a test written now and will include this in my v2 submission.
Please also separate the selftest into another patch.
Will do.
