[PATCH 1/2] bridge: leave carrier on for empty bridge

[PATCH 1/2] bridge: leave carrier on for empty bridge

[Stephen Hemminger]

[-- Attachment #1: br-carrier-default.patch --]
[-- Type: text/plain, Size: 955 bytes --]

This resolves a regression seen by some users of bridging.
Some users use the bridge like a dummy device. 
They expect to be able to put an IPv6 address on the device
with no ports attached during boot.

Note: the bridge still will reflect the state of ports in the
bridge if there are any added.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
This fix needs to go to stable for 3.0 and 2.6.39


--- a/net/bridge/br_device.c	2011-09-01 08:52:27.596631192 -0700
+++ b/net/bridge/br_device.c	2011-09-01 09:01:03.256611801 -0700
@@ -91,7 +91,6 @@ static int br_dev_open(struct net_device
 {
 	struct net_bridge *br = netdev_priv(dev);
 
-	netif_carrier_off(dev);
 	netdev_update_features(dev);
 	netif_start_queue(dev);
 	br_stp_enable_bridge(br);
@@ -108,8 +107,6 @@ static int br_dev_stop(struct net_device
 {
 	struct net_bridge *br = netdev_priv(dev);
 
-	netif_carrier_off(dev);
-
 	br_stp_disable_bridge(br);
 	br_multicast_stop(br);
 

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Nicolas de Pesloüan]

Le 02/09/2011 19:22, Stephen Hemminger a écrit :
> This resolves a regression seen by some users of bridging.
> Some users use the bridge like a dummy device.
> They expect to be able to put an IPv6 address on the device
> with no ports attached during boot.
>
> Note: the bridge still will reflect the state of ports in the
> bridge if there are any added.

Doesn't this jeopardize the behavior introduced in 1faa4356a3bd89ea11fb92752d897cff3a20ec0e
"bridge: control carrier based on ports online"?

If the user starts the DHCP client before adding the first port to the bridge, the DHCP client will 
have a carrier and start the autoconfiguration process. This was the old behavior, but you fixed it.

	Nicolas.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Stephen Hemminger]

On Fri, 02 Sep 2011 23:39:03 +0200
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:

> Le 02/09/2011 19:22, Stephen Hemminger a écrit :
> > This resolves a regression seen by some users of bridging.
> > Some users use the bridge like a dummy device.
> > They expect to be able to put an IPv6 address on the device
> > with no ports attached during boot.
> >
> > Note: the bridge still will reflect the state of ports in the
> > bridge if there are any added.
> 
> Doesn't this jeopardize the behavior introduced in 1faa4356a3bd89ea11fb92752d897cff3a20ec0e
> "bridge: control carrier based on ports online"?
> 
> If the user starts the DHCP client before adding the first port to the bridge, the DHCP client will 
> have a carrier and start the autoconfiguration process. This was the old behavior, but you fixed it.
> 
> 	Nicolas.
> 

There is no perfect solution.
If DHCP works then IPv6 breaks?

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Ang Way Chuang]

On 03/09/11 07:11, Stephen Hemminger wrote:
> On Fri, 02 Sep 2011 23:39:03 +0200
> Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:
>
>> Le 02/09/2011 19:22, Stephen Hemminger a écrit :
>>> This resolves a regression seen by some users of bridging.
>>> Some users use the bridge like a dummy device.
>>> They expect to be able to put an IPv6 address on the device
>>> with no ports attached during boot.
>>>
>>> Note: the bridge still will reflect the state of ports in the
>>> bridge if there are any added.

Guilty as charged :) It's a valid use case in libvirtd. Thanks for the patch. I've tried your patch.
It doesn't work. There is still no carrier. Thing still break. The whole of commit
1faa4356a including the part where liveports is used to toggle carrier has to be reverted. Otherwise, it doesn't work.
I've tried that on my machine.

>> Doesn't this jeopardize the behavior introduced in 1faa4356a3bd89ea11fb92752d897cff3a20ec0e
>> "bridge: control carrier based on ports online"?
>>
>> If the user starts the DHCP client before adding the first port to the bridge, the DHCP client will 
>> have a carrier and start the autoconfiguration process. This was the old behavior, but you fixed it.
>>
>> 	Nicolas.
>>
> There is no perfect solution.
> If DHCP works then IPv6 breaks?

A more ideal solution in this case is to add the option to enabling carrier on sysfs and modify libvirtd
to turn on the carrier if IPv6 is enabled. But it will still break the existing configuration until everyone
upgrade to the latest libvirtd and kernel. Since there is no other complain from other user with this setup,
I guess nobody actually assigns IPv6 to libvirtd network device at this moment (partly because virt-manager
doesn't expose that functionality yet??).

> --
> 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
>

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Stephen Hemminger]

On Sat, 03 Sep 2011 11:15:41 +0900
Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:

> A more ideal solution in this case is to add the option to enabling carrier on sysfs and modify libvirtd
> to turn on the carrier if IPv6 is enabled. But it will still break the existing configuration until everyone
> upgrade to the latest libvirtd and kernel. Since there is no other complain from other user with this setup,
> I guess nobody actually assigns IPv6 to libvirtd network device at this moment (partly because virt-manager
> doesn't expose that functionality yet??).

Not sure about adding an option to support a configuration that is only available
by manually editing the xml files. There can't be that many people using libvirt in
this way, and fixing libvirt seems like a better solution.

Rather than rush in a kludge to handle this, let's take a few days and
examine what libvirt is trying to do and how it is doing it.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Marc Haber]

On Sat, Sep 03, 2011 at 11:15:41AM +0900, Ang Way Chuang wrote:
> A more ideal solution in this case is to add the option to enabling
> carrier on sysfs and modify libvirtd to turn on the carrier if IPv6 is
> enabled.

I would even do it manually from an init script if that saves me from
having an extra dummy interface on a bridge just to force it up.

>  Since there is no other
>  complain from other user with this setup, I guess nobody actually
>  assigns IPv6 to libvirtd network device at this moment

I do. Daily. And get bitten every time I reboot. A _major_ nuisance.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."    Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Nicolas de Pesloüan]

Le 03/09/2011 00:11, Stephen Hemminger a écrit :
> On Fri, 02 Sep 2011 23:39:03 +0200
> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com>  wrote:
>
>> Le 02/09/2011 19:22, Stephen Hemminger a écrit :
>>> This resolves a regression seen by some users of bridging.
>>> Some users use the bridge like a dummy device.
>>> They expect to be able to put an IPv6 address on the device
>>> with no ports attached during boot.
>>>
>>> Note: the bridge still will reflect the state of ports in the
>>> bridge if there are any added.
>>
>> Doesn't this jeopardize the behavior introduced in 1faa4356a3bd89ea11fb92752d897cff3a20ec0e
>> "bridge: control carrier based on ports online"?
>>
>> If the user starts the DHCP client before adding the first port to the bridge, the DHCP client will
>> have a carrier and start the autoconfiguration process. This was the old behavior, but you fixed it.
>>
>> 	Nicolas.
>>
>
> There is no perfect solution.
> If DHCP works then IPv6 breaks?

Instead of asserting carrier when the bridge have no port, can't we assert carrier when the three 
following condition are true at the same time :

- The bridge have no port.
- At least one IP address is setup on the bridge.
- The two above conditions are true for more than a configurable amount of seconds, with a default 
of 10, for example.

This would only delay carrier on for a few seconds for the regression and keep the current behavior 
(carrier off until at least 1 port is on) for DHCP.

	Nicolas.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Stephen Hemminger]

