| Message ID | cover.1692262560.git.leonro@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | devlink: Add port function attributes | expand |
On Thu, 17 Aug 2023 12:11:22 +0300 Leon Romanovsky wrote: > Introduce hypervisor-level control knobs to set the functionality of PCI > VF devices passed through to guests. The administrator of a hypervisor > host may choose to change the settings of a port function from the > defaults configured by the device firmware. > > The software stack has two types of IPsec offload - crypto and packet. > Specifically, the ip xfrm command has sub-commands for "state" and > "policy" that have an "offload" parameter. With ip xfrm state, both > crypto and packet offload types are supported, while ip xfrm policy can > only be offloaded in packet mode. > > The series introduces two new boolean attributes of a port function: > ipsec_crypto and ipsec_packet. The goal is to provide a similar level of > granularity for controlling VF IPsec offload capabilities, which would > be aligned with the software model. This will allow users to decide if > they want both types of offload enabled for a VF, just one of them, or > none at all (which is the default). > > At a high level, the difference between the two knobs is that with > ipsec_crypto, only XFRM state can be offloaded. Specifically, only the > crypto operation (Encrypt/Decrypt) is offloaded. With ipsec_packet, both > XFRM state and policy can be offloaded. Furthermore, in addition to > crypto operation offload, IPsec encapsulation is also offloaded. For > XFRM state, choosing between crypto and packet offload types is > possible. From the HW perspective, different resources may be required > for each offload type. What's going on with all the outstanding nVidia patches?! The expectation is 1 series per vendor / driver. Let's say 2 if there are core changes. You had 5 outstanding today. I'm tossing this out.
On Thu, Aug 17, 2023 at 08:07:25PM -0700, Jakub Kicinski wrote: > On Thu, 17 Aug 2023 12:11:22 +0300 Leon Romanovsky wrote: > > Introduce hypervisor-level control knobs to set the functionality of PCI > > VF devices passed through to guests. The administrator of a hypervisor > > host may choose to change the settings of a port function from the > > defaults configured by the device firmware. > > > > The software stack has two types of IPsec offload - crypto and packet. > > Specifically, the ip xfrm command has sub-commands for "state" and > > "policy" that have an "offload" parameter. With ip xfrm state, both > > crypto and packet offload types are supported, while ip xfrm policy can > > only be offloaded in packet mode. > > > > The series introduces two new boolean attributes of a port function: > > ipsec_crypto and ipsec_packet. The goal is to provide a similar level of > > granularity for controlling VF IPsec offload capabilities, which would > > be aligned with the software model. This will allow users to decide if > > they want both types of offload enabled for a VF, just one of them, or > > none at all (which is the default). > > > > At a high level, the difference between the two knobs is that with > > ipsec_crypto, only XFRM state can be offloaded. Specifically, only the > > crypto operation (Encrypt/Decrypt) is offloaded. With ipsec_packet, both > > XFRM state and policy can be offloaded. Furthermore, in addition to > > crypto operation offload, IPsec encapsulation is also offloaded. For > > XFRM state, choosing between crypto and packet offload types is > > possible. From the HW perspective, different resources may be required > > for each offload type. > > What's going on with all the outstanding nVidia patches?! > The expectation is 1 series per vendor / driver. Let's say > 2 if there are core changes. You had 5 outstanding today. I sent only three security related series, two of three were already reviewed and waiting to be applied [1,2]. This third series is only one which touches core. It is very strange to expect 1 series per vendor/driver without taking into account the size of that driver and the amount of upstream work involvement from that vendor. Thanks [1] https://patchwork.kernel.org/project/netdevbpf/list/?series=774239&state=* [2] https://patchwork.kernel.org/project/netdevbpf/list/?series=775702 > > I'm tossing this out. > -- > pw-bot: defer
On Fri, 18 Aug 2023 07:19:59 +0300 Leon Romanovsky wrote: > It is very strange to expect 1 series per vendor/driver without taking > into account the size of that driver and the amount of upstream work > involvement from that vendor. According to the "reviewer rotation" nVidia is supposed to be reviewing this week. Sorry it fell on you in particular, but as a company y'all definitely are not pulling your weight. Then Saeed pings me to pull your RDMA stuff faster, and you have to audacity to call the basic rules we had for a very long time "very strange". SMH
On Fri, Aug 18, 2023 at 09:38:12AM -0700, Jakub Kicinski wrote: > On Fri, 18 Aug 2023 07:19:59 +0300 Leon Romanovsky wrote: > > It is very strange to expect 1 series per vendor/driver without taking > > into account the size of that driver and the amount of upstream work > > involvement from that vendor. > > According to the "reviewer rotation" nVidia is supposed to be reviewing > this week. Sorry it fell on you in particular, but as a company y'all > definitely are not pulling your weight. I can't speak for my company and for my colleagues who are OOO during these days because of summer break, but only for me. I'm trying my best to review and reduce maintainers burden. Should I stop? > Then Saeed pings me to pull your RDMA stuff faster Saeed explained to you why he needs it faster - merge conflict. > and you have to audacity to call the basic rules we had for a very long > time "very strange". This rule relies on basic contract of 1 series -> fast review/acceptance. Once fast review/acceptance doesn't happen, what else do you expect from me? Thanks > > SMH
On Fri, 18 Aug 2023 21:36:40 +0300 Leon Romanovsky wrote: > > and you have to audacity to call the basic rules we had for a very long > > time "very strange". > > This rule relies on basic contract of 1 series -> fast review/acceptance. > Once fast review/acceptance doesn't happen, what else do you expect from me? Since you don't understand what I'm asking please let Saeed post your patches.
On 18 Aug 14:32, Jakub Kicinski wrote: >On Fri, 18 Aug 2023 21:36:40 +0300 Leon Romanovsky wrote: >> > and you have to audacity to call the basic rules we had for a very long >> > time "very strange". >> >> This rule relies on basic contract of 1 series -> fast review/acceptance. >> Once fast review/acceptance doesn't happen, what else do you expect from me? > >Since you don't understand what I'm asking please let Saeed post >your patches. > I usually try to avoid posting API changes as part of my pure mlx5 PRs. So they get their fare share of focused review. I prefer if such standalone patchsets to be submitted individually, as they are mostly introducing new devlink knobs. This one isn't a new series hence the V3, so I am not sure I can agree with the parallel submissions issue here. For the two ipsec UDP and TCP selector patches [1] I agree they should've been part of my PRs. [1] https://patchwork.kernel.org/project/netdevbpf/cover/cover.1691521680.git.leonro@nvidia.com/ I will take them to my next PR. Thanks, Saeed.
From: Leon Romanovsky <leonro@nvidia.com> v3: - Changed newly introduced IPsec blocking routine and as an outcome of testing already existing one. I'm sending them together to avoid merge conflicts. - Refactored patches to separate IFC part - Simplified v2: https://lore.kernel.org/netdev/20230421104901.897946-1-dchumak@nvidia.com/ - Improve docs of ipsec_crypto vs ipsec_packet devlink attributes - also see patches 2,4 for the changelog. --------------------------------------------------------------------------------- From Dima: Introduce hypervisor-level control knobs to set the functionality of PCI VF devices passed through to guests. The administrator of a hypervisor host may choose to change the settings of a port function from the defaults configured by the device firmware. The software stack has two types of IPsec offload - crypto and packet. Specifically, the ip xfrm command has sub-commands for "state" and "policy" that have an "offload" parameter. With ip xfrm state, both crypto and packet offload types are supported, while ip xfrm policy can only be offloaded in packet mode. The series introduces two new boolean attributes of a port function: ipsec_crypto and ipsec_packet. The goal is to provide a similar level of granularity for controlling VF IPsec offload capabilities, which would be aligned with the software model. This will allow users to decide if they want both types of offload enabled for a VF, just one of them, or none at all (which is the default). At a high level, the difference between the two knobs is that with ipsec_crypto, only XFRM state can be offloaded. Specifically, only the crypto operation (Encrypt/Decrypt) is offloaded. With ipsec_packet, both XFRM state and policy can be offloaded. Furthermore, in addition to crypto operation offload, IPsec encapsulation is also offloaded. For XFRM state, choosing between crypto and packet offload types is possible. From the HW perspective, different resources may be required for each offload type. Examples of when a user prefers to enable IPsec packet offload for a VF when using switchdev mode: $ devlink port show pci/0000:06:00.0/1 pci/0000:06:00.0/1: type eth netdev enp6s0pf0vf0 flavour pcivf pfnum 0 vfnum 0 function: hw_addr 00:00:00:00:00:00 roce enable migratable disable ipsec_crypto disable ipsec_packet disable $ devlink port function set pci/0000:06:00.0/1 ipsec_packet enable $ devlink port show pci/0000:06:00.0/1 pci/0000:06:00.0/1: type eth netdev enp6s0pf0vf0 flavour pcivf pfnum 0 vfnum 0 function: hw_addr 00:00:00:00:00:00 roce enable migratable disable ipsec_crypto disable ipsec_packet enable This enables the corresponding IPsec capability of the function before it's enumerated, so when the driver reads the capability from the device firmware, it is enabled. The driver is then able to configure corresponding features and ops of the VF net device to support IPsec state and policy offloading. Thanks Dima Chumak (4): devlink: Expose port function commands to control IPsec crypto offloads devlink: Expose port function commands to control IPsec packet offloads net/mlx5: Implement devlink port function cmds to control ipsec_crypto net/mlx5: Implement devlink port function cmds to control ipsec_packet Leon Romanovsky (4): net/mlx5: Drop extra layer of locks in IPsec net/mlx5e: Rewrite IPsec vs. TC block interface net/mlx5: Add IFC bits to support IPsec enable/disable net/mlx5: Provide an interface to block change of IPsec capabilities .../ethernet/mellanox/mlx5/switchdev.rst | 20 + .../networking/devlink/devlink-port.rst | 55 +++ .../net/ethernet/mellanox/mlx5/core/Makefile | 2 +- .../mellanox/mlx5/core/en_accel/ipsec.c | 20 +- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 63 ++- .../mellanox/mlx5/core/esw/devlink_port.c | 6 + .../ethernet/mellanox/mlx5/core/esw/ipsec.c | 369 ++++++++++++++++++ .../net/ethernet/mellanox/mlx5/core/eswitch.c | 41 ++ .../net/ethernet/mellanox/mlx5/core/eswitch.h | 48 ++- .../mellanox/mlx5/core/eswitch_offloads.c | 259 +++++++++--- include/linux/mlx5/driver.h | 1 + include/linux/mlx5/mlx5_ifc.h | 3 + include/net/devlink.h | 30 ++ include/uapi/linux/devlink.h | 4 + net/devlink/leftover.c | 104 +++++ 15 files changed, 917 insertions(+), 108 deletions(-) create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec.c