John Fastabend [off-list ref] wrote on 05/30/2012 08:37:06
AM:

Some comments below:

+static int rtnl_bridge_notify(struct net_device *dev, u16 flags)
+{
...
+		 if (!flags && master && master->netdev_ops->

ndo_bridge_getlink)

+		 		 err = master->netdev_ops->ndo_bridge_getlink(skb,

0, 0, dev);

+		 else if (dev->netdev_ops->ndo_bridge_getlink)
+		 		 err = dev->netdev_ops->ndo_bridge_getlink(skb, 0,

0, dev);

I think you should do something like:

        if ((flags == BRIDGE_FLAGS_MASTER) && ...)
                ...

Also you could use BRIDGE_FLAGS_MASTER=1, SELF=2, and use
"if (flags & BRIDGE_FLAGS_MASTER)" for consistency?


+		 if (!err)
+		 		 err = rtnl_bridge_notify(dev, flags);

It is possible to return a reporting error even though
the operation succeeded. Maybe something that could be
done here to indicate that the operation succeeded, or
is that a TODO?

 static int rtnl_bridge_setlink(struct sk_buff *skb, struct nlmsghdr

*nlh,

                 void *arg)
 {

..

+   if (!flags && dev->master &&
+       dev->master->netdev_ops->ndo_bridge_setlink)
+      err = dev->master->netdev_ops->ndo_bridge_setlink(dev, nlh);
+   else if ((flags & BRIDGE_FLAGS_SELF) &&
+         dev->netdev_ops->ndo_bridge_setlink)

Same usage of MASTER here.

Thanks,
- KK