Re: [PATCHv2 net-next 5/5] selftests/net: add offload checking test for virtual interface

[Hangbin Liu <liuhangbin@gmail.com>]

On Mon, Sep 08, 2025 at 11:36:21AM +0200, Sabrina Dubroca wrote:
> > The esp-hw-offload is fixed on netdevsim
> > 
> > # ethtool -k eni0np1 | grep -i esp-hw-offload
> > esp-hw-offload: on [fixed]
> > 
> > There is no way to disable it.
> 
> I don't think this is intentional. nsim_ipsec_init only adds
> NSIM_ESP_FEATURES to ->features but not to ->hw_features, but I think
> it was just forgotten. I added a few in 494bd83bb519 ("netdevsim: add
> more hw_features"), extending nsim_ipsec_init (and nsim_macsec_init
> since I made the same mistake) to also add features to ->hw_features
> would make sense to me.

This could be done in another patch.

> 
> > After we add the netdevsim to bond,
> > the bond also shows "esp-hw-offload off" as the flag is inherit
> > in dev->hw_enc_features, not dev->features.
> 
> Did you mean dev->hw_features?

No, the xfrm_features in patch 01 updates dev->hw_enc_features, not
dev->hw_features. Do you think if we should update dev->hw_features in the
patch?

> 
> > It looks the only way to check if bond dev->hw_enc_features has NETIF_F_HW_ESP
> > is try set xfrm offload. As
> 
> Was this test meant to check hw_enc_features?
> 
> To check hw_enc_features, I think the only way would be sending GSO
> packets, since it's only used in those situations.

Oh.. That would make the test complex. Can we ignore this test first?

BTW, I'm a bit lost in the callbacks.gso_segment. e.g.

esp4_gso_segment
 - xfrm4_outer_mode_gso_segment
   - xfrm4_transport_gso_segment
     - ops->callbacks.gso_segment

But who calls esp4_gso_segment? I can't find where the features is assigned.

> 
> 
> > static int xfrm_api_check(struct net_device *dev)
> > {
> 
> But this doesn't get called when creating a new xfrm state. Trying to
> create a new offloaded xfrm state doesn't look at any of the
> netdev->*features (and we can't change that behavior anymore).
> 
> xfrm_api_check only gets called for NETDEV_REGISTER/NETDEV_FEAT_CHANGE
> to validate whether the netdevice is set up correctly.

Thanks for correcting me.

Regards
Hangbin