-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NETOBSERV-1478: Add eBPF Agent filtering capability #307
NETOBSERV-1478: Add eBPF Agent filtering capability #307
Conversation
@msherif1234: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #307 +/- ##
==========================================
+ Coverage 33.63% 33.91% +0.27%
==========================================
Files 47 48 +1
Lines 3865 4043 +178
==========================================
+ Hits 1300 1371 +71
- Misses 2477 2575 +98
- Partials 88 97 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@msherif1234: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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. |
dd718b8
to
1c1a598
Compare
@msherif1234: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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. |
1c1a598
to
40d2fe5
Compare
@msherif1234: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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. |
0babff1
to
f447aba
Compare
@msherif1234: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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. |
f447aba
to
4aaa74d
Compare
@msherif1234: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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. |
aa89a6a
to
47a0ed8
Compare
@msherif1234: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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. |
/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=e235a56 make set-agent-image |
47a0ed8
to
6659399
Compare
7976549
to
afcc0c9
Compare
filterFlows = defineMetric( | ||
"filtered_flows_total", | ||
"Number of filtered flows", | ||
TypeCounter, | ||
"source", | ||
"reason", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update the dashboards ?
Maybe we can manage that in netobserv/network-observability-operator#607 or create a followup in Jira
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if we need to update the dashboard or we just add it to the query list not sure @jotak but that is correct we can either added to 607 PR on a following PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do a follow-up JIRA then; I would say the goal for NETOBSERV-1478 is reached already
For users who enable filtering, I guess it can be interesting to know the ratio of filtered in/filtered out flows indeed, so yeah why not adding that to the health dashboard
bpf/flows_filter.h
Outdated
if (rule->icmpCode != 0) { | ||
if (rule->icmpCode == id->icmp_code) { | ||
BPF_PRINTK("icmpCode matched\n"); | ||
result++; | ||
} else { | ||
result = 0; | ||
goto end; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it makes sense to have icmpCode
independent of icmpType
so we should move that check inside of if (rule->icmpType != 0)
condition above and document that icmpCode
requires icmpType
to be set.
WDYT ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are two independent filters so u set one or the other or both or none
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but each type have its own related code implementation:
https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml
so it doesn't make any sense to have icmpCode
filter only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see so either type + code or type only I will fix it
/lgtm |
Signed-off-by: Mohamed Mahmoud <[email protected]>
Signed-off-by: Mohamed Mahmoud <[email protected]>
Signed-off-by: Mohamed Mahmoud <[email protected]>
Signed-off-by: Mohamed Mahmoud <[email protected]>
… metric Signed-off-by: Mohamed Mahmoud <[email protected]>
aff6995
to
28bce79
Compare
/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=bd5c6e0 make set-agent-image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @msherif1234
/label qe-approved |
@msherif1234: This pull request references NETOBSERV-1478 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.16.0" version, but no target version was set. In response to this:
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. |
/approve |
[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:
Approvers can indicate their approval by writing |
Description
This PR allow provisioning global filter to limit what goes to eBPF flows table
this also fix #266
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.