[PATCH v3] net: Remove low_thresh in ip defrag

[PATCH v3] net: Remove low_thresh in ip defrag

[Angus Chen]

As low_thresh has no work in fragment reassembles,mark it to be unused.
And Mark it deprecated in sysctl Document.

Signed-off-by: Angus Chen <angus.chen@jaguarmicro.com>
---

v2:
 Fix some spelling errors,and remove low_thresh from struct fqdir.
 suggested by Jakub Kicinski <kuba@kernel.org>.

v3:
  Change low_thresh to low_thresh_unused in struct fqdir.

 Documentation/networking/nf_conntrack-sysctl.rst |  1 +
 include/net/inet_frag.h                          |  5 ++++-
 net/ieee802154/6lowpan/reassembly.c              |  6 +++---
 net/ipv4/ip_fragment.c                           | 10 ++++------
 net/ipv6/netfilter/nf_conntrack_reasm.c          |  6 +++---
 net/ipv6/reassembly.c                            |  6 +++---
 6 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/Documentation/networking/nf_conntrack-sysctl.rst b/Documentation/networking/nf_conntrack-sysctl.rst
index 8b1045c3b59e..9ca356bc7217 100644
--- a/Documentation/networking/nf_conntrack-sysctl.rst
+++ b/Documentation/networking/nf_conntrack-sysctl.rst
@@ -55,6 +55,7 @@ nf_conntrack_frag6_high_thresh - INTEGER
 	nf_conntrack_frag6_low_thresh is reached.
 
 nf_conntrack_frag6_low_thresh - INTEGER
+	(Obsolete since linux-4.17)
 	default 196608
 
 	See nf_conntrack_frag6_low_thresh
diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index b23ddec3cd5c..6ef360e4b729 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -13,7 +13,10 @@
 struct fqdir {
 	/* sysctls */
 	long			high_thresh;
-	long			low_thresh;
+	/* low_thresh is unused since linux-4.17,
+	 * keep it just to avoid getting warning from ensure_safe_net_sysctl.
+	 */
+	long			low_thresh_unused;
 	int			timeout;
 	int			max_dist;
 	struct inet_frags	*f;
diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
index a91283d1e5bf..c46cbe001646 100644
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -374,8 +374,8 @@ static int __net_init lowpan_frags_ns_sysctl_register(struct net *net)
 	}
 
 	table[0].data	= &ieee802154_lowpan->fqdir->high_thresh;
-	table[0].extra1	= &ieee802154_lowpan->fqdir->low_thresh;
-	table[1].data	= &ieee802154_lowpan->fqdir->low_thresh;
+	table[0].extra1	= &ieee802154_lowpan->fqdir->low_thresh_unused;
+	table[1].data	= &ieee802154_lowpan->fqdir->low_thresh_unused;
 	table[1].extra2	= &ieee802154_lowpan->fqdir->high_thresh;
 	table[2].data	= &ieee802154_lowpan->fqdir->timeout;
 
@@ -451,7 +451,7 @@ static int __net_init lowpan_frags_init_net(struct net *net)
 		return res;
 
 	ieee802154_lowpan->fqdir->high_thresh = IPV6_FRAG_HIGH_THRESH;
-	ieee802154_lowpan->fqdir->low_thresh = IPV6_FRAG_LOW_THRESH;
+	ieee802154_lowpan->fqdir->low_thresh_unused = IPV6_FRAG_LOW_THRESH;
 	ieee802154_lowpan->fqdir->timeout = IPV6_FRAG_TIMEOUT;
 
 	res = lowpan_frags_ns_sysctl_register(net);
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 69c00ffdcf3e..9d72673b048b 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -609,8 +609,8 @@ static int __net_init ip4_frags_ns_ctl_register(struct net *net)
 
 	}
 	table[0].data	= &net->ipv4.fqdir->high_thresh;
-	table[0].extra1	= &net->ipv4.fqdir->low_thresh;
-	table[1].data	= &net->ipv4.fqdir->low_thresh;
+	table[0].extra1	= &net->ipv4.fqdir->low_thresh_unused;
+	table[1].data	= &net->ipv4.fqdir->low_thresh_unused;
 	table[1].extra2	= &net->ipv4.fqdir->high_thresh;
 	table[2].data	= &net->ipv4.fqdir->timeout;
 	table[3].data	= &net->ipv4.fqdir->max_dist;
@@ -674,12 +674,10 @@ static int __net_init ipv4_frags_init_net(struct net *net)
 	 * A 64K fragment consumes 129736 bytes (44*2944)+200
 	 * (1500 truesize == 2944, sizeof(struct ipq) == 200)
 	 *
-	 * We will commit 4MB at one time. Should we cross that limit
-	 * we will prune down to 3MB, making room for approx 8 big 64K
-	 * fragments 8x128k.
+	 * We will commit 4MB at one time. And mark low_thresh be unused.
 	 */
 	net->ipv4.fqdir->high_thresh = 4 * 1024 * 1024;
