RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread Rodney Cummings
I stand corrected. Thanks for clearing that up Andrew.

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Monday, October 16, 2017 11:57 AM
> To: Rodney Cummings 
> Cc: Vivien Didelot ;
> netdev@vger.kernel.org; linux-ker...@vger.kernel.org;
> ker...@savoirfairelinux.com; David S. Miller ;
> Florian Fainelli ; David Laight
> 
> Subject: Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
> 
> > How do you guarantee that this MAC address is used for PAUSE, and only
> for PAUSE?
> 
> That is the only thing the data sheet says this MAC address is used
> for.
> 
>   Andrew


Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread Andrew Lunn
> How do you guarantee that this MAC address is used for PAUSE, and only for 
> PAUSE?

That is the only thing the data sheet says this MAC address is used
for.

Andrew


RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread Rodney Cummings
PAUSE is only one higher layer entity. The source MAC address may not matter 
there, but it definitely matters for other higher layer entities (like IEEE Std 
1588).

The Marvell switch can run more than just PAUSE as a higher layer protocol.

How do you guarantee that this MAC address is used for PAUSE, and only for 
PAUSE?

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Monday, October 16, 2017 11:48 AM
> To: Rodney Cummings 
> Cc: Vivien Didelot ;
> netdev@vger.kernel.org; linux-ker...@vger.kernel.org;
> ker...@savoirfairelinux.com; David S. Miller ;
> Florian Fainelli ; David Laight
> 
> Subject: Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
> 
> On Mon, Oct 16, 2017 at 04:28:04PM +, Rodney Cummings wrote:
> > Hi Andrew,
> >
> > I may have misunderstood.
> >
> > If this MAC address is the destination
> 
> Nope. This is the source address, for Pause frames.
> 
> > My concern is that for a source MAC address, a local random MAC
> > address is not safe in all networks, because it has the potential
> > for duplication. That topic has been discussed quite a bit in IEEE
> > 802.
> 
> Duplications don't matter, for pause frames. The source address
> appears to be unused. And these frames don't get passed the direct
> peers MAC layer.
> 
>   Andrew


Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread Andrew Lunn
On Mon, Oct 16, 2017 at 04:28:04PM +, Rodney Cummings wrote:
> Hi Andrew,
> 
> I may have misunderstood.
> 
> If this MAC address is the destination

Nope. This is the source address, for Pause frames.

> My concern is that for a source MAC address, a local random MAC
> address is not safe in all networks, because it has the potential
> for duplication. That topic has been discussed quite a bit in IEEE
> 802.

Duplications don't matter, for pause frames. The source address
appears to be unused. And these frames don't get passed the direct
peers MAC layer.

Andrew


Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread Andrew Lunn
On Mon, Oct 16, 2017 at 04:30:34PM +, David Laight wrote:
> From: Andrew Lunn
> > Sent: 16 October 2017 17:10
> ...
> > So, received Pause frames never leave the MAC. They don't get bridged,
> > nor do they get passed up for host processing. They are purely point
> > to point between two MAC peers. The destination is unambiguous. It is
> > simple the other MAC peer. The destination address makes it clear it
> > is a pause frame, the the source address seems to be unneeded.
> > 
> > In this context, a random MAC addresses are safe.
> 
> Is there any reason why a fixed value (say 00:00:00:00:00:00)
> can't be used?

I was going to suggest 42:42:42:42:42:42 :-)

  Andrew


RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread David Laight
From: Andrew Lunn
> Sent: 16 October 2017 17:10
...
> So, received Pause frames never leave the MAC. They don't get bridged,
> nor do they get passed up for host processing. They are purely point
> to point between two MAC peers. The destination is unambiguous. It is
> simple the other MAC peer. The destination address makes it clear it
> is a pause frame, the the source address seems to be unneeded.
> 
> In this context, a random MAC addresses are safe.

Is there any reason why a fixed value (say 00:00:00:00:00:00)
can't be used?

> In the more general case, i would agree with you. Collisions are
> possible, causing problems.

For IP MAC addresses only go as far as the first router.
So the duplicates would (typically) have to be within the same subnet.
This makes the chance of a duplicate random address unlikely.

(Unless you have an un-subnetted class A network consisting of
multiple 1km long coax segments connected by 1km long repeaters
making a single collision domain.)

David



RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread Rodney Cummings
Hi Andrew,

I may have misunderstood.

If this MAC address is the destination, a local random MAC address doesn't 
work. PAUSE is using one of the 802.1Q reserved destination MAC addresses (as 
does LLDP, MSTP, 1588, etc). PAUSE needs to use that specially assigned address 
(01-80-C2-00-00-01).

I assumed we were talking about the source MAC address. For the source MAC 
address, I would agree that a local random MAC address works in some narrow 
scenarios.

My concern is that for a source MAC address, a local random MAC address is not 
safe in all networks, because it has the potential for duplication. That topic 
has been discussed quite a bit in IEEE 802.

Rodney

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Monday, October 16, 2017 11:10 AM
> To: Rodney Cummings 
> Cc: Vivien Didelot ;
> netdev@vger.kernel.org; linux-ker...@vger.kernel.org;
> ker...@savoirfairelinux.com; David S. Miller ;
> Florian Fainelli ; David Laight
> 
> Subject: Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
> 
> On Mon, Oct 16, 2017 at 03:23:42PM +, Rodney Cummings wrote:
> > I am concerned about this proposed change.
> >
> > According to IEEE Std 802.1Q, a "higher layer entity" (i.e. end
> > station) that is internal to the bridge (i.e. switch) can use the
> > same individual global MAC address that the switch is using. That
> > behavior is common.
> >
> > Use of a local semi-random MAC address is dangerous on
> > Ethernet. Local MAC addresses can only be safely used in very narrow
> > use cases, most of which are Wi-Fi. Generally speaking, local MAC
> > addresses can be duplicated in the network, which causes many
> > Ethernet protocols to break down. Therefore, general-purpose
> > implementations like DSA cannot use a local MAC address in a
> > reliable manner.
> 
> Hi Rodney
> 
> This is interesting. So i did some research. The Marvell Switch only
> uses this MAC address for Pause frames. Nothing else.
> 
> 802.3-2015 Section 2, Annex 31B says:
> 
>   The globally assigned 48-bit multicast address 01-80-C2-00-00-01 has
>   been reserved for use in MAC Control PAUSE frames for inhibiting
>   transmission of data frames from a DTE in a full duplex mode IEEE
>   802.3 LAN. IEEE 802.1D-conformant bridges will not forward frames
>   sent to this multicast destination address, regardless of the state
>   of the bridge’s ports, or whether or not the bridge
>   implements the MAC Control sublayer. To allow generic full duplex
>   flow control, stations implementing the PAUSE operation shall
>   instruct the MAC (e.g., through layer management) to enable
>   reception of frames with destination address equal to this multicast
>   address.
> 
>   NOTE—By definition, an IEEE 802.3 LAN operating in full
>   duplex mode comprises exactly two stations, thus there is no
>   ambiguity regarding the destination DTE’s identity. The use
>   of a well-known multicast address relieves the MAC Control sublayer
>   and its client from having to know, and maintain knowledge of, the
>   individual 48-bit address of the other DTE in a full duplex
>   environment.
> 
>   When MAC Control PFC operation (see Annex 31D and IEEE Std 802.1Q)
>   has been enabled, MAC Control PAUSE operation shall be disabled.
> 
> So, received Pause frames never leave the MAC. They don't get bridged,
> nor do they get passed up for host processing. They are purely point
> to point between two MAC peers. The destination is unambiguous. It is
> simple the other MAC peer. The destination address makes it clear it
> is a pause frame, the the source address seems to be unneeded.
> 
> In this context, a random MAC addresses are safe.
> 
> In the more general case, i would agree with you. Collisions are
> possible, causing problems.
> 
>Andrew


Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread Andrew Lunn
On Mon, Oct 16, 2017 at 03:23:42PM +, Rodney Cummings wrote:
> I am concerned about this proposed change.
> 
> According to IEEE Std 802.1Q, a "higher layer entity" (i.e. end
> station) that is internal to the bridge (i.e. switch) can use the
> same individual global MAC address that the switch is using. That
> behavior is common.
>
> Use of a local semi-random MAC address is dangerous on
> Ethernet. Local MAC addresses can only be safely used in very narrow
> use cases, most of which are Wi-Fi. Generally speaking, local MAC
> addresses can be duplicated in the network, which causes many
> Ethernet protocols to break down. Therefore, general-purpose
> implementations like DSA cannot use a local MAC address in a
> reliable manner.

Hi Rodney

This is interesting. So i did some research. The Marvell Switch only
uses this MAC address for Pause frames. Nothing else.

802.3-2015 Section 2, Annex 31B says:

  The globally assigned 48-bit multicast address 01-80-C2-00-00-01 has
  been reserved for use in MAC Control PAUSE frames for inhibiting
  transmission of data frames from a DTE in a full duplex mode IEEE
  802.3 LAN. IEEE 802.1D-conformant bridges will not forward frames
  sent to this multicast destination address, regardless of the state
  of the bridge’s ports, or whether or not the bridge
  implements the MAC Control sublayer. To allow generic full duplex
  flow control, stations implementing the PAUSE operation shall
  instruct the MAC (e.g., through layer management) to enable
  reception of frames with destination address equal to this multicast
  address.

  NOTE—By definition, an IEEE 802.3 LAN operating in full
  duplex mode comprises exactly two stations, thus there is no
  ambiguity regarding the destination DTE’s identity. The use
  of a well-known multicast address relieves the MAC Control sublayer
  and its client from having to know, and maintain knowledge of, the
  individual 48-bit address of the other DTE in a full duplex
  environment.

  When MAC Control PFC operation (see Annex 31D and IEEE Std 802.1Q)
  has been enabled, MAC Control PAUSE operation shall be disabled.

