[ofa-general] [PATCH 5/7 V3] osm: QoS - adding QoS policy options

Yevgeny Kliteynik kliteyn at dev.mellanox.co.il
Thu Aug 30 04:25:52 PDT 2007


Eitan Zahavi wrote:
> The standard option that is used by auto tools to have dependencies that
> are only used by a maintainer is:
> --enable-maintainer-mode
> 
> We use it to hide .l/.y/.i dependencies from the Makefile such that only
> those who declare themselves maintainers will refresh automatically
> generated source files. 

We don't have generated files in the git, so if someone gets OpenSM from
git repository, he *has* to generate those files when compiling OpenSM, which
means that the .l/.y dependencies cannot be hidden behind maintainer mode.

Another way to deal with this is to include generated files in the source tree,
and then re-generating those files would be hidden behind --enable-maintainer-mode
config option, and a user won't need lex/yacc at all to compile OpenSM.

I suggested this option when I was working on QoS for OFED 1.2, but as I
recall, the suggestion has met fierce resistance.

-- Yevgeny

> Why do you want to invent a new one?
> 
> Eitan Zahavi
> Senior Engineering Director, Software Architect
> Mellanox Technologies LTD
> Tel:+972-4-9097208
> Fax:+972-4-9593245
> P.O. Box 586 Yokneam 20692 ISRAEL
> 
>  
> 
>> -----Original Message-----
>> From: general-bounces at lists.openfabrics.org 
>> [mailto:general-bounces at lists.openfabrics.org] On Behalf Of 
>> Yevgeny Kliteynik
>> Sent: Thursday, August 30, 2007 4:32 AM
>> To: Hal Rosenstock
>> Cc: OpenIB
>> Subject: Re: [ofa-general] [PATCH 5/7 V3] osm: QoS - adding 
>> QoS policy options
>>
>> Hal Rosenstock wrote:
>>> On 8/29/07, Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il> wrote:
>>>> Hal Rosenstock wrote:
>>>>> On 8/29/07, Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il> wrote:
>>>>>> Hal Rosenstock wrote:
>>>>>>> On 8/27/07, Yevgeny Kliteynik 
>> <kliteyn at dev.mellanox.co.il> wrote:
>>>>>>>> Adding QoS policy file option to OpenSM optionsm and 
>> QoS Policy 
>>>>>>>> field to subn object.
>>>>>>>>
>>>>>>>> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>>>>>>>> ---
>>>>>>>>  opensm/include/opensm/osm_base.h   |   17 +++++++++++++++++
>>>>>>>>  opensm/include/opensm/osm_subnet.h |   13 +++++++++++++
>>>>>>>>  opensm/opensm/main.c               |   12 +++++++++++-
>>>>>>>>  opensm/opensm/osm_subnet.c         |   10 +++++++++-
>>>>>>>>  4 files changed, 50 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/opensm/include/opensm/osm_base.h 
>>>>>>>> b/opensm/include/opensm/osm_base.h
>>>>>>>> index 545779b..0a35e22 100644
>>>>>>>> --- a/opensm/include/opensm/osm_base.h
>>>>>>>> +++ b/opensm/include/opensm/osm_base.h
>>>>>>>> @@ -224,6 +224,23 @@ BEGIN_C_DECLS  #define 
>>>>>>>> OSM_DEFAULT_PARTITION_CONFIG_FILE 
>> "/etc/ofa/opensm-partitions.conf"
>>>>>>>>  #endif
>>>>>>>>  /***********/
>>>>>>>> +
>>>>>>>> +/****d* OpenSM: Base/OSM_DEFAULT_QOS_POLICY_FILE
>>>>>>>> +* NAME
>>>>>>>> +*      OSM_DEFAULT_QOS_POLICY_FILE
>>>>>>>> +*
>>>>>>>> +* DESCRIPTION
>>>>>>>> +*      Specifies the default QoS policy file name
>>>>>>>> +*
>>>>>>>> +* SYNOPSIS
>>>>>>>> +*/
>>>>>>>> +#ifdef __WIN__
>>>>>>>> +#define OSM_DEFAULT_QOS_POLICY_FILE strcat(GetOsmCachePath(), 
>>>>>>>> +"osm-qos-policy.conf") #else #define 
>> OSM_DEFAULT_QOS_POLICY_FILE 
>>>>>>>> +"/etc/ofa/opensm-qos-policy.conf"
>>>>>>>> +#endif
>>>>>>>> +/***********/
>>>>>>>> +
>>>>>>>>  /****d* OpenSM: Base/OSM_DEFAULT_SWEEP_INTERVAL_SECS
>>>>>>>>  * NAME
>>>>>>>>  *      OSM_DEFAULT_SWEEP_INTERVAL_SECS
>>>>>>>> diff --git a/opensm/include/opensm/osm_subnet.h 
>>>>>>>> b/opensm/include/opensm/osm_subnet.h
>>>>>>>> index a7543dc..5e8b522 100644
>>>>>>>> --- a/opensm/include/opensm/osm_subnet.h
>>>>>>>> +++ b/opensm/include/opensm/osm_subnet.h
>>>>>>>> @@ -68,6 +68,7 @@ BEGIN_C_DECLS
>>>>>>>>  #define OSM_SUBNET_VECTOR_GROW_SIZE                    1
>>>>>>>>  #define OSM_SUBNET_VECTOR_CAPACITY                     256
>>>>>>>>  struct _osm_opensm_t;
>>>>>>>> +struct _osm_qos_policy_t;
>>>>>>>>
>>>>>>>>  /****h* OpenSM/Subnet
>>>>>>>>  * NAME
>>>>>>>> @@ -257,6 +258,7 @@ typedef struct _osm_subn_opt {
>>>>>>>>        char *partition_config_file;
>>>>>>>>        boolean_t no_partition_enforcement;
>>>>>>>>        boolean_t no_qos;
>>>>>>>> +       char *qos_policy_file;
>>>>>>>>        boolean_t accum_log_file;
>>>>>>>>        char *console;
>>>>>>>>        uint16_t console_port;
>>>>>>>> @@ -398,6 +400,13 @@ typedef struct _osm_subn_opt {
>>>>>>>>  *              specified the log file will be 
>> truncated upon reaching
>>>>>>>>  *              this limit.
>>>>>>>>  *
>>>>>>>> +*      no_qos
>>>>>>>> +*              Boolean that specifies whether the 
>> OpenSM QoS functionality
>>>>>>>> +*              should be off or on.
>>>>>>>> +*
>>>>>>>> +*      qos_policy_file
>>>>>>>> +*              Name of the QoS policy file.
>>>>>>>> +*
>>>>>>> Are these options mutually exclusive or not ?
>>>>>> If QoS in OpenSM is off, the qos_policy_file will be ignored
>>>>> But aren't there two QoSs ? Can't the old QoS be run 
>> without this ? 
>>>>> I think that is a requirement.
>>>> OK, there are three ways we want this thing to work:
>>>> 1. QoS is off
>>>> 2. The old QoS is on but w/o policy file 3. The old QoS is 
>> on, plus 
>>>> reading policy file
>>>>
>>>> The first option is clear: if a user doesn't turns QoS on 
>> (-Q), QoS is off as before.
>>>> Second and third options: if QoS is on, OpenSM looks for 
>> policy file 
>>>> in the default location or in other location that was provided by 
>>>> user. If the file is not found, QoS works as before.
>>> This sounds OK to me and is my first preference.
>>>
>>>> Do we want to add additional option for "enhanced" QoS?
>>>> If so, we will have three QoS-ralated command line options:
>>>>  - option for turning the QoS on (currently -Q)
>>>>  - option to turn the new QoS on (some new letter - must get
>>>>    one quick before they all run out... :)
>>>>  - option for policy file location if differs from default 
>> (currently 
>>>> -Y)
>>> This seems like the least preferable to me. Also, would 
>> need to deal 
>>> with both on which seems to mean use new QoS.
>>>
>>>> Alternatively, we can turn -Q option into levels:
>>>>  -Q 0: QoS is off (default)
>>>>  -Q 1: old QoS is on
>>>>  -Q 2: old QoS plus reading policy file
>>> This one also seems OK to me (second preference).
>>>
>>> Anyone else with an opinion on this ? Sasha ?
>>>
>>> Also, what about --enable-qos-policy build option for those 
>> who don't 
>>> want to include this in their build ?
>> Do you mean  permanently, or just until the new QoS will be stable?
>> Note that if someone gets ofed rpm, he gets the generated 
>> files in this rpm, so he doesn't need les or yacc, so this 
>> option would be useful only for machines that have git but 
>> don't have lex or yacc.
>> Do you think it's a real issue?
>>
>> -- Yevgeny
>>
>>> -- Hal
>>>
>>>> -- Yevgeny
>>>>
>>>>>>>>  *      accum_log_file
>>>>>>>>  *              If TRUE (default) - the log file will 
>> be accumulated.
>>>>>>>>  *              If FALSE - the log file will be erased 
>> before starting current opensm run.
>>>>>>>> @@ -551,6 +560,7 @@ typedef struct _osm_subn {
>>>>>>>>        ib_net64_t sm_port_guid;
>>>>>>>>        uint8_t sm_state;
>>>>>>>>        osm_subn_opt_t opt;
>>>>>>>> +       struct _osm_qos_policy_t *p_qos_policy;
>>>>>>>>        uint16_t max_unicast_lid_ho;
>>>>>>>>        uint16_t max_multicast_lid_ho;
>>>>>>>>        uint8_t min_ca_mtu;
>>>>>>>> @@ -619,6 +629,9 @@ typedef struct _osm_subn {
>>>>>>>>  *      opt
>>>>>>>>  *              Subnet options structure contains site 
>> specific configuration.
>>>>>>>>  *
>>>>>>>> +*      p_qos_policy
>>>>>>>> +*              Subnet QoS policy structure.
>>>>>>>> +*
>>>>>>>>  *      max_unicast_lid_ho
>>>>>>>>  *              The minimal max unicast lid reported 
>> by all switches
>>>>>>>>  *
>>>>>>>> diff --git a/opensm/opensm/main.c b/opensm/opensm/main.c index 
>>>>>>>> e2541d8..599c4dc 100644
>>>>>>>> --- a/opensm/opensm/main.c
>>>>>>>> +++ b/opensm/opensm/main.c
>>>>>>>> @@ -269,6 +269,10 @@ void show_usage(void)
>>>>>>>>               "          The default name is \'"
>>>>>>>>               OSM_DEFAULT_PARTITION_CONFIG_FILE "\'.\n\n");
>>>>>>>>        printf("-Q\n" "--qos\n" "          This option 
>> enables QoS setup.\n\n");
>>>>>>>> +       printf("-Y\n"
>>>>>>>> +              "--qos_policy_file\n"
>>>>>>>> +              "          This option defines the 
>> optional QoS policy file.\n"
>>>>>>>> +              "          The default name is \'" 
>> OSM_DEFAULT_QOS_POLICY_FILE "\'.\n\n");
>>>>>>>>        printf("-N\n"
>>>>>>>>               "--no_part_enforce\n"
>>>>>>>>               "          This option disables 
>> partition enforcement on switch external ports.\n\n");
>>>>>>>> @@ -580,7 +584,7 @@ int main(int argc, char *argv[])
>>>>>>>>        char *ignore_guids_file_name = NULL;
>>>>>>>>        uint32_t val;
>>>>>>>>        const char *const short_option =
>>>>>>>> -           
>> "i:f:ed:g:l:L:s:t:a:u:R:zM:U:S:P:NBIQvVhorcyxp:n:q:k:C:";
>>>>>>>> +           
>>>>>>>> + "i:f:ed:g:l:L:s:t:a:u:R:zM:U:S:P:Y:NBIQvVhorcyxp:n:q:k:C:";
>>>>>>>>
>>>>>>>>        /*
>>>>>>>>           In the array below, the 2nd parameter specifies the 
>>>>>>>> number @@ -604,6 +608,7 @@ int main(int argc, char *argv[])
>>>>>>>>                {"Pconfig", 1, NULL, 'P'},
>>>>>>>>                {"no_part_enforce", 0, NULL, 'N'},
>>>>>>>>                {"qos", 0, NULL, 'Q'},
>>>>>>>> +               {"qos_policy_file", 1, NULL, 'Y'},
>>>>>>>>                {"maxsmps", 1, NULL, 'n'},
>>>>>>>>                {"console", 1, NULL, 'q'},
>>>>>>>>                {"V", 0, NULL, 'V'}, @@ -823,6 +828,11 @@ int 
>>>>>>>> main(int argc, char *argv[])
>>>>>>>>                        opt.no_qos = FALSE;
>>>>>>>>                        break;
>>>>>>>>
>>>>>>>> +               case 'Y':
>>>>>>>> +                       opt.qos_policy_file = optarg;
>>>>>>>> +                       printf(" QoS policy file 
>> \'%s\'\n", optarg);
>>>>>>>> +                       break;
>>>>>>>> +
>>>>>>> There should also be an update to the OpenSM man page for this.
>>>>>> That and a text file with description of policy file syntax
>>>>> Good. Thanks.
>>>>>
>>>>> -- Hal
>>>>>
>>>>>> -- Yevgeny
>>>>>>
>>>>>>> -- Hal
>>>>>>>
>>>>>>>>                case 'y':
>>>>>>>>                        opt.exit_on_fatal = FALSE;
>>>>>>>>                        printf(" Staying on fatal 
>> initialization 
>>>>>>>> errors\n"); diff --git a/opensm/opensm/osm_subnet.c 
>>>>>>>> b/opensm/opensm/osm_subnet.c index 818d73f..4162522 100644
>>>>>>>> --- a/opensm/opensm/osm_subnet.c
>>>>>>>> +++ b/opensm/opensm/osm_subnet.c
>>>>>>>> @@ -452,6 +452,7 @@ void osm_subn_set_default_opt(IN 
>> osm_subn_opt_t * const p_opt)
>>>>>>>>        p_opt->partition_config_file = 
>> OSM_DEFAULT_PARTITION_CONFIG_FILE;
>>>>>>>>        p_opt->no_partition_enforcement = FALSE;
>>>>>>>>        p_opt->no_qos = TRUE;
>>>>>>>> +       p_opt->qos_policy_file = OSM_DEFAULT_QOS_POLICY_FILE;
>>>>>>>>        p_opt->accum_log_file = TRUE;
>>>>>>>>        p_opt->port_profile_switch_nodes = FALSE;
>>>>>>>>        p_opt->pfn_ui_pre_lid_assign = NULL; @@ -1178,6 
>> +1179,9 @@ 
>>>>>>>> ib_api_status_t osm_subn_parse_conf_file(IN osm_subn_opt_t * 
>>>>>>>> const p_opts)
>>>>>>>>
>>>>>>>>                opts_unpack_boolean("no_qos", p_key, p_val, 
>>>>>>>> &p_opts->no_qos);
>>>>>>>>
>>>>>>>> +               opts_unpack_charp("qos_policy_file",
>>>>>>>> +                                   p_key, p_val, 
>>>>>>>> + &p_opts->qos_policy_file);
>>>>>>>> +
>>>>>>>>                opts_unpack_boolean("accum_log_file",
>>>>>>>>                                    p_key, p_val, 
>>>>>>>> &p_opts->accum_log_file);
>>>>>>>>
>>>>>>>> @@ -1541,7 +1545,11 @@ ib_api_status_t 
>> osm_subn_write_conf_file(IN osm_subn_opt_t * const p_opts)
>>>>>>>>        fprintf(opts_file,
>>>>>>>>                "#\n# QoS OPTIONS\n#\n"
>>>>>>>>                "# Disable QoS setup\n"
>>>>>>>> -               "no_qos %s\n\n", p_opts->no_qos ? 
>> "TRUE" : "FALSE");
>>>>>>>> +               "no_qos %s\n\n"
>>>>>>>> +               "# QoS policy file to be used\n"
>>>>>>>> +               "qos_policy_file %s\n\n",
>>>>>>>> +               p_opts->no_qos ? "TRUE" : "FALSE",
>>>>>>>> +               p_opts->qos_policy_file);
>>>>>>>>
>>>>>>>>        subn_dump_qos_options(opts_file,
>>>>>>>>                              "QoS default options", "qos",
>>>>>>>> --
>>>>>>>> 1.5.1.4
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> general mailing list
>>>>>>>> general at lists.openfabrics.org
>>>>>>>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>>>>>>>>
>>>>>>>> To unsubscribe, please visit 
>>>>>>>> http://openib.org/mailman/listinfo/openib-general
>>>>>>>>
>> _______________________________________________
>> general mailing list
>> general at lists.openfabrics.org
>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>>
>> To unsubscribe, please visit 
>> http://openib.org/mailman/listinfo/openib-general
>>
> 




More information about the general mailing list