diff mbox series

[RFC,iproute2-next,05/11] ip: nexthop: always parse attributes for printing

Message ID 20210929152848.1710552-6-razor@blackwall.org (mailing list archive)
State Superseded
Delegated to: David Ahern
Headers show
Series ip: nexthop: cache nexthops and print routes' nh info | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Nikolay Aleksandrov Sept. 29, 2021, 3:28 p.m. UTC 
From: Nikolay Aleksandrov <nikolay@nvidia.com>

Always parse nexthop group attributes into structure for printing. Nexthop
print helpers take structures and print them accordingly.

Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
---
 ip/ipnexthop.c | 117 +++++++++++++++++++++----------------------------
 1 file changed, 49 insertions(+), 68 deletions(-)

Comments

David Ahern Sept. 30, 2021, 3:37 a.m. UTC | #1 
On 9/29/21 9:28 AM, Nikolay Aleksandrov wrote:
> @@ -481,56 +457,61 @@ int print_nexthop(struct nlmsghdr *n, void *arg)
>  	if (filter.proto && filter.proto != nhm->nh_protocol)
>  		return 0;
>  
> -	parse_rtattr_flags(tb, NHA_MAX, RTM_NHA(nhm), len, NLA_F_NESTED);
> -
> +	err = parse_nexthop_rta(fp, nhm, len, &nhe);
> +	if (err) {
> +		close_json_object();
> +		fprintf(stderr, "Error parsing nexthop: %s\n", strerror(err));
> +		return -1;
> +	}

so you are doing that but in reverse order. Re-order the patches such
that existing code is refactored with the parse functions first.
Nikolay Aleksandrov Sept. 30, 2021, 7:10 a.m. UTC | #2 
On 30/09/2021 06:37, David Ahern wrote:
> On 9/29/21 9:28 AM, Nikolay Aleksandrov wrote:
>> @@ -481,56 +457,61 @@ int print_nexthop(struct nlmsghdr *n, void *arg)
>>  	if (filter.proto && filter.proto != nhm->nh_protocol)
>>  		return 0;
>>  
>> -	parse_rtattr_flags(tb, NHA_MAX, RTM_NHA(nhm), len, NLA_F_NESTED);
>> -
>> +	err = parse_nexthop_rta(fp, nhm, len, &nhe);
>> +	if (err) {
>> +		close_json_object();
>> +		fprintf(stderr, "Error parsing nexthop: %s\n", strerror(err));
>> +		return -1;
>> +	}
> 
> so you are doing that but in reverse order. Re-order the patches such
> that existing code is refactored with the parse functions first.
> 

I added the parse functions first without any functional changes and converted
the existing printing code to use them in this patch after that. I don't mind
doing the conversion after each parse function is added, too.
diff mbox series

Patch

diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
index 9340d8941277..e334b852aa55 100644
--- a/ip/ipnexthop.c
+++ b/ip/ipnexthop.c
@@ -220,28 +220,22 @@  static bool __valid_nh_group_attr(const struct rtattr *g_attr)
 	return num && num * sizeof(struct nexthop_grp) == RTA_PAYLOAD(g_attr);
 }
 
-static void print_nh_group(FILE *fp, const struct rtattr *grps_attr)
+static void print_nh_group(const struct nh_entry *nhe)
 {
-	struct nexthop_grp *nhg = RTA_DATA(grps_attr);
-	int num = RTA_PAYLOAD(grps_attr) / sizeof(*nhg);
 	int i;
 
-	if (!__valid_nh_group_attr(grps_attr)) {
-		fprintf(fp, "<invalid nexthop group>");
-		return;
-	}
-
 	open_json_array(PRINT_JSON, "group");
 	print_string(PRINT_FP, NULL, "%s", "group ");
-	for (i = 0; i < num; ++i) {
+	for (i = 0; i < nhe->nh_groups_cnt; ++i) {
 		open_json_object(NULL);
 
 		if (i)
 			print_string(PRINT_FP, NULL, "%s", "/");
 
-		print_uint(PRINT_ANY, "id", "%u", nhg[i].id);
-		if (nhg[i].weight)
-			print_uint(PRINT_ANY, "weight", ",%u", nhg[i].weight + 1);
+		print_uint(PRINT_ANY, "id", "%u", nhe->nh_groups[i].id);
+		if (nhe->nh_groups[i].weight)
+			print_uint(PRINT_ANY, "weight", ",%u",
+				   nhe->nh_groups[i].weight + 1);
 
 		close_json_object();
 	}
@@ -261,15 +255,14 @@  static const char *nh_group_type_name(__u16 type)
 	}
 }
 
