Remove sockops-enable and friends #23606

mohit-marathe · 2023-02-07T11:25:08Z

Signed-off-by: Mohit Marathe mohitmarathe23@gmail.com

Please ensure your pull request adheres to the following guidelines:

For first time contributors, read Submitting a pull request
All code is covered by unit and/or runtime tests where feasible.
All commits contain a well written commit description including a title,
description and a Fixes: #XXX line if the commit addresses a particular
GitHub issue.
If your commit description contains a Fixes: <commit-id> tag, then
please add the commit author[s] as reviewer[s] to this issue.
All commits are signed off. See the section Developer’s Certificate of Origin
Provide a title or release-note blurb suitable for the release notes.
Thanks for contributing!

Fixes: #23556

Deleted the files mentioned in the issue. Additionally I deleted all occurrences of sockops

bpf/Makefile

tklauser · 2023-02-07T12:04:34Z

The build failure looks related to this PR: https://app.travis-ci.com/github/cilium/cilium/builds/260413218

I think you need the sock_key forward declaration from bpf_alignchecker.c as well:

cilium/bpf/bpf_alignchecker.c

Line 38 in a73044d

struct sock_key _16;

mohit-marathe · 2023-02-08T03:31:20Z

@tklauser, sorry I don't understand, should I remove struct sock_key _16 from bpf_alignchecker.c?

aspsk

A few cleanups and one question

Makefile

bpf/lib/maps.h

Documentation/network/ebpf/intro.rst

Makefile

mohit-marathe · 2023-04-03T15:51:10Z

I accidentally synced my branch with master, so I'm going to reset some commits.

qmonnet · 2023-04-03T16:45:50Z

/test

Job 'Cilium-PR-K8s-1.25-kernel-4.19' failed:

Click to show.

Test Name

K8sDatapathConfig Host firewall With VXLAN and endpoint routes

Failure Output

FAIL: Error deleting resource /home/jenkins/workspace/Cilium-PR-K8s-1.25-kernel-4.19/src/github.com/cilium/cilium/test/k8s/manifests/host-policies.yaml: Cannot retrieve "cilium-jckm2"'s policy revision: cannot get policy revision: ""

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.25-kernel-4.19 so I can create one.

.github/workflows/tests-datapath-verifier.yaml

qmonnet · 2023-04-04T08:24:06Z

So sorry, we've got conflicts to address again 😞

mohit-marathe · 2023-04-04T08:37:39Z

@qmonnet ✌️Not a problem!

qmonnet · 2023-04-04T09:00:44Z

/test

tklauser

It looks like something went wrong in pkg/defaults/defaults.go on rebase.

pkg/defaults/defaults.go

bpf/node_config.h

This removes every occurrences of sockops and sockmap as it was deprecated in v1.13. Fixes: cilium#23556 Signed-off-by: Mohit Marathe <mohitmarathe23@gmail.com>

qmonnet · 2023-04-04T10:11:29Z

/test

tklauser

Thanks

qmonnet

All seems in order this time.

We're still waiting for the last few tests to complete, but everything is green so far, with one exception for BPF complexity tests. This is expected, and we validated that these tests pass on #24507.

Pending the last tests complete successfully, this is all good for me.

Thanks a lot!

aspsk · 2023-04-04T13:01:06Z

ConformanceGKE triggered this flake: #22368

qmonnet · 2023-04-04T14:42:11Z

