Re: [RFC PATCH v3 18/37] bpf tools: Record map accessing instructions for each program
On 2015/5/19 2:34, Alexei Starovoitov wrote: On 5/17/15 3:56 AM, Wang Nan wrote: This patch records the indics of instructions which are needed to be relocated. Those information are saved in 'reloc_desc' field in 'struct bpf_program'. In loading phase (this patch takes effect in opening phase), the collected instructions will be replaced by map loading instructions. Since we are going to close the ELF file and clear all data at the end of 'opening' phase, ELF information will no longer be valid in 'loading' phase. We have to locate the instructions before maps are loaded, instead of directly modifying the instruction. 'struct bpf_map_def' is introduce in this patch to let us know how many maps defined in the object. Signed-off-by: Wang Nan ... +/* + * packed attribute is unnecessary for 'bpf_map_def'. + */ +struct bpf_map_def { +unsigned int type; +unsigned int key_size; +unsigned int value_size; +unsigned int max_entries; +}; the comment looks out of place. 'packed' is necessary somewhere else? What were you concerned about here? I see a warning message: make: Entering directory `/home/w00229757/kernel-hydrogen/tools/lib/bpf' CC libbpf.o In file included from libbpf.c:23:0: libbpf.h:31:1: error: packed attribute is unnecessary for ‘bpf_map_def’ [-Werror=packed] } __attribute__((packed)); ^ cc1: all warnings being treated as errors if I append __attribute__((packed)) here. This is because Makefile.include in tools/script as '-Wpacked' warning message and it is inherited by perf. When compiling with perf this problem breaks compiling. Therefore I don't use 'packed' here. However I think there should be one. This comment is for that. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v3 18/37] bpf tools: Record map accessing instructions for each program
On 2015/5/19 2:34, Alexei Starovoitov wrote: On 5/17/15 3:56 AM, Wang Nan wrote: This patch records the indics of instructions which are needed to be relocated. Those information are saved in 'reloc_desc' field in 'struct bpf_program'. In loading phase (this patch takes effect in opening phase), the collected instructions will be replaced by map loading instructions. Since we are going to close the ELF file and clear all data at the end of 'opening' phase, ELF information will no longer be valid in 'loading' phase. We have to locate the instructions before maps are loaded, instead of directly modifying the instruction. 'struct bpf_map_def' is introduce in this patch to let us know how many maps defined in the object. Signed-off-by: Wang Nan wangn...@huawei.com ... +/* + * packed attribute is unnecessary for 'bpf_map_def'. + */ +struct bpf_map_def { +unsigned int type; +unsigned int key_size; +unsigned int value_size; +unsigned int max_entries; +}; the comment looks out of place. 'packed' is necessary somewhere else? What were you concerned about here? I see a warning message: make: Entering directory `/home/w00229757/kernel-hydrogen/tools/lib/bpf' CC libbpf.o In file included from libbpf.c:23:0: libbpf.h:31:1: error: packed attribute is unnecessary for ‘bpf_map_def’ [-Werror=packed] } __attribute__((packed)); ^ cc1: all warnings being treated as errors if I append __attribute__((packed)) here. This is because Makefile.include in tools/script as '-Wpacked' warning message and it is inherited by perf. When compiling with perf this problem breaks compiling. Therefore I don't use 'packed' here. However I think there should be one. This comment is for that. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v3 18/37] bpf tools: Record map accessing instructions for each program
On 5/17/15 3:56 AM, Wang Nan wrote: This patch records the indics of instructions which are needed to be relocated. Those information are saved in 'reloc_desc' field in 'struct bpf_program'. In loading phase (this patch takes effect in opening phase), the collected instructions will be replaced by map loading instructions. Since we are going to close the ELF file and clear all data at the end of 'opening' phase, ELF information will no longer be valid in 'loading' phase. We have to locate the instructions before maps are loaded, instead of directly modifying the instruction. 'struct bpf_map_def' is introduce in this patch to let us know how many maps defined in the object. Signed-off-by: Wang Nan ... +/* + * packed attribute is unnecessary for 'bpf_map_def'. + */ +struct bpf_map_def { + unsigned int type; + unsigned int key_size; + unsigned int value_size; + unsigned int max_entries; +}; the comment looks out of place. 'packed' is necessary somewhere else? What were you concerned about here? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v3 18/37] bpf tools: Record map accessing instructions for each program
On 5/17/15 3:56 AM, Wang Nan wrote: This patch records the indics of instructions which are needed to be relocated. Those information are saved in 'reloc_desc' field in 'struct bpf_program'. In loading phase (this patch takes effect in opening phase), the collected instructions will be replaced by map loading instructions. Since we are going to close the ELF file and clear all data at the end of 'opening' phase, ELF information will no longer be valid in 'loading' phase. We have to locate the instructions before maps are loaded, instead of directly modifying the instruction. 'struct bpf_map_def' is introduce in this patch to let us know how many maps defined in the object. Signed-off-by: Wang Nan wangn...@huawei.com ... +/* + * packed attribute is unnecessary for 'bpf_map_def'. + */ +struct bpf_map_def { + unsigned int type; + unsigned int key_size; + unsigned int value_size; + unsigned int max_entries; +}; the comment looks out of place. 'packed' is necessary somewhere else? What were you concerned about here? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/