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

add example using Linux TC hook for network flow monitoring #1352

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

smagnani96
Copy link
Contributor

While an example program already exists for the incoming traffic hook with XDP, an example for a TC program was missing.

This example shows how to load an eBPF program that monitors both incoming and outgoing TCP/UDP/IP flows identified with the 5-tuple session identifier (IP addresses, L4 ports, IP protocol).

The statistics are periodically displayed, and the content of the map is erased after a given number of iterations to allow potential new flows to be monitored.

The common header file is updated accordingly, to introduce a few shrunk data structures used during the packet processing logic (useful also for future examples).

Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Thanks for this nice example! I find that examples work best when they are quite narrow in scope: they show how to do one particular thing well. What would you say is that particular thing here?

I see a couple of candidates:

  • Use of TCX
  • Sharing code between XDP and TC
  • Subtleties of LRU maps
  • Batch operations
  • Something else?

My personal preference is to choose one of them and strip the example down to the essentials. For example, rework this to showcase how to use the TCX link for flow monitoring on both ingress and egress. Drop the things that increase the lines of code needed beyond that: no XDP, no map clearing, no batch ops. WDYT?

@smagnani96
Copy link
Contributor Author

Thanks for this nice example! I find that examples work best when they are quite narrow in scope: they show how to do one particular thing well. What would you say is that particular thing here?

I see a couple of candidates:

  • Use of TCX
  • Sharing code between XDP and TC
  • Subtleties of LRU maps
  • Batch operations
  • Something else?

My personal preference is to choose one of them and strip the example down to the essentials. For example, rework this to showcase how to use the TCX link for flow monitoring on both ingress and egress. Drop the things that increase the lines of code needed beyond that: no XDP, no map clearing, no batch ops. WDYT?

Thanks for the feedback, I'm glad to help. I totally agree with you, there are so many things showcased in this example, even though they're all interesting and expand the example section. I'd proceed with:

  1. removing XDP from this example and leaving TXC for both Ingress and Egress
  2. remove the deletion of entries from the map

Let me know if I can go on this road.
I don't know how much informative you'd like the example section to be, so please tell me whether I can create other smaller examples showcasing the removed functionalities or not
And also, if you have ideas/needs for other examples (or even source features), don't hesitate :)

@smagnani96 smagnani96 force-pushed the example/shared_ingress_egress branch 2 times, most recently from acbb091 to 8b7ca9b Compare February 24, 2024 12:21
@smagnani96 smagnani96 changed the title add example using Linux TC hook for Egress and XDP hook for Ingress for Flow monitoring add example using Linux TC hook for network flow monitoring Feb 24, 2024
@smagnani96 smagnani96 force-pushed the example/shared_ingress_egress branch from 8b7ca9b to 5123133 Compare February 28, 2024 16:51
@smagnani96 smagnani96 force-pushed the example/shared_ingress_egress branch from 5123133 to 5e4d783 Compare March 13, 2024 20:20
@smagnani96 smagnani96 marked this pull request as ready for review March 13, 2024 20:23
@smagnani96 smagnani96 requested a review from a team as a code owner March 13, 2024 20:23
@ti-mo
Copy link
Collaborator

ti-mo commented Mar 15, 2024

@S41m0n Thanks for the ping. I think Lorenz was suggesting to pick one of the items in the list to showcase in this PR. Given the focus of this example is tcx, I'd go for that one. All the other bullets can be examples of their own.

I'll convert to a draft for now, please mark as ready to review when done. Thanks!

@ti-mo ti-mo marked this pull request as draft March 15, 2024 07:59
@smagnani96
Copy link
Contributor Author

@S41m0n Thanks for the ping. I think Lorenz was suggesting to pick one of the items in the list to showcase in this PR. Given the focus of this example is tcx, I'd go for that one. All the other bullets can be examples of their own.

I'll convert to a draft for now, please mark as ready to review when done. Thanks!

Hi @ti-mo and thanks for the feedback. If I'm not mistaken, the second commit should have removed everything except the "Use of TCx" from the bullet list. The example now is composed by:

  1. Data plane: attached to TC Ingress and Egress to monitor connections (5-tuple session identifier) and count in/out packets
  2. Control plane: periodically outputs the map's content

If you'd like to keep it even simpler, let me know :)

@smagnani96 smagnani96 force-pushed the example/shared_ingress_egress branch from 5e4d783 to 3abb86c Compare May 6, 2024 13:01
@smagnani96
Copy link
Contributor Author

@S41m0n Thanks for the ping. I think Lorenz was suggesting to pick one of the items in the list to showcase in this PR. Given the focus of this example is tcx, I'd go for that one. All the other bullets can be examples of their own.
I'll convert to a draft for now, please mark as ready to review when done. Thanks!

Hi @ti-mo and thanks for the feedback. If I'm not mistaken, the second commit should have removed everything except the "Use of TCx" from the bullet list. The example now is composed by:

  1. Data plane: attached to TC Ingress and Egress to monitor connections (5-tuple session identifier) and count in/out packets
  2. Control plane: periodically outputs the map's content

If you'd like to keep it even simpler, let me know :)

@ti-mo @lmb With respect to the previous proposal, I think the example is now simpler, dealing with only Ingress/Egress flow monitoring through TC. What are your opinions about this? Thanks in advance for the feedback.

@lmb
Copy link
Collaborator

lmb commented Jun 19, 2024

Hi @S41m0n, sorry for the long delay. I've written up what kind of examples fit into the library here: https://ebpf-go.dev/contributing/new-example/ I think it makes sense to have an example for tcx. Would you be open to make the BPF side even simpler? I think just counting the number of ingress / egress packets would be sufficient.

This example shows how to load an eBPF program that monitors both incoming and outgoing TCP/UDP/IP flows identified with the 5-tuple session identifier (IP addresses, L4 ports, IP protocol).
The statistics are periodically displayed, and the content of the map is erased after a given number of iterations to allow potential new flows to be monitored.
The common header file is updated accordingly, to introduce few shrinked data structures used during the packet processing logic (useful also for future examples).

Signed-off-by: Simone Magnani <simonemagnani.96@gmail.com>
Signed-off-by: Simone Magnani <simonemagnani.96@gmail.com>
@smagnani96 smagnani96 force-pushed the example/shared_ingress_egress branch from 3abb86c to 7058638 Compare June 19, 2024 17:09
@smagnani96
Copy link
Contributor Author

Hi @S41m0n, sorry for the long delay. I've written up what kind of examples fit into the library here: https://ebpf-go.dev/contributing/new-example/ I think it makes sense to have an example for tcx. Would you be open to make the BPF side even simpler? I think just counting the number of ingress / egress packets would be sufficient.

@lmb That's amazing, thanks for the guide! I just simplified the example to only count ingress and egress packets, as you suggested. Let me know :)

@smagnani96 smagnani96 force-pushed the example/shared_ingress_egress branch from 7058638 to af08538 Compare June 19, 2024 17:20
@lmb lmb force-pushed the example/shared_ingress_egress branch from af08538 to 89277a0 Compare June 21, 2024 08:40
@lmb lmb marked this pull request as ready for review June 21, 2024 08:41
Signed-off-by: Simone Magnani <simonemagnani.96@gmail.com>
@lmb lmb force-pushed the example/shared_ingress_egress branch from 89277a0 to 5ebeeeb Compare June 21, 2024 08:44
@lmb lmb merged commit c20a334 into cilium:main Jun 21, 2024
17 checks passed
@lmb
Copy link
Collaborator

lmb commented Jun 21, 2024

Thanks again!

@smagnani96 smagnani96 deleted the example/shared_ingress_egress branch July 3, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants