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

WIP: NETOBSERV-1245: fix TCP DNS query #169

Closed
wants to merge 1 commit into from

Conversation

msherif1234
Copy link
Contributor

@msherif1234 msherif1234 commented Aug 14, 2023

there is an additional 1 byte for length in the DNS header when use TCP which they don't exists with UDP, this PR harden DNS hook but won't fix the TCP payload issue because DNS headers aren't in the 1st segment and for that we need to have a new helper that read the following skb segment to get DNS hdr that won't be done in the current release

  • DNS over TCP
    image (9)

  • DNS over UDP
    image (10)

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Aug 14, 2023

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

In response to this:

there is an additional 2 bytes of length in the DNS header when use TCP which they don't exists with UDP

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
Copy link

openshift-ci bot commented Aug 14, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from msherif1234. For more information see the Kubernetes Code Review Process.

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

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Aug 14, 2023

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

In response to this:

there is an additional 2 bytes of length in the DNS header when use TCP which they don't exists with UDP

  • DNS over TCP
    image (9)

  • DNS over UDP
    image (10)

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.

@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 Aug 14, 2023
@github-actions
Copy link

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

It will expire after two weeks.

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

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

@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 Aug 14, 2023
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (244fe07) 31.71% compared to head (504c909) 31.79%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
+ Coverage   31.71%   31.79%   +0.08%     
==========================================
  Files          37       37              
  Lines        3371     3362       -9     
==========================================
  Hits         1069     1069              
+ Misses       2239     2230       -9     
  Partials       63       63              
Flag Coverage Δ
unittests 31.79% <0.00%> (+0.08%) ⬆️

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

Files Coverage Δ
pkg/ebpf/tracer.go 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@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 Aug 14, 2023
@github-actions
Copy link

New image:
quay.io/netobserv/netobserv-ebpf-agent:103b248

It will expire after two weeks.

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

USER=netobserv VERSION=103b248 make set-agent-image

@msherif1234 msherif1234 changed the title NETOBSERV-1245: fix TCP DNS query WIP: NETOBSERV-1245: fix TCP DNS query Aug 14, 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 Aug 15, 2023
@msherif1234 msherif1234 changed the title WIP: NETOBSERV-1245: fix TCP DNS query NETOBSERV-1245: fix TCP DNS query Aug 15, 2023
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Aug 15, 2023

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

In response to this:

there is an additional 2 bytes of length in the DNS header when use TCP which they don't exists with UDP, this PR harden DNS hook but won't fix the TCP payload issue because DNS headers aren't in the 1st segment and for that we need to have a new helper that read the following skb segment to get DNS hdr that won't be done in the current release

  • DNS over TCP
    image (9)

  • DNS over UDP
    image (10)

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.

@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 Aug 15, 2023
@github-actions
Copy link

New image:
quay.io/netobserv/netobserv-ebpf-agent:64552c0

It will expire after two weeks.

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

USER=netobserv VERSION=64552c0 make set-agent-image

@msherif1234 msherif1234 changed the title NETOBSERV-1245: fix TCP DNS query WIP: NETOBSERV-1245: fix TCP DNS query Aug 15, 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 Oct 5, 2023
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Oct 5, 2023

@msherif1234: This pull request references NETOBSERV-1245 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.15.0" version, but no target version was set.

In response to this:

there is an additional 2 bytes of length in the DNS header when use TCP which they don't exists with UDP, this PR harden DNS hook but won't fix the TCP payload issue because DNS headers aren't in the 1st segment and for that we need to have a new helper that read the following skb segment to get DNS hdr that won't be done in the current release

  • DNS over TCP
    image (9)

  • DNS over UDP
    image (10)

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-robot
Copy link
Collaborator

openshift-ci-robot commented Oct 5, 2023

@msherif1234: This pull request references NETOBSERV-1245 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.15.0" version, but no target version was set.

In response to this:

there is an additional 2 bytes of length in the DNS header when use TCP which they don't exists with UDP, this PR harden DNS hook but won't fix the TCP payload issue because DNS headers aren't in the 1st segment and for that we need to have a new helper that read the following skb segment to get DNS hdr that won't be done in the current release

  • DNS over TCP
    image (9)

  • DNS over UDP
    image (10)

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-robot
Copy link
Collaborator

openshift-ci-robot commented Oct 6, 2023

@msherif1234: This pull request references NETOBSERV-1245 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.15.0" version, but no target version was set.

In response to this:

there is an additional 1 byte for length in the DNS header when use TCP which they don't exists with UDP, this PR harden DNS hook but won't fix the TCP payload issue because DNS headers aren't in the 1st segment and for that we need to have a new helper that read the following skb segment to get DNS hdr that won't be done in the current release

  • DNS over TCP
    image (9)

  • DNS over UDP
    image (10)

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.

@msherif1234 msherif1234 force-pushed the fix_tcp_dns branch 2 times, most recently from e3ebd56 to bb0033d Compare October 9, 2023 20:57
@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 Oct 9, 2023
@github-actions
Copy link

github-actions bot commented Oct 9, 2023

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

It will expire after two weeks.

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

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

@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 Oct 10, 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 Oct 11, 2023
@github-actions
Copy link

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

It will expire after two weeks.

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

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

@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 Oct 11, 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 Oct 11, 2023
@github-actions
Copy link

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

It will expire after two weeks.

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

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

@msherif1234
Copy link
Contributor Author

/ok-to-test

@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 Oct 11, 2023
@msherif1234 msherif1234 added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Oct 11, 2023
@github-actions
Copy link

New image:
quay.io/netobserv/netobserv-ebpf-agent:4adfa32

It will expire after two weeks.

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

USER=netobserv VERSION=4adfa32 make set-agent-image

@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 Oct 11, 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 Oct 11, 2023
@github-actions
Copy link

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

It will expire after two weeks.

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress jira/valid-reference 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.

3 participants