Menu
▾
▴
openvpn-devel
|
From: ordex (C. Review) <ge...@op...> - 2025-07-22 20:22:30
|
Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1093?usp=email
to review the following change.
Change subject: multi: store multi_context address inside top instance
......................................................................
multi: store multi_context address inside top instance
Future modifications to DCO require accessing the
server multi_context object.
Since it is currently a stack variable that is pointed
by no one, we'd need to pass it to all kind of functions
to ensure it can reach the DCO code.
To make the implementation simpler, it is preferable to
simply assign its address to a struct context's field.
While at it, make some multi_* functions static as they
used only inside multi.c, where they are defined.
Change-Id: Ibf64c681e02ac572d339d4d98e75ceb0cd417c45
Signed-off-by: Antonio Quartulli <an...@ma...>
---
M src/openvpn/multi.c
M src/openvpn/multi.h
M src/openvpn/openvpn.h
3 files changed, 15 insertions(+), 18 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/93/1093/1
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 4696686..ec260a2 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -290,9 +290,10 @@
/*
* Main initialization function, init multi_context object.
*/
-void
-multi_init(struct multi_context *m, struct context *t)
+static void
+multi_init(struct context *t)
{
+ struct multi_context *m = t->multi;
int dev = DEV_TYPE_UNDEF;
msg(D_MULTI_LOW, "MULTI: multi_init called, r=%d v=%d",
@@ -706,7 +707,7 @@
/*
* Called on shutdown or restart.
*/
-void
+static void
multi_uninit(struct multi_context *m)
{
if (m->hash)
@@ -3922,14 +3923,14 @@
}
}
-void
-multi_top_init(struct multi_context *m, struct context *top)
+static void
+multi_top_init(struct context *top)
{
- inherit_context_top(&m->top, top);
- m->top.c2.buffers = init_context_buffers(&top->c2.frame);
+ inherit_context_top(&top->multi->top, top);
+ top->multi->top.c2.buffers = init_context_buffers(&top->c2.frame);
}
-void
+static void
multi_top_free(struct multi_context *m)
{
close_context(&m->top, -1, CC_GC_FREE);
@@ -4324,6 +4325,7 @@
struct multi_context multi;
top->mode = CM_TOP;
+ top->multi = &multi;
context_clear_2(top);
/* initialize top-tunnel instance */
@@ -4334,10 +4336,10 @@
}
/* initialize global multi_context object */
- multi_init(&multi, top);
+ multi_init(top);
/* initialize our cloned top object */
- multi_top_init(&multi, top);
+ multi_top_init(top);
/* initialize management interface */
init_management_callback_multi(&multi);
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index fe9e847..8b2704c 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -263,14 +263,6 @@
* Called by mtcp.c, mudp.c, or other (to be written) protocol drivers
*/
-void multi_init(struct multi_context *m, struct context *t);
-
-void multi_uninit(struct multi_context *m);
-
-void multi_top_init(struct multi_context *m, struct context *top);
-
-void multi_top_free(struct multi_context *m);
-
struct multi_instance *multi_create_instance(struct multi_context *m, const struct mroute_addr *real,
struct link_socket *sock);
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 3c8ce39..7d48888 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -491,6 +491,9 @@
* CM_P2P, \c CM_TOP, \c CM_TOP_CLONE,
* \c CM_CHILD_UDP, and \c CM_CHILD_TCP. */
+ struct multi_context *multi; /**< Pointer to the main P2MP context.
+ * Non-NULL only when mode == CM_TOP. */
+
struct gc_arena gc; /**< Garbage collection arena for
* allocations done in the scope of this
* context structure. */
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1093?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ibf64c681e02ac572d339d4d98e75ceb0cd417c45
Gerrit-Change-Number: 1093
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-23 06:10:20
|
Attention is currently required from: flichtenheld, ordex, plaisthos. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1093?usp=email ) Change subject: multi: store multi_context address inside top instance ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1093?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ibf64c681e02ac572d339d4d98e75ceb0cd417c45 Gerrit-Change-Number: 1093 Gerrit-PatchSet: 1 Gerrit-Owner: ordex <an...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: ordex <an...@ma...> Gerrit-Comment-Date: Wed, 23 Jul 2025 06:10:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: Gert D. <ge...@gr...> - 2025-07-23 06:10:46
|
From: Antonio Quartulli <an...@ma...> Future modifications to DCO require accessing the server multi_context object. Since it is currently a stack variable that is pointed by no one, we'd need to pass it to all kind of functions to ensure it can reach the DCO code. To make the implementation simpler, it is preferable to simply assign its address to a struct context's field. While at it, make some multi_* functions static as they used only inside multi.c, where they are defined. Change-Id: Ibf64c681e02ac572d339d4d98e75ceb0cd417c45 Signed-off-by: Antonio Quartulli <an...@ma...> Acked-by: Gert Doering <ge...@gr...> --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1093 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 4696686..ec260a2 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -290,9 +290,10 @@ /* * Main initialization function, init multi_context object. */ -void -multi_init(struct multi_context *m, struct context *t) +static void +multi_init(struct context *t) { + struct multi_context *m = t->multi; int dev = DEV_TYPE_UNDEF; msg(D_MULTI_LOW, "MULTI: multi_init called, r=%d v=%d", @@ -706,7 +707,7 @@ /* * Called on shutdown or restart. */ -void +static void multi_uninit(struct multi_context *m) { if (m->hash) @@ -3922,14 +3923,14 @@ } } -void -multi_top_init(struct multi_context *m, struct context *top) +static void +multi_top_init(struct context *top) { - inherit_context_top(&m->top, top); - m->top.c2.buffers = init_context_buffers(&top->c2.frame); + inherit_context_top(&top->multi->top, top); + top->multi->top.c2.buffers = init_context_buffers(&top->c2.frame); } -void +static void multi_top_free(struct multi_context *m) { close_context(&m->top, -1, CC_GC_FREE); @@ -4324,6 +4325,7 @@ struct multi_context multi; top->mode = CM_TOP; + top->multi = &multi; context_clear_2(top); /* initialize top-tunnel instance */ @@ -4334,10 +4336,10 @@ } /* initialize global multi_context object */ - multi_init(&multi, top); + multi_init(top); /* initialize our cloned top object */ - multi_top_init(&multi, top); + multi_top_init(top); /* initialize management interface */ init_management_callback_multi(&multi); diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index fe9e847..8b2704c 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -263,14 +263,6 @@ * Called by mtcp.c, mudp.c, or other (to be written) protocol drivers */ -void multi_init(struct multi_context *m, struct context *t); - -void multi_uninit(struct multi_context *m); - -void multi_top_init(struct multi_context *m, struct context *top); - -void multi_top_free(struct multi_context *m); - struct multi_instance *multi_create_instance(struct multi_context *m, const struct mroute_addr *real, struct link_socket *sock); diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index 3c8ce39..7d48888 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -491,6 +491,9 @@ * CM_P2P, \c CM_TOP, \c CM_TOP_CLONE, * \c CM_CHILD_UDP, and \c CM_CHILD_TCP. */ + struct multi_context *multi; /**< Pointer to the main P2MP context. + * Non-NULL only when mode == CM_TOP. */ + struct gc_arena gc; /**< Garbage collection arena for * allocations done in the scope of this * context structure. */ |
|
From: Gert D. <ge...@gr...> - 2025-07-23 09:40:02
|
The change is fairly trivial, but helps basically all sort of
"async things that need to access the multi context" later on.
I do look forward to crashing the new DCO code by running it in
"non multi" mode, though :-) (which is the risk that code changes
relying on this commit bring with them - being run in a non-multi
environment).
Unlike the other patches in this series, *this* has been also subjected
to a full client/server side test run - it looks harmless, but sometimes
I overlook things a test run finds.
Your patch has been applied to the master branch.
commit 7f5a6deae33a338a23d7e8ff8526db8fdddf4bc2
Author: Antonio Quartulli
Date: Wed Jul 23 08:10:25 2025 +0200
multi: store multi_context address inside top instance
Signed-off-by: Antonio Quartulli <an...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32266.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-23 09:40:22
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1093?usp=email ) Change subject: multi: store multi_context address inside top instance ...................................................................... multi: store multi_context address inside top instance Future modifications to DCO require accessing the server multi_context object. Since it is currently a stack variable that is pointed by no one, we'd need to pass it to all kind of functions to ensure it can reach the DCO code. To make the implementation simpler, it is preferable to simply assign its address to a struct context's field. While at it, make some multi_* functions static as they used only inside multi.c, where they are defined. Change-Id: Ibf64c681e02ac572d339d4d98e75ceb0cd417c45 Signed-off-by: Antonio Quartulli <an...@ma...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg32266.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/multi.c M src/openvpn/multi.h M src/openvpn/openvpn.h 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 4696686..ec260a2 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -290,9 +290,10 @@ /* * Main initialization function, init multi_context object. */ -void -multi_init(struct multi_context *m, struct context *t) +static void +multi_init(struct context *t) { + struct multi_context *m = t->multi; int dev = DEV_TYPE_UNDEF; msg(D_MULTI_LOW, "MULTI: multi_init called, r=%d v=%d", @@ -706,7 +707,7 @@ /* * Called on shutdown or restart. */ -void +static void multi_uninit(struct multi_context *m) { if (m->hash) @@ -3922,14 +3923,14 @@ } } -void -multi_top_init(struct multi_context *m, struct context *top) +static void +multi_top_init(struct context *top) { - inherit_context_top(&m->top, top); - m->top.c2.buffers = init_context_buffers(&top->c2.frame); + inherit_context_top(&top->multi->top, top); + top->multi->top.c2.buffers = init_context_buffers(&top->c2.frame); } -void +static void multi_top_free(struct multi_context *m) { close_context(&m->top, -1, CC_GC_FREE); @@ -4324,6 +4325,7 @@ struct multi_context multi; top->mode = CM_TOP; + top->multi = &multi; context_clear_2(top); /* initialize top-tunnel instance */ @@ -4334,10 +4336,10 @@ } /* initialize global multi_context object */ - multi_init(&multi, top); + multi_init(top); /* initialize our cloned top object */ - multi_top_init(&multi, top); + multi_top_init(top); /* initialize management interface */ init_management_callback_multi(&multi); diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index fe9e847..8b2704c 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -263,14 +263,6 @@ * Called by mtcp.c, mudp.c, or other (to be written) protocol drivers */ -void multi_init(struct multi_context *m, struct context *t); - -void multi_uninit(struct multi_context *m); - -void multi_top_init(struct multi_context *m, struct context *top); - -void multi_top_free(struct multi_context *m); - struct multi_instance *multi_create_instance(struct multi_context *m, const struct mroute_addr *real, struct link_socket *sock); diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index 3c8ce39..7d48888 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -491,6 +491,9 @@ * CM_P2P, \c CM_TOP, \c CM_TOP_CLONE, * \c CM_CHILD_UDP, and \c CM_CHILD_TCP. */ + struct multi_context *multi; /**< Pointer to the main P2MP context. + * Non-NULL only when mode == CM_TOP. */ + struct gc_arena gc; /**< Garbage collection arena for * allocations done in the scope of this * context structure. */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1093?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ibf64c681e02ac572d339d4d98e75ceb0cd417c45 Gerrit-Change-Number: 1093 Gerrit-PatchSet: 2 Gerrit-Owner: ordex <an...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: merged |
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-23 09:40:22
|
cron2 has uploaded a new patch set (#2) to the change originally created by ordex. ( http://gerrit.openvpn.net/c/openvpn/+/1093?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: multi: store multi_context address inside top instance ...................................................................... multi: store multi_context address inside top instance Future modifications to DCO require accessing the server multi_context object. Since it is currently a stack variable that is pointed by no one, we'd need to pass it to all kind of functions to ensure it can reach the DCO code. To make the implementation simpler, it is preferable to simply assign its address to a struct context's field. While at it, make some multi_* functions static as they used only inside multi.c, where they are defined. Change-Id: Ibf64c681e02ac572d339d4d98e75ceb0cd417c45 Signed-off-by: Antonio Quartulli <an...@ma...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg32266.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/multi.c M src/openvpn/multi.h M src/openvpn/openvpn.h 3 files changed, 15 insertions(+), 18 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/93/1093/2 diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 4696686..ec260a2 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -290,9 +290,10 @@ /* * Main initialization function, init multi_context object. */ -void -multi_init(struct multi_context *m, struct context *t) +static void +multi_init(struct context *t) { + struct multi_context *m = t->multi; int dev = DEV_TYPE_UNDEF; msg(D_MULTI_LOW, "MULTI: multi_init called, r=%d v=%d", @@ -706,7 +707,7 @@ /* * Called on shutdown or restart. */ -void +static void multi_uninit(struct multi_context *m) { if (m->hash) @@ -3922,14 +3923,14 @@ } } -void -multi_top_init(struct multi_context *m, struct context *top) +static void +multi_top_init(struct context *top) { - inherit_context_top(&m->top, top); - m->top.c2.buffers = init_context_buffers(&top->c2.frame); + inherit_context_top(&top->multi->top, top); + top->multi->top.c2.buffers = init_context_buffers(&top->c2.frame); } -void +static void multi_top_free(struct multi_context *m) { close_context(&m->top, -1, CC_GC_FREE); @@ -4324,6 +4325,7 @@ struct multi_context multi; top->mode = CM_TOP; + top->multi = &multi; context_clear_2(top); /* initialize top-tunnel instance */ @@ -4334,10 +4336,10 @@ } /* initialize global multi_context object */ - multi_init(&multi, top); + multi_init(top); /* initialize our cloned top object */ - multi_top_init(&multi, top); + multi_top_init(top); /* initialize management interface */ init_management_callback_multi(&multi); diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index fe9e847..8b2704c 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -263,14 +263,6 @@ * Called by mtcp.c, mudp.c, or other (to be written) protocol drivers */ -void multi_init(struct multi_context *m, struct context *t); - -void multi_uninit(struct multi_context *m); - -void multi_top_init(struct multi_context *m, struct context *top); - -void multi_top_free(struct multi_context *m); - struct multi_instance *multi_create_instance(struct multi_context *m, const struct mroute_addr *real, struct link_socket *sock); diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index 3c8ce39..7d48888 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -491,6 +491,9 @@ * CM_P2P, \c CM_TOP, \c CM_TOP_CLONE, * \c CM_CHILD_UDP, and \c CM_CHILD_TCP. */ + struct multi_context *multi; /**< Pointer to the main P2MP context. + * Non-NULL only when mode == CM_TOP. */ + struct gc_arena gc; /**< Garbage collection arena for * allocations done in the scope of this * context structure. */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1093?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ibf64c681e02ac572d339d4d98e75ceb0cd417c45 Gerrit-Change-Number: 1093 Gerrit-PatchSet: 2 Gerrit-Owner: ordex <an...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: newpatchset |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X