On 17/06/2019 20:59, Jiong Wang wrote: > Edward Cree writes: > >> On 14/06/2019 16:13, Jiong Wang wrote: >>> Just an update and keep people posted. >>> >>> Working on linked list based approach, the implementation looks like the >>> following, mostly a combine of discussions happened and Naveen's patch, >>> please feel free to comment. >>> >>> - Use the reserved opcode 0xf0 with BPF_ALU as new pseudo insn code >>> BPF_LIST_INSN. (0xf0 is also used with BPF_JMP class for tail call). >>> >>> - Introduce patch pool into bpf_prog->aux to keep all patched insns. >> It's not clear to me what the point of the patch pool is, rather than just >> doing the patch straight away. > I used pool because I was thinking insn to be patched could be high > percentage, so doing lots of alloc call is going to be less efficient? so > allocate a big pool, and each time when creating new patch node, allocate > it from the pool directly. Node is addressed using pool_base + offset, each > node only need to keep offset. Good idea; but in that case it doesn't need to be a pool of patches (storing their prev and next), just a pool of insns. I.e. struct bpf_insn pool[many]; then in orig prog when patching an insn replace it with BPF_LIST_INSN. If we later decide to patch an insn within a patch, we can replace it (i.e. the entry in bpf_insn_pool) with another BPF_LIST_INSN pointing to some later bit of the pool, then we just have a little bit of recursion at linearise time. Does that work? -Ed
