[Devel] [PATCH hci-8.1 v3 0/6] ve/printk: Fix logs leakage via ve_printk called from IRQ context

[Devel] [PATCH hci-8.1 v3 0/6] ve/printk: Fix logs leakage via ve_printk called from IRQ context

[Vladimir Riabchun]

Printk function that printed logs to containers' dmesgs mostly used
get_exec_env() for getting VE. This is fine for task contexts and
might be wrong for IRQ contexts.

This patchset fixes this in the following way:
 - Report non-task usages of ve_printk function which get
   VE from current task. Do it with WARN so users will be
   frightened and will report it to us so we can fix it.
   This will also make an issue visible to our internal tests.
 - Reject printing to containers when VE is unreliable.
 - Calculate correct time offsets when calling printk. It can
   be delayed and the context might change, we can see time
   travels if time was taken from a different container.

With these fixes failcounts can be now used in any context,
so the last failcount - neigh_tbl - is added.

This was tested by our neoltp tests on host and inside containers.
No tests crashed, so there an no obvious IRQ callers of ve_printk
functions.

https://virtuozzo.atlassian.net/browse/VSTOR-146839

Feature: ve printk
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
Changes in v3:
- Patch 3: updated comments and removed ve_struct declaration in printk.h.
- Patch 4: removed stale GET)VE_LOG macro and wrapped owner_ve
  assignment in init_log_state with CONFIG_VE check.
- Link to v2: http://lore.virtuozzo.com/20261006-ve-printk-irq-v2-0-db1dc4f6ed07@virtuozzo.com

Changes in v2:
- Patch 2: fixed uninitialized r value in ve_dev_printk_emit.
- Patch 3: fixed incorrect __printf annotation for ve_printk_deferred.
- Patch 3: now on IRQ case ve_printk_ratelimited falls back to ve0.
- Patch 3: removed forgotten get_exec_env() usages from ve_printk
  functions.
- Patch 4: completely reworked and simplified.
- Patch 5: fix __ve_printk_ratelimited stub.
- Patch 6: brought back table type logging to host.
- Link to v1: http://lore.virtuozzo.com/20261002-ve-printk-irq-v1-0-cb55896d455c@virtuozzo.com

---
Vladimir Riabchun (6):
      ve/printk: Add sanity checks in ve_printk functions
      ve/printk: Report potentially leaking usage of ve_printk functions
      ve/printk: Specify ve in printks
      ve/printk: Fix time virtualization
      ve/failcount: Use ve_printk with specified ve
      ve: add neigh_tbl failcounter

 drivers/base/core.c                 |   8 ++-
 include/linux/net.h                 |  16 +++---
 include/linux/printk.h              |  43 ++++++++++++---
 include/linux/ratelimit_types.h     |   6 ++-
 include/linux/ve.h                  |  17 +++---
 kernel/printk/internal.h            |   2 +
 kernel/printk/printk.c              | 101 ++++++++++++++++++++++++++++--------
 kernel/ve/ve.c                      |   3 ++
 lib/ratelimit.c                     |   7 +--
 net/core/neighbour.c                |   5 +-
 net/core/utils.c                    |   4 +-
 net/netfilter/nf_conntrack_core.c   |   2 +-
 net/netfilter/nf_conntrack_expect.c |   2 +-
 net/netfilter/xt_TCPMSS.c           |   4 +-
 14 files changed, 160 insertions(+), 60 deletions(-)
---
base-commit: ca54f152160587a74ff17cb52feef16f7b9d4e2b
change-id: 20260923-ve-printk-irq-027c3378660f

Best regards,
-- 
Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 1/6] ve/printk: Add sanity checks in ve_printk functions

[Vladimir Riabchun]

This patch adds dst sanity check and reorders VE_LOG checks
in if.

https://virtuozzo.atlassian.net/browse/VSTOR-146839

Feature: ve printk
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 drivers/base/core.c    | 4 +++-
 kernel/printk/printk.c | 8 ++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index ebc6c5570a4d..2d233ffb5661 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4945,12 +4945,14 @@ int ve_dev_printk_emit(int dst, int level, const struct device *dev, const char
 	va_list args, args2;
 	int r;
 
+	WARN_ON_ONCE(!dst || (dst & ~VE_LOG_BOTH));
+
 	va_start(args, fmt);
 	va_copy(args2, args);
 
 	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
 		r = dev_vprintk_emit(level, dev, fmt, args);
-	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
+	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
 		r = ve_dev_vprintk_emit(level, dev, fmt, args2);
 
 	va_end(args2);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8940b36e790b..efe2c3279c85 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2535,10 +2535,12 @@ static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
 	va_list args2;
 	int r = 0;
 
+	WARN_ON_ONCE(!dst || (dst & ~VE_LOG_BOTH));
+
 	va_copy(args2, args);
 	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
 		r = vprintk(fmt, args);
-	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
+	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
 		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
 	va_end(args2);
 
@@ -2567,10 +2569,12 @@ static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args
 	va_list args2;
 	int r = 0;
 
+	WARN_ON_ONCE(!dst || (dst & ~VE_LOG_BOTH));
+
 	va_copy(args2, args);
 	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
 		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
-	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
+	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
 		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);
 	va_end(args2);
 

-- 
2.52.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 1/6] ve/printk: Add sanity checks in ve_printk functions

[no-reply]

> This patch adds dst sanity check and reorders VE_LOG checks
> in if.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-146839
> 
> Feature: ve printk
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261012-ve-printk-irq-v3-0-efc95fd5c78a@virtuozzo.com?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 2/6] ve/printk: Report potentially leaking usage of ve_printk functions

[Vladimir Riabchun]

We don't want to rely on get_exec_env() value in non-task context.
When this happens, let's issue a WARN and either reject printing or
return host log state.

Some messages may probably go to host dmesg instead of CT, but at
least we won't leak info to random container.

https://virtuozzo.atlassian.net/browse/VSTOR-146839

