| Message ID | 20250205135341.542720-1-gal@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Symmetric OR-XOR RSS hash | expand |
On Wed, 5 Feb 2025 15:53:39 +0200 Gal Pressman wrote: > Add support for a new type of input_xfrm: Symmetric OR-XOR. > Symmetric OR-XOR performs hash as follows: > (SRC_IP | DST_IP, SRC_IP ^ DST_IP, SRC_PORT | DST_PORT, SRC_PORT ^ DST_PORT) > > Configuration is done through ethtool -x/X command. > For mlx5, the default is already symmetric hash, this patch now exposes > this to userspace and allows enabling/disabling of the feature. Please add a selftest (hw-only is fine, netdevsim can't do flow hashing).
On 07/02/2025 3:32, Jakub Kicinski wrote: > On Wed, 5 Feb 2025 15:53:39 +0200 Gal Pressman wrote: >> Add support for a new type of input_xfrm: Symmetric OR-XOR. >> Symmetric OR-XOR performs hash as follows: >> (SRC_IP | DST_IP, SRC_IP ^ DST_IP, SRC_PORT | DST_PORT, SRC_PORT ^ DST_PORT) >> >> Configuration is done through ethtool -x/X command. >> For mlx5, the default is already symmetric hash, this patch now exposes >> this to userspace and allows enabling/disabling of the feature. > > Please add a selftest (hw-only is fine, netdevsim can't do flow > hashing). I don't understand the rationale, the new input_xfrm field didn't deserve a selftest, why does a new value to the field does? Testing this would require new userspace ethtool (which has not been submitted yet), I don't think it's wise to implement a test before the user interface/output is merged. I assume you want an additional case in rss_ctx.py?
On Sun, 9 Feb 2025 09:59:22 +0200 Gal Pressman wrote: > I don't understand the rationale, the new input_xfrm field didn't > deserve a selftest, why does a new value to the field does? Ahmed and Sudheer added ETHTOOL_MSG_RSS_GET as part of their work. Everyone pays off a little bit of technical debt to get their feature in. I don't appreciate your reaction. Please stop acting as if nVidia was a victim of some grand conspiracy within netdev. > Testing this would require new userspace ethtool (which has not been > submitted yet), I don't think it's wise to implement a test before the > user interface/output is merged. No it doesn't. You can call netlink directly from Python or C. > I assume you want an additional case in rss_ctx.py? No, separate test.
On 11/02/2025 2:27, Jakub Kicinski wrote: > On Sun, 9 Feb 2025 09:59:22 +0200 Gal Pressman wrote: >> I don't understand the rationale, the new input_xfrm field didn't >> deserve a selftest, why does a new value to the field does? > > Ahmed and Sudheer added ETHTOOL_MSG_RSS_GET as part of their work. > Everyone pays off a little bit of technical debt to get their > feature in. I agree with the idea that extensions to ethtool uapi should be accompanied by conversion to netlink. I don't see a connection to testing. If a maintainer has certain expectations about which changes require tests, it should be documented and enforced so it's not up to the maintainer's mood. FWIW, I don't believe kernel contributions should be blocked by lack of a test. > > I don't appreciate your reaction. Please stop acting as if nVidia was > a victim of some grand conspiracy within netdev. I don't know what you're talking about, you've mistaken me for someone else.. > >> Testing this would require new userspace ethtool (which has not been >> submitted yet), I don't think it's wise to implement a test before the >> user interface/output is merged. > > No it doesn't. You can call netlink directly from Python or C. > >> I assume you want an additional case in rss_ctx.py? > > No, separate test. Will address in v3.
On 11 Feb 17:26, Gal Pressman wrote: >On 11/02/2025 2:27, Jakub Kicinski wrote: >> On Sun, 9 Feb 2025 09:59:22 +0200 Gal Pressman wrote: >>> I don't understand the rationale, the new input_xfrm field didn't >>> deserve a selftest, why does a new value to the field does? >> >> Ahmed and Sudheer added ETHTOOL_MSG_RSS_GET as part of their work. >> Everyone pays off a little bit of technical debt to get their >> feature in. > >I agree with the idea that extensions to ethtool uapi should be >accompanied by conversion to netlink. > >I don't see a connection to testing. If a maintainer has certain >expectations about which changes require tests, it should be documented >and enforced so it's not up to the maintainer's mood. FWIW, I don't >believe kernel contributions should be blocked by lack of a test. > >> >> I don't appreciate your reaction. Please stop acting as if nVidia was >> a victim of some grand conspiracy within netdev. The main problem is the tone in theses responses, we appreciate your reviews and comments, but sometimes the responses and comments are a bit too hostile. > >I don't know what you're talking about, you've mistaken me for someone >else.. > Misunderstanding ? .. Gal's response was technical, objective and free from any "I'm a victim" complaints, so I get why he's confused. I understand that there's some friction going on with a few nVidia WIP features, but there's no reasons for comments and reactions to not remain in the technical realm. Anyway I am happy to discuss all open issues and misunderstandings offline. All we need is to just align expectations and work towards a shared plan and a maintainer vs contributor friendly policy. LKM.