LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Cole Dishington <Cole.Dishington@alliedtelesis.co.nz>
Cc: pablo@netfilter.org,
	Anthony Lineham <anthony.lineham@alliedtelesis.co.nz>,
	Scott Parlane <scott.parlane@alliedtelesis.co.nz>,
	Blair Steven <blair.steven@alliedtelesis.co.nz>,
	Jozsef Kadlecsik <kadlec@netfilter.org>,
	Florian Westphal <fw@strlen.de>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	"open list:NETFILTER" <netfilter-devel@vger.kernel.org>,
	"open list:NETFILTER" <coreteam@netfilter.org>,
	"open list:NETWORKING [GENERAL]" <netdev@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: netfilter: Add RFC-7597 Section 5.1 PSID support
Date: Mon, 26 Apr 2021 14:23:24 +0200	[thread overview]
Message-ID: <20210426122324.GA975@breakpoint.cc> (raw)
In-Reply-To: <20210422023506.4651-1-Cole.Dishington@alliedtelesis.co.nz>

Cole Dishington <Cole.Dishington@alliedtelesis.co.nz> wrote:
> This adds support for masquerading into a smaller subset of ports -
> defined by the PSID values from RFC-7597 Section 5.1. This is part of
> the support for MAP-E and Lightweight 4over6, which allows multiple
> devices to share an IPv4 address by splitting the L4 port / id into
> ranges.
> 
> Co-developed-by: Anthony Lineham <anthony.lineham@alliedtelesis.co.nz>
> Signed-off-by: Anthony Lineham <anthony.lineham@alliedtelesis.co.nz>
> Co-developed-by: Scott Parlane <scott.parlane@alliedtelesis.co.nz>
> Signed-off-by: Scott Parlane <scott.parlane@alliedtelesis.co.nz>
> Signed-off-by: Blair Steven <blair.steven@alliedtelesis.co.nz>
> Signed-off-by: Cole Dishington <Cole.Dishington@alliedtelesis.co.nz>
> ---
>  include/net/netfilter/nf_conntrack.h          |   2 +
>  .../netfilter/nf_conntrack_tuple_common.h     |   5 +
>  include/uapi/linux/netfilter/nf_nat.h         |   3 +-
>  net/netfilter/nf_nat_core.c                   | 101 ++++++++++++++++--
>  net/netfilter/nf_nat_ftp.c                    |  23 ++--
>  net/netfilter/nf_nat_helper.c                 |  15 ++-
>  6 files changed, 120 insertions(+), 29 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
> index 439379ca9ffa..d63d38aa7188 100644
> --- a/include/net/netfilter/nf_conntrack.h
> +++ b/include/net/netfilter/nf_conntrack.h
> @@ -92,6 +92,8 @@ struct nf_conn {
>  	/* If we were expected by an expectation, this will be it */
>  	struct nf_conn *master;
>  
> +	struct nf_nat_range2 *range;

Increasing nf_conn size should be avoided unless
absolutely neccessary.

> --- a/include/uapi/linux/netfilter/nf_conntrack_tuple_common.h
> +++ b/include/uapi/linux/netfilter/nf_conntrack_tuple_common.h
> @@ -39,6 +39,11 @@ union nf_conntrack_man_proto {
>  	struct {
>  		__be16 key;	/* GRE key is 32bit, PPtP only uses 16bit */
>  	} gre;
> +	struct {
> +		unsigned char psid_length;
> +		unsigned char offset;
> +		__be16 psid;
> +	} psid;

This breaks the ABI, you cannot change these structures.

This is the reason there is a 'struct nf_nat_range2', it wasn't
possible to add to the existing 'struct nf_nat_range'.

> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index b7c3c902290f..7730ce4ca9a9 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -232,13 +232,33 @@ static bool nf_nat_inet_in_range(const struct nf_conntrack_tuple *t,
>  static bool l4proto_in_range(const struct nf_conntrack_tuple *tuple,
>  			     enum nf_nat_manip_type maniptype,
>  			     const union nf_conntrack_man_proto *min,
> -			     const union nf_conntrack_man_proto *max)
> +			     const union nf_conntrack_man_proto *max,
> +			     bool is_psid)
>  {

...
>  	__be16 port;
>  
> +	int m = 0;
> +	u16 offset_mask = 0;
> +	u16 psid_mask = 0;
> +
> +	/* In this case we are in PSID mode and the rules are all different */
> +	if (is_psid) {
> +		/* m = number of bits in each valid range */
> +		m = 16 - min->psid.psid_length - min->psid.offset;
> +		offset_mask = ((1 << min->psid.offset) - 1) <<
> +				(16 - min->psid.offset);
> +		psid_mask = ((1 << min->psid.psid_length) - 1) << m;
> +	}

...

Is it really needed to place all of this in the nat core?

The only thing that has to be done in the NAT core, afaics, is to
suppress port reallocation attmepts when NF_NAT_RANGE_PSID is set.

Is there a reason why nf_nat_masquerade_ipv4/6 can't be changed instead
to do what you want?

AFAICS its enough to set NF_NAT_RANGE_PROTO_SPECIFIED and init the
upper/lower boundaries, i.e. change input given to nf_nat_setup_info().

>  	get_unique_tuple(&new_tuple, &curr_tuple, range, ct, maniptype);
> +	if (range) {
> +		if (!ct->range)
> +			ct->range = kmalloc(sizeof(*ct->range), 0);

If you absolutely have to store extra data in nf_conn, please extend
struct nf_conn_nat, masquerade already stores the interface index, so
you could place the psid len/offset there as well.

> +	/* Find a port that matches the MASQ rule. */
> +	nf_nat_l4proto_unique_tuple(&exp->tuple, ct->range,
> +				    dir ? NF_NAT_MANIP_SRC : NF_NAT_MANIP_DST,
> +				    ct);
> +	port = ntohs(exp->tuple.dst.u.tcp.port);
> +	nf_ct_expect_related(exp, 0);

This removes there error check for nf_ct_expect_related(), why?

Also, how is this going to be used?

I see no changes to any of the nftables or iptables modules that would
be needed for userspace to enable this feature.

  reply	other threads:[~2021-04-26 12:23 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-22  2:35 Cole Dishington
2021-04-26 12:23 ` Florian Westphal [this message]
2021-06-29  0:48   ` Cole Dishington
2021-06-30 14:20     ` Florian Westphal
     [not found]       ` <20210705040856.25191-1-Cole.Dishington@alliedtelesis.co.nz>
2021-07-05  4:08         ` [PATCH] net: netfilter: Add RFC-7597 Section 5.1 PSID support xtables API Cole Dishington
2021-07-05  4:08         ` [PATCH] net: netfilter: Add RFC-7597 Section 5.1 PSID support Cole Dishington
2021-07-05 10:39           ` Florian Westphal
2021-07-16  0:27             ` [PATCH 0/3] " Cole Dishington
2021-07-16  0:27               ` [PATCH 1/3] net: netfilter: Add RFC-7597 Section 5.1 PSID support xtables API Cole Dishington
2021-07-16  0:27               ` [PATCH 2/3] net: netfilter: Add RFC-7597 Section 5.1 PSID support Cole Dishington
2021-07-16 15:18                 ` Florian Westphal
2021-07-19  1:21                   ` Cole Dishington
2021-07-22  7:17                     ` Florian Westphal
2021-07-25 23:28                       ` Cole Dishington
2021-07-26 14:37                         ` Florian Westphal
2021-08-09  4:10                           ` [PATCH net-next 0/3] " Cole Dishington
2021-08-09  4:10                             ` [PATCH net-next 1/3] net: netfilter: Add RFC-7597 Section 5.1 PSID support xtables API Cole Dishington
2021-08-09  4:10                             ` [PATCH net-next 2/3] net: netfilter: Add RFC-7597 Section 5.1 PSID support Cole Dishington
2021-08-25 17:05                               ` Pablo Neira Ayuso
2021-08-29 21:30                                 ` Cole Dishington
2021-08-09  4:10                             ` [PATCH net-next 3/3] selftests: netfilter: Add RFC-7597 Section 5.1 PSID selftests Cole Dishington
2021-07-16  0:27               ` [PATCH " Cole Dishington
2021-07-05  4:08         ` [PATCH] " Cole Dishington

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210426122324.GA975@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=Cole.Dishington@alliedtelesis.co.nz \
    --cc=anthony.lineham@alliedtelesis.co.nz \
    --cc=blair.steven@alliedtelesis.co.nz \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=kadlec@netfilter.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=scott.parlane@alliedtelesis.co.nz \
    --subject='Re: [PATCH] net: netfilter: Add RFC-7597 Section 5.1 PSID support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).