All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Olivier Matz" <olivier.matz@6wind.com>
Cc: <dev@dpdk.org>
Subject: [dpdk-dev] The mbuf API needs some cleaning up
Date: Mon, 13 Jul 2020 11:57:38 +0200	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35C6111E@smartserver.smartshare.dk> (raw)

The MBUF library exposes some macros and constants without the RTE_ prefix. I propose cleaning up these, so better names get into the coming LTS release.

The worst is:
#define MBUF_INVALID_PORT UINT16_MAX

I say it's the worst because when we were looking for the official "invalid" port value for our application, we didn't find this one. (Probably because its documentation is wrong.)

MBUF_INVALID_PORT is defined in rte_mbuf_core.h without any description, and in rte_mbuf.h, where it is injected between the rte_pktmbuf_reset() function and its description, so the API documentation shows the function's description for the constant, and no description for the function.

I propose keeping it at a sensible location in rte_mbuf_core.h only, adding a description, and renaming it to:
#define RTE_PORT_INVALID UINT16_MAX

For backwards compatibility, we could add:
/* this old name is deprecated */
#define MBUF_INVALID_PORT RTE_PORT_INVALID

I also wonder why there are no compiler warnings about the double definition?


There are also the data buffer location constants:
#define EXT_ATTACHED_MBUF    (1ULL << 61)
and
#define IND_ATTACHED_MBUF    (1ULL << 62)

There are already macros (with good names) for reading these, so simply adding the RTE_ prefix to these two constants suffices.


And all the packet offload flags, such as:
#define PKT_RX_VLAN          (1ULL << 0)

They are supposed to be used by applications, so I guess we should keep them unchanged for ABI stability reasons.


And the local macro:
#define MBUF_RAW_ALLOC_CHECK(m) do { \

This might as well be an internal inline function:
/* internal */
static inline void
__rte_mbuf_raw_alloc_check(const struct rte_mbuf *m)

Or we could keep it a macro and move it next to
__rte_mbuf_sanity_check(), keeping it clear that it is only relevant when RTE_LIBRTE_MBUF_DEBUG is set. But rename it to lower case, similar to the __rte_mbuf_sanity_check() macro.


Med venlig hilsen / kind regards
- Morten Brørup


             reply	other threads:[~2020-07-13  9:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13  9:57 Morten Brørup [this message]
2020-07-31 15:24 ` [dpdk-dev] The mbuf API needs some cleaning up Olivier Matz
2020-08-03  8:42   ` Morten Brørup
2020-08-03 10:41     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=98CBD80474FA8B44BF855DF32C47DC35C6111E@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.