Optimize netdb Services database #304
Conversation
mariomac
changed the title
mariomac
added
the
ok-to-test
|
New image: ["quay.io/netobserv/flowlogs-pipeline:9621604"]. It will expire after two weeks. |
| # /etc/protocols: | ||
| # $Id: protocols,v 1.12 2016/07/08 12:27 ovasik Exp $ |
There was a problem hiding this comment.
As noted in go help test, I think testdata is more idiomatic name for the fixtures directory.
The go tool will ignore a directory named "testdata", making it available
to hold ancillary data needed by the tests.
There was a problem hiding this comment.
Good point. Didn't know it. Thanks!
| // verify it also finds service name by protocol alias | ||
| assert.Equal(t, "netbios-dgm", db.ByPortAndProtocolName(138, "TCP")) | ||
|
|
||
| // verify multiple ports can be associated to the same protocol |
There was a problem hiding this comment.
I don't understand the comment
// verify multiple ports can be associated to the same protocol
The cases below all have the same port but different protocols. In contrast to the comment that states multiple ports and the same protocol.
There was a problem hiding this comment.
yeah I guess that's a typo in comment, @mariomac ? Should be multiple protocols to same port ?
| protos, err := os.Open("/etc/protocols") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("opening /etc/protocols: %w", err) | ||
| } | ||
| services, err := os.Open("/etc/services") |
There was a problem hiding this comment.
Shouldn't these os.Open() be followed by defer file.Close() as done in the tests?
| protos, err := os.Open("/etc/protocols") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("opening /etc/protocols: %w", err) | ||
| } | ||
| services, err := os.Open("/etc/services") |
There was a problem hiding this comment.
In the old implementation, the paths to the protocols file and the services file were configurable (jsonNetworkTransform.ProtocolsFile and jsonNetworkTransform.ServicesFile). Don't we want to keep them configurable (rather than hardcoded)?
github-actions
bot
removed
the
ok-to-test
Codecov Report
@@ Coverage Diff @@
## main #304 +/- ##
==========================================
+ Coverage 68.67% 68.73% +0.05%
==========================================
Files 81 81
Lines 4699 4679 -20
==========================================
- Hits 3227 3216 -11
+ Misses 1276 1269 -7
+ Partials 196 194 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
4 participants
The previous version of the
netdb.gofile required to iterate across of tenths of thousands (worst case) of services and protocols on each query.This version preloads all the services and protocols information as maps for a O(1) lookup on each flow information.
Benchmarks comparing new vs old version:
To reproduce all the benchmarks, you should checkout commit
5ac2502081c1a47101f758030b988466cbf554ec, as the old code is removed in the head commit.