* [PATCH net-next v2] net-timestamp: namespacify the sysctl_tstamp_allow_data
@ 2024-10-02 4:18 Jason Xing
2024-10-02 7:41 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Jason Xing @ 2024-10-02 4:18 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, willemdebruijn.kernel, willemb
Cc: netdev, Jason Xing
From: Jason Xing <kernelxing@tencent.com>
Let it be tuned in per netns by admins.
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
v2
Link: https://lore.kernel.org/all/66fa81b2ddf10_17948d294bb@willemb.c.googlers.com.notmuch/
1. remove the static global from sock.c
2. reorder the tests
3. I removed the patch [1/3] because I made one mistake
4. I also removed the patch [2/3] because Willem soon will propose a
packetdrill test that is better.
Now, I only need to write this standalone patch.
---
include/net/netns/core.h | 1 +
include/net/sock.h | 2 --
net/core/net_namespace.c | 1 +
net/core/skbuff.c | 2 +-
net/core/sock.c | 2 --
net/core/sysctl_net_core.c | 18 +++++++++---------
6 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/include/net/netns/core.h b/include/net/netns/core.h
index 78214f1b43a2..ef8b3105c632 100644
--- a/include/net/netns/core.h
+++ b/include/net/netns/core.h
@@ -23,6 +23,7 @@ struct netns_core {
#if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL)
struct cpumask *rps_default_mask;
#endif
+ int sysctl_tstamp_allow_data;
};
#endif
diff --git a/include/net/sock.h b/include/net/sock.h
index c58ca8dd561b..4f31be0fd671 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2808,8 +2808,6 @@ void sk_get_meminfo(const struct sock *sk, u32 *meminfo);
extern __u32 sysctl_wmem_max;
extern __u32 sysctl_rmem_max;
-extern int sysctl_tstamp_allow_data;
-
extern __u32 sysctl_wmem_default;
extern __u32 sysctl_rmem_default;
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index e39479f1c9a4..e78c01912c64 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -317,6 +317,7 @@ static __net_init void preinit_net_sysctl(struct net *net)
*/
net->core.sysctl_optmem_max = 128 * 1024;
net->core.sysctl_txrehash = SOCK_TXREHASH_ENABLED;
+ net->core.sysctl_tstamp_allow_data = 1;
}
/* init code that must occur even if setup_net() is not called. */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 74149dc4ee31..00afeb90c23a 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5506,7 +5506,7 @@ static bool skb_may_tx_timestamp(struct sock *sk, bool tsonly)
{
bool ret;
- if (likely(READ_ONCE(sysctl_tstamp_allow_data) || tsonly))
+ if (likely(tsonly || READ_ONCE(sock_net(sk)->core.sysctl_tstamp_allow_data)))
return true;
read_lock_bh(&sk->sk_callback_lock);
diff --git a/net/core/sock.c b/net/core/sock.c
index fe87f9bd8f16..93b6c1d0317d 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -286,8 +286,6 @@ EXPORT_SYMBOL(sysctl_rmem_max);
__u32 sysctl_wmem_default __read_mostly = SK_WMEM_MAX;
__u32 sysctl_rmem_default __read_mostly = SK_RMEM_MAX;
-int sysctl_tstamp_allow_data __read_mostly = 1;
-
DEFINE_STATIC_KEY_FALSE(memalloc_socks_key);
EXPORT_SYMBOL_GPL(memalloc_socks_key);
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 86a2476678c4..83622799eb80 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -491,15 +491,6 @@ static struct ctl_table net_core_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
- {
- .procname = "tstamp_allow_data",
- .data = &sysctl_tstamp_allow_data,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ZERO,
- .extra2 = SYSCTL_ONE
- },
#ifdef CONFIG_RPS
{
.procname = "rps_sock_flow_entries",
@@ -665,6 +656,15 @@ static struct ctl_table netns_core_table[] = {
.extra2 = SYSCTL_ONE,
.proc_handler = proc_dou8vec_minmax,
},
+ {
+ .procname = "tstamp_allow_data",
+ .data = &init_net.core.sysctl_tstamp_allow_data,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE
+ },
/* sysctl_core_net_init() will set the values after this
* to readonly in network namespaces
*/
--
2.37.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2] net-timestamp: namespacify the sysctl_tstamp_allow_data
2024-10-02 4:18 [PATCH net-next v2] net-timestamp: namespacify the sysctl_tstamp_allow_data Jason Xing
@ 2024-10-02 7:41 ` Eric Dumazet
2024-10-02 10:26 ` Jason Xing
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2024-10-02 7:41 UTC (permalink / raw)
To: Jason Xing
Cc: davem, kuba, pabeni, willemdebruijn.kernel, willemb, netdev,
Jason Xing
On Wed, Oct 2, 2024 at 6:18 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> Let it be tuned in per netns by admins.
>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> v2
> Link: https://lore.kernel.org/all/66fa81b2ddf10_17948d294bb@willemb.c.googlers.com.notmuch/
> 1. remove the static global from sock.c
> 2. reorder the tests
> 3. I removed the patch [1/3] because I made one mistake
> 4. I also removed the patch [2/3] because Willem soon will propose a
> packetdrill test that is better.
> Now, I only need to write this standalone patch.
> ---
> include/net/netns/core.h | 1 +
> include/net/sock.h | 2 --
> net/core/net_namespace.c | 1 +
> net/core/skbuff.c | 2 +-
> net/core/sock.c | 2 --
> net/core/sysctl_net_core.c | 18 +++++++++---------
> 6 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/include/net/netns/core.h b/include/net/netns/core.h
> index 78214f1b43a2..ef8b3105c632 100644
> --- a/include/net/netns/core.h
> +++ b/include/net/netns/core.h
> @@ -23,6 +23,7 @@ struct netns_core {
> #if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL)
> struct cpumask *rps_default_mask;
> #endif
> + int sysctl_tstamp_allow_data;
> };
This adds another hole for no good reason.
Please put this after sysctl_txrehash.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2] net-timestamp: namespacify the sysctl_tstamp_allow_data
2024-10-02 7:41 ` Eric Dumazet
@ 2024-10-02 10:26 ` Jason Xing
2024-10-02 10:29 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Jason Xing @ 2024-10-02 10:26 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, willemdebruijn.kernel, willemb, netdev,
Jason Xing
Hello Eric,
On Wed, Oct 2, 2024 at 4:41 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Oct 2, 2024 at 6:18 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Let it be tuned in per netns by admins.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> > v2
> > Link: https://lore.kernel.org/all/66fa81b2ddf10_17948d294bb@willemb.c.googlers.com.notmuch/
> > 1. remove the static global from sock.c
> > 2. reorder the tests
> > 3. I removed the patch [1/3] because I made one mistake
> > 4. I also removed the patch [2/3] because Willem soon will propose a
> > packetdrill test that is better.
> > Now, I only need to write this standalone patch.
> > ---
> > include/net/netns/core.h | 1 +
> > include/net/sock.h | 2 --
> > net/core/net_namespace.c | 1 +
> > net/core/skbuff.c | 2 +-
> > net/core/sock.c | 2 --
> > net/core/sysctl_net_core.c | 18 +++++++++---------
> > 6 files changed, 12 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/net/netns/core.h b/include/net/netns/core.h
> > index 78214f1b43a2..ef8b3105c632 100644
> > --- a/include/net/netns/core.h
> > +++ b/include/net/netns/core.h
> > @@ -23,6 +23,7 @@ struct netns_core {
> > #if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL)
> > struct cpumask *rps_default_mask;
> > #endif
> > + int sysctl_tstamp_allow_data;
> > };
>
> This adds another hole for no good reason.
> Please put this after sysctl_txrehash.
Thanks for your reminder.
Before this patch:
struct netns_core {
struct ctl_table_header * sysctl_hdr; /* 0 0x8 */
int sysctl_somaxconn; /* 0x8 0x4 */
int sysctl_optmem_max; /* 0xc 0x4 */
u8 sysctl_txrehash; /* 0x10 0x1 */
/* XXX 7 bytes hole, try to pack */
struct prot_inuse * prot_inuse; /* 0x18 0x8 */
struct cpumask * rps_default_mask; /* 0x20 0x8 */
/* size: 40, cachelines: 1, members: 6 */
/* sum members: 33, holes: 1, sum holes: 7 */
/* last cacheline: 40 bytes */
};
After this patch:
struct netns_core {
struct ctl_table_header * sysctl_hdr; /* 0 0x8 */
int sysctl_somaxconn; /* 0x8 0x4 */
int sysctl_optmem_max; /* 0xc 0x4 */
u8 sysctl_txrehash; /* 0x10 0x1 */
/* XXX 7 bytes hole, try to pack */
struct prot_inuse * prot_inuse; /* 0x18 0x8 */
struct cpumask * rps_default_mask; /* 0x20 0x8 */
int sysctl_tstamp_allow_data; /* 0x28 0x4 */
/* size: 48, cachelines: 1, members: 7 */
/* sum members: 37, holes: 1, sum holes: 7 */
/* padding: 4 */
/* last cacheline: 48 bytes */
};
See this line "/* sum members: 37, holes: 1, sum holes: 7 */", so I
don't think I introduce a new hole here.
After trying the suggestion you mentioned, the sum holes decreases from 7 to 3:
struct netns_core {
struct ctl_table_header * sysctl_hdr; /* 0 0x8 */
int sysctl_somaxconn; /* 0x8 0x4 */
int sysctl_optmem_max; /* 0xc 0x4 */
u8 sysctl_txrehash; /* 0x10 0x1 */
/* XXX 3 bytes hole, try to pack */
int sysctl_tstamp_allow_data; /* 0x14 0x4 */
struct prot_inuse * prot_inuse; /* 0x18 0x8 */
struct cpumask * rps_default_mask; /* 0x20 0x8 */
/* size: 40, cachelines: 1, members: 7 */
/* sum members: 37, holes: 1, sum holes: 3 */
/* last cacheline: 40 bytes */
};
I will adjust the patch as you said. Thank you, Eric.
Thanks,
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2] net-timestamp: namespacify the sysctl_tstamp_allow_data
2024-10-02 10:26 ` Jason Xing
@ 2024-10-02 10:29 ` Eric Dumazet
2024-10-02 10:34 ` Jason Xing
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2024-10-02 10:29 UTC (permalink / raw)
To: Jason Xing
Cc: davem, kuba, pabeni, willemdebruijn.kernel, willemb, netdev,
Jason Xing
On Wed, Oct 2, 2024 at 12:27 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> Hello Eric,
>
> On Wed, Oct 2, 2024 at 4:41 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Oct 2, 2024 at 6:18 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> > >
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > Let it be tuned in per netns by admins.
> > >
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > ---
> > > v2
> > > Link: https://lore.kernel.org/all/66fa81b2ddf10_17948d294bb@willemb.c.googlers.com.notmuch/
> > > 1. remove the static global from sock.c
> > > 2. reorder the tests
> > > 3. I removed the patch [1/3] because I made one mistake
> > > 4. I also removed the patch [2/3] because Willem soon will propose a
> > > packetdrill test that is better.
> > > Now, I only need to write this standalone patch.
> > > ---
> > > include/net/netns/core.h | 1 +
> > > include/net/sock.h | 2 --
> > > net/core/net_namespace.c | 1 +
> > > net/core/skbuff.c | 2 +-
> > > net/core/sock.c | 2 --
> > > net/core/sysctl_net_core.c | 18 +++++++++---------
> > > 6 files changed, 12 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/include/net/netns/core.h b/include/net/netns/core.h
> > > index 78214f1b43a2..ef8b3105c632 100644
> > > --- a/include/net/netns/core.h
> > > +++ b/include/net/netns/core.h
> > > @@ -23,6 +23,7 @@ struct netns_core {
> > > #if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL)
> > > struct cpumask *rps_default_mask;
> > > #endif
> > > + int sysctl_tstamp_allow_data;
> > > };
> >
> > This adds another hole for no good reason.
> > Please put this after sysctl_txrehash.
>
> Thanks for your reminder.
>
> Before this patch:
> struct netns_core {
> struct ctl_table_header * sysctl_hdr; /* 0 0x8 */
> int sysctl_somaxconn; /* 0x8 0x4 */
> int sysctl_optmem_max; /* 0xc 0x4 */
> u8 sysctl_txrehash; /* 0x10 0x1 */
>
> /* XXX 7 bytes hole, try to pack */
>
> struct prot_inuse * prot_inuse; /* 0x18 0x8 */
> struct cpumask * rps_default_mask; /* 0x20 0x8 */
>
> /* size: 40, cachelines: 1, members: 6 */
> /* sum members: 33, holes: 1, sum holes: 7 */
> /* last cacheline: 40 bytes */
> };
>
> After this patch:
> struct netns_core {
> struct ctl_table_header * sysctl_hdr; /* 0 0x8 */
> int sysctl_somaxconn; /* 0x8 0x4 */
> int sysctl_optmem_max; /* 0xc 0x4 */
> u8 sysctl_txrehash; /* 0x10 0x1 */
>
> /* XXX 7 bytes hole, try to pack */
>
> struct prot_inuse * prot_inuse; /* 0x18 0x8 */
> struct cpumask * rps_default_mask; /* 0x20 0x8 */
> int sysctl_tstamp_allow_data; /* 0x28 0x4 */
>
> /* size: 48, cachelines: 1, members: 7 */
> /* sum members: 37, holes: 1, sum holes: 7 */
> /* padding: 4 */
> /* last cacheline: 48 bytes */
> };
>
> See this line "/* sum members: 37, holes: 1, sum holes: 7 */", so I
> don't think I introduce a new hole here.
You certainly did. /* padding: 4 */
Overall size grew by 8 bytes, while adding one 4 byte field.
>
> After trying the suggestion you mentioned, the sum holes decreases from 7 to 3:
> struct netns_core {
> struct ctl_table_header * sysctl_hdr; /* 0 0x8 */
> int sysctl_somaxconn; /* 0x8 0x4 */
> int sysctl_optmem_max; /* 0xc 0x4 */
> u8 sysctl_txrehash; /* 0x10 0x1 */
>
> /* XXX 3 bytes hole, try to pack */
>
> int sysctl_tstamp_allow_data; /* 0x14 0x4 */
> struct prot_inuse * prot_inuse; /* 0x18 0x8 */
> struct cpumask * rps_default_mask; /* 0x20 0x8 */
>
> /* size: 40, cachelines: 1, members: 7 */
> /* sum members: 37, holes: 1, sum holes: 3 */
> /* last cacheline: 40 bytes */
> };
>
> I will adjust the patch as you said. Thank you, Eric.
>
> Thanks,
> Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2] net-timestamp: namespacify the sysctl_tstamp_allow_data
2024-10-02 10:29 ` Eric Dumazet
@ 2024-10-02 10:34 ` Jason Xing
0 siblings, 0 replies; 5+ messages in thread
From: Jason Xing @ 2024-10-02 10:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, willemdebruijn.kernel, willemb, netdev,
Jason Xing
On Wed, Oct 2, 2024 at 7:30 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Oct 2, 2024 at 12:27 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > Hello Eric,
> >
> > On Wed, Oct 2, 2024 at 4:41 PM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Wed, Oct 2, 2024 at 6:18 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> > > >
> > > > From: Jason Xing <kernelxing@tencent.com>
> > > >
> > > > Let it be tuned in per netns by admins.
> > > >
> > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > > ---
> > > > v2
> > > > Link: https://lore.kernel.org/all/66fa81b2ddf10_17948d294bb@willemb.c.googlers.com.notmuch/
> > > > 1. remove the static global from sock.c
> > > > 2. reorder the tests
> > > > 3. I removed the patch [1/3] because I made one mistake
> > > > 4. I also removed the patch [2/3] because Willem soon will propose a
> > > > packetdrill test that is better.
> > > > Now, I only need to write this standalone patch.
> > > > ---
> > > > include/net/netns/core.h | 1 +
> > > > include/net/sock.h | 2 --
> > > > net/core/net_namespace.c | 1 +
> > > > net/core/skbuff.c | 2 +-
> > > > net/core/sock.c | 2 --
> > > > net/core/sysctl_net_core.c | 18 +++++++++---------
> > > > 6 files changed, 12 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/include/net/netns/core.h b/include/net/netns/core.h
> > > > index 78214f1b43a2..ef8b3105c632 100644
> > > > --- a/include/net/netns/core.h
> > > > +++ b/include/net/netns/core.h
> > > > @@ -23,6 +23,7 @@ struct netns_core {
> > > > #if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL)
> > > > struct cpumask *rps_default_mask;
> > > > #endif
> > > > + int sysctl_tstamp_allow_data;
> > > > };
> > >
> > > This adds another hole for no good reason.
> > > Please put this after sysctl_txrehash.
> >
> > Thanks for your reminder.
> >
> > Before this patch:
> > struct netns_core {
> > struct ctl_table_header * sysctl_hdr; /* 0 0x8 */
> > int sysctl_somaxconn; /* 0x8 0x4 */
> > int sysctl_optmem_max; /* 0xc 0x4 */
> > u8 sysctl_txrehash; /* 0x10 0x1 */
> >
> > /* XXX 7 bytes hole, try to pack */
> >
> > struct prot_inuse * prot_inuse; /* 0x18 0x8 */
> > struct cpumask * rps_default_mask; /* 0x20 0x8 */
> >
> > /* size: 40, cachelines: 1, members: 6 */
> > /* sum members: 33, holes: 1, sum holes: 7 */
> > /* last cacheline: 40 bytes */
> > };
> >
> > After this patch:
> > struct netns_core {
> > struct ctl_table_header * sysctl_hdr; /* 0 0x8 */
> > int sysctl_somaxconn; /* 0x8 0x4 */
> > int sysctl_optmem_max; /* 0xc 0x4 */
> > u8 sysctl_txrehash; /* 0x10 0x1 */
> >
> > /* XXX 7 bytes hole, try to pack */
> >
> > struct prot_inuse * prot_inuse; /* 0x18 0x8 */
> > struct cpumask * rps_default_mask; /* 0x20 0x8 */
> > int sysctl_tstamp_allow_data; /* 0x28 0x4 */
> >
> > /* size: 48, cachelines: 1, members: 7 */
> > /* sum members: 37, holes: 1, sum holes: 7 */
> > /* padding: 4 */
> > /* last cacheline: 48 bytes */
> > };
> >
> > See this line "/* sum members: 37, holes: 1, sum holes: 7 */", so I
> > don't think I introduce a new hole here.
>
> You certainly did. /* padding: 4 */
>
> Overall size grew by 8 bytes, while adding one 4 byte field.
Oh, I learned. Thanks for your instructions.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-02 10:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-02 4:18 [PATCH net-next v2] net-timestamp: namespacify the sysctl_tstamp_allow_data Jason Xing
2024-10-02 7:41 ` Eric Dumazet
2024-10-02 10:26 ` Jason Xing
2024-10-02 10:29 ` Eric Dumazet
2024-10-02 10:34 ` Jason Xing
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).