| Message ID | 20211026215953.8951-1-luizluca@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | [1/2] interface-ip: copy more info for target host route | expand |
On 2021-10-26 23:59, luizluca@gmail.com wrote: > From: Luiz Angelo Daros de Luca <luizluca@gmail.com> > > interface_ip_add_target_route was adding a host route without > copying other confs like type, source, online). The result was that this: > > unreachable 192.168.0.9 metric 123 > > was being converted to: > > 192.168.0.9 dev lo scope link metric 123 > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > --- > interface-ip.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/interface-ip.c b/interface-ip.c > index 648f521..7c60fec 100644 > --- a/interface-ip.c > +++ b/interface-ip.c > @@ -301,9 +301,26 @@ interface_ip_add_target_route(union if_addr *addr, bool v6, struct interface *if > route->mask = v6 ? 128 : 32; > memcpy(&route->addr, addr, addrsize); > memcpy(&route->nexthop, &r_next->nexthop, sizeof(route->nexthop)); > - route->mtu = r_next->mtu; > - route->metric = r_next->metric; > - route->table = r_next->table; > + if (r_next->flags & DEVROUTE_MTU) { > + route->mtu = r_next->mtu; > + route->flags |= DEVROUTE_MTU; > + } > + if (r_next->flags & DEVROUTE_METRIC) { > + route->metric = r_next->metric; > + route->flags |= DEVROUTE_METRIC; > + } > + if (r_next->flags & DEVROUTE_TABLE) { > + route->table = r_next->table; > + route->flags |= DEVROUTE_TABLE; > + } > + if (r_next->flags & DEVROUTE_TYPE) { > + route->type = r_next->type; > + route->flags |= DEVROUTE_TYPE; > + } > + if (r_next->flags & DEVROUTE_ONLINK) > + route->flags |= DEVROUTE_ONLINK; How about leaving the route->{mtu,metric,table} assignment as-is and doing something like this: route->flags |= r->next & (DEVROUTE_MTU | DEVROUTE_METRIC | DEVROUTE_TYPE | DEVROUTE_ONLINK); - Felix
> > @@ -301,9 +301,26 @@ interface_ip_add_target_route(union if_addr *addr, bool v6, struct interface *if > > route->mask = v6 ? 128 : 32; > > memcpy(&route->addr, addr, addrsize); > > memcpy(&route->nexthop, &r_next->nexthop, sizeof(route->nexthop)); > > - route->mtu = r_next->mtu; > > - route->metric = r_next->metric; > > - route->table = r_next->table; > > + if (r_next->flags & DEVROUTE_MTU) { > > + route->mtu = r_next->mtu; > > + route->flags |= DEVROUTE_MTU; > > + } > > + if (r_next->flags & DEVROUTE_METRIC) { > > + route->metric = r_next->metric; > > + route->flags |= DEVROUTE_METRIC; > > + } > > + if (r_next->flags & DEVROUTE_TABLE) { > > + route->table = r_next->table; > > + route->flags |= DEVROUTE_TABLE; > > + } > > + if (r_next->flags & DEVROUTE_TYPE) { > > + route->type = r_next->type; > > + route->flags |= DEVROUTE_TYPE; > > + } > > + if (r_next->flags & DEVROUTE_ONLINK) > > + route->flags |= DEVROUTE_ONLINK; > How about leaving the route->{mtu,metric,table} assignment as-is and > doing something like this: > route->flags |= r->next & (DEVROUTE_MTU | DEVROUTE_METRIC | > DEVROUTE_TYPE | DEVROUTE_ONLINK); > Sure. I'll resent as the code bellow after your ACK. I noticed that metric was being added to the OS route even though DEVROUTE_METRIC was never set. The same might also happen with mtu and table (however, I didn't test it). You didn't mention DEVROUTE_TABLE in your suggestion but I added it anyway. I think that, in fact, table will never be used as interface_ip_find_route_target() will ignore routes with the table flag. Should I remove it, both the flag and the assignment, or it is not worth it? I added source/mask even without using it in my case as I think a new route without it would break the original route proposal. It is still missing "valid_until" but I'm not sure if it should be added or not as it might get out of sync with the original route if that one gets updated. @@ -298,12 +300,17 @@ interface_ip_add_target_route(union if_addr *addr, bool v6, struct interface *if return NULL; route->flags = v6 ? DEVADDR_INET6 : DEVADDR_INET4; + route->flags |= r->next & (DEVROUTE_MTU | DEVROUTE_METRIC | + DEVROUTE_TYPE | DEVROUTE_ONLINK | DEVROUTE_TABLE); route->mask = v6 ? 128 : 32; memcpy(&route->addr, addr, addrsize); memcpy(&route->nexthop, &r_next->nexthop, sizeof(route->nexthop)); route->mtu = r_next->mtu; route->metric = r_next->metric; route->table = r_next->table; + route->type = r_next->type; + memcpy(&route->source, &r_next->source, addrsize); + route->sourcemask = r_next->sourcemask; route->iface = iface; vlist_add(&iface->host_routes, &route->node, route);
diff --git a/interface-ip.c b/interface-ip.c index 648f521..7c60fec 100644 --- a/interface-ip.c +++ b/interface-ip.c @@ -301,9 +301,26 @@ interface_ip_add_target_route(union if_addr *addr, bool v6, struct interface *if route->mask = v6 ? 128 : 32; memcpy(&route->addr, addr, addrsize); memcpy(&route->nexthop, &r_next->nexthop, sizeof(route->nexthop)); - route->mtu = r_next->mtu; - route->metric = r_next->metric; - route->table = r_next->table; + if (r_next->flags & DEVROUTE_MTU) { + route->mtu = r_next->mtu; + route->flags |= DEVROUTE_MTU; + } + if (r_next->flags & DEVROUTE_METRIC) { + route->metric = r_next->metric; + route->flags |= DEVROUTE_METRIC; + } + if (r_next->flags & DEVROUTE_TABLE) { + route->table = r_next->table; + route->flags |= DEVROUTE_TABLE; + } + if (r_next->flags & DEVROUTE_TYPE) { + route->type = r_next->type; + route->flags |= DEVROUTE_TYPE; + } + if (r_next->flags & DEVROUTE_ONLINK) + route->flags |= DEVROUTE_ONLINK; + memcpy(&route->source, &r_next->source, addrsize); + route->sourcemask = r_next->sourcemask; route->iface = iface; vlist_add(&iface->host_routes, &route->node, route);