Re: [dpdk-dev] [PATCH v3] mbuf/ip_frag: Move mbuf chaining to common code

[Simon Kagstrom <simon.kagstrom@netinsight.net>]

Ping?

// Simon

On Mon, 7 Sep 2015 14:50:09 +0200
Simon Kagstrom <simon.kagstrom@netinsight.net> wrote:

> Chaining/segmenting mbufs can be useful in many places, so make it
> global.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> Signed-off-by: Johan Faltstrom <johan.faltstrom@netinsight.net>
> ---
> ChangeLog:
> v2:
>   * Check for nb_segs byte overflow (Olivier MATZ)
>   * Don't reset nb_segs in tail (Olivier MATZ)
> v3:
>   * Describe performance implications of linear search
>   * Correct check-for-out-of-bounds (Konstantin Ananyev)
> 
>  lib/librte_ip_frag/ip_frag_common.h      | 23 ---------------------
>  lib/librte_ip_frag/rte_ipv4_reassembly.c |  7 +++++--
>  lib/librte_ip_frag/rte_ipv6_reassembly.c |  7 +++++--
>  lib/librte_mbuf/rte_mbuf.h               | 34 ++++++++++++++++++++++++++++++++
>  4 files changed, 44 insertions(+), 27 deletions(-)
> 
> diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_frag_common.h
> index 6b2acee..cde6ed4 100644
> --- a/lib/librte_ip_frag/ip_frag_common.h
> +++ b/lib/librte_ip_frag/ip_frag_common.h
> @@ -166,27 +166,4 @@ ip_frag_reset(struct ip_frag_pkt *fp, uint64_t tms)
>  	fp->frags[IP_FIRST_FRAG_IDX] = zero_frag;
>  }
>  
> -/* chain two mbufs */
> -static inline void
> -ip_frag_chain(struct rte_mbuf *mn, struct rte_mbuf *mp)
> -{
> -	struct rte_mbuf *ms;
> -
> -	/* adjust start of the last fragment data. */
> -	rte_pktmbuf_adj(mp, (uint16_t)(mp->l2_len + mp->l3_len));
> -
> -	/* chain two fragments. */
> -	ms = rte_pktmbuf_lastseg(mn);
> -	ms->next = mp;
> -
> -	/* accumulate number of segments and total length. */
> -	mn->nb_segs = (uint8_t)(mn->nb_segs + mp->nb_segs);
> -	mn->pkt_len += mp->pkt_len;
> -
> -	/* reset pkt_len and nb_segs for chained fragment. */
> -	mp->pkt_len = mp->data_len;
> -	mp->nb_segs = 1;
> -}
> -
> -
>  #endif /* _IP_FRAG_COMMON_H_ */
> diff --git a/lib/librte_ip_frag/rte_ipv4_reassembly.c b/lib/librte_ip_frag/rte_ipv4_reassembly.c
> index 5d24843..26d07f9 100644
> --- a/lib/librte_ip_frag/rte_ipv4_reassembly.c
> +++ b/lib/librte_ip_frag/rte_ipv4_reassembly.c
> @@ -63,7 +63,9 @@ ipv4_frag_reassemble(const struct ip_frag_pkt *fp)
>  			/* previous fragment found. */
>  			if(fp->frags[i].ofs + fp->frags[i].len == ofs) {
>  
> -				ip_frag_chain(fp->frags[i].mb, m);
> +				/* adjust start of the last fragment data. */
> +				rte_pktmbuf_adj(m, (uint16_t)(m->l2_len + m->l3_len));
> +				rte_pktmbuf_chain(fp->frags[i].mb, m);
>  
>  				/* update our last fragment and offset. */
>  				m = fp->frags[i].mb;
> @@ -78,7 +80,8 @@ ipv4_frag_reassemble(const struct ip_frag_pkt *fp)
>  	}
>  
>  	/* chain with the first fragment. */
> -	ip_frag_chain(fp->frags[IP_FIRST_FRAG_IDX].mb, m);
> +	rte_pktmbuf_adj(m, (uint16_t)(m->l2_len + m->l3_len));
> +	rte_pktmbuf_chain(fp->frags[IP_FIRST_FRAG_IDX].mb, m);
>  	m = fp->frags[IP_FIRST_FRAG_IDX].mb;
>  
>  	/* update mbuf fields for reassembled packet. */
> diff --git a/lib/librte_ip_frag/rte_ipv6_reassembly.c b/lib/librte_ip_frag/rte_ipv6_reassembly.c
> index 1f1c172..5969b4a 100644
> --- a/lib/librte_ip_frag/rte_ipv6_reassembly.c
> +++ b/lib/librte_ip_frag/rte_ipv6_reassembly.c
> @@ -86,7 +86,9 @@ ipv6_frag_reassemble(const struct ip_frag_pkt *fp)
>  			/* previous fragment found. */
>  			if (fp->frags[i].ofs + fp->frags[i].len == ofs) {
>  
> -				ip_frag_chain(fp->frags[i].mb, m);
> +				/* adjust start of the last fragment data. */
> +				rte_pktmbuf_adj(m, (uint16_t)(m->l2_len + m->l3_len));
> +				rte_pktmbuf_chain(fp->frags[i].mb, m);
>  
>  				/* update our last fragment and offset. */
>  				m = fp->frags[i].mb;
> @@ -101,7 +103,8 @@ ipv6_frag_reassemble(const struct ip_frag_pkt *fp)
>  	}
>  
>  	/* chain with the first fragment. */
> -	ip_frag_chain(fp->frags[IP_FIRST_FRAG_IDX].mb, m);
> +	rte_pktmbuf_adj(m, (uint16_t)(m->l2_len + m->l3_len));
> +	rte_pktmbuf_chain(fp->frags[IP_FIRST_FRAG_IDX].mb, m);
>  	m = fp->frags[IP_FIRST_FRAG_IDX].mb;
>  
>  	/* update mbuf fields for reassembled packet. */
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index d7c9030..f1f1400 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1775,6 +1775,40 @@ static inline int rte_pktmbuf_is_contiguous(const struct rte_mbuf *m)
>  }
>  
>  /**
> + * Chain an mbuf to another, thereby creating a segmented packet.
> + *
> + * Note: The implementation will do a linear walk over the segments to find
> + * the tail entry. For cases when there are many segments, it's better to
> + * chain the entries manually.
> + *
> + * @param head the head of the mbuf chain (the first packet)
> + * @param tail the mbuf to put last in the chain
> + *
> + * @return 0 on success, -EOVERFLOW if the chain is full (256 entries)
> + */
> +static inline int rte_pktmbuf_chain(struct rte_mbuf *head, struct rte_mbuf *tail)
> +{
> +	struct rte_mbuf *cur_tail;
> +
> +	/* Check for number-of-segments-overflow */
> +	if (head->nb_segs + tail->nb_segs >= 1 << (sizeof(head->nb_segs) * 8))
> +		return -EOVERFLOW;
> +
> +	/* Chain 'tail' onto the old tail */
> +	cur_tail = rte_pktmbuf_lastseg(head);
> +	cur_tail->next = tail;
> +
> +	/* accumulate number of segments and total length. */
> +	head->nb_segs = (uint8_t)(head->nb_segs + tail->nb_segs);
> +	head->pkt_len += tail->pkt_len;
> +
> +	/* pkt_len is only set in the head */
> +	tail->pkt_len = tail->data_len;
> +
> +	return 0;
> +}
> +
> +/**
>   * Dump an mbuf structure to the console.
>   *
>   * Dump all fields for the given packet mbuf and all its associated