On Sat, 03 Sep 2011 20:32:28 +0200
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:

> Le 03/09/2011 00:11, Stephen Hemminger a écrit :
> > On Fri, 02 Sep 2011 23:39:03 +0200
> > Nicolas de Pesloüan<nicolas.2p.debian@gmail.com>  wrote:
> >
> >> Le 02/09/2011 19:22, Stephen Hemminger a écrit :
> >>> This resolves a regression seen by some users of bridging.
> >>> Some users use the bridge like a dummy device.
> >>> They expect to be able to put an IPv6 address on the device
> >>> with no ports attached during boot.
> >>>
> >>> Note: the bridge still will reflect the state of ports in the
> >>> bridge if there are any added.
> >>
> >> Doesn't this jeopardize the behavior introduced in 1faa4356a3bd89ea11fb92752d897cff3a20ec0e
> >> "bridge: control carrier based on ports online"?
> >>
> >> If the user starts the DHCP client before adding the first port to the bridge, the DHCP client will
> >> have a carrier and start the autoconfiguration process. This was the old behavior, but you fixed it.
> >>
> >> 	Nicolas.
> >>
> >
> > There is no perfect solution.
> > If DHCP works then IPv6 breaks?
> 
> Instead of asserting carrier when the bridge have no port, can't we assert carrier when the three 
> following condition are true at the same time :
> 
> - The bridge have no port.
> - At least one IP address is setup on the bridge.
> - The two above conditions are true for more than a configurable amount of seconds, with a default 
> of 10, for example.
> 
> This would only delay carrier on for a few seconds for the regression and keep the current behavior 
> (carrier off until at least 1 port is on) for DHCP.

