Add L2 announcement IPv6 support #39648

msune · 2025-05-20T23:30:36Z

This patchset adds L2 announcement IPv6 support.

Patchset:

23c6929a58 docs/l2_announcements: remove IPv6 limitation
08019f3ef8 pkg: add support for L2 IPv6 announcements
db22a4c2d4 pkg/datapath: garp->gneigh and implement ND adv.
234e1b4a76 pkg: add v6 L2 responder maps
98ee4c6ff3 Add mdlayher/ndp go package
71721fa802 bpf/test: add L2 announce IPv6 unit test
749f591e3f bpf: Add support for IPv6 L2 announcements
aa2dc65eec bpf/test/pktgen: add IPv6 {svc,ext}_one addresses
623417f1c4 bpf: rescope IPv6 MC helpers (sol. addr)

Depends on #39574 and #39579. Related/implements #34983.

Add L2 announcement IPv6 support

github-actions · 2025-06-20T02:11:07Z

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

maintainer-s-little-helper · 2025-06-20T06:55:00Z

Commit 0cc17e9 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

msune · 2025-06-26T17:15:24Z

OK, ready for review. I finally could spent some more time on it.

A couple of notes:

Besides the BPF unit tests, I've manually tested it using kind, both for targeted and non-targeted NDP NS.
Reviewers let me know if there is some more test coverage you think it's necessary, given the state of coverage for the v4.
So, with the current implementation (in main for v4), if for whatever reason the GARP can't be sent, the map is not populated with that entry, so it will never make it into the map unless there are changes. See this. Should we warn and continue? I didn't change this behaviour as part of the PR just yet.

@dylandreimerink I would appreciate if you could give it a run too.

As part of cilium#39648 review, a bug was disovered in which l2_responder counters were 0ed during reconciliation. Fix it by making sure desiredMap contains old entries with satisfied=true so that counters are not 0ed, as per suggested by Dylan. Reported-by: Joe Stringer <joe@cilium.io> Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com> Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

msune · 2025-07-10T12:36:06Z

@joestringer many thanks for the thorough review 🙏

Nice work!

Thx

A bunch of my feedback is around little things like extra unneeded parameters, some ineffective statements, some ways we could simplify the code (eg avoid casts). There's some comments that reference the wrong structures or files that don't exist. All nice-to-have things but not critical; see below for details.

All either addressed or not applicable.

The most notable feedback below is around how single-stack is handled, since it didn't look like pkg/datapath/gneigh/processor.go is aware of which protocols are enabled. Seemed like this might cause errors to show up in agent logs for protocols that are disabled. But maybe I just missed some detail about how the disabled protocols are skipped over.

I've explained in the thread, I think we are good. I think we would have seen this already, as the L2 announce tests in E2E (indirect testing) only enable IPv4. But I could be wrong.

I raised some other lesser questions like whether we really need another map or could reuse the same map for both address families, and also some ideas about simplifying / reusing Go code. See below for the threads. I think these are worth considering, but I'd be fine with merging this PR and then iterating on top.

In agreement, but I think this is further work 😉

joestringer

LGTM for my code owners. I followed up on one point from my prior review here: https://github.com/cilium/cilium/pull/39648/files#r2198302693 , but I think this is also OK to fix up after merging this.

tommyp1ckles

Just some minor stuff, but the changes look good to me.

bpf/tests/pktgen.h

go.mod

pkg/maps/l2v6respondermap/l2_responder_map6.go

pkg/maps/l2v6respondermap/l2_responder_map6_fake.go

pkg/maps/l2v6respondermap/l2_responder_map6.go

pkg/datapath/l2responder/l2responder.go

MC helpers are only really used for Solicitation addresses, therefore make narrow the scope to make them more useful. Adapted tests. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

Define v6_svc_one address required by the V6 l2_announce test. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

This patch adds supports in bpf_host for IPv6 L2 announcements. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

This commit adds tc_l2_announce6 unit test, to cover IPv6 support for L2 announce. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

This commit adds mdlayher/ndp golang package to implement support for sending Gratuitous Neighbour packets. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

This commit adds v6 L2 responder map. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

This commit: * Renames garp cell to gneigh to be more generic. * Adds support to gneigh for sending gratuitous Neighbour Advertisement messages. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

This commit adds support for IPv6 L2 announcements by adding the necessary logic in the l2responder/ cell using the gratuitous ND functionality now part of the gneigh cell. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

On l2-announcements.rst: * Remove IPv6 limitation. * Adapt text to include NDP where only ARP was mentioned. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

Prior to this commit if a gratuitous ARP/NDP was unable to be sent, the L2 announce map (v4/v6) was not populated with the entry, leading to an inconsistency. This was not a big deal as _generally_ nothing should prevent us to send these packets. Change the behaviour to Warn and continue. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

As part of cilium#39648 review, a bug was disovered in which l2_responder counters were 0ed during reconciliation. Fix it by making sure desiredMap contains old entries with satisfied=true so that counters are not 0ed, as per suggested by Dylan. Reported-by: Joe Stringer <joe@cilium.io> Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com> Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

msune · 2025-07-11T08:32:09Z

Rebased. All comments addressed, so I believe we are ready to merge 🚀

dylandreimerink · 2025-07-11T11:09:55Z

/test

muhlba91 · 2025-07-29T07:57:17Z

qq: will this be in the 1.18 release already?

msune · 2025-07-29T07:59:29Z

qq: will this be in the 1.18 release already?

Nope, 1.19.

RonaldPhilipsen · 2025-08-03T08:19:01Z

qq: will this be in the 1.18 release already?

