netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Benc <jbenc@redhat.com>
To: David Ahern <dsahern@gmail.com>
Cc: Roopa Prabhu <roopa@cumulusnetworks.com>,
	Girish Moodalbail <girish.moodalbail@oracle.com>,
	pravin shelar <pshelar@ovn.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Matthias Schiffer <mschiffer@universe-factory.net>
Subject: Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting
Date: Fri, 11 Aug 2017 19:28:06 +0200	[thread overview]
Message-ID: <20170811192806.4910368f@griffin> (raw)
In-Reply-To: <0af5e8f9-36cf-8aed-b431-ffc19eac4c8f@gmail.com>

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 out of scope of this patch to
change that. (It's also not easy - most of the newlink logic would have
to be repeated in validate.)

The !data check needs to be done prior to blindly dereferencing into
data. It's currently in the validate callback. A bit chaotic but it
works. This patch just adds error messages.

> Whatever attributes are required but missing should be the message
> returned.

See the message I proposed in the previous email. newlink will take
care of the rest.

Feel free to submit cleanups, the vxlan driver could use them.

 Jiri

  reply	other threads:[~2017-08-11 17:28 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-10 21:16 [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting Girish Moodalbail
2017-08-11 15:47 ` Jiri Benc
2017-08-11 16:19 ` Roopa Prabhu
2017-08-11 16:24   ` Matthias Schiffer
2017-08-11 16:39   ` Jiri Benc
2017-08-11 16:56     ` David Ahern
2017-08-11 17:11       ` Jiri Benc
2017-08-11 17:17         ` David Ahern
2017-08-11 17:28           ` Jiri Benc [this message]
2017-08-11 17:21     ` Girish Moodalbail
2017-08-12  2:24     ` Roopa Prabhu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170811192806.4910368f@griffin \
    --to=jbenc@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=girish.moodalbail@oracle.com \
    --cc=mschiffer@universe-factory.net \
    --cc=netdev@vger.kernel.org \
    --cc=pshelar@ovn.org \
    --cc=roopa@cumulusnetworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).