This fails on two counts:
1. Bridge's often run without IP addresses!
2. DHCP won't try and send out request until carrier is true.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Nicolas de Pesloüan]

Le 04/09/2011 06:14, Stephen Hemminger a écrit :

>> Instead of asserting carrier when the bridge have no port, can't we assert carrier when the three
>> following condition are true at the same time :
>>
>> - The bridge have no port.
>> - At least one IP address is setup on the bridge.
>> - The two above conditions are true for more than a configurable amount of seconds, with a default
>> of 10, for example.
>>
>> This would only delay carrier on for a few seconds for the regression and keep the current behavior
>> (carrier off until at least 1 port is on) for DHCP.
>
> This fails on two counts:
> 1. Bridge's often run without IP addresses!
> 2. DHCP won't try and send out request until carrier is true.

Sorry, I missed to say that we should of course also assert carrier on if one port has carrier on.

And rethinking about it, the delay is probably useless :

bridge_carrier_on = at_least_one_port_has_carrier_on | (bridge_has_no_port & bridge_has_at_least_one_ip)

That way :
- for those using bridge without any port, manually setting the IP will assert carrier on. (By the 
way, why don't they use a dummy device instead?)

- for those using bridge with ports:
-- Using any kind of autoconfig will work as expected. Carrier will only be asserted at the time 
first port get carrier.
-- Using static IP confifiguration, carrier will possibly be erroneously reported as on during the 
small time gap between IP address configuration and first port is added to the bridge. This time gap 
may be removed by simply configuring the IP after the first port is added. This is probably already 
true for most distribs. And anyway, this time gap is probably not a problem.
-- Carrier will also be erroneously reported as on after removing the last port, if the bridge still 
has an IP. (But we can arrange for this not to happen).

And in order to ensure user really understand why carrier is on of off, we can simply issue an INFO 
message for the non-natural case (bridge_has_no_port & bridga_has_at_least_one_ip).

I consider all this reasonable.

	Nicolas.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Stephen Hemminger]

On Sun, 04 Sep 2011 09:35:10 +0200
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:

> Le 04/09/2011 06:14, Stephen Hemminger a écrit :
> 
> >> Instead of asserting carrier when the bridge have no port, can't we assert carrier when the three
> >> following condition are true at the same time :
> >>
> >> - The bridge have no port.
> >> - At least one IP address is setup on the bridge.
> >> - The two above conditions are true for more than a configurable amount of seconds, with a default
> >> of 10, for example.
> >>
> >> This would only delay carrier on for a few seconds for the regression and keep the current behavior
> >> (carrier off until at least 1 port is on) for DHCP.
> >
> > This fails on two counts:
> > 1. Bridge's often run without IP addresses!
> > 2. DHCP won't try and send out request until carrier is true.
> 
> Sorry, I missed to say that we should of course also assert carrier on if one port has carrier on.
> 
> And rethinking about it, the delay is probably useless :
> 
> bridge_carrier_on = at_least_one_port_has_carrier_on | (bridge_has_no_port & bridge_has_at_least_one_ip)
> 
> That way :
> - for those using bridge without any port, manually setting the IP will assert carrier on. (By the 
> way, why don't they use a dummy device instead?)
> 
> - for those using bridge with ports:
> -- Using any kind of autoconfig will work as expected. Carrier will only be asserted at the time 
> first port get carrier.
> -- Using static IP confifiguration, carrier will possibly be erroneously reported as on during the 
> small time gap between IP address configuration and first port is added to the bridge. This time gap 
> may be removed by simply configuring the IP after the first port is added. This is probably already 
> true for most distribs. And anyway, this time gap is probably not a problem.
> -- Carrier will also be erroneously reported as on after removing the last port, if the bridge still 
> has an IP. (But we can arrange for this not to happen).
> 
> And in order to ensure user really understand why carrier is on of off, we can simply issue an INFO 
> message for the non-natural case (bridge_has_no_port & bridga_has_at_least_one_ip).
> 
> I consider all this reasonable.
> 
> 	Nicolas.

Any bridge behaviour based on IP address configuration is a
layering violation and won't work.  The problem is related to dynamic issues
with IPv6 and DHCP and needs to be addressed at that level.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Krzysztof Olędzki]

