bpf: support for adding DSCP for egress traffic by mazdakn · Pull Request #10881 · projectcalico/calico

mazdakn · 2025-08-25T23:27:25Z

Description

Add support for setting DSCP for egress traffic leaving cluster.
This is the follow up on the main PRs:

Workload endpoints: Set DSCP for workload's egress traffic leaving cluster #10718
Host endpoints: Set DSCP for host endpoints #10825

The follow up PR includes a change to only set DSCP for traffic leaving cluster.

Related issues/PRs

Todos

Tests
Documentation
Release note

Release Note

TBD

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

docs-pr-required: This change requires a change to the documentation that has not been completed yet.
docs-completed: This change has all necessary documentation completed.
docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

release-note-required: This PR has user-facing changes. Most PRs should have this label.
release-note-not-required: This PR has no user-facing changes.

Other optional labels:

cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

felix/bpf/ut/qos_test.go

tomastigera · 2025-09-04T18:11:41Z

felix/bpf-gpl/qos.h

+	// The 2 least significant bits are assigned to ECN and must not be touched.
+	ip_hdr(ctx)->tos = (__u8) ((ip_hdr(ctx)->tos & 0x03) | (dscp << 2));
+
+	__wsum ip_csum = bpf_csum_diff(0, 0, (__u32 *)ctx->ip_header, sizeof(struct iphdr), 0);


I think you should set csum to 0 if you want to compute the csum of the entire ip header the checksum is computed as the 16-bit ones' complement of the ones' complement sum of all 16-bit words in the header. This includes the Header Checksum field itself, which is set to zero during computation.

tomastigera

nothing fundamental, mostly nits

tomastigera · 2025-09-04T18:21:40Z

felix/bpf-gpl/qos.h

+	if (EGRESS_DSCP < 0) {
+		return TC_ACT_UNSPEC;
+	}


Would it make sense to take it out from this function? Could -1 be technically a valid value? (guess not since all valid values are 6bits only). Perhaps a flag that this field is set?

Adding a flag to check if this value is set is probably an overkill. we can check EGRESS_DSCP >= 0 in the caller part.

tomastigera · 2025-09-04T18:24:03Z

felix/bpf/ut/bpf_prog_test.go


+func withEgressDSCP(value int8) testOption {
+	return func(o *testOpts) {
+		o.dscp = &value


if -1 is invalid, you could have used -1 as invalid as well instead of using a pointer

I added it this way only to consider the default value of type. Since default value of int, 0, is a valid DSCP value.
I managed to find the initialization part, so now type is int.

tomastigera · 2025-09-04T18:26:19Z

felix/dataplane/linux/bpf_ep_mgr.go

 		return nil, err
 	}

+	ap.DSCP = -1


tomastigera · 2025-09-04T18:26:33Z

felix/dataplane/linux/bpf_ep_mgr.go

 		}
 	}

+	ap.DSCP = -1


you set the dscp to -1 but then you deny packets if it is set to -1 🤔

It wasn't dropping packets, but clearly the return value was confusing. Changed the return val to bool.

felix/bpf-gpl/qos.h

tomastigera · 2025-09-04T18:33:59Z

felix/bpf-gpl/tc.c

 		deny_reason(ctx, CALI_REASON_DROPPED_BY_QOS);
 		goto deny;
 	}