Nope, 1.19.

Sad, I was really looking forward to enabling matter without relying on metallb

Either way, thanks for all the effort!

As part of cilium#39648 review, a bug was disovered in which l2_responder counters were 0ed during reconciliation. Fix it by making sure desiredMap contains old entries with satisfied=true so that counters are not 0ed, as per suggested by Dylan. Reported-by: Joe Stringer <joe@cilium.io> Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com> Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 20, 2025

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label May 20, 2025

This was referenced May 22, 2025

L2-announcements lack ipv6-support #28985

Closed

bpf: add IPv6 mcast addr helpers and cleanup #39579

Merged

github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jun 20, 2025

msune force-pushed the l2_announce_ipv6_final branch from d7ce7b8 to 0cc17e9 Compare June 20, 2025 06:54

maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 20, 2025

msune force-pushed the l2_announce_ipv6_final branch from 0cc17e9 to 0c4682f Compare June 20, 2025 07:04

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 20, 2025

msune force-pushed the l2_announce_ipv6_final branch 4 times, most recently from ed8bee9 to 636f517 Compare June 20, 2025 08:04

joestringer added release-note/major This PR introduces major new functionality to Cilium. dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs labels Jun 20, 2025

maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jun 20, 2025

github-actions bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jun 21, 2025

msune force-pushed the l2_announce_ipv6_final branch from 636f517 to 23c6929 Compare June 26, 2025 16:56

msune marked this pull request as ready for review June 26, 2025 17:15

msune requested review from a team as code owners June 26, 2025 17:15

msune requested review from HadrienPatte and tommyp1ckles June 26, 2025 17:15

msune force-pushed the l2_announce_ipv6_final branch from 1b26c19 to ed4bcc3 Compare July 10, 2025 09:48

msune force-pushed the l2_announce_ipv6_final branch from 61237f6 to dc141e7 Compare July 10, 2025 12:30

joestringer approved these changes Jul 10, 2025

View reviewed changes

tommyp1ckles approved these changes Jul 11, 2025

View reviewed changes

msune added 11 commits July 11, 2025 10:05

bpf: rescope IPv6 MC helpers (sol. addr)

5f38ad2

MC helpers are only really used for Solicitation addresses, therefore make narrow the scope to make them more useful. Adapted tests. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

bpf/test/pktgen: add v6_svc_one address

b233106

Define v6_svc_one address required by the V6 l2_announce test. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

bpf: Add support for IPv6 L2 announcements

35152c6

This patch adds supports in bpf_host for IPv6 L2 announcements. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

bpf/test: add L2 announce IPv6 unit test

59bc3c8

This commit adds tc_l2_announce6 unit test, to cover IPv6 support for L2 announce. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

Add mdlayher/ndp go package

41777ba

This commit adds mdlayher/ndp golang package to implement support for sending Gratuitous Neighbour packets. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

pkg: add v6 L2 responder map

eec9882

This commit adds v6 L2 responder map. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

pkg/datapath: garp->gneigh and implement ND adv.

4638fc4

This commit: * Renames garp cell to gneigh to be more generic. * Adds support to gneigh for sending gratuitous Neighbour Advertisement messages. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

pkg: add support for L2 IPv6 announcements

4015e0b

This commit adds support for IPv6 L2 announcements by adding the necessary logic in the l2responder/ cell using the gratuitous ND functionality now part of the gneigh cell. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

docs/l2_announcements: remove IPv6 limitation

ccd55e7

On l2-announcements.rst: * Remove IPv6 limitation. * Adapt text to include NDP where only ARP was mentioned. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

msune force-pushed the l2_announce_ipv6_final branch from dc141e7 to 671b7ee Compare July 11, 2025 08:23

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 Jul 11, 2025

dylandreimerink added this pull request to the merge queue Jul 11, 2025

Merged via the queue into cilium:main with commit 236f0cd Jul 11, 2025
69 of 70 checks passed

msune deleted the l2_announce_ipv6_final branch July 11, 2025 14:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add L2 announcement IPv6 support #39648

Add L2 announcement IPv6 support #39648

Uh oh!

msune commented May 20, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jun 20, 2025

Uh oh!

maintainer-s-little-helper bot commented Jun 20, 2025

Uh oh!

msune commented Jun 26, 2025

Uh oh!

msune commented Jul 10, 2025

Uh oh!

joestringer left a comment

Uh oh!

tommyp1ckles left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msune commented Jul 11, 2025

Uh oh!

dylandreimerink commented Jul 11, 2025

Uh oh!

Uh oh!

muhlba91 commented Jul 29, 2025

Uh oh!

msune commented Jul 29, 2025

Uh oh!

RonaldPhilipsen commented Aug 3, 2025

Uh oh!

Uh oh!

Add L2 announcement IPv6 support #39648

Add L2 announcement IPv6 support #39648

Uh oh!

Conversation

msune commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 20, 2025

Uh oh!

maintainer-s-little-helper bot commented Jun 20, 2025

Uh oh!

msune commented Jun 26, 2025

Uh oh!

msune commented Jul 10, 2025

Uh oh!

joestringer left a comment

Choose a reason for hiding this comment

Uh oh!

tommyp1ckles left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msune commented Jul 11, 2025

Uh oh!

dylandreimerink commented Jul 11, 2025

Uh oh!

Uh oh!

muhlba91 commented Jul 29, 2025

Uh oh!

msune commented Jul 29, 2025

Uh oh!

RonaldPhilipsen commented Aug 3, 2025

Uh oh!

Uh oh!

msune commented May 20, 2025 •

edited

Loading