[v2,1/4] ring: future proof flag settings
Checks
Commit Message
All API's should check that they support the flag values passed.
These checks ensure that the extra bits can safely be used
without risk of ABI breakage.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/librte_ring/rte_ring.c | 10 ++++++++++
1 file changed, 10 insertions(+)
Comments
<snip>
>
> All API's should check that they support the flag values passed.
> These checks ensure that the extra bits can safely be used without risk of ABI
> breakage.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/librte_ring/rte_ring.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index
> ebe5ccf0de68..70685121581f 100644
> --- a/lib/librte_ring/rte_ring.c
> +++ b/lib/librte_ring/rte_ring.c
> @@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = { };
> EAL_REGISTER_TAILQ(rte_ring_tailq)
>
> +/* mask of all valid flag values to ring_create() */
> +#define RING_F_MASK 0x007F
Is it better to construct this using the actual flag #defines?
> +
> /* true if x is a power of 2 */
> #define POWEROF2(x) ((((x)-1) & (x)) == 0)
>
> @@ -197,6 +200,13 @@ rte_ring_init(struct rte_ring *r, const char *name,
> unsigned count,
> RTE_BUILD_BUG_ON(offsetof(struct rte_ring_headtail, tail) !=
> offsetof(struct rte_ring_rts_headtail, tail.val.pos));
>
> + /* future proof flags, only allow supported values */
> + if (flags & ~RING_F_MASK) {
> + RTE_LOG(ERR, RING,
> + "Unsupported flags requested %#x\n", flags);
> + return -EINVAL;
> + }
> +
> /* init the ring structure */
> memset(r, 0, sizeof(*r));
> ret = strlcpy(r->name, name, sizeof(r->name));
> --
> 2.20.1
On Fri, 24 Apr 2020 18:07:15 +0000
Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> <snip>
>
> >
> > All API's should check that they support the flag values passed.
> > These checks ensure that the extra bits can safely be used without risk of ABI
> > breakage.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > lib/librte_ring/rte_ring.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index
> > ebe5ccf0de68..70685121581f 100644
> > --- a/lib/librte_ring/rte_ring.c
> > +++ b/lib/librte_ring/rte_ring.c
> > @@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = { };
> > EAL_REGISTER_TAILQ(rte_ring_tailq)
> >
> > +/* mask of all valid flag values to ring_create() */
> > +#define RING_F_MASK 0x007F
> Is it better to construct this using the actual flag #defines?
sure, but it gets long
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> Sent: Friday, April 24, 2020 8:02 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings
>
> On Fri, 24 Apr 2020 18:07:15 +0000
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
>
> > <snip>
> >
> > >
> > > All API's should check that they support the flag values passed.
> > > These checks ensure that the extra bits can safely be used without risk of ABI
> > > breakage.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > > lib/librte_ring/rte_ring.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index
> > > ebe5ccf0de68..70685121581f 100644
> > > --- a/lib/librte_ring/rte_ring.c
> > > +++ b/lib/librte_ring/rte_ring.c
> > > @@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = { };
> > > EAL_REGISTER_TAILQ(rte_ring_tailq)
> > >
> > > +/* mask of all valid flag values to ring_create() */
> > > +#define RING_F_MASK 0x007F
> > Is it better to construct this using the actual flag #defines?
>
> sure, but it gets long
+1 to use public defines here.
@@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = {
};
EAL_REGISTER_TAILQ(rte_ring_tailq)
+/* mask of all valid flag values to ring_create() */
+#define RING_F_MASK 0x007F
+
/* true if x is a power of 2 */
#define POWEROF2(x) ((((x)-1) & (x)) == 0)
@@ -197,6 +200,13 @@ rte_ring_init(struct rte_ring *r, const char *name, unsigned count,
RTE_BUILD_BUG_ON(offsetof(struct rte_ring_headtail, tail) !=
offsetof(struct rte_ring_rts_headtail, tail.val.pos));
+ /* future proof flags, only allow supported values */
+ if (flags & ~RING_F_MASK) {
+ RTE_LOG(ERR, RING,
+ "Unsupported flags requested %#x\n", flags);
+ return -EINVAL;
+ }
+
/* init the ring structure */
memset(r, 0, sizeof(*r));
ret = strlcpy(r->name, name, sizeof(r->name));