[cmaster-next] [PATCH] lib, zebra: Minimize display of link-params sub data

Donald Sharp sharpd at cumulusnetworks.com
Fri Dec 2 07:42:40 EST 2016


My latest patch changed to this:
-  if (IS_PARAM_SET(iflp, LP_TE))
+  if (IS_PARAM_SET(iflp, LP_TE) && IS_PARAM_SET(iflp, LP_TE_METRIC))

donald

On Thu, Dec 1, 2016 at 11:34 AM,  <olivier.dugeon at orange.com> wrote:
> Donald,
>
> Yes, but in the same time you propose to compare te-metric and metric in
> your patch:
>
> -  if (IS_PARAM_SET(iflp, LP_TE))
> +  if (IS_PARAM_SET(iflp, LP_TE) && iflp->te_metric !=
> (u_int32_t)ifp->metric)
>
> Just a typo error ? Or I missed something ? I just suggest to not apply this
> part of the patch.
>
> Regards
>
> Olivier
>
>
> Le 01/12/2016 à 16:35, Donald Sharp a écrit :
>
> I'm not saying the metric is redundant, I'm saying that we should only
> display the te_metric if it has been configed different than what the
> link has been set to.
>
> donald
>
> On Thu, Dec 1, 2016 at 9:32 AM,  <olivier.dugeon at orange.com> wrote:
>
> Hi Donald,
>
> Seems good for bandwidth (I check that there is no problem when
> communicating with IGP daemon trhough zbus), but not sure it is suitable for
> the metric.
>
> Indeed, te_metric could be equal to the metric. Yes, I agree it is
> redundant, but it is 2 separate values from the IGP point of view. So, I
> prefer to keep the original test that just check if the metric has been set
> or not to determine if we need to print the te_metric value or not. Perhaps,
> to avoid any ambiguity, we could change the 'metric' statement by
> 'te-metric' statement. So, it will be more distinguishable from the standard
> IGP metric.
>
> Regards
>
> Olivier
>
>
> Le 01/12/2016 à 14:25, Donald Sharp a écrit :
>
> Olivier -
>
> Here's my attempt to clean up the display of link-params.  Thoughts?
>
> donald
>
> On Thu, Dec 1, 2016 at 8:16 AM, Donald Sharp <sharpd at cumulusnetworks.com>
> wrote:
>
> When link-params is configured it auto starts displaying
> 6000-02# conf t
> dell-s6000-02(config)# int swp1
> dell-s6000-02(config-if)# link-params
> dell-s6000-02(config-link-params)# admin-grp 0x12345678
> dell-s6000-02(config-link-params)# end
> dell-s6000-02# show run
>
> interface swp1
>  link-params
>   enable
>   metric 0              <----Remove the bw lines
>   max-bw 1.25e+06
>   max-rsv-bw 1.25e+06
>   unrsv-bw 0 1.25e+06
>   unrsv-bw 1 1.25e+06
>   unrsv-bw 2 1.25e+06
>   unrsv-bw 3 1.25e+06
>   unrsv-bw 4 1.25e+06
>   unrsv-bw 5 1.25e+06
>   unrsv-bw 6 1.25e+06
>   unrsv-bw 7 1.25e+06
>   admin-grp 305419896
>   exit-link-params
> !
>
> I'd like to reduce this to:
>
> interface enp0s3
>  ip igmp
>  ip pim sm
>  link-params
>   enable
>   admin-grp 0x12345678    <----- Fix this to be what we entered
>   exit-link-params
> !
>
> (cherry picked from commit fc256e310c9d231e8bc1e0a147009463c95ab19f)
> ---
>  lib/if.c          | 10 +++++-----
>  lib/if.h          |  1 +
>  zebra/interface.c | 13 +++++++------
>  3 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/lib/if.c b/lib/if.c
> index df53f8d..e745dc9 100644
> --- a/lib/if.c
> +++ b/lib/if.c
> @@ -1366,14 +1366,14 @@ if_link_params_get (struct interface *ifp)
>    iflp->te_metric = ifp->metric;
>
>    /* Compute default bandwidth based on interface */
> -  int bw = (float)((ifp->bandwidth ? ifp->bandwidth : DEFAULT_BANDWIDTH)
> -                   * TE_KILO_BIT / TE_BYTE);
> +  iflp->default_bw = ((ifp->bandwidth ? ifp->bandwidth : DEFAULT_BANDWIDTH)
> +                     * TE_KILO_BIT / TE_BYTE);
>
>    /* Set Max, Reservable and Unreserved Bandwidth */
> -  iflp->max_bw = bw;
> -  iflp->max_rsv_bw = bw;
> +  iflp->max_bw = iflp->default_bw;
> +  iflp->max_rsv_bw = iflp->default_bw;
>    for (i = 0; i < MAX_CLASS_TYPE; i++)
> -    iflp->unrsv_bw[i] = bw;
> +    iflp->unrsv_bw[i] = iflp->default_bw;
>
>    /* Update Link parameters status */
>    iflp->lp_status = LP_TE | LP_MAX_BW | LP_MAX_RSV_BW | LP_UNRSV_BW;
> diff --git a/lib/if.h b/lib/if.h
> index 7fdd46d..0041a1a 100644
> --- a/lib/if.h
> +++ b/lib/if.h
> @@ -174,6 +174,7 @@ struct if_stats
>  struct if_link_params {
>    u_int32_t lp_status;   /* Status of Link Parameters: */
>    u_int32_t te_metric;   /* Traffic Engineering metric */
> +  float default_bw;
>    float max_bw;          /* Maximum Bandwidth */
>    float max_rsv_bw;      /* Maximum Reservable Bandwidth */
>    float unrsv_bw[MAX_CLASS_TYPE];     /* Unreserved Bandwidth per Class
> Type (8) */
> diff --git a/zebra/interface.c b/zebra/interface.c
> index b87f61f..f7ec650 100644
> --- a/zebra/interface.c
> +++ b/zebra/interface.c
> @@ -2787,20 +2787,21 @@ link_params_config_write (struct vty *vty, struct
> interface *ifp)
>
>    vty_out (vty, " link-params%s", VTY_NEWLINE);
>    vty_out(vty, "  enable%s", VTY_NEWLINE);
> -  if (IS_PARAM_SET(iflp, LP_TE))
> +  if (IS_PARAM_SET(iflp, LP_TE) && iflp->te_metric !=
> (u_int32_t)ifp->metric)
>      vty_out(vty, "  metric %u%s",iflp->te_metric, VTY_NEWLINE);
> -  if (IS_PARAM_SET(iflp, LP_MAX_BW))
> +  if (IS_PARAM_SET(iflp, LP_MAX_BW) && iflp->max_bw != iflp->default_bw)
>      vty_out(vty, "  max-bw %g%s", iflp->max_bw, VTY_NEWLINE);
> -  if (IS_PARAM_SET(iflp, LP_MAX_RSV_BW))
> +  if (IS_PARAM_SET(iflp, LP_MAX_RSV_BW) && iflp->max_rsv_bw !=
> iflp->default_bw)
>      vty_out(vty, "  max-rsv-bw %g%s", iflp->max_rsv_bw, VTY_NEWLINE);
>    if (IS_PARAM_SET(iflp, LP_UNRSV_BW))
>      {
>        for (i = 0; i < 8; i++)
> -        vty_out(vty, "  unrsv-bw %d %g%s",
> -            i, iflp->unrsv_bw[i], VTY_NEWLINE);
> +       if (iflp->unrsv_bw[i] != iflp->default_bw)
> +         vty_out(vty, "  unrsv-bw %d %g%s",
> +                 i, iflp->unrsv_bw[i], VTY_NEWLINE);
>      }
>    if (IS_PARAM_SET(iflp, LP_ADM_GRP))
> -    vty_out(vty, "  admin-grp %u%s", iflp->admin_grp, VTY_NEWLINE);
> +    vty_out(vty, "  admin-grp 0x%x%s", iflp->admin_grp, VTY_NEWLINE);
>    if (IS_PARAM_SET(iflp, LP_DELAY))
>      {
>        vty_out(vty, "  delay %u", iflp->av_delay);
> --
> 2.5.5
>
>
> _________________________________________________________________________________________________________________________
>
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu
> ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
> electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou
> falsifie. Merci.
>
> This message and its attachments may contain confidential or privileged
> information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and
> delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been
> modified, changed or falsified.
> Thank you.
>
>
> _________________________________________________________________________________________________________________________
>
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu
> ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
> electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou
> falsifie. Merci.
>
> This message and its attachments may contain confidential or privileged
> information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and
> delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been
> modified, changed or falsified.
> Thank you.




More information about the dev mailing list