[Swan-dev] [PATCH libreswan v2 2/3] pluto, whack: Add nic-offload 'auto' mode

Antony Antony antony at phenome.org
Sat Aug 5 04:22:00 UTC 2017


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, 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:
> >   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 <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..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) ? "yes" : "no"
> >  	);
> >  
> >  	{
> > 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..86b845fae 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, "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" ?
> 
> >  			continue;
> >  
> >  		case CD_NO_NAT_KEEPALIVE:	/* --no-nat_keepalive */
> > -- 
> > 2.11.0
> > 


More information about the Swan-dev mailing list