Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: fix hardware bridging

[Vivien Didelot]

On Oct. Wednesday 14 (42) 06:44 PM, Florian Fainelli wrote:
> On 14/10/15 18:28, Vivien Didelot wrote:
> > On Oct. Thursday 15 (42) 12:46 AM, Andrew Lunn wrote:
> >> On Sun, Oct 11, 2015 at 06:08:34PM -0400, Vivien Didelot wrote:
> >>> DSA and its drivers currently hook the NETDEV_CHANGEUPPER net_device 
> >>> event in
> >>> order to configure the VLAN map of every port.
> >>>
> >>> This VLAN map is a feature of these switch chips to hardcode and restrict 
> >>> which
> >>> output ports a given input port can egress frames to.
> >>>
> >>> A Linux bridge is a simple untagged VLAN propagated by the bridge code 
> >>> itself.
> >>> With a proper 802.1Q support, a driver does not need this hook anymore, 
> >>> and
> >>> will simply program the related VLAN object.
> >>>
> >>> This patchset improves the hardware bridging code in the mv88e6xxx driver 
> >>> with
> >>> a strict 802.1Q mode.
> >>
> >> Hi Vivien
> >>
> >> I just tested this as part of net-next/master, and found a problem
> >>
> >> If i do:
> >>
> >> ip link set lan0 up
> >> ip addr add 192.168.10.2/24 dev lan0
> >>
> >> It will not ping. Looking in sys/kernel/debug/dsa0/stats i see
> >> broadcast packets, probably ARP, being received at the port.
> >> But they are not being forwarded out the CPU port.
> >>
> >> If however i do
> >>
> >> brctl addbr br0
> >> brctl addif br0 lan0
> >> ip addr add 192.168.10.2/24 dev br0
> >> ip link set br0 up
> >>
> >> i can ping.
> >>
> >> So it looks like we are too restrictive by default. You should be able
> >> to use interfaces as they are, without a bridge.
> > 
> > Correct, if the ports are not in a VLAN by default, they cannot talk.
> 
> The expectation for DSA devices, if no bridge device is configured is to
> have each port be able to talk to the CPU port only, but this has to
> work out of the box.

OK, I might have forgotten this requirement. I also just noticed that
you mentioned it in Documentation/networking/dsa/dsa.txt. Thanks for the
reminder.

> > 
> > If you want to, I think the special VLAN 0 can be used for that
> > purpose. IIRC, in a given configuration, Linux add the interfaces
> > (thus programs the hardware) with VLAN 0. I'm not sure when, maybe
> > when the .ndo_vlan_rx_add_vid is implemented, I need to give it a
> > shot.
> 
> But if you do that, won't that put all DSA ports into VLAN 0? Would
> not that break isolation between each ports as expected for a DSA
> switch?

You're correct, then VLAN 0 is not an option. I have something else in
mind, fix coming soon.

> > 
> > Otherwise, I can send you a patch configuring the VLAN 0 on switch
> > setup if this is the behavior we want.

Thanks,
-v
--
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

Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: fix hardware bridging

[Vivien Didelot]

On Oct. Thursday 15 (42) 04:52 AM, Andrew Lunn wrote:
> On Wed, Oct 14, 2015 at 09:28:55PM -0400, Vivien Didelot wrote:
> > On Oct. Thursday 15 (42) 12:46 AM, Andrew Lunn wrote:
> > > On Sun, Oct 11, 2015 at 06:08:34PM -0400, Vivien Didelot wrote:
> > > > DSA and its drivers currently hook the NETDEV_CHANGEUPPER net_device 
> > > > event in
> > > > order to configure the VLAN map of every port.
> > > > 
> > > > This VLAN map is a feature of these switch chips to hardcode and 
> > > > restrict which
> > > > output ports a given input port can egress frames to.
> > > > 
> > > > A Linux bridge is a simple untagged VLAN propagated by the bridge code 
> > > > itself.
> > > > With a proper 802.1Q support, a driver does not need this hook anymore, 
> > > > and
> > > > will simply program the related VLAN object.
> > > > 
> > > > This patchset improves the hardware bridging code in the mv88e6xxx 
> > > > driver with
> > > > a strict 802.1Q mode.
> > > 
> > > Hi Vivien
> > > 
> > > I just tested this as part of net-next/master, and found a problem
> > > 
> > > If i do:
> > > 
> > > ip link set lan0 up
> > > ip addr add 192.168.10.2/24 dev lan0
> > > 
> > > It will not ping. Looking in sys/kernel/debug/dsa0/stats i see
> > > broadcast packets, probably ARP, being received at the port.
> > > But they are not being forwarded out the CPU port.
> > > 
> > > If however i do
> > > 
> > > brctl addbr br0
> > > brctl addif br0 lan0
> > > ip addr add 192.168.10.2/24 dev br0
> > > ip link set br0 up
> > > 
> > > i can ping.
> > > 
> > > So it looks like we are too restrictive by default. You should be able
> > > to use interfaces as they are, without a bridge.
> > 
> > Correct, if the ports are not in a VLAN by default, they cannot talk.
> 
> Hi Vivien 
> 
> This is a regression. Ports of the switch should work like normal
> Linux interfaces. And up until now, they did. This patchset changed
> that.
> 
> As Florian pointed out, these interfaces are separated from each
> other. So you need something like a bridge per port by default, which
> then gets removed and replaced when a port is added to a Linux bridge.

