Author: melifaro
Date: Sun Oct 4 13:24:58 2020
New Revision: 366424
URL: https://svnweb.freebsd.org/changeset/base/366424
Log:
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.
Modified:
head/sys/net/route/nhop_ctl.c
head/sys/net/route/nhop_var.h
head/sys/net/route/route_ctl.c
head/sys/net/rtsock.c
head/tests/sys/net/routing/test_rtsock_l3.c
Modified: head/sys/net/route/nhop_ctl.c
==============================================================================
--- head/sys/net/route/nhop_ctl.c Sun Oct 4 06:14:51 2020
(r366423)
+++ head/sys/net/route/nhop_ctl.c Sun Oct 4 13:24:58 2020
(r366424)
@@ -164,8 +164,7 @@ cmp_priv(const struct nhop_priv *_one, const struct nh
if (memcmp(_one->nh, _two->nh, NHOP_END_CMP) != 0)
return (0);
- if ((_one->nh_type != _two->nh_type) ||
- (_one->nh_family != _two->nh_family))
+ if (memcmp(_one, _two, NH_PRIV_END_CMP) != 0)
return (0);
return (1);
Modified: head/sys/net/route/nhop_var.h
==============================================================================
--- head/sys/net/route/nhop_var.h Sun Oct 4 06:14:51 2020
(r366423)
+++ head/sys/net/route/nhop_var.h Sun Oct 4 13:24:58 2020
(r366424)
@@ -74,19 +74,24 @@ struct nh_control {
/* Control plane-only nhop data */
struct nhop_object;
struct nhop_priv {
- uint32_t nh_idx; /* nexthop index */
+ /* nhop lookup comparison start */
uint8_t nh_family; /* address family of the lookup
*/
+ uint8_t spare;
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 */
u_int nh_refcnt; /* number of references,
refcount(9) */
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 nh_control *nh_control; /* backreference to the rnh */
struct nhop_priv *nh_next; /* hash table membership */
struct vnet *nh_vnet; /* vnet nhop belongs to */
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)) && \
((_nh)->nh_priv->rt_flags & RTF_PINNED))
Modified: head/sys/net/route/route_ctl.c
==============================================================================
--- head/sys/net/route/route_ctl.c Sun Oct 4 06:14:51 2020
(r366423)
+++ head/sys/net/route/route_ctl.c Sun Oct 4 13:24:58 2020
(r366424)
@@ -733,8 +733,15 @@ rib_change_route(uint32_t fibnum, struct rt_addrinfo *
/* Check if updated gateway exists */
if ((info->rti_flags & RTF_GATEWAY) &&
- (info->rti_info[RTAX_GATEWAY] == NULL))
- return (EINVAL);
+ (info->rti_info[RTAX_GATEWAY] == NULL)) {
+
+ /*
+ * 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
Modified: head/sys/net/rtsock.c
==============================================================================
--- head/sys/net/rtsock.c Sun Oct 4 06:14:51 2020 (r366423)
+++ head/sys/net/rtsock.c Sun Oct 4 13:24:58 2020 (r366424)
@@ -965,6 +965,7 @@ route_output(struct mbuf *m, struct socket *so, ...)
#endif
nh = rc.rc_nh_new;
rtm->rtm_index = nh->nh_ifp->if_index;
+ rtm->rtm_flags = rc.rc_rt->rte_flags |
nhop_get_rtflags(nh);
}
break;
Modified: head/tests/sys/net/routing/test_rtsock_l3.c
==============================================================================
--- head/tests/sys/net/routing/test_rtsock_l3.c Sun Oct 4 06:14:51 2020
(r366423)
+++ head/tests/sys/net/routing/test_rtsock_l3.c Sun Oct 4 13:24:58 2020
(r366424)
@@ -431,8 +431,8 @@ ATF_TC_BODY(rtm_add_v4_gw_direct_success, tc)
verify_route_message(rtm, RTM_ADD, (struct sockaddr *)&net4,
(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, RTF_DONE | RTF_GATEWAY |
RTF_STATIC);
+ verify_route_message_extra(rtm, c->ifindex,
+ RTF_UP | RTF_DONE | RTF_GATEWAY | RTF_STATIC);
}
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);
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 */
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,
- "Tests IPv4 path mtu change");
+ "Tests IPv4 path mtu change");
ATF_TC_BODY(rtm_change_v4_mtu_success, tc)
{
@@ -639,7 +639,56 @@ ATF_TC_BODY(rtm_change_v4_mtu_success, tc)
"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_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,
(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, RTF_DONE | RTF_GATEWAY |
RTF_STATIC);
+ verify_route_message_extra(rtm, c->ifindex,
+ RTF_UP | RTF_DONE | RTF_GATEWAY | RTF_STATIC);
}
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);
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 */
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);
}
+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_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,
(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)
@@ -946,9 +1045,8 @@ ATF_TC_BODY(rtm_add_v6_temporal1_success, tc)
verify_route_message(rtm, RTM_DELETE, (struct sockaddr *)&net6,
(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, RTF_GATEWAY | RTF_DONE |
RTF_STATIC);
+ verify_route_message_extra(rtm, c->ifindex,
+ RTF_DONE | RTF_GATEWAY | RTF_STATIC);
}
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_change_v4_gw_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_mtu_success);
+ ATF_TP_ADD_TC(tp, rtm_change_v6_flags_success);
/* ifaddr tests */
ATF_TP_ADD_TC(tp, rtm_add_v6_gu_ifa_hostroute_success);
ATF_TP_ADD_TC(tp, rtm_add_v6_gu_ifa_prefixroute_success);
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
