On 08/01/2018 07:09 AM, Tobin C. Harding wrote:
> There are a few minor typos and grammatical issues. We should however
> try to keep the current flavour of the document.
>
> Fix typos and grammar if glaringly required.
>
> Signed-off-by: Tobin C. Harding <me@xxxxxxxx>
Overall looks good, just some minor nits:
> Documentation/networking/filter.rst | 65 +++++++++++++++--------------
> 1 file changed, 33 insertions(+), 32 deletions(-)
>
> diff --git a/Documentation/networking/filter.rst b/Documentation/networking/filter.rst
> index 99dfa74fc4f7..b989a6c882b8 100644
> --- a/Documentation/networking/filter.rst
> +++ b/Documentation/networking/filter.rst
> @@ -32,10 +32,10 @@ removing the old one and placing your new one in its place, assuming your
> filter has passed the checks, otherwise if it fails the old filter will
> remain on that socket.
>
> -SO_LOCK_FILTER option allows to lock the filter attached to a socket. Once
> -set, a filter cannot be removed or changed. This allows one process to
> +SO_LOCK_FILTER option allows locking of the filter attached to a socket.
> +Once set, a filter cannot be removed or changed. This allows one process to
> setup a socket, attach a filter, lock it then drop privileges and be
> -assured that the filter will be kept until the socket is closed.
> +assured that the filter will be kept until the socket is closed.
^-- looks like extra whitespace slipped in?
> The biggest user of this construct might be libpcap. Issuing a high-level
> filter command like ``tcpdump -i em1 port 22`` passes through the libpcap
> @@ -470,7 +470,7 @@ JIT compiler
> ============
>
> The Linux kernel has a built-in BPF JIT compiler for x86_64, SPARC, PowerPC,
> -ARM, ARM64, MIPS and s390 and can be enabled through CONFIG_BPF_JIT. The JIT
> +ARM, ARM64, MIPS and s390 which can be enabled through CONFIG_BPF_JIT. The JIT
> compiler is transparently invoked for each attached filter from user space
> or for internal kernel users if it has been previously enabled by root::
>
> @@ -580,7 +580,7 @@ Internally, for the kernel interpreter, a different instruction set
> format with similar underlying principles from BPF described in previous
> paragraphs is being used. However, the instruction set format is modelled
> closer to the underlying architecture to mimic native instruction sets, so
> -that a better performance can be achieved (more details later). This new
> +that better performance can be achieved (more details later). This new
> ISA is called 'eBPF' or 'internal BPF' interchangeably. (Note: eBPF which
> originates from [e]xtended BPF is not the same as BPF extensions! While
> eBPF is an ISA, BPF extensions date back to classic BPF's 'overloading'
> @@ -655,12 +655,12 @@ Some core changes of the new internal format:
>
> 32-bit architectures run 64-bit internal BPF programs via interpreter.
> Their JITs may convert BPF programs that only use 32-bit subregisters into
> - native instruction set and let the rest being interpreted.
> + native instruction set and let the rest be interpreted.
>
> - Operation is 64-bit, because on 64-bit architectures, pointers are also
> - 64-bit wide, and we want to pass 64-bit values in/out of kernel functions,
> - so 32-bit eBPF registers would otherwise require to define register-pair
> - ABI, thus, there won't be able to use a direct eBPF register to HW register
> + Operation is 64-bit since on 64-bit architectures pointers are also
> + 64-bit wide and we want to pass 64-bit values in/out of kernel functions.
> + 32-bit eBPF registers would otherwise require us to define a register-pair
> + ABI, thus we would not be able to use a direct eBPF register to HW register
> mapping and JIT would need to do combine/split/move operations for every
> register in and out of the function, which is complex, bug prone and slow.
> Another reason is the use of atomic 64-bit counters.
> @@ -694,7 +694,7 @@ Some core changes of the new internal format:
> situations without performance penalty.
>
> After an in-kernel function call, R1 - R5 are reset to unreadable and R0 has
> - a return value of the function. Since R6 - R9 are callee saved, their state
> + the return value of the function. Since R6 - R9 are callee saved, their state
> is preserved across the call.
>
> For example, consider three C functions::
> @@ -732,7 +732,7 @@ Some core changes of the new internal format:
> are currently not supported, but these restrictions can be lifted if necessary
> in the future.
>
> - On 64-bit architectures all register map to HW registers one to one. For
> + On 64-bit architectures all registers map to HW registers one to one. For
> example, x86_64 JIT compiler can map them as ... ::
>
> R0 - rax
> @@ -831,9 +831,10 @@ A program, that is translated internally consists of the following elements::
>
> op:16, jt:8, jf:8, k:32 ==> op:8, dst_reg:4, src_reg:4, off:16, imm:32
>
> -So far 87 internal BPF instructions were implemented. 8-bit ``op`` opcode field
> -has room for new instructions. Some of them may use 16/24/32 byte encoding. New
> -instructions must be multiple of 8 bytes to preserve backward compatibility.
> +So far 87 internal BPF instructions have been implemented. 8-bit ``op``
> +opcode field has room for new instructions. Some of them may use 16/24/32
> +byte encoding. New instructions must be a multiple of 8 bytes to preserve
> +backward compatibility.
>
> Internal BPF is a general purpose RISC instruction set. Not every register and
> every instruction are used during translation from original BPF to new format.
> @@ -844,11 +845,11 @@ out of registers and would have to resort to spill/fill to stack.
>
> Internal BPF can used as generic assembler for last step performance
> optimizations, socket filters and seccomp are using it as assembler. Tracing
> -filters may use it as assembler to generate code from kernel. In kernel usage
> +filters may use it as assembler to generate code from kernel. In-kernel usage
^-- ditto
> may not be bounded by security considerations, since generated internal BPF code
> -may be optimizing internal code path and not being exposed to the user space.
> -Safety of internal BPF can come from a verifier (TBD). In such use cases as
> -described, it may be used as safe instruction set.
> +may use an optimised internal code path and may not be being exposed to user
> +space. Safety of internal BPF can come from a verifier (TBD). In such use cases
> +as described, it may be used as safe as the instruction set.
>
> Just like the original BPF, the new format runs within a controlled environment,
> is deterministic and the kernel can easily prove that. The safety of the program
> @@ -945,7 +946,7 @@ Classic BPF is using BPF_MISC class to represent A = X and X = A moves.
> eBPF is using BPF_MOV | BPF_X | BPF_ALU code instead. Since there are no
> BPF_MISC operations in eBPF, the class 7 is used as BPF_ALU64 to mean
> exactly the same operations as BPF_ALU, but with 64-bit wide operands
> -instead. So BPF_ADD | BPF_X | BPF_ALU64 means 64-bit addition, i.e.:
> +instead. So BPF_ADD | BPF_X | BPF_ALU64 means 64-bit addition i.e.
> dst_reg = dst_reg + src_reg
>
> Classic BPF wastes the whole BPF_RET class to represent a single 'ret'
> @@ -1024,8 +1025,8 @@ Where size is one of: BPF_B or BPF_H or BPF_W or BPF_DW. Note that 1 and
> 2 byte atomic increments are not supported.
>
> eBPF has one 16-byte instruction: BPF_LD | BPF_DW | BPF_IMM which consists
> -of two consecutive ``struct bpf_insn`` 8-byte blocks and interpreted as single
> -instruction that loads 64-bit immediate value into a dst_reg.
> +of two consecutive ``struct bpf_insn`` 8-byte blocks and is interpreted as
> +a single instruction that loads 64-bit immediate value into a dst_reg.
> Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
> 32-bit immediate value into a register.
>
> @@ -1035,8 +1036,8 @@ eBPF verifier
> The safety of the eBPF program is determined in two steps.
>
> First step does DAG check to disallow loops and other CFG validation.
> -In particular it will detect programs that have unreachable instructions.
> -(though classic BPF checker allows them)
> +In particular it will detect programs that have unreachable instructions
> +(though classic BPF checker allows them).
>
> Second step starts from the first insn and descends all possible paths.
> It simulates execution of every insn and observes the state change of
> @@ -1107,7 +1108,7 @@ For example::
> bpf_ld R0 = *(u32 *)(R10 - 4)
> bpf_exit
>
> -is invalid program.
> +is an invalid program.
> Though R10 is correct read-only register and has type PTR_TO_STACK
> and R10 - 4 is within stack bounds, there were no stores into that location.
>
> @@ -1118,13 +1119,13 @@ Allowed function calls are customized with bpf_verifier_ops->get_func_proto()
> The eBPF verifier will check that registers match argument constraints.
> After the call register R0 will be set to return type of the function.
>
> -Function calls is a main mechanism to extend functionality of eBPF programs.
> -Socket filters may let programs to call one set of functions, whereas tracing
> -filters may allow completely different set.
> +Function calls is an important mechanism to extend functionality of eBPF
> +programs. Socket filters may let programs call one set of functions,
^-- ditto
> +whereas tracing filters may allow a completely different set.
>
> -If a function made accessible to eBPF program, it needs to be thought through
> -from safety point of view. The verifier will guarantee that the function is
> -called with valid arguments.
> +If a function is made accessible to eBPF program, it needs to be thought
> +through from a safety point of view. The verifier will guarantee that the
> +function is called with valid arguments.
>
> seccomp vs socket filters have different security restrictions for classic BPF.
> Seccomp solves this by two stage verifier: classic BPF verifier is followed
> @@ -1202,7 +1203,7 @@ checked and found to be non-NULL, all copies can become PTR_TO_MAP_VALUEs.
> As well as range-checking, the tracked information is also used for enforcing
> alignment of pointer accesses. For instance, on most systems the packet pointer
> is 2 bytes after a 4-byte alignment. If a program adds 14 bytes to that to jump
> -over the Ethernet header, then reads IHL and addes (IHL * 4), the resulting
> +over the Ethernet header, then reads IHL and adds (IHL * 4), the resulting
> pointer will have a variable offset known to be 4n+2 for some n, so adding the 2
> bytes (NET_IP_ALIGN) gives a 4-byte alignment and so word-sized accesses through
> that pointer are safe.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
