Simplifying observed interfaces - moving it to main map #509
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
bpf/flows.c
Outdated
| bpf_printk("error creating new observed_intf: %d\n", ret); | ||
| } | ||
| } | ||
| add_observed_intf(aggregate_flow, skb->ifindex, direction); |
There was a problem hiding this comment.
make interface update part of update_existing_flow()
There was a problem hiding this comment.
done, but I still take the lock in add_observed_intf for 2 reasons:
- we can't call a function while having a lock (so for instance the call to
increase_counterisn't allowed) - we can't read map value while having the lock, so all the for-loop to check for existing intf+direction isn't allowed during locking
In other words we need to keep lock as short time as possible
Observed interfaces is a structure managed from the TC hook, so it is elligible for being written in the main map instead of the additional metrics map. Moreover, it is very frequently created (the vast majority of flows being observed from at least two interfaces), so having it in the additional maps sort of defeats one of its purpose, that was allowing to reduce the memory used by separating commonly flow data from more uncommon data. In other words, when this is set into the addtional map, the size map becomes similar to main map, whereas if removed the size map can be shrinked to a much lower number.
jotak
force-pushed
the
move-obsintf-map
branch
from
93b677d to
0921905
Compare
jotak
added
the
ok-to-test
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=6970084 make set-agent-image |
... and ignore direction for duplicate interface checks (tradeoff to minimize loss of data in case of max reached) Also some padding optimisation
github-actions
bot
removed
the
ok-to-test
Observed interfaces is a structure managed from the TC hook, so it is elligible for being written in the main map instead of the additional metrics map. Moreover, it is very frequently created (the vast majority of flows being observed from at least two interfaces), so having it in the additional maps sort of defeats one of its purpose, that was allowing to reduce the memory used by separating commonly flow data from more uncommon data.
In other words, when this is set into the addtional map, the size map becomes similar to main map, whereas if removed the size map can be shrinked to a much lower number.
Also:
Description
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.