So, received Pause frames never leave the MAC. They don't get bridged,
nor do they get passed up for host processing. They are purely point
to point between two MAC peers. The destination is unambiguous. It is
simple the other MAC peer. The destination address makes it clear it
is a pause frame, the the source address seems to be unneeded.

In this context, a random MAC addresses are safe.

In the more general case, i would agree with you. Collisions are
possible, causing problems.

   Andrew


RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-16 Thread Rodney Cummings
I am concerned about this proposed change.

According to IEEE Std 802.1Q, a "higher layer entity" (i.e. end station) that 
is internal to the bridge (i.e. switch) can use the same individual global MAC 
address that the switch is using. That behavior is common.

Use of a local semi-random MAC address is dangerous on Ethernet. Local MAC 
addresses can only be safely used in very narrow use cases, most of which are 
Wi-Fi. Generally speaking, local MAC addresses can be duplicated in the 
network, which causes many Ethernet protocols to break down. Therefore, 
general-purpose implementations like DSA cannot use a local MAC address in a 
reliable manner.

> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of Vivien Didelot
> Sent: Friday, October 13, 2017 1:18 PM
> To: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; ker...@savoirfairelinux.com; David S.
> Miller ; Florian Fainelli ;
> Andrew Lunn ; David Laight ;
> Vivien Didelot 
> Subject: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
> 
> An Ethernet switch may support having a MAC address, which can be used
> as the switch's source address in transmitted full-duplex Pause frames.
> 
> If a DSA switch supports the related .set_addr operation, the DSA core
> sets the master's MAC address on the switch.
> 
> This won't make sense anymore in a multi-CPU ports system, because there
> won't be a unique master device assigned to a switch tree.
> 
> Moreover this operation is confusing because it makes the user think
> that it could be used to program the switch with the MAC address of the
> CPU/management port such that MAC address learning can be disabled on
> said port, but in fact, that's not how it is currently used.
> 
> To fix this, assign a random MAC address at setup time in the mv88e6060
> and mv88e6xxx drivers before removing .set_addr completely from DSA.
> 
> Changes in v3:
>   - include fix for mv88e6060 switch MAC address setter.
> 
> Changes in v2:
>   - remove .set_addr implementation from drivers and use a random MAC.
> 
> Vivien Didelot (5):
>   net: dsa: mv88e6xxx: setup random mac address
>   net: dsa: mv88e6060: fix switch MAC address
>   net: dsa: mv88e6060: setup random mac address
>   net: dsa: dsa_loop: remove .set_addr
>   net: dsa: remove .set_addr
> 
>  drivers/net/dsa/dsa_loop.c   |  8 
>  drivers/net/dsa/mv88e6060.c  | 37 ++-
> --
>  drivers/net/dsa/mv88e6xxx/chip.c | 33 +
>  include/net/dsa.h|  1 -
>  net/dsa/dsa2.c   |  6 --
>  net/dsa/legacy.c |  6 --
>  6 files changed, 43 insertions(+), 48 deletions(-)
> 
> --
> 2.14.2



Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

2017-10-14 Thread David Miller
From: Vivien Didelot 
Date: Fri, 13 Oct 2017 14:18:04 -0400

> An Ethernet switch may support having a MAC address, which can be used
> as the switch's source address in transmitted full-duplex Pause frames.
> 
> If a DSA switch supports the related .set_addr operation, the DSA core
> sets the master's MAC address on the switch.
> 
> This won't make sense anymore in a multi-CPU ports system, because there
> won't be a unique master device assigned to a switch tree.
> 
> Moreover this operation is confusing because it makes the user think
> that it could be used to program the switch with the MAC address of the
> CPU/management port such that MAC address learning can be disabled on
> said port, but in fact, that's not how it is currently used.
> 
> To fix this, assign a random MAC address at setup time in the mv88e6060
> and mv88e6xxx drivers before removing .set_addr completely from DSA.
> 
> Changes in v3:
>   - include fix for mv88e6060 switch MAC address setter.
> 
> Changes in v2:
>   - remove .set_addr implementation from drivers and use a random MAC.

Series applied,thanks Vivien.