On 2011-09-04 18:36, Stephen Hemminger wrote:
> On Sun, 04 Sep 2011 09:35:10 +0200
> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com>  wrote:
>
>> Le 04/09/2011 06:14, Stephen Hemminger a écrit :
>>
>>>> Instead of asserting carrier when the bridge have no port, can't we assert carrier when the three
>>>> following condition are true at the same time :
>>>>
>>>> - The bridge have no port.
>>>> - At least one IP address is setup on the bridge.
>>>> - The two above conditions are true for more than a configurable amount of seconds, with a default
>>>> of 10, for example.
>>>>
>>>> This would only delay carrier on for a few seconds for the regression and keep the current behavior
>>>> (carrier off until at least 1 port is on) for DHCP.
>>>
>>> This fails on two counts:
>>> 1. Bridge's often run without IP addresses!
>>> 2. DHCP won't try and send out request until carrier is true.
>>
>> Sorry, I missed to say that we should of course also assert carrier on if one port has carrier on.
>>
>> And rethinking about it, the delay is probably useless :
>>
>> bridge_carrier_on = at_least_one_port_has_carrier_on | (bridge_has_no_port&  bridge_has_at_least_one_ip)
>>
>> That way :
>> - for those using bridge without any port, manually setting the IP will assert carrier on. (By the
>> way, why don't they use a dummy device instead?)
>>
>> - for those using bridge with ports:
>> -- Using any kind of autoconfig will work as expected. Carrier will only be asserted at the time
>> first port get carrier.
>> -- Using static IP confifiguration, carrier will possibly be erroneously reported as on during the
>> small time gap between IP address configuration and first port is added to the bridge. This time gap
>> may be removed by simply configuring the IP after the first port is added. This is probably already
>> true for most distribs. And anyway, this time gap is probably not a problem.
>> -- Carrier will also be erroneously reported as on after removing the last port, if the bridge still
>> has an IP. (But we can arrange for this not to happen).
>>
>> And in order to ensure user really understand why carrier is on of off, we can simply issue an INFO
>> message for the non-natural case (bridge_has_no_port&  bridga_has_at_least_one_ip).
>>
>> I consider all this reasonable.
>>
>> 	Nicolas.
>
> Any bridge behaviour based on IP address configuration is a
> layering violation and won't work.  The problem is related to dynamic issues
> with IPv6 and DHCP and needs to be addressed at that level.

Maybe we can simply add a switch controlling if a bridge with no 
attached ports has carrier off (default) or on.

For example:
  echo {0|1} > /sys/devices/virtual/net/brX/bridge/orphaned_carrier

  brctl orphaned_carrier brX {on|off}

Best regards,

				Krzysztof Olędzki

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Ang Way Chuang]

