Attention is currently required from: its_Giaan, plaisthos.
cron2 has posted comments on this change. (
http://gerrit.openvpn.net/c/openvpn/+/524?usp=email )
Change subject: Route: add support for user defined routing table
......................................................................
Patch Set 4: Code-Review-1
(1 comment)
Patchset:
PS4:
Sorry for being late to the party - I have looked at it, and while the general
code works, I find it a bit excessive in lines of code. Namely, I would opt
for "do not support route-table as an extra argument to `route`", as this + the
checks really makes up half the code - for little value. Since it's not
supposed to be pushable, routes-with-table need to be in the client config, and
in that case, I could just put
```
route-table 123
route
route
route-table 456
route
route
route-table 999
# for everything that comes in pushed
```
into my config. If I ever really need more than 1-2 route tables.
Also, we shouldn't have `#ifdef ENABLE_SITNL` in the parser for `route` and
`route-ipv6`...
Further, the hunk
```
+ int table_id = 0; /* unspec table */
...
+ if (options->route_default_table_id)
+ {
+ table_id = options->route_default_table_id;
+ }
```
is sort of semi-useful - if route_default_table_id is 0, we do not assign it to
table_id, which is also 0. Why bother with an extra variable here that will
have the same value as the `options` variable, at all times? I'd just pass
`options->route_default_table_id` downwards, and be done with it.
The `do_init_route_ipv6_list()` code also does the `table_id = ...` dance, but
then passes NULL towards `add_route_ipv6_to_option_list()`, which does not look
intentional...
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/524?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: I3e4ebef484d2a04a383a65ede5617ee98bf218a7
Gerrit-Change-Number: 524
Gerrit-PatchSet: 4
Gerrit-Owner: its_Giaan <[email protected]>
Gerrit-Reviewer: cron2 <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: its_Giaan <[email protected]>
Gerrit-Comment-Date: Wed, 19 Jun 2024 09:13:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
