All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] bonding: Avoid adding slave devices to inactive bonding
@ 2021-07-29  1:46 zhudi (J)
  0 siblings, 0 replies; 5+ messages in thread
From: zhudi (J) @ 2021-07-29  1:46 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: vfalico, kuba, davem, netdev, Chenxiang (EulerOS)

> >> zhudi <zhudi21@huawei.com> wrote:
> >>
> >> >We need to refuse to add slave devices to the bonding which does
> >> >not set IFF_UP flag, otherwise some problems will be caused(such as
> >> >bond_set_carrier() will not sync carrier state to upper net device).
> >> >The ifenslave command can prevent such use case, but through the
> sysfs
> >> >interface, slave devices can still be added regardless of whether
> >> >the bonding is set with IFF_UP flag or not.
> >>
> >> 	What specifically happens in the carrier state issue you
> >> mention?  Are there other specific issues?
> >
> >yes, The following steps can cause problems:
> >	1)bond0 is down
> >	2) ip link add link bond0 name ipvlan type ipvlan mode l2
> >	3) echo +enp2s7 >/sys/class/net/bond0/bonding/slaves
> >	4) ip link set bond0 up
> >
> >	After these steps, use ip link command, we found ipvlan has NO-
> CARRIER:
> >		ipvlan@bond0: <NO-CARRIER,
> BROADCAST,MULTICAST,UP,M-DOWN> mtu 1500 qdisc noqueue state>
> >
> >	This is because,  bond_enslave()->bond_set_carrier()-
> >netif_carrier_on()...->netdev_state_change()
> >will not sync carrier state to ipvlan because the IFF_UP  flag is not set.
> 
> 	Correct, it's not supposed to at the time bond_enslave is called
> (because the bond is down at that time).  Later, when the bond is set
> link up, NETDEV_UP and NETDEV_CHANGE netdevice events are issued, and
> devices logically stacked above the bond can receive a notification at
> that time.  This is what, e.g., VLAN does, in vlan_device_event,
> ultimately by calling netif_stacked_transfer_operstate if I'm following
> the code correctly.

	In fact, netif_carrier_on() has been called in bond_enslave(), so when we
set bond link up, only NETDEV_UP event will be issued(dev_change_flags()-> __dev_notify_flags()). 
Unlike VLAN device,  ipvlan device does not handle NETDEV_UP event.

	The focus now is the ipvlan device needs to handle  NETDEV_UP event.

> 
> 	Looking at ipvlan, it does register for netdevice events
> (ipvlan_device_event) but doesn't appear to handle events for the device
> it is linked to.
> 
> >> 	As far as I can recall, adding interfaces to the bond while the
> >> bond is down has worked for a very long time, so I'm concerned that
> >> disabling that functionality will have impact on existing
> >> configurations.
> >>
> >> 	Also, to the best of my knowledge, the currently packaged
> >> ifenslave programs are scripts that utilize the sysfs interface.  I'm
> >> unaware of current usage of the old C ifenslave program (removed from
> >> the kernel source in 2013), although the kernel code should still
> >> support it.
> >
> >	We still use old ifenslave command and it  does only allow to add
> slave to bonding with up state, code is as follows:
> >
> >	/* check if master is up; if not then fail any operation */
> >	if (!(master_flags.ifr_flags & IFF_UP)) {
> >		fprintf(stderr,
> >			"Illegal operation; the specified master interface "
> >			"'%s' is not up.\n",
> >			master_ifname);
> >		res = 1;
> >		goto out;
> >	}
> >
> >	If so, the behavior of the new tool is inconsistent with that of the old
> tool.
> 
> 	This restriction in ifenslave.c dates to 15-20 years ago, when
> (as I recall) the master->slave infrastructure in the kernel could not
> handle such linking while the interface was down.  Another side effect
> at the time was that setting a bond down would release all slaves.
> 	
> 	In any event, the old ifenslave.c is not the standard by which
> we set the behavior of the driver.  It is obsolete and no longer
> developed or distributed upstream.

	I didn't track the change history of the bonding,  thank you for your explanation.

