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

docs: Update diagram to remove mention of sockops #24735

Closed
qmonnet opened this issue Apr 4, 2023 · 3 comments · Fixed by #24824
Closed

docs: Update diagram to remove mention of sockops #24735

qmonnet opened this issue Apr 4, 2023 · 3 comments · Fixed by #24824
Assignees
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.

Comments

@qmonnet
Copy link
Member

qmonnet commented Apr 4, 2023

Sockops and friends have been removed in #23606. Anton reported that they're still referenced in a picture in the docs. We want to update that picture to remove these outdated references.

Pictures (`cilium/Documentation/network/ebpf/_static/cilium_bpf_*.svg`) will still refer to sockmap et al.

Originally posted by @aspsk in #23606 (comment)

@qmonnet qmonnet added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. labels Apr 4, 2023
@joestringer
Copy link
Member

We might consider switching these to something like mermaid instead, to make them easier to update over time.

@zacharysarah
Copy link
Contributor

@joestringer 👋🏻

We might consider switching these to something like mermaid instead, to make them easier to update over time.

I'd love to implement Mermaid, if not in this stack then in a future iteration.

As it is, the first I had to take in #24824 was to prettify the SVG enough to be human readable.

@qmonnet
Copy link
Member Author

qmonnet commented Apr 12, 2023

For the record, the Sphinx version we use is recent enough to support Mermaid. We haven't enabled the add-on because nobody has really needed it so far, as far as I know.

Someone reported two months ago that there was the matter of non-HTML formats to address (but does anyone use something else than the plain text files and the HTML version?), and possibly some subtleties to troubleshoot:

For the html docs, adding the mermaid entry to certain pages worked, while on others not (I honestly failed to spot the difference in the source, but I didn't spend much time on that). On those not working, the mermaid js was not loaded at all. When adding the line below in conf.py to load the js file, it worked on all pages, but still it is not that small as a file (~1MB):

html_js_files = ["https://cdnjs.cloudflare.com/ajax/libs/mermaid/9.3.0/mermaid.min.js"] 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants