Hi,
> -----Original Message-----
> From: gianma...@mandelbit.com <gianma...@mandelbit.com>
> Sent: mercoledì 19 aprile 2023 12:28
> To: 'Frank Lichtenheld' <fr...@lichtenheld.com>
> Cc: openvpn-devel@lists.sourceforge.net
> Subject: Re: [Openvpn-devel] [PATCH v4] Route: add support for user
defined
> routing table
>
>
> Hi,
>
> > -----Original Message-----
> > From: Frank Lichtenheld <fr...@lichtenheld.com>
> > Sent: mercoledì 19 aprile 2023 11:56
> > To: Gianmarco De Gregori <gianma...@mandelbit.com>
> > Cc: openvpn-devel@lists.sourceforge.net
> > Subject: Re: [Openvpn-devel] [PATCH v4] Route: add support for user
> defined
> > routing table
> >
> > On Tue, Apr 18, 2023 at 05:26:55PM +0200, Gianmarco De Gregori wrote:
> > > Add the ability for users to specify a custom routing table where
> > > routes should be installed in.
> > > As of now routes are always installed in the main routing table of
> > > the operating system, however, with the new --route-table option it
> > > is possibile to specify the ID of the default routing table to be
> > > used by --route(-ipv6).
> > >
> > > The --route(-ipv6) directives have been extended with an additional
> > > argument (5th for --route) (4th for --route-ipv6) so that each of
> > > them can possibly use an independent routing table.
> > >
> > > Please note: this feature is currently supported only by Linux/SITNL.
> > > Support for other platforms should be added in related backends.
> > >
> > > Fixes: Trac #1399
> > > Signed-off-by: Gianmarco De Gregori <gianma...@mandelbit.com>
> > > ---
> > > Changes from v1:
> > > * Fixed parameters (metric and table_id) order in init_route_list()
> > > call
> in init.c
> > : 1535.
> > >
> > > Changes from v2:
> > > * Add route_default_table_id to show_settings() in options.c : 1800.
> > >
> > > Changes from v3:
> > > * Switched table_id data type from uint32_t to int.
> > > * Added discard to pulled routing table_id from server in case of
> > > pull
> mode.
> >
> > One question about that, see below.
> >
> > [...]
> > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c index
> > > 2680f268..a908566a 100644
> > > --- a/src/openvpn/options.c
> > > +++ b/src/openvpn/options.c
> >
> > side-note: --route-table missing in usage_message.
>
> Gonna add it soon, thanks for reminding.
>
> >
> > > @@ -1912,6 +1912,7 @@ show_settings(const struct options *o)
> > > SHOW_STR(route_script);
> > > SHOW_STR(route_default_gateway);
> > > SHOW_INT(route_default_metric);
> > > + SHOW_INT(route_default_table_id);
> > > SHOW_BOOL(route_noexec);
> > > SHOW_INT(route_delay);
> > > SHOW_INT(route_delay_window);
> > > @@ -6956,7 +6957,15 @@ add_option(struct options *options,
> > > cnol_check_alloc(options);
> > > add_client_nat_to_option_list(options->client_nat, p[1],
> > > p[2],
> p[3],
> > p[4], msglevel);
> > > }
> > > - else if (streq(p[0], "route") && p[1] && !p[5])
> > > + else if (streq(p[0], "route-table") && p[1] && !p[2])
> > > + {
> > > +#ifndef ENABLE_SITNL
> > > + msg(M_WARN, "NOTE: --route-table specified, but not
> > > +supported on this platform"); #endif
> > > + VERIFY_PERMISSION(OPT_P_ROUTE);
> > > + options->route_default_table_id = positive_atoi(p[1]);
> > > + }
> > > + else if (streq(p[0], "route") && p[1] && !p[6])
> > > {
> > > VERIFY_PERMISSION(OPT_P_ROUTE);
> > > rol_check_alloc(options);
> > > @@ -6977,10 +6986,31 @@ add_option(struct options *options,
> > > msg(msglevel, "route parameter gateway '%s' must be
> > > a
> valid
> > address", p[3]);
> > > goto err;
> > > }
> > > + /* p[4] is metric, if specified */
> > > +
> > > + /* discard pulled routing table_id from server
> > > + * since this must be an entirely local choice */
> >
> >
> > Don't you need that check for --route-table as well?
>
> There is no need since if the table_id is specified via --route-table that
will be
> used regardless, otherwise the main table with id 0 will be used.
Sure, I had not considered the case of pulling the route-table from server.
My bad, gonna add it soon also.
>
> >
> > > + if (p[5])
> > > + {
> > > + p[5] = NULL;
> > > + }
> > > + }
> > > + /* at the moment the routing table id is supported only by
> > > +Linux/SITNL */ #ifndef ENABLE_SITNL
> > > + if (p[5])
> > > + {
> > > + static bool route_table_warned = false;
> > > +
> > > + if (!route_table_warned)
> > > + {
> > > + msg(M_WARN, "NOTE: table specified for --route, but
> > > + not
> > supported on this platform");
> > > + route_table_warned = true;
> > > + }
> > > }
> > > - add_route_to_option_list(options->routes, p[1], p[2], p[3],
> p[4]);
> > > +#endif
> > > + add_route_to_option_list(options->routes, p[1], p[2], p[3],
> > > +p[4], p[5]);
> > > }
> > > - else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
> > > + else if (streq(p[0], "route-ipv6") && p[1] && !p[5])
> > > {
> > > VERIFY_PERMISSION(OPT_P_ROUTE);
> > > rol6_check_alloc(options);
> >
>
Regards,
Gianmarco De Gregori
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
