Merge pull request #12125 from louis-6wind/fix-link-params

lib,zebra,ospf: link-params are not flushed after "no enable"

lib/if.c

@@ -1095,13 +1095,15 @@ const char *if_link_type_str(enum zebra_link_type llt)
 
 struct if_link_params *if_link_params_get(struct interface *ifp)
 {
+	return ifp->link_params;
+}
+
+struct if_link_params *if_link_params_enable(struct interface *ifp)
+{
+	struct if_link_params *iflp;
 	int i;
 
-	if (ifp->link_params != NULL)
+	iflp = if_link_params_init(ifp);
-		return ifp->link_params;
-
-	struct if_link_params *iflp =
-		XCALLOC(MTYPE_IF_LINK_PARAMS, sizeof(struct if_link_params));
 
 	/* Compute default bandwidth based on interface */
 	iflp->default_bw =
@@ -1129,6 +1131,20 @@ struct if_link_params *if_link_params_get(struct interface *ifp)
 	return iflp;
 }
 
+struct if_link_params *if_link_params_init(struct interface *ifp)
+{
+	struct if_link_params *iflp = if_link_params_get(ifp);
+
+	if (iflp)
+		return iflp;
+
+	iflp = XCALLOC(MTYPE_IF_LINK_PARAMS, sizeof(struct if_link_params));
+
+	ifp->link_params = iflp;
+
+	return iflp;
+}
+
 void if_link_params_free(struct interface *ifp)
 {
 	XFREE(MTYPE_IF_LINK_PARAMS, ifp->link_params);

lib/if.h

@@ -588,6 +588,8 @@ struct connected *connected_get_linklocal(struct interface *ifp);
 
 /* link parameters */
 struct if_link_params *if_link_params_get(struct interface *);
+struct if_link_params *if_link_params_enable(struct interface *ifp);
+struct if_link_params *if_link_params_init(struct interface *ifp);
 void if_link_params_free(struct interface *);
 
 /* Northbound. */

lib/zclient.c

@@ -2299,13 +2299,22 @@ static int zclient_handle_error(ZAPI_CALLBACK_ARGS)
 	return 0;
 }
 
