[Swan-dev] [libreswan RFC 2/3] pluto, whack: Add nic-offload 'auto' mode
Antony Antony
antony at phenome.org
Mon Jul 24 18:48:53 UTC 2017
Hi Ilan,
Thanks for the patches.
I reviewed the patches. May be Hugh can clarify the first one. I wonder the
comment following that line need to be fix as well.
For the second and the third patch here is my suggestion.
no|yes|auto is probably better keywords in the libreswan context.
Libreswan do not use first letter capitalizing as you proposed.
I am not sure about making auto default. Paul would ethtool detection work
on RHEL 6.x? Ilan have tested this on RHEL/CentOS 6.x, without a Mellanox
NIC?
-antony
On Tue, Jul 11, 2017 at 06:09:06PM +0300, ilant at mellanox.com wrote:
> From: Ilan Tayari <ilant at mellanox.com>
>
> Convert nic-offload configuration from boolean to 3-choice
> enum. The behavior is:
> Never - never attempt nic offload
> Always - always attempt nic offload, and fail if it fails
> Auto - auto-detect nic capability. attempt nic-offload if
> supported, and fallback to non-offload if it fails.
>
> Default is Auto.
>
> Signed-off-by: Ilan Tayari <ilant at mellanox.com>
> ---
> include/pluto_constants.h | 6 ++++++
> include/whack.h | 2 +-
> lib/libipsecconf/confread.c | 2 +-
> lib/libipsecconf/keywords.c | 8 +++++++-
> programs/pluto/connections.c | 5 +++--
> programs/pluto/connections.h | 2 +-
> programs/whack/whack.c | 13 ++++++++++---
> 7 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/include/pluto_constants.h b/include/pluto_constants.h
> index f683e0570..5f6009852 100644
> --- a/include/pluto_constants.h
> +++ b/include/pluto_constants.h
> @@ -723,6 +723,12 @@ enum encaps_options {
> encaps_yes = 3,
> };
>
> +enum nic_offload_options {
> + nic_offload_never = 0,
> + nic_offload_always = 1,
> + nic_offload_auto = 2,
> +};
> +
> enum ynf_options {
> ynf_no = 0,
> ynf_yes = 1,
> diff --git a/include/whack.h b/include/whack.h
> index 9e8a15ae1..bde7909db 100644
> --- a/include/whack.h
> +++ b/include/whack.h
> @@ -155,7 +155,7 @@ struct whack_message {
> unsigned long sa_replay_window;
> deltatime_t r_timeout; /* in secs */
> unsigned long r_interval; /* in msec */
> - bool nic_offload;
> + enum nic_offload_options nic_offload;
>
> /* For IKEv1 RFC 3706 - Dead Peer Detection */
> deltatime_t dpd_delay;
> diff --git a/lib/libipsecconf/confread.c b/lib/libipsecconf/confread.c
> index da2c11f28..2544351a9 100644
> --- a/lib/libipsecconf/confread.c
> +++ b/lib/libipsecconf/confread.c
> @@ -150,7 +150,7 @@ void ipsecconf_default_values(struct starter_config *cfg)
> POLICY_IKE_FRAG_ALLOW | /* ike_frag=yes */
> POLICY_ESN_NO; /* esn=no */
>
> - cfg->conn_default.options[KBF_NIC_OFFLOAD] = FALSE;
> + cfg->conn_default.options[KBF_NIC_OFFLOAD] = nic_offload_auto;
> cfg->conn_default.options[KBF_IKELIFETIME] = IKE_SA_LIFETIME_DEFAULT;
>
> cfg->conn_default.options[KBF_REPLAY_WINDOW] = IPSEC_SA_DEFAULT_REPLAY_WINDOW;
> diff --git a/lib/libipsecconf/keywords.c b/lib/libipsecconf/keywords.c
> index a2f554f96..cf8c178e4 100644
> --- a/lib/libipsecconf/keywords.c
> +++ b/lib/libipsecconf/keywords.c
> @@ -389,6 +389,12 @@ static const struct keyword_enum_value kw_ocsp_method_values[] = {
> };
> static const struct keyword_enum_values kw_ocsp_method_list = VALUES_INITIALIZER(kw_ocsp_method_values);
>
> +static const struct keyword_enum_value kw_nic_offload_values[] = {
> + { "never", nic_offload_never },
> + { "always", nic_offload_always },
> + { "auto", nic_offload_auto },
> +};
> +static const struct keyword_enum_values kw_nic_offload_list = VALUES_INITIALIZER(kw_nic_offload_values);
>
> /* MASTER KEYWORD LIST
> * Note: this table is terminated by an entry with keyname == NULL.
> @@ -610,7 +616,7 @@ const struct keyword_def ipsec_conf_keywords_v2[] = {
> { "modecfgwins1", kv_conn, kt_obsolete, KBF_WARNIGNORE, NOT_ENUM },
> { "modecfgwins2", kv_conn, kt_obsolete, KBF_WARNIGNORE, NOT_ENUM },
>
> - { "nic-offload", kv_conn, kt_bool, KBF_NIC_OFFLOAD, NOT_ENUM },
> + { "nic-offload", kv_conn, kt_enum, KBF_NIC_OFFLOAD, &kw_nic_offload_list },
> { "encapsulation", kv_conn, kt_enum, KBF_ENCAPS, &kw_encaps_list },
> { "forceencaps", kv_conn, kt_obsolete, KBF_WARNIGNORE, NOT_ENUM },
>
> diff --git a/programs/pluto/connections.c b/programs/pluto/connections.c
> index 0f34a95d3..74036d603 100644
> --- a/programs/pluto/connections.c
> +++ b/programs/pluto/connections.c
> @@ -4095,12 +4095,13 @@ void show_one_connection(const struct connection *c)
> strcpy(markstr, "unset");
>
> whack_log(RC_COMMENT,
> - "\"%s\"%s: nflog-group: %s; mark: %s; vti-iface:%s; vti-routing:%s; vti-shared:%s;%s",
> + "\"%s\"%s: nflog-group: %s; mark: %s; vti-iface:%s; vti-routing:%s; vti-shared:%s; nic-offload:%s",
> c->name, instance, nflogstr, markstr,
> c->vti_iface == NULL ? "unset" : c->vti_iface,
> c->vti_routing ? "yes" : "no",
> c->vti_shared ? "yes" : "no",
> - c->nic_offload ? " nic-offload:yes;" : ""
> + (c->nic_offload == nic_offload_auto) ? "auto" :
> + (c->nic_offload == nic_offload_always) ? "always" : "never"
> );
>
> {
> diff --git a/programs/pluto/connections.h b/programs/pluto/connections.h
> index d5fee13d9..7c10ce998 100644
> --- a/programs/pluto/connections.h
> +++ b/programs/pluto/connections.h
> @@ -242,7 +242,7 @@ struct connection {
> deltatime_t r_timeout; /* max time (in secs) for one packet exchange attempt */
> reqid_t sa_reqid;
> int encapsulation;
> - bool nic_offload;
> + enum nic_offload_options nic_offload;
>
> /* RFC 3706 DPD */
> deltatime_t dpd_delay; /* time between checks */
> diff --git a/programs/whack/whack.c b/programs/whack/whack.c
> index 9157fc6b5..77f2d8a08 100644
> --- a/programs/whack/whack.c
> +++ b/programs/whack/whack.c
> @@ -675,7 +675,7 @@ static const struct option long_opts[] = {
> { "pfsgroup", required_argument, NULL, CD_PFSGROUP + OO },
> { "esp", required_argument, NULL, CD_ESP + OO },
> { "remote_peer_type", required_argument, NULL, CD_REMOTEPEERTYPE + OO },
> - { "nic-offload", no_argument, NULL, CD_NIC_OFFLOAD + OO},
> + { "nic-offload", required_argument, NULL, CD_NIC_OFFLOAD + OO},
>
>
> PS("ikev1-allow", IKEV1_ALLOW),
> @@ -945,7 +945,7 @@ int main(int argc, char **argv)
> msg.modecfg_domain = NULL;
> msg.modecfg_banner = NULL;
>
> - msg.nic_offload = FALSE;
> + msg.nic_offload = nic_offload_auto;
> msg.sa_ike_life_seconds = deltatime(IKE_SA_LIFETIME_DEFAULT);
> msg.sa_ipsec_life_seconds = deltatime(IPSEC_SA_LIFETIME_DEFAULT);
> msg.sa_rekey_margin = deltatime(SA_REPLACEMENT_MARGIN_DEFAULT);
> @@ -1698,7 +1698,14 @@ int main(int argc, char **argv)
> continue;
>
> case CD_NIC_OFFLOAD: /* --nic-offload */
> - msg.nic_offload = TRUE;
> + if (streq(optarg, "never"))
> + msg.nic_offload = nic_offload_never;
> + else if (streq(optarg, "always"))
> + msg.nic_offload = nic_offload_always;
> + else if (streq(optarg, "auto"))
> + msg.nic_offload = nic_offload_auto;
> + else
> + diag("--nic_offload options are 'never', 'always' or 'auto'");
> continue;
>
> case CD_NO_NAT_KEEPALIVE: /* --no-nat_keepalive */
> --
> 2.11.0
>
-------------- next part --------------
>From 86566d662d93da9d848e7c853490aabbce3565f0 Mon Sep 17 00:00:00 2001
From: Antony Antony <antony at phenome.org>
Date: Wed, 12 Jul 2017 21:32:05 +0200
Subject: [PATCH 1/2] pluto: nic-offload configuration from boolean to 3-choice
enum
The behavior is:
no - do not attempt nic offload
yes - only nic offload, and fail if it fails
auto - auto-detect nic capability. attempt nic-offload if
supported, and fallback to non-offload if it fails.
Default is Auto.
---
include/pluto_constants.h | 4 ++--
lib/libipsecconf/keywords.c | 4 ++--
programs/pluto/connections.c | 2 +-
programs/whack/whack.c | 10 +++++-----
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/pluto_constants.h b/include/pluto_constants.h
index 5f60098..896e1c9 100644
--- a/include/pluto_constants.h
+++ b/include/pluto_constants.h
@@ -724,9 +724,9 @@ enum encaps_options {
};
enum nic_offload_options {
- nic_offload_never = 0,
- nic_offload_always = 1,
+ nic_offload_no = 1,
nic_offload_auto = 2,
+ nic_offload_yes = 3,
};
enum ynf_options {
diff --git a/lib/libipsecconf/keywords.c b/lib/libipsecconf/keywords.c
index cf8c178..2f2275a 100644
--- a/lib/libipsecconf/keywords.c
+++ b/lib/libipsecconf/keywords.c
@@ -390,9 +390,9 @@ static const struct keyword_enum_value kw_ocsp_method_values[] = {
static const struct keyword_enum_values kw_ocsp_method_list = VALUES_INITIALIZER(kw_ocsp_method_values);
static const struct keyword_enum_value kw_nic_offload_values[] = {
- { "never", nic_offload_never },
- { "always", nic_offload_always },
+ { "no", nic_offload_no },
{ "auto", nic_offload_auto },
+ { "yes", nic_offload_yes },
};
static const struct keyword_enum_values kw_nic_offload_list = VALUES_INITIALIZER(kw_nic_offload_values);
diff --git a/programs/pluto/connections.c b/programs/pluto/connections.c
index a0d4118..7cc5d5f 100644
--- a/programs/pluto/connections.c
+++ b/programs/pluto/connections.c
@@ -4114,7 +4114,7 @@ void show_one_connection(const struct connection *c)
c->vti_routing ? "yes" : "no",
c->vti_shared ? "yes" : "no",
(c->nic_offload == nic_offload_auto) ? "auto" :
- (c->nic_offload == nic_offload_always) ? "always" : "never"
+ (c->nic_offload == nic_offload_yes) ? "yes" : "no"
);
{
diff --git a/programs/whack/whack.c b/programs/whack/whack.c
index 77f2d8a..6fe13f3 100644
--- a/programs/whack/whack.c
+++ b/programs/whack/whack.c
@@ -1698,14 +1698,14 @@ int main(int argc, char **argv)
continue;
case CD_NIC_OFFLOAD: /* --nic-offload */
- if (streq(optarg, "never"))
- msg.nic_offload = nic_offload_never;
- else if (streq(optarg, "always"))
- msg.nic_offload = nic_offload_always;
+ if (streq(optarg, "no"))
+ msg.nic_offload = nic_offload_no;
+ else if (streq(optarg, "auto"))
+ msg.nic_offload = nic_offload_yes;
else if (streq(optarg, "auto"))
msg.nic_offload = nic_offload_auto;
else
- diag("--nic_offload options are 'never', 'always' or 'auto'");
+ diag("--nic_offload options are 'no', 'auto' or 'yes'");
continue;
case CD_NO_NAT_KEEPALIVE: /* --no-nat_keepalive */
--
2.4.11
-------------- next part --------------
>From b0e347b262543fd6d2a8cd21afcbe2e251231d57 Mon Sep 17 00:00:00 2001
From: Antony Antony <antony at phenome.org>
Date: Wed, 12 Jul 2017 21:57:04 +0200
Subject: [PATCH 2/2] pluto: squash "Provide per-device detection callback in
kernel_ops."
---
programs/pluto/kernel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/programs/pluto/kernel.c b/programs/pluto/kernel.c
index ecaacd2..4051501 100644
--- a/programs/pluto/kernel.c
+++ b/programs/pluto/kernel.c
@@ -1717,7 +1717,7 @@ static bool del_spi(ipsec_spi_t spi, int proto,
static void setup_esp_nic_offload(struct kernel_sa *sa, struct connection *c,
bool *nic_offload_fallback)
{
- if (c->nic_offload == nic_offload_never)
+ if (c->nic_offload == nic_offload_no)
return;
if (!c->interface || !c->interface->ip_dev ||
!c->interface->ip_dev->id_rname)
--
2.4.11
More information about the Swan-dev
mailing list