From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:35960 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750883AbeCILWv (ORCPT ); Fri, 9 Mar 2018 06:22:51 -0500 Date: Fri, 9 Mar 2018 12:22:48 +0100 From: Jiri Benc To: Simon Horman Cc: Jiri Pirko , Jamal Hadi Salim , Cong Wang , netdev@vger.kernel.org, oss-drivers@netronome.com, Pieter Jansen van Vuuren Subject: Re: [PATCH/RFC 2/3] net/sched: act_tunnel_key: add extended ack support Message-ID: <20180309122248.1979e00f@redhat.com> In-Reply-To: <20180306170805.19500-3-simon.horman@netronome.com> References: <20180306170805.19500-1-simon.horman@netronome.com> <20180306170805.19500-3-simon.horman@netronome.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On Tue, 6 Mar 2018 18:08:04 +0100, Simon Horman wrote: > - if (!tb[TCA_TUNNEL_KEY_PARMS]) > + if (!tb[TCA_TUNNEL_KEY_PARMS]) { > + NL_SET_ERR_MSG(extack, "Missing tunnel key parameter"); "parameters" (it's not just one parameter) > @@ -107,6 +109,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla, > break; > case TCA_TUNNEL_KEY_ACT_SET: > if (!tb[TCA_TUNNEL_KEY_ENC_KEY_ID]) { > + NL_SET_ERR_MSG(extack, "Missing tunnel key enc id"); This is not much helpful to the user. What's "enc"? I guess "Missing tunnel key id" would be enough and better. > @@ -144,6 +147,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla, > 0, flags, > key_id, 0); > } else { > + NL_SET_ERR_MSG(extack, "Missing both ipv4 and ipv6 enc src and dst"); We don't need both but only one of them. And again, "enc" does not have a clear meaning. "Missing either IPv4 or IPv6 source and destination address"? In addition, it makes little sense to me to add extacks to just some of the errors in the tunnel_key_init function. Please add extacks to all of them. Thanks, Jiri