Hi,
On 13/04/2021 14:20, Vladislav Grishenko wrote:
> Current default gateway selection for zero destignation address just
little typ0: destiGnation
> dumps and parses all the routing tables. If any of non-main table
> with default route comes first, wrong default gateway can be picked.
> Since adding/removing routes currently handles only main table,
> let's stick to RT_TABLE_MAIN while selecting default route too.
>
> Signed-off-by: Vladislav Grishenko <themi...@yandex-team.ru>
> ---
> src/openvpn/networking_sitnl.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
> index 2bc70a50..56543648 100644
> --- a/src/openvpn/networking_sitnl.c
> +++ b/src/openvpn/networking_sitnl.c
> @@ -426,6 +426,7 @@ typedef struct {
> inet_address_t gw;
> char iface[IFNAMSIZ];
> bool default_only;
> + unsigned int table;
> } route_res_t;
>
> static int
> @@ -435,7 +436,7 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
> struct rtmsg *r = NLMSG_DATA(n);
> struct rtattr *rta = RTM_RTA(r);
> int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
> - unsigned int ifindex = 0;
> + unsigned int table, ifindex = 0;
>
> /* filter-out non-zero dst prefixes */
> if (res->default_only && r->rtm_dst_len != 0)
> @@ -443,6 +444,9 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
> return 1;
> }
>
> + /* route table, ignored with RTA_TABLE */
> + table = r->rtm_table;
> +
> while (RTA_OK(rta, len))
> {
> switch (rta->rta_type)
> @@ -460,11 +464,22 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
> case RTA_GATEWAY:
> memcpy(&res->gw, RTA_DATA(rta), res->addr_size);
> break;
> +
> + /* route table */
> + case RTA_TABLE:
> + table = *(unsigned int *)RTA_DATA(rta);
> + break;
> }
>
> rta = RTA_NEXT(rta, len);
> }
>
> + /* filter-out zero dns prefixes from other tables */
This comments seems off?
I'd suggest something like:
/* filter out any route not coming from the selected table */
> + if (res->table && res->table != table)
> + {
> + return 1;
Am I wrong or at this point we have already copied the GW to res->gw
thus, the caller will believe we have a result?
IMHO we should save the GW pointer in a temporary variable and copy it
only after this check has passed.
As a side notw, could you please resend this patch based on release/2.5?
We should start by fixing the bug there and then porting this patch to
master.
Regards,
> + }
> +
> if (!if_indextoname(ifindex, res->iface))
> {
> msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifname for index %d",
> @@ -507,6 +522,7 @@ sitnl_route_best_gw(sa_family_t af_family, const
> inet_address_t *dst,
> {
> req.n.nlmsg_flags |= NLM_F_DUMP;
> res.default_only = true;
> + res.table = RT_TABLE_MAIN;
> }
> else
> {
>
--
Antonio Quartulli
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
