netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/2] macsec: linkstate fixes
@ 2018-10-28  8:33 Sabrina Dubroca
  2018-10-28  8:33 ` [PATCH net 1/2] macsec: update operstate when lower device changes Sabrina Dubroca
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Sabrina Dubroca @ 2018-10-28  8:33 UTC (permalink / raw)
  To: netdev; +Cc: Sabrina Dubroca, Radu Rendec, Patrick Talbert

This series fixes issues with handling administrative and operstate of
macsec devices.

Radu Rendec proposed another version of the first patch [0] but I'd
rather not follow the behavior of vlan devices, going with macvlan
does instead. Patrick Talbert also reported the same issue to me.

The second patch is a follow-up. The restriction on setting the device
up is a bit unreasonable, and operstate provides the information we
need in this case.

[0] https://patchwork.ozlabs.org/patch/971374/

Sabrina Dubroca (2):
  macsec: update operstate when lower device changes
  macsec: let the administrator set UP state even if lowerdev is down

 drivers/net/macsec.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

-- 
2.19.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net 1/2] macsec: update operstate when lower device changes
  2018-10-28  8:33 [PATCH net 0/2] macsec: linkstate fixes Sabrina Dubroca
@ 2018-10-28  8:33 ` Sabrina Dubroca
  2018-10-28  8:33 ` [PATCH net 2/2] macsec: let the administrator set UP state even if lowerdev is down Sabrina Dubroca
  2018-10-29  2:26 ` [PATCH net 0/2] macsec: linkstate fixes David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Sabrina Dubroca @ 2018-10-28  8:33 UTC (permalink / raw)
  To: netdev; +Cc: Sabrina Dubroca, Radu Rendec, Patrick Talbert

Like all other virtual devices (macvlan, vlan), the operstate of a
macsec device should match the state of its lower device. This is done
by calling netif_stacked_transfer_operstate from its netdevice notifier.

We also need to call netif_stacked_transfer_operstate when a new macsec
device is created, so that its operstate is set properly. This is only
relevant when we try to bring the device up directly when we create it.

Radu Rendec proposed a similar patch, inspired from the 802.1q driver,
that included changing the administrative state of the macsec device,
instead of just the operstate. This version is similar to what the
macvlan driver does, and updates only the operstate.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Reported-by: Radu Rendec <radu.rendec@gmail.com>
Reported-by: Patrick Talbert <ptalbert@redhat.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 4bb90b6867a2..6195b8edafc0 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3306,6 +3306,9 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
 	if (err < 0)
 		goto del_dev;
 
+	netif_stacked_transfer_operstate(real_dev, dev);
+	linkwatch_fire_event(dev);
+
 	macsec_generation++;
 
 	return 0;
@@ -3490,6 +3493,20 @@ static int macsec_notify(struct notifier_block *this, unsigned long event,
 		return NOTIFY_DONE;
 
 	switch (event) {
+	case NETDEV_DOWN:
+	case NETDEV_UP:
+	case NETDEV_CHANGE: {
+		struct macsec_dev *m, *n;
+		struct macsec_rxh_data *rxd;
+
+		rxd = macsec_data_rtnl(real_dev);
+		list_for_each_entry_safe(m, n, &rxd->secys, secys) {
+			struct net_device *dev = m->secy.netdev;
+
+			netif_stacked_transfer_operstate(real_dev, dev);
+		}
+		break;
+	}
 	case NETDEV_UNREGISTER: {
 		struct macsec_dev *m, *n;
 		struct macsec_rxh_data *rxd;
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH net 2/2] macsec: let the administrator set UP state even if lowerdev is down
  2018-10-28  8:33 [PATCH net 0/2] macsec: linkstate fixes Sabrina Dubroca
  2018-10-28  8:33 ` [PATCH net 1/2] macsec: update operstate when lower device changes Sabrina Dubroca
@ 2018-10-28  8:33 ` Sabrina Dubroca
  2018-10-29  2:26 ` [PATCH net 0/2] macsec: linkstate fixes David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Sabrina Dubroca @ 2018-10-28  8:33 UTC (permalink / raw)
  To: netdev; +Cc: Sabrina Dubroca, Radu Rendec, Patrick Talbert

Currently, the kernel doesn't let the administrator set a macsec device
up unless its lower device is currently up. This is inconsistent, as a
macsec device that is up won't automatically go down when its lower
device goes down.

Now that linkstate propagation works, there's really no reason for this
limitation, so let's remove it.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Reported-by: Radu Rendec <radu.rendec@gmail.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 6195b8edafc0..64a982563d59 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2812,9 +2812,6 @@ static int macsec_dev_open(struct net_device *dev)
 	struct net_device *real_dev = macsec->real_dev;
 	int err;
 
-	if (!(real_dev->flags & IFF_UP))
-		return -ENETDOWN;
-
 	err = dev_uc_add(real_dev, dev->dev_addr);
 	if (err < 0)
 		return err;
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net 0/2] macsec: linkstate fixes
  2018-10-28  8:33 [PATCH net 0/2] macsec: linkstate fixes Sabrina Dubroca
  2018-10-28  8:33 ` [PATCH net 1/2] macsec: update operstate when lower device changes Sabrina Dubroca
  2018-10-28  8:33 ` [PATCH net 2/2] macsec: let the administrator set UP state even if lowerdev is down Sabrina Dubroca
@ 2018-10-29  2:26 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2018-10-29  2:26 UTC (permalink / raw)
  To: sd; +Cc: netdev, radu.rendec, ptalbert

From: Sabrina Dubroca <sd@queasysnail.net>
Date: Sun, 28 Oct 2018 09:33:08 +0100

> This series fixes issues with handling administrative and operstate of
> macsec devices.
> 
> Radu Rendec proposed another version of the first patch [0] but I'd
> rather not follow the behavior of vlan devices, going with macvlan
> does instead. Patrick Talbert also reported the same issue to me.
> 
> The second patch is a follow-up. The restriction on setting the device
> up is a bit unreasonable, and operstate provides the information we
> need in this case.
> 
> [0] https://patchwork.ozlabs.org/patch/971374/

Series applied, thank you.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-10-29 11:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-28  8:33 [PATCH net 0/2] macsec: linkstate fixes Sabrina Dubroca
2018-10-28  8:33 ` [PATCH net 1/2] macsec: update operstate when lower device changes Sabrina Dubroca
2018-10-28  8:33 ` [PATCH net 2/2] macsec: let the administrator set UP state even if lowerdev is down Sabrina Dubroca
2018-10-29  2:26 ` [PATCH net 0/2] macsec: linkstate fixes David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).