Skip to content

Conversation

lmb
Copy link
Contributor

@lmb lmb commented May 24, 2024

loader: remove datapathSHA256

datapathSHA256 was added in commit a530ac0b70 ("loader: Hash datapath 
objects and store results"). It is fed into the endpoint / template hash to
ensure that templates are recompiled when bundled source code changes.

Since at least 48591d8f42 ("loader: simplify template cache invalidation")
the endpoint program cache doesn't reuse results from an older / different
cilium process. This means that we're always using the correct source code
by construction. Remove datapathSHA256 and related machinery.

In the process of refactoring it turns out that we swallow errors from 
WriteNodeConfig in hashDatapath, which has so far obscured failing tests. 
The comment that writing to an in-memory hash is correct, but misleading. 
WriteNodeConfig also returns errors when certain devices are missing.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

@lmb lmb added area/loader Impacts the loading of BPF programs into the kernel. release-note/misc This PR makes changes that have no direct user impact. labels May 24, 2024
@lmb
Copy link
Contributor Author

lmb commented May 24, 2024

/test

@lmb lmb force-pushed the pr/lmb/rm-datapath-sha branch from 603c37e to 251bb6a Compare May 24, 2024 10:49
@lmb
Copy link
Contributor Author

lmb commented May 24, 2024

/test

@lmb lmb force-pushed the pr/lmb/rm-datapath-sha branch from 251bb6a to 1dd8dc7 Compare May 28, 2024 11:07
@lmb
Copy link
Contributor Author

lmb commented May 28, 2024

/test

@lmb lmb force-pushed the pr/lmb/rm-datapath-sha branch from 1dd8dc7 to 9511290 Compare May 28, 2024 11:26
@lmb
Copy link
Contributor Author

lmb commented May 28, 2024

/test

@lmb lmb force-pushed the pr/lmb/rm-datapath-sha branch from 9511290 to 62d0bc0 Compare May 28, 2024 14:40
@lmb
Copy link
Contributor Author

lmb commented May 28, 2024

/test

@lmb lmb force-pushed the pr/lmb/rm-datapath-sha branch from 62d0bc0 to a619137 Compare May 28, 2024 14:55
@lmb
Copy link
Contributor Author

lmb commented May 28, 2024

/test

@lmb lmb force-pushed the pr/lmb/rm-datapath-sha branch from a619137 to 00e0786 Compare May 28, 2024 15:48
@lmb
Copy link
Contributor Author

lmb commented May 28, 2024

/test

@lmb lmb marked this pull request as ready for review May 28, 2024 15:51
@lmb lmb requested review from a team as code owners May 28, 2024 15:51
@lmb lmb requested review from dylandreimerink and borkmann May 28, 2024 15:51
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 4, 2024
@julianwiedmann julianwiedmann added dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jun 4, 2024
@lmb lmb force-pushed the pr/lmb/rm-datapath-sha branch from 00e0786 to a337a7d Compare June 7, 2024 09:26
@lmb
Copy link
Contributor Author

lmb commented Jun 7, 2024

/test

@lmb lmb removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 7, 2024
datapathSHA256 was added in commit a530ac0 ("loader: Hash datapath
objects and store results"). It is fed into the endpoint / template
hash to ensure that templates are recompiled when bundled source
code changes.

Since at least 48591d8 ("loader: simplify template cache
invalidation") the endpoint program cache doesn't reuse results from
an older / different cilium process. This means that we're always
using the correct source code by construction.
Remove datapathSHA256 and related machinery.

In the process of refactoring it turns out that we swallow errors from
WriteNodeConfig in hashDatapath, which has so far obscured failing tests.
The comment that writing to an in-memory hash is correct, but misleading.
WriteNodeConfig also returns errors when certain devices are missing.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
@lmb lmb force-pushed the pr/lmb/rm-datapath-sha branch from a337a7d to d2d08d0 Compare June 7, 2024 09:57
@lmb
Copy link
Contributor Author

lmb commented Jun 7, 2024

/test

@lmb lmb enabled auto-merge June 7, 2024 10:31
@lmb lmb added this pull request to the merge queue Jun 7, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 7, 2024
Merged via the queue into cilium:main with commit f056579 Jun 7, 2024
@lmb lmb deleted the pr/lmb/rm-datapath-sha branch June 7, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/loader Impacts the loading of BPF programs into the kernel. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants