[ofa-general] Re: [PATCH 3/3] osm: QoS - parsing port names

[Sasha Khapyorsky]

Hi Yevgeny,

On 00:32 Mon 15 Oct     , Yevgeny Kliteynik wrote:
> >>>> Added CA-by-name hash to the QoS policy object and
> >>> Why it is called "CA"-by-name? In the code below I see that hash is
> >>> created for all nodes (including switches and routers).
> >>  In osm_qos_policy.c:
> >>
> >>  	if (p_node->node_info.node_type == IB_NODE_TYPE_CA)
> >>  		st_insert(p_qos_policy->p_ca_hash,
> >>  			  (st_data_t)p_node->print_desc,
> >>  			  (st_data_t)p_node);
> > Ok, so what is wrong with switches and routers? Why it cannot be
> > specified by "name"?
> 
>  Switches have the NodeDescription filled by FW, and it's usually the
>  same string for all the switches.

It must not be same. Also I suppose that node description can be changed
at least for some managed switches even today.

>  Also, what would be the meaning of
>  "host id" for switches?

I don't like "host id" approach - it assume a predefined node
description format.

>  As for routers - I don't know what's going on there, since I didn't
>  get the chance to lay my hands on it yet (those IB routers are hard
>  to get right now :-)

So I think it is better to include switches and routers here and to use
"node name" instead of "CA name". In worst case when all records are
same we will lost one or two hash table entries per fabric.

> >>  From /etc/init.d/openibd:
> >>
> >>      # Add node description to sysfs
> >>      IBSYSDIR="/sys/class/infiniband"
> >>      if [ -d ${IBSYSDIR} ]; then
> >>          declare -i hca_id=1
> >>          for hca in ${IBSYSDIR}/*
> >>          do
> >>              if [ -e ${hca}/node_desc ]; then
> >>                  echo -n "$(hostname -s) HCA-${hca_id}" >> 
> >> ${hca}/node_desc
> >>              fi
> >>              let hca_id++
> >>          done
> >>      fi
> > This script is optional, even when used the way how node_desc is
> > generated can be easily changed. I think it is not good idea to copy the
> > algorithm to OpenSM code and in this way to enforce an user to use the
> > only this hardcoded node_desc format.
> > Actually this (or another similar) script is sort of config file, as
> > well as qos policy file, and both are in admin's hands. So basically I
> > agree that it is ok to require to define node_desc (if an admin wishes
> > to use names for her QoS). _But_ we cannot dictate how it should be
> > generated - it clearly must be user's and not our choice.
> > So instead of approaching hardcoded node_desc format I think that name
> > definition in qos policy file should refer node_desc as whole string
> > (well, in improved case it could be single substring with wild cards).
> 
>  Ok, so let's elaborate on this.
> 
>  Currently NodeDescription is filled by the openibd script.
>  Although this script can be modified by admin, I doubt that an average
>  admin would like to tweak it. Thus, I believe that in most cases the
>  NodeDescription will look this way: "node-id hca-num".

It must not be so. For instance I'm not using this script at all, OpenSM
must not be installed as part of OFED, nodes can run other than Linux
OSes (Solar*, Win*, etc.), etc.

>  If we want to allow port names to have number ranges or asterisk
>  (and we do want it), then we have to have *some* format.

Why? What is wrong with plain strings?

Look at example - you are able to use wildcards with 'ls' command even
if file names doesn't have any predefined format. Right?

>  So here's my suggestion:
>  1. First of all, when the ca-by-name hash is created, osm will check that
>     the NodeDescriptions are unique. If they aren't - parsing of the port
>     names will be off, even if specified in the policy file.

It is overkill IMO - an user is responsible to setup things properly,
finally it is her choice in which tricky way to use it.

>  2. If a port name doesn't have any special characters, it will be compared
>     to the NodeDescription as is, and it'd better be unique

Ok.

>  3. If the admin would like to include num. ranges and asterisks in the
>     port name, he has to make sure that the NodeDescription is created
>     like it is created now by openibd.

Again, why this limitation is needed? What is wrong with wildcards like
"myname*", "hostname[1-3] *", etc.?

>  Sounds ok?

(2) + (3 without format limitation) looks fine for me.

Sasha