OK the timeout on ConformanceGKE is a known issue, and this workflow is not marked as required.
The comments from Timo have been addressed (and he's off this week), we're just missing a review from @cilium/endpoint (@tommyp1ckles, you validated once already but before the PR touched files owned by that team, would you mind taking another quick look please?)

qmonnet · 2023-04-04T14:54:14Z

@mohit-marathe Thanks a lot for this work and for being super reactive all along!! 🚀
If you feel like contributing again in the future, just know that we don't usually get so many rebases to do. 😃

mohit-marathe · 2023-04-04T16:19:40Z

@qmonnet Thanks! I learned a lot by working on this issue(like rebase) 😃 . I'm still learning about networking and ebpf. I hope to contribute and learn a lot from the community.
Cheers ✌️

mohit-marathe requested review from a team as code owners February 7, 2023 11:25

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 7, 2023

mohit-marathe requested review from tklauser, ti-mo, aspsk, tommyp1ckles, sayboras and qmonnet February 7, 2023 11:25

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Feb 7, 2023

tklauser reviewed Feb 7, 2023

View reviewed changes

bpf/Makefile Show resolved Hide resolved

tklauser added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Feb 7, 2023

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 7, 2023

tklauser added area/sockops dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Feb 7, 2023

maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Feb 7, 2023

tklauser added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Feb 7, 2023

maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Feb 7, 2023

aspsk requested changes Apr 3, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

bpf/lib/maps.h Show resolved Hide resolved

Documentation/network/ebpf/intro.rst Show resolved Hide resolved

aspsk reviewed Apr 3, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

mohit-marathe force-pushed the sockops-deletion branch 2 times, most recently from 713a4d2 to 4c423f3 Compare April 3, 2023 15:59

qmonnet reviewed Apr 3, 2023

View reviewed changes

.github/workflows/tests-datapath-verifier.yaml Outdated Show resolved Hide resolved

mohit-marathe force-pushed the sockops-deletion branch from 4c423f3 to fd3c22e Compare April 3, 2023 19:28

mohit-marathe force-pushed the sockops-deletion branch from fd3c22e to 197a252 Compare April 4, 2023 08:35

aspsk approved these changes Apr 4, 2023

View reviewed changes

tklauser requested changes Apr 4, 2023

View reviewed changes

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

bpf/node_config.h Show resolved Hide resolved

Remove sockops-enable and friends

ab9efc7

This removes every occurrences of sockops and sockmap as it was deprecated in v1.13. Fixes: cilium#23556 Signed-off-by: Mohit Marathe <mohitmarathe23@gmail.com>

mohit-marathe force-pushed the sockops-deletion branch from 197a252 to ab9efc7 Compare April 4, 2023 10:01

tklauser approved these changes Apr 4, 2023

View reviewed changes

tklauser requested a review from qmonnet April 4, 2023 11:39

qmonnet approved these changes Apr 4, 2023

View reviewed changes

qmonnet removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Apr 4, 2023

aanm approved these changes Apr 4, 2023

View reviewed changes

qmonnet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 4, 2023

qmonnet removed the request for review from ti-mo April 4, 2023 14:47

aanm merged commit 87c8746 into cilium:master Apr 4, 2023
55 of 57 checks passed

qmonnet mentioned this pull request Apr 4, 2023

docs: Update diagram to remove mention of sockops #24735

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove sockops-enable and friends #23606

Remove sockops-enable and friends #23606

mohit-marathe commented Feb 7, 2023

tklauser commented Feb 7, 2023

mohit-marathe commented Feb 8, 2023

aspsk left a comment

mohit-marathe commented Apr 3, 2023

qmonnet commented Apr 3, 2023 •

edited by maintainer-s-little-helper bot

Test Name

Failure Output

qmonnet commented Apr 4, 2023

mohit-marathe commented Apr 4, 2023

qmonnet commented Apr 4, 2023

tklauser left a comment

qmonnet commented Apr 4, 2023

tklauser left a comment

qmonnet left a comment

aspsk commented Apr 4, 2023

qmonnet commented Apr 4, 2023

qmonnet commented Apr 4, 2023

mohit-marathe commented Apr 4, 2023

Remove sockops-enable and friends #23606

Remove sockops-enable and friends #23606

Conversation

mohit-marathe commented Feb 7, 2023

tklauser commented Feb 7, 2023

mohit-marathe commented Feb 8, 2023

aspsk left a comment

Choose a reason for hiding this comment

mohit-marathe commented Apr 3, 2023

qmonnet commented Apr 3, 2023 • edited by maintainer-s-little-helper bot

Test Name

Failure Output

qmonnet commented Apr 4, 2023

mohit-marathe commented Apr 4, 2023

qmonnet commented Apr 4, 2023

tklauser left a comment

Choose a reason for hiding this comment

qmonnet commented Apr 4, 2023

tklauser left a comment

Choose a reason for hiding this comment

qmonnet left a comment

Choose a reason for hiding this comment

aspsk commented Apr 4, 2023

qmonnet commented Apr 4, 2023

qmonnet commented Apr 4, 2023

mohit-marathe commented Apr 4, 2023

qmonnet commented Apr 3, 2023 •

edited by maintainer-s-little-helper bot