I'll fix this regression and try the exact same example you provided.

> We also need to take care of VLANs. When the port is not a member of a
> linux bridge, i expect all VLAN tagged frames to be received, as well
> as untagged frames. This is normal Linux behaviour. But i never got
> around to testing this with DSA.

Thanks for testing,
-v
--
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

Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: fix hardware bridging

[Andrew Lunn]

On Sun, Oct 11, 2015 at 06:08:34PM -0400, Vivien Didelot wrote:
> DSA and its drivers currently hook the NETDEV_CHANGEUPPER net_device event in
> order to configure the VLAN map of every port.
> 
> This VLAN map is a feature of these switch chips to hardcode and restrict 
> which
> output ports a given input port can egress frames to.
> 
> A Linux bridge is a simple untagged VLAN propagated by the bridge code itself.
> With a proper 802.1Q support, a driver does not need this hook anymore, and
> will simply program the related VLAN object.
> 
> This patchset improves the hardware bridging code in the mv88e6xxx driver with
> a strict 802.1Q mode.

Hi Vivien

I just tested this as part of net-next/master, and found a problem

If i do:

ip link set lan0 up
ip addr add 192.168.10.2/24 dev lan0

It will not ping. Looking in sys/kernel/debug/dsa0/stats i see
broadcast packets, probably ARP, being received at the port.
But they are not being forwarded out the CPU port.

If however i do

brctl addbr br0
brctl addif br0 lan0
ip addr add 192.168.10.2/24 dev br0
ip link set br0 up

i can ping.

So it looks like we are too restrictive by default. You should be able
to use interfaces as they are, without a bridge.

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

Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: fix hardware bridging

[Vivien Didelot]

On Oct. Thursday 15 (42) 12:46 AM, Andrew Lunn wrote:
> On Sun, Oct 11, 2015 at 06:08:34PM -0400, Vivien Didelot wrote:
> > DSA and its drivers currently hook the NETDEV_CHANGEUPPER net_device event 
> > in
> > order to configure the VLAN map of every port.
> > 
> > This VLAN map is a feature of these switch chips to hardcode and restrict 
> > which
> > output ports a given input port can egress frames to.
> > 
> > A Linux bridge is a simple untagged VLAN propagated by the bridge code 
> > itself.
> > With a proper 802.1Q support, a driver does not need this hook anymore, and
> > will simply program the related VLAN object.
> > 
> > This patchset improves the hardware bridging code in the mv88e6xxx driver 
> > with
> > a strict 802.1Q mode.
> 
> Hi Vivien
> 
> I just tested this as part of net-next/master, and found a problem
> 
> If i do:
> 
> ip link set lan0 up
> ip addr add 192.168.10.2/24 dev lan0
> 
> It will not ping. Looking in sys/kernel/debug/dsa0/stats i see
> broadcast packets, probably ARP, being received at the port.
> But they are not being forwarded out the CPU port.
> 
> If however i do
> 
> brctl addbr br0
> brctl addif br0 lan0
> ip addr add 192.168.10.2/24 dev br0
> ip link set br0 up
> 
> i can ping.
> 
> So it looks like we are too restrictive by default. You should be able
> to use interfaces as they are, without a bridge.

