[RFC PATCH net-next 00/12] Documentation updates for switchdev and DSA

[RFC PATCH net-next 00/12] Documentation updates for switchdev and DSA

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Many changes were made to the code but of course the documentation was
not kept up to date. This is an attempt to update some of the verbiage.
Suggestions for improvement would be very much appreciated.

Florian Fainelli (1):
  Documentation: networking: switchdev: clarify device driver behavior

Vladimir Oltean (11):
  Documentation: networking: update the graphical representation
  Documentation: networking: dsa: rewrite chapter about tagging protocol
  Documentation: networking: dsa: remove static port count from
    limitations
  Documentation: networking: dsa: remove references to switchdev
    prepare/commit
  Documentation: networking: dsa: remove TODO about porting more vendor
    drivers
  Documentation: networking: dsa: document the port_bridge_flags method
  Documentation: networking: dsa: mention integration with devlink
  Documentation: networking: dsa: add paragraph for the LAG offload
  Documentation: networking: dsa: add paragraph for the MRP offload
  Documentation: networking: dsa: add paragraph for the HSR/PRP offload
  Documentation: networking: switchdev: fix command for static FDB
    entries

 Documentation/networking/dsa/dsa.rst   | 344 ++++++++++++++++++++-----
 Documentation/networking/switchdev.rst | 167 +++++++++++-
 2 files changed, 441 insertions(+), 70 deletions(-)

-- 
2.25.1

[RFC PATCH net-next 01/12] Documentation: networking: update the graphical representation

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

While preparing some slides for a customer presentation, I found the
existing high-level view to be a bit confusing, so I modified it a
little bit.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 45 +++++++++++++++++-----------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index e9517af5fe02..e20fbad2241a 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -172,23 +172,34 @@ Graphical representation
 Summarized, this is basically how DSA looks like from a network device
 perspective::
 
-
-                |---------------------------
-                | CPU network device (eth0)|
-                ----------------------------
-                | <tag added by switch     |
-                |                          |
-                |                          |
-                |        tag added by CPU> |
-        |--------------------------------------------|
-        |            Switch driver                   |
-        |--------------------------------------------|
-                  ||        ||         ||
-              |-------|  |-------|  |-------|
-              | sw0p0 |  | sw0p1 |  | sw0p2 |
-              |-------|  |-------|  |-------|
-
-
+                Unaware application
+              opens and binds socket
+                       |  ^
+                       |  |
+           +-----------v--|--------------------+
+           |+------+ +------+ +------+ +------+|
+           || swp0 | | swp1 | | swp2 | | swp3 ||
+           |+------+-+------+-+------+-+------+|
+           |          DSA switch driver        |
+           +-----------------------------------+
+                         |        ^
+            Tag added by |        | Tag consumed by
+           switch driver |        | switch driver
+                         v        |
+           +-----------------------------------+
+           | Unmodified host interface driver  | Software
+   --------+-----------------------------------+------------
+           |       Host interface (eth0)       | Hardware
+           +-----------------------------------+
+                         |        ^
+         Tag consumed by |        | Tag added by
+         switch hardware |        | switch hardware
+                         v        |
+           +-----------------------------------+
+           |               Switch              |
+           |+------+ +------+ +------+ +------+|
+           || swp0 | | swp1 | | swp2 | | swp3 ||
+           ++------+-+------+-+------+-+------++
 
 Slave MDIO bus
 --------------
-- 
2.25.1

Re: [RFC PATCH net-next 01/12] Documentation: networking: update the graphical representation

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> While preparing some slides for a customer presentation, I found the
> existing high-level view to be a bit confusing, so I modified it a
> little bit.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 01/12] Documentation: networking: update the graphical representation

[Tobias Waldekranz]

On Sun, Feb 21, 2021 at 23:33, Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> While preparing some slides for a customer presentation, I found the
> existing high-level view to be a bit confusing, so I modified it a
> little bit.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Reviewed-by: Tobias Waldekranz <tobias@waldekranz.com>

[RFC PATCH net-next 02/12] Documentation: networking: dsa: rewrite chapter about tagging protocol

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The chapter about tagging protocols is out of date because it doesn't
mention all taggers that have been added since last documentation
update. But judging based on that, it will always tend to lag behind,
and there's no good reason why we would enumerate the supported
hardware. Instead we could do something more useful and explain what
there is to know about tagging protocols instead.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 126 +++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 8 deletions(-)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index e20fbad2241a..fc98b5774fb6 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -65,14 +65,8 @@ Note that DSA does not currently create network interfaces for the "cpu" and
 Switch tagging protocols
 ------------------------
 
-DSA currently supports 5 different tagging protocols, and a tag-less mode as
-well. The different protocols are implemented in:
-
-- ``net/dsa/tag_trailer.c``: Marvell's 4 trailer tag mode (legacy)
-- ``net/dsa/tag_dsa.c``: Marvell's original DSA tag
-- ``net/dsa/tag_edsa.c``: Marvell's enhanced DSA tag
-- ``net/dsa/tag_brcm.c``: Broadcom's 4 bytes tag
-- ``net/dsa/tag_qca.c``: Qualcomm's 2 bytes tag
+DSA supports many vendor-specific tagging protocols, one software-defined
+tagging protocol, and a tag-less mode as well (``DSA_TAG_PROTO_NONE``).
 
 The exact format of the tag protocol is vendor specific, but in general, they
 all contain something which:
@@ -80,6 +74,122 @@ all contain something which:
 - identifies which port the Ethernet frame came from/should be sent to
 - provides a reason why this frame was forwarded to the management interface
 
+All tagging protocols are in ``net/dsa/tag_*.c`` files and implement the
+methods of the ``struct dsa_device_ops`` structure, which are detailed below.
+
+Tagging protocols generally fall in one of three categories:
+
+- The switch-specific frame header is located before the Ethernet header,
+  shifting to the right (from the perspective of the DSA master's frame
+  parser) the MAC DA, MAC SA, EtherType and the entire L2 payload.
+- The switch-specific frame header is located before the EtherType, keeping the
+  MAC DA and MAC SA in place from the DSA master's perspective, but shifting
+  the 'real' EtherType and L2 payload to the right.
+- The switch-specific frame header is located at the tail of the packet,
+  keeping all frame headers in place and not altering the view of the packet
+  that the DSA master's frame parser has.
+
+A tagging protocol may tag all packets with switch tags of the same length, or
+the tag length might vary (for example packets with PTP timestamps might
+require an extended switch tag, or there might be one tag length on TX and a
+different one on RX). Either way, the tagging protocol driver must populate the
+``struct dsa_device_ops::overhead`` with the length in octets of the longest
+switch frame header. The DSA framework will automatically adjust the MTU of the
+master interface to accomodate for this extra size in order for DSA user ports
+to support the standard MTU (L2 payload length) of 1500 octets. The ``overhead``
+is also used to request from the network stack, on a best-effort basis, the
+allocation of packets with a ``needed_headroom`` or ``needed_tailroom``
+sufficient such that the act of pushing the switch tag on transmission of a
+packet does not cause it to reallocate due to lack of memory.
+
+Even though applications are not expected to parse DSA-specific frame headers,
+the format on the wire of the tagging protocol represents an Application Binary
+Interface exposed by the kernel towards user space, for decoders such as
+``libpcap``. The tagging protocol driver must populate the ``proto`` member of
+``struct dsa_device_ops`` with a value that uniquely describes the
+characteristics of the interaction required between the switch hardware and the
+data path driver: the offset of each bit field within the frame header and any
+stateful processing required to deal with the frames (as may be required for
+PTP timestamping).
+
+By definition, all switches within the same DSA switch tree use the same
+tagging protocol. In case of a packet transiting a fabric with more than one
+switch, the switch-specific frame header is inserted by the first switch in the
+fabric that the packet was received on. This header typically contains
+information regarding its type (whether it is a control frame that must be
+trapped to the CPU, or a data frame to be forwarded). Control frames should be
+decapsulated only by the software data path, whereas data frames might also be
+autonomously forwarded towards other user ports of other switches from the same
+fabric, and in this case, the outermost switch ports must decapsulate the packet.
+
+It is possible to construct cascaded setups of DSA switches even if their
+tagging protocols are not compatible with one another. In this case, there are
+no DSA links in this fabric, and each switch constitutes a disjoint DSA switch
+tree. The DSA links are viewed as simply a pair of a DSA master (the out-facing
+port of the upstream DSA switch) and a CPU port (the in-facing port of the
+downstream DSA switch).
+
+The tagging protocol of the attached DSA switch tree can be viewed through the
+``dsa/tagging`` sysfs attribute of the DSA master::
+
+    cat /sys/class/net/eth0/dsa/tagging
+
+If the hardware and driver are capable, the tagging protocol of the DSA switch
+tree can be changed at runtime. This is done by writing the new tagging
+protocol name to the same sysfs device attribute as above (the DSA master and
+all attached switch ports must be down while doing this).
+
+It is desirable that all tagging protocols are testable with the ``dsa_loop``
+mockup driver, which can be attached to any network interface. The goal is that
+any network interface should be able of transmitting the same packet in the
+same way, and the tagger should decode the same received packet in the same way
+regardless of the driver used for the switch control path, and the driver used
+for the DSA master.
+
+The transmission of a packet goes through the tagger's ``xmit`` function.
+The passed ``struct sk_buff *skb`` has ``skb->data`` pointing at
+``skb_mac_header(skb)``, i.e. at the destination MAC address, and the passed
+``struct net_device *dev`` represents the virtual DSA user network interface
+whose hardware counterpart the packet must be steered to (i.e. ``swp0``).
+The job of this method is to prepare the skb in a way that the switch will
+understand what egress port the packet is for (and not deliver it towards other
+ports). Typically this is fulfilled by pushing a frame header. Checking for
+insufficient size in the skb headroom or tailroom is unnecessary provided that
+the ``overhead`` and ``tail_tag`` properties were filled out properly, because
+DSA ensures there is enough space before calling this method.
+
+The reception of a packet goes through the tagger's ``rcv`` function. The
+passed ``struct sk_buff *skb`` has ``skb->data`` pointing at
+``skb_mac_header(skb) + ETH_ALEN`` octets, i.e. to where the first octet after
+the EtherType would have been, were this frame not tagged. The role of this
+method is to consume the frame header, adjust ``skb->data`` to really point at
+the first octet after the EtherType, and to change ``skb->dev`` to point to the
+virtual DSA user network interface corresponding to the physical front-facing
+switch port that the packet was received on.
+
+Some tagging protocols, such as those in category 1 (shifting the MAC DA as
+seen by the DSA master), require the DSA master to operate in promiscuous mode,
+to receive all frames regardless of the value of the MAC DA. This can be done
+by setting the ``promisc_on_master`` property of the ``struct dsa_device_ops``.
+
+Since tagging protocols in category 1 and 2 break software (and most often also
+hardware) packet dissection on the DSA master, features such as RPS (Receive
+Packet Steering) on the DSA master would be broken. The DSA framework deals
+with this by hooking into the flow dissector and shifting the offset at which
+the IP header is to be found in the tagged frame as seen by the DSA master.
+This behavior is automatic based on the ``overhead`` value of the tagging
+protocol. If not all packets are of equal size, the tagger can implement the
+``flow_dissect`` method of the ``struct dsa_device_ops`` and override this
+default behavior by specifying the correct offset incurred by each individual
+RX packet. Tail taggers do not cause issues to the flow dissector.
+
+Hardware manufacturers are strongly discouraged to do this, but some tagging
+protocols might not provide source port information on RX for all packets, but
+e.g. only for control traffic (link-local PDUs). In this case, by implementing
+the ``filter`` method of ``struct dsa_device_ops``, the tagger might select
+which packets are to be redirected on RX towards the virtual DSA user network
+interfaces, and which are to be left in the DSA master's RX data path.
+
 Master network devices
 ----------------------
 
-- 
2.25.1

Re: [RFC PATCH net-next 02/12] Documentation: networking: dsa: rewrite chapter about tagging protocol

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The chapter about tagging protocols is out of date because it doesn't
> mention all taggers that have been added since last documentation
> update. But judging based on that, it will always tend to lag behind,
> and there's no good reason why we would enumerate the supported
> hardware. Instead we could do something more useful and explain what
> there is to know about tagging protocols instead.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

