Re: [PATCH] passt.1: Fix typo, improve wording in examples of port forwarding specifiers

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4844 bytes --]

On Thu, Feb 16, 2023 at 02:09:00AM +0100, Stefano Brivio wrote:
> Based on a patch from Laine, and reports from Laine and Yalan: fix
> the "22-80:32-90" example, and improve wording for the other ones:
> instead of using "to" to denote the end of a range, use "between ...
> and", so that it's clear we're *not* referring to target ports.
> 
> Reported-by: Laine Stump <laine@redhat.com>
> Reported-by: Yalan Zhang <yalzhang@redhat.com>
> Fixes: da20f57f19dc ("passt, qrap: Add man pages")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Although, a few additional refinements suggested below:

> ---
>  passt.1 | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/passt.1 b/passt.1
> index 528763b..ec09c22 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -338,7 +338,7 @@ Examples:
>  .RS
>  .TP
>  -t 22
> -Forward local port 22 to 22 on the guest
> +Forward local port 22 to port 22 on the guest
>  .TP
>  -t 22:23
>  Forward local port 22 to port 23 on the guest
> @@ -347,10 +347,11 @@ Forward local port 22 to port 23 on the guest
>  Forward local ports 22 and 25 to ports 22 and 25 on the guest
>  .TP
>  -t 22-80
> -Forward local ports 22 to 80 to corresponding ports on the guest
> +Forward local ports between 22 and 80 to corresponding ports on the guest
>  .TP
> --t 22-80-32:90
> -Forward local ports 22 to 80 to corresponding ports on the guest plus 10
> +-t 22-80:32-90
> +Forward local ports between 22 and 80 to corresponding ports on the guest, plus
> +10

I find it a bit hard to parse how the "plus 10" attaches to the rest
of the sentence.  I'd suggest:

	Forward local ports between 22 and 80 to ports between 32 and
	90 on the guest.

From a pedantic point of view that's less precise, but I think the
meaning of it going to be clearer in practice to most readers

>  .TP
>  -t 192.0.2.1/22
>  Forward local port 22, bound to 192.0.2.1, to port 22 on the guest
> @@ -359,10 +360,10 @@ Forward local port 22, bound to 192.0.2.1, to port 22 on the guest
>  Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22
>  .TP
>  -t 2000-5000,~3000-3010
> -Forward local ports 2000 to 5000, but not 3000 to 3010
> +Forward local ports between 2000 and 5000, but not between 3000 to 3010

I's suggest:
	Forward local ports between 2000 and 5000, except for those
	between 3000 and 3010.



>  .TP
>  -t 192.0.2.1/20-30,~25
> -Forward local ports 20 to 24, and 26 to 30, bound to 192.0.2.1
> +Forward local ports between 20 to 24, and between 26 to 30, bound to 192.0.2.1

Here you have both "between" and "to".

Maybe,
	For the local address 192.0.2.1, forward ports between 20 and
	24 and between 26 and 30.

>  .TP
>  -t ~20000-20010
>  Forward all ports to the guest, except for the range from 20000 to 20010
> @@ -425,12 +426,13 @@ Forward local port 22 to port 23 in the target namespace
>  Forward local ports 22 and 25 to ports 22 and 25 in the target namespace
>  .TP
>  -t 22-80
> -Forward local ports 22 to 80 to corresponding ports in the target namespace
> -.TP
> --t 22-80-32:90
> -Forward local ports 22 to 80 to corresponding ports plus 10 in the target
> +Forward local ports between 22 and 80 to corresponding ports in the target
>  namespace
>  .TP
> +-t 22-80:32-90
> +Forward local ports between 22 and 80 to corresponding ports, plus 10, in the
> +target namespace
> +.TP

Same suggestion regarding "plus 10" as above.

>  -t 192.0.2.1/22
>  Forward local port 22, bound to 192.0.2.1, to port 22 in the target namespace
>  .TP
> @@ -438,13 +440,14 @@ Forward local port 22, bound to 192.0.2.1, to port 22 in the target namespace
>  Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22
>  .TP
>  -t 2000-5000,~3000-3010
> -Forward local ports 2000 to 5000, but not 3000 to 3010
> +Forward local ports between 2000 and 5000, but not between 3000 to 3010

Same suggestion to use "except for"

>  .TP
>  -t 192.0.2.1/20-30,~25
> -Forward local ports 20 to 24, and 26 to 30, bound to 192.0.2.1
> +Forward local ports between 20 to 24, and between 26 to 30, bound to 192.0.2.1

And again.

>  .TP
>  -t ~20000-20010
> -Forward all ports to the namespace, except for the range from 20000 to 20010
> +Forward all ports to the target namespace, except for the range from 20000 to
> +20010

Suggest,
	Forward all ports to the namespace, except for those between
	20000 and 20010.

>  .RE
>  
>  IPv6 bound ports are also forwarded for IPv4.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]