Fix route flags update during RTM_CHANGE.

Nexthop lookup was not consireding rt_flags when doing
 structure comparison, which lead to an original nexthop
 selection when changing flags. Fix the case by adding
 rt_flags field into comparison and rearranging nhop_priv
 fields to allow for efficient matching.
Fix `route change X/Y flags` case - recent changes
 disallowed specifying RTF_GATEWAY flag without actual gateway.
 It turns out, route(8) fills in RTF_GATEWAY by default, unless
 -interface flag is specified. Fix regression by clearing
 RTF_GATEWAY flag instead of failing.
Fix route flag reporting in RTM_CHANGE messages by explicitly
 updating rtm_flags after operation competion.
Add IPv4/IPv6 tests for flag-only route changes.
This commit is contained in:
Alexander V. Chernikov 2020-10-04 13:24:58 +00:00
parent df01340989
commit 1b95005e95
5 changed files with 129 additions and 17 deletions

View File

@ -164,8 +164,7 @@ cmp_priv(const struct nhop_priv *_one, const struct nhop_priv *_two)
if (memcmp(_one->nh, _two->nh, NHOP_END_CMP) != 0) if (memcmp(_one->nh, _two->nh, NHOP_END_CMP) != 0)
return (0); return (0);
if ((_one->nh_type != _two->nh_type) || if (memcmp(_one, _two, NH_PRIV_END_CMP) != 0)
(_one->nh_family != _two->nh_family))
return (0); return (0);
return (1); return (1);

View File

@ -74,13 +74,16 @@ struct nh_control {
/* Control plane-only nhop data */ /* Control plane-only nhop data */
struct nhop_object; struct nhop_object;
struct nhop_priv { struct nhop_priv {
uint32_t nh_idx; /* nexthop index */ /* nhop lookup comparison start */
uint8_t nh_family; /* address family of the lookup */ uint8_t nh_family; /* address family of the lookup */
uint8_t spare;
uint16_t nh_type; /* nexthop type */ uint16_t nh_type; /* nexthop type */
uint32_t rt_flags; /* routing flags for the control plane */
/* nhop lookup comparison end */
uint32_t nh_idx; /* nexthop index */
void *cb_func; /* function handling additional rewrite caps */ void *cb_func; /* function handling additional rewrite caps */
u_int nh_refcnt; /* number of references, refcount(9) */ u_int nh_refcnt; /* number of references, refcount(9) */
u_int nh_linked; /* refcount(9), == 2 if linked to the list */ u_int nh_linked; /* refcount(9), == 2 if linked to the list */
int rt_flags; /* routing flags for the control plane */
struct nhop_object *nh; /* backreference to the dataplane nhop */ struct nhop_object *nh; /* backreference to the dataplane nhop */
struct nh_control *nh_control; /* backreference to the rnh */ struct nh_control *nh_control; /* backreference to the rnh */
struct nhop_priv *nh_next; /* hash table membership */ struct nhop_priv *nh_next; /* hash table membership */
@ -88,6 +91,8 @@ struct nhop_priv {
struct epoch_context nh_epoch_ctx; /* epoch data for nhop */ struct epoch_context nh_epoch_ctx; /* epoch data for nhop */
}; };
#define NH_PRIV_END_CMP (__offsetof(struct nhop_priv, nh_idx))
#define NH_IS_PINNED(_nh) ((!NH_IS_NHGRP(_nh)) && \ #define NH_IS_PINNED(_nh) ((!NH_IS_NHGRP(_nh)) && \
((_nh)->nh_priv->rt_flags & RTF_PINNED)) ((_nh)->nh_priv->rt_flags & RTF_PINNED))

View File