+	if (set_dscp(ctx) == TC_ACT_SHOT) {


Suggested change

if (set_dscp(ctx) == TC_ACT_SHOT) {

if ((CALI_F_FROM_WEP || CALI_F_TO_HEP) && EGRESS_DSCP >= 0 && !set_dscp(ctx)) {

you could make the set_dscp return bool. I'd suggest name it qos_set_dscp will suggest to rename the rate functions to qos_* too

Copilot

Pull Request Overview

This PR implements DSCP (Differentiated Services Code Point) support for BPF dataplane in Calico, extending the existing iptables/nftables DSCP functionality to the eBPF implementation. The changes enable setting DSCP values for egress traffic leaving the cluster when using the BPF dataplane.

Key Changes

Removed BPF dataplane skip conditions from DSCP tests to enable testing with BPF
Added BPF-specific DSCP implementation in the C code with IPv4/IPv6 support
Extended BPF program configuration to include DSCP parameters

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
felix/fv/dscp_test.go	Enables DSCP tests for BPF dataplane and conditionally skips iptables-specific verification
felix/fv/bpf_test.go	Removes obsolete BPFIPv6Support function that returned false
felix/dataplane/linux/bpf_ep_mgr.go	Adds DSCP configuration logic for workload and host endpoints
felix/bpf/ut/qos_test.go	Adds comprehensive unit tests for DSCP functionality in both IPv4 and IPv6
felix/bpf/ut/bpf_prog_test.go	Extends test framework with DSCP configuration support
felix/bpf/tc/attach.go	Adds DSCP field to AttachPoint struct and configuration
felix/bpf/libbpf/libbpf_common.go	Adds DSCP field to TcGlobalData struct
felix/bpf/libbpf/libbpf_api.h	Updates C API to include DSCP parameter
felix/bpf/libbpf/libbpf.go	Passes DSCP value to C implementation
felix/bpf-gpl/tc.c	Integrates DSCP setting into packet processing flow
felix/bpf-gpl/qos.h	Implements DSCP setting logic for IPv4 and IPv6 packets
felix/bpf-gpl/globals.h	Adds DSCP field to global data structure
felix/bpf-gpl/bpf.h	Defines EGRESS_DSCP macro for accessing DSCP configuration

felix/bpf-gpl/qos.h

tomastigera

LGTM ... it would be great in the follow up PR to add a FV test that would set the DSCP. There are tests with external client (mainly for nodeports) and that could verify various scenarios. And we can use tcpdump to verify that the value is set when expected.

mazdakn · 2025-09-09T15:52:58Z

@tomastigera we have some FV for this:

calico/felix/fv/dscp_test.go

Line 193 in bd40575

It("applying DSCP annotation should result is adding correct rules", func() {

But I'm going to add more for a couple of more scenarios.

first commit

fc5a8db

Copilot AI review requested due to automatic review settings August 25, 2025 23:27

mazdakn requested a review from a team as a code owner August 25, 2025 23:27

marvin-tigera added this to the Calico v3.31.0 milestone Aug 25, 2025

marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Aug 25, 2025

This comment was marked as outdated.

Sign in to view

mazdakn added 6 commits August 26, 2025 15:02

more

926b653

Delete felix/bpf-gpl/tags

ba2b53c

more

0e52b7d

more

ddad8b4

Merge remote-tracking branch 'open-source/master' into dscp-bpf

4aeeff0

update FV

838e285

mazdakn changed the title ~~[WIP] ebpf: support for adding DSCP for egress traffic~~ bpf: support for adding DSCP for egress traffic Aug 29, 2025

mazdakn added 2 commits August 29, 2025 14:00

revert infra changs

2999372

more

4bfe4a5

tomastigera reviewed Sep 2, 2025

View reviewed changes

felix/bpf/ut/qos_test.go Outdated Show resolved Hide resolved

mazdakn added 2 commits September 2, 2025 14:47

fix

c7c4092

Merge remote-tracking branch 'open-source/master' into dscp-bpf

4ae7536

mazdakn added docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact and removed release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Sep 2, 2025

mazdakn added 3 commits September 3, 2025 16:01

updates

1a10b80

Merge remote-tracking branch 'open-source/master' into dscp-bpf

153eb51

fix merge

75ac818

mazdakn requested review from Copilot and tomastigera September 3, 2025 23:47

This comment was marked as outdated.

Sign in to view

remove unnecessary manipulation

447c6e1

mazdakn requested a review from Copilot September 4, 2025 00:06

This comment was marked as outdated.

Sign in to view

fix typo

2fe2f17

mazdakn requested a review from Copilot September 4, 2025 00:10

This comment was marked as outdated.

Sign in to view

Merge remote-tracking branch 'origin/master' into HEAD

c40ced5

tomastigera reviewed Sep 4, 2025

View reviewed changes

mazdakn added 3 commits September 5, 2025 13:53

Merge branch 'master' into dscp-bpf

c0148e8

Merge remote-tracking branch 'open-source/master' into dscp-bpf

78242b4

markup

c162827

mazdakn requested review from Copilot and tomastigera September 8, 2025 19:02

Copilot AI reviewed Sep 8, 2025

View reviewed changes

felix/bpf-gpl/qos.h Show resolved Hide resolved

some more

00f4c0c

tomastigera approved these changes Sep 9, 2025

View reviewed changes

mazdakn merged commit bd40575 into projectcalico:master Sep 9, 2025
2 of 3 checks passed

mazdakn deleted the dscp-bpf branch September 9, 2025 15:46

mazdakn mentioned this pull request Sep 9, 2025

BPF: Set DSCP only for traffic leaving cluster #10934

Merged

3 tasks

mazdakn mentioned this pull request Sep 19, 2025

Set DSCP for workload's egress traffic leaving cluster #10718

Merged

3 tasks

	if (set_dscp(ctx) == TC_ACT_SHOT) {
	if ((CALI_F_FROM_WEP \|\| CALI_F_TO_HEP) && EGRESS_DSCP >= 0 && !set_dscp(ctx)) {

Conversation

mazdakn commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues/PRs

Todos

Release Note

Reminder for the reviewer

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomastigera left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Uh oh!

tomastigera left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mazdakn commented Sep 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mazdakn commented Aug 25, 2025 •

edited

Loading