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
ipsec: Add support of ipsec-interface
, DPD options and authby
#2478
Conversation
Skipping CI for Draft Pull Request. |
16b1105
to
7adf030
Compare
ipsec-interface
and DPD optionsipsec-interface
and DPD options
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
ipsec-interface
and DPD optionsipsec-interface
, DPD options and authby
1eb7e36
to
69c1169
Compare
cd8978a
to
04d85f2
Compare
@@ -26,6 +26,7 @@ fn np_iface_type_to_nmstate( | |||
nispor::IfaceType::Vxlan => InterfaceType::Vxlan, | |||
nispor::IfaceType::Ipoib => InterfaceType::InfiniBand, | |||
nispor::IfaceType::Tun => InterfaceType::Tun, | |||
nispor::IfaceType::Other(v) if v == "xfrm" => InterfaceType::Xfrm, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not support the xfrm
interface in nispor ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is urgent case, release new version of nispor take times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can at least add a "TODO" comment ? to move it to nispor ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am creating a JIRA card to track this work :)
c21b157
to
5581760
Compare
skip_serializing_if = "Option::is_none", | ||
rename = "ipsec-interface", | ||
default, | ||
deserialize_with = "parse_ipsec_iface" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is not easier to make ipsec_interface a struct with a bool and a number ? instead of this multitype "yes", "no", number string ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cathay4t do you have the NetworkManager ref around ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have to follow exactly the same format, we can make it more "structural" like:
use-ipsec-interfae: true|false
ipsec-interface-number: 33
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no. If you have better way of parsing yes|no|u32, please state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to invent new stuff, just follow ipsec.conf
manpage here. The section is named as libreswan
, so I will refrain myself on abstracting stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is patch to NetworkManager-libreswan for this : https://gitlab.gnome.org/GNOME/NetworkManager-libreswan/-/merge_requests/25/
I have scratch build in https://people.redhat.com/fge/ipsec_nmstate/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only support integer or string now. no bool support.
Add support of these libreswan Ipsec options: * `ipsec-interface: 'yes'|'no'|u32` * `authby: String` * `dpddelay: u64` * `dpdtimeout: u64` * `dpdaction: String` Integration test case included but marked as OK to fail as NetworkManager-libreswan supporting these options is not released yet. Signed-off-by: Gris Ge <fge@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add support of these libreswan Ipsec options:
ipsec-interface: 'yes'|'no'|u32
dpddelay: u64
dpdtimeout: u64
dpdaction: String
authby: String
Integration test case included but marked as OK to fail as
NetworkManager-libreswan supporting these options is not released yet.