Re: [dpdk-dev] rte_mbuf library likely()/unlikely()

["Morten Brørup" <mb@smartsharesystems.com>]

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, July 23, 2018 7:38 PM
> To: Morten Brørup
> Cc: Olivier Matz; dev@dpdk.org
> Subject: Re: [dpdk-dev] rte_mbuf library likely()/unlikely()
> 
> On Mon, 23 Jul 2018 15:53:42 +0200
> Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> > Hi Olivier,
> >
> >
> >
> > I noticed that __rte_pktmbuf_read() could do with an unlikely(), so I
> went through the entire library. Here are my suggested modifications.
> >
> >
> >
> >
> >
> > diff -bu rte_mbuf.c.orig rte_mbuf.c
> >
> > --- rte_mbuf.c.orig     2018-07-23 15:13:22.000000000 +0200
> >
> > +++ rte_mbuf.c  2018-07-23 15:32:53.000000000 +0200
> >
> > @@ -173,19 +173,19 @@
> >
> > {
> >
> >         unsigned int nb_segs, pkt_len;
> >
> >
> >
> > -       if (m == NULL)
> >
> > +       if (unlikely(m == NULL))
> >
> >                 rte_panic("mbuf is NULL\n");
> >
> >
> 
> Adding is unlikely is not necessary since rte_panic is marked with cold
> attribute
> which has the same effect.

I was not aware of this. Although it is not visible from the source code files using rte_panic(), it probably means we shouldn't as so much as I thought. Here's an updated patch for rte_mbuf.c, where it is relevant. The other two suggested patches are unaffected.

diff -bu rte_mbuf.c.orig rte_mbuf.c
--- rte_mbuf.c.orig     2018-07-23 15:13:22.000000000 +0200
+++ rte_mbuf.c  2018-07-23 20:52:35.000000000 +0200
@@ -249,7 +249,7 @@
        const struct rte_mbuf *seg = m;
        uint32_t buf_off = 0, copy_len;

-       if (off + len > rte_pktmbuf_pkt_len(m))
+       if (unlikely(off + len > rte_pktmbuf_pkt_len(m)))
                return NULL;

        while (off >= rte_pktmbuf_data_len(seg)) {
@@ -257,7 +257,7 @@
                seg = seg->next;
        }

-       if (off + len <= rte_pktmbuf_data_len(seg))
+       if (likely(off + len <= rte_pktmbuf_data_len(seg)))
                return rte_pktmbuf_mtod_offset(seg, char *, off);

        /* rare case: header is split among several segments */
@@ -344,7 +344,7 @@
        unsigned int i;
        int ret;

-       if (buflen == 0)
+       if (unlikely(buflen == 0))
                return -1;

        buf[0] = '\0';
@@ -355,9 +355,9 @@
                if (name == NULL)
                        name = rx_flags[i].default_name;
                ret = snprintf(buf, buflen, "%s ", name);
-               if (ret < 0)
+               if (unlikely(ret < 0))
                        return -1;
-               if ((size_t)ret >= buflen)
+               if (unlikely((size_t)ret >= buflen))
                        return -1;
                buf += ret;
                buflen -= ret;
@@ -440,7 +440,7 @@
        unsigned int i;
        int ret;

-       if (buflen == 0)
+       if (unlikely(buflen == 0))
                return -1;

        buf[0] = '\0';
@@ -451,9 +451,9 @@
                if (name == NULL)
                        name = tx_flags[i].default_name;
                ret = snprintf(buf, buflen, "%s ", name);
-               if (ret < 0)
+               if (unlikely(ret < 0))
                        return -1;
-               if ((size_t)ret >= buflen)
+               if (unlikely((size_t)ret >= buflen))
                        return -1;
                buf += ret;
                buflen -= ret;


Med venlig hilsen / kind regards
- Morten Brørup