lib,zebra: do not enable link-params when a link-params command fails

A given interface has no enabled link-params context. If a link-params
configuration command fails, the link-params is wrongly enabled:

> r4(config-link-params)# no enable
> r4(config-link-params)# delay
>   (0-16777215)  Average delay in micro-second as decimal (0...16777215)
> r4(config-link-params)# delay 50 min 300 max 500
> Average delay should be comprise between Min (300) and Max (500) delay
> r4(config-link-params)# do sh run zebra
> (...)
> interface eth-rt1
> link-params
>  enable
> exit-link-params

link-params are enabled if and only if the interface structure has a
valid link_params pointer. Before checking the command validity,
if_link_params_get() is called to retrieve the link-params pointer.
However, this function initializes the pointer if it is NULL.

Only use if_link_params_get() to retrieve the pointer to avoid
confusion. In command setting functions, initialize the link_params
pointer if needed only after the validation of the command.

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

diff --git a/lib/if.c b/lib/if.c
index fa4fdb82d38af2f01bba2717f256af4e1a7d0a91..deb0690dcf12c0eab5d4b648b6d4d70199d865dc 100644 (file)
--- a/lib/if.c
+++ b/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)
 {
-       int i;
+       return ifp->link_params;
+}
 
-       if (ifp->link_params != NULL)
-               return ifp->link_params;
+struct if_link_params *if_link_params_enable(struct interface *ifp)
+{
+       struct if_link_params *iflp;
+       int i;
 
-       struct if_link_params *iflp =
-               XCALLOC(MTYPE_IF_LINK_PARAMS, sizeof(struct if_link_params));
+       iflp = if_link_params_init(ifp);
 
        /* 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);

diff --git a/lib/if.h b/lib/if.h
index 1c948b875a458a6e178f087042ef70eb3d8aa80a..478a90d63a7cf7d4d422da362a8448d8427ff9c5 100644 (file)
--- a/lib/if.h
+++ b/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. */

diff --git a/zebra/interface.c b/zebra/interface.c
index c674b499ac82ad6ec4e30506300262ab653630ec..5d62ec071f472f9c9593f083809ebc6165f187b8 100644 (file)
--- a/zebra/interface.c
+++ b/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 (IS_ZEBRA_DEBUG_EVENT || IS_ZEBRA_DEBUG_MPLS)
-                       zlog_debug(
-                               "Link-params: failed to init TE link parameters  %s",
-                               ifp->name);
-
-               return CMD_WARNING_CONFIG_FAILED;
-       }
+       if (!if_link_params_get(ifp))
+               if_link_params_enable(ifp);
 
        /* 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])
-           || (bw <= iflp->unrsv_bw[1]) || (bw <= iflp->unrsv_bw[2])
-           || (bw <= iflp->unrsv_bw[3]) || (bw <= iflp->unrsv_bw[4])
-           || (bw <= iflp->unrsv_bw[5]) || (bw <= iflp->unrsv_bw[6])
-           || (bw <= iflp->unrsv_bw[7]) || (bw <= iflp->ava_bw)
-           || (bw <= iflp->res_bw) || (bw <= iflp->use_bw)) {
+       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[3]) || (bw <= iflp->unrsv_bw[4]) ||
+                    (bw <= iflp->unrsv_bw[5]) || (bw <= iflp->unrsv_bw[6]) ||
+                    (bw <= iflp->unrsv_bw[7]) || (bw <= iflp->ava_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)
-                   && (delay < iflp->min_delay || delay > iflp->max_delay)) {
+               if (iflp && IS_PARAM_SET(iflp, LP_MM_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);