[libvirt] [PATCH v2 03/17] vircgroup: introduce virCgroupV2DevicesAttachProg

[Daniel P. Berrangé]

On Mon, Jan 14, 2019 at 04:47:37PM +0100, Pavel Hrdina wrote:
> This function loads the BPF prog with prepared map into kernel and
> attaches it into guest cgroup.  It can be also used to replace existing
> program in the cgroup if we need to resize BPF map to store more rules
> for devices. The old program will be closed and removed from kernel.
> 
> There are two possible ways how to create BPF program:
> 
>     - One way is to write simple C-like code which can by compiled into
>       BPF object file which can be loaded into kernel using elfutils.
> 
>     - The second way is to define macros which looks like assembler
>       instructions and can be used directly to create BPF program that
>       can be directly loaded into kernel.
> 
> Since the program is not too complex we can use the second option.

I'm really not liking this to be honest. Even for this "simple" program,
I struggle to understand what the code below is doing. It is basically
assembly language, which is inevitably hard to follow even for simple
things.

I'd like to see us use the BPF C compiler to build the loaded
program unless there's a compelling reason why it won't work

> +
> +
> +static int
> +virCgroupV2DevicesLoadProg(int mapfd)
> +{
> +# define VIR_CGROUP_BPF_LOOKUP \
> +    /* prepare key param on stack */ \
> +    VIR_BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8), \
> +    VIR_BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), \
> +    VIR_BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), \
> +    /* lookup key (major << 32) | minor in map */ \
> +    VIR_BPF_LD_MAP_FD(BPF_REG_1, mapfd), \
> +    VIR_BPF_CALL_INSN(BPF_FUNC_map_lookup_elem)
> +
> +# define VIR_CGROUP_BPF_CHECK_PERM \
> +    /* if no key skip perm check */ \
> +    VIR_BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 6), \
> +    /* get perms from map */ \
> +    VIR_BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0), \
> +    /* get perms from ctx */ \
> +    VIR_BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_6, 0), \
> +    /* (map perms) & (ctx perms) */ \
> +    VIR_BPF_ALU64_REG(BPF_AND, BPF_REG_1, BPF_REG_2), \
> +    /* if (map perms) & (ctx perms) == (ctx perms) exit, otherwise continue */ \
> +    VIR_BPF_JMP_REG(BPF_JNE, BPF_REG_1, BPF_REG_2, 2), \
> +    /* set ret 1 and exit */ \
> +    VIR_BPF_MOV64_IMM(BPF_REG_0, 1), \
> +    VIR_BPF_EXIT_INSN()
> +
> +    struct bpf_insn prog[] = {
> +        /* save ctx, argument passed to BPF program */
> +        VIR_BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
> +
> +        /* get major from ctx and shift << 32 */
> +        VIR_BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_6, 4),
> +        VIR_BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 32),
> +        /* get minor from ctx and | to shifted major */
> +        VIR_BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_6, 8),
> +        VIR_BPF_ALU64_REG(BPF_OR, BPF_REG_2, BPF_REG_3),
> +        /* lookup ((major << 32) | minor) in map and check perms */
> +        VIR_CGROUP_BPF_LOOKUP,
> +        VIR_CGROUP_BPF_CHECK_PERM,
> +
> +        /* get major from ctx and shift << 32 */
> +        VIR_BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_6, 4),
> +        VIR_BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 32),
> +        /* use -1 as minor and | to shifted major */
> +        VIR_BPF_MOV32_IMM(BPF_REG_3, -1),
> +        VIR_BPF_ALU64_REG(BPF_OR, BPF_REG_2, BPF_REG_3),
> +        /* lookup ((major << 32) | -1) in map and check perms */
> +        VIR_CGROUP_BPF_LOOKUP,
> +        VIR_CGROUP_BPF_CHECK_PERM,
> +
> +        /* use -1 as major and shift << 32 */
> +        VIR_BPF_MOV32_IMM(BPF_REG_2, -1),
> +        VIR_BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 32),
> +        /* get minor from ctx and | to shifted major */
> +        VIR_BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_6, 8),
> +        VIR_BPF_ALU64_REG(BPF_OR, BPF_REG_2, BPF_REG_3),
> +        /* lookup ((-1 << 32) | minor) in map and check perms */
> +        VIR_CGROUP_BPF_LOOKUP,
> +        VIR_CGROUP_BPF_CHECK_PERM,
> +
> +        /* use -1 as key which means major = -1 and minor = -1 */
> +        VIR_BPF_MOV64_IMM(BPF_REG_2, -1),
> +        /* lookup -1 in map and check perms*/
> +        VIR_CGROUP_BPF_LOOKUP,
> +        VIR_CGROUP_BPF_CHECK_PERM,
> +
> +        /* no key was found, exit with 0 */
> +        VIR_BPF_MOV64_IMM(BPF_REG_0, 0),
> +        VIR_BPF_EXIT_INSN(),
> +    };
> +
> +    return virBPFLoadProg(prog, BPF_PROG_TYPE_CGROUP_DEVICE, ARRAY_CARDINALITY(prog));


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|