Re: [PATCH net-next v8 2/4] net: Introduce generic failover module
Hi Sridhar,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url:
https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180427-183842
smatch warnings:
net/core/net_failover.c:229 net_failover_change_mtu() error: we previously
assumed 'primary_dev' could be null (see line 219)
net/core/net_failover.c:279 net_failover_vlan_rx_add_vid() error: we previously
assumed 'primary_dev' could be null (see line 269)
#
https://github.com/0day-ci/linux/commit/5a5f2e3efcb699867db79543dfebe764927b9c93
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 5a5f2e3efcb699867db79543dfebe764927b9c93
vim +/primary_dev +229 net/core/net_failover.c
5a5f2e3e Sridhar Samudrala 2018-04-25 211
5a5f2e3e Sridhar Samudrala 2018-04-25 212 static int
net_failover_change_mtu(struct net_device *dev, int new_mtu)
5a5f2e3e Sridhar Samudrala 2018-04-25 213 {
5a5f2e3e Sridhar Samudrala 2018-04-25 214 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 215 struct net_device *primary_dev,
*standby_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 216 int ret = 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 217
5a5f2e3e Sridhar Samudrala 2018-04-25 218 primary_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 @219 if (primary_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 220 ret =
dev_set_mtu(primary_dev, new_mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 221 if (ret)
5a5f2e3e Sridhar Samudrala 2018-04-25 222 return ret;
5a5f2e3e Sridhar Samudrala 2018-04-25 223 }
5a5f2e3e Sridhar Samudrala 2018-04-25 224
5a5f2e3e Sridhar Samudrala 2018-04-25 225 standby_dev =
rcu_dereference(nfo_info->standby_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 226 if (standby_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 227 ret =
dev_set_mtu(standby_dev, new_mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 228 if (ret) {
5a5f2e3e Sridhar Samudrala 2018-04-25 @229
dev_set_mtu(primary_dev, dev->mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 230 return ret;
5a5f2e3e Sridhar Samudrala 2018-04-25 231 }
5a5f2e3e Sridhar Samudrala 2018-04-25 232 }
5a5f2e3e Sridhar Samudrala 2018-04-25 233
5a5f2e3e Sridhar Samudrala 2018-04-25 234 dev->mtu = new_mtu;
5a5f2e3e Sridhar Samudrala 2018-04-25 235
5a5f2e3e Sridhar Samudrala 2018-04-25 236 return 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 237 }
5a5f2e3e Sridhar Samudrala 2018-04-25 238
5a5f2e3e Sridhar Samudrala 2018-04-25 239 static void
net_failover_set_rx_mode(struct net_device *dev)
5a5f2e3e Sridhar Samudrala 2018-04-25 240 {
5a5f2e3e Sridhar Samudrala 2018-04-25 241 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 242 struct net_device *slave_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 243
5a5f2e3e Sridhar Samudrala 2018-04-25 244 rcu_read_lock();
5a5f2e3e Sridhar Samudrala 2018-04-25 245
5a5f2e3e Sridhar Samudrala 2018-04-25 246 slave_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 247 if (slave_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 248
dev_uc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 249
dev_mc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 250 }
5a5f2e3e Sridhar Samudrala 2018-04-25 251
5a5f2e3e Sridhar Samudrala 2018-04-25 252 slave_dev =
rcu_dereference(nfo_info->standby_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 253 if (slave_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 254
dev_uc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 255
dev_mc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 256 }
5a5f2e3e Sridhar Samudrala 2018-04-25 257
5a5f2e3e Sridhar Samudrala 2018-04-25 258 rcu_read_unlock();
5a5f2e3e Sridhar Samudrala 2018-04-25 259 }
5a5f2e3e Sridhar Samudrala 2018-04-25 260
5a5f2e3e Sridhar Samudrala 2018-04-25 261 static int
net_failover_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
5a5f2e3e Sridhar Samudrala 2018-04-25 262
u16 vid)
5a5f2e3e Sridhar Samudrala 2018-04-25 263 {
5a5f2e3e Sridhar Samudrala 2018-04-25 264 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 265 struct net_device *primary_dev,
*standby_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 266 int ret = 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 267
5a5f2e3e Sridhar Samudrala 2018-04-25 268 primary_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar Samudral
Re: [PATCH net-next v8 2/4] net: Introduce generic failover module
Hi Sridhar,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url:
https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180427-183842
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/core/net_failover.c:544:39: sparse: incorrect type in argument 1
>> (different address spaces) @@expected struct net_device *dev @@got
>> struct net_devicestruct net_device *dev @@
net/core/net_failover.c:544:39:expected struct net_device *dev
net/core/net_failover.c:544:39:got struct net_device [noderef]
*standby_dev
net/core/net_failover.c:547:39: sparse: incorrect type in argument 1
(different address spaces) @@expected struct net_device *dev @@got
struct net_devicestruct net_device *dev @@
net/core/net_failover.c:547:39:expected struct net_device *dev
net/core/net_failover.c:547:39:got struct net_device [noderef]
*primary_dev
>> net/core/net_failover.c:112:12: sparse: context imbalance in
>> 'net_failover_select_queue' - wrong count at exit
vim +544 net/core/net_failover.c
446
447 static int net_failover_slave_register(struct net_device *slave_dev)
448 {
449 struct net_failover_info *nfo_info;
450 struct net_failover_ops *nfo_ops;
451 struct net_device *failover_dev;
452 bool slave_is_standby;
453 u32 orig_mtu;
454 int err;
455
456 ASSERT_RTNL();
457
458 failover_dev = net_failover_get_bymac(slave_dev->perm_addr,
&nfo_ops);
459 if (!failover_dev)
460 goto done;
461
462 if (failover_dev->type != slave_dev->type)
463 goto done;
464
465 if (nfo_ops && nfo_ops->slave_register)
466 return nfo_ops->slave_register(slave_dev, failover_dev);
467
468 nfo_info = netdev_priv(failover_dev);
469 slave_is_standby = (slave_dev->dev.parent ==
failover_dev->dev.parent);
470 if (slave_is_standby ? rtnl_dereference(nfo_info->standby_dev) :
471 rtnl_dereference(nfo_info->primary_dev)) {
472 netdev_err(failover_dev, "%s attempting to register as
slave dev when %s already present\n",
473 slave_dev->name,
474 slave_is_standby ? "standby" : "primary");
475 goto done;
476 }
477
478 /* We want to allow only a direct attached VF device as a
primary
479 * netdev. As there is no easy way to check for a VF device,
restrict
480 * this to a pci device.
481 */
482 if (!slave_is_standby && (!slave_dev->dev.parent ||
483!dev_is_pci(slave_dev->dev.parent)))
484 goto done;
485
486 if (failover_dev->features & NETIF_F_VLAN_CHALLENGED &&
487 vlan_uses_dev(failover_dev)) {
488 netdev_err(failover_dev, "Device %s is VLAN challenged
and failover device has VLAN set up\n",
489 failover_dev->name);
490 goto done;
491 }
492
493 /* Align MTU of slave with failover dev */
494 orig_mtu = slave_dev->mtu;
495 err = dev_set_mtu(slave_dev, failover_dev->mtu);
496 if (err) {
497 netdev_err(failover_dev, "unable to change mtu of %s to
%u register failed\n",
498 slave_dev->name, failover_dev->mtu);
499 goto done;
500 }
501
502 dev_hold(slave_dev);
503
504 if (netif_running(failover_dev)) {
505 err = dev_open(slave_dev);
506 if (err && (err != -EBUSY)) {
507 netdev_err(failover_dev, "Opening slave %s
failed err:%d\n",
508 slave_dev->name, err);
509 goto err_dev_open;
510 }
511 }
512
513 netif_addr_lock_bh(failover_dev);
514 dev_uc_sync_multiple(slave_dev, failover_dev);
515 dev_uc_sync_multiple(slave_dev, failover_dev);
516 netif_addr_unlock_bh(failover_dev);
517
518 err = vlan_vids_add_by_dev(slave_dev, failover_dev);
519 if (err) {
520 netdev_err(failover_dev, "Failed to add vlan ids to
device %s err:%d\n",
521 slave_dev->name, err);
522 goto err_vlan_add;
523 }
524
525 err = netdev_rx_handler_register(slave_dev,
net_fa
