Active connection tracking in ebpf #32584
Conversation
e8f0244
to
beecf87
Compare
This comment was marked as resolved.
This comment was marked as resolved.
c7a9677
to
58f239e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
58f239e
to
6bd2077
Compare
6bd2077
to
66e1d80
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@julianwiedmann would you be able to review this for @cilium/sig-datapath, I'm assigned sig-cli but my review would also count towards datapath - and I'm not familiar enough with this code to provide a thorough review. |
76f4e16
to
27af564
Compare
LRS stands for Load Reporting Service from Envoy. Please see: https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/load_stats/v3/lrs.proto. I am bad at naming stuff, so I am open to suggestions |
|
@AwesomePatrol Does the TODO list in the second commits message represent work that's still to be done? |
|
Based on the TODOs and the missing description in the second patch, this looks like it still needs some more work. Flipping to draft for now to get it off my queue, please flip back when you're ready! 🙏 |
27af564
to
b97e055
Compare
Yes and no. TODOs describe work that will be done in PR that starts exposing these counters as Prometheus metrics (not this one).
I removed TODOs from the commit message as to not confuse any other reviewers. What descriptions are missing? I migrated to cell/hive and renamed |
323a58c
to
36a789c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
I'm guessing Julian means the commit descriptions. They should describe what the commit is doing, but more importantly why and any subtleties to help reviewers understand what is going on. See examples in the |
36a789c
to
4571040
Compare
|
/test |
This is needed to break a cyclic dependency on LB{4,6}_map from lb.h in
lrs.h (next patch) imported by conntrack.h.
Signed-off-by: Aleksander Mistewicz <amistewicz@google.com>
Add new map - LB_ACT_MAP - behind ENABLE_ACTIVE_CONNECTION_TRACKING flag
with counters of opened and closed connections. Behavior of eBPF remains
completely unchanged when ENABLE_ACTIVE_CONNECTION_TRACKING flag is not
set.
When an entry, to conntrack table (with dir == CT_SERVICE) is created,
an entry in LB_ACT_MAP.opened is incremented by one. The same occurs
when a connection is reopened. When connection is closed (action ==
ACTION_CLOSE and dir == CT_SERVICE), the related LB_ACT_MAP.closed is
incremented by one.
LB_ACT_MAP is keyed by svc_id (also known as rev_nat_index) and zone.
Backend ID is used to fetch the backend definition from
LB{4,6}_BACKEND_MAP which also contains zone field. This field is
populated only when EndpointSlice contains a reference to zone in
FixedZoneMapping (so it is possible to convert between uint8 ID and
string).
Signed-off-by: Aleksander Mistewicz <amistewicz@google.com>
Signed-off-by: Aleksander Mistewicz <amistewicz@google.com>
Signed-off-by: Aleksander Mistewicz <amistewicz@google.com>
Set ACT map size to backends*zones Remove unnecessary methods for ACT key/value Remove ACT key/value's deepcopy code as it is not needed Signed-off-by: Aleksander Mistewicz <amistewicz@google.com>
a0a23e9
to
31d3ec5
Compare
As described in cilium/design-cfps#31, this is part of #31752
Example configuration (in
cilium-config) for KIND cluster:where topology labels were set on the nodes:
Zone mapping can be verified with
cilium map get cilium_lb4_backends_v3:LRS accounting can be verified with
cilium map get cilium_lb_act: