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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aL6soY3gEj-LIovi@fedora>
Date: Mon, 8 Sep 2025 10:14:57 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: netdev@...r.kernel.org, Jay Vosburgh <jv@...sburgh.net>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jiri Pirko <jiri@...nulli.us>, Simon Horman <horms@...nel.org>,
	Ido Schimmel <idosch@...dia.com>, Shuah Khan <shuah@...nel.org>,
	Stanislav Fomichev <sdf@...ichev.me>,
	Kuniyuki Iwashima <kuniyu@...gle.com>,
	Ahmed Zaki <ahmed.zaki@...el.com>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	bridge@...ts.linux.dev, linux-kselftest@...r.kernel.org
Subject: Re: [PATCHv2 net-next 5/5] selftests/net: add offload checking test
 for virtual interface

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