Attention is currently required from: flichtenheld, mrbff.

plaisthos has posted comments on this change by mrbff. ( 
http://gerrit.openvpn.net/c/openvpn/+/721?usp=email )

Change subject: route: extended logic to omit gateway when unnecessary
......................................................................


Patch Set 11: Code-Review-2

(7 comments)

Patchset:

PS11:
I am seriously confused by this PR. The premise of the PR seems that routes 
that are setup on the startup of OpenVPN (CM_TOP) need to be handled differrent 
than routes on the startup of a client or routes that are added later, e.g. by 
CCD files but I do not see a strong reason why these initial routes should be 
special in that regard.


File src/openvpn/route.h:

http://gerrit.openvpn.net/c/openvpn/+/721/comment/c375c7a6_648618a0?usp=email :
PS11, Line 274: bool add_route_ipv6(struct route_ipv6 *r, const struct tuntap 
*tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx, 
const bool is_multipoint);
The naming here and the usage seem to disagree. `is_multipoint` sounds like the 
usage in a p2mp server context but the actual usage is that this flag is only 
used for the CM_TOP instance, which is more the initialisation of p2mp on 
startup. So this naming is quite confusing here.

Also why adding a flag as bool to the function vs adding it as extra flag in 
the route structure.


File src/openvpn/route.c:

http://gerrit.openvpn.net/c/openvpn/+/721/comment/d77240b0_a0c41a4e?usp=email :
PS11, Line 1811:     /* FIXME -- add on-link support for Dragonfly */
Has this been really tested on DragonFly BSD?


http://gerrit.openvpn.net/c/openvpn/+/721/comment/f8856fdf_5f40d6a4?usp=email :
PS11, Line 1600:         && !( (r4->flags & RT_METRIC_DEFINED) && r4->metric == 
0 ) )
how does the metric play into needing a gateway or not?


http://gerrit.openvpn.net/c/openvpn/+/721/comment/ac755fc1_a42389d2?usp=email :
PS11, Line 1607:         return true;
A little bit of explanation why DCO is different is needed.


http://gerrit.openvpn.net/c/openvpn/+/721/comment/657a25df_8073c041?usp=email :
PS11, Line 1898:     }
Has this been tested on Open and NetBSD?


http://gerrit.openvpn.net/c/openvpn/+/721/comment/715e26fa_29fc79e0?usp=email :
PS11, Line 2010: {
I think this duplicates the code that was publicly accessible in 
https://gerrit.openvpn.net/c/openvpn/+/1191



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/721?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I87777e74b1fd34781e1d72c9f994eb84f39d800c
Gerrit-Change-Number: 721
Gerrit-PatchSet: 11
Gerrit-Owner: mrbff <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Attention: mrbff <[email protected]>
Gerrit-Comment-Date: Mon, 13 Oct 2025 15:01:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to