Domain name filterchecks #1213
Domain name filterchecks #1213
Conversation
mattpag
force-pushed
the
fqdn-filterchecks
branch
4 times, most recently
from
95aae31
to
9a3bb48
Compare
| else if(info.m_timeout < max_refresh_timeout) | ||
| { | ||
| // double the timeout until 320 secs | ||
| info.m_timeout <<= 1; |
There was a problem hiding this comment.
Does this really double the timeout? It looks like m_timeout is set to base_refresh_timeout on line 47. So on every subsequent lookup, it looks like m_timeout is set to base_refresh_timeout and then doubled.
There was a problem hiding this comment.
Thanks for catching this, I broke it during the last refactoring pass. Will fix!
userspace/libsinsp/dns_manager.cpp
Outdated
| } | ||
| else // AF_INET6 | ||
| { | ||
| std::array<uint32_t, 4> v6_ip; |
There was a problem hiding this comment.
There's a semi-standard (in that I use it elsewhere in the ipv6 related code for filterchecks, etc) type ipv6addr that you could use here.
There was a problem hiding this comment.
I was focused on having an object that provide an equality operator to use in dns_info::operator=, and you're right, ipv6addr should be better. Will do!
userspace/libsinsp/dns_manager.cpp
Outdated
| if(is_v6) | ||
| { | ||
| std::array<uint32_t, 4> v6_ip; | ||
| memcpy(v6_ip.data(), addr, 16); |
There was a problem hiding this comment.
I was initially confused because I thought addr was only an ipv6 addr due to the type being uint32_t. Can you make it a void * and cast it based on is_v6? Also, you could pass an address family instead of a bool is_v6. That would make it look a bit more like other libc functions that work on both ipv4 and ipv6 addresses.
There was a problem hiding this comment.
I agree with you, will do
userspace/libsinsp/filterchecks.cpp
Outdated
| } | ||
| } | ||
|
|
||
| break; | ||
| case TYPE_SNET: | ||
| case TYPE_SERVERIP: | ||
| case TYPE_SERVERIP_NAME: |
There was a problem hiding this comment.
I think these cases only share a couple of lines (fetching the fdinfo), but are otherwise unrelated. Do you want to have a separate case for the _NAME ones?
There was a problem hiding this comment.
I was trying to minimize the code duplication (already heavy in this file) but yes, in this case I suppose it's better as you are proposing. Will do.
|
|
||
| if(m_cmpop == CO_IN) | ||
| { | ||
| for (uint16_t i=0; i < m_val_storages.size(); i++) |
There was a problem hiding this comment.
You might want to rely on the base class version of flt_compare here. For the _NAME filterchecks, you should just be able to extract them as a string and then do the normal comparison operators. This is especially important for IN as you can do a set membership test instead of looping over the values and comparing each value.
There was a problem hiding this comment.
The resolution goes from the domain name to the IP, not the other way around, so I have to call sinsp_dns_manager::match() at least once for every domain name in the list.
But, as optimization, after the domain name has been resolved I should be able to do what you're proposing because then sinsp_dns_manager::name_of will work.
I'll do that and then, if needed, we can talk more about it.
There was a problem hiding this comment.
Ah right, I see. The names are used as arguments to the matching function, not the values.
|
@mstemm PTAL |
mattpag
force-pushed
the
fqdn-filterchecks
branch
from
de851b5
to
3c217d5
Compare
|
Rebased to pick up #1214. |
sinsp_dns_manager is in charge of resolving a list of domain names and, using a separate thread, keeping them in sync over time.
This new set of filters allows to match the related IP address with a domain name, and to display a previously matched one.
- Change sinsp_dns_manager:{match,name_of}() signatures to be
more consistent with standard libc address functions
- Separate handling of fd.{c,s}ip.name in
sinsp_filter_check_fd::extract()
mattpag
force-pushed
the
fqdn-filterchecks
branch
from
3c217d5
to
c0fbb82
Compare
|
Rebased again to pick up #1218 too. |
|
Ok the changes look good to me! |
Add support for a new set of filterchecks, in the form of
fd.*ip.name, that allows to match the related IP address with a domain name, and to display a previously matched one.Domain name resolutions are handled in a separate thread spawned the first time the new filterchecks are resolved. It's in charge of keep the resolutions in sync over time.
A new dependency for the Intel TBB library has been added. It's used for its high-performance concurrent maps.
Once the PR is approved, the TBB download URL has to be changed to point under
download.draios.com.