[dpdk-dev] kni: add chained mbufs support

Message ID 1461600664-16481-1-git-send-email-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Ferruh Yigit April 25, 2016, 4:11 p.m. UTC
  rx_q fifo may have chained mbufs, merge them into single skb before
handing to the network stack.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 .../linuxapp/eal/include/exec-env/rte_kni_common.h |  4 +-
 lib/librte_eal/linuxapp/kni/kni_net.c              | 83 ++++++++++++++++------
 2 files changed, 64 insertions(+), 23 deletions(-)
  

Comments

Zhang, Helin April 26, 2016, 6:49 a.m. UTC | #1
Have you tested with it?
I think we need to test it in a longer time, e.g. 1 hour
My commetns inlined.

Thanks,
Helin

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, April 26, 2016 12:11 AM
> To: dev@dpdk.org
> Cc: Zhang, Helin; Yigit, Ferruh
> Subject: [PATCH] kni: add chained mbufs support
> 
> rx_q fifo may have chained mbufs, merge them into single skb before
> handing to the network stack.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  .../linuxapp/eal/include/exec-env/rte_kni_common.h |  4 +-
>  lib/librte_eal/linuxapp/kni/kni_net.c              | 83 ++++++++++++++++------
>  2 files changed, 64 insertions(+), 23 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> index 7e5e598..2acdfd9 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> @@ -113,7 +113,9 @@ struct rte_kni_mbuf {
>  	void *buf_addr __attribute__((__aligned__(RTE_CACHE_LINE_SIZE)));
>  	char pad0[10];
>  	uint16_t data_off;      /**< Start address of data in segment buffer.
> */
> -	char pad1[4];
> +	char pad1[2];
> +	uint8_t nb_segs;        /**< Number of segments. */
> +	char pad4[1];
>  	uint64_t ol_flags;      /**< Offload features. */
>  	char pad2[4];
>  	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len.
> */
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
> b/lib/librte_eal/linuxapp/kni/kni_net.c
> index cfa8339..570de71 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -156,7 +156,8 @@ kni_net_rx_normal(struct kni_dev *kni)
>  	/* Transfer received packets to netif */
>  	for (i = 0; i < num_rx; i++) {
>  		kva = (void *)va[i] - kni->mbuf_va + kni->mbuf_kva;
> -		len = kva->data_len;
> +		len = kva->pkt_len;
> +
>  		data_kva = kva->buf_addr + kva->data_off - kni->mbuf_va
>  				+ kni->mbuf_kva;
> 
> @@ -165,22 +166,41 @@ kni_net_rx_normal(struct kni_dev *kni)
>  			KNI_ERR("Out of mem, dropping pkts\n");
>  			/* Update statistics */
>  			kni->stats.rx_dropped++;
> +			continue;
>  		}
> -		else {
> -			/* Align IP on 16B boundary */
> -			skb_reserve(skb, 2);
> +
> +		/* Align IP on 16B boundary */
> +		skb_reserve(skb, 2);
> +
> +		if (kva->nb_segs == 0) {
I guess it should compare nb_segs with 1, but not 0. Am I wrong?

>  			memcpy(skb_put(skb, len), data_kva, len);
> -			skb->dev = dev;
> -			skb->protocol = eth_type_trans(skb, dev);
> -			skb->ip_summed = CHECKSUM_UNNECESSARY;
> +		} else {
> +			int nb_segs;
> +			int kva_nb_segs = kva->nb_segs;
> 
> -			/* Call netif interface */
> -			netif_rx_ni(skb);
> +			for (nb_segs = 0; nb_segs < kva_nb_segs; nb_segs++)
Kva_nb_segs might not needed at all, use kva->nb_segs directly?

> {
> +				memcpy(skb_put(skb, kva->data_len),
> +					data_kva, kva->data_len);
> 
> -			/* Update statistics */
> -			kni->stats.rx_bytes += len;
> -			kni->stats.rx_packets++;
> +				if (!kva->next)
> +					break;
> +
> +				kva = kva->next - kni->mbuf_va + kni-
> >mbuf_kva;
> +				data_kva = kva->buf_addr + kva->data_off
> +					- kni->mbuf_va + kni->mbuf_kva;
> +			}
>  		}
> +
> +		skb->dev = dev;
> +		skb->protocol = eth_type_trans(skb, dev);
> +		skb->ip_summed = CHECKSUM_UNNECESSARY;
> +
> +		/* Call netif interface */
> +		netif_rx_ni(skb);
> +
> +		/* Update statistics */
> +		kni->stats.rx_bytes += len;
> +		kni->stats.rx_packets++;
>  	}
> 
>  	/* Burst enqueue mbufs into free_q */
> @@ -317,7 +337,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
>  	/* Copy mbufs to sk buffer and then call tx interface */
>  	for (i = 0; i < num; i++) {
>  		kva = (void *)va[i] - kni->mbuf_va + kni->mbuf_kva;
> -		len = kva->data_len;
> +		len = kva->pkt_len;
>  		data_kva = kva->buf_addr + kva->data_off - kni->mbuf_va +
>  				kni->mbuf_kva;
> 
> @@ -338,20 +358,39 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
>  		if (skb == NULL) {
>  			KNI_ERR("Out of mem, dropping pkts\n");
>  			kni->stats.rx_dropped++;
> +			continue;
>  		}
> -		else {
> -			/* Align IP on 16B boundary */
> -			skb_reserve(skb, 2);
> +
> +		/* Align IP on 16B boundary */
> +		skb_reserve(skb, 2);
> +
> +		if (kva->nb_segs == 0) {
The same commnets as above.

>  			memcpy(skb_put(skb, len), data_kva, len);
> -			skb->dev = dev;
> -			skb->ip_summed = CHECKSUM_UNNECESSARY;
> +		} else {
> +			int nb_segs;
> +			int kva_nb_segs = kva->nb_segs;
> 
> -			kni->stats.rx_bytes += len;
> -			kni->stats.rx_packets++;
> +			for (nb_segs = 0; nb_segs < kva_nb_segs; nb_segs++)
The same comments as above.
> {
> +				memcpy(skb_put(skb, kva->data_len),
> +					data_kva, kva->data_len);
> +
> +				if (!kva->next)
> +					break;
> 
> -			/* call tx interface */
> -			kni_net_tx(skb, dev);
> +				kva = kva->next - kni->mbuf_va + kni-
> >mbuf_kva;
> +				data_kva = kva->buf_addr + kva->data_off
> +					- kni->mbuf_va + kni->mbuf_kva;
> +			}
>  		}
> +
> +		skb->dev = dev;
> +		skb->ip_summed = CHECKSUM_UNNECESSARY;
> +
> +		kni->stats.rx_bytes += len;
> +		kni->stats.rx_packets++;
> +
> +		/* call tx interface */
> +		kni_net_tx(skb, dev);
>  	}
> 
>  	/* enqueue all the mbufs from rx_q into free_q */
> --
> 2.5.5
  
Ferruh Yigit April 26, 2016, 8:05 a.m. UTC | #2
On 4/26/2016 7:49 AM, Zhang, Helin wrote:
> Have you tested with it?
Yes, has been tested.

> I think we need to test it in a longer time, e.g. 1 hour
I will make a longevity test before sending next patch.

> My commetns inlined.
> 
> Thanks,
> Helin
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Tuesday, April 26, 2016 12:11 AM
>> To: dev@dpdk.org
>> Cc: Zhang, Helin; Yigit, Ferruh
>> Subject: [PATCH] kni: add chained mbufs support
>>
>> rx_q fifo may have chained mbufs, merge them into single skb before
>> handing to the network stack.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>  .../linuxapp/eal/include/exec-env/rte_kni_common.h |  4 +-
>>  lib/librte_eal/linuxapp/kni/kni_net.c              | 83 ++++++++++++++++------
>>  2 files changed, 64 insertions(+), 23 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
>> b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
>> index 7e5e598..2acdfd9 100644
>> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
>> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
>> @@ -113,7 +113,9 @@ struct rte_kni_mbuf {
>>  	void *buf_addr __attribute__((__aligned__(RTE_CACHE_LINE_SIZE)));
>>  	char pad0[10];
>>  	uint16_t data_off;      /**< Start address of data in segment buffer.
>> */
>> -	char pad1[4];
>> +	char pad1[2];
>> +	uint8_t nb_segs;        /**< Number of segments. */
>> +	char pad4[1];
>>  	uint64_t ol_flags;      /**< Offload features. */
>>  	char pad2[4];
>>  	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len.
>> */
>> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
>> b/lib/librte_eal/linuxapp/kni/kni_net.c
>> index cfa8339..570de71 100644
>> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
>> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
>> @@ -156,7 +156,8 @@ kni_net_rx_normal(struct kni_dev *kni)
>>  	/* Transfer received packets to netif */
>>  	for (i = 0; i < num_rx; i++) {
>>  		kva = (void *)va[i] - kni->mbuf_va + kni->mbuf_kva;
>> -		len = kva->data_len;
>> +		len = kva->pkt_len;
>> +
>>  		data_kva = kva->buf_addr + kva->data_off - kni->mbuf_va
>>  				+ kni->mbuf_kva;
>>
>> @@ -165,22 +166,41 @@ kni_net_rx_normal(struct kni_dev *kni)
>>  			KNI_ERR("Out of mem, dropping pkts\n");
>>  			/* Update statistics */
>>  			kni->stats.rx_dropped++;
>> +			continue;
>>  		}
>> -		else {
>> -			/* Align IP on 16B boundary */
>> -			skb_reserve(skb, 2);
>> +
>> +		/* Align IP on 16B boundary */
>> +		skb_reserve(skb, 2);
>> +
>> +		if (kva->nb_segs == 0) {
> I guess it should compare nb_segs with 1, but not 0. Am I wrong?
> 

Right, this needs to be 1, I will send a new patch.

>>  			memcpy(skb_put(skb, len), data_kva, len);
>> -			skb->dev = dev;
>> -			skb->protocol = eth_type_trans(skb, dev);
>> -			skb->ip_summed = CHECKSUM_UNNECESSARY;
>> +		} else {
>> +			int nb_segs;
>> +			int kva_nb_segs = kva->nb_segs;
>>
>> -			/* Call netif interface */
>> -			netif_rx_ni(skb);
>> +			for (nb_segs = 0; nb_segs < kva_nb_segs; nb_segs++)
> Kva_nb_segs might not needed at all, use kva->nb_segs directly?
> 

It is needed, kva keeps updated, so need to save number of segment for
first mbuf.

>> {
>> +				memcpy(skb_put(skb, kva->data_len),
>> +					data_kva, kva->data_len);
>>
>> -			/* Update statistics */
>> -			kni->stats.rx_bytes += len;
>> -			kni->stats.rx_packets++;
>> +				if (!kva->next)
>> +					break;
>> +
>> +				kva = kva->next - kni->mbuf_va + kni-
>>> mbuf_kva;
>> +				data_kva = kva->buf_addr + kva->data_off
>> +					- kni->mbuf_va + kni->mbuf_kva;
>> +			}
>>  		}
>> +
>> +		skb->dev = dev;
>> +		skb->protocol = eth_type_trans(skb, dev);
>> +		skb->ip_summed = CHECKSUM_UNNECESSARY;
>> +
>> +		/* Call netif interface */
>> +		netif_rx_ni(skb);
>> +
>> +		/* Update statistics */
>> +		kni->stats.rx_bytes += len;
>> +		kni->stats.rx_packets++;
>>  	}
>>
>>  	/* Burst enqueue mbufs into free_q */
>> @@ -317,7 +337,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
>>  	/* Copy mbufs to sk buffer and then call tx interface */
>>  	for (i = 0; i < num; i++) {
>>  		kva = (void *)va[i] - kni->mbuf_va + kni->mbuf_kva;
>> -		len = kva->data_len;
>> +		len = kva->pkt_len;
>>  		data_kva = kva->buf_addr + kva->data_off - kni->mbuf_va +
>>  				kni->mbuf_kva;
>>
>> @@ -338,20 +358,39 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
>>  		if (skb == NULL) {
>>  			KNI_ERR("Out of mem, dropping pkts\n");
>>  			kni->stats.rx_dropped++;
>> +			continue;
>>  		}
>> -		else {
>> -			/* Align IP on 16B boundary */
>> -			skb_reserve(skb, 2);
>> +
>> +		/* Align IP on 16B boundary */
>> +		skb_reserve(skb, 2);
>> +
>> +		if (kva->nb_segs == 0) {
> The same commnets as above.
> 
>>  			memcpy(skb_put(skb, len), data_kva, len);
>> -			skb->dev = dev;
>> -			skb->ip_summed = CHECKSUM_UNNECESSARY;
>> +		} else {
>> +			int nb_segs;
>> +			int kva_nb_segs = kva->nb_segs;
>>
>> -			kni->stats.rx_bytes += len;
>> -			kni->stats.rx_packets++;
>> +			for (nb_segs = 0; nb_segs < kva_nb_segs; nb_segs++)
> The same comments as above.
>> {
>> +				memcpy(skb_put(skb, kva->data_len),
>> +					data_kva, kva->data_len);
>> +
>> +				if (!kva->next)
>> +					break;
>>
>> -			/* call tx interface */
>> -			kni_net_tx(skb, dev);
>> +				kva = kva->next - kni->mbuf_va + kni-
>>> mbuf_kva;
>> +				data_kva = kva->buf_addr + kva->data_off
>> +					- kni->mbuf_va + kni->mbuf_kva;
>> +			}
>>  		}
>> +
>> +		skb->dev = dev;
>> +		skb->ip_summed = CHECKSUM_UNNECESSARY;
>> +
>> +		kni->stats.rx_bytes += len;
>> +		kni->stats.rx_packets++;
>> +
>> +		/* call tx interface */
>> +		kni_net_tx(skb, dev);
>>  	}
>>
>>  	/* enqueue all the mbufs from rx_q into free_q */
>> --
>> 2.5.5
>
  

Patch

diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index 7e5e598..2acdfd9 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -113,7 +113,9 @@  struct rte_kni_mbuf {
 	void *buf_addr __attribute__((__aligned__(RTE_CACHE_LINE_SIZE)));
 	char pad0[10];
 	uint16_t data_off;      /**< Start address of data in segment buffer. */
-	char pad1[4];
+	char pad1[2];
+	uint8_t nb_segs;        /**< Number of segments. */
+	char pad4[1];
 	uint64_t ol_flags;      /**< Offload features. */
 	char pad2[4];
 	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len. */
diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
index cfa8339..570de71 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -156,7 +156,8 @@  kni_net_rx_normal(struct kni_dev *kni)
 	/* Transfer received packets to netif */
 	for (i = 0; i < num_rx; i++) {
 		kva = (void *)va[i] - kni->mbuf_va + kni->mbuf_kva;
-		len = kva->data_len;
+		len = kva->pkt_len;
+
 		data_kva = kva->buf_addr + kva->data_off - kni->mbuf_va
 				+ kni->mbuf_kva;
 
@@ -165,22 +166,41 @@  kni_net_rx_normal(struct kni_dev *kni)
 			KNI_ERR("Out of mem, dropping pkts\n");
 			/* Update statistics */
 			kni->stats.rx_dropped++;
+			continue;
 		}
-		else {
-			/* Align IP on 16B boundary */
-			skb_reserve(skb, 2);
+
+		/* Align IP on 16B boundary */
+		skb_reserve(skb, 2);
+
+		if (kva->nb_segs == 0) {
 			memcpy(skb_put(skb, len), data_kva, len);
-			skb->dev = dev;
-			skb->protocol = eth_type_trans(skb, dev);
-			skb->ip_summed = CHECKSUM_UNNECESSARY;
+		} else {
+			int nb_segs;
+			int kva_nb_segs = kva->nb_segs;
 
-			/* Call netif interface */
-			netif_rx_ni(skb);
+			for (nb_segs = 0; nb_segs < kva_nb_segs; nb_segs++) {
+				memcpy(skb_put(skb, kva->data_len),
+					data_kva, kva->data_len);
 
-			/* Update statistics */
-			kni->stats.rx_bytes += len;
-			kni->stats.rx_packets++;
+				if (!kva->next)
+					break;
+
+				kva = kva->next - kni->mbuf_va + kni->mbuf_kva;
+				data_kva = kva->buf_addr + kva->data_off
+					- kni->mbuf_va + kni->mbuf_kva;
+			}
 		}
+
+		skb->dev = dev;
+		skb->protocol = eth_type_trans(skb, dev);
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+		/* Call netif interface */
+		netif_rx_ni(skb);
+
+		/* Update statistics */
+		kni->stats.rx_bytes += len;
+		kni->stats.rx_packets++;
 	}
 
 	/* Burst enqueue mbufs into free_q */
@@ -317,7 +337,7 @@  kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 	/* Copy mbufs to sk buffer and then call tx interface */
 	for (i = 0; i < num; i++) {
 		kva = (void *)va[i] - kni->mbuf_va + kni->mbuf_kva;
-		len = kva->data_len;
+		len = kva->pkt_len;
 		data_kva = kva->buf_addr + kva->data_off - kni->mbuf_va +
 				kni->mbuf_kva;
 
@@ -338,20 +358,39 @@  kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		if (skb == NULL) {
 			KNI_ERR("Out of mem, dropping pkts\n");
 			kni->stats.rx_dropped++;
+			continue;
 		}
-		else {
-			/* Align IP on 16B boundary */
-			skb_reserve(skb, 2);
+
+		/* Align IP on 16B boundary */
+		skb_reserve(skb, 2);
+
+		if (kva->nb_segs == 0) {
 			memcpy(skb_put(skb, len), data_kva, len);
-			skb->dev = dev;
-			skb->ip_summed = CHECKSUM_UNNECESSARY;
+		} else {
+			int nb_segs;
+			int kva_nb_segs = kva->nb_segs;
 
-			kni->stats.rx_bytes += len;
-			kni->stats.rx_packets++;
+			for (nb_segs = 0; nb_segs < kva_nb_segs; nb_segs++) {
+				memcpy(skb_put(skb, kva->data_len),
+					data_kva, kva->data_len);
+
+				if (!kva->next)
+					break;
 
-			/* call tx interface */
-			kni_net_tx(skb, dev);
+				kva = kva->next - kni->mbuf_va + kni->mbuf_kva;
+				data_kva = kva->buf_addr + kva->data_off
+					- kni->mbuf_va + kni->mbuf_kva;
+			}
 		}
+
+		skb->dev = dev;
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+		kni->stats.rx_bytes += len;
+		kni->stats.rx_packets++;
+
+		/* call tx interface */
+		kni_net_tx(skb, dev);
 	}
 
 	/* enqueue all the mbufs from rx_q into free_q */