On Mon, May 07, 2018 at 05:24:27PM -0700, Samudrala, Sridhar wrote: > > > On 5/7/2018 4:53 PM, Stephen Hemminger wrote: > > On Mon, 7 May 2018 15:10:44 -0700 > > Sridhar Samudrala <sridhar.samudrala@xxxxxxxxx> wrote: > > > > > +static struct net_device *net_failover_get_bymac(u8 *mac, > > > + struct net_failover_ops **ops) > > > +{ > > > + struct net_device *failover_dev; > > > + struct net_failover *failover; > > > + > > > + spin_lock(&net_failover_lock); > > > + list_for_each_entry(failover, &net_failover_list, list) { > > > + failover_dev = rtnl_dereference(failover->failover_dev); > > > + if (ether_addr_equal(failover_dev->perm_addr, mac)) { > > > + *ops = rtnl_dereference(failover->ops); > > > + spin_unlock(&net_failover_lock); > > > + return failover_dev; > > > + } > > > + } > > > + spin_unlock(&net_failover_lock); > > > + return NULL; > > > +} > > This is broken if non-ethernet devices such as Infiniband are present. > > There is check to make sure that a slave and failover devices are of the same type in > net_failover_slave_register() > > failover_dev = net_failover_get_bymac(slave_dev->perm_addr, &nfo_ops); > if (!failover_dev) > goto done; > > if (failover_dev->type != slave_dev->type) > goto done; > > Do you think this is not good enough? I had an explicit check for ARPHRD_ETHER in > earlier patchsets, but removed it based on Jiri's comment. Right but how is ether_addr_equal supposed to work if types are identical but not ethernet? This can also benefit from a comment referring to the check in net_failover_slave_register. -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization