[net-next PATCH v7 1/8] octeontx2-pf: map skb data as device writeable

Bharat Bhushan posted 8 patches 1 year, 3 months ago
There is a newer version of this series
[net-next PATCH v7 1/8] octeontx2-pf: map skb data as device writeable
Posted by Bharat Bhushan 1 year, 3 months ago
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(-)

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;
 }
-- 
2.34.1
Re: [net-next PATCH v7 1/8] octeontx2-pf: map skb data as device writeable
Posted by Jakub Kicinski 1 year, 3 months ago
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?
Re: [net-next PATCH v7 1/8] octeontx2-pf: map skb data as device writeable
Posted by Bharat Bhushan 1 year, 3 months ago
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
Re: [net-next PATCH v7 1/8] octeontx2-pf: map skb data as device writeable
Posted by Jakub Kicinski 1 year, 3 months ago
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.
Re: [net-next PATCH v7 1/8] octeontx2-pf: map skb data as device writeable
Posted by Bharat Bhushan 1 year, 3 months ago
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