Feature: ve printk
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 drivers/base/core.c    |  6 ++++--
 kernel/printk/printk.c | 11 ++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2d233ffb5661..ba50bdbfb648 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4943,7 +4943,7 @@ static int ve_dev_vprintk_emit(int level, const struct device *dev, const char *
 int ve_dev_printk_emit(int dst, int level, const struct device *dev, const char *fmt, ...)
 {
 	va_list args, args2;
-	int r;
+	int r = 0;
 
 	WARN_ON_ONCE(!dst || (dst & ~VE_LOG_BOTH));
 
@@ -4952,7 +4952,9 @@ int ve_dev_printk_emit(int dst, int level, const struct device *dev, const char
 
 	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
 		r = dev_vprintk_emit(level, dev, fmt, args);
-	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
+
+	/* Report IRQ usage here - printing to VE requires explicit ve reference */
+	if ((dst & VE_LOG) && !WARN_ON_ONCE(!in_task()) && !ve_is_super(get_exec_env()))
 		r = ve_dev_vprintk_emit(level, dev, fmt, args2);
 
 	va_end(args2);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index efe2c3279c85..b06fc835d7c7 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -560,7 +560,8 @@ struct log_state *ve_log_state(void)
 {
 	struct log_state *log = &init_log_state;
 #ifdef CONFIG_VE
-	if (get_exec_env()->log_state)
+	/* get_exec_env() is reliable only in task context */
+	if (!WARN_ON_ONCE(!in_task()) && get_exec_env()->log_state)
 		log = get_exec_env()->log_state;
 #endif
 	return log;
@@ -2540,7 +2541,9 @@ static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
 	va_copy(args2, args);
 	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
 		r = vprintk(fmt, args);
-	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
+
+	/* Report IRQ usage here - printing to VE requires explicit ve reference */
+	if ((dst & VE_LOG) && !WARN_ON_ONCE(!in_task()) && !ve_is_super(get_exec_env()))
 		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
 	va_end(args2);
 
@@ -2574,7 +2577,9 @@ static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args
 	va_copy(args2, args);
 	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
 		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
-	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
+
+	/* Report IRQ usage here - printing to VE requires explicit ve reference */
+	if ((dst & VE_LOG) && !WARN_ON_ONCE(!in_task()) && !ve_is_super(get_exec_env()))
 		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);
 	va_end(args2);
 

-- 
2.52.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 2/6] ve/printk: Report potentially leaking usage of ve_printk functions

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] ve/printk: host-log branch still keyed on get_exec_env() from IRQ/softirq
--

commit 8d09395a5f1fa2043bb21991f8eb2152c9a5ad97
Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

ve/printk: Report potentially leaking usage of ve_printk functions

This patch adds WARN_ON_ONCE(!in_task()) checks to the VE_LOG branches of
ve_vprintk(), ve_vprintk_deferred() and ve_dev_printk_emit(), and to
ve_log_state(). The goal is to avoid relying on get_exec_env() in
non-task context, so that nothing leaks to a random container.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-146839

> diff --git a/drivers/base/core.c b/drivers/base/core.c
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c

[ ... ]

> @@ -4952,7 +4952,9 @@ int ve_dev_printk_emit(int dst, int level, const struct device *dev, const char
>  
>  	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>  		r = dev_vprintk_emit(level, dev, fmt, args);
> -	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
> +
> +	/* Report IRQ usage here - printing to VE requires explicit ve reference */
> +	if ((dst & VE_LOG) && !WARN_ON_ONCE(!in_task()) && !ve_is_super(get_exec_env()))
>  		r = ve_dev_vprintk_emit(level, dev, fmt, args2);

[Severity: Low]
Is the host-log branch still keyed on get_exec_env() in softirq or IRQ
context?

Only the VE_LOG branch is guarded by in_task().  The preceding host
branch is unchanged:

    if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
        r = dev_vprintk_emit(level, dev, fmt, args);

With dst == VE_LOG in softirq, the result depends on which task was
interrupted:

    - host task interrupted: the message goes to the host log, and then
      the VE branch only produces a WARN, even though that branch would
      have been skipped anyway.
    - container task interrupted: neither branch runs, so the message is
      silently dropped.

In both cases r stays 0, so the caller cannot tell what happened.

Nothing leaks to a container, but the outcome is not deterministic.
Would it make sense to check in_task() once before either branch and
apply a single rule for non-task context?

ve_dev_printk_emit() takes no ve argument, so it always relies on
get_exec_env().  The in-tree caller net/core/dev.c:__ve_netdev_printk()
passes no ve and can run from softirq/NAPI context, so it reaches this
path.

The later patches in the series ("Specify ve in printks" and "Use
ve_printk with specified ve") only help callers that pass an explicit ve.
This netdev path and ve_dev_printk_emit() are not covered by them.

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c

[ ... ]

> @@ -2540,7 +2541,9 @@ static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
>  	va_copy(args2, args);
>  	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>  		r = vprintk(fmt, args);
> -	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
> +
> +	/* Report IRQ usage here - printing to VE requires explicit ve reference */
> +	if ((dst & VE_LOG) && !WARN_ON_ONCE(!in_task()) && !ve_is_super(get_exec_env()))
>  		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);

[Severity: Low]
Does ve_vprintk() have the same problem?

The host-log decision still uses ve_is_super(get_exec_env()) without an
in_task() check.  In the series-final state it reads:

    if (!ve)
        ve = get_exec_env();

    va_copy(args2, args);
    if (ve_is_super(ve) || (dst & VE0_LOG))
        r = vprintk(fmt, args);

    if ((dst & VE_LOG)) {
        if (!ve_provided && WARN_ON_ONCE(!in_task()))
            goto done;

So a VE_LOG-only message from softirq with ve == NULL is either printed
to the host log or dropped, depending on the interrupted task.  The WARN
can also fire when a host task was interrupted, although the VE branch
would have been skipped anyway.

Could the host-branch decision be made deterministic for non-task
context with no explicit ve?

> @@ -2574,7 +2577,9 @@ static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args
>  	va_copy(args2, args);
>  	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>  		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
> -	if ((dst & VE_LOG) && !ve_is_super(get_exec_env()))
> +
> +	/* Report IRQ usage here - printing to VE requires explicit ve reference */
> +	if ((dst & VE_LOG) && !WARN_ON_ONCE(!in_task()) && !ve_is_super(get_exec_env()))
>  		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);

[Severity: Low]
The same pattern appears in ve_vprintk_deferred().  The host branch
ve_is_super(get_exec_env()) || (dst & VE0_LOG) is unguarded, and the
final-state code only guards the VE_LOG branch with
"if (!ve_provided && WARN_ON_ONCE(!in_task())) goto done;".

Can the same non-deterministic host-log versus drop outcome occur here
when this is called from IRQ or softirq with ve == NULL?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261012-ve-printk-irq-v3-0-efc95fd5c78a@virtuozzo.com?part=2

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 3/6] ve/printk: Specify ve in printks

[Vladimir Riabchun]

Introduce ve_printk helpers that accept VE structure,
so we can provide correct VE where get_exec_env is unreliable.

This is true for ve_printk_deferred functions, and their users.
Now VE ratelimiting requires specified VE, and this requirement
is further forwarded to ve_net_ratelimited kind.

https://virtuozzo.atlassian.net/browse/VSTOR-146839

Feature: ve printk
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 include/linux/net.h                 | 16 ++++----
 include/linux/printk.h              | 29 +++++++++++---
 include/linux/ratelimit_types.h     |  6 ++-
 kernel/printk/printk.c              | 80 ++++++++++++++++++++++++++++++-------
 lib/ratelimit.c                     |  7 ++--
 net/core/utils.c                    |  4 +-
 net/netfilter/nf_conntrack_core.c   |  2 +-
 net/netfilter/nf_conntrack_expect.c |  2 +-
 net/netfilter/xt_TCPMSS.c           |  4 +-
 9 files changed, 110 insertions(+), 40 deletions(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index 3f3bbe9f149d..4c34b9a0386f 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -265,7 +265,7 @@ struct socket *sockfd_lookup(int fd, int *err);
 struct socket *sock_from_file(struct file *file);
 #define		     sockfd_put(sock) fput(sock->file)
 int net_ratelimit(void);
-int ve_net_ratelimit(int dst);
+int ve_net_ratelimit(int dst, struct ve_struct *ve);
 
 #define net_ratelimited_function(function, ...)			\
 do {								\
@@ -273,10 +273,10 @@ do {								\
 		function(__VA_ARGS__);				\
 } while (0)
 
-#define ve_net_ratelimited_function(function, dst, ...)		\
+#define ve_net_ratelimited_function(function, dst, ve, ...)	\
 do {								\
-	if (ve_net_ratelimit(dst))				\
-		function(dst, __VA_ARGS__);			\
+	if (ve_net_ratelimit(dst, ve))				\
+		function(dst, ve, __VA_ARGS__);			\
 } while (0)
 
 #define net_emerg_ratelimited(fmt, ...)				\
@@ -311,10 +311,10 @@ do {									\
 	no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
 #endif
 
-#define net_velog_ratelimited(fmt, ...)                                \
-	ve_net_ratelimited_function(ve_printk, VE_LOG, fmt, ##__VA_ARGS__)
-#define net_veboth_ratelimited(fmt, ...)                               \
-	ve_net_ratelimited_function(ve_printk, VE_LOG_BOTH, fmt, ##__VA_ARGS__)
+#define net_velog_ratelimited(ve, fmt, ...)						\
+	ve_net_ratelimited_function(__ve_printk, VE_LOG, ve, fmt, ##__VA_ARGS__)
+#define net_veboth_ratelimited(ve, fmt, ...)						\
+	ve_net_ratelimited_function(__ve_printk, VE_LOG_BOTH, ve, fmt, ##__VA_ARGS__)
 
 #define net_get_random_once(buf, nbytes)			\
 	get_random_once((buf), (nbytes))
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 3b0af0deb6c3..8428ad64ffa0 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -76,6 +76,7 @@ extern int console_printk[];
 #define VE0_LOG		1
 #define VE_LOG		2
 #define VE_LOG_BOTH	(VE0_LOG | VE_LOG)
+struct ve_struct;
 
 extern void console_verbose(void);
 
@@ -160,6 +161,11 @@ int ve_vprintk_emit(int facility, int level,
 		    const struct dev_printk_info *dev_info,
 		    const char *fmt, va_list args);
 
+asmlinkage __printf(5, 0)
+int __ve_vprintk_emit(struct ve_struct *ve, int facility, int level,
+		    const struct dev_printk_info *dev_info,
+		    const char *fmt, va_list args);
+
 asmlinkage __printf(1, 0)
 int vprintk(const char *fmt, va_list args);
 __printf(1, 0)
@@ -171,10 +177,12 @@ int _printk(const char *fmt, ...);
 asmlinkage __printf(2, 3) __cold
 int ve_printk(int dst, const char *fmt, ...);
 
-asmlinkage __printf(2, 3) __cold
-int ve_printk_deferred(int dst, const char *fmt, ...);
+asmlinkage __printf(3, 4) __cold
+int __ve_printk(int dst, struct ve_struct *ve, const char *fmt, ...);
+
+asmlinkage __printf(3, 4) __cold
+int ve_printk_deferred(int dst, struct ve_struct *ve, const char *fmt, ...);
 
-struct ve_struct;
 int ve_log_init(struct ve_struct *ve);
 void ve_log_destroy(struct ve_struct *ve);
 asmlinkage __printf(2, 3) __cold
@@ -248,8 +256,13 @@ int ve_printk(int dst, const char *s, ...)
 {
 	return 0;
 }
-static inline __printf(2, 3) __cold
-int ve_printk_deferred(int dst, const char *s, ...)
+static inline __printf(3, 4) __cold
+int __ve_printk(int dst, struct ve_struct *ve, const char *s, ...)
+{
+	return 0;
+}
+static inline __printf(3, 4) __cold
+int ve_printk_deferred(int dst, struct ve_struct *ve, const char *s, ...)
 {
 	return 0;
 }
@@ -746,8 +759,12 @@ struct pi_entry {
 	static DEFINE_RATELIMIT_STATE(_rs,				\
 				      DEFAULT_RATELIMIT_INTERVAL,	\
 				      DEFAULT_RATELIMIT_BURST);		\
+	struct ve_struct *__ve = get_exec_env();			\
+									\
+	if (WARN_ON_ONCE((dst & VE_LOG) && !in_task()))			\
+		__ve = get_ve0();					\
 									\
-	if (ve_ratelimit(dst, &_rs))					\
+	if (ve_ratelimit(dst, __ve, &_rs))				\
 		ve_printk(dst, fmt, ##__VA_ARGS__);			\
 })
 #else
diff --git a/include/linux/ratelimit_types.h b/include/linux/ratelimit_types.h
index 3902066f0fee..aa9eee2a2455 100644
--- a/include/linux/ratelimit_types.h
+++ b/include/linux/ratelimit_types.h
@@ -44,7 +44,9 @@ struct ratelimit_state {
 extern int ___ratelimit(struct ratelimit_state *rs, const char *func);
 #define __ratelimit(state) ___ratelimit(state, __func__)
 
-extern int __ve_ratelimit(int dst, struct ratelimit_state *rs, const char *func);
-#define ve_ratelimit(dst, state) __ve_ratelimit(dst, state, __func__)
+struct ve_struct;
+extern int __ve_ratelimit(int dst, struct ve_struct *ve,
+	struct ratelimit_state *rs, const char *func);
+#define ve_ratelimit(dst, ve, state) __ve_ratelimit(dst, ve, state, __func__)
 
 #endif /* _LINUX_RATELIMIT_TYPES_H */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b06fc835d7c7..f0897de062a8 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -567,6 +567,16 @@ struct log_state *ve_log_state(void)
 	return log;
 }
 
+struct log_state *__ve_log_state(struct ve_struct *ve)
+{
+	struct log_state *log = &init_log_state;
+#ifdef CONFIG_VE
+	if (ve->log_state)
+		log = ve->log_state;
+#endif
+	return log;
+}
+
 void log_poll_wait(struct file *filp, poll_table *p)
 {
 	poll_wait(filp, &ve_log_state()->wait, p);
@@ -2525,26 +2535,44 @@ asmlinkage int ve_vprintk_emit(int facility, int level,
 }
 EXPORT_SYMBOL(ve_vprintk_emit);
 
+asmlinkage int __ve_vprintk_emit(struct ve_struct *ve, int facility, int level,
+			       const struct dev_printk_info *dev_info,
+			       const char *fmt, va_list args)
+{
+	return vprintk_emit_log(__ve_log_state(ve),
+				facility, level, dev_info,
+				fmt, args);
+}
+EXPORT_SYMBOL(__ve_vprintk_emit);
+
 int vprintk_default(const char *fmt, va_list args)
 {
 	return vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args);
 }
 EXPORT_SYMBOL_GPL(vprintk_default);
 
-static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
+static asmlinkage int ve_vprintk(int dst, struct ve_struct *ve,
+				const char *fmt, va_list args)
 {
 	va_list args2;
-	int r = 0;
+	int r = 0, ve_provided = (ve != NULL);
 
 	WARN_ON_ONCE(!dst || (dst & ~VE_LOG_BOTH));
 
+	if (!ve)
+		ve = get_exec_env();
+
 	va_copy(args2, args);
-	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
+	if (ve_is_super(ve) || (dst & VE0_LOG))
 		r = vprintk(fmt, args);
 
-	/* Report IRQ usage here - printing to VE requires explicit ve reference */
-	if ((dst & VE_LOG) && !WARN_ON_ONCE(!in_task()) && !ve_is_super(get_exec_env()))
-		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
+	if ((dst & VE_LOG)) {
+		if (!ve_provided && WARN_ON_ONCE(!in_task()))
+			goto done;
+		if (!ve_is_super(ve))
+			r = __ve_vprintk_emit(ve, 0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
+	}
+done:
 	va_end(args2);
 
 	return r;
@@ -2560,39 +2588,61 @@ asmlinkage int ve_printk(int dst, const char *fmt, ...)
 	int r;
 
 	va_start(args, fmt);
-	r = ve_vprintk(dst, fmt, args);
+	r = ve_vprintk(dst, NULL, fmt, args);
 	va_end(args);
 
 	return r;
 }
 EXPORT_SYMBOL(ve_printk);
 
-static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args)
+asmlinkage int __ve_printk(int dst, struct ve_struct *ve, const char *fmt, ...)
+{
+	va_list args;
+	int r;
+
+	va_start(args, fmt);
+	r = ve_vprintk(dst, ve, fmt, args);
+	va_end(args);
+
+	return r;
+}
+EXPORT_SYMBOL(__ve_printk);
+
+static asmlinkage int ve_vprintk_deferred(int dst, struct ve_struct *ve,
+					const char *fmt, va_list args)
 {
 	va_list args2;
-	int r = 0;
+	int r = 0, ve_provided = (ve != NULL);
 
 	WARN_ON_ONCE(!dst || (dst & ~VE_LOG_BOTH));
 
+	if (!ve)
+		ve = get_exec_env();
+
 	va_copy(args2, args);
-	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
+	if (ve_is_super(ve) || (dst & VE0_LOG))
 		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
 
-	/* Report IRQ usage here - printing to VE requires explicit ve reference */
-	if ((dst & VE_LOG) && !WARN_ON_ONCE(!in_task()) && !ve_is_super(get_exec_env()))
-		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);
+	if ((dst & VE_LOG)) {
+		if (!ve_provided && WARN_ON_ONCE(!in_task()))
+			goto done;
+		if (!ve_is_super(ve))
+			r = __ve_vprintk_emit(ve, 0, LOGLEVEL_SCHED, NULL, fmt, args2);
+	}
+done:
 	va_end(args2);
 
 	return r;
 }
 
-asmlinkage int ve_printk_deferred(int dst, const char *fmt, ...)
+asmlinkage int ve_printk_deferred(int dst, struct ve_struct *ve,
+	const char *fmt, ...)
 {
 	va_list args;
 	int r;
 
 	va_start(args, fmt);
-	r = ve_vprintk_deferred(dst, fmt, args);
+	r = ve_vprintk_deferred(dst, ve, fmt, args);
 	va_end(args);
 
 	return r;
diff --git a/lib/ratelimit.c b/lib/ratelimit.c
index 98409e870eea..98c45bb19e50 100644
--- a/lib/ratelimit.c
+++ b/lib/ratelimit.c
@@ -11,6 +11,7 @@
 #include <linux/ratelimit.h>
 #include <linux/jiffies.h>
 #include <linux/export.h>
+#include <linux/ve_proto.h>
 
 /*
  * __ratelimit - rate limiting
@@ -24,7 +25,7 @@
  * 0 means callbacks will be suppressed.
  * 1 means go ahead and do it.
  */
-int __ve_ratelimit(int dst, struct ratelimit_state *rs, const char *func)
+int __ve_ratelimit(int dst, struct ve_struct *ve, struct ratelimit_state *rs, const char *func)
 {
 	/* Paired with WRITE_ONCE() in .proc_handler().
 	 * Changing two values seperately could be inconsistent
@@ -55,7 +56,7 @@ int __ve_ratelimit(int dst, struct ratelimit_state *rs, const char *func)
 
 		if (m) {
 			if (!(rs->flags & RATELIMIT_MSG_ON_RELEASE)) {
-				ve_printk_deferred(dst, KERN_WARNING
+				ve_printk_deferred(dst, ve, KERN_WARNING
 						"%s: %d callbacks suppressed\n", func, m);
 			}
 		}
@@ -77,6 +78,6 @@ EXPORT_SYMBOL(__ve_ratelimit);
 
 int ___ratelimit(struct ratelimit_state *rs, const char *func)
 {
-	return __ve_ratelimit(VE0_LOG, rs, func);
+	return __ve_ratelimit(VE0_LOG, get_ve0(), rs, func);
 }
 EXPORT_SYMBOL(___ratelimit);
diff --git a/net/core/utils.c b/net/core/utils.c
index cffc3bc44baa..bf4e53205a8e 100644
--- a/net/core/utils.c
+++ b/net/core/utils.c
@@ -41,9 +41,9 @@ int net_ratelimit(void)
 }
 EXPORT_SYMBOL(net_ratelimit);
 
-int ve_net_ratelimit(int dst)
+int ve_net_ratelimit(int dst, struct ve_struct *ve)
 {
-	return ve_ratelimit(dst, &net_ratelimit_state);
+	return ve_ratelimit(dst, ve, &net_ratelimit_state);
 }
 EXPORT_SYMBOL(ve_net_ratelimit);
 
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index f078136edddd..8900a41b48b6 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1701,7 +1701,7 @@ __nf_conntrack_alloc(struct net *net,
 			if (!conntrack_gc_work.early_drop)
 				conntrack_gc_work.early_drop = true;
 			atomic_dec(&cnet->count);
-			net_veboth_ratelimited(KERN_WARNING "VE%s: "
+			net_veboth_ratelimited(net->owner_ve, KERN_WARNING "VE%s: "
 					       "nf_conntrack table full in netns %u, dropping packet\n",
 					       ve_name(net->owner_ve), net->ns.inum);
 			return ERR_PTR(-ENOMEM);
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 4b077b2cb432..23e78957bb79 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -508,7 +508,7 @@ static inline int __nf_ct_expect_check(struct nf_conntrack_expect *expect,
 
 	cnet = nf_ct_pernet(net);
 	if (cnet->expect_count >= cnet->expect_max) {
-		net_veboth_ratelimited(KERN_WARNING "VE%s: "
+		net_veboth_ratelimited(net->owner_ve, KERN_WARNING "VE%s: "
 					"nf_conntrack: expectation table full in netns %u\n",
 					ve_name(net->owner_ve), net->ns.inum);
 		ret = -EMFILE;
diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 822b3f71570e..0b9ccc8987e2 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -105,8 +105,8 @@ tcpmss_mangle_packet(struct sk_buff *skb,
 		unsigned int min_mtu = min(dst_mtu(skb_dst(skb)), in_mtu);
 
 		if (min_mtu <= minlen) {
-			net_velog_ratelimited("unknown or invalid path-MTU (%u)\n",
-					    min_mtu);
+			net_velog_ratelimited(net->owner_ve, "unknown or invalid path-MTU (%u)\n",
+					      min_mtu);
 			return -1;
 		}
 		newmss = min_mtu - minlen;

-- 
2.52.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 3/6] ve/printk: Specify ve in printks

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] ve/ratelimit: global net_ratelimit_state mixes suppressed counts across VEs
--

commit e1b1b77160f4d75abebd7d4609682186ec201a82
Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

ve/printk: Specify ve in printks

This commit adds ve_printk helpers that take an explicit VE pointer, so
that printing and ratelimiting no longer depend on get_exec_env() where it
is unreliable. The VE requirement is forwarded through ve_net_ratelimit()
and the net_velog_ratelimited()/net_veboth_ratelimited() macros, and the
netfilter callers now pass net->owner_ve.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-146839

> diff --git a/lib/ratelimit.c b/lib/ratelimit.c
> --- a/lib/ratelimit.c
> +++ b/lib/ratelimit.c

[ ... ]

> @@ -55,7 +56,7 @@ int __ve_ratelimit(int dst, struct ve_struct *ve, struct ratelimit_state *rs, const char *func)
>  
>  		if (m) {
>  			if (!(rs->flags & RATELIMIT_MSG_ON_RELEASE)) {
> -				ve_printk_deferred(dst, KERN_WARNING
> +				ve_printk_deferred(dst, ve, KERN_WARNING
>  						"%s: %d callbacks suppressed\n", func, m);
>  			}
>  		}

[Severity: Low]
This isn't a regression in the new interface, but the miss count printed
here comes from the shared ratelimit_state, so it includes suppressions
caused by every VE. Is it intended that the aggregated count is written
only into the log of whichever VE happens to reset the interval?

The shared state is defined once in net/core/utils.c:

net/core/utils.c:
    DEFINE_RATELIMIT_STATE(net_ratelimit_state, 5 * HZ, 10);
    ...
    int ve_net_ratelimit(int dst, struct ve_struct *ve)
    {
            return ve_ratelimit(dst, ve, &net_ratelimit_state);
    }

For example, if a container fills its own conntrack table and triggers
net_veboth_ratelimited() in __nf_conntrack_alloc(), the "N callbacks
suppressed" notice in another VE's log can include misses from the first
VE. A busy VE can also consume the shared burst of 10 per 5 seconds and
suppress the netfilter warnings of other VEs.

This patch now makes the target VE explicit, but the ratelimit_state
remains global. Would a per-VE ratelimit_state be needed for the ve
argument to give per-VE behaviour?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261012-ve-printk-irq-v3-0-efc95fd5c78a@virtuozzo.com?part=3

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 4/6] ve/printk: Fix time virtualization

[Vladimir Riabchun]

The usage of get_exec_env() in ve_timens_add_boottime_ns can lead
to time travels - a printk from IRQ might use container time for
printing to host dmesg and vice versa.

Do time adjustments on message submission in vprintk_store_log,
there we know the final destination and VE, so we can get actual
time namespace and offsets there.

https://virtuozzo.atlassian.net/browse/VSTOR-146839

Feature: ve printk
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 kernel/printk/internal.h |  2 ++
 kernel/printk/printk.c   | 14 +++++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index d00ac40d8401..10c2c15f9fb8 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -354,11 +354,13 @@ struct log_state {
 	 */
 	struct latched_seq clear_seq;
 	struct printk_ringbuffer *prb;
+	struct ve_struct *owner_ve;
 };
 
 extern struct log_state init_log_state;
 
 struct log_state *ve_log_state(void);
+struct log_state *__ve_log_state(struct ve_struct *ve);
 
 bool printk_get_next_message(struct log_state *log,
 			     struct printk_message *pmsg, u64 seq,
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index f0897de062a8..efe2d3069d7a 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -534,6 +534,9 @@ struct log_state init_log_state = {
 		.val[1]	= 0,
 	},
 	.prb = &printk_rb_static,
+#ifdef CONFIG_VE
+	.owner_ve = &ve0,
+#endif
 };
 
 /* kdump relies on some log_* symbols, let's make it happy */
@@ -698,12 +701,12 @@ static void append_char(char **pp, char *e, char c)
 		*(*pp)++ = c;
 }
 
-static inline u64 ve_timens_add_boottime_ns(u64 nsec)
+static inline u64 ve_timens_add_boottime_ns(u64 nsec, struct ve_struct *ve)
 {
 	struct time_namespace *ve_time_ns;
 	struct timens_offsets *ns_offsets;
 
-	ve_time_ns = ve_get_time_ns(get_exec_env());
+	ve_time_ns = ve_get_time_ns(ve);
 	if (unlikely(!ve_time_ns)) {
 		/* container not yet started */
 		return nsec;
@@ -731,8 +734,6 @@ static ssize_t info_print_ext_header(char *buf, size_t size,
 	caller[0] = '\0';
 #endif
 
-	/* shift the timestamp on the Container uptime value */
-	ts_usec = ve_timens_add_boottime_ns(ts_usec);
 	do_div(ts_usec, 1000);
 
 	return scnprintf(buf, size, "%u,%llu,%llu,%c%s;",
@@ -1431,9 +1432,6 @@ static size_t print_time(u64 ts, char *buf)
 {
 	unsigned long rem_nsec;
 
-	/* shift the timestamp on the Container uptime value */
-	ts = ve_timens_add_boottime_ns(ts);
-
 	rem_nsec = do_div(ts, 1000000000);
 
 	return sprintf(buf, "[%5lu.%06lu]",
@@ -2329,6 +2327,7 @@ static int vprintk_store_log(struct log_state *log, int facility, int level,
 	 * timestamp with respect to the caller.
 	 */
 	ts_nsec = local_clock();
+	ts_nsec = ve_timens_add_boottime_ns(ts_nsec, log->owner_ve);
 
 	caller_id = printk_caller_id();
 
@@ -5292,6 +5291,7 @@ int ve_log_init(struct ve_struct *ve)
 	seqcount_latch_init(&log->clear_seq.latch);
 
 	ve->log_state = log;
+	log->owner_ve = ve;
 	return 0;
 }
 

-- 
2.52.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 4/6] ve/printk: Fix time virtualization

[no-reply]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] printk: vprintk_store_log() can drop last time_ns ref and run free_time_ns()
- [Medium] ve/printk: store-time boottime shift leaves pre-start/post-stop VE records on host time
- [Low] printk: refcounted time_ns get/put on every message in vprintk_store_log()
--

commit d24cdca46b24c622f9dc8616a9a118ce2d8625f8
Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

ve/printk: Fix time virtualization

This patch moves the container boottime offset adjustment from the
read side (print_time() and info_print_ext_header()) to the store side
in vprintk_store_log(). The offset is taken from the time namespace of
the VE that owns the destination log_state, via a new owner_ve field.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-146839

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c

[ ... ]

> -static inline u64 ve_timens_add_boottime_ns(u64 nsec)
> +static inline u64 ve_timens_add_boottime_ns(u64 nsec, struct ve_struct *ve)
>  {
>  	struct time_namespace *ve_time_ns;
>  	struct timens_offsets *ns_offsets;
>  
> -	ve_time_ns = ve_get_time_ns(get_exec_env());
> +	ve_time_ns = ve_get_time_ns(ve);
>  	if (unlikely(!ve_time_ns)) {
>  		/* container not yet started */
>  		return nsec;
>  	}

[Severity: Low]
Is a counted reference needed here? ve_get_time_ns() does a
refcount_inc() on the time_ns under rcu_read_lock(), and the function
only reads the offsets before calling put_time_ns().

ve0 is not special-cased. init_ve has ve_nsproxy pointing at
init_nsproxy, so every host printk now reaches vprintk_store_log() ->
ve_timens_add_boottime_ns() -> ve_get_time_ns() -> get_time_ns() and
increments the init_time_ns refcount, with IRQs disabled inside
printk_enter_irqsave(). Previously this cost was paid only on the
reader side, in process context.

Could the offsets be read under rcu_read_lock() without taking a
reference?

[Severity: Medium]
Can the put_time_ns() here be the last put and run free_time_ns() from
the printk store path?

The sequence in ve_drop_context() is:

    rcu_assign_pointer(ve->ve_nsproxy, NULL);
    synchronize_rcu();
    put_nsproxy(ve_nsproxy);

A printer that took its time_ns reference through ve_get_time_ns()
before the pointer was cleared keeps its own reference. If it runs
concurrently with container stop, and the container tasks are already
gone, its put_time_ns() in ve_timens_add_boottime_ns() can drop the
last reference.

free_time_ns() then runs inside vprintk_store_log():

    dec_time_namespaces(ns->ucounts);
    put_user_ns(ns->user_ns);
    ns_free_inum(&ns->ns);
    __free_page(ns->vvar_page);
    kfree(ns);

This takes the proc_inum_ida lock and allocator and slab locks, and may
release a user_ns. Those locks could already be held by the context
that printk interrupted, so can this recurse or deadlock, or trigger a
lockdep wait-context warning?

Before the patch, the get/put ran only in task context on the reader
side. After it, any log with owner_ve set does the get/put on the store
path. The failcount patch later in this series adds
__ve_pr_warn_ratelimited() calls from ve_failcount_inc() with an
explicit ve, and those can come from arbitrary and atomic contexts.

Could the offsets be read without a counted reference, or the put be
deferred?

[ ... ]

> @@ -1431,9 +1432,6 @@ static size_t print_time(u64 ts, char *buf)
>  {
>  	unsigned long rem_nsec;
>  
> -	/* shift the timestamp on the Container uptime value */
> -	ts = ve_timens_add_boottime_ns(ts);
> -
>  	rem_nsec = do_div(ts, 1000000000);

[Severity: Medium]
Is it intended that records stored while the VE has no ve_nsproxy now
keep the raw host timestamp permanently?

With the read-side shift removed here, print_time() and
info_print_ext_header() print whatever is in the ring buffer. In
vprintk_store_log(), ve_timens_add_boottime_ns() returns the unshifted
local_clock() value when ve_get_time_ns() returns NULL ("container not
yet started").

ve_nsproxy is NULL before ve_grab_context() in ve_start_container(),
while ve_log_init() sets up the log_state at cgroup creation. It is NULL
again after ve_drop_context(), which runs in the start error path and
when the VE is stopped.

The VE log can be written in those windows, since __ve_vprintk_emit()
and ve_log_printk() only check ve_is_super() and not whether the VE is
running. For example, ve_log_printk(net->owner_ve, ...) from
net/netfilter/nf_log.c, or ve_printk(VE_LOG, ...) from kernel/exit.c,
mm/memory.c, net/core/dev.c and net/ipv4/netfilter/ip_tables.c.

Records stored before start or after stop would then carry host uptime,
while records stored in between carry host uptime plus the (usually
negative) container boottime offset. Can the same container dmesg or
/dev/kmsg then show large timestamps followed by small ones?

Since nothing re-bases old records at read time any more, would it make
sense to mention this trade-off in the commit message?

[ ... ]

> @@ -2329,6 +2327,7 @@ static int vprintk_store_log(struct log_state *log, int facility, int level,
>  	 * timestamp with respect to the caller.
>  	 */
>  	ts_nsec = local_clock();
> +	ts_nsec = ve_timens_add_boottime_ns(ts_nsec, log->owner_ve);

This is the call site for the two comments above on
ve_timens_add_boottime_ns().

>  
>  	caller_id = printk_caller_id();

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261012-ve-printk-irq-v3-0-efc95fd5c78a@virtuozzo.com?part=4

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 5/6] ve/failcount: Use ve_printk with specified ve

[Vladimir Riabchun]

ve_failcount_inc could be called from IRQ context, and we want the message
to go to the right place. Use new  __ve_printk helpers for this.

https://virtuozzo.atlassian.net/browse/VSTOR-146839

Feature: ve printk
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 include/linux/printk.h | 18 ++++++++++++++----
 include/linux/ve.h     | 16 ++++++++--------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 8428ad64ffa0..e15087a25918 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -754,24 +754,31 @@ struct pi_entry {
 		printk(fmt, ##__VA_ARGS__);				\
 })
 
-#define ve_printk_ratelimited(dst, fmt, ...)				\
+#define __ve_printk_ratelimited(dst, ve, fmt, ...)			\
 ({									\
 	static DEFINE_RATELIMIT_STATE(_rs,				\
 				      DEFAULT_RATELIMIT_INTERVAL,	\
 				      DEFAULT_RATELIMIT_BURST);		\
+									\
+	if (ve_ratelimit(dst, ve, &_rs))				\
+		__ve_printk(dst, ve, fmt, ##__VA_ARGS__);		\
+})
+
+#define ve_printk_ratelimited(dst, fmt, ...)				\
+({									\
 	struct ve_struct *__ve = get_exec_env();			\
 									\
 	if (WARN_ON_ONCE((dst & VE_LOG) && !in_task()))			\
 		__ve = get_ve0();					\
-									\
-	if (ve_ratelimit(dst, __ve, &_rs))				\
-		ve_printk(dst, fmt, ##__VA_ARGS__);			\
+	__ve_printk_ratelimited(dst, __ve, fmt, ##__VA_ARGS__);		\
 })
 #else
 #define printk_ratelimited(fmt, ...)					\
 	no_printk(fmt, ##__VA_ARGS__)
 #define ve_printk_ratelimited(dst, fmt, ...)				\
 	no_printk(fmt, ##__VA_ARGS__)
+#define __ve_printk_ratelimited(dst, ve, fmt, ...)			\
+	no_printk(fmt, ##__VA_ARGS__)
 #endif
 
 #define pr_emerg_ratelimited(fmt, ...)					\
@@ -789,6 +796,7 @@ struct pi_entry {
 #define pr_info_ratelimited(fmt, ...)					\
 	printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 /* no pr_cont_ratelimited, don't do that... */
+
 #define ve_pr_emerg_ratelimited(dst, fmt, ...)					\
 	ve_printk_ratelimited(dst, KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
 #define ve_pr_alert_ratelimited(dst, fmt, ...)					\
@@ -799,6 +807,8 @@ struct pi_entry {
 	ve_printk_ratelimited(dst, KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
 #define ve_pr_warn_ratelimited(dst, fmt, ...)					\
 	ve_printk_ratelimited(dst, KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+#define __ve_pr_warn_ratelimited(dst, ve, fmt, ...)				\
+	__ve_printk_ratelimited(dst, ve, KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
 #define ve_pr_notice_ratelimited(dst, fmt, ...)					\
 	ve_printk_ratelimited(dst, KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
 #define ve_pr_info_ratelimited(dst, fmt, ...)					\
diff --git a/include/linux/ve.h b/include/linux/ve.h
index a400d21593b2..946df4eebe89 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -156,14 +156,14 @@ extern int nr_ve;
 #define BPF_PROG_MAX_NR_DEFAULT	256	/* number of loaded BPF progs per-VE */
 #define MNT_MAX_NR_DEFAULT	4096	/* number of mounts per-VE */
 
-#define ve_failcount_inc(ve, name)					\
-do {									\
-	struct ve_struct *__ve = (ve);					\
-									\
-	atomic_inc(&__ve->name##_failcount);				\
-	ve_pr_warn_ratelimited(VE_LOG_BOTH, "CT%s: hits the " #name	\
-		" limit, see the '" #name "' counter in ve.failcount "	\
-		"of the container's ve cgroup\n", ve_name(__ve));	\
+#define ve_failcount_inc(ve, name)						\
+do {										\
+	struct ve_struct *__ve = (ve);						\
+										\
+	atomic_inc(&__ve->name##_failcount);					\
+	__ve_pr_warn_ratelimited(VE_LOG_BOTH, __ve, "CT%s: hits the " #name	\
+		" limit, see the '" #name "' counter in ve.failcount "		\
+		"of the container's ve cgroup\n", ve_name(__ve));		\
 } while (0)
 
 #ifdef CONFIG_VE

-- 
2.52.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 5/6] ve/failcount: Use ve_printk with specified ve

[no-reply]

> ve_failcount_inc could be called from IRQ context, and we want the message
> to go to the right place. Use new  __ve_printk helpers for this.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-146839
> 
> Feature: ve printk
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261012-ve-printk-irq-v3-0-efc95fd5c78a@virtuozzo.com?part=5


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 6/6] ve: add neigh_tbl failcounter

[Vladimir Riabchun]

Count hits of the per-VE neigh_tbl limit in ve.failcount.

This one was delayed due to a high risk of IRQ-context hits,
when destination VE can't be obtained from get_exec_env.

https://virtuozzo.atlassian.net/browse/VSTOR-135520
https://virtuozzo.atlassian.net/browse/VSTOR-146839

Feature: per-ve failcounters
Feature: ve printk
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 include/linux/ve.h   | 1 +
 kernel/ve/ve.c       | 3 +++
 net/core/neighbour.c | 5 +++--
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/ve.h b/include/linux/ve.h
index 946df4eebe89..44e04dc75823 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -97,6 +97,7 @@ struct ve_struct {
 
 	atomic_t		arp_neigh_nr;
 	atomic_t		nd_neigh_nr;
+	atomic_t		neigh_tbl_failcount;
 	unsigned long		meminfo_val;
 
 	/*
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 51cc635be2e2..35fad9658149 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -118,6 +118,7 @@ struct ve_struct ve0 = {
 
 	.arp_neigh_nr		= ATOMIC_INIT(0),
 	.nd_neigh_nr		= ATOMIC_INIT(0),
+	.neigh_tbl_failcount	= ATOMIC_INIT(0),
 	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
 	.mnt_max_nr		= INT_MAX,
 	.mnt_failcount		= ATOMIC_INIT(0),
@@ -834,6 +835,7 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
 
 	atomic_set(&ve->arp_neigh_nr, 0);
 	atomic_set(&ve->nd_neigh_nr, 0);
+	atomic_set(&ve->neigh_tbl_failcount, 0);
 	ve->mnt_max_nr = MNT_MAX_NR_DEFAULT;
 	atomic_set(&ve->mnt_failcount, 0);
 	atomic_set(&ve->mnt_avail_nr, MNT_MAX_NR_DEFAULT);
@@ -1098,6 +1100,7 @@ static const struct ve_failcount_entry {
 	VE_FC_ENTRY(mnt),
 	VE_FC_ENTRY(netif),
 	VE_FC_ENTRY(bpf_prog),
+	VE_FC_ENTRY(neigh_tbl),
 #ifdef CONFIG_AIO
 	VE_FC_ENTRY(aio),
 #endif
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index f90deb17fb25..6c2fc778a8ab 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -520,8 +520,9 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
 	    (glob_entries >= READ_ONCE(tbl->gc_thresh2) &&
 	     time_after(now, READ_ONCE(tbl->last_flush) + 5 * HZ))) {
 		if (!neigh_forced_gc(tbl, ve) && entries >= gc_thresh3) {
-			net_info_ratelimited("%s: neighbor table overflow!\n",
-					     tbl->id);
+			net_info_ratelimited("%s: neighbor table overflow for ve %s!\n",
+					     tbl->id, ve_name(ve));
+			ve_failcount_inc(ve, neigh_tbl);
 			NEIGH_CACHE_STAT_INC(tbl, table_fulls);
 			goto out_entries;
 		}

-- 
2.52.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 6/6] ve: add neigh_tbl failcounter

[no-reply]

> Count hits of the per-VE neigh_tbl limit in ve.failcount.
> 
> This one was delayed due to a high risk of IRQ-context hits,
> when destination VE can't be obtained from get_exec_env.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> https://virtuozzo.atlassian.net/browse/VSTOR-146839
> 
> Feature: per-ve failcounters
> Feature: ve printk
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261012-ve-printk-irq-v3-0-efc95fd5c78a@virtuozzo.com?part=6


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 6/6] ve: add neigh_tbl failcounter

[Vasileios Almpanis]

> Count hits of the per-VE neigh_tbl limit in ve.failcount.
> 
> This one was delayed due to a high risk of IRQ-context hits,
> when destination VE can't be obtained from get_exec_env.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> https://virtuozzo.atlassian.net/browse/VSTOR-146839
> 
> Feature: per-ve failcounters
> Feature: ve printk
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
>
> diff --git a/include/linux/ve.h b/include/linux/ve.h
> index 946df4eebe89..44e04dc75823 100644
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h
> @@ -97,6 +97,7 @@ struct ve_struct {
>  
>  	atomic_t		arp_neigh_nr;
>  	atomic_t		nd_neigh_nr;
> +	atomic_t		neigh_tbl_failcount;
>  	unsigned long		meminfo_val;
>  
>  	/*
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index 51cc635be2e2..35fad9658149 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -118,6 +118,7 @@ struct ve_struct ve0 = {
>  
>  	.arp_neigh_nr		= ATOMIC_INIT(0),
>  	.nd_neigh_nr		= ATOMIC_INIT(0),
> +	.neigh_tbl_failcount	= ATOMIC_INIT(0),
>  	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
>  	.mnt_max_nr		= INT_MAX,
>  	.mnt_failcount		= ATOMIC_INIT(0),
> @@ -834,6 +835,7 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
>  
>  	atomic_set(&ve->arp_neigh_nr, 0);
>  	atomic_set(&ve->nd_neigh_nr, 0);
> +	atomic_set(&ve->neigh_tbl_failcount, 0);
>  	ve->mnt_max_nr = MNT_MAX_NR_DEFAULT;
>  	atomic_set(&ve->mnt_failcount, 0);
>  	atomic_set(&ve->mnt_avail_nr, MNT_MAX_NR_DEFAULT);
> @@ -1098,6 +1100,7 @@ static const struct ve_failcount_entry {
>  	VE_FC_ENTRY(mnt),
>  	VE_FC_ENTRY(netif),
>  	VE_FC_ENTRY(bpf_prog),
> +	VE_FC_ENTRY(neigh_tbl),
>  #ifdef CONFIG_AIO
>  	VE_FC_ENTRY(aio),
>  #endif
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index f90deb17fb25..6c2fc778a8ab 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -520,8 +520,9 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
>  	    (glob_entries >= READ_ONCE(tbl->gc_thresh2) &&
>  	     time_after(now, READ_ONCE(tbl->last_flush) + 5 * HZ))) {
>  		if (!neigh_forced_gc(tbl, ve) && entries >= gc_thresh3) {
> -			net_info_ratelimited("%s: neighbor table overflow!\n",
> -					     tbl->id);
> +			net_info_ratelimited("%s: neighbor table overflow for ve %s!\n",
> +					     tbl->id, ve_name(ve));

Here on hosts with more neighbors than the threshold the host will get a
message which points to a failcount file it doesn't exist. Maybe only
increase failcount in case ve is not super?

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 0/6] ve/printk: Fix logs leakage via ve_printk called from IRQ context

[Vasileios Almpanis]

On Mon, 12 Oct 2026 00:52:27 +0200, Vladimir Riabchun <vladimir.riabchun@virtuozzo.com> wrote:
> ve/printk: Fix logs leakage via ve_printk called from IRQ context
> 
> Printk function that printed logs to containers' dmesgs mostly used
> get_exec_env() for getting VE. This is fine for task contexts and
> might be wrong for IRQ contexts.
> 
> This patchset fixes this in the following way:
>  - Report non-task usages of ve_printk function which get
>    VE from current task. Do it with WARN so users will be
>    frightened and will report it to us so we can fix it.
>    This will also make an issue visible to our internal tests.
>  - Reject printing to containers when VE is unreliable.
>  - Calculate correct time offsets when calling printk. It can
>    be delayed and the context might change, we can see time
>    travels if time was taken from a different container.
> 
> [...]

Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel