[LTP] [PATCH v5 2/2] net/route: Add netlink based route change tests
Alexey Kodanev
alexey.kodanev@oracle.com
Wed Apr 29 19:04:33 CEST 2020
On 29.04.2020 13:41, Petr Vorel wrote:
> Hi Alexey,
>
>> Minor comments below.
> Thanks for your review!
>
Hi Petr,
>>> + for (i = 0; i < num_loops; i++) {
>>> + if (iface_len > 1)
>>> + tst_res(TINFO, "testing gw: %s, iface: %s",
>>> + p_gw->ip_str, p_iface->iface);
>>> + else if (gw_len > 1)
>>> + tst_res(TINFO, "testing gw: %s", p_gw->ip_str);
>>> + else
>>> + tst_res(TINFO, "testing dst: %s/%d", p_dst->ip_str, prefix);
>
>
>> It would be better to avoid printing it on every iteration, especially
>> with the large NS_TIMES.
> Understand, I thought you wouldn't like it. How about print only on error?
>
>>> +
>>> + rtnl_route(p_iface->index, p_dst->ip, gw ? p_gw->ip : NULL,
>>> + prefix, RTM_NEWROUTE);
>>> + send_udp(p_rhost->ip);
>>> + rtnl_route(p_iface->index, p_dst->ip, gw ? p_gw->ip : NULL,
>>> + prefix, RTM_DELROUTE);
>>> +
>>> + if (gw)
>>> + p_gw = p_gw->next ?: gw;
>>> + p_dst = p_dst->next ?: dst;
>>> + p_iface = p_iface->next ?: iface;
>>> + p_rhost = p_rhost->next ?: rhost;
>>> + }
>>> +
>>> + tst_res(TPASS, "routes created and deleted");
>>> +}
>
> Something like this?
>
> static void print_route_info(int iface, struct sockaddr *dst,
> struct sockaddr *gw, int type)
> {
Perhaps brk_on_route_error() as it has tst_brk()?
> char dst_str[INET6_ADDRSTRLEN], gw_str[INET6_ADDRSTRLEN];
> tst_sock_addr(dst, sizeof(dst), dst_str, sizeof(dst_str));
> if (gw)
> tst_sock_addr(gw, sizeof(gw), gw_str, sizeof(gw_str));
>
> tst_res(TINFO, "type: %s, iface: %d, dst: %s, gw: %s",
> type == RTM_NEWROUTE ? "RTM_NEWROUTE" : "RTM_DELROUTE",
> iface, dst_str, gw ? gw_str : "null");
> tst_brk(TBROK, "failed due previous netlink errors");
"failed due to the previous..."
Also, what about passing the error message type and errno to this
function, i.e. changing "TFAIL, TINFO, TBROK" to "TINFO, TFAIL"?
static void brk_on_route_error(const char *msg, int errno, int iface, ...
{
...
tst_res(TINFO, "...");
tst_brk(TFAIL, "%s failed (errno=%d): %s", msg, errno, strerror(errno));
> }
>
> static void rtnl_route(int iface, struct addrinfo *dst, struct addrinfo *gw,
> int type)
> {
> struct mnl_socket *nl;
> char buf[MNL_SOCKET_BUFFER_SIZE];
> struct nlmsghdr *nlh;
> struct rtmsg *rtm;
> uint32_t seq, portid;
> struct in6_addr dst_in6, gw_in6;
> in_addr_t dst_ip, gw_ip;
> int ret;>
> nlh = mnl_nlmsg_put_header(buf);
> nlh->nlmsg_type = type;
>
> nlh->nlmsg_flags = NLM_F_ACK;
> if (type == RTM_NEWROUTE)
> nlh->nlmsg_flags |= NLM_F_REQUEST | NLM_F_CREATE | NLM_F_REPLACE;
>
> nlh->nlmsg_seq = seq = time(NULL);
>
> rtm = mnl_nlmsg_put_extra_header(nlh, sizeof(struct rtmsg));
> rtm->rtm_family = family;
> rtm->rtm_dst_len = prefix;
> rtm->rtm_src_len = 0;
> rtm->rtm_tos = 0;
> rtm->rtm_protocol = RTPROT_STATIC;
> rtm->rtm_table = RT_TABLE_MAIN;
> rtm->rtm_type = RTN_UNICAST;
> rtm->rtm_scope = gw ? RT_SCOPE_UNIVERSE : RT_SCOPE_LINK;
> rtm->rtm_flags = 0;
>
> if (is_ipv6) {
> dst_in6 = ((struct sockaddr_in6 *)dst->ai_addr)->sin6_addr;
> mnl_attr_put(nlh, RTA_DST, sizeof(struct in6_addr), &dst_in6);
> } else {
> dst_ip = ((struct sockaddr_in *)dst->ai_addr)->sin_addr.s_addr;
> mnl_attr_put_u32(nlh, RTA_DST, dst_ip);
> }
>
> mnl_attr_put_u32(nlh, RTA_OIF, iface);
>
> if (gw) {
> if (is_ipv6) {
> gw_in6 = ((struct sockaddr_in6 *)gw->ai_addr)->sin6_addr;
> mnl_attr_put(nlh, RTA_GATEWAY, sizeof(struct in6_addr), &gw_in6);
> } else {
> gw_ip = ((struct sockaddr_in *)gw->ai_addr)->sin_addr.s_addr;
> mnl_attr_put_u32(nlh, RTA_GATEWAY, gw_ip);
> }
> }
>
> nl = mnl_socket_open(NETLINK_ROUTE);
> if (nl == NULL) {
> tst_res(TFAIL, "mnl_socket_open failed (errno=%d): %s", errno,
> strerror(errno));
> goto err;
Then:
brk_on_route_error("mnl_socket_open", errno, iface, dst, gw, type);
> }
>
> if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0) {
> tst_res(TFAIL, "mnl_socket_bind failed (errno=%d): %s", errno,
> strerror(errno));
> goto err;
> }
>
> portid = mnl_socket_get_portid(nl);
>
> if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
> tst_res(TFAIL, "mnl_socket_sendto failed (errno=%d): %s", errno,
> strerror(errno));
> goto err;
> }
>
> ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
> if (ret < 0) {
> tst_res(TFAIL, "mnl_socket_recvfrom failed (ret=%d, errno=%d): %s",
> ret, errno, strerror(errno));
> goto err;
> }
>
> ret = mnl_cb_run(buf, ret, seq, portid, NULL, NULL);
> if (ret < 0) {
> tst_res(TFAIL, "mnl_cb_run failed (ret=%d, errno=%d): %s", ret,
> errno, strerror(errno));
> goto err;
> }
>
> mnl_socket_close(nl);
> return;
> err:
> print_route_info(iface, dst->ai_addr, gw ? gw->ai_addr: NULL, type);
> }
>
>
>>> +
>>> +static struct tst_option options[] = {
>>> + {"6", &ipv6_opt, "-6 Use IPv6 (default is IPv4)"},
>>> + {"c:", &c_opt, " Num loops (mandatory)"},
>
>> "-c x ...
> Fixed.
>
>
> BTW I also removed prefix parameter from rtnl_route() (it's a global parameter,
> family is not passed either).
>
> https://github.com/pevik/ltp/blob/route/c.v5.fixes/testcases/network/stress/route/route-change-netlink.c
> + below is full diff against posted version.
>
> Kind regards,
> Petr
>
> diff --git testcases/network/stress/route/route-change-netlink.c testcases/network/stress/route/route-change-netlink.c
> index 57ae02a3c..80677f1a4 100644
> --- testcases/network/stress/route/route-change-netlink.c
> +++ testcases/network/stress/route/route-change-netlink.c
> @@ -177,7 +177,22 @@ static void cleanup(void)
> mnl_socket_close(nl);
> }
>
> -static void rtnl_route(int iface, struct addrinfo *dst, struct addrinfo *gw, uint32_t prefix, int type)
> +static void print_route_info(int iface, struct sockaddr *dst,
> + struct sockaddr *gw, int type)
> +{
> + char dst_str[INET6_ADDRSTRLEN], gw_str[INET6_ADDRSTRLEN];
> + tst_sock_addr(dst, sizeof(dst), dst_str, sizeof(dst_str));
> + if (gw)
> + tst_sock_addr(gw, sizeof(gw), gw_str, sizeof(gw_str));
> +
> + tst_res(TINFO, "type: %s, iface: %d, dst: %s, gw: %s",
> + type == RTM_NEWROUTE ? "RTM_NEWROUTE" : "RTM_DELROUTE",
> + iface, dst_str, gw ? gw_str : "null");
> + tst_brk(TBROK, "failed due previous netlink errors");
> +}
> +
> +static void rtnl_route(int iface, struct addrinfo *dst, struct addrinfo *gw,
> + int type)
> {
> struct mnl_socket *nl;
> char buf[MNL_SOCKET_BUFFER_SIZE];
> @@ -229,31 +244,44 @@ static void rtnl_route(int iface, struct addrinfo *dst, struct addrinfo *gw, uin
> }
>
> nl = mnl_socket_open(NETLINK_ROUTE);
> - if (nl == NULL)
> - tst_brk(TBROK, "mnl_socket_open failed (errno=%d): %s", errno,
> + if (nl == NULL) {
> + tst_res(TFAIL, "mnl_socket_open failed (errno=%d): %s", errno,
> strerror(errno));
> + goto err;
> + }
>
> - if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0)
> - tst_brk(TBROK, "mnl_socket_bind failed (errno=%d): %s", errno,
> + if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0) {
> + tst_res(TFAIL, "mnl_socket_bind failed (errno=%d): %s", errno,
> strerror(errno));
> + goto err;
> + }
>
> portid = mnl_socket_get_portid(nl);
>
> - if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0)
> - tst_brk(TBROK, "mnl_socket_sendto failed (errno=%d): %s", errno,
> + if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
> + tst_res(TFAIL, "mnl_socket_sendto failed (errno=%d): %s", errno,
> strerror(errno));
> + goto err;
> + }
>
> ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
> - if (ret < 0)
> - tst_brk(TBROK, "mnl_socket_recvfrom failed (ret=%d, errno=%d): %s",
> + if (ret < 0) {
> + tst_res(TFAIL, "mnl_socket_recvfrom failed (ret=%d, errno=%d): %s",
> ret, errno, strerror(errno));
> + goto err;
> + }
>
> ret = mnl_cb_run(buf, ret, seq, portid, NULL, NULL);
> - if (ret < 0)
> - tst_brk(TBROK, "mnl_cb_run failed (ret=%d, errno=%d): %s", ret,
> + if (ret < 0) {
> + tst_res(TFAIL, "mnl_cb_run failed (ret=%d, errno=%d): %s", ret,
> errno, strerror(errno));
> + goto err;
> + }
>
> mnl_socket_close(nl);
> + return;
> +err:
> + print_route_info(iface, dst->ai_addr, gw ? gw->ai_addr: NULL, type);
> }
>
> static void send_udp(struct addrinfo *rhost_addrinfo)
> @@ -274,19 +302,11 @@ static void run(void)
> struct iface *p_iface = iface;
>
> for (i = 0; i < num_loops; i++) {
> - if (iface_len > 1)
> - tst_res(TINFO, "testing gw: %s, iface: %s",
> - p_gw->ip_str, p_iface->iface);
> - else if (gw_len > 1)
> - tst_res(TINFO, "testing gw: %s", p_gw->ip_str);
> - else
> - tst_res(TINFO, "testing dst: %s/%d", p_dst->ip_str, prefix);
> -
> rtnl_route(p_iface->index, p_dst->ip, gw ? p_gw->ip : NULL,
> - prefix, RTM_NEWROUTE);
> + RTM_NEWROUTE);
> send_udp(p_rhost->ip);
> rtnl_route(p_iface->index, p_dst->ip, gw ? p_gw->ip : NULL,
> - prefix, RTM_DELROUTE);
> + RTM_DELROUTE);
>
> if (gw)
> p_gw = p_gw->next ?: gw;
> @@ -300,7 +320,7 @@ static void run(void)
>
> static struct tst_option options[] = {
> {"6", &ipv6_opt, "-6 Use IPv6 (default is IPv4)"},
> - {"c:", &c_opt, " Num loops (mandatory)"},
> + {"c:", &c_opt, "-c x Num loops (mandatory)"},
> {"d:", &d_opt, "-d iface Interface to work on (mandatory)"},
> {"g:", &g_opt, "-g x Gateway IP"},
> {"p:", &p_opt, "-p port Rhost port (mandatory)"},
>
More information about the ltp
mailing list