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: <aL9PSoTwhn-HFWrH@krikkit>
Date: Mon, 8 Sep 2025 23:48:58 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Hangbin Liu <liuhangbin@...il.com>
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

2025-09-08, 10:14:57 +0000, Hangbin Liu wrote:
> 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.

If it's not needed for this series, sure.


> > > 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.

Ok. But hw_enc_features is not the reason ethtool shows
"esp-hw-offload off". This line is:

	bond_dev->hw_features |= BOND_XFRM_FEATURES;

(from bond_setup)

> Do you think if we should update dev->hw_features in the
> patch?

For dev->hw_features (and dev->features) maybe not, since that depends
on the upper device's features and implementation. I'm not sure we can
have a common function without changing the behavior on at least one
type of device.

But maybe ndo_fix_features could use a common
netdev_fix_features_from_lowers? bond/team/bridge have very similar
implementations.

> > > 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?

Ok for me.

> 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.

inet_gso_segment via inet_offloads[] (ESP is a L4 proto like UDP etc).

-- 
Sabrina