-static void print_nh_group_type(FILE *fp, const struct rtattr *grp_type_attr)
+static void print_nh_group_type(__u16 nh_grp_type)
 {
-	__u16 type = rta_getattr_u16(grp_type_attr);
-
-	if (type == NEXTHOP_GRP_TYPE_MPATH)
+	if (nh_grp_type == NEXTHOP_GRP_TYPE_MPATH)
 		/* Do not print type in order not to break existing output. */
 		return;
 
-	print_string(PRINT_ANY, "type", "type %s ", nh_group_type_name(type));
+	print_string(PRINT_ANY, "type", "type %s ",
+		     nh_group_type_name(nh_grp_type));
 }
 
 static void parse_nh_res_group_rta(const struct rtattr *res_grp_attr,
@@ -299,39 +292,22 @@  static void parse_nh_res_group_rta(const struct rtattr *res_grp_attr,
 	}
 }
 
-static void print_nh_res_group(FILE *fp, const struct rtattr *res_grp_attr)
+static void print_nh_res_group(const struct nha_res_grp *res_grp)
 {
-	struct rtattr *tb[NHA_RES_GROUP_MAX + 1];
-	struct rtattr *rta;
 	struct timeval tv;
 
-	parse_rtattr_nested(tb, NHA_RES_GROUP_MAX, res_grp_attr);
-
 	open_json_object("resilient_args");
 
-	if (tb[NHA_RES_GROUP_BUCKETS])
-		print_uint(PRINT_ANY, "buckets", "buckets %u ",
-			   rta_getattr_u16(tb[NHA_RES_GROUP_BUCKETS]));
+	print_uint(PRINT_ANY, "buckets", "buckets %u ", res_grp->buckets);
 
-	if (tb[NHA_RES_GROUP_IDLE_TIMER]) {
-		rta = tb[NHA_RES_GROUP_IDLE_TIMER];
-		__jiffies_to_tv(&tv, rta_getattr_u32(rta));
-		print_tv(PRINT_ANY, "idle_timer", "idle_timer %g ", &tv);
-	}
+	__jiffies_to_tv(&tv, res_grp->idle_timer);
+	print_tv(PRINT_ANY, "idle_timer", "idle_timer %g ", &tv);
 
-	if (tb[NHA_RES_GROUP_UNBALANCED_TIMER]) {
-		rta = tb[NHA_RES_GROUP_UNBALANCED_TIMER];
-		__jiffies_to_tv(&tv, rta_getattr_u32(rta));
-		print_tv(PRINT_ANY, "unbalanced_timer", "unbalanced_timer %g ",
-			 &tv);
-	}
+	__jiffies_to_tv(&tv, res_grp->unbalanced_timer);
+	print_tv(PRINT_ANY, "unbalanced_timer", "unbalanced_timer %g ", &tv);
 
-	if (tb[NHA_RES_GROUP_UNBALANCED_TIME]) {
-		rta = tb[NHA_RES_GROUP_UNBALANCED_TIME];
-		__jiffies_to_tv(&tv, rta_getattr_u32(rta));
-		print_tv(PRINT_ANY, "unbalanced_time", "unbalanced_time %g ",
-			 &tv);
-	}
+	__jiffies_to_tv(&tv, res_grp->unbalanced_time);
+	print_tv(PRINT_ANY, "unbalanced_time", "unbalanced_time %g ", &tv);
 
 	close_json_object();
 }
