zebra: fix vni NB conversion

- unnecessary command duplication
- usage of oper data during validation
- unnecessary checks for things that can't happen

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

zebra/zebra_nb_config.c

@@ -3533,70 +3533,33 @@ int lib_vrf_zebra_filter_nht_route_map_modify(struct nb_cb_modify_args *args)
 int lib_vrf_zebra_l3vni_id_modify(struct nb_cb_modify_args *args)
 {
 	struct vrf *vrf;
-	struct zebra_vrf *zvrf;
 	vni_t vni = 0;
-	struct zebra_l3vni *zl3vni = NULL;
-	char err[ERR_STR_SZ];
 	bool pfx_only = false;
-	const struct lyd_node *pn_dnode;
+	uint32_t count;
-	const char *vrfname;
+
+	vni = yang_dnode_get_uint32(args->dnode, NULL);
 
 	switch (args->event) {
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
 		return NB_OK;
 	case NB_EV_VALIDATE:
-		vni = yang_dnode_get_uint32(args->dnode, NULL);
+		count = yang_dnode_count(args->dnode,
-		/* Get vrf info from parent node, reject configuration
+					 "/frr-vrf:lib/vrf/frr-zebra:zebra[l3vni-id='%u']",
-		 * if zebra vrf already mapped to different vni id.
+					 vni);
-		 */
+		if (count > 1) {
-		pn_dnode = yang_dnode_get_parent(args->dnode, "vrf");
+			snprintfrr(args->errmsg, args->errmsg_len,
-		vrfname = yang_dnode_get_string(pn_dnode, "name");
+				   "vni %u is already mapped to another vrf",
-		zvrf = zebra_vrf_lookup_by_name(vrfname);
+				   vni);
-		if (!zvrf) {
-			snprintf(args->errmsg, args->errmsg_len,
-				 "zebra vrf info not found for vrf:%s.",
-				 vrfname);
 			return NB_ERR_VALIDATION;
 		}
-		if (zvrf->l3vni && zvrf->l3vni != vni) {
-			snprintf(
-				args->errmsg, args->errmsg_len,
-				"vni %u cannot be configured as vni %u is already configured under the vrf",
-				vni, zvrf->l3vni);
-			return NB_ERR_VALIDATION;
-		}
-
-		/* Check if this VNI is already present in the system */
-		zl3vni = zl3vni_lookup(vni);
-		if (zl3vni) {
-			snprintf(args->errmsg, args->errmsg_len,
-				 "VNI %u is already configured as L3-VNI", vni);
-			return NB_ERR_VALIDATION;
-		}
-
 		break;
 	case NB_EV_APPLY:
-
 		vrf = nb_running_get_entry(args->dnode, NULL, true);
-		zvrf = zebra_vrf_lookup_by_name(vrf->name);
-		vni = yang_dnode_get_uint32(args->dnode, NULL);
-		/* Note: This covers lib_vrf_zebra_prefix_only_modify() config
-		 * along with l3vni config
-		 */
 		pfx_only = yang_dnode_get_bool(args->dnode, "../prefix-only");
 
-		if (zebra_vxlan_process_vrf_vni_cmd(zvrf, vni, err, ERR_STR_SZ,
+		zebra_vxlan_process_vrf_vni_cmd(vrf->info, vni,
-						    pfx_only ? 1 : 0, 1)
+						pfx_only ? 1 : 0, 1);
-		    != 0) {
-			if (IS_ZEBRA_DEBUG_VXLAN)
-				snprintf(
-					args->errmsg, args->errmsg_len,
-					"vrf vni %u mapping failed with error: %s",
-					vni, err);
-			return NB_ERR;
-		}
-
 		break;
 	}
 
@@ -3606,10 +3569,7 @@ int lib_vrf_zebra_l3vni_id_modify(struct nb_cb_modify_args *args)
 int lib_vrf_zebra_l3vni_id_destroy(struct nb_cb_destroy_args *args)
 {
 	struct vrf *vrf;
-	struct zebra_vrf *zvrf;
 	vni_t vni = 0;
-	char err[ERR_STR_SZ];
-	uint8_t filter = 0;
 
 	switch (args->event) {
 	case NB_EV_PREPARE:
@@ -3618,32 +3578,9 @@ int lib_vrf_zebra_l3vni_id_destroy(struct nb_cb_destroy_args *args)
 		return NB_OK;
 	case NB_EV_APPLY:
 		vrf = nb_running_get_entry(args->dnode, NULL, true);
-		zvrf = zebra_vrf_lookup_by_name(vrf->name);
 		vni = yang_dnode_get_uint32(args->dnode, NULL);
 
-		if (!zl3vni_lookup(vni))
+		zebra_vxlan_process_vrf_vni_cmd(vrf->info, vni, 0, 0);
-			return NB_OK;
-
-		if (zvrf->l3vni != vni) {
-			snprintf(args->errmsg, args->errmsg_len,
-				 "vrf %s has different vni %u mapped",
-				 vrf->name, zvrf->l3vni);
-			return NB_ERR;
-		}
-
-		if (is_l3vni_for_prefix_routes_only(zvrf->l3vni))
-			filter = 1;
-
-		if (zebra_vxlan_process_vrf_vni_cmd(zvrf, vni, err, ERR_STR_SZ,
-						    filter, 0)
-		    != 0) {
-			if (IS_ZEBRA_DEBUG_VXLAN)
-				zlog_debug(
-					"vrf vni %u unmapping failed with error: %s",
-					vni, err);
-			return NB_ERR;
-		}
-
 		break;
 	}
 

zebra/zebra_vty.c

@@ -2728,142 +2728,48 @@ DEFPY(evpn_mh_redirect_off, evpn_mh_redirect_off_cmd,
 	return zebra_evpn_mh_redirect_off(vty, redirect_off);
 }
 
-DEFUN (default_vrf_vni_mapping,
+DEFPY_YANG (vni_mapping,
-       default_vrf_vni_mapping_cmd,
+       vni_mapping_cmd,
-       "vni " CMD_VNI_RANGE "[prefix-routes-only]",
+       "vni " CMD_VNI_RANGE "[prefix-routes-only$filter]",
-       "VNI corresponding to the DEFAULT VRF\n"
-       "VNI-ID\n"
-       "Prefix routes only \n")
-{
-	char xpath[XPATH_MAXLEN];
-	int filter = 0;
-
-	if (argc == 3)
-		filter = 1;
-
-	snprintf(xpath, sizeof(xpath), FRR_VRF_KEY_XPATH "/frr-zebra:zebra",
-		 VRF_DEFAULT_NAME);
-	nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
-
-	snprintf(xpath, sizeof(xpath),
-		 FRR_VRF_KEY_XPATH "/frr-zebra:zebra/l3vni-id",
-		 VRF_DEFAULT_NAME);
-	nb_cli_enqueue_change(vty, xpath, NB_OP_MODIFY, argv[1]->arg);
-
-	if (filter) {
-		snprintf(xpath, sizeof(xpath),
-			 FRR_VRF_KEY_XPATH "/frr-zebra:zebra/prefix-only",
-			 VRF_DEFAULT_NAME);
-		nb_cli_enqueue_change(vty, xpath, NB_OP_MODIFY, "true");
-	}
-
-	return nb_cli_apply_changes(vty, NULL);
-}
-
-DEFUN (no_default_vrf_vni_mapping,
-       no_default_vrf_vni_mapping_cmd,
-       "no vni " CMD_VNI_RANGE "[prefix-routes-only]",
-       NO_STR
-       "VNI corresponding to DEFAULT VRF\n"
-       "VNI-ID\n"
-       "Prefix routes only \n")
-{
-	char xpath[XPATH_MAXLEN];
-	int filter = 0;
-	vni_t vni = strtoul(argv[2]->arg, NULL, 10);
-	struct zebra_vrf *zvrf = NULL;
-
-	zvrf = zebra_vrf_lookup_by_id(VRF_DEFAULT);
-
-	if (argc == 4)
-		filter = 1;
-
-	if (zvrf->l3vni != vni) {
-		vty_out(vty, "VNI %d doesn't exist in VRF: %s \n", vni,
-			zvrf->vrf->name);
-		return CMD_WARNING;
-	}
-
-	snprintf(xpath, sizeof(xpath),
-		 FRR_VRF_KEY_XPATH "/frr-zebra:zebra/l3vni-id",
-		 VRF_DEFAULT_NAME);
-	nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, argv[2]->arg);
-
-	if (filter) {
-		snprintf(xpath, sizeof(xpath),
-			 FRR_VRF_KEY_XPATH "/frr-zebra:zebra/prefix-only",
-			 VRF_DEFAULT_NAME);
-		nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, "true");
-	}
-
-	snprintf(xpath, sizeof(xpath), FRR_VRF_KEY_XPATH "/frr-zebra:zebra",
-		 VRF_DEFAULT_NAME);
-	nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL);
-
-	return nb_cli_apply_changes(vty, NULL);
-}
-
-DEFUN (vrf_vni_mapping,
-       vrf_vni_mapping_cmd,
-       "vni " CMD_VNI_RANGE "[prefix-routes-only]",
        "VNI corresponding to tenant VRF\n"
        "VNI-ID\n"
        "prefix-routes-only\n")
 {
-	int filter = 0;
-
-	ZEBRA_DECLVAR_CONTEXT_VRF(vrf, zvrf);
-
-	assert(vrf);
-	assert(zvrf);
-
-	if (argc == 3)
-		filter = 1;
-
-	nb_cli_enqueue_change(vty, "./frr-zebra:zebra", NB_OP_CREATE, NULL);
 	nb_cli_enqueue_change(vty, "./frr-zebra:zebra/l3vni-id", NB_OP_MODIFY,
-			      argv[1]->arg);
+			      vni_str);
 
 	if (filter)
 		nb_cli_enqueue_change(vty, "./frr-zebra:zebra/prefix-only",
 				      NB_OP_MODIFY, "true");
+	else
+		nb_cli_enqueue_change(vty, "./frr-zebra:zebra/prefix-only",
+				      NB_OP_DESTROY, NULL);
+
+	if (vty->node == CONFIG_NODE)
+		return nb_cli_apply_changes(vty, "/frr-vrf:lib/vrf[name='%s']",
+					    VRF_DEFAULT_NAME);
 
 	return nb_cli_apply_changes(vty, NULL);
 }
 
-DEFUN (no_vrf_vni_mapping,
+DEFPY_YANG (no_vni_mapping,
-       no_vrf_vni_mapping_cmd,
+       no_vni_mapping_cmd,
-       "no vni " CMD_VNI_RANGE "[prefix-routes-only]",
+       "no vni [" CMD_VNI_RANGE "[prefix-routes-only$filter]]",
        NO_STR
        "VNI corresponding to tenant VRF\n"
        "VNI-ID\n"
        "prefix-routes-only\n")
 {
-	int filter = 0;
-
-	ZEBRA_DECLVAR_CONTEXT_VRF(vrf, zvrf);
-	vni_t vni = strtoul(argv[2]->arg, NULL, 10);
-
-	assert(vrf);
-	assert(zvrf);
-
-	if (argc == 4)
-		filter = 1;
-
-	if (zvrf->l3vni != vni) {
-		vty_out(vty, "VNI %d doesn't exist in VRF: %s \n", vni,
-			zvrf->vrf->name);
-		return CMD_WARNING;
-	}
-
 	nb_cli_enqueue_change(vty, "./frr-zebra:zebra/l3vni-id", NB_OP_DESTROY,
-			      argv[2]->arg);
+			      NULL);
 
 	if (filter)
 		nb_cli_enqueue_change(vty, "./frr-zebra:zebra/prefix-only",
-				      NB_OP_DESTROY, "true");
+				      NB_OP_DESTROY, NULL);
 
-	nb_cli_enqueue_change(vty, "./frr-zebra:zebra", NB_OP_DESTROY, NULL);
+	if (vty->node == CONFIG_NODE)
+		return nb_cli_apply_changes(vty, "/frr-vrf:lib/vrf[name='%s']",
+					    VRF_DEFAULT_NAME);
 
 	return nb_cli_apply_changes(vty, NULL);
 }