On 05/09/11 02:12, Krzysztof Olędzki wrote:
> On 2011-09-04 18:36, Stephen Hemminger wrote:
>> On Sun, 04 Sep 2011 09:35:10 +0200
>> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com>  wrote:
>>
>>> Le 04/09/2011 06:14, Stephen Hemminger a écrit :
>>>
>>>>> Instead of asserting carrier when the bridge have no port, can't we assert carrier when the three
>>>>> following condition are true at the same time :
>>>>>
>>>>> - The bridge have no port.
>>>>> - At least one IP address is setup on the bridge.
>>>>> - The two above conditions are true for more than a configurable amount of seconds, with a default
>>>>> of 10, for example.
>>>>>
>>>>> This would only delay carrier on for a few seconds for the regression and keep the current behavior
>>>>> (carrier off until at least 1 port is on) for DHCP.
>>>>
>>>> This fails on two counts:
>>>> 1. Bridge's often run without IP addresses!
>>>> 2. DHCP won't try and send out request until carrier is true.
>>>
>>> Sorry, I missed to say that we should of course also assert carrier on if one port has carrier on.
>>>
>>> And rethinking about it, the delay is probably useless :
>>>
>>> bridge_carrier_on = at_least_one_port_has_carrier_on | (bridge_has_no_port&  bridge_has_at_least_one_ip)
>>>
>>> That way :
>>> - for those using bridge without any port, manually setting the IP will assert carrier on. (By the
>>> way, why don't they use a dummy device instead?)
>>>
>>> - for those using bridge with ports:
>>> -- Using any kind of autoconfig will work as expected. Carrier will only be asserted at the time
>>> first port get carrier.
>>> -- Using static IP confifiguration, carrier will possibly be erroneously reported as on during the
>>> small time gap between IP address configuration and first port is added to the bridge. This time gap
>>> may be removed by simply configuring the IP after the first port is added. This is probably already
>>> true for most distribs. And anyway, this time gap is probably not a problem.
>>> -- Carrier will also be erroneously reported as on after removing the last port, if the bridge still
>>> has an IP. (But we can arrange for this not to happen).
>>>
>>> And in order to ensure user really understand why carrier is on of off, we can simply issue an INFO
>>> message for the non-natural case (bridge_has_no_port&  bridga_has_at_least_one_ip).
>>>
>>> I consider all this reasonable.
>>>
>>>     Nicolas.
>>
>> Any bridge behaviour based on IP address configuration is a
>> layering violation and won't work.  The problem is related to dynamic issues
>> with IPv6 and DHCP and needs to be addressed at that level.
>
> Maybe we can simply add a switch controlling if a bridge with no attached ports has carrier off (default) or on.
>
> For example:
>  echo {0|1} > /sys/devices/virtual/net/brX/bridge/orphaned_carrier
>
>  brctl orphaned_carrier brX {on|off}
>
> Best regards,
>
>                 Krzysztof Olędzki
My colleague investigated the problem. In the case of libvirt, there are 2 issues at hand:
- dnsmasq cannot bind to port 53 for IPv6 address, therefore fails.
- radvd fails to start because the interface is treated as being down.

This blog entry from Daniel Berrange explains the configuration used by libvirt:
http://berrange.com/posts/2011/06/16/providing-ipv6-connectivity-to-virtual-guests-with-libvirt-and-kvm/
It's quite possible that other users may have the same problem in the future if they plan to use radvd or
dnsmasq on a bridge interface. I'm not sure if there are other applications that depend on the carrier on
a bridge interface to function properly. Come to think of it, unless the user is aware of the change
in carrier on the bridge interface, it is very hard for them to diagnose the problem and apply the right
solution (toggling the carrier). Ideally, there is no reason why dnsmasq and radvd should be started if
there is no port attached to a bridge interface. But, we cannot assume that's how it is done.

I think that it is prudent that the whole commit should be reverted because the problem may not be limited
to libvirt use case.
>
> -- 
> 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
>

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Nicolas de Pesloüan]

Le 04/09/2011 18:36, Stephen Hemminger a écrit :
> On Sun, 04 Sep 2011 09:35:10 +0200
> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com>  wrote:
>
>> Le 04/09/2011 06:14, Stephen Hemminger a écrit :
>>
>>>> Instead of asserting carrier when the bridge have no port, can't we assert carrier when the three
>>>> following condition are true at the same time :
>>>>
>>>> - The bridge have no port.
>>>> - At least one IP address is setup on the bridge.
>>>> - The two above conditions are true for more than a configurable amount of seconds, with a default
>>>> of 10, for example.
>>>>
>>>> This would only delay carrier on for a few seconds for the regression and keep the current behavior
>>>> (carrier off until at least 1 port is on) for DHCP.
>>>
>>> This fails on two counts:
>>> 1. Bridge's often run without IP addresses!
>>> 2. DHCP won't try and send out request until carrier is true.
>>
>> Sorry, I missed to say that we should of course also assert carrier on if one port has carrier on.
>>
>> And rethinking about it, the delay is probably useless :
>>
>> bridge_carrier_on = at_least_one_port_has_carrier_on | (bridge_has_no_port&  bridge_has_at_least_one_ip)
>>
>> That way :
>> - for those using bridge without any port, manually setting the IP will assert carrier on. (By the
>> way, why don't they use a dummy device instead?)
>>
>> - for those using bridge with ports:
>> -- Using any kind of autoconfig will work as expected. Carrier will only be asserted at the time
>> first port get carrier.
>> -- Using static IP confifiguration, carrier will possibly be erroneously reported as on during the
>> small time gap between IP address configuration and first port is added to the bridge. This time gap
>> may be removed by simply configuring the IP after the first port is added. This is probably already
>> true for most distribs. And anyway, this time gap is probably not a problem.
>> -- Carrier will also be erroneously reported as on after removing the last port, if the bridge still
>> has an IP. (But we can arrange for this not to happen).
>>
>> And in order to ensure user really understand why carrier is on of off, we can simply issue an INFO
>> message for the non-natural case (bridge_has_no_port&  bridga_has_at_least_one_ip).
>>
>> I consider all this reasonable.
>>
>> 	Nicolas.
>
> Any bridge behaviour based on IP address configuration is a
> layering violation and won't work.  The problem is related to dynamic issues
> with IPv6 and DHCP and needs to be addressed at that level.

Well, this is not a bridge behavior, this is a behavior of the local (non physical) interface of the 
bridge. If this interface were physical, it would have been external to the bridge (on one of the 
hosts connected to the bridge).  As such, this behavior is not related to the layering of the 
bridge, so I don't consider it a layering violation.

My proposed change doesn't impact the way the bridge works (forwards packets) in any way.

And it really solves both issues.

	Nicolas.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Stephen Hemminger]

