Re: [PATCH net-next v8 2/4] net: Introduce generic failover module

2018-04-28 Thread Dan Carpenter 
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

Re: [PATCH net-next v8 2/4] net: Introduce generic failover module

2018-04-27 Thread kbuild test robot 
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, 
_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,

[PATCH net-next v8 2/4] net: Introduce generic failover module

2018-04-25 Thread Sridhar Samudrala 
This provides a generic interface for paravirtual drivers to listen
for netdev register/unregister/link change events from pci ethernet
devices with the same MAC and takeover their datapath. The notifier and
event handling code is based on the existing netvsc implementation.

It exposes 2 sets of interfaces to the paravirtual drivers.
1. For paravirtual drivers like virtio_net that use 3 netdev model, the
   the failover module provides interfaces to create/destroy additional
   master netdev and all the slave events are managed internally.
net_failover_create()
net_failover_destroy()
   A failover netdev is created that acts a master device and controls 2
   slave devices. The original virtio_net netdev is registered as 'standby'
   netdev and a passthru/vf device with the same MAC gets registered as
   'primary' netdev. Both 'standby' and 'primary' netdevs are associated
   with the same 'pci' device.  The user accesses the network interface via
   'failover' netdev. The 'failover' netdev chooses 'primary' netdev as
   default for transmits when it is available with link up and running.
2. For existing netvsc driver that uses 2 netdev model, no master netdev
   is created. The paravirtual driver registers each instance of netvsc
   as a 'failover' netdev  along with a set of ops to manage the slave
   events. There is no 'standby' netdev in this model. A passthru/vf device
   with the same MAC gets registered as 'primary' netdev.
net_failover_register()
net_failover_unregister()

Signed-off-by: Sridhar Samudrala 
---
 include/linux/netdevice.h  |  16 +
 include/net/net_failover.h |  94 +
 net/Kconfig|  18 +
 net/core/Makefile  |   1 +
 net/core/net_failover.c| 892 +
 5 files changed, 1021 insertions(+)
 create mode 100644 include/net/net_failover.h
 create mode 100644 net/core/net_failover.c

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 14e0777ffcfb..b04dbf7dcf1b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1401,6 +1401,8 @@ struct net_device_ops {
  * entity (i.e. the master device for bridged veth)
  * @IFF_MACSEC: device is a MACsec device
  * @IFF_NO_RX_HANDLER: device doesn't support the rx_handler hook
+ * @IFF_FAILOVER: device is a failover master device
+ * @IFF_FAILOVER_SLAVE: device is lower dev of a failover master device
  */
 enum netdev_priv_flags {
IFF_802_1Q_VLAN = 1<<0,
@@ -1430,6 +1432,8 @@ enum netdev_priv_flags {
IFF_PHONY_HEADROOM  = 1<<24,
IFF_MACSEC  = 1<<25,
IFF_NO_RX_HANDLER   = 1<<26,
+   IFF_FAILOVER= 1<<27,
+   IFF_FAILOVER_SLAVE  = 1<<28,
 };
 
 #define IFF_802_1Q_VLANIFF_802_1Q_VLAN
@@ -1458,6 +1462,8 @@ enum netdev_priv_flags {
 #define IFF_RXFH_CONFIGUREDIFF_RXFH_CONFIGURED
 #define IFF_MACSEC IFF_MACSEC
 #define IFF_NO_RX_HANDLER  IFF_NO_RX_HANDLER
+#define IFF_FAILOVER   IFF_FAILOVER
+#define IFF_FAILOVER_SLAVE IFF_FAILOVER_SLAVE
 
 /**
  * struct net_device - The DEVICE structure.
@@ -4308,6 +4314,16 @@ static inline bool netif_is_rxfh_configured(const struct 
net_device *dev)
return dev->priv_flags & IFF_RXFH_CONFIGURED;
 }
 
+static inline bool netif_is_failover(const struct net_device *dev)
+{
+   return dev->priv_flags & IFF_FAILOVER;
+}
+
+static inline bool netif_is_failover_slave(const struct net_device *dev)
+{
+   return dev->priv_flags & IFF_FAILOVER_SLAVE;
+}
+
 /* This device needs to keep skb dst for qdisc enqueue or ndo_start_xmit() */
 static inline void netif_keep_dst(struct net_device *dev)
 {
diff --git a/include/net/net_failover.h b/include/net/net_failover.h
new file mode 100644
index ..8d431685634a
--- /dev/null
+++ b/include/net/net_failover.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018, Intel Corporation. */
+
+#ifndef _NET_FAILOVER_H
+#define _NET_FAILOVER_H
+
+#include 
+
+struct net_failover_ops {
+   int (*slave_register)(struct net_device *slave_dev,
+ struct net_device *failover_dev);
+   int (*slave_unregister)(struct net_device *slave_dev,
+   struct net_device *failover_dev);
+   int (*slave_link_change)(struct net_device *slave_dev,
+struct net_device *failover_dev);
+};
+
+struct net_failover {
+   struct list_head list;
+   struct net_device __rcu *failover_dev;
+   struct net_failover_ops __rcu *ops;
+};
+
+/* failover state */
+struct net_failover_info {
+   /* primary netdev with same MAC */
+   struct net_device __rcu *primary_dev;
+
+   /* standby netdev */
+   struct net_device __rcu *standby_dev;
+
+   /* primary