@@ -4696,10 +4602,10 @@ void zebra_vty_init(void)
 	install_element(CONFIG_NODE, &evpn_mh_neigh_holdtime_cmd);
 	install_element(CONFIG_NODE, &evpn_mh_startup_delay_cmd);
 	install_element(CONFIG_NODE, &evpn_mh_redirect_off_cmd);
-	install_element(CONFIG_NODE, &default_vrf_vni_mapping_cmd);
+	install_element(CONFIG_NODE, &vni_mapping_cmd);
-	install_element(CONFIG_NODE, &no_default_vrf_vni_mapping_cmd);
+	install_element(CONFIG_NODE, &no_vni_mapping_cmd);
-	install_element(VRF_NODE, &vrf_vni_mapping_cmd);
+	install_element(VRF_NODE, &vni_mapping_cmd);
-	install_element(VRF_NODE, &no_vrf_vni_mapping_cmd);
+	install_element(VRF_NODE, &no_vni_mapping_cmd);
 
 	install_element(VIEW_NODE, &show_dataplane_cmd);
 	install_element(VIEW_NODE, &show_dataplane_providers_cmd);

zebra/zebra_vxlan.c

@@ -5174,9 +5174,8 @@ void zebra_vxlan_macvlan_up(struct interface *ifp)
 	}
 }
 
