lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 01 May 2014 19:39:49 +0200 From: Daniel Borkmann <dborkman@...hat.com> To: Eric Dumazet <eric.dumazet@...il.com> CC: davem@...emloft.net, ast@...mgrid.com, netdev@...r.kernel.org Subject: Re: [PATCH net-next v3 1/3] net: filter: simplify label names from jump-table On 05/01/2014 07:20 PM, Eric Dumazet wrote: > On Thu, 2014-05-01 at 18:16 +0200, Daniel Borkmann wrote: >> This patch simplifies label naming for the BPF jump-table. >> When we define labels via DL(), we just concatenate/textify >> the combination of instruction opcode which consists of the >> class, subclass, word size, target register and so on. Each >> time we leave BPF_ prefix intact, so that e.g. the preprocessor >> generates a label BPF_ALU_BPF_ADD_BPF_X for DL(BPF_ALU, BPF_ADD, >> BPF_X) whereas a label name of ALU_ADD_X is much more easy >> to grasp. Pure cleanup only. >> >> Signed-off-by: Daniel Borkmann <dborkman@...hat.com> >> Acked-by: Alexei Starovoitov <ast@...mgrid.com> >> --- > > This exactly makes harder to use grep and future code reviews. > > # git grep -n BPF_XADD > include/linux/filter.h:21:#define BPF_XADD 0xc0 /* exclusive add */ > net/core/filter.c:224: DL(BPF_STX, BPF_XADD, BPF_W), > net/core/filter.c:225: DL(BPF_STX, BPF_XADD, BPF_DW), > net/core/filter.c:481: BPF_STX_BPF_XADD_BPF_W: /* lock xadd *(u32 *)(A + insn->off) += X */ > net/core/filter.c:485: BPF_STX_BPF_XADD_BPF_DW: /* lock xadd *(u64 *)(A + insn->off) += X */ > > Compare now with : > > # git grep -n XADD > > So I am quite against this patch. Well, if you review the code itself in filter.c it makes it hard to read and review, with this patch, you'll immediately get the label name and what it actually does, so I think it's quite convenient and way more readable by itself. For grepping, you can always add subdirectories you're searching for, besides that I don't think that everything in the kernel needs to have unique names only so that one can grep for it among the whole tree. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists