Re: [Swan-dev] [PATCH libreswan v2 2/3] pluto, whack: Add nic-offload 'auto' mode
> -Original Message-
> From: Antony Antony [mailto:ant...@phenome.org]
> Subject: Re: [PATCH libreswan v2 2/3] pluto, whack: Add nic-offload 'auto'
> mode
>
> > +enum nic_offload_options {
> > + nic_offload_no = 0,
>
> the feed back I sent used nic_offload_no = 1
> that is less confusing. Most vaules get zeroed so preferably start from 1.
>
> > + nic_offload_yes = 1,
> > + nic_offload_auto = 2,
> > +};
Right.
> > case CD_NIC_OFFLOAD: /* --nic-offload */
> > - msg.nic_offload = TRUE;
> > + if (streq(optarg, "no"))
> > + msg.nic_offload = nic_offload_no;
> > + else if (streq(optarg, "yes"))
> > + msg.nic_offload = nic_offload_yes;
> > + else if (streq(optarg, "auto"))
> > + msg.nic_offload = nic_offload_auto;
> > + else
> > + diag("--nic_offload options are 'no', 'yes' or
> 'auto'");
>
> the last line diag should be "--nic-offload" ?
>
Right again.
Did you fix both things in the merged patch in master?
Ilan.
___
Swan-dev mailing list
Swan-dev@lists.libreswan.org
https://lists.libreswan.org/mailman/listinfo/swan-dev
Re: [Swan-dev] [PATCH libreswan v2 2/3] pluto, whack: Add nic-offload 'auto' mode
Hi Ilan,
now all three patches are on the libreswan master. I added a few minor style
changes. please test merge.
-antony
On Fri, Aug 04, 2017 at 02:30:45PM +0200, Antony Antony wrote:
> a couple minor comments. 1/3 is already applied by Paul.
> Here are my comments about 2/3 and will send another one for 3/3
>
> On Wed, Aug 02, 2017 at 06:22:27PM +0300, il...@mellanox.com wrote:
> > From: Ilan Tayari
> >
> > Convert nic-offload configuration from boolean to 3-choice enum.
> > The behavior is:
> > no - Never attempt nic offload
> > yes - Always attempt nic offload, and fail if it fails
> > auto - Auto-detect kernel and NIC capability. attempt nic-offload
> > only if supported, and fallback to non-offload if it fails.
> >
> > Default is auto.
> >
> > Signed-off-by: Ilan Tayari
> > ---
> > 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..a03ed8e5b 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_no = 0,
>
> the feed back I sent used nic_offload_no = 1
> that is less confusing. Most vaules get zeroed so preferably start from 1.
>
> > + nic_offload_yes = 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..23ab2e036 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[] = {
> > + { "no", nic_offload_no },
> > + { "yes", nic_offload_yes },
> > + { "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 7d7b9fa17..7cc5d5f40 100644
> > --- a/programs/pluto/connections.c
> > +++ b/programs/pluto/connections.c
> > @@ -4108,12 +4108,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->na
Re: [Swan-dev] [PATCH libreswan v2 2/3] pluto, whack: Add nic-offload 'auto' mode
a couple minor comments. 1/3 is already applied by Paul.
Here are my comments about 2/3 and will send another one for 3/3
On Wed, Aug 02, 2017 at 06:22:27PM +0300, il...@mellanox.com wrote:
> From: Ilan Tayari
>
> Convert nic-offload configuration from boolean to 3-choice enum.
> The behavior is:
> no - Never attempt nic offload
> yes - Always attempt nic offload, and fail if it fails
> auto - Auto-detect kernel and NIC capability. attempt nic-offload
> only if supported, and fallback to non-offload if it fails.
>
> Default is auto.
>
> Signed-off-by: Ilan Tayari
> ---
> 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..a03ed8e5b 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_no = 0,
the feed back I sent used nic_offload_no = 1
that is less confusing. Most vaules get zeroed so preferably start from 1.
> + nic_offload_yes = 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..23ab2e036 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[] = {
> + { "no", nic_offload_no },
> + { "yes", nic_offload_yes },
> + { "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 7d7b9fa17..7cc5d5f40 100644
> --- a/programs/pluto/connections.c
> +++ b/programs/pluto/connections.c
> @@ -4108,12 +4108,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_yes)
