[collectd] [patch] Fix for bogus interfaces under Solaris

[Florian Forster]

Hi Eddy,

thank you very much for your patch!

On Wed, Aug 31, 2011 at 10:18:35AM -0400, Eddy Geez wrote:
> +               /* ignore kstat entry if not the regular statistic set */
> +               if (strncmp (ksp_chain->ks_name, ksp_chain->ks_module,
> strlen(ksp_chain->ks_module)))
> +                       continue;

What does this do? Or rather, why does this do something desirable? I
think this is meant to only allow "modules" and "names" like "eth" /
"eth0" and will filter out "eth" / "mac".

For this particular case, I think I'd prefer if you could use the
"ignore list" to filter these out, e.g.:

  <Plugin interface>
    Interface "mac"
    IgnoreSelected true
  </Plugin>

Rather than adding special code like this one, I could picture a
different change instead. Something along the lines of:

  if ks_name == "mac"
    type_instance = ks_module + ks_instance + "-mac"
  else
    default behavior

This should give each "mac" a unique name. If you're not interested in
the statistics at all, you can filter them out using:

  Interface "/-mac$/"
  IgnoreSelected true

> -               if ((val = get_kstat_value (ksp_chain, "obytes")) == -1LL)
> +               if ((val = get_kstat_value (ksp_chain, "ifspeed")) == -1LL)

I'm guessing this is meant to avoid the problem with the wrsmd
"interfaces". Are you sure all "valid" interfaces have an interface
speed set in Solaris? There are some weird interfaces out there and in
my experience tunnel interfaces or bonding interfaces may have a hard
time telling their "speed".

These interfaces can also be filtered out using:

  Interface "/^wrsmd/"
  IgnoreSelected true

I agree that "interface" is probably not the most intuitive place for
these kind of statistics, but from what I read we're talking about
remote DMA here, so one *might* argue that this is networky ;) Besides,
removing these "interfaces" would break backwards compatibility and I
wouldn't be surprised if we get complains from people actually using WCI
devices …

> Also, for what it's worth, I needed to add this line to 'configure.in'
> so that 'errno' worked properly under Solaris. Otherwise the call to
> stat() in rrd_write in 'rrdtool.c' always resulted in a bogus error
> about the disk being out of space.
> 
> if test "x$ac_system" = "xSolaris"
> then
>         AC_DEFINE(_POSIX_PTHREAD_SEMANTICS, 1, [Define to enforce
> POSIX thread semantics under Solaris.])
>         AC_DEFINE(_REENTRANT, 1, [Define so errno works under Solaris.])
> fi

Oh, which version of Solaris is that?

Best regards,
—octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x0C705A15
http://octo.it/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mailman.verplant.org/pipermail/collectd/attachments/20110903/4baf931a/attachment.pgp>