Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support

On Fri, Jan 22, 2021 at 11:56 AM Florian Fainelli [off-list ref] wrote:

On 1/22/2021 7:59 AM, George McCollister wrote:

quoted

Add support for offloading of HSR/PRP (IEC 62439-3) tag insertion
tag removal, duplicate generation and forwarding on DSA switches.

Use a new netdev_priv_flag IFF_HSR to indicate that a device is an HSR
device so DSA can tell them apart from other devices in
dsa_slave_changeupper.

Add DSA_NOTIFIER_HSR_JOIN and DSA_NOTIFIER_HSR_LEAVE which trigger calls
to .port_hsr_join and .port_hsr_leave in the DSA driver for the switch.

The DSA switch driver should then set netdev feature flags for the
HSR/PRP operation that it offloads.
    NETIF_F_HW_HSR_TAG_INS
    NETIF_F_HW_HSR_TAG_RM
    NETIF_F_HW_HSR_FWD
    NETIF_F_HW_HSR_DUP

For HSR, insertion involves the switch adding a 6 byte HSR header after
the 14 byte Ethernet header. For PRP it adds a 6 byte trailer.

Tag removal involves automatically stripping the HSR/PRP header/trailer
in the switch. This is possible when the switch also preforms auto
deduplication using the HSR/PRP header/trailer (making it no longer
required).

Forwarding involves automatically forwarding between redundant ports in
an HSR. This is crucial because delay is accumulated as a frame passes
through each node in the ring.

Duplication involves the switch automatically sending a single frame
from the CPU port to both redundant ports. This is required because the
inserted HSR/PRP header/trailer must contain the same sequence number
on the frames sent out both redundant ports.

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

This is just a high level overview for now, but I would split this into two:

- a patch that adds HSR offload to the existing HSR stack and introduces
the new netdev_features_t bits to support HSR offload, more on that below

- a patch that does the plumbing between HSR and within the DSA framework

Okay. I think I could just move everything that touches DSA into a
second commit. I kind of pondered this to begin but settled on it
being unnecessary.

quoted

---

Do you think we can start with a hsr-hw-offload feature and create new
bits to described how challenged a device may be with HSR offload? Is it
 reasonable assumption that functional hardware should be able to
offload all of these functions or none of them?

I'm fine with either way. I see the ksz9477 supports HSR (not sure
which version) and it looks like it doesn't do header insertion or
removal.

I expect the insertion and removal would always come as a pair but I
suppose someone could always do something weird and just do one.
Forwarding and duplication seem like a given for anything claiming
HSR/PRP hardware support.

Knowing this do you think I should go ahead and just change it to
hsr-hw-offload and if someone adds ksz9477 support or whatever later
they can add a new bit?

It may be a good idea to know what the platform that Murali is working
on or has worked on is capable of doing, too.

Yeah, I copied him but it bounced as did yours.

[snip]

quoted

+static inline bool netif_is_hsr_master(const struct net_device *dev)
+{
+     return dev->flags & IFF_MASTER && dev->priv_flags & IFF_HSR;
+}
+
+static inline bool netif_is_hsr_slave(const struct net_device *dev)
+{
+     return dev->flags & IFF_SLAVE && dev->priv_flags & IFF_HSR;
+}

I believe the kernel community is now trying to eliminate the use of the
terms master and slave, can you replace these with some HSR specific
naming maybe?

Understood however these both already exist and are referenced in ~40
places. Are you saying I should create different macros that wrap them
or change everywhere they are used?

--
Florian