search

sflow / host-sflow

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

Make switching between ipv4 and ipv6 for interface adaptors #43

Closed ghost closed 3 years ago

ghost commented 3 years ago

Signed-off-by: Maksym Belei Maksym_Belei@jabil.com

- What I did There is the related issue in SONiC community: https://github.com/Azure/sonic-buildimage/issues/6951 According to the issue, hsflowd should switch sflow agent to IPv6, if IPv4 is not present for the interface, or, it was deleted from the system. With the changes, hsflowd will switch the agent to IPv6 address if IPv4 no more available and will back to IPv4 if it will be added to the system. The switching is being performed according to address priority ranking.

- How I did it By crossing around all the records, related to required network interface, inside both localIP and localIP6 containers. All the found records is being compared and selected the most priority address. So, if the IPv4 address will be deleted from the system, the next in priority rank address will be selected.

- How to verify it Please, take a look on steps to reproduce in https://github.com/Azure/sonic-buildimage/issues/6951. Sflow agent should switch between IPv4 and IPv6 while removing or adding a IPv4 entry for the related network interface.

- Additional information As I have only device with SONiC for testing, I have made the changes inside host-sflow only for Linux platform. Probably, the same changes could be desirable for other platforms

ghost commented 3 years ago

@sflow, please, take a look on this PR. More details regarding the issue could be found here: https://github.com/Azure/sonic-buildimage/issues/6951. Maybe, you could suggest some more efficient way to solve the issue? I have a mind about alternative solution. A some kind of related check could be added to readIPv6Addresses function in readInterfaces.c file, but, in my opinion, current solution is little bit clearer.

sflow commented 3 years ago

What version are you running? If I have understood then this commit on Jan 26 will do what you want:

https://github.com/sflow/host-sflow/commit/be8f404f5a03a9507cdf3087b62d1db8950e68c4

ghost commented 3 years ago

I checked the issue on build from March 1st 2021, so, change https://github.com/sflow/host-sflow/commit/be8f404f5a03a9507cdf3087b62d1db8950e68c4 is present in it. But, our case is next: https://github.com/sflow/host-sflow/blob/master/src/Linux/hsflowconfig.c#L1107. We have the agent in current settings. Here is a peace of log from hsflowd:

dbg1: selectAgentAddress
dbg1: selectAgentAddress pegged to device in current settings
dbg1: selectAgentAddress selected agentIP with highest priority: device=Loopback1, address=192.168.101.1, previous=240.127.1.1, changed=YES
sflow commented 3 years ago

Oh, I see what you mean now. The flexibility to select an IPv6 agent address is not there when /etc/hsflowd.conf specifies the agent device explicitly. It only looks at the primary IPv4 address. That's a bug. In that scenario it should choose between all the addresses that belong to that interface, applying the priority rules. That way if the interface currently has no IPv4 address (or if /etc/hsflowd.conf has something like agent.cidr=::/0) then it can use the IPv6 address instead.

I'll take a look. But you might find that you can work around the problem by adding an agent.cidr=list of CIDRs entry instead of an agent=device entry. The list of CIDRs should accept both v4 and v6 CIDRs so you can choose your own priority system. Maybe something like agent.cidr = 10.100.0.0/16,FEC0::/16. If that makes sense for your network?

ghost commented 3 years ago

Thank you for your suggestion. But, I think the workaround will be not completely suitable for SONiC as it has concrete CLI command to set sflow agent interface. Also, the workaround works with range of IP addresses, but concrete network interface could have different addresses, what makes hard to handle all of them in such way. In addition, configuring of hsflowd in SONiC makes through redis database in mod_sonic.c, so, the workaround still requires changes in source code of hsflowd. I'd like to try to modify my changes to select IP address for concrete adaptor by priority, how it has done in be8f404. I believe the solution should be the similar, but we need to check only adaptor with required dev name. Am I right?

ghost commented 3 years ago

@sflow, I have updated the PR with the changes, according to your last comment. Now, the IP address is being selected according to priority ranking, as it does for cases, where there is no hard coded agent device in the daemon configuration. I have tested it on the device, works good for me. Could you check it?

sflow commented 3 years ago

I checked in a change for this. It treats the agent=device setting as a constraint that narrows the election to consider only the subset of IPv4 and IPv6 addresses that belong to that device:

https://github.com/sflow/host-sflow/commit/2703ecb173bdbc6b1bc6cdea524eafa9640f141e

So it can end up with an IPv6 address if there was no IPv4 option.

It's just possible that this change may cause an existing system to pick a different address than it did before with the same settings. That is unlikely though, and fixing this bug was deemed more important.

Thank you for pointing this out. If you confirm that this change works for you then we'll tag a new release.

ghost commented 3 years ago

Thank you for pointing this out. If you confirm that this change works for you then we'll tag a new release. @sflow, yes, the changes work for me. If there are no objections or suggestions from side of Sflow team, could you merge this changes?

ghost commented 3 years ago

@sflow, sorry for misunderstanding. I see that change 2703ecb is currently merged and it looks like the change is completely represents changes in current PR, am I right? If so, I can close the PR as unnecessary.

ghost commented 3 years ago

@sflow, I have tested change https://github.com/sflow/host-sflow/commit/2703ecb173bdbc6b1bc6cdea524eafa9640f141e. Looks good. Could you tag a new release?