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>

diff --git a/lib/zclient.c b/lib/zclient.c
index 8ec82ab7bb91c5bd0f3def83b7e981e7f3f0af0e..f5d45b40ef871a2403052ebf1e7c1ca2744e1f95 100644 (file)
--- 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);

diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c
index 761ba789b87afb1ab3193956ec67432477444a6a..4d7ad21bf367d5e4b3b4b62aa4fd42aade3116f5 100644 (file)
--- a/zebra/zapi_msg.c
+++ b/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 */