Message-ID: <20201030155255.6599e46a@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Fri, 30 Oct 2020 15:52:55 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Motiejus Jakštys <desired.mta@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        linux-doc@...r.kernel.org, trivial@...nel.org
Subject: Re: [PATCH] Documentation: tproxy: more gentle intro

On Tue, 27 Oct 2020 14:06:20 +0200 Motiejus Jakštys wrote:
> Clarify tproxy odcumentation, so it's easier to read/understand without
> a-priori in-kernel transparent proxying knowledge:
> 
> - re-shuffle the sections, as the "router" section is easier to
>   understand when getting started.
> - add a link to HAProxy page. This is where I learned most about what
>   tproxy is, so I believe it is reasonable to include.
> - removed a reference to linux 2.2.
> 
> Plus Sphinx formatting/cosmetic changes.
> 
> Signed-off-by: Motiejus Jakštys <desired.mta@...il.com>
> ---
>  Documentation/networking/tproxy.rst | 155 +++++++++++++++-------------
>  1 file changed, 83 insertions(+), 72 deletions(-)
> 
> diff --git a/Documentation/networking/tproxy.rst b/Documentation/networking/tproxy.rst
> index 00dc3a1a66b4..0f43159046fb 100644
> --- a/Documentation/networking/tproxy.rst
> +++ b/Documentation/networking/tproxy.rst
> @@ -1,42 +1,77 @@
>  .. SPDX-License-Identifier: GPL-2.0
>  
> -=========================
> -Transparent proxy support
> -=========================
> +==========================
> +Transparent proxy (TPROXY)
> +==========================
>  
> -This feature adds Linux 2.2-like transparent proxy support to current kernels.
> -To use it, enable the socket match and the TPROXY target in your kernel config.
> -You will need policy routing too, so be sure to enable that as well.
> +TPROXY enables forwarding and intercepting packets that were destined
> +for other destination IPs, without using NAT chain or REDIRECT targets.

"destined for other destination" does not sound good.

Better say endpoint than IPs, IP is the name of a protocol.

> -From Linux 4.18 transparent proxy support is also available in nf_tables.
> +Redirecting traffic
> +===================
>  
> -1. Making non-local sockets work
> -================================
> +TPROXY is often used to "intercept" traffic on a router. This is usually done
> +with the iptables ``REDIRECT`` target, however, there are serious limitations:
> +it modifies the packets to change the destination address -- which might not be
> +acceptable in certain situations, e.g.:
> +- UDP: you won't be able to find out the original destination address.
> +- TCP: getting the original destination address is racy.

I don't think this rewrite of the examples helps. Also it doesn't
render right. Please leave the original wording.

> -The idea is that you identify packets with destination address matching a local
> -socket on your box, set the packet mark to a certain value::
> +The ``TPROXY`` target provides similar functionality without relying on NAT.
> +Simply add rules like this to the iptables ruleset above:

There are no rules "above" after the reordering.

> -    # iptables -t mangle -N DIVERT
> -    # iptables -t mangle -A PREROUTING -p tcp -m socket -j DIVERT
> -    # iptables -t mangle -A DIVERT -j MARK --set-mark 1
> -    # iptables -t mangle -A DIVERT -j ACCEPT
> +.. code-block:: sh

> +To use tproxy you'll need to have the following modules compiled for iptables:
>  
> -As an example implementation, tcprdr is available here:
> -https://git.breakpoint.cc/cgit/fw/tcprdr.git/
> -This tool is written by Florian Westphal and it was used for testing during the
> -nf_tables implementation.
> + - ``NETFILTER_XT_MATCH_SOCKET``
> + - ``NETFILTER_XT_TARGET_TPROXY``
>  
> -3. Iptables and nf_tables extensions
> -====================================
> +For nf_tables:
>  
> -To use tproxy you'll need to have the following modules compiled for iptables:
> + - ``NFT_TPROXY``
> + - ``NFT_SOCKET``

What happened to the mention of policy routing in the kernel support?

> - - NETFILTER_XT_MATCH_SOCKET
> - - NETFILTER_XT_TARGET_TPROXY
> +Application support
> +======================

> +HAproxy
> +-------
>  
> -Squid 3.HEAD has support built-in. To use it, pass
> -'--enable-linux-netfilter' to configure and set the 'tproxy' option on
> -the HTTP listener you redirect traffic to with the TPROXY iptables
> -target.
> +Documented in `Haproxy blog`_.

Can we add some words here, beyond just a link?

> -For more information please consult the following page on the Squid
> -wiki: http://wiki.squid-cache.org/Features/Tproxy4
> +.. _`Squid wiki`: http://wiki.squid-cache.org/Features/Tproxy4
> +.. _`HAproxy blog`: https://www.haproxy.com/blog/howto-transparent-proxying-and-binding-with-haproxy-and-aloha-load-balancer/

Overall I can see how the document can be hard to grasp, but I'm not
sure the reordering is an improvement. In the doc as is the first
section describes simple local receive of traffic not destined for
local host. Second describes TPROXY redirect. 

Perhaps their headings or content could be clarified but reorder
doesn't make much sense IMHO.

Powered by blists - more mailing lists