DSA slaves not inheriting hw_enc_features and xfrmdev_ops?

DSA slaves not inheriting hw_enc_features and xfrmdev_ops?

[R W van Schagen]

Hi all,

When I register a master device (eth0) with ESP hardware offload:

	netdev->xfrmdev_ops = &mtk_xfrmdev_ops;
	netdev->features |= NETIF_F_HW_ESP;
	netdev->hw_enc_features |= NETIF_F_HW_ESP;

Only the “features” are inherited by the DSA slaves. When those
get registered without the xfrmdev_ops the HW_ESP feature is
dropped again.

Is this a “bug” and should I make a patch to fix this or is this actually
a design feature?

As a work-around I am using a notifier call and add those features but
I don’t think this is the proper way to do this in a production driver.

Thanks
Richard van Schagen

Re: DSA slaves not inheriting hw_enc_features and xfrmdev_ops?

[Andrew Lunn]

On Wed, Oct 20, 2021 at 09:28:40AM +0800, R W van Schagen wrote:
> Hi all,
> 
> When I register a master device (eth0) with ESP hardware offload:
> 
> 	netdev->xfrmdev_ops = &mtk_xfrmdev_ops;
> 	netdev->features |= NETIF_F_HW_ESP;
> 	netdev->hw_enc_features |= NETIF_F_HW_ESP;
> 
> Only the “features” are inherited by the DSA slaves. When those
> get registered without the xfrmdev_ops the HW_ESP feature is
> dropped again.
> 
> Is this a “bug” and should I make a patch to fix this or is this actually
> a design feature?

Design feature.

The problem is, for most MAC devices, the additional DSA
header/trailer messes up all acceleration. The HW does not understand
the header/trailer, don't know they have to skip it, have trouble
finding the IP header, etc. So in general, we turn off all
acceleration features.

If you pair a Marvell MAC with a Marvell Switch, there is a good
chance it understands the Marvell DSA header and some forms of
acceleration work. Same goes for a Broadcom MAC with a Broadcom
switch. But pair a Freescale MAC with a Marvell Switch and even basic
IP checksumming does not work, the FEC HW cannot find the IP header.

> As a work-around I am using a notifier call and add those features but
> I don’t think this is the proper way to do this in a production driver.

It is not a simple problem to solve in a generic way. You end up with
an M by S matrices for HW features which work, where M is the MAC and
S is the switch.

So for you board, with your specific pairing of MAC and Switch, which
i assume is a mediatek MAC connected to a mediatek switch, using a
notifier call is not too unreasonable.

We could also consider DT properties, indicating which features work
for this board. That should be a reasonably generic solution, which
you can implement in the DSA core.

	    Andrew

Re: DSA slaves not inheriting hw_enc_features and xfrmdev_ops?

[R W van Schagen]

> On 20 Oct 2021, at 20:06, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> On Wed, Oct 20, 2021 at 09:28:40AM +0800, R W van Schagen wrote:
>> Hi all,
>> 
>> When I register a master device (eth0) with ESP hardware offload:
>> 
>> 	netdev->xfrmdev_ops = &mtk_xfrmdev_ops;
>> 	netdev->features |= NETIF_F_HW_ESP;
>> 	netdev->hw_enc_features |= NETIF_F_HW_ESP;
>> 
>> Only the “features” are inherited by the DSA slaves. When those
>> get registered without the xfrmdev_ops the HW_ESP feature is
>> dropped again.
>> 
>> Is this a “bug” and should I make a patch to fix this or is this actually
>> a design feature?
> 
> Design feature.
> 
> The problem is, for most MAC devices, the additional DSA
> header/trailer messes up all acceleration. The HW does not understand
> the header/trailer, don't know they have to skip it, have trouble
> finding the IP header, etc. So in general, we turn off all
> acceleration features.
> 
> If you pair a Marvell MAC with a Marvell Switch, there is a good
> chance it understands the Marvell DSA header and some forms of
> acceleration work. Same goes for a Broadcom MAC with a Broadcom
> switch. But pair a Freescale MAC with a Marvell Switch and even basic
> IP checksumming does not work, the FEC HW cannot find the IP header.
> 
>> As a work-around I am using a notifier call and add those features but
>> I don’t think this is the proper way to do this in a production driver.
> 
> It is not a simple problem to solve in a generic way. You end up with
> an M by S matrices for HW features which work, where M is the MAC and
> S is the switch.
> 
> So for you board, with your specific pairing of MAC and Switch, which
> i assume is a mediatek MAC connected to a mediatek switch, using a
> notifier call is not too unreasonable.
> 
> We could also consider DT properties, indicating which features work
> for this board. That should be a reasonably generic solution, which
> you can implement in the DSA core.
> 
> 	    Andrew

Thanks for the explanation. For now I will proceed using notifier callbacks.

One more strange thing I am noticing: Even if I set NETIF_F_GSO_ESP
I am still not getting any GSO packets (skb_is_gso is always false) so my
transmit speeds are like 2/3 of the receive speeds. Hardware Decryption vs
Encryption is not 100% the same, but it is close.

I am getting the esp_gro_receive callbacks, but not the esp_gso_segment,
BUT: for some reason my packets still get TCP GSO.

Richard

Re: DSA slaves not inheriting hw_enc_features and xfrmdev_ops?

[Andrew Lunn]

> Thanks for the explanation. For now I will proceed using notifier callbacks.
> 
> One more strange thing I am noticing: Even if I set NETIF_F_GSO_ESP
> I am still not getting any GSO packets (skb_is_gso is always false) so my
> transmit speeds are like 2/3 of the receive speeds. Hardware Decryption vs
> Encryption is not 100% the same, but it is close.
> 
> I am getting the esp_gro_receive callbacks, but not the esp_gso_segment,
> BUT: for some reason my packets still get TCP GSO.

I'm not too familiar with GSO. But my understanding is that you create
a template set of headers which need to be placed onto each frame when
the segmentation actually happens. For DSA, that template would need
to include the DSA header. As far as i understand, there is nothing in
the DSA core that allows for adding the DSA headers into the template.
So you might be able to do GSO at the slave interface, but when the
slave passes frames to the master, you then require segmentation to
happen, so the tag driver can add the DSA header.

	Andrew