> 
> 	-J
> 
> >> >So we introduce a new BOND_OPTFLAG_IFUP flag to avoid adding slave
> >> >devices to inactive bonding.
> >> >
> >> >Signed-off-by: zhudi <zhudi21@huawei.com>
> >> >---
> >> > drivers/net/bonding/bond_options.c | 4 +++-
> >> > include/net/bond_options.h         | 4 +++-
> >> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >> >
> >> >diff --git a/drivers/net/bonding/bond_options.c
> >> b/drivers/net/bonding/bond_options.c
> >> >index 0cf25de6f46d..6d2f44b3528d 100644
> >> >--- a/drivers/net/bonding/bond_options.c
> >> >+++ b/drivers/net/bonding/bond_options.c
> >> >@@ -387,7 +387,7 @@ static const struct bond_option
> >> bond_opts[BOND_OPT_LAST] = {
> >> > 		.id = BOND_OPT_SLAVES,
> >> > 		.name = "slaves",
> >> > 		.desc = "Slave membership management",
> >> >-		.flags = BOND_OPTFLAG_RAWVAL,
> >> >+		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFUP,
> >> > 		.set = bond_option_slaves_set
> >> > 	},
> >> > 	[BOND_OPT_TLB_DYNAMIC_LB] = {
> >> >@@ -583,6 +583,8 @@ static int bond_opt_check_deps(struct bonding
> >> *bond,
> >> > 		return -ENOTEMPTY;
> >> > 	if ((opt->flags & BOND_OPTFLAG_IFDOWN) && (bond->dev->flags &
> >> IFF_UP))
> >> > 		return -EBUSY;
> >> >+	if ((opt->flags & BOND_OPTFLAG_IFUP) && !(bond->dev->flags &
> >> IFF_UP))
> >> >+		return -EPERM;
> >> >
> >> > 	return 0;
> >> > }
> >> >diff --git a/include/net/bond_options.h b/include/net/bond_options.h
> >> >index 9d382f2f0bc5..742f5cc81adf 100644
> >> >--- a/include/net/bond_options.h
> >> >+++ b/include/net/bond_options.h
> >> >@@ -15,11 +15,13 @@
> >> >  * BOND_OPTFLAG_NOSLAVES - check if the bond device is empty
> before
> >> setting
> >> >  * BOND_OPTFLAG_IFDOWN - check if the bond device is down before
> >> setting
> >> >  * BOND_OPTFLAG_RAWVAL - the option parses the value itself
> >> >+ * BOND_OPTFLAG_IFUP - check if the bond device is up before setting
> >> >  */
> >> > enum {
> >> > 	BOND_OPTFLAG_NOSLAVES	= BIT(0),
> >> > 	BOND_OPTFLAG_IFDOWN	= BIT(1),
> >> >-	BOND_OPTFLAG_RAWVAL	= BIT(2)
> >> >+	BOND_OPTFLAG_RAWVAL	= BIT(2),
> >> >+	BOND_OPTFLAG_IFUP	= BIT(3)
> >> > };
> >> >
> >> > /* Value type flags:
> >> >--
> >> >2.27.0
> >> >
> 
> ---
> 	-Jay Vosburgh, jay.vosburgh@canonical.com

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

* Re: [PATCH] bonding: Avoid adding slave devices to inactive bonding
  2021-07-28  6:51 zhudi (J)
@ 2021-07-28 19:57 ` Jay Vosburgh
  0 siblings, 0 replies; 5+ messages in thread
From: Jay Vosburgh @ 2021-07-28 19:57 UTC (permalink / raw)
  To: zhudi (J); +Cc: vfalico, kuba, davem, netdev, Chenxiang (EulerOS)

zhudi (J) <zhudi21@huawei.com> wrote:

>> zhudi <zhudi21@huawei.com> wrote:
>> 
>> >We need to refuse to add slave devices to the bonding which does
>> >not set IFF_UP flag, otherwise some problems will be caused(such as
>> >bond_set_carrier() will not sync carrier state to upper net device).
>> >The ifenslave command can prevent such use case, but through the sysfs
>> >interface, slave devices can still be added regardless of whether
>> >the bonding is set with IFF_UP flag or not.
>> 
>> 	What specifically happens in the carrier state issue you
>> mention?  Are there other specific issues?
>
>yes, The following steps can cause problems:
>	1)bond0 is down
>	2) ip link add link bond0 name ipvlan type ipvlan mode l2
>	3) echo +enp2s7 >/sys/class/net/bond0/bonding/slaves
>	4) ip link set bond0 up
>
>	After these steps, use ip link command, we found ipvlan has NO-CARRIER:
>		ipvlan@bond0: <NO-CARRIER, BROADCAST,MULTICAST,UP,M-DOWN> mtu 1500 qdisc noqueue state>
>
>	This is because,  bond_enslave()->bond_set_carrier()->netif_carrier_on()...->netdev_state_change() 
>will not sync carrier state to ipvlan because the IFF_UP  flag is not set.

	Correct, it's not supposed to at the time bond_enslave is called
(because the bond is down at that time).  Later, when the bond is set
link up, NETDEV_UP and NETDEV_CHANGE netdevice events are issued, and
devices logically stacked above the bond can receive a notification at
that time.  This is what, e.g., VLAN does, in vlan_device_event,
ultimately by calling netif_stacked_transfer_operstate if I'm following
the code correctly.

	Looking at ipvlan, it does register for netdevice events
(ipvlan_device_event) but doesn't appear to handle events for the device
it is linked to.

>> 	As far as I can recall, adding interfaces to the bond while the
>> bond is down has worked for a very long time, so I'm concerned that
>> disabling that functionality will have impact on existing
>> configurations.
>> 
>> 	Also, to the best of my knowledge, the currently packaged
>> ifenslave programs are scripts that utilize the sysfs interface.  I'm
>> unaware of current usage of the old C ifenslave program (removed from
>> the kernel source in 2013), although the kernel code should still
>> support it.
>
>	We still use old ifenslave command and it  does only allow to add slave to bonding with up state, code is as follows:
>
>	/* check if master is up; if not then fail any operation */
>	if (!(master_flags.ifr_flags & IFF_UP)) {
>		fprintf(stderr,
>			"Illegal operation; the specified master interface "
>			"'%s' is not up.\n",
>			master_ifname);
>		res = 1;
>		goto out;
>	}
>
>	If so, the behavior of the new tool is inconsistent with that of the old tool.

	This restriction in ifenslave.c dates to 15-20 years ago, when
(as I recall) the master->slave infrastructure in the kernel could not
handle such linking while the interface was down.  Another side effect
at the time was that setting a bond down would release all slaves.

	In any event, the old ifenslave.c is not the standard by which
we set the behavior of the driver.  It is obsolete and no longer
developed or distributed upstream.

	-J

>> >So we introduce a new BOND_OPTFLAG_IFUP flag to avoid adding slave
>> >devices to inactive bonding.
>> >
>> >Signed-off-by: zhudi <zhudi21@huawei.com>
>> >---
>> > drivers/net/bonding/bond_options.c | 4 +++-
>> > include/net/bond_options.h         | 4 +++-
>> > 2 files changed, 6 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/drivers/net/bonding/bond_options.c
>> b/drivers/net/bonding/bond_options.c
>> >index 0cf25de6f46d..6d2f44b3528d 100644
>> >--- a/drivers/net/bonding/bond_options.c
>> >+++ b/drivers/net/bonding/bond_options.c
>> >@@ -387,7 +387,7 @@ static const struct bond_option
>> bond_opts[BOND_OPT_LAST] = {
>> > 		.id = BOND_OPT_SLAVES,
>> > 		.name = "slaves",
>> > 		.desc = "Slave membership management",
>> >-		.flags = BOND_OPTFLAG_RAWVAL,
>> >+		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFUP,
>> > 		.set = bond_option_slaves_set
>> > 	},
>> > 	[BOND_OPT_TLB_DYNAMIC_LB] = {
>> >@@ -583,6 +583,8 @@ static int bond_opt_check_deps(struct bonding
>> *bond,
>> > 		return -ENOTEMPTY;
>> > 	if ((opt->flags & BOND_OPTFLAG_IFDOWN) && (bond->dev->flags &
>> IFF_UP))
>> > 		return -EBUSY;
>> >+	if ((opt->flags & BOND_OPTFLAG_IFUP) && !(bond->dev->flags &
>> IFF_UP))
>> >+		return -EPERM;
>> >
>> > 	return 0;
>> > }
>> >diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>> >index 9d382f2f0bc5..742f5cc81adf 100644
>> >--- a/include/net/bond_options.h
>> >+++ b/include/net/bond_options.h
>> >@@ -15,11 +15,13 @@
>> >  * BOND_OPTFLAG_NOSLAVES - check if the bond device is empty before
>> setting
>> >  * BOND_OPTFLAG_IFDOWN - check if the bond device is down before
>> setting
>> >  * BOND_OPTFLAG_RAWVAL - the option parses the value itself
>> >+ * BOND_OPTFLAG_IFUP - check if the bond device is up before setting
>> >  */
>> > enum {
>> > 	BOND_OPTFLAG_NOSLAVES	= BIT(0),
>> > 	BOND_OPTFLAG_IFDOWN	= BIT(1),
>> >-	BOND_OPTFLAG_RAWVAL	= BIT(2)
>> >+	BOND_OPTFLAG_RAWVAL	= BIT(2),
>> >+	BOND_OPTFLAG_IFUP	= BIT(3)
>> > };
>> >
>> > /* Value type flags:
>> >--
>> >2.27.0
>> >

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

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

* Re: [PATCH] bonding: Avoid adding slave devices to inactive bonding
@ 2021-07-28  6:51 zhudi (J)
  2021-07-28 19:57 ` Jay Vosburgh
  0 siblings, 1 reply; 5+ messages in thread
From: zhudi (J) @ 2021-07-28  6:51 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: vfalico, kuba, davem, netdev, Chenxiang (EulerOS)

> zhudi <zhudi21@huawei.com> wrote:
> 
> >We need to refuse to add slave devices to the bonding which does
> >not set IFF_UP flag, otherwise some problems will be caused(such as
> >bond_set_carrier() will not sync carrier state to upper net device).
> >The ifenslave command can prevent such use case, but through the sysfs
> >interface, slave devices can still be added regardless of whether
> >the bonding is set with IFF_UP flag or not.
> 
> 	What specifically happens in the carrier state issue you
> mention?  Are there other specific issues?

yes, The following steps can cause problems:
	1)bond0 is down
	2) ip link add link bond0 name ipvlan type ipvlan mode l2
	3) echo +enp2s7 >/sys/class/net/bond0/bonding/slaves
	4) ip link set bond0 up

	After these steps, use ip link command, we found ipvlan has NO-CARRIER:
		ipvlan@bond0: <NO-CARRIER, BROADCAST,MULTICAST,UP,M-DOWN> mtu 1500 qdisc noqueue state>

	This is because,  bond_enslave()->bond_set_carrier()->netif_carrier_on()...->netdev_state_change() 
will not sync carrier state to ipvlan because the IFF_UP  flag is not set.

> 	As far as I can recall, adding interfaces to the bond while the
> bond is down has worked for a very long time, so I'm concerned that
> disabling that functionality will have impact on existing
> configurations.
> 
> 	Also, to the best of my knowledge, the currently packaged
> ifenslave programs are scripts that utilize the sysfs interface.  I'm
> unaware of current usage of the old C ifenslave program (removed from
> the kernel source in 2013), although the kernel code should still
> support it.

	We still use old ifenslave command and it  does only allow to add slave to bonding with up state, code is as follows:

	/* check if master is up; if not then fail any operation */
	if (!(master_flags.ifr_flags & IFF_UP)) {
		fprintf(stderr,
			"Illegal operation; the specified master interface "
			"'%s' is not up.\n",
			master_ifname);
		res = 1;
		goto out;
	}

	If so, the behavior of the new tool is inconsistent with that of the old tool.

> 
> 	-J
> 
> >So we introduce a new BOND_OPTFLAG_IFUP flag to avoid adding slave
> >devices to inactive bonding.
> >
> >Signed-off-by: zhudi <zhudi21@huawei.com>
> >---
> > drivers/net/bonding/bond_options.c | 4 +++-
> > include/net/bond_options.h         | 4 +++-
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/net/bonding/bond_options.c
> b/drivers/net/bonding/bond_options.c
> >index 0cf25de6f46d..6d2f44b3528d 100644
> >--- a/drivers/net/bonding/bond_options.c
> >+++ b/drivers/net/bonding/bond_options.c
> >@@ -387,7 +387,7 @@ static const struct bond_option
> bond_opts[BOND_OPT_LAST] = {
> > 		.id = BOND_OPT_SLAVES,
> > 		.name = "slaves",
> > 		.desc = "Slave membership management",
> >-		.flags = BOND_OPTFLAG_RAWVAL,
> >+		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFUP,
> > 		.set = bond_option_slaves_set
> > 	},
> > 	[BOND_OPT_TLB_DYNAMIC_LB] = {
> >@@ -583,6 +583,8 @@ static int bond_opt_check_deps(struct bonding
> *bond,
> > 		return -ENOTEMPTY;
> > 	if ((opt->flags & BOND_OPTFLAG_IFDOWN) && (bond->dev->flags &
> IFF_UP))
> > 		return -EBUSY;
> >+	if ((opt->flags & BOND_OPTFLAG_IFUP) && !(bond->dev->flags &
> IFF_UP))
> >+		return -EPERM;
> >
> > 	return 0;
> > }
> >diff --git a/include/net/bond_options.h b/include/net/bond_options.h
> >index 9d382f2f0bc5..742f5cc81adf 100644
> >--- a/include/net/bond_options.h
> >+++ b/include/net/bond_options.h
> >@@ -15,11 +15,13 @@
> >  * BOND_OPTFLAG_NOSLAVES - check if the bond device is empty before
> setting
> >  * BOND_OPTFLAG_IFDOWN - check if the bond device is down before
> setting
> >  * BOND_OPTFLAG_RAWVAL - the option parses the value itself
> >+ * BOND_OPTFLAG_IFUP - check if the bond device is up before setting
> >  */
> > enum {
> > 	BOND_OPTFLAG_NOSLAVES	= BIT(0),
> > 	BOND_OPTFLAG_IFDOWN	= BIT(1),
> >-	BOND_OPTFLAG_RAWVAL	= BIT(2)
> >+	BOND_OPTFLAG_RAWVAL	= BIT(2),
> >+	BOND_OPTFLAG_IFUP	= BIT(3)
> > };
> >
> > /* Value type flags:
> >--
> >2.27.0
> >
> 
> ---
> 	-Jay Vosburgh, jay.vosburgh@canonical.com

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

* Re: [PATCH] bonding: Avoid adding slave devices to inactive bonding
  2021-07-28  3:35 zhudi
@ 2021-07-28  5:06 ` Jay Vosburgh
  0 siblings, 0 replies; 5+ messages in thread
From: Jay Vosburgh @ 2021-07-28  5:06 UTC (permalink / raw)
  To: zhudi; +Cc: vfalico, kuba, davem, netdev, rose.chen

zhudi <zhudi21@huawei.com> wrote:

>We need to refuse to add slave devices to the bonding which does
>not set IFF_UP flag, otherwise some problems will be caused(such as
>bond_set_carrier() will not sync carrier state to upper net device).
>The ifenslave command can prevent such use case, but through the sysfs
>interface, slave devices can still be added regardless of whether
>the bonding is set with IFF_UP flag or not.

	What specifically happens in the carrier state issue you
mention?  Are there other specific issues?

	As far as I can recall, adding interfaces to the bond while the
bond is down has worked for a very long time, so I'm concerned that
disabling that functionality will have impact on existing
configurations.

	Also, to the best of my knowledge, the currently packaged
ifenslave programs are scripts that utilize the sysfs interface.  I'm
unaware of current usage of the old C ifenslave program (removed from
the kernel source in 2013), although the kernel code should still
support it.

	-J

>So we introduce a new BOND_OPTFLAG_IFUP flag to avoid adding slave
>devices to inactive bonding.
>
>Signed-off-by: zhudi <zhudi21@huawei.com>
>---
> drivers/net/bonding/bond_options.c | 4 +++-
> include/net/bond_options.h         | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index 0cf25de6f46d..6d2f44b3528d 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -387,7 +387,7 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> 		.id = BOND_OPT_SLAVES,
> 		.name = "slaves",
> 		.desc = "Slave membership management",
>-		.flags = BOND_OPTFLAG_RAWVAL,
>+		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFUP,
> 		.set = bond_option_slaves_set
> 	},
> 	[BOND_OPT_TLB_DYNAMIC_LB] = {
>@@ -583,6 +583,8 @@ static int bond_opt_check_deps(struct bonding *bond,
> 		return -ENOTEMPTY;
> 	if ((opt->flags & BOND_OPTFLAG_IFDOWN) && (bond->dev->flags & IFF_UP))
> 		return -EBUSY;
>+	if ((opt->flags & BOND_OPTFLAG_IFUP) && !(bond->dev->flags & IFF_UP))
>+		return -EPERM;
> 
> 	return 0;
> }
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index 9d382f2f0bc5..742f5cc81adf 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -15,11 +15,13 @@
>  * BOND_OPTFLAG_NOSLAVES - check if the bond device is empty before setting
>  * BOND_OPTFLAG_IFDOWN - check if the bond device is down before setting
>  * BOND_OPTFLAG_RAWVAL - the option parses the value itself
>+ * BOND_OPTFLAG_IFUP - check if the bond device is up before setting
>  */
> enum {
> 	BOND_OPTFLAG_NOSLAVES	= BIT(0),
> 	BOND_OPTFLAG_IFDOWN	= BIT(1),
>-	BOND_OPTFLAG_RAWVAL	= BIT(2)
>+	BOND_OPTFLAG_RAWVAL	= BIT(2),
>+	BOND_OPTFLAG_IFUP	= BIT(3)
> };
> 
> /* Value type flags:
>-- 
>2.27.0
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

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

* [PATCH] bonding: Avoid adding slave devices to inactive bonding
@ 2021-07-28  3:35 zhudi
  2021-07-28  5:06 ` Jay Vosburgh
  0 siblings, 1 reply; 5+ messages in thread
From: zhudi @ 2021-07-28  3:35 UTC (permalink / raw)
  To: j.vosburgh, vfalico, kuba, davem; +Cc: netdev, zhudi21, rose.chen

We need to refuse to add slave devices to the bonding which does
not set IFF_UP flag, otherwise some problems will be caused(such as
bond_set_carrier() will not sync carrier state to upper net device).
The ifenslave command can prevent such use case, but through the sysfs
interface, slave devices can still be added regardless of whether
the bonding is set with IFF_UP flag or not.

So we introduce a new BOND_OPTFLAG_IFUP flag to avoid adding slave
devices to inactive bonding.

Signed-off-by: zhudi <zhudi21@huawei.com>
---
 drivers/net/bonding/bond_options.c | 4 +++-
 include/net/bond_options.h         | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 0cf25de6f46d..6d2f44b3528d 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -387,7 +387,7 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.id = BOND_OPT_SLAVES,
 		.name = "slaves",
 		.desc = "Slave membership management",
-		.flags = BOND_OPTFLAG_RAWVAL,
+		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFUP,
 		.set = bond_option_slaves_set
 	},
 	[BOND_OPT_TLB_DYNAMIC_LB] = {
@@ -583,6 +583,8 @@ static int bond_opt_check_deps(struct bonding *bond,
 		return -ENOTEMPTY;
 	if ((opt->flags & BOND_OPTFLAG_IFDOWN) && (bond->dev->flags & IFF_UP))
 		return -EBUSY;
+	if ((opt->flags & BOND_OPTFLAG_IFUP) && !(bond->dev->flags & IFF_UP))
+		return -EPERM;
 
 	return 0;
 }
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 9d382f2f0bc5..742f5cc81adf 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -15,11 +15,13 @@
  * BOND_OPTFLAG_NOSLAVES - check if the bond device is empty before setting
  * BOND_OPTFLAG_IFDOWN - check if the bond device is down before setting
  * BOND_OPTFLAG_RAWVAL - the option parses the value itself
+ * BOND_OPTFLAG_IFUP - check if the bond device is up before setting
  */
 enum {
 	BOND_OPTFLAG_NOSLAVES	= BIT(0),
 	BOND_OPTFLAG_IFDOWN	= BIT(1),
-	BOND_OPTFLAG_RAWVAL	= BIT(2)
+	BOND_OPTFLAG_RAWVAL	= BIT(2),
+	BOND_OPTFLAG_IFUP	= BIT(3)
 };
 
 /* Value type flags:
-- 
2.27.0


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

end of thread, other threads:[~2021-07-29  1:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29  1:46 [PATCH] bonding: Avoid adding slave devices to inactive bonding zhudi (J)
  -- strict thread matches above, loose matches on Subject: below --
2021-07-28  6:51 zhudi (J)
2021-07-28 19:57 ` Jay Vosburgh
2021-07-28  3:35 zhudi
2021-07-28  5:06 ` Jay Vosburgh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.