| Message ID | 20240609173358.193178-1-ast@fiberby.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: flower: validate encapsulation control flags | expand |
On Sun, Jun 09, 2024 at 05:33:50PM +0000, Asbjørn Sloth Tønnesen wrote: > Now that all drivers properly rejects unsupported flower control flags > used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar > checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL. > > There are currently just 4 drivers supporting this key, and > 3 of those currently doesn't validate encapsulated control flags. > > Encapsulation control flags may currently be unused, but they should > still be validated by the drivers, so that drivers will properly > reject any new flags when they are introduced. > > This series adds some helper functions, and implements them in all > 4 drivers. > Reviewed-by: Davide Caratti <dcaratti@redhat.com>
On Sun, 9 Jun 2024 17:33:50 +0000 Asbjørn Sloth Tønnesen wrote: > Now that all drivers properly rejects unsupported flower control flags > used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar > checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL. Thanks for doing this work! Do you have any more of such series left? Could we perhaps consider recording the driver support somewhere in struct net_device and do the rejecting in the core? That's much easier to extend when adding new flags than updating all the drivers. This series I think may not be a great first candidate as IIUC all drivers would reject so the flag would be half-dead...
Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Sun, 9 Jun 2024 17:33:50 +0000 you wrote: > Now that all drivers properly rejects unsupported flower control flags > used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar > checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL. > > There are currently just 4 drivers supporting this key, and > 3 of those currently doesn't validate encapsulated control flags. > > [...] Here is the summary with links: - [net-next,1/5] flow_offload: add encapsulation control flag helpers https://git.kernel.org/netdev/net-next/c/b48a1540b73a - [net-next,2/5] sfc: use flow_rule_is_supp_enc_control_flags() https://git.kernel.org/netdev/net-next/c/2ede54f8786f - [net-next,3/5] net/mlx5e: flower: validate encapsulation control flags https://git.kernel.org/netdev/net-next/c/28d19ec91755 - [net-next,4/5] nfp: flower: validate encapsulation control flags https://git.kernel.org/netdev/net-next/c/34cdd9847820 - [net-next,5/5] ice: flower: validate encapsulation control flags https://git.kernel.org/netdev/net-next/c/5a1b015d521d You are awesome, thank you!
Hi Jakub, On 6/13/24 1:04 AM, Jakub Kicinski wrote: > On Sun, 9 Jun 2024 17:33:50 +0000 Asbjørn Sloth Tønnesen wrote: >> Now that all drivers properly rejects unsupported flower control flags >> used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar >> checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL. > > Thanks for doing this work! Thank you, for maintaining the tree! > Do you have any more of such series left? Not at the moment, I only stumbled upon this, because I read the code with an eye on adding a new IS_JUMBO_FRAME flag (patches for which are almost ready). Once this[1] currently RFC patch goes in, I might try to move all control flags in under FLOW_DIS_F_*, to get rid of the then FLOW_DIS_(IS_FRAGMENT|FIRST_FRAG|ENCAPSULATION|F_*). [1] [RFC PATCH net-next 1/9] net/sched: flower: define new tunnel flags https://lore.kernel.org/netdev/20240611235355.177667-2-ast@fiberby.net/ > Could we perhaps consider > recording the driver support somewhere in struct net_device and do > the rejecting in the core? Sure, it could work for the control flags, and used_keys validation, but I am not sure if it is worth it, as most of the validation is very specific to the limitations of the different hardware. An easy first step in that direction would be to move the used_keys checks behind a helper, and possibly storing used_keys in struct net_device. > That's much easier to extend when adding > new flags than updating all the drivers. That's how it is now, with the new helpers, as all flags are unsupported, unless the driver specifically supports it. Any new flag only needs to be added to the core, and drivers only needs to be updated when they implement offloading support for a flag. > This series I think may not be a great first candidate as IIUC all > drivers would reject so the flag would be half-dead... Correct. I don't know if there is any hardware support planned for the tunnel-related encapsulation control flags.