sflow / host-sflow

host-sflow agent
http://sflow.net
Other
146 stars 55 forks source link

Sampling rate doesn't get programmed if sflow is enabled on pre-provisioned bonds #22

Closed yevvievvy closed 7 years ago

yevvievvy commented 7 years ago

This issue is observed in both the Base and Premium versions of OS10.

Sampling rate config doesn't get programmed in the h/w for LAG (bonds), if sflow is enabled on a pre-provisioned LAG (say a static lag with member slave ports already added).

Would uncommenting adaptorNIO->bond_master check help solve this issue ? For this config scenario updateBondCounters might be called only from agentCB_getCounters_interface...??

hsflowd version @2.0.8-1 : https://github.com/sflow/host-sflow/blob/v2.0.8-1/src/Linux/readPackets.c#123 if(/adaptorNIO->bond_master ||/ adaptorNIO->bond_slave) { updateBondCounters(sp, adaptor);

Likely updateBondCounters function would quit on failed if (procFile) check for bond_slave (member port name) @ https://github.com/sflow/host-sflow/blob/v2.0.8-1/src/Linux/readNioCounters.c#41 ?

Pls check

sflow commented 7 years ago

It looks like you are right that updateBondCounters() is not going to be called as often as it should be. However it should still be called every time the interfaces are re-checked here: https://github.com/sflow/host-sflow/blob/master/src/Linux/hsflowd.c#L566

The question is: will /proc/net/bonding/ be populated on OS10 in all cases? Or do we need to call out to another script to learn things like attachedAggID and actorSystemID? If LACP is actually running then it would be good if we could populate the LACP counters too, but the important thing is to get attachedAggID (LAG ifIndex) and actorSystemID (LAG/MLAG MAC-address).

The intention is to enable packet-sampling on the physical slave ports only. If an interface is marked as a bond_master then mod_os10 won't call out to set the sampling rate: https://github.com/sflow/host-sflow/blob/master/src/Linux/mod_os10.c#L249-L253 Does that work on OS10? Is /opt/dell/os10/bin/cps_config_sflow OK with setting sampling on each slave separately?

The test you are referring to in readPackets.c controls the export of the sFlow LAG counters structure (http://sflow.org/sflow_lag.txt). As long as the slaves are all reporting packet-samples and counter-samples that include this structure, then the sFlow collector has the full picture. So this structure should be included even if the LAG is static. It doesn't have to be running LACP. https://github.com/sflow/host-sflow/blob/master/src/Linux/readPackets.c#L119-L129

(Doing it this way also neatly avoids a difficult scenario that bites when you have a LAG with slaves on different line cards. To export counters+packet-samples with correct sequence numbers for a "pan-chassis-LAG" like that would require some awkward inter-processor coordination.)

yevvievvy commented 7 years ago

I mean, sflow is not programmed for bond slaves (physical member ports). Hence no packet-samples get reported. This being the primary issue to be addressed, for mentioned sequence of configuration. That said, if sflow is enabled first on the LAG (via switchport=bo1 added in hsflowd.conf & hsflowd run) and member ports are added next, this issue doesn't show up. Also during the original issue, if a member port is added additionally, sflow gets programmed in the h/w.

Atleast in the premium, at the kernel only static LAGs are created & no LACP is run at the Linux OS level. This behavior wouldn't be any different in Base too, I suppose.

sflow commented 7 years ago

hsflowd mod_os10 will configure sampling on physical ports that (a) have non-zero ifSpeed and (b) match the switchport regex. Quite separately, metadata about LAGs is collected and sent along with counter samples.

So I don't think it makes sense to set switchport=bo1 in hsflowd.conf. Why does that even work at all? When the LAG is created, do the names of the slave ports change so that they match the regex "bo1"? Is that why this is difficult? If so, what do the names become?

At the point where you translate the CLI config into a new hsflowd.conf, could you not translate "bo1" into something like switchport="e101-020-1|e101-020-2", or even something like "bo1-020-1|bo1-020-2" if that is what the names have become? I guess that would mean you have to restart hsflowd whenever the LAG membership changed, but at least the model would be clear. What do you think?

sflow commented 7 years ago

I checked in changes that should allow you to list the physical switch ports one at a time in the os10 section of the config file. If present, this takes over from the regex pattern. Example:

os10 { port { dev=e101-100-1 } port { dev=e101-100-2 } }

This should help to overcome the limitation that the regex string has a maximum length. Please let me know if this works.