[snip]

> +Some tagging protocols, such as those in category 1 (shifting the MAC DA as
> +seen by the DSA master), require the DSA master to operate in promiscuous mode,
> +to receive all frames regardless of the value of the MAC DA. This can be done
> +by setting the ``promisc_on_master`` property of the ``struct dsa_device_ops``.

Nit: may require. DSA_TAG_PROTO_BRCM_PREPEND is an example of category 1 
tagger however the usual (and only?) DSA master (bgmac) does not require 
promiscuous mode. With that:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 02/12] Documentation: networking: dsa: rewrite chapter about tagging protocol

[Andrew Lunn]

> +It is desirable that all tagging protocols are testable with the ``dsa_loop``
> +mockup driver, which can be attached to any network interface. The goal is that
> +any network interface should be able of transmitting the same packet in the

should be _capable_ of ??

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [RFC PATCH net-next 02/12] Documentation: networking: dsa: rewrite chapter about tagging protocol

[Tobias Waldekranz]

On Sun, Feb 21, 2021 at 23:33, Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> The chapter about tagging protocols is out of date because it doesn't
> mention all taggers that have been added since last documentation
> update. But judging based on that, it will always tend to lag behind,
> and there's no good reason why we would enumerate the supported
> hardware. Instead we could do something more useful and explain what
> there is to know about tagging protocols instead.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  Documentation/networking/dsa/dsa.rst | 126 +++++++++++++++++++++++++--
>  1 file changed, 118 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index e20fbad2241a..fc98b5774fb6 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -65,14 +65,8 @@ Note that DSA does not currently create network interfaces for the "cpu" and
>  Switch tagging protocols
>  ------------------------
>  
> -DSA currently supports 5 different tagging protocols, and a tag-less mode as
> -well. The different protocols are implemented in:
> -
> -- ``net/dsa/tag_trailer.c``: Marvell's 4 trailer tag mode (legacy)
> -- ``net/dsa/tag_dsa.c``: Marvell's original DSA tag
> -- ``net/dsa/tag_edsa.c``: Marvell's enhanced DSA tag
> -- ``net/dsa/tag_brcm.c``: Broadcom's 4 bytes tag
> -- ``net/dsa/tag_qca.c``: Qualcomm's 2 bytes tag
> +DSA supports many vendor-specific tagging protocols, one software-defined
> +tagging protocol, and a tag-less mode as well (``DSA_TAG_PROTO_NONE``).
>  
>  The exact format of the tag protocol is vendor specific, but in general, they
>  all contain something which:
> @@ -80,6 +74,122 @@ all contain something which:
>  - identifies which port the Ethernet frame came from/should be sent to
>  - provides a reason why this frame was forwarded to the management interface
>  
> +All tagging protocols are in ``net/dsa/tag_*.c`` files and implement the
> +methods of the ``struct dsa_device_ops`` structure, which are detailed below.
> +
> +Tagging protocols generally fall in one of three categories:
> +
> +- The switch-specific frame header is located before the Ethernet header,
> +  shifting to the right (from the perspective of the DSA master's frame
> +  parser) the MAC DA, MAC SA, EtherType and the entire L2 payload.
> +- The switch-specific frame header is located before the EtherType, keeping the
> +  MAC DA and MAC SA in place from the DSA master's perspective, but shifting
> +  the 'real' EtherType and L2 payload to the right.
> +- The switch-specific frame header is located at the tail of the packet,
> +  keeping all frame headers in place and not altering the view of the packet
> +  that the DSA master's frame parser has.

A nit, but should this be a numbered list since "category 1 and 2" is
referenced later?

> +
> +A tagging protocol may tag all packets with switch tags of the same length, or
> +the tag length might vary (for example packets with PTP timestamps might
> +require an extended switch tag, or there might be one tag length on TX and a
> +different one on RX). Either way, the tagging protocol driver must populate the
> +``struct dsa_device_ops::overhead`` with the length in octets of the longest
> +switch frame header. The DSA framework will automatically adjust the MTU of the
> +master interface to accomodate for this extra size in order for DSA user ports
> +to support the standard MTU (L2 payload length) of 1500 octets. The ``overhead``
> +is also used to request from the network stack, on a best-effort basis, the
> +allocation of packets with a ``needed_headroom`` or ``needed_tailroom``
> +sufficient such that the act of pushing the switch tag on transmission of a
> +packet does not cause it to reallocate due to lack of memory.
> +
> +Even though applications are not expected to parse DSA-specific frame headers,
> +the format on the wire of the tagging protocol represents an Application Binary
> +Interface exposed by the kernel towards user space, for decoders such as
> +``libpcap``. The tagging protocol driver must populate the ``proto`` member of
> +``struct dsa_device_ops`` with a value that uniquely describes the
> +characteristics of the interaction required between the switch hardware and the
> +data path driver: the offset of each bit field within the frame header and any
> +stateful processing required to deal with the frames (as may be required for
> +PTP timestamping).
> +
> +By definition, all switches within the same DSA switch tree use the same
> +tagging protocol. In case of a packet transiting a fabric with more than one

This is not strictly true for mv88e6xxx. The connection between the tree
and the CPU may use Ethertyped DSA tags, while inter-switch links use
regular DSA tags.

However, I think it is better to keep this definition short, as it is
"true enough" :)

> +switch, the switch-specific frame header is inserted by the first switch in the
> +fabric that the packet was received on. This header typically contains
> +information regarding its type (whether it is a control frame that must be
> +trapped to the CPU, or a data frame to be forwarded). Control frames should be
> +decapsulated only by the software data path, whereas data frames might also be
> +autonomously forwarded towards other user ports of other switches from the same
> +fabric, and in this case, the outermost switch ports must decapsulate the packet.
> +
> +It is possible to construct cascaded setups of DSA switches even if their
> +tagging protocols are not compatible with one another. In this case, there are
> +no DSA links in this fabric, and each switch constitutes a disjoint DSA switch
> +tree. The DSA links are viewed as simply a pair of a DSA master (the out-facing
> +port of the upstream DSA switch) and a CPU port (the in-facing port of the
> +downstream DSA switch).
> +
> +The tagging protocol of the attached DSA switch tree can be viewed through the
> +``dsa/tagging`` sysfs attribute of the DSA master::
> +
> +    cat /sys/class/net/eth0/dsa/tagging
> +
> +If the hardware and driver are capable, the tagging protocol of the DSA switch
> +tree can be changed at runtime. This is done by writing the new tagging
> +protocol name to the same sysfs device attribute as above (the DSA master and
> +all attached switch ports must be down while doing this).
> +
> +It is desirable that all tagging protocols are testable with the ``dsa_loop``
> +mockup driver, which can be attached to any network interface. The goal is that
> +any network interface should be able of transmitting the same packet in the
> +same way, and the tagger should decode the same received packet in the same way
> +regardless of the driver used for the switch control path, and the driver used
> +for the DSA master.
> +
> +The transmission of a packet goes through the tagger's ``xmit`` function.
> +The passed ``struct sk_buff *skb`` has ``skb->data`` pointing at
> +``skb_mac_header(skb)``, i.e. at the destination MAC address, and the passed
> +``struct net_device *dev`` represents the virtual DSA user network interface
> +whose hardware counterpart the packet must be steered to (i.e. ``swp0``).
> +The job of this method is to prepare the skb in a way that the switch will
> +understand what egress port the packet is for (and not deliver it towards other
> +ports). Typically this is fulfilled by pushing a frame header. Checking for
> +insufficient size in the skb headroom or tailroom is unnecessary provided that
> +the ``overhead`` and ``tail_tag`` properties were filled out properly, because
> +DSA ensures there is enough space before calling this method.
> +
> +The reception of a packet goes through the tagger's ``rcv`` function. The
> +passed ``struct sk_buff *skb`` has ``skb->data`` pointing at
> +``skb_mac_header(skb) + ETH_ALEN`` octets, i.e. to where the first octet after
> +the EtherType would have been, were this frame not tagged. The role of this
> +method is to consume the frame header, adjust ``skb->data`` to really point at
> +the first octet after the EtherType, and to change ``skb->dev`` to point to the
> +virtual DSA user network interface corresponding to the physical front-facing
> +switch port that the packet was received on.
> +
> +Some tagging protocols, such as those in category 1 (shifting the MAC DA as
> +seen by the DSA master), require the DSA master to operate in promiscuous mode,
> +to receive all frames regardless of the value of the MAC DA. This can be done
> +by setting the ``promisc_on_master`` property of the ``struct dsa_device_ops``.
> +
> +Since tagging protocols in category 1 and 2 break software (and most often also
> +hardware) packet dissection on the DSA master, features such as RPS (Receive
> +Packet Steering) on the DSA master would be broken. The DSA framework deals
> +with this by hooking into the flow dissector and shifting the offset at which
> +the IP header is to be found in the tagged frame as seen by the DSA master.
> +This behavior is automatic based on the ``overhead`` value of the tagging
> +protocol. If not all packets are of equal size, the tagger can implement the
> +``flow_dissect`` method of the ``struct dsa_device_ops`` and override this
> +default behavior by specifying the correct offset incurred by each individual
> +RX packet. Tail taggers do not cause issues to the flow dissector.
> +
> +Hardware manufacturers are strongly discouraged to do this, but some tagging
> +protocols might not provide source port information on RX for all packets, but
> +e.g. only for control traffic (link-local PDUs). In this case, by implementing
> +the ``filter`` method of ``struct dsa_device_ops``, the tagger might select
> +which packets are to be redirected on RX towards the virtual DSA user network
> +interfaces, and which are to be left in the DSA master's RX data path.
> +
>  Master network devices
>  ----------------------
>  
> -- 
> 2.25.1

Great stuff!

Reviewed-by: Tobias Waldekranz <tobias@waldekranz.com>

Re: [RFC PATCH net-next 02/12] Documentation: networking: dsa: rewrite chapter about tagging protocol

[Vladimir Oltean]

On Thu, Feb 25, 2021 at 09:29:21PM +0100, Tobias Waldekranz wrote:
> This is not strictly true for mv88e6xxx. The connection between the tree
> and the CPU may use Ethertyped DSA tags, while inter-switch links use
> regular DSA tags.
> 
> However, I think it is better to keep this definition short, as it is
> "true enough" :)

What is the use case for this? Build a DSA tree out of old switches
which support only DSA, plus new switches which support both DSA and
EDSA, and have the host CPU see only EDSA, with the cascaded switches
playing the role of DSA->EDSA adapters for the leaf switches?
Is there any point in doing this? If it ever becomes necessary to
support this, can't we just say that you should configure your entire
DSA tree to use either DSA or EDSA, whichever happens to be supported
across all devices? We already have support for changing the tag
protocol, mv88e6xxx should implement it, then we could add some logic
somewhere to scan for the DSA tree at probe time and figure out a common
denominator.

Re: [RFC PATCH net-next 02/12] Documentation: networking: dsa: rewrite chapter about tagging protocol

[Tobias Waldekranz]

On Fri, Feb 26, 2021 at 20:12, Vladimir Oltean <olteanv@gmail.com> wrote:
> On Thu, Feb 25, 2021 at 09:29:21PM +0100, Tobias Waldekranz wrote:
>> This is not strictly true for mv88e6xxx. The connection between the tree
>> and the CPU may use Ethertyped DSA tags, while inter-switch links use
>> regular DSA tags.
>> 
>> However, I think it is better to keep this definition short, as it is
>> "true enough" :)
>
> What is the use case for this? Build a DSA tree out of old switches
> which support only DSA, plus new switches which support both DSA and
> EDSA, and have the host CPU see only EDSA, with the cascaded switches
> playing the role of DSA->EDSA adapters for the leaf switches?
> Is there any point in doing this? If it ever becomes necessary to
> support this, can't we just say that you should configure your entire
> DSA tree to use either DSA or EDSA, whichever happens to be supported
> across all devices? We already have support for changing the tag
> protocol, mv88e6xxx should implement it, then we could add some logic
> somewhere to scan for the DSA tree at probe time and figure out a common
> denominator.

This is already supported today. Cascade ports are _always_ set to
DSA. There are 2 reasons for that that I can think of:

