Re: [PATCH 1/3] bridge: preserve random init MAC address
On 04/22/2014 03:41 PM, Luis R. Rodriguez wrote: > On Wed, Mar 19, 2014 at 7:05 PM, Luis R. Rodriguez wrote: >> On Tue, Mar 18, 2014 at 08:10:56PM -0700, Stephen Hemminger wrote: >>> On Wed, 12 Mar 2014 20:15:25 -0700 >>> "Luis R. Rodriguez" wrote: >>> As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. >>> >>> What is supposed to happen is that the recalculate chooses >>> the lowest MAC address of the slaves. If there are no slaves >>> it might as well just calculate a new random value. There is >>> not great merit in preserving the original defunct address. >>> >>> Or something like this >>> --- a/net/bridge/br_stp_if.c 2014-02-12 08:21:56.733857356 -0800 >>> +++ b/net/bridge/br_stp_if.c 2014-03-18 20:09:09.334388826 -0700 >>> @@ -235,6 +235,9 @@ bool br_stp_recalculate_bridge_id(struct >>> addr = p->dev->dev_addr; >>> >>> } >>> + >>> + if (addr == br_mac_zero) >>> + return false; /* keep original address */ >>> >>> if (ether_addr_equal(br->bridge_id.addr, addr)) >>> return false; /* no change */ >>> >>> that just keeps the old value. >> >> The old value could be a port which got root blocked, I think >> it can be confusing to see that happen. Either way feel free to >> make the call, I'll provide more details below on perhaps one reason >> to keep the original MAC address. > > Stephen, I'd like to respin this series to address all pending > feedback, I'd still like your feedback / call / judgement on this > part. I'm fine either way, just wanted to ensure I highlight the > reasoning of why I kept the original random MAC address. Please keep > in mind that at this point I'm convinced bridging is the *wrong* > solution to networking with guests but it is being used in a lot of > current topologies, this would just help with smoothing out corner > cases. > Hi Luis I took at this series again, I think it might make to generate a new random address instead of storing the original. There is not much point in storing the original since it's been gone, and most likely not advertised anywhere anyway. As for virtualization configuration you described, a lot of times there is a single port in the down state that remains. At least that's how libvirt manages it's bridged networks. Thanks -vlad >>> The bridge is in a meaningless state when there are no ports, >> >> Some virtualization topologies may want a backend with no link (or >> perhaps one which is dynamic, with the option to have none) to the >> internet but just a bridge so guests sharing the bridge can talk to >> each other. In this case bridging can be used only to link the >> batch of guests. >> >> In this case the bridge simply acts as a switch, but also can be used as the >> interface for DHCP, for example. In such a case the guests will be doing >> ARP to get to the DHCP server. There's a flurry of ways one can try to get >> all this meshed together including enablign an ARP proxy but I'm looking >> at ways to optimize this further -- but I'd like to address the current >> usage cases first. >> >>> and when the first port is added back it will be used as the >>> new bridge id. >> >> Sure. Let me know how you think we should proceed with this patch based >> on the above. > > Thanks in advance. > > Luis > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Apr 22, 2014 at 12:41 PM, Luis R. Rodriguez wrote: > Stephen, I'd like to respin this series to address all pending > feedback, I'd still like your feedback / call / judgement on this > part. I'm fine either way, just wanted to ensure I highlight the > reasoning of why I kept the original random MAC address. Re-poke. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Apr 22, 2014 at 12:41 PM, Luis R. Rodriguez mcg...@do-not-panic.com wrote: Stephen, I'd like to respin this series to address all pending feedback, I'd still like your feedback / call / judgement on this part. I'm fine either way, just wanted to ensure I highlight the reasoning of why I kept the original random MAC address. Re-poke. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On 04/22/2014 03:41 PM, Luis R. Rodriguez wrote: On Wed, Mar 19, 2014 at 7:05 PM, Luis R. Rodriguez mcg...@suse.com wrote: On Tue, Mar 18, 2014 at 08:10:56PM -0700, Stephen Hemminger wrote: On Wed, 12 Mar 2014 20:15:25 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. What is supposed to happen is that the recalculate chooses the lowest MAC address of the slaves. If there are no slaves it might as well just calculate a new random value. There is not great merit in preserving the original defunct address. Or something like this --- a/net/bridge/br_stp_if.c 2014-02-12 08:21:56.733857356 -0800 +++ b/net/bridge/br_stp_if.c 2014-03-18 20:09:09.334388826 -0700 @@ -235,6 +235,9 @@ bool br_stp_recalculate_bridge_id(struct addr = p-dev-dev_addr; } + + if (addr == br_mac_zero) + return false; /* keep original address */ if (ether_addr_equal(br-bridge_id.addr, addr)) return false; /* no change */ that just keeps the old value. The old value could be a port which got root blocked, I think it can be confusing to see that happen. Either way feel free to make the call, I'll provide more details below on perhaps one reason to keep the original MAC address. Stephen, I'd like to respin this series to address all pending feedback, I'd still like your feedback / call / judgement on this part. I'm fine either way, just wanted to ensure I highlight the reasoning of why I kept the original random MAC address. Please keep in mind that at this point I'm convinced bridging is the *wrong* solution to networking with guests but it is being used in a lot of current topologies, this would just help with smoothing out corner cases. Hi Luis I took at this series again, I think it might make to generate a new random address instead of storing the original. There is not much point in storing the original since it's been gone, and most likely not advertised anywhere anyway. As for virtualization configuration you described, a lot of times there is a single port in the down state that remains. At least that's how libvirt manages it's bridged networks. Thanks -vlad The bridge is in a meaningless state when there are no ports, Some virtualization topologies may want a backend with no link (or perhaps one which is dynamic, with the option to have none) to the internet but just a bridge so guests sharing the bridge can talk to each other. In this case bridging can be used only to link the batch of guests. In this case the bridge simply acts as a switch, but also can be used as the interface for DHCP, for example. In such a case the guests will be doing ARP to get to the DHCP server. There's a flurry of ways one can try to get all this meshed together including enablign an ARP proxy but I'm looking at ways to optimize this further -- but I'd like to address the current usage cases first. and when the first port is added back it will be used as the new bridge id. Sure. Let me know how you think we should proceed with this patch based on the above. Thanks in advance. Luis -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Wed, Mar 19, 2014 at 7:05 PM, Luis R. Rodriguez wrote: > On Tue, Mar 18, 2014 at 08:10:56PM -0700, Stephen Hemminger wrote: >> On Wed, 12 Mar 2014 20:15:25 -0700 >> "Luis R. Rodriguez" wrote: >> >> > As it is now if you add create a bridge it gets started >> > with a random MAC address and if you then add a net_device >> > as a slave but later kick it out you end up with a zero >> > MAC address. Instead preserve the original random MAC >> > address and use it. >> >> What is supposed to happen is that the recalculate chooses >> the lowest MAC address of the slaves. If there are no slaves >> it might as well just calculate a new random value. There is >> not great merit in preserving the original defunct address. >> >> Or something like this >> --- a/net/bridge/br_stp_if.c 2014-02-12 08:21:56.733857356 -0800 >> +++ b/net/bridge/br_stp_if.c 2014-03-18 20:09:09.334388826 -0700 >> @@ -235,6 +235,9 @@ bool br_stp_recalculate_bridge_id(struct >> addr = p->dev->dev_addr; >> >> } >> + >> + if (addr == br_mac_zero) >> + return false; /* keep original address */ >> >> if (ether_addr_equal(br->bridge_id.addr, addr)) >> return false; /* no change */ >> >> that just keeps the old value. > > The old value could be a port which got root blocked, I think > it can be confusing to see that happen. Either way feel free to > make the call, I'll provide more details below on perhaps one reason > to keep the original MAC address. Stephen, I'd like to respin this series to address all pending feedback, I'd still like your feedback / call / judgement on this part. I'm fine either way, just wanted to ensure I highlight the reasoning of why I kept the original random MAC address. Please keep in mind that at this point I'm convinced bridging is the *wrong* solution to networking with guests but it is being used in a lot of current topologies, this would just help with smoothing out corner cases. >> The bridge is in a meaningless state when there are no ports, > > Some virtualization topologies may want a backend with no link (or > perhaps one which is dynamic, with the option to have none) to the > internet but just a bridge so guests sharing the bridge can talk to > each other. In this case bridging can be used only to link the > batch of guests. > > In this case the bridge simply acts as a switch, but also can be used as the > interface for DHCP, for example. In such a case the guests will be doing > ARP to get to the DHCP server. There's a flurry of ways one can try to get > all this meshed together including enablign an ARP proxy but I'm looking > at ways to optimize this further -- but I'd like to address the current > usage cases first. > >> and when the first port is added back it will be used as the >> new bridge id. > > Sure. Let me know how you think we should proceed with this patch based > on the above. Thanks in advance. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Wed, Mar 19, 2014 at 7:05 PM, Luis R. Rodriguez mcg...@suse.com wrote: On Tue, Mar 18, 2014 at 08:10:56PM -0700, Stephen Hemminger wrote: On Wed, 12 Mar 2014 20:15:25 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. What is supposed to happen is that the recalculate chooses the lowest MAC address of the slaves. If there are no slaves it might as well just calculate a new random value. There is not great merit in preserving the original defunct address. Or something like this --- a/net/bridge/br_stp_if.c 2014-02-12 08:21:56.733857356 -0800 +++ b/net/bridge/br_stp_if.c 2014-03-18 20:09:09.334388826 -0700 @@ -235,6 +235,9 @@ bool br_stp_recalculate_bridge_id(struct addr = p-dev-dev_addr; } + + if (addr == br_mac_zero) + return false; /* keep original address */ if (ether_addr_equal(br-bridge_id.addr, addr)) return false; /* no change */ that just keeps the old value. The old value could be a port which got root blocked, I think it can be confusing to see that happen. Either way feel free to make the call, I'll provide more details below on perhaps one reason to keep the original MAC address. Stephen, I'd like to respin this series to address all pending feedback, I'd still like your feedback / call / judgement on this part. I'm fine either way, just wanted to ensure I highlight the reasoning of why I kept the original random MAC address. Please keep in mind that at this point I'm convinced bridging is the *wrong* solution to networking with guests but it is being used in a lot of current topologies, this would just help with smoothing out corner cases. The bridge is in a meaningless state when there are no ports, Some virtualization topologies may want a backend with no link (or perhaps one which is dynamic, with the option to have none) to the internet but just a bridge so guests sharing the bridge can talk to each other. In this case bridging can be used only to link the batch of guests. In this case the bridge simply acts as a switch, but also can be used as the interface for DHCP, for example. In such a case the guests will be doing ARP to get to the DHCP server. There's a flurry of ways one can try to get all this meshed together including enablign an ARP proxy but I'm looking at ways to optimize this further -- but I'd like to address the current usage cases first. and when the first port is added back it will be used as the new bridge id. Sure. Let me know how you think we should proceed with this patch based on the above. Thanks in advance. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Mar 18, 2014 at 08:10:56PM -0700, Stephen Hemminger wrote: > On Wed, 12 Mar 2014 20:15:25 -0700 > "Luis R. Rodriguez" wrote: > > > As it is now if you add create a bridge it gets started > > with a random MAC address and if you then add a net_device > > as a slave but later kick it out you end up with a zero > > MAC address. Instead preserve the original random MAC > > address and use it. > > What is supposed to happen is that the recalculate chooses > the lowest MAC address of the slaves. If there are no slaves > it might as well just calculate a new random value. There is > not great merit in preserving the original defunct address. > > Or something like this > --- a/net/bridge/br_stp_if.c 2014-02-12 08:21:56.733857356 -0800 > +++ b/net/bridge/br_stp_if.c 2014-03-18 20:09:09.334388826 -0700 > @@ -235,6 +235,9 @@ bool br_stp_recalculate_bridge_id(struct > addr = p->dev->dev_addr; > > } > + > + if (addr == br_mac_zero) > + return false; /* keep original address */ > > if (ether_addr_equal(br->bridge_id.addr, addr)) > return false; /* no change */ > > that just keeps the old value. The old value could be a port which got root blocked, I think it can be confusing to see that happen. Either way feel free to make the call, I'll provide more details below on perhaps one reason to keep the original MAC address. > The bridge is in a meaningless state when there are no ports, Some virtualization topologies may want a backend with no link (or perhaps one which is dynamic, with the option to have none) to the internet but just a bridge so guests sharing the bridge can talk to each other. In this case bridging can be used only to link the batch of guests. In this case the bridge simply acts as a switch, but also can be used as the interface for DHCP, for example. In such a case the guests will be doing ARP to get to the DHCP server. There's a flurry of ways one can try to get all this meshed together including enablign an ARP proxy but I'm looking at ways to optimize this further -- but I'd like to address the current usage cases first. > and when the first port is added back it will be used as the > new bridge id. Sure. Let me know how you think we should proceed with this patch based on the above. Luis pgp54ixXHGRLh.pgp Description: PGP signature
Re: [Bridge] [PATCH 1/3] bridge: preserve random init MAC address
On Tue, 2014-03-18 at 18:10 -0700, Luis R. Rodriguez wrote: > On Tue, Mar 18, 2014 at 6:04 PM, Toshiaki Makita > wrote: > > (2014/03/19 9:50), Luis R. Rodriguez wrote: > >> On Tue, Mar 18, 2014 at 5:42 PM, Toshiaki Makita > >> wrote: > >>> nit, > >>> If the last detached port happens to have the same addr as > >>> random_init_addr, this seems to call br_stp_change_bridge_id() even > >>> though bridge_id is not changed. > >> > >> Ah good point. > >> > >>> Shouldn't the assignment of random_init_addr be done before the check of > >>> "no change"? > >> > >> Good question, should we even allow two ports to have the same MAC > >> address or should we complain and refuse to add it? If so that should > >> mean we should also have to monitor any manual address changes or > >> events for address changes on the ports. > > > > This was recently discussed by Stephen and me. > > I'm thinking it should be allowed. > > > > http://marc.info/?l=linux-netdev=139182743919257=2 > > Great now that that's sorted out though I still think calling > br_stp_change_bridge_id() is right just as calling the update features > as the device is different. It could however be confusing when this > situation is run and folks might report odd bugs unless we could tell > them apart clearly. Thoughts? br_stp_change_bridge_id() is currently called only if bridge_id.addr should be changed. If the addr should not be changed but some updates are needed, br_stp_recalculate_bridge_id() doesn't seem to fit into it. Toshiaki Makita -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Bridge] [PATCH 1/3] bridge: preserve random init MAC address
On Tue, 2014-03-18 at 18:10 -0700, Luis R. Rodriguez wrote: On Tue, Mar 18, 2014 at 6:04 PM, Toshiaki Makita makita.toshi...@lab.ntt.co.jp wrote: (2014/03/19 9:50), Luis R. Rodriguez wrote: On Tue, Mar 18, 2014 at 5:42 PM, Toshiaki Makita makita.toshi...@lab.ntt.co.jp wrote: nit, If the last detached port happens to have the same addr as random_init_addr, this seems to call br_stp_change_bridge_id() even though bridge_id is not changed. Ah good point. Shouldn't the assignment of random_init_addr be done before the check of no change? Good question, should we even allow two ports to have the same MAC address or should we complain and refuse to add it? If so that should mean we should also have to monitor any manual address changes or events for address changes on the ports. This was recently discussed by Stephen and me. I'm thinking it should be allowed. http://marc.info/?l=linux-netdevm=139182743919257w=2 Great now that that's sorted out though I still think calling br_stp_change_bridge_id() is right just as calling the update features as the device is different. It could however be confusing when this situation is run and folks might report odd bugs unless we could tell them apart clearly. Thoughts? br_stp_change_bridge_id() is currently called only if bridge_id.addr should be changed. If the addr should not be changed but some updates are needed, br_stp_recalculate_bridge_id() doesn't seem to fit into it. Toshiaki Makita -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Mar 18, 2014 at 08:10:56PM -0700, Stephen Hemminger wrote: On Wed, 12 Mar 2014 20:15:25 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. What is supposed to happen is that the recalculate chooses the lowest MAC address of the slaves. If there are no slaves it might as well just calculate a new random value. There is not great merit in preserving the original defunct address. Or something like this --- a/net/bridge/br_stp_if.c 2014-02-12 08:21:56.733857356 -0800 +++ b/net/bridge/br_stp_if.c 2014-03-18 20:09:09.334388826 -0700 @@ -235,6 +235,9 @@ bool br_stp_recalculate_bridge_id(struct addr = p-dev-dev_addr; } + + if (addr == br_mac_zero) + return false; /* keep original address */ if (ether_addr_equal(br-bridge_id.addr, addr)) return false; /* no change */ that just keeps the old value. The old value could be a port which got root blocked, I think it can be confusing to see that happen. Either way feel free to make the call, I'll provide more details below on perhaps one reason to keep the original MAC address. The bridge is in a meaningless state when there are no ports, Some virtualization topologies may want a backend with no link (or perhaps one which is dynamic, with the option to have none) to the internet but just a bridge so guests sharing the bridge can talk to each other. In this case bridging can be used only to link the batch of guests. In this case the bridge simply acts as a switch, but also can be used as the interface for DHCP, for example. In such a case the guests will be doing ARP to get to the DHCP server. There's a flurry of ways one can try to get all this meshed together including enablign an ARP proxy but I'm looking at ways to optimize this further -- but I'd like to address the current usage cases first. and when the first port is added back it will be used as the new bridge id. Sure. Let me know how you think we should proceed with this patch based on the above. Luis pgp54ixXHGRLh.pgp Description: PGP signature
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Mar 18, 2014 at 8:10 PM, Stephen Hemminger wrote: > On Wed, 12 Mar 2014 20:15:25 -0700 > "Luis R. Rodriguez" wrote: > >> As it is now if you add create a bridge it gets started >> with a random MAC address and if you then add a net_device >> as a slave but later kick it out you end up with a zero >> MAC address. Instead preserve the original random MAC >> address and use it. > > What is supposed to happen is that the recalculate chooses > the lowest MAC address of the slaves. If there are no slaves > it might as well just calculate a new random value. There is > not great merit in preserving the original defunct address. OK I'll go and add some changes for this then. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Wed, 12 Mar 2014 20:15:25 -0700 "Luis R. Rodriguez" wrote: > As it is now if you add create a bridge it gets started > with a random MAC address and if you then add a net_device > as a slave but later kick it out you end up with a zero > MAC address. Instead preserve the original random MAC > address and use it. What is supposed to happen is that the recalculate chooses the lowest MAC address of the slaves. If there are no slaves it might as well just calculate a new random value. There is not great merit in preserving the original defunct address. Or something like this that just keeps the old value. The bridge is in a meaningless state when there are no ports, and when the first port is added back it will be used as the new bridge id. --- a/net/bridge/br_stp_if.c2014-02-12 08:21:56.733857356 -0800 +++ b/net/bridge/br_stp_if.c2014-03-18 20:09:09.334388826 -0700 @@ -235,6 +235,9 @@ bool br_stp_recalculate_bridge_id(struct addr = p->dev->dev_addr; } + + if (addr == br_mac_zero) + return false; /* keep original address */ if (ether_addr_equal(br->bridge_id.addr, addr)) return false; /* no change */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Mar 18, 2014 at 6:04 PM, Toshiaki Makita wrote: > (2014/03/19 9:50), Luis R. Rodriguez wrote: >> On Tue, Mar 18, 2014 at 5:42 PM, Toshiaki Makita >> wrote: >>> nit, >>> If the last detached port happens to have the same addr as >>> random_init_addr, this seems to call br_stp_change_bridge_id() even >>> though bridge_id is not changed. >> >> Ah good point. >> >>> Shouldn't the assignment of random_init_addr be done before the check of >>> "no change"? >> >> Good question, should we even allow two ports to have the same MAC >> address or should we complain and refuse to add it? If so that should >> mean we should also have to monitor any manual address changes or >> events for address changes on the ports. > > This was recently discussed by Stephen and me. > I'm thinking it should be allowed. > > http://marc.info/?l=linux-netdev=139182743919257=2 Great now that that's sorted out though I still think calling br_stp_change_bridge_id() is right just as calling the update features as the device is different. It could however be confusing when this situation is run and folks might report odd bugs unless we could tell them apart clearly. Thoughts? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
(2014/03/19 9:50), Luis R. Rodriguez wrote: > On Tue, Mar 18, 2014 at 5:42 PM, Toshiaki Makita > wrote: >> nit, >> If the last detached port happens to have the same addr as >> random_init_addr, this seems to call br_stp_change_bridge_id() even >> though bridge_id is not changed. > > Ah good point. > >> Shouldn't the assignment of random_init_addr be done before the check of >> "no change"? > > Good question, should we even allow two ports to have the same MAC > address or should we complain and refuse to add it? If so that should > mean we should also have to monitor any manual address changes or > events for address changes on the ports. This was recently discussed by Stephen and me. I'm thinking it should be allowed. http://marc.info/?l=linux-netdev=139182743919257=2 Toshiaki Makita > > Stephen? > > Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Mar 18, 2014 at 5:42 PM, Toshiaki Makita wrote: > nit, > If the last detached port happens to have the same addr as > random_init_addr, this seems to call br_stp_change_bridge_id() even > though bridge_id is not changed. Ah good point. > Shouldn't the assignment of random_init_addr be done before the check of > "no change"? Good question, should we even allow two ports to have the same MAC address or should we complain and refuse to add it? If so that should mean we should also have to monitor any manual address changes or events for address changes on the ports. Stephen? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
(2014/03/13 12:15), Luis R. Rodriguez wrote: > From: "Luis R. Rodriguez" > > As it is now if you add create a bridge it gets started > with a random MAC address and if you then add a net_device > as a slave but later kick it out you end up with a zero > MAC address. Instead preserve the original random MAC > address and use it. > > If you manually set the bridge address that will always > be respected. This change only takes effect if at the time > of computing the new root port we determine we have found > no candidates. > > Cc: Stephen Hemminger > Cc: bri...@lists.linux-foundation.org > Cc: net...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: xen-de...@lists.xenproject.org > Cc: k...@vger.kernel.org > Signed-off-by: Luis R. Rodriguez > --- > net/bridge/br_device.c | 1 + > net/bridge/br_private.h | 1 + > net/bridge/br_stp_if.c | 3 +++ > 3 files changed, 5 insertions(+) > > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c > index b063050..5f13eac 100644 > --- a/net/bridge/br_device.c > +++ b/net/bridge/br_device.c > @@ -368,6 +368,7 @@ void br_dev_setup(struct net_device *dev) > br->bridge_id.prio[1] = 0x00; > > ether_addr_copy(br->group_addr, eth_reserved_addr_base); > + ether_addr_copy(br->random_init_addr, dev->dev_addr); > > br->stp_enabled = BR_NO_STP; > br->group_fwd_mask = BR_GROUPFWD_DEFAULT; > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index e1ca1dc..32a06da 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -240,6 +240,7 @@ struct net_bridge > unsigned long bridge_hello_time; > unsigned long bridge_forward_delay; > > + u8 random_init_addr[ETH_ALEN]; > u8 group_addr[ETH_ALEN]; > u16 root_port; > > diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c > index 189ba1e..4c9ad45 100644 > --- a/net/bridge/br_stp_if.c > +++ b/net/bridge/br_stp_if.c > @@ -239,6 +239,9 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br) > if (ether_addr_equal(br->bridge_id.addr, addr)) > return false; /* no change */ > > + if (ether_addr_equal(addr, br_mac_zero)) > + addr = br->random_init_addr; > + > br_stp_change_bridge_id(br, addr); > return true; > } nit, If the last detached port happens to have the same addr as random_init_addr, this seems to call br_stp_change_bridge_id() even though bridge_id is not changed. Shouldn't the assignment of random_init_addr be done before the check of "no change"? Toshiaki Makita -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
(2014/03/13 12:15), Luis R. Rodriguez wrote: From: Luis R. Rodriguez mcg...@suse.com As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. If you manually set the bridge address that will always be respected. This change only takes effect if at the time of computing the new root port we determine we have found no candidates. Cc: Stephen Hemminger step...@networkplumber.org Cc: bri...@lists.linux-foundation.org Cc: net...@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: xen-de...@lists.xenproject.org Cc: k...@vger.kernel.org Signed-off-by: Luis R. Rodriguez mcg...@suse.com --- net/bridge/br_device.c | 1 + net/bridge/br_private.h | 1 + net/bridge/br_stp_if.c | 3 +++ 3 files changed, 5 insertions(+) diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index b063050..5f13eac 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -368,6 +368,7 @@ void br_dev_setup(struct net_device *dev) br-bridge_id.prio[1] = 0x00; ether_addr_copy(br-group_addr, eth_reserved_addr_base); + ether_addr_copy(br-random_init_addr, dev-dev_addr); br-stp_enabled = BR_NO_STP; br-group_fwd_mask = BR_GROUPFWD_DEFAULT; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index e1ca1dc..32a06da 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -240,6 +240,7 @@ struct net_bridge unsigned long bridge_hello_time; unsigned long bridge_forward_delay; + u8 random_init_addr[ETH_ALEN]; u8 group_addr[ETH_ALEN]; u16 root_port; diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c index 189ba1e..4c9ad45 100644 --- a/net/bridge/br_stp_if.c +++ b/net/bridge/br_stp_if.c @@ -239,6 +239,9 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br) if (ether_addr_equal(br-bridge_id.addr, addr)) return false; /* no change */ + if (ether_addr_equal(addr, br_mac_zero)) + addr = br-random_init_addr; + br_stp_change_bridge_id(br, addr); return true; } nit, If the last detached port happens to have the same addr as random_init_addr, this seems to call br_stp_change_bridge_id() even though bridge_id is not changed. Shouldn't the assignment of random_init_addr be done before the check of no change? Toshiaki Makita -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Mar 18, 2014 at 5:42 PM, Toshiaki Makita makita.toshi...@lab.ntt.co.jp wrote: nit, If the last detached port happens to have the same addr as random_init_addr, this seems to call br_stp_change_bridge_id() even though bridge_id is not changed. Ah good point. Shouldn't the assignment of random_init_addr be done before the check of no change? Good question, should we even allow two ports to have the same MAC address or should we complain and refuse to add it? If so that should mean we should also have to monitor any manual address changes or events for address changes on the ports. Stephen? Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
(2014/03/19 9:50), Luis R. Rodriguez wrote: On Tue, Mar 18, 2014 at 5:42 PM, Toshiaki Makita makita.toshi...@lab.ntt.co.jp wrote: nit, If the last detached port happens to have the same addr as random_init_addr, this seems to call br_stp_change_bridge_id() even though bridge_id is not changed. Ah good point. Shouldn't the assignment of random_init_addr be done before the check of no change? Good question, should we even allow two ports to have the same MAC address or should we complain and refuse to add it? If so that should mean we should also have to monitor any manual address changes or events for address changes on the ports. This was recently discussed by Stephen and me. I'm thinking it should be allowed. http://marc.info/?l=linux-netdevm=139182743919257w=2 Toshiaki Makita Stephen? Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Mar 18, 2014 at 6:04 PM, Toshiaki Makita makita.toshi...@lab.ntt.co.jp wrote: (2014/03/19 9:50), Luis R. Rodriguez wrote: On Tue, Mar 18, 2014 at 5:42 PM, Toshiaki Makita makita.toshi...@lab.ntt.co.jp wrote: nit, If the last detached port happens to have the same addr as random_init_addr, this seems to call br_stp_change_bridge_id() even though bridge_id is not changed. Ah good point. Shouldn't the assignment of random_init_addr be done before the check of no change? Good question, should we even allow two ports to have the same MAC address or should we complain and refuse to add it? If so that should mean we should also have to monitor any manual address changes or events for address changes on the ports. This was recently discussed by Stephen and me. I'm thinking it should be allowed. http://marc.info/?l=linux-netdevm=139182743919257w=2 Great now that that's sorted out though I still think calling br_stp_change_bridge_id() is right just as calling the update features as the device is different. It could however be confusing when this situation is run and folks might report odd bugs unless we could tell them apart clearly. Thoughts? Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Wed, 12 Mar 2014 20:15:25 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. What is supposed to happen is that the recalculate chooses the lowest MAC address of the slaves. If there are no slaves it might as well just calculate a new random value. There is not great merit in preserving the original defunct address. Or something like this that just keeps the old value. The bridge is in a meaningless state when there are no ports, and when the first port is added back it will be used as the new bridge id. --- a/net/bridge/br_stp_if.c2014-02-12 08:21:56.733857356 -0800 +++ b/net/bridge/br_stp_if.c2014-03-18 20:09:09.334388826 -0700 @@ -235,6 +235,9 @@ bool br_stp_recalculate_bridge_id(struct addr = p-dev-dev_addr; } + + if (addr == br_mac_zero) + return false; /* keep original address */ if (ether_addr_equal(br-bridge_id.addr, addr)) return false; /* no change */ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] bridge: preserve random init MAC address
On Tue, Mar 18, 2014 at 8:10 PM, Stephen Hemminger step...@networkplumber.org wrote: On Wed, 12 Mar 2014 20:15:25 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. What is supposed to happen is that the recalculate chooses the lowest MAC address of the slaves. If there are no slaves it might as well just calculate a new random value. There is not great merit in preserving the original defunct address. OK I'll go and add some changes for this then. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] bridge: preserve random init MAC address
From: "Luis R. Rodriguez" As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. If you manually set the bridge address that will always be respected. This change only takes effect if at the time of computing the new root port we determine we have found no candidates. Cc: Stephen Hemminger Cc: bri...@lists.linux-foundation.org Cc: net...@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: xen-de...@lists.xenproject.org Cc: k...@vger.kernel.org Signed-off-by: Luis R. Rodriguez --- net/bridge/br_device.c | 1 + net/bridge/br_private.h | 1 + net/bridge/br_stp_if.c | 3 +++ 3 files changed, 5 insertions(+) diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index b063050..5f13eac 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -368,6 +368,7 @@ void br_dev_setup(struct net_device *dev) br->bridge_id.prio[1] = 0x00; ether_addr_copy(br->group_addr, eth_reserved_addr_base); + ether_addr_copy(br->random_init_addr, dev->dev_addr); br->stp_enabled = BR_NO_STP; br->group_fwd_mask = BR_GROUPFWD_DEFAULT; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index e1ca1dc..32a06da 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -240,6 +240,7 @@ struct net_bridge unsigned long bridge_hello_time; unsigned long bridge_forward_delay; + u8 random_init_addr[ETH_ALEN]; u8 group_addr[ETH_ALEN]; u16 root_port; diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c index 189ba1e..4c9ad45 100644 --- a/net/bridge/br_stp_if.c +++ b/net/bridge/br_stp_if.c @@ -239,6 +239,9 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br) if (ether_addr_equal(br->bridge_id.addr, addr)) return false; /* no change */ + if (ether_addr_equal(addr, br_mac_zero)) + addr = br->random_init_addr; + br_stp_change_bridge_id(br, addr); return true; } -- 1.8.5.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] bridge: preserve random init MAC address
From: Luis R. Rodriguez mcg...@suse.com As it is now if you add create a bridge it gets started with a random MAC address and if you then add a net_device as a slave but later kick it out you end up with a zero MAC address. Instead preserve the original random MAC address and use it. If you manually set the bridge address that will always be respected. This change only takes effect if at the time of computing the new root port we determine we have found no candidates. Cc: Stephen Hemminger step...@networkplumber.org Cc: bri...@lists.linux-foundation.org Cc: net...@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: xen-de...@lists.xenproject.org Cc: k...@vger.kernel.org Signed-off-by: Luis R. Rodriguez mcg...@suse.com --- net/bridge/br_device.c | 1 + net/bridge/br_private.h | 1 + net/bridge/br_stp_if.c | 3 +++ 3 files changed, 5 insertions(+) diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index b063050..5f13eac 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -368,6 +368,7 @@ void br_dev_setup(struct net_device *dev) br-bridge_id.prio[1] = 0x00; ether_addr_copy(br-group_addr, eth_reserved_addr_base); + ether_addr_copy(br-random_init_addr, dev-dev_addr); br-stp_enabled = BR_NO_STP; br-group_fwd_mask = BR_GROUPFWD_DEFAULT; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index e1ca1dc..32a06da 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -240,6 +240,7 @@ struct net_bridge unsigned long bridge_hello_time; unsigned long bridge_forward_delay; + u8 random_init_addr[ETH_ALEN]; u8 group_addr[ETH_ALEN]; u16 root_port; diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c index 189ba1e..4c9ad45 100644 --- a/net/bridge/br_stp_if.c +++ b/net/bridge/br_stp_if.c @@ -239,6 +239,9 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br) if (ether_addr_equal(br-bridge_id.addr, addr)) return false; /* no change */ + if (ether_addr_equal(addr, br_mac_zero)) + addr = br-random_init_addr; + br_stp_change_bridge_id(br, addr); return true; } -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