-static int link_params_set_value(struct stream *s, struct if_link_params *iflp)
+static int link_params_set_value(struct stream *s, struct interface *ifp)
 {
+	uint8_t link_params_enabled;
+	struct if_link_params *iflp;
+	uint32_t bwclassnum;
+
+	iflp = if_link_params_get(ifp);
 
 	if (iflp == NULL)
-		return -1;
+		iflp = if_link_params_init(ifp);
 
-	uint32_t bwclassnum;
+	STREAM_GETC(s, link_params_enabled);
+	if (!link_params_enabled) {
+		if_link_params_free(ifp);
+		return 0;
+	}
 
 	STREAM_GETL(s, iflp->lp_status);
 	STREAM_GETL(s, iflp->te_metric);
@@ -2346,9 +2355,9 @@ struct interface *zebra_interface_link_params_read(struct stream *s,
 						   bool *changed)
 {
 	struct if_link_params *iflp;
-	struct if_link_params iflp_copy;
+	struct if_link_params iflp_prev;
 	ifindex_t ifindex;
-	bool params_changed = false;
+	bool iflp_prev_set;
 
 	STREAM_GETL(s, ifindex);
 
@@ -2361,22 +2370,32 @@ struct interface *zebra_interface_link_params_read(struct stream *s,
 		return NULL;
 	}
 
-	if (ifp->link_params == NULL)
+	iflp = if_link_params_get(ifp);
-		params_changed = true;
+	if (iflp) {
+		iflp_prev_set = true;
+		memcpy(&iflp_prev, ifp->link_params, sizeof(iflp_prev));
+	} else
+		iflp_prev_set = false;
 
-	if ((iflp = if_link_params_get(ifp)) == NULL)
+	/* read the link_params from stream
-		return NULL;
+	 * Free ifp->link_params if the stream has no params
-
+	 * to means that link-params are not enabled on links.
-	memcpy(&iflp_copy, iflp, sizeof(iflp_copy));
+	 */
-
+	if (link_params_set_value(s, ifp) != 0)
-	if (link_params_set_value(s, iflp) != 0)
 		goto stream_failure;
 
-	if (memcmp(&iflp_copy, iflp, sizeof(iflp_copy)))
+	if (changed == NULL)
-		params_changed = true;
+		return ifp;
 
-	if (changed)
+	if (iflp_prev_set && iflp) {
-		*changed = params_changed;
+		if (memcmp(&iflp_prev, iflp, sizeof(iflp_prev)))
+			*changed = true;
+		else
+			*changed = false;
+	} else if (!iflp_prev_set && !iflp)
+		*changed = false;
+	else
+		*changed = true;
 
 	return ifp;
 
@@ -2415,10 +2434,8 @@ static void zebra_interface_if_set_value(struct stream *s,
 	/* Read Traffic Engineering status */
 	link_params_status = stream_getc(s);
 	/* Then, Traffic Engineering parameters if any */
-	if (link_params_status) {
+	if (link_params_status)
-		struct if_link_params *iflp = if_link_params_get(ifp);
+		link_params_set_value(s, ifp);
-		link_params_set_value(s, iflp);
-	}
 
 	nexthop_group_interface_state_change(ifp, old_ifindex);
 
@@ -2435,12 +2452,20 @@ size_t zebra_interface_link_params_write(struct stream *s,
 	struct if_link_params *iflp;
 	int i;
 
-	if (s == NULL || ifp == NULL || ifp->link_params == NULL)
+	if (s == NULL || ifp == NULL)
 		return 0;
 
 	iflp = ifp->link_params;
 	w = 0;
 
+	/* encode if link_params is enabled */
+	if (iflp) {
+		w += stream_putc(s, true);
+	} else {
+		w += stream_putc(s, false);
+		return w;
+	}
+
 	w += stream_putl(s, iflp->lp_status);
 
 	w += stream_putl(s, iflp->te_metric);

zebra/interface.c

@@ -3279,14 +3279,8 @@ DEFUN (link_params_enable,
 			"Link-params: enable TE link parameters on interface %s",
 			ifp->name);
 
-	if (!if_link_params_get(ifp)) {
+	if (!if_link_params_get(ifp))
-		if (IS_ZEBRA_DEBUG_EVENT || IS_ZEBRA_DEBUG_MPLS)
+		if_link_params_enable(ifp);
-			zlog_debug(
-				"Link-params: failed to init TE link parameters  %s",
-				ifp->name);
-
-		return CMD_WARNING_CONFIG_FAILED;
-	}
 
 	/* force protocols to update LINK STATE due to parameters change */
 	if (if_is_operative(ifp))
@@ -3330,6 +3324,9 @@ DEFUN (link_params_metric,
 
 	metric = strtoul(argv[idx_number]->arg, NULL, 10);
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update TE metric if needed */
 	link_param_cmd_set_uint32(ifp, &iflp->te_metric, LP_TE_METRIC, metric);
 
@@ -3370,17 +3367,20 @@ DEFUN (link_params_maxbw,
 
 	/* Check that Maximum bandwidth is not lower than other bandwidth
 	 * parameters */
-	if ((bw <= iflp->max_rsv_bw) || (bw <= iflp->unrsv_bw[0])
+	if (iflp && ((bw <= iflp->max_rsv_bw) || (bw <= iflp->unrsv_bw[0]) ||
-	    || (bw <= iflp->unrsv_bw[1]) || (bw <= iflp->unrsv_bw[2])
+		     (bw <= iflp->unrsv_bw[1]) || (bw <= iflp->unrsv_bw[2]) ||
-	    || (bw <= iflp->unrsv_bw[3]) || (bw <= iflp->unrsv_bw[4])
+		     (bw <= iflp->unrsv_bw[3]) || (bw <= iflp->unrsv_bw[4]) ||
-	    || (bw <= iflp->unrsv_bw[5]) || (bw <= iflp->unrsv_bw[6])
+		     (bw <= iflp->unrsv_bw[5]) || (bw <= iflp->unrsv_bw[6]) ||
-	    || (bw <= iflp->unrsv_bw[7]) || (bw <= iflp->ava_bw)
+		     (bw <= iflp->unrsv_bw[7]) || (bw <= iflp->ava_bw) ||
-	    || (bw <= iflp->res_bw) || (bw <= iflp->use_bw)) {
+		     (bw <= iflp->res_bw) || (bw <= iflp->use_bw))) {
 		vty_out(vty,
 			"Maximum Bandwidth could not be lower than others bandwidth\n");
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Maximum Bandwidth if needed */
 	link_param_cmd_set_float(ifp, &iflp->max_bw, LP_MAX_BW, bw);
 
@@ -3406,13 +3406,16 @@ DEFUN (link_params_max_rsv_bw,
 
 	/* Check that bandwidth is not greater than maximum bandwidth parameter
 	 */
-	if (bw > iflp->max_bw) {
+	if (iflp && bw > iflp->max_bw) {
 		vty_out(vty,
 			"Maximum Reservable Bandwidth could not be greater than Maximum Bandwidth (%g)\n",
 			iflp->max_bw);
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Maximum Reservable Bandwidth if needed */
 	link_param_cmd_set_float(ifp, &iflp->max_rsv_bw, LP_MAX_RSV_BW, bw);
 
@@ -3448,13 +3451,16 @@ DEFUN (link_params_unrsv_bw,
 
 	/* Check that bandwidth is not greater than maximum bandwidth parameter
 	 */
-	if (bw > iflp->max_bw) {
+	if (iflp && bw > iflp->max_bw) {
 		vty_out(vty,
 			"UnReserved Bandwidth could not be greater than Maximum Bandwidth (%g)\n",
 			iflp->max_bw);
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Unreserved Bandwidth if needed */
 	link_param_cmd_set_float(ifp, &iflp->unrsv_bw[priority], LP_UNRSV_BW,
 				 bw);
@@ -3479,6 +3485,9 @@ DEFUN (link_params_admin_grp,
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Administrative Group if needed */
 	link_param_cmd_set_uint32(ifp, &iflp->admin_grp, LP_ADM_GRP, value);
 
@@ -3521,6 +3530,9 @@ DEFUN (link_params_inter_as,
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	as = strtoul(argv[idx_number]->arg, NULL, 10);
 
 	/* Update Remote IP and Remote AS fields if needed */
@@ -3548,6 +3560,9 @@ DEFUN (no_link_params_inter_as,
 	VTY_DECLVAR_CONTEXT(interface, ifp);
 	struct if_link_params *iflp = if_link_params_get(ifp);
 
+	if (!iflp)
+		return CMD_SUCCESS;
+
 	/* Reset Remote IP and AS neighbor */
 	iflp->rmt_as = 0;
 	iflp->rmt_ip.s_addr = 0;
@@ -3595,13 +3610,17 @@ DEFUN (link_params_delay,
 		 * Therefore, it is also allowed that the average
 		 * delay be equal to the min delay or max delay.
 		 */
-		if (IS_PARAM_SET(iflp, LP_MM_DELAY)
+		if (iflp && IS_PARAM_SET(iflp, LP_MM_DELAY) &&
-		    && (delay < iflp->min_delay || delay > iflp->max_delay)) {
+		    (delay < iflp->min_delay || delay > iflp->max_delay)) {
 			vty_out(vty,
 				"Average delay should be in range Min (%d) - Max (%d) delay\n",
 				iflp->min_delay, iflp->max_delay);
 			return CMD_WARNING_CONFIG_FAILED;
 		}
+
+		if (!iflp)
+			iflp = if_link_params_enable(ifp);
+
 		/* Update delay if value is not set or change */
 		if (IS_PARAM_UNSET(iflp, LP_DELAY) || iflp->av_delay != delay) {
 			iflp->av_delay = delay;
@@ -3626,6 +3645,10 @@ DEFUN (link_params_delay,
 				low, high);
 			return CMD_WARNING_CONFIG_FAILED;
 		}
+
+		if (!iflp)
+			iflp = if_link_params_enable(ifp);
+
 		/* Update Delays if needed */
 		if (IS_PARAM_UNSET(iflp, LP_DELAY)
 		    || IS_PARAM_UNSET(iflp, LP_MM_DELAY)
@@ -3656,6 +3679,9 @@ DEFUN (no_link_params_delay,
 	VTY_DECLVAR_CONTEXT(interface, ifp);
 	struct if_link_params *iflp = if_link_params_get(ifp);
 
+	if (!iflp)
+		return CMD_SUCCESS;
+
 	/* Unset Delays */
 	iflp->av_delay = 0;
 	UNSET_PARAM(iflp, LP_DELAY);
@@ -3683,6 +3709,9 @@ DEFUN (link_params_delay_var,
 
 	value = strtoul(argv[idx_number]->arg, NULL, 10);
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Delay Variation if needed */
 	link_param_cmd_set_uint32(ifp, &iflp->delay_var, LP_DELAY_VAR, value);
 
@@ -3723,6 +3752,9 @@ DEFUN (link_params_pkt_loss,
 	if (fval > MAX_PKT_LOSS)
 		fval = MAX_PKT_LOSS;
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Packet Loss if needed */
 	link_param_cmd_set_float(ifp, &iflp->pkt_loss, LP_PKT_LOSS, fval);
 
@@ -3762,13 +3794,16 @@ DEFUN (link_params_res_bw,
 
 	/* Check that bandwidth is not greater than maximum bandwidth parameter
 	 */
-	if (bw > iflp->max_bw) {
+	if (iflp && bw > iflp->max_bw) {
 		vty_out(vty,
 			"Residual Bandwidth could not be greater than Maximum Bandwidth (%g)\n",
 			iflp->max_bw);
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Residual Bandwidth if needed */
 	link_param_cmd_set_float(ifp, &iflp->res_bw, LP_RES_BW, bw);
 
@@ -3808,13 +3843,16 @@ DEFUN (link_params_ava_bw,
 
 	/* Check that bandwidth is not greater than maximum bandwidth parameter
 	 */
-	if (bw > iflp->max_bw) {
+	if (iflp && bw > iflp->max_bw) {
 		vty_out(vty,
 			"Available Bandwidth could not be greater than Maximum Bandwidth (%g)\n",
 			iflp->max_bw);
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Residual Bandwidth if needed */
 	link_param_cmd_set_float(ifp, &iflp->ava_bw, LP_AVA_BW, bw);
 
@@ -3854,13 +3892,16 @@ DEFUN (link_params_use_bw,
 
 	/* Check that bandwidth is not greater than maximum bandwidth parameter
 	 */
-	if (bw > iflp->max_bw) {
+	if (iflp && bw > iflp->max_bw) {
 		vty_out(vty,
 			"Utilised Bandwidth could not be greater than Maximum Bandwidth (%g)\n",
 			iflp->max_bw);
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	if (!iflp)
+		iflp = if_link_params_enable(ifp);
+
 	/* Update Utilized Bandwidth if needed */
 	link_param_cmd_set_float(ifp, &iflp->use_bw, LP_USE_BW, bw);
 

zebra/zapi_msg.c

@@ -232,11 +232,6 @@ int zsend_interface_link_params(struct zserv *client, struct interface *ifp)
 {
 	struct stream *s = stream_new(ZEBRA_MAX_PACKET_SIZ);
 
-	if (!ifp->link_params) {
-		stream_free(s);
-		return 0;
-	}
-
 	zclient_create_header(s, ZEBRA_INTERFACE_LINK_PARAMS, ifp->vrf->vrf_id);
 
 	/* Add Interface Index */