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

Enhancement: Enable IPsec support in OVNKubernetes #473

Merged
merged 1 commit into from
Feb 12, 2021
Merged

Enhancement: Enable IPsec support in OVNKubernetes #473

merged 1 commit into from
Feb 12, 2021

Conversation

markdgray
Copy link
Contributor

Signed-off-by: Mark Gray mark.d.gray@redhat.com

@markdgray
Copy link
Contributor Author

/assign @stbenjam

russellb
russellb requested changes Sep 11, 2020
View reviewed changes
@markdgray
Copy link
Contributor Author

FYI: @mcurry-rh @knobunc @fepan

danwinship
danwinship reviewed Sep 11, 2020
View reviewed changes
JAORMX
JAORMX reviewed Sep 14, 2020
View reviewed changes
@markdgray markdgray changed the title Enhancement: Enable IPsec support in OVNKubernetes [WIP] Enhancement: Enable IPsec support in OVNKubernetes Sep 23, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 23, 2020
@mccv1r0
Copy link

mccv1r0 commented Oct 8, 2020

We don't want to rule out the possibility of non OVN interfaces used by multus also being able to use IPsec concurrently. Will there be separate copies of e.g. ipsec.conf or will some sort of serialization be needed when accessing the same files?

@mccv1r0
Copy link

mccv1r0 commented Oct 8, 2020

Will HW offload be a requirement now or in the future? With shared-gateway, when eth0 is inside br-int, will HW offload of IPsec even be possible?

@jackevans43
Copy link

@mccv1r0 I would say now. For small webapps etc software IPsec is fine, but for anything serious it'll be a killer - easily take a 10Gbps link down into the hundreds,

@markdgray
Copy link
Contributor Author

We don't want to rule out the possibility of non OVN interfaces used by multus also being able to use IPsec concurrently. Will there be separate copies of e.g. ipsec.conf or will some sort of serialization be needed when accessing the same files?

You can bind libreswan to a specific interface and have multiple "ipsec.conf" files so shouldn't be a problem

Will HW offload be a requirement now or in the future? With shared-gateway, when eth0 is inside br-int, will HW offload of IPsec even be possible?

I'd have to look to check how the shared gateway code works but it depends at what point in the IP stack OVS is sending the packets. Either way, I will test this. Thanks

@markdgray markdgray closed this Oct 28, 2020
@markdgray markdgray changed the title [WIP] Enhancement: Enable IPsec support in OVNKubernetes Enhancement: Enable IPsec support in OVNKubernetes Oct 28, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 28, 2020
@markdgray
Copy link
Contributor Author

Accidently closed this.

@markdgray markdgray mentioned this pull request Oct 28, 2020
Closed
@markdgray markdgray reopened this Oct 30, 2020
@markdgray markdgray force-pushed the ovn-kubernetes-ipsec branch from 1436017 to 369d8ac Compare October 30, 2020 14:31
@markdgray
Copy link
Contributor Author

With @russellb 's help, I reopened this.

@markdgray markdgray requested a review from russellb December 4, 2020 18:28
@markdgray
Copy link
Contributor Author

@russellb You had changes requested on this. I reworked everything a while ago. Is it ok?

@markdgray markdgray force-pushed the ovn-kubernetes-ipsec branch from 369d8ac to 8805b3f Compare December 8, 2020 15:16
@dcbw
Copy link

dcbw commented Dec 8, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 8, 2020
@markdgray
Copy link
Contributor Author

Can this be merged please?

@russellb
Copy link
Member

/approve
/lgtm

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: russellb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 11, 2021
@russellb
Copy link
Member

/retest

1 similar comment
@markdgray
Copy link
Contributor Author

/retest

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@russellb
Copy link
Member

/lgtm remove

@russellb russellb removed the lgtm Indicates that a PR is ready to be merged. label Feb 11, 2021
@russellb
Copy link
Member

@markdgray you'll have to fix the lint job

@markdgray
Enhancement: Enable IPsec support in OVNKubernetes
a182add 
Signed-off-by: Mark Gray <mark.d.gray@redhat.com>
@markdgray markdgray force-pushed the ovn-kubernetes-ipsec branch from 8805b3f to a182add Compare February 12, 2021 08:27
@markdgray
Copy link
Contributor Author

Thanks @russellb. Done.

@russellb
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 12, 2021
@openshift-merge-robot openshift-merge-robot merged commit ed05107 into openshift:master Feb 12, 2021
@markdgray
Copy link
Contributor Author

Hurray!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lack lack lack left review comments

@danwinship danwinship danwinship left review comments

@JAORMX JAORMX JAORMX left review comments

@jboxman jboxman jboxman left review comments

@squeed squeed squeed left review comments

@mrogers950 mrogers950 mrogers950 left review comments

@aojea aojea aojea left review comments

@dcbw dcbw dcbw left review comments

@mccv1r0 mccv1r0 mccv1r0 left review comments

@jackevans43 jackevans43 jackevans43 left review comments

@dustymabe dustymabe Awaiting requested review from dustymabe

@jsafrane jsafrane Awaiting requested review from jsafrane

@russellb russellb Awaiting requested review from russellb

Assignees

@russellb russellb

@stbenjam stbenjam

@dcbw dcbw

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.