NETOBSERV-2005: Support multiple flow filter rules using json fmt string #473

msherif1234 · 2024-12-05T15:20:53Z

Description

Example config and the flow filter map content

oc get pods  -n netobserv-privileged netobserv-ebpf-agent-tksvd -o yaml | grep "FLOW_FILTER_RULES" -A1
    - name: FLOW_FILTER_RULES
      value: '[{"ip_cidr":"1.1.1.1/24","protocol":"TCP","source_port":53,"action":"Accept","tcp_flags":"SYN"},{"ip_cidr":"2.2.2.2/24","protocol":"UDP","source_port":80,"action":"Accept","tcp_flags":"SYN"},{"ip_cidr":"3.3.3.3/24","protocol":"ICMP","action":"Accept"}]'

bpftool map dump id 223 
[{
        "key": {
            "prefix_len": 24,
            "ip_data": [1,1,1,1,0,0,0,0,0,0,0,0,0,0,0,0
            ]
        },
        "value": {
            "protocol": 6,
            "dstPortStart": 0,
            "dstPortEnd": 0,
            "dstPort1": 0,
            "dstPort2": 0,
            "srcPortStart": 53,
            "srcPortEnd": 0,
            "srcPort1": 0,
            "srcPort2": 0,
            "portStart": 0,
            "portEnd": 0,
            "port1": 0,
            "port2": 0,
            "icmpType": 0,
            "icmpCode": 0,
            "direction": "MAX_DIRECTION",
            "action": "ACCEPT",
            "tcpFlags": "SYN_FLAG",
            "filter_drops": 0,
            "ip": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
            ]
        }
    },{
        "key": {
            "prefix_len": 24,
            "ip_data": [2,2,2,2,0,0,0,0,0,0,0,0,0,0,0,0
            ]
        },
        "value": {
            "protocol": 17,
            "dstPortStart": 0,
            "dstPortEnd": 0,
            "dstPort1": 0,
            "dstPort2": 0,
            "srcPortStart": 80,
            "srcPortEnd": 0,
            "srcPort1": 0,
            "srcPort2": 0,
            "portStart": 0,
            "portEnd": 0,
            "port1": 0,
            "port2": 0,
            "icmpType": 0,
            "icmpCode": 0,
            "direction": "MAX_DIRECTION",
            "action": "ACCEPT",
            "tcpFlags": "SYN_FLAG",
            "filter_drops": 0,
            "ip": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
            ]
        }
    },{
        "key": {
            "prefix_len": 24,
            "ip_data": [3,3,3,3,0,0,0,0,0,0,0,0,0,0,0,0
            ]
        },
        "value": {
            "protocol": 1,
            "dstPortStart": 0,
            "dstPortEnd": 0,
            "dstPort1": 0,
            "dstPort2": 0,
            "srcPortStart": 0,
            "srcPortEnd": 0,
            "srcPort1": 0,
            "srcPort2": 0,
            "portStart": 0,
            "portEnd": 0,
            "port1": 0,
            "port2": 0,
            "icmpType": 0,
            "icmpCode": 0,
            "direction": "MAX_DIRECTION",
            "action": "ACCEPT",
            "tcpFlags": 0,
            "filter_drops": 0,
            "ip": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
            ]
        }
    }
]

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.

codecov · 2024-12-05T15:45:32Z

Codecov Report

Attention: Patch coverage is 5.47945% with 69 lines in your changes missing coverage. Please review.

Project coverage is 29.41%. Comparing base (70003f2) to head (24151f9).

Files with missing lines	Patch %	Lines
pkg/agent/agent.go	0.00%	22 Missing ⚠️
pkg/agent/packets_agent.go	0.00%	22 Missing ⚠️
pkg/tracer/flow_filter.go	4.34%	20 Missing and 2 partials ⚠️
pkg/model/flow_content.go	0.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #473      +/-   ##
==========================================
- Coverage   29.68%   29.41%   -0.28%     
==========================================
  Files          51       51              
  Lines        4915     4947      +32     
==========================================
- Hits         1459     1455       -4     
- Misses       3350     3382      +32     
- Partials      106      110       +4

