Re: [dpdk-dev] [PATCH 2/2] mbuf: reset nb_segs of chained packet
Hi Ilya, On Thu, Nov 16, 2017 at 11:15:18PM +0400, Ilya Matveychikov wrote: > > > On Nov 16, 2017, at 9:01 PM, Stephen Hemminger > > wrote: > > > > On Thu, 16 Nov 2017 18:05:35 +0400 > > Ilya Matveychikov wrote: > > > >> Fixes: 139debc42dc0 ("mbuf: move chaining from ip_frag library") > >> Cc: simon.kagst...@netinsight.net > >> > >> Signed-off-by: Ilya V. Matveychikov > >> --- > >> lib/librte_mbuf/rte_mbuf.h | 5 - > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > >> index ce8a05ddf..2126dc94b 100644 > >> --- a/lib/librte_mbuf/rte_mbuf.h > >> +++ b/lib/librte_mbuf/rte_mbuf.h > >> @@ -1828,9 +1828,12 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf > >> *head, struct rte_mbuf *tail > >>head->nb_segs += tail->nb_segs; > >>head->pkt_len += tail->pkt_len; > >> > >> - /* pkt_len is only set in the head */ > >> + /* nb_segs and pkt_len are only set in the head */ > >> + tail->nb_segs = 1; > >>tail->pkt_len = tail->data_len; > >> > >> + __rte_mbuf_sanity_check(head, 1); > >> + > >>return 0; > >> } > > > > My understanding is that nb_segs and pkt_len are only valid > > in head. For other packets in the chain nb_segs and pkt_len > > can be anything. > > So why not to keep them in consistency with multi-seg logic? > I mean that pkt_len/nb_segs for the head always have meaning but for > the rest of chain pkt_len is the same as data_len and nb_segs := 1 > Stephen is right: like most mbuf fields, nb_segs and pkt_len are only valid for the first mbuf of the chain. What would be the advantage of changing this? In addition, I think it would require to do the same change that in many places, like drivers that build multi-seg mbufs. If you are fixing an issue, please describe it in the commit log. Olivier
Re: [dpdk-dev] [PATCH 2/2] mbuf: reset nb_segs of chained packet
> On Nov 16, 2017, at 9:01 PM, Stephen Hemminger > wrote: > > On Thu, 16 Nov 2017 18:05:35 +0400 > Ilya Matveychikov wrote: > >> Fixes: 139debc42dc0 ("mbuf: move chaining from ip_frag library") >> Cc: simon.kagst...@netinsight.net >> >> Signed-off-by: Ilya V. Matveychikov >> --- >> lib/librte_mbuf/rte_mbuf.h | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h >> index ce8a05ddf..2126dc94b 100644 >> --- a/lib/librte_mbuf/rte_mbuf.h >> +++ b/lib/librte_mbuf/rte_mbuf.h >> @@ -1828,9 +1828,12 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf >> *head, struct rte_mbuf *tail >> head->nb_segs += tail->nb_segs; >> head->pkt_len += tail->pkt_len; >> >> -/* pkt_len is only set in the head */ >> +/* nb_segs and pkt_len are only set in the head */ >> +tail->nb_segs = 1; >> tail->pkt_len = tail->data_len; >> >> +__rte_mbuf_sanity_check(head, 1); >> + >> return 0; >> } > > My understanding is that nb_segs and pkt_len are only valid > in head. For other packets in the chain nb_segs and pkt_len > can be anything. So why not to keep them in consistency with multi-seg logic? I mean that pkt_len/nb_segs for the head always have meaning but for the rest of chain pkt_len is the same as data_len and nb_segs := 1
Re: [dpdk-dev] [PATCH 2/2] mbuf: reset nb_segs of chained packet
On Thu, 16 Nov 2017 18:05:35 +0400 Ilya Matveychikov wrote: > Fixes: 139debc42dc0 ("mbuf: move chaining from ip_frag library") > Cc: simon.kagst...@netinsight.net > > Signed-off-by: Ilya V. Matveychikov > --- > lib/librte_mbuf/rte_mbuf.h | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index ce8a05ddf..2126dc94b 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -1828,9 +1828,12 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf > *head, struct rte_mbuf *tail > head->nb_segs += tail->nb_segs; > head->pkt_len += tail->pkt_len; > > - /* pkt_len is only set in the head */ > + /* nb_segs and pkt_len are only set in the head */ > + tail->nb_segs = 1; > tail->pkt_len = tail->data_len; > > + __rte_mbuf_sanity_check(head, 1); > + > return 0; > } My understanding is that nb_segs and pkt_len are only valid in head. For other packets in the chain nb_segs and pkt_len can be anything.