Skip to content
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-934: Add support for SCTP, ICMPv4/v6 protocols to ebpf agent #103

Merged
merged 7 commits into from
Mar 20, 2023
Merged

NETOBSERV-934: Add support for SCTP, ICMPv4/v6 protocols to ebpf agent #103

merged 7 commits into from
Mar 20, 2023

Conversation

msherif1234
Copy link
Contributor

@msherif1234 msherif1234 commented Mar 15, 2023

Extend L4 protocols support to include

  • SCTP
  • ICMPv4
  • ICMPv6

Note: SCTP structure is not exported hence I am using local version of it in the ebpf code I checked with RHEL team and they said this probably fine because its very unlikely this structure will ever change and there is no plans to exported either.

unit-testing

ipv4: 07:58:14.541222 enp1s0 IP 192.168.122.69:0 > 192.168.122.1:0: protocol:icmp type: 8 code: 0 dir:1 bytes:490 packets:5 flags:0 ends: 07:58:18.637230
ipv4: 07:58:14.541512 enp1s0 IP 192.168.122.1:0 > 192.168.122.69:0: protocol:icmp type: 0 code: 0 dir:0 bytes:490 packets:5 flags:0 ends: 07:58:18.637502

@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Merging #103 (55d9dd3) into main (8eb86a9) will decrease coverage by 0.29%.
The diff coverage is 23.52%.

@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
- Coverage   41.88%   41.59%   -0.29%     
==========================================
  Files          30       30              
  Lines        2008     2041      +33     
==========================================
+ Hits          841      849       +8     
- Misses       1129     1154      +25     
  Partials       38       38              
Flag Coverage Δ
unittests 41.59% <23.52%> (-0.29%) ⬇️

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

Impacted Files Coverage Δ
pkg/ebpf/tracer.go 0.00% <0.00%> (ø)
pkg/exporter/ipfix.go 0.00% <0.00%> (ø)
pkg/exporter/proto.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@msherif1234 msherif1234 changed the title WIP: Add support for SCTP, ICMP amd ICMPv6 protocols to ebpf code Add support for SCTP, ICMP amd ICMPv6 protocols to ebpf code Mar 16, 2023
go.mod Outdated Show resolved Hide resolved
@msherif1234 msherif1234 requested a review from jotak March 17, 2023 13:13
@msherif1234
Copy link
Contributor Author

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 19, 2023
@github-actions
Copy link

New image: ["quay.io/netobserv/netobserv-ebpf-agent:e6854dd"]. It will expire after two weeks.

bpf/flows.c Show resolved Hide resolved
praveingk
praveingk previously approved these changes Mar 20, 2023
Copy link
Collaborator

@praveingk praveingk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍🏽

@openshift-ci openshift-ci bot added the lgtm label Mar 20, 2023
@openshift-ci openshift-ci bot removed the lgtm label Mar 20, 2023
@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 20, 2023
@msherif1234 msherif1234 changed the title Add support for SCTP, ICMP amd ICMPv6 protocols to ebpf code NETOBSERV-934: Add support for SCTP, ICMP amd ICMPv6 protocols to ebpf code Mar 20, 2023
@msherif1234
Copy link
Contributor Author

/ok-to-test

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 20, 2023

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

In response to this:

Extend L4 protocols support to include

  • SCTP
  • ICMPv4
  • ICMPv6

Note: SCTP structure is not exported hence I am using local version of it in the ebpf code I checked with RHEL team and they said this probably fine because its very unlikely this structure will ever change and there is no plans to exported either.

unit-testing

ipv4: 07:58:14.541222 enp1s0 IP 192.168.122.69:0 > 192.168.122.1:0: protocol:icmp type: 8 code: 0 dir:1 bytes:490 packets:5 flags:0 ends: 07:58:18.637230
ipv4: 07:58:14.541512 enp1s0 IP 192.168.122.1:0 > 192.168.122.69:0: protocol:icmp type: 0 code: 0 dir:0 bytes:490 packets:5 flags:0 ends: 07:58:18.637502

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 kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 20, 2023
@github-actions
Copy link

New image: ["quay.io/netobserv/netobserv-ebpf-agent:6dbf3dd"]. It will expire after two weeks.

@msherif1234 msherif1234 changed the title NETOBSERV-934: Add support for SCTP, ICMP amd ICMPv6 protocols to ebpf code NETOBSERV-934: Add support for SCTP, ICMPv4/v6 protocols to ebpf code Mar 20, 2023
@msherif1234 msherif1234 changed the title NETOBSERV-934: Add support for SCTP, ICMPv4/v6 protocols to ebpf code NETOBSERV-934: Add support for SCTP, ICMPv4/v6 protocols to ebpf agent Mar 20, 2023
bpf/flows.c Outdated Show resolved Hide resolved
Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 20, 2023
@msherif1234
Copy link
Contributor Author

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 20, 2023
@github-actions
Copy link

New image: ["quay.io/netobserv/netobserv-ebpf-agent:4c509fc"]. It will expire after two weeks.

@openshift-ci openshift-ci bot added the lgtm label Mar 20, 2023
@jotak
Copy link
Member

jotak commented Mar 20, 2023

/lgtm
/approve
thanks @msherif1234 !

@openshift-ci
Copy link

openshift-ci bot commented Mar 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jotak

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit c62173a into netobserv:main Mar 20, 2023
shach33 pushed a commit to praveingk/netobserv-ebpf-agent that referenced this pull request Apr 6, 2023
netobserv#103)

* Add support for SCTP, ICMP amd ICMPv6 protocols to ebpf code

Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>

* update GRPC protobuf definition to include icmp fields

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add ICMP and ICMPv6 ipfix support

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add ICMPv4/6 ebpf agent support

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Update unit-test cases

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add verifier error check to catch JIT errors

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* update flowlogs dump collector tool to include ICMP

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

---------

Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
shach33 pushed a commit to praveingk/netobserv-ebpf-agent that referenced this pull request Apr 6, 2023
netobserv#103)

* Add support for SCTP, ICMP amd ICMPv6 protocols to ebpf code

Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>

* update GRPC protobuf definition to include icmp fields

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add ICMP and ICMPv6 ipfix support

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add ICMPv4/6 ebpf agent support

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Update unit-test cases

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add verifier error check to catch JIT errors

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* update flowlogs dump collector tool to include ICMP

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

---------

Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
dushyantbehl pushed a commit to dushyantbehl/netobserv-ebpf-agent that referenced this pull request Apr 28, 2023
netobserv#103)

* Add support for SCTP, ICMP amd ICMPv6 protocols to ebpf code

Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>

* update GRPC protobuf definition to include icmp fields

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add ICMP and ICMPv6 ipfix support

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add ICMPv4/6 ebpf agent support

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Update unit-test cases

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* Add verifier error check to catch JIT errors

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

* update flowlogs dump collector tool to include ICMP

Signed-off-by: msherif1234 <mmahmoud@redhat.com>

---------

Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>
Signed-off-by: msherif1234 <mmahmoud@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved jira/valid-reference lgtm ok-to-test To set manually when a PR is safe to test. Triggers image build on PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants