[PATCH net-next 1/3] net: Do not unconditionally turn on carrier when clearing protodown

[Ido Schimmel <idosch@nvidia.com>]

The protodown functionality is currently supported by vxlan and macvlan
and it allows to keep an interface down by turning off its carrier:

 # ip link add name dummy1 up type dummy
 # ip link add name macvlan1 up link dummy1 type macvlan mode bridge
 # ip link set dev macvlan1 protodown on
 $ ip -br link show dev macvlan1
 macvlan1@dummy1  DOWN           0a:5c:a3:05:c7:86 <NO-CARRIER,BROADCAST,MULTICAST,UP>

When protodown is cleared, the core unconditionally enables carrier on
the interface:

 # ip link set dev macvlan1 protodown off
 $ ip -br link show dev macvlan1
 macvlan1@dummy1  UP             0a:5c:a3:05:c7:86 <BROADCAST,MULTICAST,UP,LOWER_UP>

This is wrong as it means that a macvlan can end up with a carrier when
its lower device does not have a carrier:

 # ip link set dev dummy1 carrier off
 $ ip -br link show dev macvlan1
 macvlan1@dummy1  LOWERLAYERDOWN 0a:5c:a3:05:c7:86 <NO-CARRIER,BROADCAST,MULTICAST,UP>
 # ip link set dev macvlan1 protodown on
 # ip link set dev macvlan1 protodown off
 $ ip -br link show dev macvlan1
 macvlan1@dummy1  UP             0a:5c:a3:05:c7:86 <BROADCAST,MULTICAST,UP,LOWER_UP>

Solve this by adding a new NDO that allows different drivers to react
differently to protodown being cleared. Change the core to invoke the
NDO and implement it for vxlan and macvlan. Maintain the current
behavior for the former, but for macvlan implement the NDO by
transferring the operational state from its lower device.

Output with the patch:

 # ip link add name dummy1 up type dummy
 # ip link add name macvlan1 up link dummy1 type macvlan mode bridge
 # ip link set dev dummy1 carrier off
 $ ip -br link show dev macvlan1
 macvlan1@dummy1  LOWERLAYERDOWN 0a:5c:a3:05:c7:86 <NO-CARRIER,BROADCAST,MULTICAST,UP>
 # ip link set dev macvlan1 protodown on
 # ip link set dev macvlan1 protodown off
 $ ip -br link show dev macvlan1
 macvlan1@dummy1  LOWERLAYERDOWN 0a:5c:a3:05:c7:86 <NO-CARRIER,BROADCAST,MULTICAST,UP>
 # ip link set dev dummy1 carrier on
 $ ip -br link show dev macvlan1
 macvlan1@dummy1  UP             0a:5c:a3:05:c7:86 <BROADCAST,MULTICAST,UP,LOWER_UP>
 # ip link set dev macvlan1 protodown on
 # ip link set dev macvlan1 protodown off
 $ ip -br link show dev macvlan1
 macvlan1@dummy1  UP             0a:5c:a3:05:c7:86 <BROADCAST,MULTICAST,UP,LOWER_UP>

Make the NDO only about clearing protodown to avoid suspicion that this
is about re-introducing the NDO that was removed by commit 2106efda785b
("net: remove .ndo_change_proto_down").

Remove net_device::change_proto_down since we can now use the presence
of the new NDO as an indication that changing protodown is supported.

Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 Documentation/networking/net_cachelines/net_device.rst | 1 -
 drivers/net/macvlan.c                                  | 7 ++++++-
 drivers/net/vxlan/vxlan_core.c                         | 8 +++++++-
 include/linux/netdevice.h                              | 6 ++++--
 net/core/dev.c                                         | 4 ++--
 net/core/rtnetlink.c                                   | 2 +-
 6 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
index 1c19bb7705df..267af4957b0d 100644
--- a/Documentation/networking/net_cachelines/net_device.rst
+++ b/Documentation/networking/net_cachelines/net_device.rst
@@ -167,7 +167,6 @@ struct lock_class_key*              qdisc_tx_busylock
 bool                                proto_down
 unsigned:1                          wol_enabled
 unsigned_long:1                     see_all_hwtstamp_requests
-unsigned_long:1                     change_proto_down
 unsigned_long:1                     netns_immutable
 unsigned_long:1                     fcoe_mtu
 struct list_head                    net_notifier_list
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index c40fa331836b..61effa295c49 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -904,6 +904,11 @@ static int macvlan_hwtstamp_set(struct net_device *dev,
 	return generic_hwtstamp_set_lower(real_dev, cfg, extack);
 }
 
+static void macvlan_clear_proto_down(struct net_device *dev)
+{
+	netif_stacked_transfer_operstate(macvlan_dev_real_dev(dev), dev);
+}
+
 /*
  * macvlan network devices have devices nesting below it and are a special
  * "super class" of normal network devices; split their locks off into a
@@ -1211,6 +1216,7 @@ static const struct net_device_ops macvlan_netdev_ops = {
 	.ndo_features_check	= passthru_features_check,
 	.ndo_hwtstamp_get	= macvlan_hwtstamp_get,
 	.ndo_hwtstamp_set	= macvlan_hwtstamp_set,
+	.ndo_clear_proto_down	= macvlan_clear_proto_down,
 };
 
 static void macvlan_dev_free(struct net_device *dev)
@@ -1230,7 +1236,6 @@ void macvlan_common_setup(struct net_device *dev)
 	dev->priv_flags	       &= ~IFF_TX_SKB_SHARING;
 	netif_keep_dst(dev);
 	dev->priv_flags	       |= IFF_UNICAST_FLT;
-	dev->change_proto_down	= true;
 	dev->netdev_ops		= &macvlan_netdev_ops;
 	dev->needs_free_netdev	= true;
 	dev->priv_destructor	= macvlan_dev_free;
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index e88798497503..a3ed9fbd60a6 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -3271,6 +3271,11 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
 	return 0;
 }
 
+static void vxlan_clear_proto_down(struct net_device *dev)
+{
+	netif_carrier_on(dev);
+}
+
 static const struct net_device_ops vxlan_netdev_ether_ops = {
 	.ndo_init		= vxlan_init,
 	.ndo_uninit		= vxlan_uninit,
@@ -3292,6 +3297,7 @@ static const struct net_device_ops vxlan_netdev_ether_ops = {
 	.ndo_mdb_dump		= vxlan_mdb_dump,
 	.ndo_mdb_get		= vxlan_mdb_get,
 	.ndo_fill_metadata_dst	= vxlan_fill_metadata_dst,
+	.ndo_clear_proto_down	= vxlan_clear_proto_down,
 };
 
 static const struct net_device_ops vxlan_netdev_raw_ops = {
@@ -3302,6 +3308,7 @@ static const struct net_device_ops vxlan_netdev_raw_ops = {
 	.ndo_start_xmit		= vxlan_xmit,
 	.ndo_change_mtu		= vxlan_change_mtu,
 	.ndo_fill_metadata_dst	= vxlan_fill_metadata_dst,
+	.ndo_clear_proto_down	= vxlan_clear_proto_down,
 };
 
 /* Info for udev, that this is a virtual tunnel endpoint */
@@ -3368,7 +3375,6 @@ static void vxlan_setup(struct net_device *dev)
 
 	netif_keep_dst(dev);
 	dev->priv_flags |= IFF_NO_QUEUE;
-	dev->change_proto_down = true;
 	dev->lltx = true;
 
 	/* MTU range: 68 - 65535 */
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0e1e581efc5a..3757dd6604b8 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1432,6 +1432,9 @@ struct netdev_net_notifier {
  *			   struct kernel_hwtstamp_config *kernel_config,
  *			   struct netlink_ext_ack *extack);
  *	Change the hardware timestamping parameters for NIC device.
+ *
+ * void (*ndo_clear_proto_down)(struct net_device *dev);
+ *	Enable carrier after dev->proto_down was cleared.
  */
 struct net_device_ops {
 	int			(*ndo_init)(struct net_device *dev);
@@ -1683,6 +1686,7 @@ struct net_device_ops {
 	int			(*ndo_hwtstamp_set)(struct net_device *dev,
 						    struct kernel_hwtstamp_config *kernel_config,
 						    struct netlink_ext_ack *extack);
+	void			(*ndo_clear_proto_down)(struct net_device *dev);
 
 #if IS_ENABLED(CONFIG_NET_SHAPER)
 	/**
@@ -2062,7 +2066,6 @@ enum netdev_reg_state {
  *			ndo_hwtstamp_set() for all timestamp requests
  *			regardless of source, even if those aren't
  *			HWTSTAMP_SOURCE_NETDEV
- *	@change_proto_down: device supports setting carrier via IFLA_PROTO_DOWN
  *	@netns_immutable: interface can't change network namespaces
  *	@fcoe_mtu:	device supports maximum FCoE MTU, 2158 bytes
  *
@@ -2476,7 +2479,6 @@ struct net_device {
 
 	/* priv_flags_slow, ungrouped to save space */
 	unsigned long		see_all_hwtstamp_requests:1;
-	unsigned long		change_proto_down:1;
 	unsigned long		netns_immutable:1;
 	unsigned long		fcoe_mtu:1;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 06c195906231..ff66762592e4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10143,14 +10143,14 @@ EXPORT_SYMBOL(netdev_port_same_parent_id);
 
 int netif_change_proto_down(struct net_device *dev, bool proto_down)
 {
-	if (!dev->change_proto_down)
+	if (!dev->netdev_ops->ndo_clear_proto_down)
 		return -EOPNOTSUPP;
 	if (!netif_device_present(dev))
 		return -ENODEV;
 	if (proto_down)
 		netif_carrier_off(dev);
 	else
-		netif_carrier_on(dev);
+		dev->netdev_ops->ndo_clear_proto_down(dev);
 	WRITE_ONCE(dev->proto_down, proto_down);
 	return 0;
 }
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b613bb6e07df..30951e5a9555 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3009,7 +3009,7 @@ static int do_set_proto_down(struct net_device *dev,
 	bool proto_down;
 	int err;
 
-	if (!dev->change_proto_down) {
+	if (!dev->netdev_ops->ndo_clear_proto_down) {
 		NL_SET_ERR_MSG(extack,  "Protodown not supported by device");
 		return -EOPNOTSUPP;
 	}
-- 
2.53.0