From: Nikolay Aleksandrov <nikolay@cumulusnetworks•com>
To: Cong Wang <cwang@twopensource•com>,
Nikolay Aleksandrov <razor@blackwall•org>
Cc: netdev <netdev@vger•kernel.org>,
"bridge@lists•linux-foundation.org"
<bridge@lists•linux-foundation.org>,
David Miller <davem@davemloft•net>, Cong Wang <amwang@redhat•com>
Subject: Re: [PATCH net] bridge: mdb: fix delmdb state in the notification
Date: Wed, 29 Jul 2015 00:43:21 +0200 [thread overview]
Message-ID: <55B80589.4070500@cumulusnetworks.com> (raw)
In-Reply-To: <CAHA+R7NJod_=R=Hh2Mz7nMiTxCMv=E4TMLsn2uOrS+f9gp+uHQ@mail.gmail.com>
On 07/29/2015 12:38 AM, Cong Wang wrote:
> On Tue, Jul 28, 2015 at 4:10 AM, Nikolay Aleksandrov
> <razor@blackwall•org> wrote:
>> From: Nikolay Aleksandrov <nikolay@cumulusnetworks•com>
>>
>> Since mdb states were introduced when deleting an entry the state was
>> left as it was set in the delete request from the user which leads to
>> the following output when doing a monitor (for example):
>> $ bridge mdb add dev br0 port eth3 grp 239.0.0.1 permanent
>> (monitor) dev br0 port eth3 grp 239.0.0.1 permanent
>> $ bridge mdb del dev br0 port eth3 grp 239.0.0.1 permanent
>> (monitor) dev br0 port eth3 grp 239.0.0.1 temp
>> ^^^
>> Note the "temp" state in the delete notification which is wrong since
>> the entry was permanent, the state in a delete is always reported as
>> "temp" regardless of the real state of the entry.
>>
>
> Hmm?
>
> I think it is iproute2 who forgets to set entry->state when deleting it?
>
> } else if (strcmp(*argv, "permanent") == 0) {
> if (cmd == RTM_NEWMDB)
> entry.state |= MDB_PERMANENT;
>
> Kernel simply returns what you pass to it.
>
> Please fix iproute2.
>
Hi Cong,
Please read the full commit log, I've explained that the state is not honored in the kernel
so it doesn't matter if iproute2 sets the correct state that you give on the command
line, that is if I give it "temp" and the entry is permanent - it will still get
deleted and the notification will have the wrong state as "temp" because I've set
it, while this way it'll at least return the correct state of the entry being deleted.
Again I'm saying that I chose this solution over a check for the entry state because
it may break some user-space tools that rely on the behaviour that the state is
not checked in the kernel.
Cheers,
Nik
next prev parent reply other threads:[~2015-07-28 22:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-28 11:10 [PATCH net] bridge: mdb: fix delmdb state in the notification Nikolay Aleksandrov
2015-07-28 22:38 ` Cong Wang
2015-07-28 22:43 ` Nikolay Aleksandrov [this message]
2015-07-29 22:03 ` David Miller
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=55B80589.4070500@cumulusnetworks.com \
--to=nikolay@cumulusnetworks$(echo .)com \
--cc=amwang@redhat$(echo .)com \
--cc=bridge@lists$(echo .)linux-foundation.org \
--cc=cwang@twopensource$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=razor@blackwall$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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