The root cause of the problem is applications that don't deal with unresolved
IPv6 addresses. I already had to solve this in our distribution for NTP in a
not bridge related problem. It is better to fix the applications to understand
IPv6 address semantics than to try and force bridge to behave in a way that
is friendly to these applications.

The earlier mail said it is a problem with dnsmasq and radvd. Let's work on understanding
if they need to be updated before jumping in with hacks to the bridge code.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Ang Way Chuang]

On 06/09/11 02:57, Stephen Hemminger wrote:
> The root cause of the problem is applications that don't deal with unresolved
> IPv6 addresses. I already had to solve this in our distribution for NTP in a
> not bridge related problem. It is better to fix the applications to understand
> IPv6 address semantics than to try and force bridge to behave in a way that
> is friendly to these applications.
Care to share the patch that you did for NTP? Perhaps, I may apply the same trick and tried it out on dnsmasq and radvd if I have time.
Thanks.
>
> The earlier mail said it is a problem with dnsmasq and radvd. Let's work on understanding
> if they need to be updated before jumping in with hacks to the bridge code.
> --
> 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
>

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Stephen Hemminger]

On Tue, 06 Sep 2011 04:02:12 +0900
Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:

> On 06/09/11 02:57, Stephen Hemminger wrote:
> > The root cause of the problem is applications that don't deal with unresolved
> > IPv6 addresses. I already had to solve this in our distribution for NTP in a
> > not bridge related problem. It is better to fix the applications to understand
> > IPv6 address semantics than to try and force bridge to behave in a way that
> > is friendly to these applications.
> Care to share the patch that you did for NTP? Perhaps, I may apply the same trick and tried it out on dnsmasq and radvd if I have time.

It has been submitted upstream, but probably an eternity until
it ever gets merged through the NTP project...

The problem happens on our system because NTP runs before interfaces
brought up.



commit 0997ebe40b2835a58ba2ea1e504e38d6d29c95ed
Author: Stephen Hemminger <stephen.hemminger@vyatta.com>
Date:   Tue Oct 26 17:55:04 2010 -0700

    Ignore IPV6 Dynamic addresses
    
    During boot link-local addresses are generated dynamically.
    These addresses are in a tentative state until after resolution occurs.
    While in the tentative state, the address can not be bound to.
    NTP daemon will see the address become available later when it rescans.
    
    (revised patch for 4.2.4p6)

diff --git a/lib/isc/unix/interfaceiter.c b/lib/isc/unix/interfaceiter.c
index 87af69e..64a9f4f 100644
--- a/lib/isc/unix/interfaceiter.c
+++ b/lib/isc/unix/interfaceiter.c
@@ -151,6 +151,7 @@ get_addr(unsigned int family, isc_netaddr_t *dst, struct sockaddr *src,
 static isc_result_t linux_if_inet6_next(isc_interfaceiter_t *);
 static isc_result_t linux_if_inet6_current(isc_interfaceiter_t *);
 static void linux_if_inet6_first(isc_interfaceiter_t *iter);
+#include <linux/if_addr.h>
 #endif
 
 #if HAVE_GETIFADDRS
@@ -216,6 +217,11 @@ linux_if_inet6_current(isc_interfaceiter_t *iter) {
 			      "/proc/net/if_inet6:strlen(%s) != 32", address);
 		return (ISC_R_FAILURE);
 	}
