| Message ID | 1359057684-6732-1-git-send-email-jiri@resnulli.us |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Jiri Pirko <jiri@resnulli.us> wrote: >Makes more sense to have randomly generated address by default than to >have all zeroes. It also allows user to for example put the bond into >bridge without need to have any slaves in it. > >Also note that this changes only behaviour of bonds with no slaves. Once >the first slave device is enslaved, its address will be used (no change >here). > >Also, fix dev_assign_type values on the way. > >Reported-by: Pavel Šimerda <psimerda@redhat.com> >Signed-off-by: Jiri Pirko <jiri@resnulli.us> Maybe I don't see it, but this feels like a bit of a hack just to get a bond with no slaves into a bridge. Am I missing something here? I just have this feeling that down the road I'm going to get questions as to why the bond gets a MAC, and then, poof, it vanishes when a slave is added. What's the point of the MAC address if it's only used to fool the bridge code? Also, when the bond's MAC changes from the random MAC to the first slave's MAC, does a notifier call need to happen? There isn't one now from the all zeroes to the first slave's, but that's from an invalid MAC to a valid one. There is already a notifier when the bond goes back to all zeroes, though. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > >v1->v2: >- fixed assign value of bond_dev->addr_assign_type in bond_set_dev_addr() >- added note to patch description > > drivers/net/bonding/bond_main.c | 28 ++++++++++++++++------------ > 1 file changed, 16 insertions(+), 12 deletions(-) > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index 564cf42..1d56ac9 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -1320,14 +1320,14 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev) > > /*---------------------------------- IOCTL ----------------------------------*/ > >-static int bond_sethwaddr(struct net_device *bond_dev, >- struct net_device *slave_dev) >+static void bond_set_dev_addr(struct net_device *bond_dev, >+ struct net_device *slave_dev) > { > pr_debug("bond_dev=%p\n", bond_dev); > pr_debug("slave_dev=%p\n", slave_dev); > pr_debug("slave_dev->addr_len=%d\n", slave_dev->addr_len); > memcpy(bond_dev->dev_addr, slave_dev->dev_addr, slave_dev->addr_len); >- return 0; >+ bond_dev->addr_assign_type = NET_ADDR_SET; > } > > static netdev_features_t bond_fix_features(struct net_device *dev, >@@ -1628,10 +1628,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) > > /* If this is the first slave, then we need to set the master's hardware > * address to be the same as the slave's. */ >- if (is_zero_ether_addr(bond->dev->dev_addr)) >- memcpy(bond->dev->dev_addr, slave_dev->dev_addr, >- slave_dev->addr_len); >- >+ if (bond->dev->addr_assign_type != NET_ADDR_SET) >+ bond_set_dev_addr(bond->dev, slave_dev); > > new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL); > if (!new_slave) { >@@ -2049,11 +2047,11 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev) > if (bond->slave_cnt == 0) { > bond_set_carrier(bond); > >- /* if the last slave was removed, zero the mac address >- * of the master so it will be set by the application >- * to the mac address of the first slave >+ /* If the last slave was removed, set random mac address >+ * of the master so it will be set by bond_enslave() >+ * to the mac address of the first slave. > */ >- memset(bond_dev->dev_addr, 0, bond_dev->addr_len); >+ eth_hw_addr_random(bond_dev); > > if (bond_vlan_used(bond)) { > pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n", >@@ -3708,7 +3706,8 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd > break; > case BOND_SETHWADDR_OLD: > case SIOCBONDSETHWADDR: >- res = bond_sethwaddr(bond_dev, slave_dev); >+ bond_set_dev_addr(bond_dev, slave_dev); >+ res = 0; > break; > case BOND_CHANGE_ACTIVE_OLD: > case SIOCBONDCHANGEACTIVE: >@@ -4858,6 +4857,11 @@ static int bond_init(struct net_device *bond_dev) > > bond_debug_register(bond); > >+ /* Ensure valid dev_addr */ >+ if (is_zero_ether_addr(bond_dev->dev_addr) && >+ bond_dev->addr_assign_type == NET_ADDR_PERM) >+ eth_hw_addr_random(bond_dev); >+ > __hw_addr_init(&bond->mc_list); > return 0; > } >-- >1.8.1 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fri, Jan 25, 2013 at 02:35:01AM CET, fubar@us.ibm.com wrote: >Jiri Pirko <jiri@resnulli.us> wrote: > >>Makes more sense to have randomly generated address by default than to >>have all zeroes. It also allows user to for example put the bond into >>bridge without need to have any slaves in it. >> >>Also note that this changes only behaviour of bonds with no slaves. Once >>the first slave device is enslaved, its address will be used (no change >>here). >> >>Also, fix dev_assign_type values on the way. >> >>Reported-by: Pavel Šimerda <psimerda@redhat.com> >>Signed-off-by: Jiri Pirko <jiri@resnulli.us> > > Maybe I don't see it, but this feels like a bit of a hack just >to get a bond with no slaves into a bridge. Am I missing something >here? I just have this feeling that down the road I'm going to get >questions as to why the bond gets a MAC, and then, poof, it vanishes >when a slave is added. What's the point of the MAC address if it's only >used to fool the bridge code? I think that generally, the device should have valid mac address in every point of its lifetime. I believe that bond is the only device who behaves differently (bridge also uses random mac when no ports are there) > > Also, when the bond's MAC changes from the random MAC to the >first slave's MAC, does a notifier call need to happen? There isn't one >now from the all zeroes to the first slave's, but that's from an invalid >MAC to a valid one. There is already a notifier when the bond goes back >to all zeroes, though. You are right, all_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev); should be called from bond_enslave() after calling bond_set_dev_addr() > > -J > >--- > -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > >> >>v1->v2: >>- fixed assign value of bond_dev->addr_assign_type in bond_set_dev_addr() >>- added note to patch description >> >> drivers/net/bonding/bond_main.c | 28 ++++++++++++++++------------ >> 1 file changed, 16 insertions(+), 12 deletions(-) >> >>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >>index 564cf42..1d56ac9 100644 >>--- a/drivers/net/bonding/bond_main.c >>+++ b/drivers/net/bonding/bond_main.c >>@@ -1320,14 +1320,14 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev) >> >> /*---------------------------------- IOCTL ----------------------------------*/ >> >>-static int bond_sethwaddr(struct net_device *bond_dev, >>- struct net_device *slave_dev) >>+static void bond_set_dev_addr(struct net_device *bond_dev, >>+ struct net_device *slave_dev) >> { >> pr_debug("bond_dev=%p\n", bond_dev); >> pr_debug("slave_dev=%p\n", slave_dev); >> pr_debug("slave_dev->addr_len=%d\n", slave_dev->addr_len); >> memcpy(bond_dev->dev_addr, slave_dev->dev_addr, slave_dev->addr_len); >>- return 0; >>+ bond_dev->addr_assign_type = NET_ADDR_SET; >> } >> >> static netdev_features_t bond_fix_features(struct net_device *dev, >>@@ -1628,10 +1628,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) >> >> /* If this is the first slave, then we need to set the master's hardware >> * address to be the same as the slave's. */ >>- if (is_zero_ether_addr(bond->dev->dev_addr)) >>- memcpy(bond->dev->dev_addr, slave_dev->dev_addr, >>- slave_dev->addr_len); >>- >>+ if (bond->dev->addr_assign_type != NET_ADDR_SET) >>+ bond_set_dev_addr(bond->dev, slave_dev); >> >> new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL); >> if (!new_slave) { >>@@ -2049,11 +2047,11 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev) >> if (bond->slave_cnt == 0) { >> bond_set_carrier(bond); >> >>- /* if the last slave was removed, zero the mac address >>- * of the master so it will be set by the application >>- * to the mac address of the first slave >>+ /* If the last slave was removed, set random mac address >>+ * of the master so it will be set by bond_enslave() >>+ * to the mac address of the first slave. >> */ >>- memset(bond_dev->dev_addr, 0, bond_dev->addr_len); >>+ eth_hw_addr_random(bond_dev); >> >> if (bond_vlan_used(bond)) { >> pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n", >>@@ -3708,7 +3706,8 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd >> break; >> case BOND_SETHWADDR_OLD: >> case SIOCBONDSETHWADDR: >>- res = bond_sethwaddr(bond_dev, slave_dev); >>+ bond_set_dev_addr(bond_dev, slave_dev); >>+ res = 0; >> break; >> case BOND_CHANGE_ACTIVE_OLD: >> case SIOCBONDCHANGEACTIVE: >>@@ -4858,6 +4857,11 @@ static int bond_init(struct net_device *bond_dev) >> >> bond_debug_register(bond); >> >>+ /* Ensure valid dev_addr */ >>+ if (is_zero_ether_addr(bond_dev->dev_addr) && >>+ bond_dev->addr_assign_type == NET_ADDR_PERM) >>+ eth_hw_addr_random(bond_dev); >>+ >> __hw_addr_init(&bond->mc_list); >> return 0; >> } >>-- >>1.8.1 >> > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fri, Jan 25, 2013 at 07:37:13AM CET, jiri@resnulli.us wrote: >Fri, Jan 25, 2013 at 02:35:01AM CET, fubar@us.ibm.com wrote: >>Jiri Pirko <jiri@resnulli.us> wrote: >> >>>Makes more sense to have randomly generated address by default than to >>>have all zeroes. It also allows user to for example put the bond into >>>bridge without need to have any slaves in it. >>> >>>Also note that this changes only behaviour of bonds with no slaves. Once >>>the first slave device is enslaved, its address will be used (no change >>>here). Also, this is not entirely true now. Example: eth1 has 52:54:00:b8:30:0b # ip link add bondx address 22:33:44:55:66:77 type bond # ip link set eth1 master bondx Now both bondx and eth1 has 22:33:44:55:66:77 Shouldn't both have 52:54:00:b8:30:0b ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jiri Pirko <jiri@resnulli.us> wrote: >Fri, Jan 25, 2013 at 07:37:13AM CET, jiri@resnulli.us wrote: >>Fri, Jan 25, 2013 at 02:35:01AM CET, fubar@us.ibm.com wrote: >>>Jiri Pirko <jiri@resnulli.us> wrote: >>> >>>>Makes more sense to have randomly generated address by default than to >>>>have all zeroes. It also allows user to for example put the bond into >>>>bridge without need to have any slaves in it. >>>> >>>>Also note that this changes only behaviour of bonds with no slaves. Once >>>>the first slave device is enslaved, its address will be used (no change >>>>here). > > >Also, this is not entirely true now. Example: > >eth1 has 52:54:00:b8:30:0b > ># ip link add bondx address 22:33:44:55:66:77 type bond ># ip link set eth1 master bondx > >Now both bondx and eth1 has 22:33:44:55:66:77 > >Shouldn't both have 52:54:00:b8:30:0b ? This is the way bonding has worked for as long as I can recall. The MAC of the first slave is assigned to the master, unless the bond has had its MAC manually set to something prior to the first slave being added. If the bond's MAC has been set manually, then that MAC is used instead of the first slave's MAC. Your patch doesn't appear to change that behavior, but it does substitute a random MAC in place of the all zeroes MAC used previously (and tests a flag to determine if the MAC has been manually set instead of checking for the all zeroes MAC). I know of customers that rely on this behavior to explicitly set the MAC of the bond (for filtering or accounting purposes). It probably should be documented more clearly, but I don't think it should be changed. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- Original Message ----- > From: "Jay Vosburgh" <fubar@us.ibm.com> > but I don't think it should be changed. Just a short question. Is there any reason for bonding interfaces to behave differently from bridging interfaces in this respect? Cheers, Pavel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fri, Jan 25, 2013 at 06:53:07PM CET, fubar@us.ibm.com wrote: >Jiri Pirko <jiri@resnulli.us> wrote: > >>Fri, Jan 25, 2013 at 07:37:13AM CET, jiri@resnulli.us wrote: >>>Fri, Jan 25, 2013 at 02:35:01AM CET, fubar@us.ibm.com wrote: >>>>Jiri Pirko <jiri@resnulli.us> wrote: >>>> >>>>>Makes more sense to have randomly generated address by default than to >>>>>have all zeroes. It also allows user to for example put the bond into >>>>>bridge without need to have any slaves in it. >>>>> >>>>>Also note that this changes only behaviour of bonds with no slaves. Once >>>>>the first slave device is enslaved, its address will be used (no change >>>>>here). >> >> >>Also, this is not entirely true now. Example: >> >>eth1 has 52:54:00:b8:30:0b >> >># ip link add bondx address 22:33:44:55:66:77 type bond >># ip link set eth1 master bondx >> >>Now both bondx and eth1 has 22:33:44:55:66:77 >> >>Shouldn't both have 52:54:00:b8:30:0b ? > > This is the way bonding has worked for as long as I can recall. >The MAC of the first slave is assigned to the master, unless the bond >has had its MAC manually set to something prior to the first slave being >added. If the bond's MAC has been set manually, then that MAC is used >instead of the first slave's MAC. > > Your patch doesn't appear to change that behavior, but it does >substitute a random MAC in place of the all zeroes MAC used previously >(and tests a flag to determine if the MAC has been manually set instead >of checking for the all zeroes MAC). Yeah, I know my patch is not changing this. I was just making sure it is correct. > > I know of customers that rely on this behavior to explicitly set >the MAC of the bond (for filtering or accounting purposes). It probably >should be documented more clearly, but I don't think it should be >changed. > > -J > >--- > -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pavel Simerda <psimerda@redhat.com> wrote: >----- Original Message ----- >> From: "Jay Vosburgh" <fubar@us.ibm.com> >> but I don't think it should be changed. > >Just a short question. Is there any reason for bonding interfaces to >behave differently from bridging interfaces in this respect? To clarify, what I don't think should change is that a manually set MAC on the bonding master should override the automatic copy of the first slave's MAC to the bonding master. The fail_over_mac active and follow settings are an exception to this, but those are special cases for unusual network hardware. As for the random MAC vs. zero MAC, I've always thought that the all zero MAC was a clear indicator that the device (the bonding master in this case) was not in a usable state (in the sense that it could not send or receive actual traffic). It's not a really big deal, though, so if the trend these days is for everything to have a MAC all the time, that's fine, as long as doing so doesn't break anything. I think the patch under discussion should be fine with the addition of the last notifier call previously discussed. Some documentation updates would be nice, too. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- Original Message ----- > From: "Jay Vosburgh" <fubar@us.ibm.com> > To: "Pavel Simerda" <psimerda@redhat.com> > Cc: netdev@vger.kernel.org, davem@davemloft.net, andy@greyhouse.net, stephen@networkplumber.org, dcbw@redhat.com, > "Jiri Pirko" <jiri@resnulli.us> > Sent: Friday, January 25, 2013 7:31:32 PM > Subject: Re: [patch net-next V2] bond: have random dev address by default instead of zeroes > > Pavel Simerda <psimerda@redhat.com> wrote: > > >----- Original Message ----- > >> From: "Jay Vosburgh" <fubar@us.ibm.com> > >> but I don't think it should be changed. > > > >Just a short question. Is there any reason for bonding interfaces to > >behave differently from bridging interfaces in this respect? > > To clarify, what I don't think should change is that a manually > set MAC on the bonding master should override the automatic copy of > the > first slave's MAC to the bonding master. The fail_over_mac active > and > follow settings are an exception to this, but those are special cases > for unusual network hardware. > > As for the random MAC vs. zero MAC, I've always thought that the > all zero MAC was a clear indicator that the device (the bonding > master > in this case) was not in a usable state (in the sense that it could > not > send or receive actual traffic). It's not a really big deal, though, > so Thanks for clarification. > if the trend these days is for everything to have a MAC all the time, > that's fine, as long as doing so doesn't break anything. > > I think the patch under discussion should be fine with the > addition of the last notifier call previously discussed. Some > documentation updates would be nice, too. > > -J > > --- > -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fri, Jan 25, 2013 at 07:31:32PM CET, fubar@us.ibm.com wrote: >Pavel Simerda <psimerda@redhat.com> wrote: > >>----- Original Message ----- >>> From: "Jay Vosburgh" <fubar@us.ibm.com> >>> but I don't think it should be changed. >> >>Just a short question. Is there any reason for bonding interfaces to >>behave differently from bridging interfaces in this respect? > > To clarify, what I don't think should change is that a manually >set MAC on the bonding master should override the automatic copy of the >first slave's MAC to the bonding master. The fail_over_mac active and >follow settings are an exception to this, but those are special cases >for unusual network hardware. > > As for the random MAC vs. zero MAC, I've always thought that the >all zero MAC was a clear indicator that the device (the bonding master >in this case) was not in a usable state (in the sense that it could not >send or receive actual traffic). It's not a really big deal, though, so >if the trend these days is for everything to have a MAC all the time, >that's fine, as long as doing so doesn't break anything. > > I think the patch under discussion should be fine with the >addition of the last notifier call previously discussed. Some >documentation updates would be nice, too. Will do :) > > -J > >--- > -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > >-- >To unsubscribe from this list: send the line "unsubscribe netdev" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2013-01-25 at 10:31 -0800, Jay Vosburgh wrote: > Pavel Simerda <psimerda@redhat.com> wrote: > > >----- Original Message ----- > >> From: "Jay Vosburgh" <fubar@us.ibm.com> > >> but I don't think it should be changed. > > > >Just a short question. Is there any reason for bonding interfaces to > >behave differently from bridging interfaces in this respect? > > To clarify, what I don't think should change is that a manually > set MAC on the bonding master should override the automatic copy of the > first slave's MAC to the bonding master. The fail_over_mac active and > follow settings are an exception to this, but those are special cases > for unusual network hardware. > > As for the random MAC vs. zero MAC, I've always thought that the > all zero MAC was a clear indicator that the device (the bonding master > in this case) was not in a usable state (in the sense that it could not > send or receive actual traffic). It's not a really big deal, though, so > if the trend these days is for everything to have a MAC all the time, > that's fine, as long as doing so doesn't break anything. Isn't that exactly what carrier means? If the carrier bit is off, nothing should expect that traffic can pass. The bond will only set its carrier ON if at least one slave exists and at least one slave has a carrier that's ON. We have overrides for buggy driver carrier checking already. Zero-MAC is somewhat redundant here as a mechanism for detecting that the bond is usable or not. Dan > I think the patch under discussion should be fine with the > addition of the last notifier call previously discussed. Some > documentation updates would be nice, too. > > -J > > --- > -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 564cf42..1d56ac9 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1320,14 +1320,14 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev) /*---------------------------------- IOCTL ----------------------------------*/ -static int bond_sethwaddr(struct net_device *bond_dev, - struct net_device *slave_dev) +static void bond_set_dev_addr(struct net_device *bond_dev, + struct net_device *slave_dev) { pr_debug("bond_dev=%p\n", bond_dev); pr_debug("slave_dev=%p\n", slave_dev); pr_debug("slave_dev->addr_len=%d\n", slave_dev->addr_len); memcpy(bond_dev->dev_addr, slave_dev->dev_addr, slave_dev->addr_len); - return 0; + bond_dev->addr_assign_type = NET_ADDR_SET; } static netdev_features_t bond_fix_features(struct net_device *dev, @@ -1628,10 +1628,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) /* If this is the first slave, then we need to set the master's hardware * address to be the same as the slave's. */ - if (is_zero_ether_addr(bond->dev->dev_addr)) - memcpy(bond->dev->dev_addr, slave_dev->dev_addr, - slave_dev->addr_len); - + if (bond->dev->addr_assign_type != NET_ADDR_SET) + bond_set_dev_addr(bond->dev, slave_dev); new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL); if (!new_slave) { @@ -2049,11 +2047,11 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev) if (bond->slave_cnt == 0) { bond_set_carrier(bond); - /* if the last slave was removed, zero the mac address - * of the master so it will be set by the application - * to the mac address of the first slave + /* If the last slave was removed, set random mac address + * of the master so it will be set by bond_enslave() + * to the mac address of the first slave. */ - memset(bond_dev->dev_addr, 0, bond_dev->addr_len); + eth_hw_addr_random(bond_dev); if (bond_vlan_used(bond)) { pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n", @@ -3708,7 +3706,8 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd break; case BOND_SETHWADDR_OLD: case SIOCBONDSETHWADDR: - res = bond_sethwaddr(bond_dev, slave_dev); + bond_set_dev_addr(bond_dev, slave_dev); + res = 0; break; case BOND_CHANGE_ACTIVE_OLD: case SIOCBONDCHANGEACTIVE: @@ -4858,6 +4857,11 @@ static int bond_init(struct net_device *bond_dev) bond_debug_register(bond); + /* Ensure valid dev_addr */ + if (is_zero_ether_addr(bond_dev->dev_addr) && + bond_dev->addr_assign_type == NET_ADDR_PERM) + eth_hw_addr_random(bond_dev); + __hw_addr_init(&bond->mc_list); return 0; }
Makes more sense to have randomly generated address by default than to have all zeroes. It also allows user to for example put the bond into bridge without need to have any slaves in it. Also note that this changes only behaviour of bonds with no slaves. Once the first slave device is enslaved, its address will be used (no change here). Also, fix dev_assign_type values on the way. Reported-by: Pavel Šimerda <psimerda@redhat.com> Signed-off-by: Jiri Pirko <jiri@resnulli.us> --- v1->v2: - fixed assign value of bond_dev->addr_assign_type in bond_set_dev_addr() - added note to patch description drivers/net/bonding/bond_main.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)