Correct, if the ports are not in a VLAN by default, they cannot talk.

If you want to, I think the special VLAN 0 can be used for that purpose.
IIRC, in a given configuration, Linux add the interfaces (thus programs
the hardware) with VLAN 0. I'm not sure when, maybe when the
.ndo_vlan_rx_add_vid is implemented, I need to give it a shot.

Otherwise, I can send you a patch configuring the VLAN 0 on switch
setup if this is the behavior we want.

Thanks,
-v
--
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

Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: fix hardware bridging

[Andrew Lunn]

On Wed, Oct 14, 2015 at 09:28:55PM -0400, Vivien Didelot wrote:
> On Oct. Thursday 15 (42) 12:46 AM, Andrew Lunn wrote:
> > On Sun, Oct 11, 2015 at 06:08:34PM -0400, Vivien Didelot wrote:
> > > DSA and its drivers currently hook the NETDEV_CHANGEUPPER net_device 
> > > event in
> > > order to configure the VLAN map of every port.
> > > 
> > > This VLAN map is a feature of these switch chips to hardcode and restrict 
> > > which
> > > output ports a given input port can egress frames to.
> > > 
> > > A Linux bridge is a simple untagged VLAN propagated by the bridge code 
> > > itself.
> > > With a proper 802.1Q support, a driver does not need this hook anymore, 
> > > and
> > > will simply program the related VLAN object.
> > > 
> > > This patchset improves the hardware bridging code in the mv88e6xxx driver 
> > > with
> > > a strict 802.1Q mode.
> > 
> > Hi Vivien
> > 
> > I just tested this as part of net-next/master, and found a problem
> > 
> > If i do:
> > 
> > ip link set lan0 up
> > ip addr add 192.168.10.2/24 dev lan0
> > 
> > It will not ping. Looking in sys/kernel/debug/dsa0/stats i see
> > broadcast packets, probably ARP, being received at the port.
> > But they are not being forwarded out the CPU port.
> > 
> > If however i do
> > 
> > brctl addbr br0
> > brctl addif br0 lan0
> > ip addr add 192.168.10.2/24 dev br0
> > ip link set br0 up
> > 
> > i can ping.
> > 
> > So it looks like we are too restrictive by default. You should be able
> > to use interfaces as they are, without a bridge.
> 
> Correct, if the ports are not in a VLAN by default, they cannot talk.

Hi Vivien 

This is a regression. Ports of the switch should work like normal
Linux interfaces. And up until now, they did. This patchset changed
that.

As Florian pointed out, these interfaces are separated from each
other. So you need something like a bridge per port by default, which
then gets removed and replaced when a port is added to a Linux bridge.

We also need to take care of VLANs. When the port is not a member of a
linux bridge, i expect all VLAN tagged frames to be received, as well
as untagged frames. This is normal Linux behaviour. But i never got
around to testing this with DSA.

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

Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: fix hardware bridging

[Guenter Roeck]

On 10/14/2015 07:52 PM, Andrew Lunn wrote:

On Wed, Oct 14, 2015 at 09:28:55PM -0400, Vivien Didelot wrote:

On Oct. Thursday 15 (42) 12:46 AM, Andrew Lunn wrote:

On Sun, Oct 11, 2015 at 06:08:34PM -0400, Vivien Didelot wrote:

DSA and its drivers currently hook the NETDEV_CHANGEUPPER net_device event in
order to configure the VLAN map of every port.

This VLAN map is a feature of these switch chips to hardcode and restrict which
output ports a given input port can egress frames to.

A Linux bridge is a simple untagged VLAN propagated by the bridge code itself.
With a proper 802.1Q support, a driver does not need this hook anymore, and
will simply program the related VLAN object.

This patchset improves the hardware bridging code in the mv88e6xxx driver with
a strict 802.1Q mode.


Hi Vivien

I just tested this as part of net-next/master, and found a problem

If i do:

ip link set lan0 up
ip addr add 192.168.10.2/24 dev lan0

It will not ping. Looking in sys/kernel/debug/dsa0/stats i see
broadcast packets, probably ARP, being received at the port.
But they are not being forwarded out the CPU port.

If however i do

brctl addbr br0
brctl addif br0 lan0
ip addr add 192.168.10.2/24 dev br0
ip link set br0 up

