Re: [PATCH iproute2-next v2] ip: add AMT support

On 10/25/21 11:28 PM, Taehee Yoo wrote:

+static void amt_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+{
+	if (!tb)
+		return;
+
+	if (tb[IFLA_AMT_MODE]) {
+		print_string(PRINT_ANY,
+			     "mode",
+			     "%s ",
+			     modename[rta_getattr_u32(tb[IFLA_AMT_MODE])]);
+	}
+
+	if (tb[IFLA_AMT_GATEWAY_PORT]) {
+		print_uint(PRINT_ANY,
+			   "gateway_port",
+			   "gateway_port %u ",
+			   rta_getattr_be16(tb[IFLA_AMT_GATEWAY_PORT]));
+	}
+
+	if (tb[IFLA_AMT_RELAY_PORT]) {
+		print_uint(PRINT_ANY,
+			   "relay_port",
+			   "relay_port %u ",
+			   rta_getattr_be16(tb[IFLA_AMT_RELAY_PORT]));
+	}
+
+	if (tb[IFLA_AMT_LOCAL_IP]) {
+		__be32 addr = rta_getattr_u32(tb[IFLA_AMT_LOCAL_IP]);
+
+		if (addr)
+			print_string(PRINT_ANY,
+				     "local",
+				     "local %s ",
+				     format_host(AF_INET, 4, &addr));

if you only print the address when it is non-zero, why send it at all?
ie., kernel side can default to 0 (any address) but that value should
not be allowed or sent as part of the API. Same with the remote and
discovery addresses.

+	}
+
+	if (tb[IFLA_AMT_REMOTE_IP]) {
+		__be32 addr = rta_getattr_u32(tb[IFLA_AMT_REMOTE_IP]);
+
+		if (addr)
+			print_string(PRINT_ANY,
+				     "remote",
+				     "remote %s ",
+				     format_host(AF_INET, 4, &addr));
+	}
+
+	if (tb[IFLA_AMT_DISCOVERY_IP]) {
+		__be32 addr = rta_getattr_u32(tb[IFLA_AMT_DISCOVERY_IP]);
+
+		if (addr) {
+			print_string(PRINT_ANY,
+				     "discovery",
+				     "discovery %s ",
+				     format_host(AF_INET, 4, &addr));
+		}
+	}
+
+	if (tb[IFLA_AMT_LINK]) {
+		unsigned int link = rta_getattr_u32(tb[IFLA_AMT_LINK]);
+
+		if (link)
+			print_string(PRINT_ANY, "link", "dev %s ",
+				     ll_index_to_name(link));

similar argument here: IFLA_AMT_LINK should only be sent if an actual
link association is configured.

+	}
+
+	if (tb[IFLA_AMT_MAX_TUNNELS]) {
+		unsigned int tunnels = rta_getattr_u32(tb[IFLA_AMT_MAX_TUNNELS]);
+
+		if (tunnels)
+			print_uint(PRINT_ANY, "max_tunnels", "max_tunnels %u ",
+				   rta_getattr_u32(tb[IFLA_AMT_MAX_TUNNELS]));

and here.

+	}
+}
+