[iproute2] bridge: Assume master at FDB modification

Message ID 1501083394-26180-1-git-send-email-arkadis@mellanox.com
State Changes Requested
Delegated to: stephen hemminger
Headers show

Commit Message

Arkadi Sharshevsky July 26, 2017, 3:36 p.m.
According to the man page the master flag should be the default, yet, the
current code assumes otherwise.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
---
 bridge/fdb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Stephen Hemminger July 26, 2017, 4:08 p.m. | #1
On Wed, 26 Jul 2017 18:36:34 +0300
Arkadi Sharshevsky <arkadis@mellanox.com> wrote:

> According to the man page the master flag should be the default, yet, the
> current code assumes otherwise.
> 
> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>

Agree that the documentation and code don't match.
But your change could break users with existing scripts by changing behavior.

It would be safer to change the man page not the code.
Arkadi Sharshevsky July 26, 2017, 4:21 p.m. | #2
On 07/26/2017 07:08 PM, Stephen Hemminger wrote:
> On Wed, 26 Jul 2017 18:36:34 +0300
> Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
> 
>> According to the man page the master flag should be the default, yet, the
>> current code assumes otherwise.
>>
>> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
> 
> Agree that the documentation and code don't match.
> But your change could break users with existing scripts by changing behavior.
> 
> It would be safer to change the man page not the code.
> 

Can we maybe set master and self by default. It doesn't make
sense by default to not include the bridge, it will not cause
regression in this case.

Patch

diff --git a/bridge/fdb.c b/bridge/fdb.c
index e5cebf9..7c77157 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -496,9 +496,9 @@  static int fdb_modify(int cmd, int flags, int argc, char **argv)
 		return -1;
 	}
 
-	/* Assume self */
+	/* Assume master */
 	if (!(req.ndm.ndm_flags&(NTF_SELF|NTF_MASTER)))
-		req.ndm.ndm_flags |= NTF_SELF;
+		req.ndm.ndm_flags |= NTF_MASTER;
 
 	/* Assume permanent */
 	if (!(req.ndm.ndm_state&(NUD_PERMANENT|NUD_REACHABLE)))