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, [email protected] wrote: > From: Ilan Tayari <[email protected]> > > 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 <[email protected]> > --- > 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 >
>From 86566d662d93da9d848e7c853490aabbce3565f0 Mon Sep 17 00:00:00 2001 From: Antony Antony <[email protected]> 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
>From b0e347b262543fd6d2a8cd21afcbe2e251231d57 Mon Sep 17 00:00:00 2001 From: Antony Antony <[email protected]> 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
_______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