-int zebra_vxlan_process_vrf_vni_cmd(struct zebra_vrf *zvrf, vni_t vni,
+void zebra_vxlan_process_vrf_vni_cmd(struct zebra_vrf *zvrf, vni_t vni,
-				    char *err, int err_str_sz, int filter,
+				     int filter, int add)
-				    int add)
 {
 	struct zebra_l3vni *zl3vni = NULL;
 	struct zebra_vrf *zvrf_evpn = NULL;
@@ -5188,21 +5187,6 @@ int zebra_vxlan_process_vrf_vni_cmd(struct zebra_vrf *zvrf, vni_t vni,
 			   add ? "ADD" : "DEL");
 
 	if (add) {
-		/* check if the vni is already present under zvrf */
-		if (zvrf->l3vni) {
-			snprintf(err, err_str_sz,
-				 "VNI is already configured under the vrf");
-			return -1;
-		}
-
-		/* check if this VNI is already present in the system */
-		zl3vni = zl3vni_lookup(vni);
-		if (zl3vni) {
-			snprintf(err, err_str_sz,
-				 "VNI is already configured as L3-VNI");
-			return -1;
-		}
-
 		/* Remove L2VNI if present */
 		zebra_vxlan_handle_vni_transition(zvrf, vni, add);
 
@@ -5248,23 +5232,7 @@ int zebra_vxlan_process_vrf_vni_cmd(struct zebra_vrf *zvrf, vni_t vni,
 
 	} else {
 		zl3vni = zl3vni_lookup(vni);
-		if (!zl3vni) {
+		assert(zl3vni);
-			snprintf(err, err_str_sz, "VNI doesn't exist");
-			return -1;
-		}
-
-		if (zvrf->l3vni != vni) {
-			snprintf(err, err_str_sz,
-					"VNI %d doesn't exist in VRF: %s",
-					vni, zvrf->vrf->name);
-			return -1;
-		}
-
-		if (filter && !CHECK_FLAG(zl3vni->filter, PREFIX_ROUTES_ONLY)) {
-			snprintf(err, ERR_STR_SZ,
-				 "prefix-routes-only is not set for the vni");
-			return -1;
-		}
 
 		zebra_vxlan_process_l3vni_oper_down(zl3vni);
 
@@ -5282,7 +5250,6 @@ int zebra_vxlan_process_vrf_vni_cmd(struct zebra_vrf *zvrf, vni_t vni,
 		/* Add L2VNI for this VNI */
 		zebra_vxlan_handle_vni_transition(zvrf, vni, add);
 	}
-	return 0;
 }
 
 int zebra_vxlan_vrf_enable(struct zebra_vrf *zvrf)

zebra/zebra_vxlan.h

@@ -178,8 +178,7 @@ extern int zebra_vxlan_if_add(struct interface *ifp);
 extern int zebra_vxlan_if_update(struct interface *ifp,
 				 struct zebra_vxlan_if_update_ctx *ctx);
 extern int zebra_vxlan_if_del(struct interface *ifp);
-extern int zebra_vxlan_process_vrf_vni_cmd(struct zebra_vrf *zvrf, vni_t vni,
+extern void zebra_vxlan_process_vrf_vni_cmd(struct zebra_vrf *zvrf, vni_t vni,
-					   char *err, int err_str_sz,
 					    int filter, int add);
 extern void zebra_vxlan_init_tables(struct zebra_vrf *zvrf);
 extern void zebra_vxlan_close_tables(struct zebra_vrf *);