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

Daemons like isisd continue to use the previous link-params after they
are removed from zebra.

For example,
>r0# sh run zebra
> (...)
> interface eth-rt1
>  link-params
>   enable
>   metric 100
>  exit-link-params
> r0# conf
> r0(config)# interface eth-rt1
> r0(config-if)#  link-params
> r0(config-link-params)#   no enable

After "no enable", "sh run zebra" displays no more link-params context.

The "no enable" causes the release of the "link_params" pointer within
the "interface" structure. The zebra function to update daemons with
a ZEBRA_INTERFACE_LINK_PARAMS zapi message is called but the function
returns without doing anything because the "link_params" pointer is
NULL. Therefore, the "link_params" pointers are kept in daemons.

When the zebra "link_params" pointer is NULL:

- Send a zapi link param message that contains no link parameters
  instead of sending no message.
- At reception in daemons, the absence of link parameters causes the
  release of the "link_params" pointer.

Fixes: 16f1b9e ("Update Traffic Engineering Support for OSPFD")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>

1 files changed, 47 insertions, 22 deletions

diff --git a/lib/zclient.c b/lib/zclient.c
index 8ec82ab7bb..f5d45b40ef 100644
--- a/lib/zclient.c
+++ b/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)
-		params_changed = true;
-
-	if ((iflp = if_link_params_get(ifp)) == NULL)
-		return NULL;
-
-	memcpy(&iflp_copy, iflp, sizeof(iflp_copy));
+	iflp = if_link_params_get(ifp);
+	if (iflp) {
+		iflp_prev_set = true;
+		memcpy(&iflp_prev, ifp->link_params, sizeof(iflp_prev));
+	} else
+		iflp_prev_set = false;
 
-	if (link_params_set_value(s, iflp) != 0)
+	/* read the link_params from stream
+	 * Free ifp->link_params if the stream has no params
+	 * to means that link-params are not enabled on links.
+	 */
+	if (link_params_set_value(s, ifp) != 0)
 		goto stream_failure;
 
-	if (memcmp(&iflp_copy, iflp, sizeof(iflp_copy)))
-		params_changed = true;
+	if (changed == NULL)
+		return ifp;
 
-	if (changed)
-		*changed = params_changed;
+	if (iflp_prev_set && iflp) {
+		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) {
-		struct if_link_params *iflp = if_link_params_get(ifp);
-		link_params_set_value(s, iflp);
-	}
+	if (link_params_status)
+		link_params_set_value(s, ifp);
 
 	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);