Subject: Re: [PATCH xfrm-next v3 0/6] Extend XFRM core to allow full offload configuration

From: Jakub Kicinski <[email-masked]>
Date: Fri, 26 Aug 2022 16:45:22 -0700

On Fri, 26 Aug 2022 09:26:57 +0300 Leon Romanovsky wrote:
> On Thu, Aug 25, 2022 at 02:36:10PM -0700, Jakub Kicinski wrote:
> > On Tue, 23 Aug 2022 16:31:57 +0300 Leon Romanovsky wrote:  
> > >  * I didn't hear any suggestion what term to use instead of
> > >    "full offload", so left it as is. It is used in commit messages
> > >    and documentation only and easy to rename.
> > >  * Added performance data and background info to cover letter
> > >  * Reused xfrm_output_resume() function to support multiple XFRM transformations
> > >  * Add PMTU check in addition to driver .xdo_dev_offload_ok validation
> > >  * Documentation is in progress, but not part of this series yet.  
> > 
> > Since the use case is somewhat in question, perhaps switch to RFC
> > postings until the drivers side incl. tc forwarding is implemented?  
> 
> Proposed driver implementation works fine with eswitch representors.
> All our flow steering magic is performed on local table entry and it
> ensures that representors receives/sends "clean" traffic.
> 
> We are using the following configuration snippet to achieve that.
> 
> ---------------------------------------------------------------------
> #!/bin/bash
> P0_OUTER_REMOTE_IP=192.168.50.2
> P0_OUTER_LOCAL_IP=192.168.50.1
> PF0=enp8s0f0
> VF0_REP=enp8s0f0_0
> 
> set -v
> # Configure IP and turn VF_REP on
> ifconfig $PF0 $P0_OUTER_LOCAL_IP/24 up
> ifconfig $VF0_REP up
> 
> # Clean all TC rules, start fresh
> tc qdisc del dev enp8s0f0 ingress >/dev/null 2>&1
> tc qdisc del dev enp8s0f0_0 ingress >/dev/null 2>&1
> 
> # Make sure steering mode is dmfs(FW) and eswitch encap is none
> devlink dev param set pci/0000:08:00.0 name flow_steering_mode value dmfs cmode runtime
> devlink dev eswitch set pci/0000:08:00.0 mode legacy
> devlink dev eswitch set pci/0000:08:00.0 encap none
> devlink dev eswitch set pci/0000:08:00.0 mode switchdev
> 
> sleep 2
> 
> tc qdisc add dev enp8s0f0 ingress
> tc qdisc add dev enp8s0f0_0 ingress
> 
> # Add TC rules
> tc filter add dev $PF0 parent ffff: protocol 802.1q chain 0 flower vlan_id 10 vlan_ethtype 802.1q cvlan_id 5 action vlan pop action vlan pop  action mirred egress redirect dev $VF0_REP
> tc filter add dev $VF0_REP parent ffff: protocol all chain 0 flower action vlan push protocol 802.1q id 5 action vlan push protocol 802.1q id 10 action mirred egress redirect dev $PF0
> tc filter show dev $PF0 ingress
> ----------------------------------------------------------------------------------------------------
> 
> We also don't offload anything related to routing as we can't
> differentiate between local traffic.

Yeah, nah, that's not what I'm asking for.
I said forwarding, not sending traffic thru a different virtual
interface. The TC rules must forward from or two the IPSec ifc.

That was the use case Jason mentioned.

> > Also the perf traces, I don't see them here.  
> 
> It is worth to separate it to standalone discussion with a title:
> "why crypto is not fast enough?". I don't think that mixed discussions
> about full offload which Steffen said that he is interested and
> research about crypto bottlenecks will be productive. These discussions
> are orthogonal.

What do you mean by crypto bottlenecks?

Please use more precise language. crypto here may mean "crypto only
offload" or "crypto as done by CPU". I have no idea which one you mean.

We are very much interested in the former, the latter is indeed out of
scope here.