Date: Sun, 4 Oct 2020 13:24:58 +0000 (UTC) From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r366424 - in head: sys/net sys/net/route tests/sys/net/routing Message-ID: <202010041324.094DOwRj023164@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202010041324.094DOwRj023164>