Thread (4 messages) 4 messages, 4 authors, 2015-07-29

Re: [PATCH net] bridge: mdb: fix delmdb state in the notification

From: Nikolay Aleksandrov <hidden>
Date: 2015-07-28 22:43:21
Also in: bridge

On 07/29/2015 12:38 AM, Cong Wang wrote:
On Tue, Jul 28, 2015 at 4:10 AM, Nikolay Aleksandrov
[off-list ref] wrote:
quoted
From: Nikolay Aleksandrov <redacted>

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help