batch move to milestone 1.3

set default ticket origin to Community

Added initial screened field value.

The easy way to fix this is to add a new configuration setting to "cn=config". This setting would allow you to define the SNMP "index" to use for that particular DS instance. For example, if I have 2 DS instances on the same host who both use port 389 on different network interfaces, I would be able to arbitrarily assign index number "50" to the first instance, and index number "51" to the second instance. In the config for the first instance, it would look something like this:

nsslapd-snmp-index: 50

The nice thing with this approach is that it requires no changes to the MIB or how our current SNMP results are interpreted.

If the new config setting is not specified, the port number should be used as the index as we do currently.

'''Here is the current status'''

• Sorry for the delay, at a first read I thought the request was to have DS listening on several interfaces (nsslapd-listenhost) and I spent some time to understand what needed to be done.

• I implement the fix and sent a review
  '''
  Here are the next steps'''

• Waiting for the review

attachment
0001-Ticket-205-snmp-counters-index-strings-for-multiple-.patch

The code for the SNMP subagent looks fine, but you will have a problem if you try to add the nsslapd-snmp-index attribute to "cn=config". If you attempt to add this new attribute using ldapmodify, the server will reject it with err=53. You can manually edit dse.ldif when ns-slapd is stopped, but you will get an error about the config attribute being unknown when you start ns-slapd.

Even though ns-slapd doesn't need to do anything with the new nsslapd-snmp-index attribute, we will still need to make some code changes to prevent these errors.

Here is the current status

• integrate the remarks from Nathan review. Basically the first patch did not taken into account a change online of nsslapd-snmp-index

Here are the next steps

• waiting for the second review

attachment
0002-Ticket-205-snmp-counters-index-strings-for-multiple-.patch

I don't think you should remove this block of code, as it is there to prevent us from processing through every line of dse.ldif after we have found everything that we were looking for in the file:
{{{
/ Stop processing this entry if we found the
* port and rundir settings /
if (got_port && got_rundir) {
break;
}
}}}

Now that one may optionally specify the nsslapd-snmp-index attribute, we will have to process through the entire LDIF if the optional attribute is not set, but we can at least stop processing the dse.ldif if we have already found the optional attribute. I think you can leave this block in and change the condition as a small optimization:

{{{
/ Stop processing this entry if we found the
* port, rundir, and SNMP index settings /
if (got_port && got_rundir && got_snmp_index) {
break;
}
}}}

Aside from that, the patch looks good to me.

Thanks for you review Nathan.

My understanding is that this accelerator test is only when parsing the entry 'dn: cn=config'. If parsing this entry and found port+rundir, it was fine to stop parsing this entry.
In case of an optional snmp-index, we need to go through the complete entry 'cn=config' to know if a snmp-index is or not defined later.

I will double check that.

Replying to [comment:14 tbordaz]:

Thanks for you review Nathan.

My understanding is that this accelerator test is only when parsing the entry 'dn: cn=config'. If parsing this entry and found port+rundir, it was fine to stop parsing this entry.
In case of an optional snmp-index, we need to go through the complete entry 'cn=config' to know if a snmp-index is or not defined later.

I will double check that.

You are correct. It is only processing cn=config, not the entire dse.ldif. We can stop parsing the entry if we find port+rundir+snmp-index.

Hi Nathan,

thanks for the review.
I changed the fix to leave the block that stop the processing if all researched info (port/rundir/snmp_index) have been found.
I attached the new patch 0003-Ticket-205-snmp-counters-index-strings-for-multiple-.patch

thierry

attachment
0003-Ticket-205-snmp-counters-index-strings-for-multiple-.patch

git merge ticket205_review
Updating bb7a41b..2f02b0d
Fast-forward
ldap/servers/slapd/libglobs.c | 40 +++++++++++++++++++++++++++++++++++++++-
ldap/servers/slapd/proto-slap.h | 1 +
ldap/servers/slapd/slap.h | 2 ++
ldap/servers/snmp/main.c | 22 +++++++++++++++++++---
4 files changed, 61 insertions(+), 4 deletions(-)

commit 2f02b0d
Author: Thierry bordaz (tbordaz) tbordaz@redhat.com
Date: Wed Apr 17 16:06:32 2013 +0200

git push origin master
Counting objects: 19, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 2.52 KiB, done.
Total 10 (delta 8), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
bb7a41b..2f02b0d master -> master

Fix compiler warning
0001-Ticket-205-Fix-compiler-warning.patch

Fixed compiler warning:

git merge ticket205
Updating 7c8c32e..1838c0b
Fast-forward
ldap/servers/slapd/libglobs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

git push origin master
Counting objects: 11, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 617 bytes, done.
Total 6 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
7c8c32e..1838c0b master -> master

commit 1838c0b

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/205

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.