Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: add MDB support

From: Sergei Shtylyov
Date: Wed Aug 31 2016 - 13:34:07 EST


Hello.

On 08/31/2016 05:46 PM, Vivien Didelot wrote:

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 93abfff..812cb47 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2240,6 +2240,15 @@ static int mv88e6xxx_port_db_dump_one(struct mv88e6xxx_chip *chip,
fdb->ndm_state = NUD_NOARP;
else
fdb->ndm_state = NUD_REACHABLE;
+ } else {

Rather than else, i think it would be safer to do

if (obj->id == SWITCHDEV_OBJ_ID_PORT_MDB) {
+ struct switchdev_obj_port_mdb *mdb;
+
+ if (!is_multicast_ether_addr(addr.mac))
+ continue;
+
+ mdb = SWITCHDEV_OBJ_PORT_MDB(obj);
+ mdb->vid = vid;
+ ether_addr_copy(mdb->addr, addr.mac);
}

It should not happen, but the day it does, we get very confused...

Do you mean the something like this?

if (obj->id == SWITCHDEV_OBJ_ID_PORT_FDB) {
...
} else if (obj->id == SWITCHDEV_OBJ_ID_PORT_MDB) {
...
} else {
return -EOPNOTSUPP;
}

I'm OK with that if you think it is better.

Just code it as a *switch*, please. :-)

[...]

MBR, Sergei