@@ -458,9 +434,9 @@  out_err:
 int print_nexthop(struct nlmsghdr *n, void *arg)
 {
 	struct nhmsg *nhm = NLMSG_DATA(n);
-	struct rtattr *tb[NHA_MAX+1];
 	FILE *fp = (FILE *)arg;
-	int len;
+	struct nh_entry nhe;
+	int len, err;
 
 	SPRINT_BUF(b1);
 
@@ -481,56 +457,61 @@  int print_nexthop(struct nlmsghdr *n, void *arg)
 	if (filter.proto && filter.proto != nhm->nh_protocol)
 		return 0;
 
-	parse_rtattr_flags(tb, NHA_MAX, RTM_NHA(nhm), len, NLA_F_NESTED);
-
+	err = parse_nexthop_rta(fp, nhm, len, &nhe);
+	if (err) {
+		close_json_object();
+		fprintf(stderr, "Error parsing nexthop: %s\n", strerror(err));
+		return -1;
+	}
 	open_json_object(NULL);
 
 	if (n->nlmsg_type == RTM_DELNEXTHOP)
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
-	if (tb[NHA_ID])
-		print_uint(PRINT_ANY, "id", "id %u ",
-			   rta_getattr_u32(tb[NHA_ID]));
+	print_uint(PRINT_ANY, "id", "id %u ", nhe.nh_id);
 
-	if (tb[NHA_GROUP])
-		print_nh_group(fp, tb[NHA_GROUP]);
+	if (nhe.nh_groups)
+		print_nh_group(&nhe);
 
-	if (tb[NHA_GROUP_TYPE])
-		print_nh_group_type(fp, tb[NHA_GROUP_TYPE]);
+	print_nh_group_type(nhe.nh_grp_type);
 
-	if (tb[NHA_RES_GROUP])
-		print_nh_res_group(fp, tb[NHA_RES_GROUP]);
+	if (nhe.nh_has_res_grp)
+		print_nh_res_group(&nhe.nh_res_grp);
 
-	if (tb[NHA_ENCAP])
-		lwt_print_encap(fp, tb[NHA_ENCAP_TYPE], tb[NHA_ENCAP]);
+	if (nhe.nh_encap)
+		lwt_print_encap(fp, &nhe.nh_encap_type.rta, nhe.nh_encap);
 
-	if (tb[NHA_GATEWAY])
-		print_rta_gateway(fp, nhm->nh_family, tb[NHA_GATEWAY]);
+	if (nhe.nh_gateway_len)
+		__print_rta_gateway(fp, nhe.nh_family,
+				    format_host(nhe.nh_family,
+						nhe.nh_gateway_len,
+						&nhe.nh_gateway));
 
-	if (tb[NHA_OIF])
-		print_rta_ifidx(fp, rta_getattr_u32(tb[NHA_OIF]), "dev");
+	if (nhe.nh_oif)
+		print_rta_ifidx(fp, nhe.nh_oif, "dev");
 
-	if (nhm->nh_scope != RT_SCOPE_UNIVERSE || show_details > 0) {
+	if (nhe.nh_scope != RT_SCOPE_UNIVERSE || show_details > 0) {
 		print_string(PRINT_ANY, "scope", "scope %s ",
-			     rtnl_rtscope_n2a(nhm->nh_scope, b1, sizeof(b1)));
+			     rtnl_rtscope_n2a(nhe.nh_scope, b1, sizeof(b1)));
 	}
 
-	if (tb[NHA_BLACKHOLE])
+	if (nhe.nh_blackhole)
 		print_null(PRINT_ANY, "blackhole", "blackhole ", NULL);
 
-	if (nhm->nh_protocol != RTPROT_UNSPEC || show_details > 0) {
+	if (nhe.nh_protocol != RTPROT_UNSPEC || show_details > 0) {
 		print_string(PRINT_ANY, "protocol", "proto %s ",
-			     rtnl_rtprot_n2a(nhm->nh_protocol, b1, sizeof(b1)));
+			     rtnl_rtprot_n2a(nhe.nh_protocol, b1, sizeof(b1)));
 	}
 
-	print_rt_flags(fp, nhm->nh_flags);
+	print_rt_flags(fp, nhe.nh_flags);
 
-	if (tb[NHA_FDB])
+	if (nhe.nh_fdb)
 		print_null(PRINT_ANY, "fdb", "fdb", NULL);
 
 	print_string(PRINT_FP, NULL, "%s", "\n");
 	close_json_object();
 	fflush(fp);
+	destroy_nexthop_entry(&nhe);
 
 	return 0;
 }