| Message ID | 20210221213355.1241450-10-olteanv@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Documentation updates for switchdev and DSA | expand |
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
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
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?
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
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.
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
> +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
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 ====