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 at 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 at 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
> 

Reply via email to