| Message ID | 20240827133210.1418411-2-bbhushan2@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | cn10k-ipsec: Add outbound inline ipsec support | expand |
On Tue, 27 Aug 2024 19:02:03 +0530 Bharat Bhushan wrote: > Crypto hardware need write permission for in-place encrypt > or decrypt operation on skb-data to support IPsec crypto > offload. That patch uses skb_unshare to make sdk data writeable sdk -> skb ? :( > for ipsec crypto offload and map skb fragment memory as > device read-write. Does the crypto engine always override the data with ciphertext? How did you test this prior to adding skb_unshare()? Could you share some performance data with this change?
On Thu, Aug 29, 2024 at 6:51 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Tue, 27 Aug 2024 19:02:03 +0530 Bharat Bhushan wrote: > > Crypto hardware need write permission for in-place encrypt > > or decrypt operation on skb-data to support IPsec crypto > > offload. That patch uses skb_unshare to make sdk data writeable > > sdk -> skb ? :( Will fix in next version > > > for ipsec crypto offload and map skb fragment memory as > > device read-write. > > Does the crypto engine always override the data with ciphertext? yes, > How did you test this prior to adding skb_unshare()? > Could you share some performance data with this change? testing using flood ping and iperf with multiple instance, I do not see any drop in performance numbers Thanks -Bharat
On Thu, 29 Aug 2024 11:17:25 +0530 Bharat Bhushan wrote: > > How did you test this prior to adding skb_unshare()? > > Could you share some performance data with this change? > > testing using flood ping and iperf with multiple instance, Makes sense, neither of these will detect corruption of data pages :( IIRC iperf just ignores the data, ping doesn't retransmit. You gotta beef up your testing... > I do not see any drop in performance numbers Well. What's the difference in CPU busy time of v5 vs v7? You'll copy all TCP packets, they are (pretty much) all clones.
On Thu, Aug 29, 2024 at 8:18 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 29 Aug 2024 11:17:25 +0530 Bharat Bhushan wrote: > > > How did you test this prior to adding skb_unshare()? > > > Could you share some performance data with this change? > > > > testing using flood ping and iperf with multiple instance, > > Makes sense, neither of these will detect corruption of data pages :( > IIRC iperf just ignores the data, ping doesn't retransmit. > You gotta beef up your testing... > > > I do not see any drop in performance numbers > > Well. What's the difference in CPU busy time of v5 vs v7? > You'll copy all TCP packets, they are (pretty much) all clones. cpu is 5-8% more busy when skb unshare. Thanks -Bharat
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c index 3eb85949677a..6ed27d900426 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c @@ -11,6 +11,7 @@ #include <linux/bpf.h> #include <linux/bpf_trace.h> #include <net/ip6_checksum.h> +#include <net/xfrm.h> #include "otx2_reg.h" #include "otx2_common.h" @@ -83,10 +84,17 @@ static unsigned int frag_num(unsigned int i) static dma_addr_t otx2_dma_map_skb_frag(struct otx2_nic *pfvf, struct sk_buff *skb, int seg, int *len) { + enum dma_data_direction dir = DMA_TO_DEVICE; const skb_frag_t *frag; struct page *page; int offset; + /* Crypto hardware need write permission for ipsec crypto offload */ + if (unlikely(xfrm_offload(skb))) { + dir = DMA_BIDIRECTIONAL; + skb = skb_unshare(skb, GFP_ATOMIC); + } + /* First segment is always skb->data */ if (!seg) { page = virt_to_page(skb->data); @@ -98,16 +106,22 @@ static dma_addr_t otx2_dma_map_skb_frag(struct otx2_nic *pfvf, offset = skb_frag_off(frag); *len = skb_frag_size(frag); } - return otx2_dma_map_page(pfvf, page, offset, *len, DMA_TO_DEVICE); + return otx2_dma_map_page(pfvf, page, offset, *len, dir); } static void otx2_dma_unmap_skb_frags(struct otx2_nic *pfvf, struct sg_list *sg) { + enum dma_data_direction dir = DMA_TO_DEVICE; + struct sk_buff *skb = NULL; int seg; + skb = (struct sk_buff *)sg->skb; + if (unlikely(xfrm_offload(skb))) + dir = DMA_BIDIRECTIONAL; + for (seg = 0; seg < sg->num_segs; seg++) { otx2_dma_unmap_page(pfvf, sg->dma_addr[seg], - sg->size[seg], DMA_TO_DEVICE); + sg->size[seg], dir); } sg->num_segs = 0; }
Crypto hardware need write permission for in-place encrypt or decrypt operation on skb-data to support IPsec crypto offload. That patch uses skb_unshare to make sdk data writeable for ipsec crypto offload and map skb fragment memory as device read-write. Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> --- v6->v7: - skb data was mapped as device writeable but it was not ensured that skb is writeable. This version calls skb_unshare() to make skb data writeable. .../ethernet/marvell/octeontx2/nic/otx2_txrx.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)