+#ifdef __linux
+	/* Ignore DAD addresses -- can't bind to them till resolved */
+	if (flags & IFA_F_TENTATIVE)
+		return (ISC_R_IGNORE);
+#endif
 	for (i = 0; i < 16; i++) {
 		unsigned char byte;
 		static const char hex[] = "0123456789abcdef";

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Nicolas de Pesloüan]

Le 05/09/2011 19:57, Stephen Hemminger a écrit :
> The root cause of the problem is applications that don't deal with unresolved
> IPv6 addresses. I already had to solve this in our distribution for NTP in a
> not bridge related problem. It is better to fix the applications to understand
> IPv6 address semantics than to try and force bridge to behave in a way that
> is friendly to these applications.

Thanks for clarifying.

> The earlier mail said it is a problem with dnsmasq and radvd. Let's work on understanding
> if they need to be updated before jumping in with hacks to the bridge code.
>

I really support the idea to keep the current behavior (assert carrier on br0 when at least one port 
have carrier) and to fix the applications to wait for the IPv6 address to be checked (DAD) instead 
of dying on bind() failure.

Thanks.

	Nicolas.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Marc Haber]

On Tue, Sep 06, 2011 at 08:52:15AM +0200, Nicolas de Pesloüan wrote:
> I really support the idea to keep the current behavior (assert
> carrier on br0 when at least one port have carrier) and to fix the
> applications to wait for the IPv6 address to be checked (DAD)
> instead of dying on bind() failure.

A really brilliant idea. Break things and then expect hundreds of apps
or millions of installations to adapt.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."    Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Nicolas de Pesloüan]

Le 25/09/2011 11:16, Marc Haber a écrit :
> On Tue, Sep 06, 2011 at 08:52:15AM +0200, Nicolas de Pesloüan wrote:
>> I really support the idea to keep the current behavior (assert
>> carrier on br0 when at least one port have carrier) and to fix the
>> applications to wait for the IPv6 address to be checked (DAD)
>> instead of dying on bind() failure.
>
> A really brilliant idea. Break things and then expect hundreds of apps
> or millions of installations to adapt.

Well... Considering the noise you made recently about the new bridge behavior, I'm sure we all 
understand this is a problem for you.

We try and find a reasonable solution between two somewhat incompatible expectations and this is not 
easy. If you have had a real reading of my proposal, you would have noticed that I really tried to 
find a solution that would be acceptable for both situations. Stephen, which is far more experienced 
than me on this topic explained that my proposal was (and still is) not applicable.

Instead of annoying us with comments like "brilliant idea" or "nice idea", feel free to propose 
something that would contribute to the solution.

	Nicolas.

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[Marc Haber]

On Sun, Sep 04, 2011 at 09:35:10AM +0200, Nicolas de Pesloüan wrote:
> - for those using bridge without any port, manually setting the IP
> will assert carrier on. (By the way, why don't they use a dummy
> device instead?)

Because there is no documentation or recommendation how to use things.
Being a networking guy, I have always treated a bridge as kind of a
switch, where it is natural to say "vlan 120, ip address a.b.c.d". I
just did the same with br0 on all my virtualization hosts, which used
to work until just recently.

Additionally, most distributions' init mechanisms are not very good in
handling dependencies between network interfaces, so writing stuff
like "configure br0, configure dummy0, add dummy0 to br0, wait for
things to settle, start daemons, start VMs" requires tweaking.

Is it recommended to actually have radvd and/or dhcpd listen on the
dummy interface wich is member of a bridge instead of the bridge itself?

> - for those using bridge with ports:
> -- Using any kind of autoconfig will work as expected. Carrier will
> only be asserted at the time first port get carrier.

This only holds when the host with the bridge is the network client,
and only if rdnssd is not used. If the host with the bridge is
supposed to run radvd and/or dhcpd, it is desired to have those
services running before the first port gets attached to the bridge
(which may be hours after the system boot in the desktop
virtualization case).

> -- Using static IP confifiguration, carrier will possibly be
> erroneously reported as on during the small time gap between IP
> address configuration and first port is added to the bridge. This
> time gap may be removed by simply configuring the IP after the first
> port is added. This is probably already true for most distribs. And
> anyway, this time gap is probably not a problem.

You're assuming that the first port gets added immediately after the
port was created. This is not the case, Virtualbox only creates the
vboxnet interface that gets added to the bridge when the VM is
started, which may be a manual operation done by the user at the time
she actually needs the VM.

