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.

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

feat: IPIP tunnel + IPSec tunnel protection support #1638

Merged
merged 7 commits into from
Mar 9, 2020

Conversation

rastislavs
Copy link
Contributor

VPP 20.01 is deprecating the old IPSec API, in favor of IPIP tunnels with IPSec tunnel protection. This PR adds support for those, which as well fixes the issue with resync of IPSec tunnels on VPP 20.01 which always ended up in a broken state.

The old IPSec tunnel type is marked as deprecated in Ligato API, but still works (but the tunnels are always (correctly) re-programmed during resync).

This PR introduces a type change in the existing IPSec NB API (IPsec SA and SPD indexes), where numeric types were modeled as strings. Since that API was rarely used up till now (there was no need for them when using the old IPSec tunnel type), but will be needed going forward, it is the best time to clean that up now.

Signed-off-by: Rastislav Szabo <raszabo@cisco.com>
Signed-off-by: Rastislav Szabo <raszabo@cisco.com>
Signed-off-by: Rastislav Szabo <raszabo@cisco.com>
@rastislavs rastislavs changed the title Ipsec feat: IPIP tunnel + IPSec tunnel protection support Mar 6, 2020
@codecov
Copy link

codecov bot commented Mar 6, 2020

Codecov Report

Merging #1638 into master will increase coverage by 1.09%.
The diff coverage is 55.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1638      +/-   ##
==========================================
+ Coverage   57.93%   59.03%   +1.09%     
==========================================
  Files         490      288     -202     
  Lines       39850    23337   -16513     
==========================================
- Hits        23089    13778    -9311     
+ Misses      14353     8441    -5912     
+ Partials     2408     1118    -1290     
Flag Coverage Δ
#e2e ?
#unittests 59.03% <55.85%> (-0.03%) ⬇️

@ondrej-fabry
Copy link
Member

Breaking changes to proto:

proto/ligato/vpp/ipsec/ipsec.proto:36:5:Field "1" on message "SecurityPolicyDatabase" changed type from "string" to "uint32".
proto/ligato/vpp/ipsec/ipsec.proto:44:9:Field "1" on message "PolicyEntry" changed type from "string" to "uint32".
proto/ligato/vpp/ipsec/ipsec.proto:74:5:Field "1" on message "SecurityAssociation" changed type from "string" to "uint32".

https://github.com/ligato/vpp-agent/pull/1638/checks?check_run_id=490037995

@rastislavs
Copy link
Contributor Author

Breaking changes to proto:

proto/ligato/vpp/ipsec/ipsec.proto:36:5:Field "1" on message "SecurityPolicyDatabase" changed type from "string" to "uint32".
proto/ligato/vpp/ipsec/ipsec.proto:44:9:Field "1" on message "PolicyEntry" changed type from "string" to "uint32".
proto/ligato/vpp/ipsec/ipsec.proto:74:5:Field "1" on message "SecurityAssociation" changed type from "string" to "uint32".

https://github.com/ligato/vpp-agent/pull/1638/checks?check_run_id=490037995

Yes, this is what I meant with

This PR introduces a type change in the existing IPSec NB API (IPsec SA and SPD indexes), where numeric types were modeled as strings. Since that API was rarely used up till now (there was no need for them when using the old IPSec tunnel type), but will be needed going forward, it is the best time to clean that up now.

Those APIs were not used anyhow up till now (at least I don't know about any of their use-case - there was no way to wire SAs to an interface). They will be used going forward. The question is, do we want to keep backward compatibility of unused APIs, or make them cleaner now, when we actually start using them?

Signed-off-by: Rastislav Szabo <raszabo@cisco.com>
Signed-off-by: Rastislav Szabo <raszabo@cisco.com>
Signed-off-by: Rastislav Szabo <raszabo@cisco.com>
@ondrej-fabry ondrej-fabry merged commit 3ad3aa3 into ligato:master Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants