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.

Sign up for GitHub

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

Jump to bottom

Fix IPsec for IPv6 overlays #3155

Merged
merged 1 commit into from Mar 18, 2022
Merged

Fix IPsec for IPv6 overlays #3155

merged 1 commit into from Mar 18, 2022

Conversation

antoninbas
Copy link
Contributor

When using IPv6, the IPsec configuration (ipsec.conf) generated by
ovs-monitor-ipsec for strongSwan is currently not correct. A patch has
been submitted upstream, but until it is accepted and merged, we apply a
temporary version of the patch.

This was tested for a VXLAN overlay in an IPv6-only cluster.

Fixes #3151

Signed-off-by: Antonin Bas abas@vmware.com

@antoninbas antoninbas added the area/transit/encryption Issues or PRs related to transit encryption (IPSec, SSL). label Dec 21, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 21, 2021 •
edited

Codecov Report

Merging #3155 (62598ee) into main (4db3c18) will decrease coverage by 10.33%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3155       +/-   ##
===========================================
- Coverage   65.32%   54.99%   -10.34%     
===========================================
  Files         268      374      +106     
  Lines       26903    51533    +24630     
===========================================
+ Hits        17574    28339    +10765     
- Misses       7417    20747    +13330     
- Partials     1912     2447      +535
Flag Coverage Δ
e2e-tests 53.56% <ø> (?)
integration-tests 35.83% <ø> (?)
kind-e2e-tests 55.86% <ø> (+0.13%) ⬆️
unit-tests 42.44% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/cniserver/pod_configuration_linux.go 26.31% <0.00%> (-40.36%) ⬇️
pkg/controller/ipam/antrea_ipam_controller.go 48.71% <0.00%> (-31.57%) ⬇️
pkg/controller/networkpolicy/endpoint_querier.go 61.46% <0.00%> (-29.97%) ⬇️
pkg/controller/egress/controller.go 62.19% <0.00%> (-26.26%) ⬇️
pkg/controller/ipam/validate.go 57.95% <0.00%> (-24.31%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam.go 55.55% <0.00%> (-23.62%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam_controller.go 55.93% <0.00%> (-23.59%) ⬇️
pkg/agent/util/iptables/lock.go 60.00% <0.00%> (-21.82%) ⬇️
pkg/controller/egress/store/egressgroup.go 37.93% <0.00%> (-21.80%) ⬇️
pkg/controller/externalippool/validate.go 55.17% <0.00%> (-21.02%) ⬇️
... and 335 more

luolanzone
luolanzone reviewed Dec 21, 2021
View reviewed changes
build/images/ovs/apply-patches.sh Outdated
# This patch is necessary to ensure that ovs-monitor-ipsec generates a correct IPsec configuration
# for strongSwan when using IPv6.
# TODO: Switch to official patch once it is merged upstream.
curl https://github.com/antoninbas/ovs/commit/126ec7700080427bf109604d9a49bcca2c74f77d.patch | \
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also check the OVS version here for this patch?

Copy link
Contributor

Choose a reason for hiding this comment

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

one question which is not related with this PR, I am wondering which one is the correct IPsec or IPSec? I saw both in our documents or codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • I think this patch can be applied to all OVS versions currently supported by this script
  • I believe the most "correct" one is IPsec, and we should try to standardize on this one

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, I will check and update IPSec to IPsec in a separate PR. I am thinking maybe we can standardize some abbrs words and common review rules and document them in our contributing.md, any suggestion? below are two main things I noticed:

  1. Make sure all K8s related words start with upper case.
  2. some abbrs. like IPsec Antrea etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me

jianjuns
jianjuns previously approved these changes Dec 21, 2021
View reviewed changes
Copy link
Contributor

@jianjuns jianjuns 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 the fix!

luolanzone
luolanzone previously approved these changes Dec 22, 2021
View reviewed changes
@antoninbas antoninbas added this to the Antrea v1.6 release milestone Jan 26, 2022
@antoninbas
Copy link
Contributor Author

Update on this: my OVS patch has been merged upstream and backported to the 2.15 release train (openvswitch/ovs@e59194b). I am tempted to drop this PR, wait until OVS 2.15.4 is released and then update the OVS version in Antrea to 2.15.4. I will make a decision before the Antrea v1.6 release freeze.

@antoninbas
Fix IPsec for IPv6 overlays
62598ee 
When using IPv6, the IPsec configuration (ipsec.conf) generated by
ovs-monitor-ipsec for strongSwan is currently not correct. A patch has
been submitted upstream, but until it is accepted and merged, we apply a
temporary version of the patch.

This was tested for a VXLAN overlay in an IPv6-only cluster.

Fixes antrea-io#3151

Signed-off-by: Antonin Bas <abas@vmware.com>
@antoninbas antoninbas dismissed stale reviews from luolanzone and jianjuns via 62598ee March 17, 2022 19:33
@antoninbas
Copy link
Contributor Author

No OVS 2.15.4 release yet, so I'm merging this for v1.6

@antoninbas
Copy link
Contributor Author

/test-all
/test-ipv6-only-all
/test-ipv6-all

@antoninbas antoninbas added the action/release-note Indicates a PR that should be included in release notes. label Mar 17, 2022
@antoninbas
Copy link
Contributor Author

/test-e2e
/test-ipv6-only-e2e

@antoninbas
Copy link
Contributor Author

/test-ipv6-only-e2e

@antoninbas
Copy link
Contributor Author

I forgot that jenkins-ipv6-only was broken because of #3437. Merging this.

@antoninbas antoninbas merged commit 3c2bfe0 into antrea-io:main Mar 18, 2022
@antoninbas antoninbas deleted the fix-ipsec-for-ipv6-overlays branch March 18, 2022 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnqn tnqn tnqn approved these changes

@jianjuns jianjuns Awaiting requested review from jianjuns

@luolanzone luolanzone Awaiting requested review from luolanzone

Assignees
No one assigned
Labels
action/release-note Indicates a PR that should be included in release notes. area/transit/encryption Issues or PRs related to transit encryption (IPSec, SSL).
Projects
None yet
Milestone
Antrea v1.6 release
Development

Successfully merging this pull request may close these issues.

IPsec mode not working with IPv6 overlays
5 participants
@antoninbas @codecov-commenter @tnqn @jianjuns @luolanzone