-	net->ipv4.fqdir->low_thresh  = 3 * 1024 * 1024;
+	net->ipv4.fqdir->low_thresh_unused  = 3 * 1024 * 1024;
 	/*
 	 * Important NOTE! Fragment queue must be destroyed before MSL expires.
 	 * RFC791 is wrong proposing to prolongate timer each fragment arrival
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index d13240f13607..013f16ecdcd1 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -82,10 +82,10 @@ static int nf_ct_frag6_sysctl_register(struct net *net)
 	nf_frag = nf_frag_pernet(net);
 
 	table[0].data	= &nf_frag->fqdir->timeout;
-	table[1].data	= &nf_frag->fqdir->low_thresh;
+	table[1].data	= &nf_frag->fqdir->low_thresh_unused;
 	table[1].extra2	= &nf_frag->fqdir->high_thresh;
 	table[2].data	= &nf_frag->fqdir->high_thresh;
-	table[2].extra1	= &nf_frag->fqdir->low_thresh;
+	table[2].extra1	= &nf_frag->fqdir->low_thresh_unused;
 
 	hdr = register_net_sysctl(net, "net/netfilter", table);
 	if (hdr == NULL)
@@ -500,7 +500,7 @@ static int nf_ct_net_init(struct net *net)
 		return res;
 
 	nf_frag->fqdir->high_thresh = IPV6_FRAG_HIGH_THRESH;
-	nf_frag->fqdir->low_thresh = IPV6_FRAG_LOW_THRESH;
+	nf_frag->fqdir->low_thresh_unused = IPV6_FRAG_LOW_THRESH;
 	nf_frag->fqdir->timeout = IPV6_FRAG_TIMEOUT;
 
 	res = nf_ct_frag6_sysctl_register(net);
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 5bc8a28e67f9..59b3fc8d9e6b 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -465,8 +465,8 @@ static int __net_init ip6_frags_ns_sysctl_register(struct net *net)
 
 	}
 	table[0].data	= &net->ipv6.fqdir->high_thresh;
-	table[0].extra1	= &net->ipv6.fqdir->low_thresh;
-	table[1].data	= &net->ipv6.fqdir->low_thresh;
+	table[0].extra1	= &net->ipv6.fqdir->low_thresh_unused;
+	table[1].data	= &net->ipv6.fqdir->low_thresh_unused;
 	table[1].extra2	= &net->ipv6.fqdir->high_thresh;
 	table[2].data	= &net->ipv6.fqdir->timeout;
 
@@ -536,7 +536,7 @@ static int __net_init ipv6_frags_init_net(struct net *net)
 		return res;
 
 	net->ipv6.fqdir->high_thresh = IPV6_FRAG_HIGH_THRESH;
-	net->ipv6.fqdir->low_thresh = IPV6_FRAG_LOW_THRESH;
+	net->ipv6.fqdir->low_thresh_unused = IPV6_FRAG_LOW_THRESH;
 	net->ipv6.fqdir->timeout = IPV6_FRAG_TIMEOUT;
 
 	res = ip6_frags_ns_sysctl_register(net);
-- 
2.25.1

Re: [PATCH v3] net: Remove low_thresh in ip defrag

[Jakub Kicinski]

On Wed, 17 May 2023 08:18:20 +0800 Angus Chen wrote:
> As low_thresh has no work in fragment reassembles,mark it to be unused.
> And Mark it deprecated in sysctl Document.
> 
> Signed-off-by: Angus Chen <angus.chen@jaguarmicro.com>

We need to revert the old patch first, we can't remove the commit from
the git history because it would change all later hashes and break
rebasing.

Why are you renaming the member? Just add the comment and update the
documentation. You said you had a tested complaint, the tester will
only read the docs, right?
-- 
pw-bot: cr

RE: [PATCH v3] net: Remove low_thresh in ip defrag

[Angus Chen]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, May 17, 2023 11:44 AM
> To: Angus Chen <angus.chen@jaguarmicro.com>
> Cc: davem@davemloft.net; dsahern@kernel.org; edumazet@google.com;
> pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> idosch@idosch.org; petrm@nvidia.com
> Subject: Re: [PATCH v3] net: Remove low_thresh in ip defrag
> 
> On Wed, 17 May 2023 08:18:20 +0800 Angus Chen wrote:
> > As low_thresh has no work in fragment reassembles,mark it to be unused.
> > And Mark it deprecated in sysctl Document.
> >
> > Signed-off-by: Angus Chen <angus.chen@jaguarmicro.com>
> 
> We need to revert the old patch first, we can't remove the commit from
> the git history because it would change all later hashes and break
> rebasing.
> 
> Why are you renaming the member? Just add the comment and update the
> documentation. You said you had a tested complaint, the tester will
> only read the docs, right?
Yes,I can be did like you and Ido Schimmel just said.
Btw, read the docs is also kind of complicated.
If we can introduce a place ,like /proc/deprecated or somewhere,
then the scripts can read it for realtime check.
After that, we put the deprecate proc entry with a list.
It maybe welcomed by DevOps Engineer also.
> --
> pw-bot: cr