i can ping.

So it looks like we are too restrictive by default. You should be able
to use interfaces as they are, without a bridge.


Correct, if the ports are not in a VLAN by default, they cannot talk.


Hi Vivien

This is a regression. Ports of the switch should work like normal
Linux interfaces. And up until now, they did. This patchset changed
that.

As Florian pointed out, these interfaces are separated from each
other. So you need something like a bridge per port by default, which
then gets removed and replaced when a port is added to a Linux bridge.

We also need to take care of VLANs. When the port is not a member of a
linux bridge, i expect all VLAN tagged frames to be received, as well
as untagged frames. This is normal Linux behaviour. But i never got
around to testing this with DSA.



There was a reason for the original code. I had wondered how it is now
supposed to work. Guess this exchange explains it. Looking forward to see
how it is going to be fixed, and too bad I don't have time to be more
involved.

Guenter

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

Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: fix hardware bridging

[Florian Fainelli]

On 14/10/15 18:28, Vivien Didelot wrote:
> On Oct. Thursday 15 (42) 12:46 AM, Andrew Lunn wrote:
>> On Sun, Oct 11, 2015 at 06:08:34PM -0400, Vivien Didelot wrote:
>>> DSA and its drivers currently hook the NETDEV_CHANGEUPPER net_device event 
>>> in
>>> order to configure the VLAN map of every port.
>>>
>>> This VLAN map is a feature of these switch chips to hardcode and restrict 
>>> which
>>> output ports a given input port can egress frames to.
>>>
>>> A Linux bridge is a simple untagged VLAN propagated by the bridge code 
>>> itself.
>>> With a proper 802.1Q support, a driver does not need this hook anymore, and
>>> will simply program the related VLAN object.
>>>
>>> This patchset improves the hardware bridging code in the mv88e6xxx driver 
>>> with
>>> a strict 802.1Q mode.
>>
>> Hi Vivien
>>
>> I just tested this as part of net-next/master, and found a problem
>>
>> If i do:
>>
>> ip link set lan0 up
>> ip addr add 192.168.10.2/24 dev lan0
>>
>> It will not ping. Looking in sys/kernel/debug/dsa0/stats i see
>> broadcast packets, probably ARP, being received at the port.
>> But they are not being forwarded out the CPU port.
>>
>> If however i do
>>
>> brctl addbr br0
>> brctl addif br0 lan0
>> ip addr add 192.168.10.2/24 dev br0
>> ip link set br0 up
>>
>> i can ping.
>>
>> So it looks like we are too restrictive by default. You should be able
>> to use interfaces as they are, without a bridge.
> 
> Correct, if the ports are not in a VLAN by default, they cannot talk.

The expectation for DSA devices, if no bridge device is configured is to
have each port be able to talk to the CPU port only, but this has to
work out of the box.

> 
> If you want to, I think the special VLAN 0 can be used for that purpose.
> IIRC, in a given configuration, Linux add the interfaces (thus programs
> the hardware) with VLAN 0. I'm not sure when, maybe when the
> .ndo_vlan_rx_add_vid is implemented, I need to give it a shot.

But if you do that, won't that put all DSA ports into VLAN 0? Would not
that break isolation between each ports as expected for a DSA switch?

> 
> Otherwise, I can send you a patch configuring the VLAN 0 on switch
> setup if this is the behavior we want.
> 
> Thanks,
> -v
> 


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

Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: fix hardware bridging

[David Miller]

From: Vivien Didelot 
Date: Sun, 11 Oct 2015 18:08:34 -0400

> DSA and its drivers currently hook the NETDEV_CHANGEUPPER net_device event in
> order to configure the VLAN map of every port.
> 
> This VLAN map is a feature of these switch chips to hardcode and restrict 
> which
> output ports a given input port can egress frames to.
> 
> A Linux bridge is a simple untagged VLAN propagated by the bridge code itself.
> With a proper 802.1Q support, a driver does not need this hook anymore, and
> will simply program the related VLAN object.
> 
> This patchset improves the hardware bridging code in the mv88e6xxx driver with
> a strict 802.1Q mode.
> 
> Ideally, the equivalent must be done for Broadcom Starfighter 2 and Rocker,
> before completely getting rid of this hook.

Series applied, thanks Vivien.
--
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