Skip to content
This repository has been archived by the owner on Feb 19, 2025. It is now read-only.

Add option for so_reuserport for NetFlow and sFlow #26

Merged
merged 1 commit into from
May 1, 2019
Merged

Add option for so_reuserport for NetFlow and sFlow #26

merged 1 commit into from
May 1, 2019

Conversation

kanocz
Copy link
Contributor

@kanocz kanocz commented Apr 29, 2019

In some cases (for better performance) it's good to have more instances of goflow running on same UDP ports, but locked to some cpu cores. This patch enables optional use of so_reuseport for UDP listen giving this possibility.

@lspgn
Copy link
Contributor

lspgn commented Apr 29, 2019

Hello,
Thank you for this PR.
Regarding multithreading the decoding CPU, you can set a number of workers that the main thread will dispatch for decoding. This handles NetFlow/IPFIX templates.
I am not entirely familiar with SO_REUSEPORT and the multi-platform support. Is it load-balanced/random/... as unless it is hashed, we'd loose the templating functionality.
I do not mind adding this feature, but I'd rather make sure this works on other platforms and would support NetFlow v9/IPFIX if that is not replaceable by setting a number of workers for processing.

@lspgn lspgn self-assigned this Apr 29, 2019
@kanocz
Copy link
Contributor Author

kanocz commented Apr 29, 2019

@lspgn it works on unix/bsd and also on windows, but system implementation is different, so no guaranty for same hashing in multi-platform context. But as for me it's good optional feature for some use cases
and yes for example on Linux when you use more instances which are locked to different cpu cores together with network card queues it can hash much-much better performance (no interconnection between threads, no numa-node issues, memory synchronization and so on)

@lspgn
Copy link
Contributor

lspgn commented Apr 29, 2019

I see: it makes sense.
Have you used it with sFlow only? Do you know anyone else who would be benefitting from this functionality or had any feedbacks?
I will take a look at the new library and merge.
Would you mind taking a look at the v3 of GoFlow? It slightly changes the part with the socket binding.

@kanocz
Copy link
Contributor Author

kanocz commented Apr 30, 2019

Yes, used it only for sFlow now... and some friends from CDN77 also used it in such way :)
Hm... looks like v3 changes many things... if you want I'll prepare same patch for v3 also, but need little bit more time to test if this will get same performance boost and make sense for v3

@lspgn
Copy link
Contributor

lspgn commented Apr 30, 2019

Good to know :)
I will release an update to v2 (among with a few others PRs).
Before releasing v3, I will add yours as well: the the gain of performance should still be present. It's mainly a change in structure (and schema): just so you're aware if you're not using vanilla GoFlow and have flow processors in the pipeline.

Thank you!

@lspgn
Copy link
Contributor

lspgn commented May 1, 2019

Just made a commit in v3, reusing your code. I need to update the structures in order to pass the reuse parameter. Need to test it now:
7752826

@lspgn lspgn merged commit f5dccb5 into cloudflare:master May 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants