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
[PATCH net-next v8 2/4] net: Introduce generic failover module
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 netdev stats */ + struc