@ -733,8 +733,15 @@ rib_change_route(uint32_t fibnum, struct rt_addrinfo *info,
/* Check if updated gateway exists */ /* Check if updated gateway exists */
if ((info->rti_flags & RTF_GATEWAY) && if ((info->rti_flags & RTF_GATEWAY) &&
(info->rti_info[RTAX_GATEWAY] == NULL)) (info->rti_info[RTAX_GATEWAY] == NULL)) {
return (EINVAL);
/*
* route(8) adds RTF_GATEWAY flag if -interface is not set.
* Remove RTF_GATEWAY to enforce consistency and maintain
* compatibility..
*/
info->rti_flags &= ~RTF_GATEWAY;
}
/* /*
* route change is done in multiple steps, with dropping and * route change is done in multiple steps, with dropping and

View File

@ -965,6 +965,7 @@ route_output(struct mbuf *m, struct socket *so, ...)
#endif #endif
nh = rc.rc_nh_new; nh = rc.rc_nh_new;
rtm->rtm_index = nh->nh_ifp->if_index; rtm->rtm_index = nh->nh_ifp->if_index;
rtm->rtm_flags = rc.rc_rt->rte_flags | nhop_get_rtflags(nh);
} }
break; break;

View File

@ -431,8 +431,8 @@ ATF_TC_BODY(rtm_add_v4_gw_direct_success, tc)
verify_route_message(rtm, RTM_ADD, (struct sockaddr *)&net4, verify_route_message(rtm, RTM_ADD, (struct sockaddr *)&net4,
(struct sockaddr *)&mask4, (struct sockaddr *)&gw4); (struct sockaddr *)&mask4, (struct sockaddr *)&gw4);
/* XXX: Currently kernel sets RTF_UP automatically but does NOT report it in the reply */ verify_route_message_extra(rtm, c->ifindex,
verify_route_message_extra(rtm, c->ifindex, RTF_DONE | RTF_GATEWAY | RTF_STATIC); RTF_UP | RTF_DONE | RTF_GATEWAY | RTF_STATIC);
} }
ATF_TC_CLEANUP(rtm_add_v4_gw_direct_success, tc) ATF_TC_CLEANUP(rtm_add_v4_gw_direct_success, tc)
@ -568,7 +568,7 @@ ATF_TC_BODY(rtm_change_v4_gw_success, tc)
(struct sockaddr *)&mask4, (struct sockaddr *)&gw4); (struct sockaddr *)&mask4, (struct sockaddr *)&gw4);
verify_route_message_extra(rtm, if_nametoindex(c->ifnames[1]), verify_route_message_extra(rtm, if_nametoindex(c->ifnames[1]),
RTF_DONE | RTF_GATEWAY | RTF_STATIC); RTF_UP | RTF_DONE | RTF_GATEWAY | RTF_STATIC);
/* Verify the change has actually taken place */ /* Verify the change has actually taken place */
prepare_route_message(rtm, RTM_GET, (struct sockaddr *)&net4, prepare_route_message(rtm, RTM_GET, (struct sockaddr *)&net4,
@ -591,7 +591,7 @@ ATF_TC_BODY(rtm_change_v4_gw_success, tc)
} }
RTM_DECLARE_ROOT_TEST(rtm_change_v4_mtu_success, RTM_DECLARE_ROOT_TEST(rtm_change_v4_mtu_success,
"Tests IPv4 path mtu change"); "Tests IPv4 path mtu change");
ATF_TC_BODY(rtm_change_v4_mtu_success, tc) ATF_TC_BODY(rtm_change_v4_mtu_success, tc)
{ {
@ -639,6 +639,55 @@ ATF_TC_BODY(rtm_change_v4_mtu_success, tc)
"expected mtu: %lu, got %lu", test_mtu, rtm->rtm_rmx.rmx_mtu); "expected mtu: %lu, got %lu", test_mtu, rtm->rtm_rmx.rmx_mtu);
} }
RTM_DECLARE_ROOT_TEST(rtm_change_v4_flags_success,
"Tests IPv4 path flags change");
ATF_TC_BODY(rtm_change_v4_flags_success, tc)
{
DECLARE_TEST_VARS;
uint32_t test_flags = RTF_PROTO1 | RTF_PROTO2 | RTF_PROTO3 | RTF_STATIC;
uint32_t desired_flags;
c = presetup_ipv4(tc);
/* Create IPv4 subnetwork with smaller prefix */
struct sockaddr_in mask4;
struct sockaddr_in net4;
struct sockaddr_in gw4;
prepare_v4_network(c, &net4, &mask4, &gw4);
prepare_route_message(rtm, RTM_ADD, (struct sockaddr *)&net4,
(struct sockaddr *)&mask4, (struct sockaddr *)&gw4);
/* Set test flags during route addition */
desired_flags = RTF_UP | RTF_DONE | RTF_GATEWAY | test_flags;
rtm->rtm_flags |= test_flags;
rtsock_send_rtm(c->rtsock_fd, rtm);
rtm = rtsock_read_rtm_reply(c->rtsock_fd, buffer, sizeof(buffer), rtm->rtm_seq);
/* Change flags */
prepare_route_message(rtm, RTM_CHANGE, (struct sockaddr *)&net4,
(struct sockaddr *)&mask4, NULL);
rtm->rtm_flags &= ~test_flags;
desired_flags &= ~test_flags;
rtsock_send_rtm(c->rtsock_fd, rtm);
rtm = rtsock_read_rtm_reply(c->rtsock_fd, buffer, sizeof(buffer), rtm->rtm_seq);
/* Verify updated flags */
verify_route_message_extra(rtm, c->ifindex, desired_flags | RTF_DONE);
/* Verify the change has actually taken place */
prepare_route_message(rtm, RTM_GET, (struct sockaddr *)&net4,
(struct sockaddr *)&mask4, NULL);
rtsock_send_rtm(c->rtsock_fd, rtm);
rtm = rtsock_read_rtm_reply(c->rtsock_fd, buffer, sizeof(buffer), rtm->rtm_seq);
verify_route_message_extra(rtm, c->ifindex, desired_flags | RTF_DONE);
}
ATF_TC_WITH_CLEANUP(rtm_add_v6_gu_gw_gu_direct_success); ATF_TC_WITH_CLEANUP(rtm_add_v6_gu_gw_gu_direct_success);
ATF_TC_HEAD(rtm_add_v6_gu_gw_gu_direct_success, tc) ATF_TC_HEAD(rtm_add_v6_gu_gw_gu_direct_success, tc)
@ -674,8 +723,8 @@ ATF_TC_BODY(rtm_add_v6_gu_gw_gu_direct_success, tc)
verify_route_message(rtm, RTM_ADD, (struct sockaddr *)&net6, verify_route_message(rtm, RTM_ADD, (struct sockaddr *)&net6,
(struct sockaddr *)&mask6, (struct sockaddr *)&gw6); (struct sockaddr *)&mask6, (struct sockaddr *)&gw6);
/* XXX: Currently kernel sets RTF_UP automatically but does NOT report it in the reply */ verify_route_message_extra(rtm, c->ifindex,
verify_route_message_extra(rtm, c->ifindex, RTF_DONE | RTF_GATEWAY | RTF_STATIC); RTF_UP | RTF_DONE | RTF_GATEWAY | RTF_STATIC);
} }
ATF_TC_CLEANUP(rtm_add_v6_gu_gw_gu_direct_success, tc) ATF_TC_CLEANUP(rtm_add_v6_gu_gw_gu_direct_success, tc)
@ -791,7 +840,7 @@ ATF_TC_BODY(rtm_change_v6_gw_success, tc)
(struct sockaddr *)&mask6, (struct sockaddr *)&gw6); (struct sockaddr *)&mask6, (struct sockaddr *)&gw6);
verify_route_message_extra(rtm, if_nametoindex(c->ifnames[1]), verify_route_message_extra(rtm, if_nametoindex(c->ifnames[1]),
RTF_DONE | RTF_GATEWAY | RTF_STATIC); RTF_UP | RTF_DONE | RTF_GATEWAY | RTF_STATIC);
/* Verify the change has actually taken place */ /* Verify the change has actually taken place */
prepare_route_message(rtm, RTM_GET, (struct sockaddr *)&net6, prepare_route_message(rtm, RTM_GET, (struct sockaddr *)&net6,
@ -862,6 +911,55 @@ ATF_TC_BODY(rtm_change_v6_mtu_success, tc)
"expected mtu: %lu, got %lu", test_mtu, rtm->rtm_rmx.rmx_mtu); "expected mtu: %lu, got %lu", test_mtu, rtm->rtm_rmx.rmx_mtu);
} }
RTM_DECLARE_ROOT_TEST(rtm_change_v6_flags_success,
"Tests IPv6 path flags change");
ATF_TC_BODY(rtm_change_v6_flags_success, tc)
{
DECLARE_TEST_VARS;
uint32_t test_flags = RTF_PROTO1 | RTF_PROTO2 | RTF_PROTO3 | RTF_STATIC;
uint32_t desired_flags;
c = presetup_ipv6(tc);
/* Create IPv6 subnetwork with smaller prefix */
struct sockaddr_in6 mask6;
struct sockaddr_in6 net6;
struct sockaddr_in6 gw6;
prepare_v6_network(c, &net6, &mask6, &gw6);
prepare_route_message(rtm, RTM_ADD, (struct sockaddr *)&net6,
(struct sockaddr *)&mask6, (struct sockaddr *)&gw6);
/* Set test flags during route addition */
desired_flags = RTF_UP | RTF_DONE | RTF_GATEWAY | test_flags;
rtm->rtm_flags |= test_flags;
rtsock_send_rtm(c->rtsock_fd, rtm);
rtm = rtsock_read_rtm_reply(c->rtsock_fd, buffer, sizeof(buffer), rtm->rtm_seq);
/* Change flags */
prepare_route_message(rtm, RTM_CHANGE, (struct sockaddr *)&net6,
(struct sockaddr *)&mask6, NULL);
rtm->rtm_flags &= ~test_flags;
desired_flags &= ~test_flags;
rtsock_send_rtm(c->rtsock_fd, rtm);
rtm = rtsock_read_rtm_reply(c->rtsock_fd, buffer, sizeof(buffer), rtm->rtm_seq);
/* Verify updated flags */
verify_route_message_extra(rtm, c->ifindex, desired_flags | RTF_DONE);
/* Verify the change has actually taken place */
prepare_route_message(rtm, RTM_GET, (struct sockaddr *)&net6,
(struct sockaddr *)&mask6, NULL);
rtsock_send_rtm(c->rtsock_fd, rtm);
rtm = rtsock_read_rtm_reply(c->rtsock_fd, buffer, sizeof(buffer), rtm->rtm_seq);
verify_route_message_extra(rtm, c->ifindex, desired_flags | RTF_DONE);
}
ATF_TC_WITH_CLEANUP(rtm_add_v4_temporal1_success); ATF_TC_WITH_CLEANUP(rtm_add_v4_temporal1_success);
ATF_TC_HEAD(rtm_add_v4_temporal1_success, tc) ATF_TC_HEAD(rtm_add_v4_temporal1_success, tc)
{ {
@ -900,7 +998,8 @@ ATF_TC_BODY(rtm_add_v4_temporal1_success, tc)
verify_route_message(rtm, RTM_DELETE, (struct sockaddr *)&net4, verify_route_message(rtm, RTM_DELETE, (struct sockaddr *)&net4,
(struct sockaddr *)&mask4, (struct sockaddr *)&gw4); (struct sockaddr *)&mask4, (struct sockaddr *)&gw4);
verify_route_message_extra(rtm, c->ifindex, RTF_GATEWAY | RTF_DONE | RTF_STATIC); verify_route_message_extra(rtm, c->ifindex,
RTF_DONE | RTF_GATEWAY | RTF_STATIC);
} }
ATF_TC_CLEANUP(rtm_add_v4_temporal1_success, tc) ATF_TC_CLEANUP(rtm_add_v4_temporal1_success, tc)
@ -946,9 +1045,8 @@ ATF_TC_BODY(rtm_add_v6_temporal1_success, tc)
verify_route_message(rtm, RTM_DELETE, (struct sockaddr *)&net6, verify_route_message(rtm, RTM_DELETE, (struct sockaddr *)&net6,
(struct sockaddr *)&mask6, (struct sockaddr *)&gw6); (struct sockaddr *)&mask6, (struct sockaddr *)&gw6);
verify_route_message_extra(rtm, c->ifindex,
/* XXX: Currently kernel sets RTF_UP automatically but does NOT report it in the reply */ RTF_DONE | RTF_GATEWAY | RTF_STATIC);
verify_route_message_extra(rtm, c->ifindex, RTF_GATEWAY | RTF_DONE | RTF_STATIC);
} }
ATF_TC_CLEANUP(rtm_add_v6_temporal1_success, tc) ATF_TC_CLEANUP(rtm_add_v6_temporal1_success, tc)
@ -1299,8 +1397,10 @@ ATF_TP_ADD_TCS(tp)
ATF_TP_ADD_TC(tp, rtm_del_v6_gu_prefix_nogw_success); ATF_TP_ADD_TC(tp, rtm_del_v6_gu_prefix_nogw_success);
ATF_TP_ADD_TC(tp, rtm_change_v4_gw_success); ATF_TP_ADD_TC(tp, rtm_change_v4_gw_success);
ATF_TP_ADD_TC(tp, rtm_change_v4_mtu_success); ATF_TP_ADD_TC(tp, rtm_change_v4_mtu_success);
ATF_TP_ADD_TC(tp, rtm_change_v4_flags_success);
ATF_TP_ADD_TC(tp, rtm_change_v6_gw_success); ATF_TP_ADD_TC(tp, rtm_change_v6_gw_success);
ATF_TP_ADD_TC(tp, rtm_change_v6_mtu_success); ATF_TP_ADD_TC(tp, rtm_change_v6_mtu_success);
ATF_TP_ADD_TC(tp, rtm_change_v6_flags_success);
/* ifaddr tests */ /* ifaddr tests */
ATF_TP_ADD_TC(tp, rtm_add_v6_gu_ifa_hostroute_success); ATF_TP_ADD_TC(tp, rtm_add_v6_gu_ifa_hostroute_success);
ATF_TP_ADD_TC(tp, rtm_add_v6_gu_ifa_prefixroute_success); ATF_TP_ADD_TC(tp, rtm_add_v6_gu_ifa_prefixroute_success);