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

olivier.dugeon at orange.com olivier.dugeon at orange.com
Fri Dec 2 09:00:04 EST 2016


Hi Donald,

LP_TE_METRIC is not defined. I only define LP_TE in lib/if.h.

Regards

Olivier


Le 02/12/2016 à 13:42, Donald Sharp a écrit :
> 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.


_________________________________________________________________________________________________________________________

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.frrouting.org/pipermail/dev/attachments/20161202/f265ee82/attachment.html>


More information about the dev mailing list