blixt icon indicating copy to clipboard operation
blixt copied to clipboard

Implement checksums properly in dataplane

Open shaneutt opened this issue 3 years ago • 13 comments

Problem Statement

We currently don't handle checksums properly or at all in several places. The purpose of this task is to get that sorted out.

Acceptance Criteria

  • [ ] find all the places we are currently handling, or NOT handling L3 and L4 checksums and get them consistent
  • [ ] use helpers like bpf_l3_cksum_replace to properly recalculate checksums

shaneutt avatar Nov 14 '22 15:11 shaneutt

Opening this up as a good issue for newcomers interested in getting involved in the ebpf dataplane bits :)

astoycos avatar Dec 06 '22 19:12 astoycos

Would like to work on this one o/

LCaparelli avatar Dec 07 '22 18:12 LCaparelli

@LCaparelli I would recommend joining the AYA discord If you're going to dive in here -> https://aya-rs.dev/community/

I already asked how to fix this bug here https://discord.com/channels/855676609003651072/1039654899408982077

astoycos avatar Dec 08 '22 14:12 astoycos

Yup, the nice folks over there already gave me a hand on the other issue I'm working on to improve the build process, thanks for the heads up @astoycos

LCaparelli avatar Dec 08 '22 20:12 LCaparelli

@LCaparelli as I understand it you may not have time for this one going forward so I'm opening it back up, however if you still want to take this one on please just let us know! :vulcan_salute:

shaneutt avatar May 03 '23 14:05 shaneutt

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Jan 30 '24 06:01 k8s-triage-robot

Not stale, we still intend this for the v0.7.0 milestone:

/remove-lifecycle stale

However, I suspect @LCaparelli is no longer working on this (do correct me if you still wanted this)? So I think we're back to help wanted.

/help

shaneutt avatar Jan 30 '24 13:01 shaneutt

@shaneutt: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to this:

Not stale, we still intend this for the v0.7.0 milestone:

/remove-lifecycle stale

However, I suspect @LCaparelli is no longer working on this (do correct me if you still wanted this)? So I think we're back to help wanted.

/help

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.

k8s-ci-robot avatar Jan 30 '24 13:01 k8s-ci-robot

I have some questions about this issue, I found some functions implemented with bpf_l4_cksum_replace and bpf_l3_cksum_replace like set_ipv4_dest_port and set_ipv4_ip_dst in tc_ingress module, so you mean that we should keep consistent in other module like tc_egress and etc. ?

maborosii avatar Sep 05 '24 06:09 maborosii

Consistency for sure, but I also think we just have a lot of hacks in place. I removed the good-first-issue on this one some time ago because basically what we need is someone to deep dive in and suss it all out first and actually figure out exactly what is needed, and then make the patch. So just know that we're happy to support you in this one but it's a bit of a deeper dive!

shaneutt avatar Sep 05 '24 14:09 shaneutt