Re: [dpdk-dev] [PATCH 1/3] mbuf: add sanity checks on segment metadata

[Olivier Matz <olivier.matz@6wind.com>]

Hi David,

On Thu, Sep 13, 2018 at 08:55:40AM +0200, David Marchand wrote:
> Hello Yongseok,
> 
> On Tue, Sep 11, 2018 at 8:16 PM, Yongseok Koh <yskoh@mellanox.com> wrote:
> >
> >> On Sep 9, 2018, at 10:45 PM, David Marchand <david.marchand@6wind.com> wrote:
> >>
> >> Add some basic checks on the segments offset and length metadata:
> >> always funny to have a < 0 tailroom cast to uint16_t ;-).
> >>
> >> Signed-off-by: David Marchand <david.marchand@6wind.com>
> >> ---
> >> lib/librte_mbuf/rte_mbuf.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> >> index e714c5a59..137a320ed 100644
> >> --- a/lib/librte_mbuf/rte_mbuf.c
> >> +++ b/lib/librte_mbuf/rte_mbuf.c
> >> @@ -200,6 +200,11 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
> >>       pkt_len = m->pkt_len;
> >>
> >>       do {
> >> +             if (m->data_off > m->buf_len)
> >> +                     rte_panic("data offset too big in mbuf segment\n");
> >> +             if ((uint32_t)m->data_off + (uint32_t)m->data_len >
> >> +                             (uint32_t)m->buf_len)
> >
> > Casting to uint32_t is needed? All of the three fields are uint16_t and it would
> > anyway happen because of the integer promotion rule. Right?
> 
> Indeed, this is unnecessary.
> Will send a v2 without this.

You can add my ack in your v2 with this change.

Thanks,
Olivier