netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Vlad Yasevich <vyasevic@redhat.com>
Cc: john.r.fastabend@intel.com, netdev@vger.kernel.org,
	bridge@lists.linux-foundation.org, jhs@mojatatu.com,
	Stephen Hemminger <stephen@networkplumber.org>,
	shemminger@vyatta.com
Subject: Re: [PATCH 3/7] bridge: Add addresses from static fdbs to bridge address list
Date: Thu, 27 Feb 2014 09:53:36 +0200	[thread overview]
Message-ID: <20140227075336.GF16484@redhat.com> (raw)
In-Reply-To: <530E25CC.6010705@redhat.com>

On Wed, Feb 26, 2014 at 12:35:08PM -0500, Vlad Yasevich wrote:
> On 02/26/2014 11:57 AM, Stephen Hemminger wrote:
> > On Wed, 26 Feb 2014 10:18:21 -0500
> > Vlad Yasevich <vyasevic@redhat.com> wrote:
> > 
> >> When a static fdb entry is created, add the mac address to the bridge
> >> address list.  This list is used to program the proper port's
> >> address list.
> >>
> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> > 
> > I don't like this level of bookkeeping it starts to mix
> > layers between the bridge network interface as entity for talking to the
> > local host, and forwarding table entries.
> 
> Actually this is one of the reasons this isn't done through the
> br->dev->uc.  Forwarding table entries are still per-port.
> 
> > 
> > Many times static entries are used as alternative to flooding in
> > environments which don't trust STP.
> 
> Ok, and how would this be problematic?  If one wants to turn off
> promisc in this environment, then receive filters needs to be properly
> programmed.
> 
> > 
> > Plus, it looks like another major source of bugs.
> > 
> 
> Any new code is a potential source of issues.  Are you saying
> No to any new code in bridge?
> 
> -vlad

I'm guessing Stephen merely worries about
multiple data structures that need to stay in
sync, and asks that you revisit
using private hw address list in the bridge.

What's the issue with walking fdb exactly?
You say
 1)  I tried using the fdb table itself as main repository, but
      this caused difficulties in synchronizing this table with
      the interface filters later on.

I'm guessing you refer to writing addresses out to ports
directly when walking the hash being impossible
since this datastructure uses rcu and spinlocks?
Fair enough but the entries you care about
seem to only be modified under RTNL so just
copy them out to a temporary list.
This might be less efficient, but will be simpler I think.

-- 
MST

  reply	other threads:[~2014-02-27  7:53 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-26 15:18 [PATCH RFC 0/7] Non-promisc bidge ports support Vlad Yasevich
2014-02-26 15:18 ` [PATCH 1/7] bridge: Turn flag change macro into a function Vlad Yasevich
2014-02-26 15:29   ` Michael S. Tsirkin
2014-02-26 15:36     ` Vlad Yasevich
2014-02-26 15:18 ` [PATCH 2/7] bridge: Keep track of ports capable of flooding Vlad Yasevich
2014-02-26 15:41   ` Michael S. Tsirkin
2014-02-26 15:41     ` Vlad Yasevich
2014-02-26 15:53       ` Michael S. Tsirkin
2014-02-27 11:59   ` Toshiaki Makita
2014-02-27 12:54     ` Vlad Yasevich
2014-02-26 15:18 ` [PATCH 3/7] bridge: Add addresses from static fdbs to bridge address list Vlad Yasevich
2014-02-26 15:46   ` Michael S. Tsirkin
2014-02-26 15:43     ` Vlad Yasevich
2014-02-26 16:23   ` Michael S. Tsirkin
2014-02-26 17:25     ` Vlad Yasevich
2014-02-26 17:33       ` Michael S. Tsirkin
2014-02-26 16:57   ` Stephen Hemminger
2014-02-26 17:35     ` Vlad Yasevich
2014-02-27  7:53       ` Michael S. Tsirkin [this message]
2014-02-27 13:08         ` Vlad Yasevich
2014-02-27 13:38           ` Michael S. Tsirkin
2014-02-26 15:18 ` [PATCH 4/7] bridge: Automatically manage port promiscuous mode Vlad Yasevich
2014-02-26 15:51   ` Michael S. Tsirkin
2014-02-26 16:02     ` Vlad Yasevich
2014-02-26 16:58   ` Stephen Hemminger
2014-02-26 17:32     ` Michael S. Tsirkin
2014-02-26 15:18 ` [PATCH 5/7] bridge: Correctly manage promiscuity when user requested it Vlad Yasevich
2014-02-26 15:18 ` [PATCH 6/7] bridge: Manage promisc mode when vlans are configured on top of a bridge Vlad Yasevich
2014-02-26 16:00   ` Michael S. Tsirkin
2014-02-26 16:05     ` Vlad Yasevich
2014-02-26 16:25       ` Michael S. Tsirkin
2014-02-27 12:06   ` Toshiaki Makita
2014-02-27 13:17     ` Vlad Yasevich
2014-02-28 19:34       ` Vlad Yasevich
2014-03-01 14:57         ` Toshiaki Makita
2014-03-03 12:12           ` Vlad Yasevich
2014-02-26 15:18 ` [PATCH 7/7] bridge: Support promisc management when all ports are non-flooding Vlad Yasevich
2014-02-26 15:57   ` Michael S. Tsirkin
2014-02-27  3:46     ` Vlad Yasevich
2014-02-27  7:29       ` Michael S. Tsirkin
2014-02-26 16:01   ` Michael S. Tsirkin
2014-02-26 16:34 ` [PATCH RFC 0/7] Non-promisc bidge ports support Michael S. Tsirkin
2014-02-26 23:59 ` Jamal Hadi Salim
2014-02-27  3:37   ` Vlad Yasevich
2014-02-27  8:54     ` [Bridge] " Amidu Sila
2014-02-27  7:20   ` Michael S. Tsirkin

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=20140227075336.GF16484@redhat.com \
    --to=mst@redhat.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=jhs@mojatatu.com \
    --cc=john.r.fastabend@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=shemminger@vyatta.com \
    --cc=stephen@networkplumber.org \
    --cc=vyasevic@redhat.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).