> -- Carrier will also be erroneously reported as on after removing
> the last port, if the bridge still has an IP.

For my use cases, this is the desired behavior.

>  (But we can arrange for this not to happen).

Please not.

> And in order to ensure user really understand why carrier is on of
> off, we can simply issue an INFO message for the non-natural case
> (bridge_has_no_port & bridga_has_at_least_one_ip).

I fail to understand why this is the non-natural case.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."    Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190

[PATCH 2/2] bridge: set flags in RTM_NEWNEIGH message correctly

[Stephen Hemminger]

[-- Attachment #1: bridge-newneigh-state.patch --]
[-- Type: text/plain, Size: 1409 bytes --]

The functionality for notification was added with 3.0. kernel
but bridge would always send new neighbour message with state == 0.
The problem is that the notify needs to be done after
the flags on ther forwarding engry are set.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---

--- a/net/bridge/br_fdb.c	2011-08-26 09:41:25.966304883 -0700
+++ b/net/bridge/br_fdb.c	2011-09-01 17:21:43.755481630 -0700
@@ -347,7 +347,6 @@ static struct net_bridge_fdb_entry *fdb_
 		fdb->is_static = 0;
 		fdb->updated = fdb->used = jiffies;
 		hlist_add_head_rcu(&fdb->hlist, head);
-		fdb_notify(fdb, RTM_NEWNEIGH);
 	}
 	return fdb;
 }
@@ -379,6 +378,7 @@ static int fdb_insert(struct net_bridge
 		return -ENOMEM;
 
 	fdb->is_local = fdb->is_static = 1;
+	fdb_notify(fdb, RTM_NEWNEIGH);
 	return 0;
 }
 
@@ -424,8 +424,11 @@ void br_fdb_update(struct net_bridge *br
 		}
 	} else {
 		spin_lock(&br->hash_lock);
-		if (likely(!fdb_find(head, addr)))
-			fdb_create(head, source, addr);
+		if (!fdb_find(head, addr)) {
+			fdb = fdb_create(head, source, addr);
+			if (fdb)
+				fdb_notify(fdb, RTM_NEWNEIGH);
+		}
 
 		/* else  we lose race and someone else inserts
 		 * it first, don't bother updating
@@ -576,6 +579,8 @@ static int fdb_add_entry(struct net_brid
 		fdb->is_local = fdb->is_static = 1;
 	else if (state & NUD_NOARP)
 		fdb->is_static = 1;
+
+	fdb_notify(fdb, RTM_NEWNEIGH);
 	return 0;
 }
 

[PATCH 1/2] bridge: leave carrier on for empty bridge

[Stephen Hemminger]

[-- Attachment #1: br-carrier-default.patch --]
[-- Type: text/plain, Size: 1058 bytes --]

This resolves a regression seen by some users of bridging.
Some users use the bridge like a dummy device. 
They expect to be able to put an IPv6 address on the device
with no ports attached. Although there are better ways of doing
this, there is no reason to not allow it.

Note: the bridge still will reflect the state of ports in the
bridge if there are any added.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
This fix needs to go to stable as well since it has
been reported as a regression.


--- a/net/bridge/br_device.c	2011-09-01 08:52:27.596631192 -0700
+++ b/net/bridge/br_device.c	2011-09-01 09:01:03.256611801 -0700
@@ -91,7 +91,6 @@ static int br_dev_open(struct net_device
 {
 	struct net_bridge *br = netdev_priv(dev);
 
-	netif_carrier_off(dev);
 	netdev_update_features(dev);
 	netif_start_queue(dev);
 	br_stp_enable_bridge(br);
@@ -108,8 +107,6 @@ static int br_dev_stop(struct net_device
 {
 	struct net_bridge *br = netdev_priv(dev);
 
-	netif_carrier_off(dev);
-
 	br_stp_disable_bridge(br);
 	br_multicast_stop(br);
 

Re: [PATCH 1/2] bridge: leave carrier on for empty bridge

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 03 Oct 2011 21:14:45 -0700

> This resolves a regression seen by some users of bridging.
> Some users use the bridge like a dummy device. 
> They expect to be able to put an IPv6 address on the device
> with no ports attached. Although there are better ways of doing
> this, there is no reason to not allow it.
> 
> Note: the bridge still will reflect the state of ports in the
> bridge if there are any added.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied to 'net' and queued up for -stable.