[PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-10 Thread Girish Moodalbail
The kernel log is not where users expect error messages for netlink requests; as we have extended acks now, we can replace pr_debug() with NL_SET_ERR_MSG_ATTR(). Signed-off-by: Matthias Schiffer Signed-off-by: Girish Moodalbail --- v1 -> v2: - addressed, error messages rewording, comments fr

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread Jiri Benc
On Thu, 10 Aug 2017 14:16:35 -0700, Girish Moodalbail wrote: > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT_RANGE], > + "Provided source port range bounds > is invalid"); s/bounds//? > + if (conf->label && !use_ipv6) { > +

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread Roopa Prabhu
On Thu, Aug 10, 2017 at 2:16 PM, Girish Moodalbail wrote: > The kernel log is not where users expect error messages for netlink > requests; as we have extended acks now, we can replace pr_debug() with > NL_SET_ERR_MSG_ATTR(). > > Signed-off-by: Matthias Schiffer > Signed-off-by: Girish Moodalbail

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread Matthias Schiffer
On 08/11/2017 06:19 PM, Roopa Prabhu wrote: > On Thu, Aug 10, 2017 at 2:16 PM, Girish Moodalbail > wrote: >> >> if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) { >> - pr_debug("invalid all zero ethernet address\n"); >> + NL_SET_ERR_

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread Jiri Benc
On Fri, 11 Aug 2017 09:19:34 -0700, Roopa Prabhu wrote: > > if (tb[IFLA_ADDRESS]) { > > if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN) { > > - pr_debug("invalid link address (not ethernet)\n"); > > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread David Ahern
On 8/11/17 10:39 AM, Jiri Benc wrote: >>> + if (!data) { >>> + NL_SET_ERR_MSG(extack, >>> + "Not enough attributes provided to perform >>> the operation"); >>> return -EINVAL; >>> + } >> "not enough attributes" > You're missing

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread Jiri Benc
On Fri, 11 Aug 2017 10:56:57 -0600, David Ahern wrote: > I would argue none of those messages are sufficient. The message should > tell the user what is missing. Good point. I guess "The IFLA_INFO_DATA attribute is missing" would be a better message. It can happen only when you're implementing yo

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread David Ahern
On 8/11/17 11:11 AM, Jiri Benc wrote: > On Fri, 11 Aug 2017 10:56:57 -0600, David Ahern wrote: >> I would argue none of those messages are sufficient. The message should >> tell the user what is missing. > > Good point. > > I guess "The IFLA_INFO_DATA attribute is missing" would be a better > mes

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread Girish Moodalbail
[snip] @@ -2933,6 +2945,8 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf, */ if ((conf->flags & ~VXLAN_F_ALLOWED_GPE) || !(conf->flags & VXLAN_F_COLLECT_METADATA)) { + NL_SET_ERR_MS

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread Jiri Benc
On Fri, 11 Aug 2017 11:17:54 -0600, David Ahern wrote: > What if a user adds IFLA_INFO_DATA but adds no vxlan attributes under > it? Still not a valid config, but it passes the !data check. Part of the checking is done in the newlink callback (vxlan_nl2conf). Not nice but that's how it is. It's ou

Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting

2017-08-11 Thread Roopa Prabhu
On Fri, Aug 11, 2017 at 9:39 AM, Jiri Benc wrote: > On Fri, 11 Aug 2017 09:19:34 -0700, Roopa Prabhu wrote: >> > if (tb[IFLA_ADDRESS]) { >> > if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN) { >> > - pr_debug("invalid link address (not ethernet)\n"); >> > +