Flag	Coverage Δ
unittests	`29.41% <5.47%> (-0.28%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/agent/config.go	`8.33% <ø> (ø)`
pkg/decode/decode_protobuf.go	`30.11% <100.00%> (+0.61%)`	⬆️
pkg/ebpf/bpf_x86_bpfel.go	`0.00% <ø> (ø)`
pkg/tracer/tracer.go	`0.00% <ø> (ø)`
pkg/model/flow_content.go	`62.02% <0.00%> (-2.45%)`	⬇️
pkg/agent/agent.go	`32.92% <0.00%> (-0.82%)`	⬇️
pkg/agent/packets_agent.go	`0.00% <0.00%> (ø)`
pkg/tracer/flow_filter.go	`45.54% <4.34%> (-2.02%)`	⬇️

... and 2 files with indirect coverage changes

msherif1234 · 2024-12-05T18:51:11Z

/ok-to-test

github-actions · 2024-12-05T18:53:25Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:1ec52cf

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=1ec52cf make set-agent-image

openshift-ci-robot · 2024-12-06T11:21:46Z

stleerh

Minor comments.
/lgtm

pkg/agent/agent.go

pkg/agent/config.go

msherif1234 · 2024-12-07T01:49:43Z

/ok-to-test

github-actions · 2024-12-07T01:51:46Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:34179d9

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=34179d9 make set-agent-image

openshift-ci-robot · 2024-12-07T11:54:38Z

@msherif1234: This pull request references NETOBSERV-2005 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

Description

Example config and the flow filter map content

oc get pods  -n netobserv-privileged netobserv-ebpf-agent-tksvd -o yaml | grep "FLOW_FILTER_RULES" -A1
   - name: FLOW_FILTER_RULES
     value: '[{"ip_cidr":"1.1.1.1/24","protocol":"TCP","source_port":53,"action":"Accept","tcp_flags":"SYN"},{"ip_cidr":"2.2.2.2/24","protocol":"UDP","source_port":80,"action":"Accept","tcp_flags":"SYN"},{"ip_cidr":"3.3.3.3/24","protocol":"ICMP","action":"Accept"}]'

bpftool map dump id 223 
[{
       "key": {
           "prefix_len": 24,
           "ip_data": [1,1,1,1,0,0,0,0,0,0,0,0,0,0,0,0
           ]
       },
       "value": {
           "protocol": 6,
           "dstPortStart": 0,
           "dstPortEnd": 0,
           "dstPort1": 0,
           "dstPort2": 0,
           "srcPortStart": 53,
           "srcPortEnd": 0,
           "srcPort1": 0,
           "srcPort2": 0,
           "portStart": 0,
           "portEnd": 0,
           "port1": 0,
           "port2": 0,
           "icmpType": 0,
           "icmpCode": 0,
           "direction": "MAX_DIRECTION",
           "action": "ACCEPT",
           "tcpFlags": "SYN_FLAG",
           "filter_drops": 0,
           "ip": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
           ]
       }
   },{
       "key": {
           "prefix_len": 24,
           "ip_data": [2,2,2,2,0,0,0,0,0,0,0,0,0,0,0,0
           ]
       },
       "value": {
           "protocol": 17,
           "dstPortStart": 0,
           "dstPortEnd": 0,
           "dstPort1": 0,
           "dstPort2": 0,
           "srcPortStart": 80,
           "srcPortEnd": 0,
           "srcPort1": 0,
           "srcPort2": 0,
           "portStart": 0,
           "portEnd": 0,
           "port1": 0,
           "port2": 0,
           "icmpType": 0,
           "icmpCode": 0,
           "direction": "MAX_DIRECTION",
           "action": "ACCEPT",
           "tcpFlags": "SYN_FLAG",
           "filter_drops": 0,
           "ip": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
           ]
       }
   },{
       "key": {
           "prefix_len": 24,
           "ip_data": [3,3,3,3,0,0,0,0,0,0,0,0,0,0,0,0
           ]
       },
       "value": {
           "protocol": 1,
           "dstPortStart": 0,
           "dstPortEnd": 0,
           "dstPort1": 0,
           "dstPort2": 0,
           "srcPortStart": 0,
           "srcPortEnd": 0,
           "srcPort1": 0,
           "srcPort2": 0,
           "portStart": 0,
           "portEnd": 0,
           "port1": 0,
           "port2": 0,
           "icmpType": 0,
           "icmpCode": 0,
           "direction": "MAX_DIRECTION",
           "action": "ACCEPT",
           "tcpFlags": 0,
           "filter_drops": 0,
           "ip": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
           ]
       }
   }
]

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.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

msherif1234 · 2024-12-10T19:05:46Z

/ok-to-test

github-actions · 2024-12-10T19:07:51Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:6139f89

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=6139f89 make set-agent-image

msherif1234 · 2024-12-12T18:00:38Z

/ok-to-test

github-actions · 2024-12-12T18:02:58Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:141b6ac

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=141b6ac make set-agent-image

msherif1234 · 2024-12-12T18:45:19Z

/ok-to-test

github-actions · 2024-12-12T18:47:17Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:f691cf5

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=f691cf5 make set-agent-image

msherif1234 · 2024-12-12T19:15:57Z

/ok-to-test

github-actions · 2024-12-12T19:18:05Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:22832d2

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=22832d2 make set-agent-image

Signed-off-by: Mohamed Mahmoud <[email protected]>

msherif1234 · 2024-12-13T14:29:49Z

/ok-to-test

github-actions · 2024-12-13T14:34:39Z

New image:
quay.io/netobserv/netobserv-ebpf-agent:405541b

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=405541b make set-agent-image

jpinsonneau

LGTM, thanks @msherif1234 !

msherif1234 · 2024-12-13T15:59:53Z

/approve

openshift-ci · 2024-12-13T15:59:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msherif1234

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [msherif1234]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jotak · 2024-12-16T07:58:16Z

bpf/flows.c

@@ -77,15 +79,7 @@ static inline void update_dns(additional_metrics *extra_metrics, pkt_info *pkt,
 }

 static inline int flow_monitor(struct __sk_buff *skb, u8 direction) {
-    // If sampling is defined, will only parse 1 out of "sampling" flows


we could have optimized this, and if no filter_sampling is defined, run the global sampling check earlier. Keep the late sampling check only when there filter-based sampling defined.
So when user runs with no filter-sampling defined, but just global sampling, it's more efficient and doesn't parse headers.

Running some perf, while user-space metrics look unchanged, using bpftop I do see a consequent increase in CPU in kernel, around +100%, when using global sampling=50 and no filter sampling, compared to main baseline before this PR was merged.
I've put some numbers here: https://docs.google.com/spreadsheets/d/1BzrAXr-XEWjizBf-tFhtHcKMNyQOvvW2HH4zXeoDxE8/edit?gid=0#gid=0

openshift-ci bot added the do-not-merge/work-in-progress label Dec 5, 2024

msherif1234 mentioned this pull request Dec 5, 2024

NETOBSERV-2005: Mutli eBPF flow filter support netobserv/network-observability-operator#927

Merged

10 tasks

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 5, 2024

msherif1234 changed the title ~~WIP: Support multiple flow filter rules using json fmt string~~ Support multiple flow filter rules using json fmt string Dec 5, 2024

openshift-ci bot removed the do-not-merge/work-in-progress label Dec 5, 2024

msherif1234 requested a review from jotak December 5, 2024 20:10

msherif1234 changed the title ~~Support multiple flow filter rules using json fmt string~~ NETOBSERV-2005: Support multiple flow filter rules using json fmt string Dec 6, 2024

openshift-ci-robot added the jira/valid-reference label Dec 6, 2024

msherif1234 mentioned this pull request Dec 6, 2024

NETOBSERV-2005: Allow populaiting only single filter rules for cli usecase netobserv/network-observability-cli#128

Merged

10 tasks

stleerh reviewed Dec 6, 2024

View reviewed changes

pkg/agent/agent.go Outdated Show resolved Hide resolved

pkg/agent/config.go Outdated Show resolved Hide resolved

openshift-ci bot assigned stleerh Dec 6, 2024

openshift-ci bot added the lgtm label Dec 6, 2024

msherif1234 force-pushed the multi_filter branch from d95862c to 3eef923 Compare December 7, 2024 01:49

openshift-ci bot removed the lgtm label Dec 7, 2024

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 7, 2024

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 7, 2024

msherif1234 requested a review from stleerh December 7, 2024 01:49

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 10, 2024

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 10, 2024

msherif1234 force-pushed the multi_filter branch from 2ed36dc to 3a7b53a Compare December 12, 2024 17:49

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 12, 2024

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 12, 2024

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 12, 2024

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 12, 2024

msherif1234 force-pushed the multi_filter branch from aeb0749 to 24151f9 Compare December 12, 2024 19:14

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 12, 2024

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 12, 2024

msherif1234 added 3 commits December 13, 2024 09:22

Support multiple flow filter rules using json fmt string

44b4ad2

Signed-off-by: Mohamed Mahmoud <[email protected]>

Add sampling filter action support

cd873a3

Signed-off-by: Mohamed Mahmoud <[email protected]>

Add the sampled value to flows

5c07db3

Signed-off-by: Mohamed Mahmoud <[email protected]>

msherif1234 force-pushed the multi_filter branch from 24151f9 to 5c07db3 Compare December 13, 2024 14:29

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 13, 2024

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Dec 13, 2024

jpinsonneau approved these changes Dec 13, 2024

View reviewed changes

openshift-ci bot assigned jpinsonneau Dec 13, 2024

openshift-ci bot added the lgtm label Dec 13, 2024

openshift-ci bot added the approved label Dec 13, 2024

msherif1234 merged commit 7e7f8c4 into netobserv:main Dec 13, 2024
8 of 10 checks passed

msherif1234 deleted the multi_filter branch December 13, 2024 17:00

jotak reviewed Dec 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NETOBSERV-2005: Support multiple flow filter rules using json fmt string #473

NETOBSERV-2005: Support multiple flow filter rules using json fmt string #473

msherif1234 commented Dec 5, 2024 •

edited

Loading

codecov bot commented Dec 5, 2024 •

edited

Loading

msherif1234 commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

openshift-ci-robot commented Dec 6, 2024 •

edited by openshift-ci bot

Loading

Description

Dependencies

Checklist

stleerh left a comment

msherif1234 commented Dec 7, 2024

github-actions bot commented Dec 7, 2024

openshift-ci-robot commented Dec 7, 2024 •

edited by openshift-ci bot

Loading

Description

Dependencies

Checklist

msherif1234 commented Dec 10, 2024

github-actions bot commented Dec 10, 2024

msherif1234 commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

msherif1234 commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

msherif1234 commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

msherif1234 commented Dec 13, 2024

github-actions bot commented Dec 13, 2024

jpinsonneau left a comment

msherif1234 commented Dec 13, 2024

openshift-ci bot commented Dec 13, 2024

jotak Dec 16, 2024 •

edited

Loading

jotak Dec 16, 2024 •

edited

Loading

NETOBSERV-2005: Support multiple flow filter rules using json fmt string #473

NETOBSERV-2005: Support multiple flow filter rules using json fmt string #473

Conversation

msherif1234 commented Dec 5, 2024 • edited Loading

Description

Dependencies

Checklist

codecov bot commented Dec 5, 2024 • edited Loading

Codecov Report

msherif1234 commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

openshift-ci-robot commented Dec 6, 2024 • edited by openshift-ci bot Loading

Description

Dependencies

Checklist

stleerh left a comment

Choose a reason for hiding this comment

msherif1234 commented Dec 7, 2024

github-actions bot commented Dec 7, 2024

openshift-ci-robot commented Dec 7, 2024 • edited by openshift-ci bot Loading

Description

Dependencies

Checklist

msherif1234 commented Dec 10, 2024

github-actions bot commented Dec 10, 2024

msherif1234 commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

msherif1234 commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

msherif1234 commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

msherif1234 commented Dec 13, 2024

github-actions bot commented Dec 13, 2024

jpinsonneau left a comment

Choose a reason for hiding this comment

msherif1234 commented Dec 13, 2024

openshift-ci bot commented Dec 13, 2024

jotak Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

jotak Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

msherif1234 commented Dec 5, 2024 •

edited

Loading

codecov bot commented Dec 5, 2024 •

edited

Loading

openshift-ci-robot commented Dec 6, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Dec 7, 2024 •

edited by openshift-ci bot

Loading

jotak Dec 16, 2024 •

edited

Loading

jotak Dec 16, 2024 •

edited

Loading