All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Rapoport <mike.rapoport@ravellosystems.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Thomas Graf <tgraf@suug.ch>,
	netdev@vger.kernel.org, David Stevens <dlstevens@us.ibm.com>
Subject: Re: [PATCH net-next v3 2/2] vxlan: allow specifying multiple default destinations
Date: Mon, 3 Jun 2013 22:47:31 +0300	[thread overview]
Message-ID: <20130603194731.GA7380@zed> (raw)
In-Reply-To: <20130603085737.72b8406f@nehalam.linuxnetplumber.net>

(added David Stevens to CC)

On Mon, Jun 03, 2013 at 08:57:37AM -0700, Stephen Hemminger wrote:
> On Sun, 2 Jun 2013 13:29:42 +0300
> Mike Rapoport <mike.rapoport@ravellosystems.com> wrote:
> 
> > On Fri, May 31, 2013 at 7:17 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> > > Looking at this code in more detail, I see a slew of problems.
> > >
> > > First the list of destinations isn't really a list. The default one is still
> > > embedded in the fdb entry. This means you can't change it safely.
> > >
> > > Also the notification via netlink only sends back a single destination
> > > value.
> > >
> > > And the lack of locking on the open coded link list means it is not safe since
> > > the forwarding table is used with RCU. In order to be safe, proper RCU
> > > barriers would be needed or better yet convert to list_rcu..
> > >
> > > Overall, I feel guilty for not inspecting this more closely and am surprised
> > > that others did not catch the lack of locking.
> > 
> > I've tried to convert remotes list to use hlist_rcu. The patch below implements the conversion, but it does not address the netlink notification issue.
> 
> 
> Ok, let me have a go at this.
> 1. Using list rather than hlist makes more sense for handling simple list

I can replace  hlist with list.

> 2. The complexity is in how do do the netlink API.
 
As far as I can tell, there are two places in the driver that should
report multiple destinations via netlink. They are vxlan_fdb_dump and
vxlan_fdb_restore. These methods can traverse the remote destinations
list and send netlink notification for each list item. 

> 3. If we can't work this out for 3.11, the multiple remotes may have to be reverted;
>    I don't want the existing API to be exposed in a release
> 

I'd really like to help to sort this out ASAP, so that I could update and resend
my patches that, by coinsidence, require list of remote destinations.
I'm not the right person to redefine the fdb API, though.

--
Sincerely yours,
Mike.

  reply	other threads:[~2013-06-03 19:47 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-29 10:00 [PATCH net-next v3 0/2] vxlan: allow specifying multiple default destinations Mike Rapoport
2013-05-29 10:00 ` [PATCH net-next v3 1/2] vxlan: introduce vxlan_rdst_append Mike Rapoport
2013-05-29 22:56   ` Stephen Hemminger
2013-05-30  8:42     ` Mike Rapoport
2013-05-29 10:00 ` [PATCH net-next v3 2/2] vxlan: allow specifying multiple default destinations Mike Rapoport
2013-05-30 11:09   ` Thomas Graf
2013-05-30 11:16     ` Mike Rapoport
2013-05-30 11:37       ` Thomas Graf
2013-05-31 16:17         ` Stephen Hemminger
2013-06-02 10:29           ` Mike Rapoport
2013-06-03 15:57             ` Stephen Hemminger
2013-06-03 19:47               ` Mike Rapoport [this message]
2013-06-03 18:26             ` [RFC] vxlan: convert remote list to list_rcu Stephen Hemminger
2013-06-03 20:18               ` David Stevens
2013-06-03 20:45                 ` Stephen Hemminger
2013-06-03 21:46                   ` David Stevens
2013-06-04  9:18                     ` Mike Rapoport
2013-06-04 12:48                       ` David Stevens
2013-06-04 17:20                         ` Mike Rapoport
2013-06-04 19:02                           ` David Stevens
2013-06-05 12:53                             ` Mike Rapoport
2013-06-04  9:10                 ` Mike Rapoport
2013-06-04 16:00                   ` Stephen Hemminger
2013-06-04 16:29                     ` David Stevens
2013-06-04 17:22                       ` Mike Rapoport
2013-05-29 10:00 ` [PATCH iproute2] vxlan: allow specifying multiple default destinations Mike Rapoport
2013-05-29 10:13   ` Cong Wang
2013-05-29 10:52     ` Mike Rapoport
2013-05-29 22:56       ` Stephen Hemminger
2013-05-30  8:42         ` Mike Rapoport
2013-05-30 11:44           ` Thomas Graf
2013-05-30 12:46             ` Mike Rapoport
2013-05-30 15:57               ` Thomas Graf
2013-06-02  7:09                 ` Mike Rapoport
2013-06-05  4:30                   ` Stephen Hemminger
2013-06-05 12:58                     ` Mike Rapoport
2013-05-30 17:07             ` Stephen Hemminger

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=20130603194731.GA7380@zed \
    --to=mike.rapoport@ravellosystems.com \
    --cc=dlstevens@us.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    --cc=tgraf@suug.ch \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.