Re: [PATCH net-next v3 3/6] net: ti: icssg-prueth: Add support for HSR frame forward offload

[MD Danish Anwar <danishanwar@ti.com>]

On 02/09/24 6:32 pm, Andrew Lunn wrote:
>> Yes, and I have already added this in this series based on your feedback
>> on v2.
>>
>> I have one question though, in emac_ndo_set_features() should I change
>> these HSR related features irrespective of the current mode?
>>
>> AFAIK, if NETIF_F_HW_HSR_FWD is set, the forwarding is offloaded to HW.
>> If NETIF_F_HW_HSR_FWD is not set the forwarding is not offloaded to HW
>> and is done in SW.
>>
>> So, I don't see any need to enable this features if we are currently in
>> switch mode. Let me know what do you think. Should I still enable this
>> feature irrespective of current mode and later handle this in
>> prueth_hsr_port_link / unlink()?
> 
> The user should not need to know about the different firmwares. So i
> would allow NETIF_F_HW_HSR_FWD at any time.
> 
> The exception would be, if you look at all the other drivers which
> implement HSR offload, if they all return an error if the offloading
> cannot be enabled, then you should do the same.
> 

Andrew, I looked at xrs700x dsa and microchip ksz dsa driver as an
example. The drivers return -EOPNOTSUPP whenever offloading is not
possible in the xrs700x_hsr_join / ksz_hsr_join api. I think the same
should be okay for ICSSG driver as well. ICSSG drivers equivalent API is
prueth_hsr_port_link() where I will return -EOPNOTSUPP whenever
offloading is not possible.

So in the .ndo_set_features() I will not add any check so that the user
wouldn't know about the current firmware and whether offloading is
supported or not. In the prueth_hsr_port_link() based on firmware
limitation we will error out.

Andrew, Roger, Let me know if this sounds good.

> 	Andrew

-- 
Thanks and Regards,
Danish