1. It is the lowest common denominator, supported by all devices, so it
   makes for an easy algorithm.

2. It adds the minimum amount of overhead (4 bytes less than EDSA). If
   you are saturating your cascade link with 64B packets, that has quite
   an impact on your maximum pps.

As for why you would choose EDSA over DSA for connecting to the CPU: I
would say that on Linux with the DSA driver there is no reason, we could
probably drop the support altogether.

Before /sys/class/net/*/dsa/tagging, tcpdump could produce better
output, but that is no longer an issue.

The other advantage with EDSA is that you can use it for control traffic
(TO_CPU), while receiving data traffic (FORWARD) either untagged
Q-tagged. So you could use more of your NIC's offloads for example. But
this does not really work with the switchdev model as there is no
separation of control/data.

Though, now that I think about it, maybe we _can_ to that with the
filter method I just learned about from reading your excellent
documentation :)

Whether we want to is another question, but my guess is that things like
L3 forwarding performance could improve quite a bit, since there is less
memmoving around of L2 headers.

[RFC PATCH net-next 03/12] Documentation: networking: dsa: remove static port count from limitations

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

After Vivien's series from 2019 containing commits 27d4d19d7c82 ("net:
dsa: remove limitation of switch index value") and ab8ccae122a4 ("net:
dsa: add ports list in the switch fabric"), this is basically no longer
true.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index fc98b5774fb6..cb59df6e80f4 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -360,14 +360,6 @@ DSA data structures are defined in ``include/net/dsa.h`` as well as
 Design limitations
 ==================
 
-Limits on the number of devices and ports
------------------------------------------
-
-DSA currently limits the number of maximum switches within a tree to 4
-(``DSA_MAX_SWITCHES``), and the number of ports per switch to 12 (``DSA_MAX_PORTS``).
-These limits could be extended to support larger configurations would this need
-arise.
-
 Lack of CPU/DSA network devices
 -------------------------------
 
@@ -697,7 +689,6 @@ two subsystems and get the best of both worlds.
 Other hanging fruits
 --------------------
 
-- making the number of ports fully dynamic and not dependent on ``DSA_MAX_PORTS``
 - allowing more than one CPU/management interface:
   http://comments.gmane.org/gmane.linux.network/365657
 - porting more drivers from other vendors:
-- 
2.25.1

Re: [RFC PATCH net-next 03/12] Documentation: networking: dsa: remove static port count from limitations

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> After Vivien's series from 2019 containing commits 27d4d19d7c82 ("net:
> dsa: remove limitation of switch index value") and ab8ccae122a4 ("net:
> dsa: add ports list in the switch fabric"), this is basically no longer
> true.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[RFC PATCH net-next 04/12] Documentation: networking: dsa: remove references to switchdev prepare/commit

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

After the recent series containing commit bae33f2b5afe ("net: switchdev:
remove the transaction structure from port attributes"), there aren't
prepare/commit transactional phases anymore in most of the switchdev
objects/attributes, and as a result, there aren't any in the DSA driver
API either. So remove this piece.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 30 ++++++++--------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index cb59df6e80f4..8fb0ceff3418 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -430,14 +430,8 @@ SWITCHDEV
 
 DSA directly utilizes SWITCHDEV when interfacing with the bridge layer, and
 more specifically with its VLAN filtering portion when configuring VLANs on top
-of per-port slave network devices. Since DSA primarily deals with
-MDIO-connected switches, although not exclusively, SWITCHDEV's
-prepare/abort/commit phases are often simplified into a prepare phase which
-checks whether the operation is supported by the DSA switch driver, and a commit
-phase which applies the changes.
-
-As of today, the only SWITCHDEV objects supported by DSA are the FDB and VLAN
-objects.
+of per-port slave network devices. As of today, the only SWITCHDEV objects
+supported by DSA are the FDB and VLAN objects.
 
 Device Tree
 -----------
@@ -616,14 +610,10 @@ Bridge VLAN filtering
   accept any 802.1Q frames irrespective of their VLAN ID, and untagged frames are
   allowed.
 
-- ``port_vlan_prepare``: bridge layer function invoked when the bridge prepares the
-  configuration of a VLAN on the given port. If the operation is not supported
-  by the hardware, this function should return ``-EOPNOTSUPP`` to inform the bridge
-  code to fallback to a software implementation. No hardware setup must be done
-  in this function. See port_vlan_add for this and details.
-
 - ``port_vlan_add``: bridge layer function invoked when a VLAN is configured
-  (tagged or untagged) for the given switch port
+  (tagged or untagged) for the given switch port. If the operation is not
+  supported by the hardware, this function should return ``-EOPNOTSUPP`` to
+  inform the bridge code to fallback to a software implementation.
 
 - ``port_vlan_del``: bridge layer function invoked when a VLAN is removed from the
   given switch port
@@ -651,14 +641,10 @@ Bridge VLAN filtering
   function that the driver has to call for each MAC address known to be behind
   the given port. A switchdev object is used to carry the VID and FDB info.
 
-- ``port_mdb_prepare``: bridge layer function invoked when the bridge prepares the
-  installation of a multicast database entry. If the operation is not supported,
-  this function should return ``-EOPNOTSUPP`` to inform the bridge code to fallback
-  to a software implementation. No hardware setup must be done in this function.
-  See ``port_fdb_add`` for this and details.
-
 - ``port_mdb_add``: bridge layer function invoked when the bridge wants to install
-  a multicast database entry, the switch hardware should be programmed with the
+  a multicast database entry. If the operation is not supported, this function
+  should return ``-EOPNOTSUPP`` to inform the bridge code to fallback to a
+  software implementation. The switch hardware should be programmed with the
   specified address in the specified VLAN ID in the forwarding database
   associated with this VLAN ID.
 
-- 
2.25.1

Re: [RFC PATCH net-next 04/12] Documentation: networking: dsa: remove references to switchdev prepare/commit

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> After the recent series containing commit bae33f2b5afe ("net: switchdev:
> remove the transaction structure from port attributes"), there aren't
> prepare/commit transactional phases anymore in most of the switchdev
> objects/attributes, and as a result, there aren't any in the DSA driver
> API either. So remove this piece.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 04/12] Documentation: networking: dsa: remove references to switchdev prepare/commit

[Andrew Lunn]

On Sun, Feb 21, 2021 at 11:33:47PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> After the recent series containing commit bae33f2b5afe ("net: switchdev:
> remove the transaction structure from port attributes"), there aren't
> prepare/commit transactional phases anymore in most of the switchdev
> objects/attributes, and as a result, there aren't any in the DSA driver
> API either. So remove this piece.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[RFC PATCH net-next 05/12] Documentation: networking: dsa: remove TODO about porting more vendor drivers

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

On one hand, the link is dead and therefore useless.

On the other hand, there are always more drivers to port, but at this
stage, DSA does not need to affirm itself as the driver model to use for
Ethernet-connected switches (since we already have 15 tagging protocols
supported and probably more switch families from various vendors), so
there is nothing actionable to do.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 8fb0ceff3418..19ce5bb0a7a4 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -677,5 +677,3 @@ Other hanging fruits
 
 - allowing more than one CPU/management interface:
   http://comments.gmane.org/gmane.linux.network/365657
-- porting more drivers from other vendors:
-  http://comments.gmane.org/gmane.linux.network/365510
-- 
2.25.1

Re: [RFC PATCH net-next 05/12] Documentation: networking: dsa: remove TODO about porting more vendor drivers

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> On one hand, the link is dead and therefore useless.
> 
> On the other hand, there are always more drivers to port, but at this
> stage, DSA does not need to affirm itself as the driver model to use for
> Ethernet-connected switches (since we already have 15 tagging protocols
> supported and probably more switch families from various vendors), so
> there is nothing actionable to do.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 05/12] Documentation: networking: dsa: remove TODO about porting more vendor drivers

[Andrew Lunn]

On Sun, Feb 21, 2021 at 11:33:48PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> On one hand, the link is dead and therefore useless.
> 
> On the other hand, there are always more drivers to port, but at this
> stage, DSA does not need to affirm itself as the driver model to use for
> Ethernet-connected switches (since we already have 15 tagging protocols
> supported and probably more switch families from various vendors), so
> there is nothing actionable to do.

Yes, we have already taken over the universe :-)

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[RFC PATCH net-next 06/12] Documentation: networking: dsa: document the port_bridge_flags method

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The documentation was already lagging behind by not mentioning the old
version of port_bridge_flags (port_set_egress_floods). So now we are
skipping one step and just explaining how a DSA driver should configure
address learning and flooding settings.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 19ce5bb0a7a4..3c6560a43ae0 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -597,6 +597,17 @@ Bridge layer
   computing a STP state change based on current and asked parameters and perform
   the relevant ageing based on the intersection results
 
+- ``port_bridge_flags``: bridge layer function invoked when a port must
+  configure its settings for e.g. flooding of unknown traffic or source address
+  learning. The switch driver is responsible for initial setup of the
+  standalone ports with address learning disabled and egress flooding of all
+  types of traffic, then the DSA core notifies of any change to the bridge port
+  flags when the port joins and leaves a bridge. DSA does not currently manage
+  the bridge port flags for the CPU port. The assumption is that address
+  learning should be statically enabled (if supported by the hardware) on the
+  CPU port, and flooding towards the CPU port should also be enabled, in lack
+  of an explicit address filtering mechanism in the DSA core.
+
 Bridge VLAN filtering
 ---------------------
 
-- 
2.25.1

Re: [RFC PATCH net-next 06/12] Documentation: networking: dsa: document the port_bridge_flags method

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The documentation was already lagging behind by not mentioning the old
> version of port_bridge_flags (port_set_egress_floods). So now we are
> skipping one step and just explaining how a DSA driver should configure
> address learning and flooding settings.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 06/12] Documentation: networking: dsa: document the port_bridge_flags method

[Andrew Lunn]

> +  the bridge port flags for the CPU port. The assumption is that address
> +  learning should be statically enabled (if supported by the hardware) on the
> +  CPU port, and flooding towards the CPU port should also be enabled, in lack
> +  of an explicit address filtering mechanism in the DSA core.

Hi Vladimir

"in lack of" is a bit odd wording. Maybe "due to a lack of"

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[RFC PATCH net-next 07/12] Documentation: networking: dsa: mention integration with devlink

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Add a short summary of the devlink features supported by the DSA core.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 3c6560a43ae0..463b48714fe9 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -394,6 +394,7 @@ DSA currently leverages the following subsystems:
 - MDIO/PHY library: ``drivers/net/phy/phy.c``, ``mdio_bus.c``
 - Switchdev:``net/switchdev/*``
 - Device Tree for various of_* functions
+- Devlink: ``net/core/devlink.c``
 
 MDIO/PHY library
 ----------------
@@ -433,6 +434,32 @@ more specifically with its VLAN filtering portion when configuring VLANs on top
 of per-port slave network devices. As of today, the only SWITCHDEV objects
 supported by DSA are the FDB and VLAN objects.
 
+Devlink
+-------
+
+DSA registers one devlink device per each physical switch in the fabric.
+For each devlink device, every physical port (i.e. user ports, CPU ports, DSA
+links and unused ports) is exposed as a devlink port.
+
+DSA drivers can make use of the following devlink features:
+- Regions: debugging feature which allows user space to dump driver-defined
+  areas of hardware information in a low-level, binary format. Both global
+  regions as well as per-port regions are supported. Since address tables and
+  VLAN tables are only inspectable by core iproute2 tools (ip-link, bridge) on
+  user ports, devlink regions can be created for dumping these tables on the
+  non-user ports too.
+- Params: a feature which enables user to configure certain low-level tunable
+  knobs pertaining to the device. Drivers may implement applicable generic
+  devlink params, or may add new device-specific devlink params.
+- Resources: a monitoring feature which enables users to see the degree of
+  utilization of certain hardware tables in the device, such as FDB, VLAN, etc.
+- Shared buffers: a QoS feature for adjusting and partitioning memory and frame
+  reservations per port and per traffic class, in the ingress and egress
+  directions, such that low-priority bulk traffic does not impede the
+  processing of high-priority critical traffic.
+
+For more details, consult ``Documentation/networking/devlink/``.
+
 Device Tree
 -----------
 
-- 
2.25.1

Re: [RFC PATCH net-next 07/12] Documentation: networking: dsa: mention integration with devlink

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Add a short summary of the devlink features supported by the DSA core.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 07/12] Documentation: networking: dsa: mention integration with devlink

[Andrew Lunn]

On Sun, Feb 21, 2021 at 11:33:50PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Add a short summary of the devlink features supported by the DSA core.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  Documentation/networking/dsa/dsa.rst | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index 3c6560a43ae0..463b48714fe9 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -394,6 +394,7 @@ DSA currently leverages the following subsystems:
>  - MDIO/PHY library: ``drivers/net/phy/phy.c``, ``mdio_bus.c``
>  - Switchdev:``net/switchdev/*``
>  - Device Tree for various of_* functions
> +- Devlink: ``net/core/devlink.c``
>  
>  MDIO/PHY library
>  ----------------
> @@ -433,6 +434,32 @@ more specifically with its VLAN filtering portion when configuring VLANs on top
>  of per-port slave network devices. As of today, the only SWITCHDEV objects
>  supported by DSA are the FDB and VLAN objects.
>  
> +Devlink
> +-------
> +
> +DSA registers one devlink device per each physical switch in the fabric.

"per each" sounds wrong to my ears. per on its own is better.

> +For each devlink device, every physical port (i.e. user ports, CPU ports, DSA
> +links and unused ports) is exposed as a devlink port.

I would probably use "or", instead of "and". 

> +
> +DSA drivers can make use of the following devlink features:
> +- Regions: debugging feature which allows user space to dump driver-defined
> +  areas of hardware information in a low-level, binary format. Both global
> +  regions as well as per-port regions are supported. Since address tables and
> +  VLAN tables are only inspectable by core iproute2 tools (ip-link, bridge) on
> +  user ports, devlink regions can be created for dumping these tables on the
> +  non-user ports too.

You might also add that additional details which don't fit the
iproute2 model can be included in regions dumps, since the format is
not restricted.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[RFC PATCH net-next 08/12] Documentation: networking: dsa: add paragraph for the LAG offload

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Add a short summary of the methods that a driver writer must implement
for offloading a link aggregation group, and what is still missing.

Cc: Tobias Waldekranz <tobias@waldekranz.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 32 ++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 463b48714fe9..0a5b06cf4d45 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -698,6 +698,38 @@ Bridge VLAN filtering
   function that the driver has to call for each MAC address known to be behind
   the given port. A switchdev object is used to carry the VID and MDB info.
 
+Link aggregation
+----------------
+
+Link aggregation is implemented in the Linux networking stack by the bonding
+and team drivers, which are modeled as virtual, stackable network interfaces.
+DSA is capable of offloading a link aggregation group (LAG) to hardware that
+supports the feature, and supports bridging between physical ports and LAGs,
+as well as between LAGs. A bonding/team interface which holds multiple physical
+ports constitutes a logical port, although DSA has no explicit concept of a
+physical port at the moment. Due to this, events where a LAG joins/leaves a
+bridge are treated as if all individual physical ports that are members of that
+LAG join/leave the bridge. Switchdev port attributes (VLAN filtering, STP
+state, etc) on a LAG are treated similarly: DSA offloads the same switchdev
+port attribute on all members of the LAG. Switchdev objects on a LAG (FDB, MDB)
+are not yet supported, since the DSA driver API does not have the concept of a
+logical port ID.
+
+- ``port_lag_join``: function invoked when a given switch port is added to a
+  LAG. The driver may return ``-EOPNOTSUPP``, and in this case, DSA will fall
+  back to a software implementation where all traffic from this port is sent to
+  the CPU.
+- ``port_lag_leave``: function invoked when a given switch port leaves a LAG
+  and returns to operation as a standalone port.
+- ``port_lag_change``: function invoked when the link state of any member of
+  the LAG changes, and the hashing function needs rebalancing only towards the
+  subset of physical LAG member ports that are up.
+
+Drivers that benefit from having an ID associated with each offloaded LAG
+can optionally populate ``ds->num_lag_ids`` from the ``dsa_switch_ops::setup``
+method. The LAG ID associated with a bonding/team interface can then be
+retrieved by a DSA switch driver using the ``dsa_lag_id`` function.
+
 TODO
 ====
 
-- 
2.25.1

Re: [RFC PATCH net-next 08/12] Documentation: networking: dsa: add paragraph for the LAG offload

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Add a short summary of the methods that a driver writer must implement
> for offloading a link aggregation group, and what is still missing.
> 
> Cc: Tobias Waldekranz <tobias@waldekranz.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 08/12] Documentation: networking: dsa: add paragraph for the LAG offload

[Andrew Lunn]

On Sun, Feb 21, 2021 at 11:33:51PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Add a short summary of the methods that a driver writer must implement
> for offloading a link aggregation group, and what is still missing.
> 
> Cc: Tobias Waldekranz <tobias@waldekranz.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  Documentation/networking/dsa/dsa.rst | 32 ++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index 463b48714fe9..0a5b06cf4d45 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -698,6 +698,38 @@ Bridge VLAN filtering
>    function that the driver has to call for each MAC address known to be behind
>    the given port. A switchdev object is used to carry the VID and MDB info.
>  
> +Link aggregation
> +----------------
> +
> +Link aggregation is implemented in the Linux networking stack by the bonding
> +and team drivers, which are modeled as virtual, stackable network interfaces.
> +DSA is capable of offloading a link aggregation group (LAG) to hardware that
> +supports the feature, and supports bridging between physical ports and LAGs,
> +as well as between LAGs. A bonding/team interface which holds multiple physical
> +ports constitutes a logical port, although DSA has no explicit concept of a
> +physical port at the moment.

Hi Vladimir

I don't understand what you mean by a physical port in this context.

Due to this, events where a LAG joins/leaves a
> +bridge are treated as if all individual physical ports that are members of that
> +LAG join/leave the bridge. Switchdev port attributes (VLAN filtering, STP
> +state, etc) on a LAG are treated similarly: DSA offloads the same switchdev
> +port attribute on all members of the LAG. Switchdev objects on a LAG (FDB, MDB)
> +are not yet supported, since the DSA driver API does not have the concept of a
> +logical port ID.
> +
> +- ``port_lag_join``: function invoked when a given switch port is added to a
> +  LAG. The driver may return ``-EOPNOTSUPP``, and in this case, DSA will fall
> +  back to a software implementation where all traffic from this port is sent to
> +  the CPU.
> +- ``port_lag_leave``: function invoked when a given switch port leaves a LAG
> +  and returns to operation as a standalone port.
> +- ``port_lag_change``: function invoked when the link state of any member of
> +  the LAG changes, and the hashing function needs rebalancing only towards the
> +  subset of physical LAG member ports that are up.

"and the hashing function needs rebalancing to only make use of the
subset of physical LAG member ports that are up."

       Andrew

Re: [RFC PATCH net-next 08/12] Documentation: networking: dsa: add paragraph for the LAG offload

[Tobias Waldekranz]

On Sun, Feb 21, 2021 at 23:33, Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Add a short summary of the methods that a driver writer must implement
> for offloading a link aggregation group, and what is still missing.
>
> Cc: Tobias Waldekranz <tobias@waldekranz.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  Documentation/networking/dsa/dsa.rst | 32 ++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index 463b48714fe9..0a5b06cf4d45 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -698,6 +698,38 @@ Bridge VLAN filtering
>    function that the driver has to call for each MAC address known to be behind
>    the given port. A switchdev object is used to carry the VID and MDB info.
>  
> +Link aggregation
> +----------------
> +
> +Link aggregation is implemented in the Linux networking stack by the bonding
> +and team drivers, which are modeled as virtual, stackable network interfaces.
> +DSA is capable of offloading a link aggregation group (LAG) to hardware that
> +supports the feature, and supports bridging between physical ports and LAGs,
> +as well as between LAGs. A bonding/team interface which holds multiple physical
> +ports constitutes a logical port, although DSA has no explicit concept of a
> +physical port at the moment. Due to this, events where a LAG joins/leaves a

s/physical/logical/ right?

> +bridge are treated as if all individual physical ports that are members of that
> +LAG join/leave the bridge. Switchdev port attributes (VLAN filtering, STP
> +state, etc) on a LAG are treated similarly: DSA offloads the same switchdev
> +port attribute on all members of the LAG. Switchdev objects on a LAG (FDB, MDB)
> +are not yet supported, since the DSA driver API does not have the concept of a
> +logical port ID.

Switchdev objects (MDB entries and VLANs) are supported, and will be
added to all members of the LAG just like attributes. Static FDB entries
are not switchdev objects though, and are therefore not supported.

> +
> +- ``port_lag_join``: function invoked when a given switch port is added to a
> +  LAG. The driver may return ``-EOPNOTSUPP``, and in this case, DSA will fall
> +  back to a software implementation where all traffic from this port is sent to
> +  the CPU.
> +- ``port_lag_leave``: function invoked when a given switch port leaves a LAG
> +  and returns to operation as a standalone port.
> +- ``port_lag_change``: function invoked when the link state of any member of
> +  the LAG changes, and the hashing function needs rebalancing only towards the
> +  subset of physical LAG member ports that are up.
> +
> +Drivers that benefit from having an ID associated with each offloaded LAG
> +can optionally populate ``ds->num_lag_ids`` from the ``dsa_switch_ops::setup``
> +method. The LAG ID associated with a bonding/team interface can then be
> +retrieved by a DSA switch driver using the ``dsa_lag_id`` function.
> +
>  TODO
>  ====
>  
> -- 
> 2.25.1

Re: [RFC PATCH net-next 08/12] Documentation: networking: dsa: add paragraph for the LAG offload

[Vladimir Oltean]

On Thu, Feb 25, 2021 at 09:42:28PM +0100, Tobias Waldekranz wrote:
> On Sun, Feb 21, 2021 at 23:33, Vladimir Oltean <olteanv@gmail.com> wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > Add a short summary of the methods that a driver writer must implement
> > for offloading a link aggregation group, and what is still missing.
> >
> > Cc: Tobias Waldekranz <tobias@waldekranz.com>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> >  Documentation/networking/dsa/dsa.rst | 32 ++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> > index 463b48714fe9..0a5b06cf4d45 100644
> > --- a/Documentation/networking/dsa/dsa.rst
> > +++ b/Documentation/networking/dsa/dsa.rst
> > @@ -698,6 +698,38 @@ Bridge VLAN filtering
> >    function that the driver has to call for each MAC address known to be behind
> >    the given port. A switchdev object is used to carry the VID and MDB info.
> >  
> > +Link aggregation
> > +----------------
> > +
> > +Link aggregation is implemented in the Linux networking stack by the bonding
> > +and team drivers, which are modeled as virtual, stackable network interfaces.
> > +DSA is capable of offloading a link aggregation group (LAG) to hardware that
> > +supports the feature, and supports bridging between physical ports and LAGs,
> > +as well as between LAGs. A bonding/team interface which holds multiple physical
> > +ports constitutes a logical port, although DSA has no explicit concept of a
> > +physical port at the moment. Due to this, events where a LAG joins/leaves a
> 
> s/physical/logical/ right?

Yes, brain fart #1.

> > +bridge are treated as if all individual physical ports that are members of that
> > +LAG join/leave the bridge. Switchdev port attributes (VLAN filtering, STP
> > +state, etc) on a LAG are treated similarly: DSA offloads the same switchdev
> > +port attribute on all members of the LAG. Switchdev objects on a LAG (FDB, MDB)
> > +are not yet supported, since the DSA driver API does not have the concept of a
> > +logical port ID.
> 
> Switchdev objects (MDB entries and VLANs) are supported, and will be
> added to all members of the LAG just like attributes. Static FDB entries
> are not switchdev objects though, and are therefore not supported.

And brain fart #2.

[RFC PATCH net-next 09/12] Documentation: networking: dsa: add paragraph for the MRP offload

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Add a short summary of the methods that a driver writer must implement
for getting an MRP instance to work on top of a DSA switch.

Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Horatiu:
- Why does ocelot support a single MRP ring if all it does is trap the
  MRP PDUs to the CPU? What is stopping it from supporting more than
  one ring?
- Why is listening for SWITCHDEV_OBJ_ID_MRP necessary at all, since it
  does nothing related to hardware configuration?
- Why is ocelot_mrp_del_vcap called from both ocelot_mrp_del and from
  ocelot_mrp_del_ring_role?
- Why does ocelot not look at the MRM/MRC ring role at all, and it traps
  all MRP PDUs to the CPU, even those which it could forward as an MRC?
  I understood from your commit d8ea7ff3995e ("net: mscc: ocelot: Add
  support for MRP") description that the hardware should be able of
  forwarding the Test PDUs as a client, however it is obviously not
  doing that.
---
 Documentation/networking/dsa/dsa.rst | 30 ++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 0a5b06cf4d45..bf82f2aed29a 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -730,6 +730,36 @@ can optionally populate ``ds->num_lag_ids`` from the ``dsa_switch_ops::setup``
 method. The LAG ID associated with a bonding/team interface can then be
 retrieved by a DSA switch driver using the ``dsa_lag_id`` function.
 
+IEC 62439-2 (MRP)
+-----------------
+
+The Media Redundancy Protocol is a topology management protocol optimized for
+fast fault recovery time for ring networks, which has some components
+implemented as a function of the bridge driver. MRP uses management PDUs
+(Test, Topology, LinkDown/Up, Option) sent at a multicast destination MAC
+address range of 01:15:4e:00:00:0x and with an EtherType of 0x88e3.
+Depending on the node's role in the ring (MRM: Media Redundancy Manager,
+MRC: Media Redundancy Client, MRA: Media Redundancy Automanager), certain MRP
+PDUs might need to be terminated locally and others might need to be forwarded.
+An MRM might also benefit from offloading to hardware the creation and
+transmission of certain MRP PDUs (Test).
+
+Normally an MRP instance can be created on top of any network interface,
+however in the case of a device with an offloaded data path such as DSA, it is
+necessary for the hardware, even if it is not MRP-aware, to be able to extract
+the MRP PDUs from the fabric before the driver can proceed with the software
+implementation. DSA today has no driver which is MRP-aware, therefore it only
+listens for the bare minimum switchdev objects required for the software assist
+to work properly. The operations are detailed below.
+
+- ``port_mrp_add`` and ``port_mrp_del``: notifies driver when an MRP instance
+  with a certain ring ID, priority, primary port and secondary port is
+  created/deleted.
+- ``port_mrp_add_ring_role`` and ``port_mrp_del_ring_role``: function invoked
+  when an MRP instance changes ring roles between MRM or MRC. This affects
+  which MRP PDUs should be trapped to software and which should be autonomously
+  forwarded.
+
 TODO
 ====
 
-- 
2.25.1

Re: [RFC PATCH net-next 09/12] Documentation: networking: dsa: add paragraph for the MRP offload

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Add a short summary of the methods that a driver writer must implement
> for getting an MRP instance to work on top of a DSA switch.
> 
> Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 09/12] Documentation: networking: dsa: add paragraph for the MRP offload

[Horatiu Vultur]

The 02/21/2021 23:33, Vladimir Oltean wrote:
> 
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Add a short summary of the methods that a driver writer must implement
> for getting an MRP instance to work on top of a DSA switch.
> 
> Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Hi Vladimir,

> 
> Horatiu:
> - Why does ocelot support a single MRP ring if all it does is trap the
>   MRP PDUs to the CPU? What is stopping it from supporting more than
>   one ring?

So the HW can support to run multiple rings. But to have an initial
basic implementation I have decided to support only one ring. So
basically is just a limitation in the driver.

> - Why is listening for SWITCHDEV_OBJ_ID_MRP necessary at all, since it
>   does nothing related to hardware configuration?

It is listening because it needs to know which ports are part of the
ring. In case you have multiple rings and do forwarding in HW you need
to know which ports are part of which ring. Also in case a MRP frame
will come on a port which is not part of the ring then that frame should
be flooded.

> - Why is ocelot_mrp_del_vcap called from both ocelot_mrp_del and from
>   ocelot_mrp_del_ring_role?

To clean after itself. Lets say a user creates a node and sets it up.
Then when she decides to delete the node, what should happen? Should it
first disable the node and then do the cleaning or just do the cleaning?
This userspace application[1] does the second option but I didn't want
to implement the driver to be specific to this application so I have put
the call in both places.

> - Why does ocelot not look at the MRM/MRC ring role at all, and it traps
>   all MRP PDUs to the CPU, even those which it could forward as an MRC?
>   I understood from your commit d8ea7ff3995e ("net: mscc: ocelot: Add
>   support for MRP") description that the hardware should be able of
>   forwarding the Test PDUs as a client, however it is obviously not
>   doing that.

It doesn't look at the role because it doesn't care. Because in both
cases is looking at the sw_backup because it doesn't support any role
completely. Maybe comment was misleading but I have put it under
'current limitations' meaning that the HW can do that but the driver
doesn't take advantage of that yet. The same applies to multiple rings
support.

The idea is to remove these limitations in the next patches and
to be able to remove these limitations then the driver will look also
at the role.

[1] https://github.com/microchip-ung/mrp

> ---
>  Documentation/networking/dsa/dsa.rst | 30 ++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index 0a5b06cf4d45..bf82f2aed29a 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -730,6 +730,36 @@ can optionally populate ``ds->num_lag_ids`` from the ``dsa_switch_ops::setup``
>  method. The LAG ID associated with a bonding/team interface can then be
>  retrieved by a DSA switch driver using the ``dsa_lag_id`` function.
> 
> +IEC 62439-2 (MRP)
> +-----------------
> +
> +The Media Redundancy Protocol is a topology management protocol optimized for
> +fast fault recovery time for ring networks, which has some components
> +implemented as a function of the bridge driver. MRP uses management PDUs
> +(Test, Topology, LinkDown/Up, Option) sent at a multicast destination MAC
> +address range of 01:15:4e:00:00:0x and with an EtherType of 0x88e3.
> +Depending on the node's role in the ring (MRM: Media Redundancy Manager,
> +MRC: Media Redundancy Client, MRA: Media Redundancy Automanager), certain MRP
> +PDUs might need to be terminated locally and others might need to be forwarded.
> +An MRM might also benefit from offloading to hardware the creation and
> +transmission of certain MRP PDUs (Test).
> +
> +Normally an MRP instance can be created on top of any network interface,
> +however in the case of a device with an offloaded data path such as DSA, it is
> +necessary for the hardware, even if it is not MRP-aware, to be able to extract
> +the MRP PDUs from the fabric before the driver can proceed with the software
> +implementation. DSA today has no driver which is MRP-aware, therefore it only
> +listens for the bare minimum switchdev objects required for the software assist
> +to work properly. The operations are detailed below.
> +
> +- ``port_mrp_add`` and ``port_mrp_del``: notifies driver when an MRP instance
> +  with a certain ring ID, priority, primary port and secondary port is
> +  created/deleted.
> +- ``port_mrp_add_ring_role`` and ``port_mrp_del_ring_role``: function invoked
> +  when an MRP instance changes ring roles between MRM or MRC. This affects
> +  which MRP PDUs should be trapped to software and which should be autonomously
> +  forwarded.
> +
>  TODO
>  ====
> 
> --
> 2.25.1
> 

-- 
/Horatiu

Re: [RFC PATCH net-next 09/12] Documentation: networking: dsa: add paragraph for the MRP offload

[Vladimir Oltean]

Hi Horatiu,

On Mon, Feb 22, 2021 at 08:46:26PM +0100, Horatiu Vultur wrote:
> > - Why does ocelot support a single MRP ring if all it does is trap the
> >   MRP PDUs to the CPU? What is stopping it from supporting more than
> >   one ring?
>
> So the HW can support to run multiple rings. But to have an initial
> basic implementation I have decided to support only one ring. So
> basically is just a limitation in the driver.

What should change in the current sw_backup implementation such that
multiple rings are supported?

> > - Why is listening for SWITCHDEV_OBJ_ID_MRP necessary at all, since it
> >   does nothing related to hardware configuration?
>
> It is listening because it needs to know which ports are part of the
> ring. In case you have multiple rings and do forwarding in HW you need
> to know which ports are part of which ring. Also in case a MRP frame
> will come on a port which is not part of the ring then that frame should
> be flooded.

If I understand correctly, you just said below that this is not
applicable to the current implementation because it is simplistic enough
that it doesn't care what ring role does the application use, because it
doesn't attempt to do any forwarding of MRP PDUs at all. If all that
there is to do for a port with sw_backup is to add a trapping rule per
port (rule which is already added per port), then what extra logic is
there to add for the second MRP instance on a different set of 2 ports?

> > - Why is ocelot_mrp_del_vcap called from both ocelot_mrp_del and from
> >   ocelot_mrp_del_ring_role?
>
> To clean after itself. Lets say a user creates a node and sets it up.
> Then when she decides to delete the node, what should happen? Should it
> first disable the node and then do the cleaning or just do the cleaning?
> This userspace application[1] does the second option but I didn't want
> to implement the driver to be specific to this application so I have put
> the call in both places.

I was actually thinking that the bridge could clean up after itself and
delete the SWITCHDEV_OBJ_ID_RING_ROLE_MRP object.

> > - Why does ocelot not look at the MRM/MRC ring role at all, and it traps
> >   all MRP PDUs to the CPU, even those which it could forward as an MRC?
> >   I understood from your commit d8ea7ff3995e ("net: mscc: ocelot: Add
> >   support for MRP") description that the hardware should be able of
> >   forwarding the Test PDUs as a client, however it is obviously not
> >   doing that.
>
> It doesn't look at the role because it doesn't care. Because in both
> cases is looking at the sw_backup because it doesn't support any role
> completely. Maybe comment was misleading but I have put it under
> 'current limitations' meaning that the HW can do that but the driver
> doesn't take advantage of that yet. The same applies to multiple rings
> support.
>
> The idea is to remove these limitations in the next patches and
> to be able to remove these limitations then the driver will look also
> at the role.
>
> [1] https://github.com/microchip-ung/mrp

By the way, how can Ocelot trap some PDUs to the CPU but forward others?
Doesn't it need to parse the MRP TLVs in order to determine whether they
are Test packets or something else?

Re: [RFC PATCH net-next 09/12] Documentation: networking: dsa: add paragraph for the MRP offload

[Horatiu Vultur]

The 02/22/2021 22:25, Vladimir Oltean wrote:
> 
Hi Vladimir,
> Hi Horatiu,
> 
> On Mon, Feb 22, 2021 at 08:46:26PM +0100, Horatiu Vultur wrote:
> > > - Why does ocelot support a single MRP ring if all it does is trap the
> > >   MRP PDUs to the CPU? What is stopping it from supporting more than
> > >   one ring?
> >
> > So the HW can support to run multiple rings. But to have an initial
> > basic implementation I have decided to support only one ring. So
> > basically is just a limitation in the driver.
> 
> What should change in the current sw_backup implementation such that
> multiple rings are supported?

Instead of single mrp_ring_id, mrp_p_port and mrp_s_port is to have a
list of these. And then when a new MRP instance is added/removed this
list should be updated. When the role is changed then find the MRP ports
from this list and put the rules to these ports.

> 
> > > - Why is listening for SWITCHDEV_OBJ_ID_MRP necessary at all, since it
> > >   does nothing related to hardware configuration?
> >
> > It is listening because it needs to know which ports are part of the
> > ring. In case you have multiple rings and do forwarding in HW you need
> > to know which ports are part of which ring. Also in case a MRP frame
> > will come on a port which is not part of the ring then that frame should
> > be flooded.
> 
> If I understand correctly, you just said below that this is not
> applicable to the current implementation because it is simplistic enough
> that it doesn't care what ring role does the application use, because it
> doesn't attempt to do any forwarding of MRP PDUs at all. If all that
> there is to do for a port with sw_backup is to add a trapping rule per
> port (rule which is already added per port), then what extra logic is
> there to add for the second MRP instance on a different set of 2 ports?

Regarding rules nothing should be changed. You just need to know which
is this new MRP instance so to put the same rules on these 2 ports. And
you can use the ring_id to determin which MRP instance it is and from
there you can find the ports.

> 
> > > - Why is ocelot_mrp_del_vcap called from both ocelot_mrp_del and from
> > >   ocelot_mrp_del_ring_role?
> >
> > To clean after itself. Lets say a user creates a node and sets it up.
> > Then when she decides to delete the node, what should happen? Should it
> > first disable the node and then do the cleaning or just do the cleaning?
> > This userspace application[1] does the second option but I didn't want
> > to implement the driver to be specific to this application so I have put
> > the call in both places.
> 
> I was actually thinking that the bridge could clean up after itself and
> delete the SWITCHDEV_OBJ_ID_RING_ROLE_MRP object.
> 
> > > - Why does ocelot not look at the MRM/MRC ring role at all, and it traps
> > >   all MRP PDUs to the CPU, even those which it could forward as an MRC?
> > >   I understood from your commit d8ea7ff3995e ("net: mscc: ocelot: Add
> > >   support for MRP") description that the hardware should be able of
> > >   forwarding the Test PDUs as a client, however it is obviously not
> > >   doing that.
> >
> > It doesn't look at the role because it doesn't care. Because in both
> > cases is looking at the sw_backup because it doesn't support any role
> > completely. Maybe comment was misleading but I have put it under
> > 'current limitations' meaning that the HW can do that but the driver
> > doesn't take advantage of that yet. The same applies to multiple rings
> > support.
> >
> > The idea is to remove these limitations in the next patches and
> > to be able to remove these limitations then the driver will look also
> > at the role.
> >
> > [1] https://github.com/microchip-ung/mrp
> 
> By the way, how can Ocelot trap some PDUs to the CPU but forward others?
> Doesn't it need to parse the MRP TLVs in order to determine whether they
> are Test packets or something else?

No it doesn't need to do that. Because Test packets are send to dmac
01:15:4e:00:00:01 while the other ring MRP frames are send to
01:15:4e:00:00:02. And Ocelot can trap frames based on the dmac.

I will create a patch with these changes when the net-next tree will
open.

-- 
/Horatiu

Re: [RFC PATCH net-next 09/12] Documentation: networking: dsa: add paragraph for the MRP offload

[Vladimir Oltean]

On Tue, Feb 23, 2021 at 02:30:28PM +0100, Horatiu Vultur wrote:
> The 02/22/2021 22:25, Vladimir Oltean wrote:
> > 
> Hi Vladimir,
> > Hi Horatiu,
> > 
> > On Mon, Feb 22, 2021 at 08:46:26PM +0100, Horatiu Vultur wrote:
> > > > - Why does ocelot support a single MRP ring if all it does is trap the
> > > >   MRP PDUs to the CPU? What is stopping it from supporting more than
> > > >   one ring?
> > >
> > > So the HW can support to run multiple rings. But to have an initial
> > > basic implementation I have decided to support only one ring. So
> > > basically is just a limitation in the driver.
> > 
> > What should change in the current sw_backup implementation such that
> > multiple rings are supported?
> 
> Instead of single mrp_ring_id, mrp_p_port and mrp_s_port is to have a
> list of these. And then when a new MRP instance is added/removed this
> list should be updated. When the role is changed then find the MRP ports
> from this list and put the rules to these ports.

A physical port can't offload more than one ring id under any
circumstance, no? So why keep a list and not just keep the MRP ring id
in the ocelot_port structure, then when the ring role changes, just
iterate through all ports and update the trapping rule on those having
the same ring id?

Also, why is it important to know which port is primary and which is
secondary?

> > > > - Why does ocelot not look at the MRM/MRC ring role at all, and it traps
> > > >   all MRP PDUs to the CPU, even those which it could forward as an MRC?
> > > >   I understood from your commit d8ea7ff3995e ("net: mscc: ocelot: Add
> > > >   support for MRP") description that the hardware should be able of
> > > >   forwarding the Test PDUs as a client, however it is obviously not
> > > >   doing that.
> > >
> > > It doesn't look at the role because it doesn't care. Because in both
> > > cases is looking at the sw_backup because it doesn't support any role
> > > completely. Maybe comment was misleading but I have put it under
> > > 'current limitations' meaning that the HW can do that but the driver
> > > doesn't take advantage of that yet. The same applies to multiple rings
> > > support.
> > >
> > > The idea is to remove these limitations in the next patches and
> > > to be able to remove these limitations then the driver will look also
> > > at the role.
> > >
> > > [1] https://github.com/microchip-ung/mrp
> > 
> > By the way, how can Ocelot trap some PDUs to the CPU but forward others?
> > Doesn't it need to parse the MRP TLVs in order to determine whether they
> > are Test packets or something else?
> 
> No it doesn't need to do that. Because Test packets are send to dmac
> 01:15:4e:00:00:01 while the other ring MRP frames are send to
> 01:15:4e:00:00:02. And Ocelot can trap frames based on the dmac.

Interesting, so I think with a little bit more forethought, the
intentions with this MRP hardware assist would have been much clearer.
From what you explained, the better implementation wouldn't have been
more complicated than the current one is, just cleaner.

Re: [RFC PATCH net-next 09/12] Documentation: networking: dsa: add paragraph for the MRP offload

[Horatiu Vultur]

The 02/23/2021 15:50, Vladimir Oltean wrote:
> On Tue, Feb 23, 2021 at 02:30:28PM +0100, Horatiu Vultur wrote:
> > The 02/22/2021 22:25, Vladimir Oltean wrote:
> > >
> > Hi Vladimir,
> > > Hi Horatiu,
> > >
> > > On Mon, Feb 22, 2021 at 08:46:26PM +0100, Horatiu Vultur wrote:
> > > > > - Why does ocelot support a single MRP ring if all it does is trap the
> > > > >   MRP PDUs to the CPU? What is stopping it from supporting more than
> > > > >   one ring?
> > > >
> > > > So the HW can support to run multiple rings. But to have an initial
> > > > basic implementation I have decided to support only one ring. So
> > > > basically is just a limitation in the driver.
> > >
> > > What should change in the current sw_backup implementation such that
> > > multiple rings are supported?
> >
> > Instead of single mrp_ring_id, mrp_p_port and mrp_s_port is to have a
> > list of these. And then when a new MRP instance is added/removed this
> > list should be updated. When the role is changed then find the MRP ports
> > from this list and put the rules to these ports.
> 
> A physical port can't offload more than one ring id under any
> circumstance, no? So why keep a list and not just keep the MRP ring id
> in the ocelot_port structure, then when the ring role changes, just
> iterate through all ports and update the trapping rule on those having
> the same ring id?

Yes, a port can be part of only one ring. Yes, you should be able to do
it also like that, I don't see any issues with that approach.

> 
> Also, why is it important to know which port is primary and which is
> secondary?

In this context is not important. It is important when MRM role is
offloaded to HW.

> 
> > > > > - Why does ocelot not look at the MRM/MRC ring role at all, and it traps
> > > > >   all MRP PDUs to the CPU, even those which it could forward as an MRC?
> > > > >   I understood from your commit d8ea7ff3995e ("net: mscc: ocelot: Add
> > > > >   support for MRP") description that the hardware should be able of
> > > > >   forwarding the Test PDUs as a client, however it is obviously not
> > > > >   doing that.
> > > >
> > > > It doesn't look at the role because it doesn't care. Because in both
> > > > cases is looking at the sw_backup because it doesn't support any role
> > > > completely. Maybe comment was misleading but I have put it under
> > > > 'current limitations' meaning that the HW can do that but the driver
> > > > doesn't take advantage of that yet. The same applies to multiple rings
> > > > support.
> > > >
> > > > The idea is to remove these limitations in the next patches and
> > > > to be able to remove these limitations then the driver will look also
> > > > at the role.
> > > >
> > > > [1] https://github.com/microchip-ung/mrp
> > >
> > > By the way, how can Ocelot trap some PDUs to the CPU but forward others?
> > > Doesn't it need to parse the MRP TLVs in order to determine whether they
> > > are Test packets or something else?
> >
> > No it doesn't need to do that. Because Test packets are send to dmac
> > 01:15:4e:00:00:01 while the other ring MRP frames are send to
> > 01:15:4e:00:00:02. And Ocelot can trap frames based on the dmac.
> 
> Interesting, so I think with a little bit more forethought, the
> intentions with this MRP hardware assist would have been much clearer.
> From what you explained, the better implementation wouldn't have been
> more complicated than the current one is, just cleaner.

A better implementation will be to have also interconnect support. Again
the idea of the patch was to add minimum support for Ocelot and from
there to build on.


-- 
/Horatiu

Re: [RFC PATCH net-next 09/12] Documentation: networking: dsa: add paragraph for the MRP offload

[Andrew Lunn]

> +implemented as a function of the bridge driver. MRP uses management PDUs
> +(Test, Topology, LinkDown/Up, Option) sent at a multicast destination MAC

"sent to", or "sent with"

> +address range of 01:15:4e:00:00:0x and with an EtherType of 0x88e3.

address in the range.

	Andrew

[RFC PATCH net-next 10/12] Documentation: networking: dsa: add paragraph for the HSR/PRP offload

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Add a short summary of the methods that a driver writer must implement
for offloading a HSR/PRP network interface.

Cc: George McCollister <george.mccollister@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 32 ++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index bf82f2aed29a..277045346f3a 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -760,6 +760,38 @@ to work properly. The operations are detailed below.
   which MRP PDUs should be trapped to software and which should be autonomously
   forwarded.
 
+IEC 62439-3 (HSR/PRP)
+---------------------
+
+The Parallel Redundancy Protocol (PRP) is a network redundancy protocol which
+works by duplicating and sequence numbering packets through two independent L2
+networks (which are unaware of the PRP tail tags carried in the packets), and
+eliminating the duplicates at the receiver. The High-availability Seamless
+Redundancy (HSR) protocol is similar in concept, except all nodes that carry
+the redundant traffic are aware of the fact that it is HSR-tagged (because HSR
+uses a header with an EtherType of 0x892f) and are physically connected in a
+ring topology. Both HSR and PRP use supervision frames for monitoring the
+health of the network and for discovering the other nodes.
+
+In Linux, both HSR and PRP are implemented in the hsr driver, which
+instantiates a virtual, stackable network interface with two member ports.
+The driver only implements the basic roles of DANH (Doubly Attached Node
+implementing HSR) and DANP (Doubly Attached Node implementing PRP); the roles
+of RedBox and QuadBox aren't (therefore, bridging a hsr network interface with
+a physical switch port is not supported).
+
+A driver which is able of offloading certain functions of a DANP or DANH should
+declare the corresponding netdev features as indicated by the documentation at
+``Documentation/networking/netdev-features.rst``. Additionally, the following
+methods must be implemented:
+
+- ``port_hsr_join``: function invoked when a given switch port is added to a
+  DANP/DANH. The driver may return ``-EOPNOTSUPP`` and in this case, DSA will
+  fall back to a software implementation where all traffic from this port is
+  sent to the CPU.
+- ``port_hsr_leave``: function invoked when a given switch port leaves a
+  DANP/DANH and returns to normal operation as a standalone port.
+
 TODO
 ====
 
-- 
2.25.1

Re: [RFC PATCH net-next 10/12] Documentation: networking: dsa: add paragraph for the HSR/PRP offload

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Add a short summary of the methods that a driver writer must implement
> for offloading a HSR/PRP network interface.
> 
> Cc: George McCollister <george.mccollister@gmail.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [RFC PATCH net-next 10/12] Documentation: networking: dsa: add paragraph for the HSR/PRP offload

[George McCollister]

On Sun, Feb 21, 2021 at 3:34 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Add a short summary of the methods that a driver writer must implement
> for offloading a HSR/PRP network interface.
>
> Cc: George McCollister <george.mccollister@gmail.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  Documentation/networking/dsa/dsa.rst | 32 ++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index bf82f2aed29a..277045346f3a 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -760,6 +760,38 @@ to work properly. The operations are detailed below.
>    which MRP PDUs should be trapped to software and which should be autonomously
>    forwarded.
>
> +IEC 62439-3 (HSR/PRP)
> +---------------------
> +
> +The Parallel Redundancy Protocol (PRP) is a network redundancy protocol which
> +works by duplicating and sequence numbering packets through two independent L2
> +networks (which are unaware of the PRP tail tags carried in the packets), and
> +eliminating the duplicates at the receiver. The High-availability Seamless
> +Redundancy (HSR) protocol is similar in concept, except all nodes that carry
> +the redundant traffic are aware of the fact that it is HSR-tagged (because HSR
> +uses a header with an EtherType of 0x892f) and are physically connected in a
> +ring topology. Both HSR and PRP use supervision frames for monitoring the
> +health of the network and for discovering the other nodes.
> +
> +In Linux, both HSR and PRP are implemented in the hsr driver, which
> +instantiates a virtual, stackable network interface with two member ports.
> +The driver only implements the basic roles of DANH (Doubly Attached Node
> +implementing HSR) and DANP (Doubly Attached Node implementing PRP); the roles
> +of RedBox and QuadBox aren't (therefore, bridging a hsr network interface with
> +a physical switch port is not supported).
> +
> +A driver which is able of offloading certain functions of a DANP or DANH should
> +declare the corresponding netdev features as indicated by the documentation at
> +``Documentation/networking/netdev-features.rst``. Additionally, the following
> +methods must be implemented:
> +
> +- ``port_hsr_join``: function invoked when a given switch port is added to a
> +  DANP/DANH. The driver may return ``-EOPNOTSUPP`` and in this case, DSA will
> +  fall back to a software implementation where all traffic from this port is
> +  sent to the CPU.
> +- ``port_hsr_leave``: function invoked when a given switch port leaves a
> +  DANP/DANH and returns to normal operation as a standalone port.
> +
>  TODO
>  ====
>
> --
> 2.25.1
>

Reviewed-by: George McCollister <george.mccollister@gmail.com>

Re: [RFC PATCH net-next 10/12] Documentation: networking: dsa: add paragraph for the HSR/PRP offload

[Andrew Lunn]

> +IEC 62439-3 (HSR/PRP)
> +---------------------
> +
> +The Parallel Redundancy Protocol (PRP) is a network redundancy protocol which
> +works by duplicating and sequence numbering packets through two independent L2
> +networks (which are unaware of the PRP tail tags carried in the packets), and
> +eliminating the duplicates at the receiver. The High-availability Seamless
> +Redundancy (HSR) protocol is similar in concept, except all nodes that carry
> +the redundant traffic are aware of the fact that it is HSR-tagged (because HSR
> +uses a header with an EtherType of 0x892f) and are physically connected in a
> +ring topology. Both HSR and PRP use supervision frames for monitoring the

I don't know HSR/PRP terms. Should it be supervisory instead of
supervision?

> +health of the network and for discovering the other nodes.

Either "discovering other nodes" or "discovery of other nodes".

> +
> +In Linux, both HSR and PRP are implemented in the hsr driver, which
> +instantiates a virtual, stackable network interface with two member ports.
> +The driver only implements the basic roles of DANH (Doubly Attached Node
> +implementing HSR) and DANP (Doubly Attached Node implementing PRP); the roles
> +of RedBox and QuadBox aren't (therefore, bridging a hsr network interface with

In colloquial English, you can get away with just 'aren't'. But in
Queens English, you should follow it with something, in this case
'supported'.

	Andrew

Re: [RFC PATCH net-next 10/12] Documentation: networking: dsa: add paragraph for the HSR/PRP offload

[George McCollister]

On Wed, Feb 24, 2021 at 7:42 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +IEC 62439-3 (HSR/PRP)
> > +---------------------
> > +
> > +The Parallel Redundancy Protocol (PRP) is a network redundancy protocol which
> > +works by duplicating and sequence numbering packets through two independent L2
> > +networks (which are unaware of the PRP tail tags carried in the packets), and
> > +eliminating the duplicates at the receiver. The High-availability Seamless
> > +Redundancy (HSR) protocol is similar in concept, except all nodes that carry
> > +the redundant traffic are aware of the fact that it is HSR-tagged (because HSR
> > +uses a header with an EtherType of 0x892f) and are physically connected in a
> > +ring topology. Both HSR and PRP use supervision frames for monitoring the
>
> I don't know HSR/PRP terms. Should it be supervisory instead of
> supervision?

IEC 62439-3 refers to them primarily as supervision frames however
supervisory frames also appears once in the document.

>
> > +health of the network and for discovering the other nodes.
>
> Either "discovering other nodes" or "discovery of other nodes".
>
> > +
> > +In Linux, both HSR and PRP are implemented in the hsr driver, which
> > +instantiates a virtual, stackable network interface with two member ports.
> > +The driver only implements the basic roles of DANH (Doubly Attached Node
> > +implementing HSR) and DANP (Doubly Attached Node implementing PRP); the roles
> > +of RedBox and QuadBox aren't (therefore, bridging a hsr network interface with
>
> In colloquial English, you can get away with just 'aren't'. But in
> Queens English, you should follow it with something, in this case
> 'supported'.
>
>         Andrew

[RFC PATCH net-next 11/12] Documentation: networking: switchdev: clarify device driver behavior

[Vladimir Oltean]

From: Florian Fainelli <f.fainelli@gmail.com>

This patch provides details on the expected behavior of switchdev
enabled network devices when operating in a "stand alone" mode, as well
as when being bridge members. This clarifies a number of things that
recently came up during a bug fixing session on the b53 DSA switch
driver.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/switchdev.rst | 120 +++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/Documentation/networking/switchdev.rst b/Documentation/networking/switchdev.rst
index ddc3f35775dc..9fb3e0fd39dc 100644
--- a/Documentation/networking/switchdev.rst
+++ b/Documentation/networking/switchdev.rst
@@ -385,3 +385,123 @@ The driver can monitor for updates to arp_tbl using the netevent notifier
 NETEVENT_NEIGH_UPDATE.  The device can be programmed with resolved nexthops
 for the routes as arp_tbl updates.  The driver implements ndo_neigh_destroy
 to know when arp_tbl neighbor entries are purged from the port.
+
+Device driver expected behavior
+-------------------------------
+
+Below is a set of defined behavior that switchdev enabled network devices must
+adhere to.
+
+Configuration-less state
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Upon driver bring up, the network devices must be fully operational, and the
+backing driver must configure the network device such that it is possible to
+send and receive traffic to this network device and it is properly separated
+from other network devices/ports (e.g.: as is frequent with a switch ASIC). How
+this is achieved is heavily hardware dependent, but a simple solution can be to
+use per-port VLAN identifiers unless a better mechanism is available
+(proprietary metadata for each network port for instance).
+
+The network device must be capable of running a full IP protocol stack
+including multicast, DHCP, IPv4/6, etc. If necessary, it should program the
+appropriate filters for VLAN, multicast, unicast etc. The underlying device
+driver must effectively be configured in a similar fashion to what it would do
+when IGMP snooping is enabled for IP multicast over these switchdev network
+devices and unsolicited multicast must be filtered as early as possible into
+the hardware.
+
+When configuring VLANs on top of the network device, all VLANs must be working,
+irrespective of the state of other network devices (e.g.: other ports being part
+of a VLAN-aware bridge doing ingress VID checking). See below for details.
+
+If the device implements e.g.: VLAN filtering, putting the interface in
+promiscuous mode should allow the reception of all VLAN tags (including those
+not present in the filter(s)).
+
+Bridged switch ports
+^^^^^^^^^^^^^^^^^^^^
+
+When a switchdev enabled network device is added as a bridge member, it should
+not disrupt any functionality of non-bridged network devices and they
+should continue to behave as normal network devices. Depending on the bridge
+configuration knobs below, the expected behavior is documented.
+
+Bridge VLAN filtering
+^^^^^^^^^^^^^^^^^^^^^
+
+The Linux bridge allows the configuration of a VLAN filtering mode (statically,
+at device creation time, and dynamically, during run time) which must be
+observed by the underlying switchdev network device/hardware:
+
+- with VLAN filtering turned off: the bridge is strictly VLAN unaware and its
+  data path will only process untagged Ethernet frames. Frames ingressing the
+  device with a VID that is not programmed into the bridge/switch's VLAN table
+  must be forwarded and may be processed using a VLAN device (see below).
+
+- with VLAN filtering turned on: the bridge is VLAN-aware and frames ingressing
+  the device with a VID that is not programmed into the bridges/switch's VLAN
+  table must be dropped (strict VID checking).
+
+Non-bridged network ports of the same switch fabric must not be disturbed in any
+way by the enabling of VLAN filtering on the bridge device(s).
+
+VLAN devices configured on top of a switchdev network device (e.g: sw0p1.100)
+which is a bridge port member must also observe the following behavior:
+
+- with VLAN filtering turned off, enslaving VLAN devices into the bridge might
+  be allowed provided that there is sufficient separation using e.g.: a
+  reserved VLAN ID (4095 for instance) for untagged traffic. The VLAN data path
+  is used to pop/push the VLAN tag such that the bridge's data path only
+  processes untagged traffic.
+
+- with VLAN filtering turned on, these VLAN devices can be created as long as
+  there is not an existing VLAN entry into the bridge with an identical VID and
+  port membership. These VLAN devices cannot be enslaved into the bridge since
+  they duplicate functionality/use case with the bridge's VLAN data path
+  processing.
+
+Because VLAN filtering can be turned on/off at runtime, the switchdev driver
+must be able to reconfigure the underlying hardware on the fly to honor the
+toggling of that option and behave appropriately.
+
+A switchdev driver can also refuse to support dynamic toggling of the VLAN
+filtering knob at runtime and require a destruction of the bridge device(s) and
+creation of new bridge device(s) with a different VLAN filtering value to
+ensure VLAN awareness is pushed down to the hardware.
+
+Finally, even when VLAN filtering in the bridge is turned off, the underlying
+switch hardware and driver may still configured itself in a VLAN-aware mode
+provided that the behavior described above is observed.
+
+Bridge IGMP snooping
+^^^^^^^^^^^^^^^^^^^^
+
+The Linux bridge allows the configuration of IGMP snooping (statically, at
+interface creation time, or dynamically, during runtime) which must be observed
+by the underlying switchdev network device/hardware in the following way:
+
+- when IGMP snooping is turned off, multicast traffic must be flooded to all
+  ports within the same bridge that have mcast_flood=true. The CPU/management
+  port should ideally not be flooded (unless the ingress interface has
+  IFF_ALLMULTI or IFF_PROMISC) and continue to learn multicast traffic through
+  the network stack notifications. If the hardware is not capable of doing that
+  then the CPU/management port must also be flooded and multicast filtering
+  happens in software.
+
+- when IGMP snooping is turned on, multicast traffic must selectively flow
+  to the appropriate network ports (including CPU/management port). Flooding of
+  unknown multicast should be only towards the ports connected to a multicast
+  router (the local device may also act as a multicast router).
+
+The switch must adhere to RFC 4541 and flood multicast traffic accordingly
+since that is what the Linux bridge implementation does.
+
+Because IGMP snooping can be turned on/off at runtime, the switchdev driver
+must be able to reconfigure the underlying hardware on the fly to honor the
+toggling of that option and behave appropriately.
+
+A switchdev driver can also refuse to support dynamic toggling of the multicast
+snooping knob at runtime and require the destruction of the bridge device(s)
+and creation of a new bridge device(s) with a different multicast snooping
+value.
-- 
2.25.1

Re: [RFC PATCH net-next 11/12] Documentation: networking: switchdev: clarify device driver behavior

[Andrew Lunn]

> +devices and unsolicited multicast must be filtered as early as possible into
> +the hardware.

'into' sounds wrong here. Probably just 'in'.

> +- with VLAN filtering turned off: the bridge is strictly VLAN unaware and its
> +  data path will only process untagged Ethernet frames. Frames ingressing the
> +  device with a VID that is not programmed into the bridge/switch's VLAN table
> +  must be forwarded and may be processed using a VLAN device (see below).

I must be missing something, because these two sentence seems to
contradict each other?

> +Finally, even when VLAN filtering in the bridge is turned off, the underlying
> +switch hardware and driver may still configured itself in a VLAN-aware mode

configure.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [RFC PATCH net-next 11/12] Documentation: networking: switchdev: clarify device driver behavior

[Ido Schimmel]

On Sun, Feb 21, 2021 at 11:33:54PM +0200, Vladimir Oltean wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> 
> This patch provides details on the expected behavior of switchdev
> enabled network devices when operating in a "stand alone" mode, as well
> as when being bridge members. This clarifies a number of things that
> recently came up during a bug fixing session on the b53 DSA switch
> driver.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  Documentation/networking/switchdev.rst | 120 +++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/Documentation/networking/switchdev.rst b/Documentation/networking/switchdev.rst
> index ddc3f35775dc..9fb3e0fd39dc 100644
> --- a/Documentation/networking/switchdev.rst
> +++ b/Documentation/networking/switchdev.rst
> @@ -385,3 +385,123 @@ The driver can monitor for updates to arp_tbl using the netevent notifier
>  NETEVENT_NEIGH_UPDATE.  The device can be programmed with resolved nexthops
>  for the routes as arp_tbl updates.  The driver implements ndo_neigh_destroy
>  to know when arp_tbl neighbor entries are purged from the port.
> +
> +Device driver expected behavior
> +-------------------------------
> +
> +Below is a set of defined behavior that switchdev enabled network devices must
> +adhere to.
> +
> +Configuration-less state
> +^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Upon driver bring up, the network devices must be fully operational, and the
> +backing driver must configure the network device such that it is possible to
> +send and receive traffic to this network device and it is properly separated
> +from other network devices/ports (e.g.: as is frequent with a switch ASIC). How
> +this is achieved is heavily hardware dependent, but a simple solution can be to
> +use per-port VLAN identifiers unless a better mechanism is available
> +(proprietary metadata for each network port for instance).
> +
> +The network device must be capable of running a full IP protocol stack
> +including multicast, DHCP, IPv4/6, etc. If necessary, it should program the
> +appropriate filters for VLAN, multicast, unicast etc. The underlying device
> +driver must effectively be configured in a similar fashion to what it would do
> +when IGMP snooping is enabled for IP multicast over these switchdev network
> +devices and unsolicited multicast must be filtered as early as possible into
> +the hardware.
> +
> +When configuring VLANs on top of the network device, all VLANs must be working,
> +irrespective of the state of other network devices (e.g.: other ports being part
> +of a VLAN-aware bridge doing ingress VID checking). See below for details.
> +
> +If the device implements e.g.: VLAN filtering, putting the interface in
> +promiscuous mode should allow the reception of all VLAN tags (including those
> +not present in the filter(s)).
> +
> +Bridged switch ports
> +^^^^^^^^^^^^^^^^^^^^
> +
> +When a switchdev enabled network device is added as a bridge member, it should
> +not disrupt any functionality of non-bridged network devices and they
> +should continue to behave as normal network devices. Depending on the bridge
> +configuration knobs below, the expected behavior is documented.
> +
> +Bridge VLAN filtering
> +^^^^^^^^^^^^^^^^^^^^^
> +
> +The Linux bridge allows the configuration of a VLAN filtering mode (statically,
> +at device creation time, and dynamically, during run time) which must be
> +observed by the underlying switchdev network device/hardware:
> +
> +- with VLAN filtering turned off: the bridge is strictly VLAN unaware and its
> +  data path will only process untagged Ethernet frames. Frames ingressing the
> +  device with a VID that is not programmed into the bridge/switch's VLAN table
> +  must be forwarded and may be processed using a VLAN device (see below).

This needs some more clarification like Andrew noted. If you put a port
in a VLAN-unaware bridge, the bridge will process all the packets,
regardless if they are tagged or untagged.

If you then create a VLAN device on top of the port and put it in a
second VLAN-unaware bridge, then the second bridge will process the VLAN
packets after they were untagged by the VLAN device. Obviously, other
VLAN-tagged packets that do not belong to the VLAN device will continue
to be processed by the first bridge.

I'm not sure if you can support such a flexible model in hardware or
not. To avoid disambiguation you can prevent user space from creating
VLAN devices on top of a port that is member in a VLAN-unaware bridge,
but this is very very limiting.

Instead, the common deployment scenario is that VLAN-unaware bridges
only forward untagged packets. Regardless if they were received untagged
or were untagged by a VLAN device.

> +
> +- with VLAN filtering turned on: the bridge is VLAN-aware and frames ingressing
> +  the device with a VID that is not programmed into the bridges/switch's VLAN
> +  table must be dropped (strict VID checking).

Worth mentioning that the VLAN protocol of the bridge plays a role in
deciding whether a packet is tagged or not. For example, a 802.1ad
bridge will also treat 802.1q tagged packets as untagged.

I would also mention the expected behavior with regards to the presence
of PVID:

* When PVID exists: Untagged and prio-tagged packets belong to the PVID
* When PVID does not exists: Untagged and prio-tagged packets are
  dropped

Note that if you really need to support a scenario where both untagged
and 802.1q tagged packets are forwarded the same, you can create a
802.1ad bridge.

> +
> +Non-bridged network ports of the same switch fabric must not be disturbed in any
> +way by the enabling of VLAN filtering on the bridge device(s).
> +
> +VLAN devices configured on top of a switchdev network device (e.g: sw0p1.100)
> +which is a bridge port member must also observe the following behavior:
> +
> +- with VLAN filtering turned off, enslaving VLAN devices into the bridge might
> +  be allowed provided that there is sufficient separation using e.g.: a
> +  reserved VLAN ID (4095 for instance) for untagged traffic. The VLAN data path
> +  is used to pop/push the VLAN tag such that the bridge's data path only
> +  processes untagged traffic.
> +
> +- with VLAN filtering turned on, these VLAN devices can be created as long as
> +  there is not an existing VLAN entry into the bridge with an identical VID and
> +  port membership. These VLAN devices cannot be enslaved into the bridge since
> +  they duplicate functionality/use case with the bridge's VLAN data path
> +  processing.
> +
> +Because VLAN filtering can be turned on/off at runtime, the switchdev driver
> +must be able to reconfigure the underlying hardware on the fly to honor the
> +toggling of that option and behave appropriately.
> +
> +A switchdev driver can also refuse to support dynamic toggling of the VLAN
> +filtering knob at runtime and require a destruction of the bridge device(s) and
> +creation of new bridge device(s) with a different VLAN filtering value to
> +ensure VLAN awareness is pushed down to the hardware.
> +
> +Finally, even when VLAN filtering in the bridge is turned off, the underlying
> +switch hardware and driver may still configured itself in a VLAN-aware mode
> +provided that the behavior described above is observed.
> +
> +Bridge IGMP snooping
> +^^^^^^^^^^^^^^^^^^^^
> +
> +The Linux bridge allows the configuration of IGMP snooping (statically, at
> +interface creation time, or dynamically, during runtime) which must be observed
> +by the underlying switchdev network device/hardware in the following way:
> +
> +- when IGMP snooping is turned off, multicast traffic must be flooded to all
> +  ports within the same bridge that have mcast_flood=true. The CPU/management
> +  port should ideally not be flooded (unless the ingress interface has
> +  IFF_ALLMULTI or IFF_PROMISC) and continue to learn multicast traffic through
> +  the network stack notifications. If the hardware is not capable of doing that
> +  then the CPU/management port must also be flooded and multicast filtering
> +  happens in software.
> +
> +- when IGMP snooping is turned on, multicast traffic must selectively flow
> +  to the appropriate network ports (including CPU/management port). Flooding of
> +  unknown multicast should be only towards the ports connected to a multicast
> +  router (the local device may also act as a multicast router).
> +
> +The switch must adhere to RFC 4541 and flood multicast traffic accordingly
> +since that is what the Linux bridge implementation does.
> +
> +Because IGMP snooping can be turned on/off at runtime, the switchdev driver
> +must be able to reconfigure the underlying hardware on the fly to honor the
> +toggling of that option and behave appropriately.
> +
> +A switchdev driver can also refuse to support dynamic toggling of the multicast
> +snooping knob at runtime and require the destruction of the bridge device(s)
> +and creation of a new bridge device(s) with a different multicast snooping
> +value.
> -- 
> 2.25.1
> 

[RFC PATCH net-next 12/12] Documentation: networking: switchdev: fix command for static FDB entries

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The "bridge fdb add" command provided in the switchdev documentation is
junk now, not only because it is syntactically incorrect and rejected by
the iproute2 bridge program, but also because it was not updated in
light of Arkadi Sharshevsky's radical switchdev refactoring in commit
29ab586c3d83 ("net: switchdev: Remove bridge bypass support from
switchdev"). Try to explain what the intended usage pattern is with the
new kernel implementation.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/switchdev.rst | 47 +++++++++++++++++++-------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/Documentation/networking/switchdev.rst b/Documentation/networking/switchdev.rst
index 9fb3e0fd39dc..d5afd2caaf91 100644
--- a/Documentation/networking/switchdev.rst
+++ b/Documentation/networking/switchdev.rst
@@ -181,18 +181,41 @@ To offloading L2 bridging, the switchdev driver/device should support:
 Static FDB Entries
 ^^^^^^^^^^^^^^^^^^
 
-The switchdev driver should implement ndo_fdb_add, ndo_fdb_del and ndo_fdb_dump
-to support static FDB entries installed to the device.  Static bridge FDB
-entries are installed, for example, using iproute2 bridge cmd::
-
-	bridge fdb add ADDR dev DEV [vlan VID] [self]
-
-The driver should use the helper switchdev_port_fdb_xxx ops for ndo_fdb_xxx
-ops, and handle add/delete/dump of SWITCHDEV_OBJ_ID_PORT_FDB object using
-switchdev_port_obj_xxx ops.
-
-XXX: what should be done if offloading this rule to hardware fails (for
-example, due to full capacity in hardware tables) ?
+A driver which implements the ``ndo_fdb_add``, ``ndo_fdb_del`` and
+``ndo_fdb_dump`` operations is able to support the command below, which adds a
+static bridge FDB entry::
+
+        bridge fdb add dev DEV ADDRESS [vlan VID] [self] static
+
+(the "static" keyword is non-optional: if not specified, the entry defaults to
+being "local", which means that it should not be forwarded)
+
+The "self" keyword (optional because it is implicit) has the role of
+instructing the kernel to fulfill the operation through the ``ndo_fdb_add``
+implementation of the ``DEV`` device itself. If ``DEV`` is a bridge port, this
+will bypass the bridge and therefore leave the software database out of sync
+with the hardware one.
+
+To avoid this, the "master" keyword can be used::
+
+        bridge fdb add dev DEV ADDRESS [vlan VID] master static
+
+The above command instructs the kernel to search for a master interface of
+``DEV`` and fulfill the operation through the ``ndo_fdb_add`` method of that.
+This time, the bridge generates a ``SWITCHDEV_FDB_ADD_TO_DEVICE`` notification
+which the port driver can handle and use it to program its hardware table. This
+way, the software and the hardware database will both contain this static FDB
+entry.
+
+Note: for new switchdev drivers that offload the Linux bridge, implementing the
+``ndo_fdb_add`` and ``ndo_fdb_del`` bridge bypass methods is strongly
+discouraged: all static FDB entries should be added on a bridge port using the
+"master" flag. The ``ndo_fdb_dump`` is an exception and can be implemented to
+visualize the hardware tables, if the device does not have an interrupt for
+notifying the operating system of newly learned/forgotten dynamic FDB
+addresses. In that case, the hardware FDB might end up having entries that the
+software FDB does not, and implementing ``ndo_fdb_dump`` is the only way to see
+them.
 
 Note: by default, the bridge does not filter on VLAN and only bridges untagged
 traffic.  To enable VLAN support, turn on VLAN filtering::
-- 
2.25.1

Re: [RFC PATCH net-next 12/12] Documentation: networking: switchdev: fix command for static FDB entries

[Florian Fainelli]

On 2/21/2021 13:33, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The "bridge fdb add" command provided in the switchdev documentation is
> junk now, not only because it is syntactically incorrect and rejected by
> the iproute2 bridge program, but also because it was not updated in
> light of Arkadi Sharshevsky's radical switchdev refactoring in commit
> 29ab586c3d83 ("net: switchdev: Remove bridge bypass support from
> switchdev"). Try to explain what the intended usage pattern is with the
> new kernel implementation.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian