> -----Original Message----- > From: David Marchand <[email protected]> > Sent: Thursday, September 3, 2020 1:11 PM > To: Pai G, Sunil <[email protected]> > Cc: ovs dev <[email protected]>; Ilya Maximets <[email protected]>; > Stokes, Ian <[email protected]> > Subject: Re: [ovs-dev] [PATCH dpdk-latest v1] build: Fix build with Sparse for > rte_mbuf.h. > > Thanks for fixing. > > Afaics in OVS, patches touching sparse headers have a sparse: prefix. > + my personal preference is not to mention actual filenames in a title. > > How about the following title? > sparse: Fix build with DPDK 20.08.
Sure , will address this in the next version. > > On Wed, Sep 2, 2020 at 3:56 PM Sunil Pai G <[email protected]> wrote: > > > > Introduction of C11 atomic instructions in rte_mbuf.h causes the build > > to fail with Sparse reporting following errors. > > > > error: undefined identifier '__atomic_add_fetch' > > error: undefined identifier '__atomic_store_n' > > > > This patch udpates the Sprase headers for rte_mbuf.h. > > typo updates* Sparse*, but I'd rather say that we add a sparse header (there > was > no rte_mbuf.h wrapper so far). > Agreed , shall address this . > > > > > Tested-at: > > https://travis-ci.org/github/Sunil-Pai-G/ovs-copy/builds/723223426 > > Signed-off-by: Sunil Pai G <[email protected]> > > --- > > include/sparse/automake.mk | 1 + > > include/sparse/rte_mbuf.h | 27 +++++++++++++++++++++++++++ > > 2 files changed, 28 insertions(+) > > create mode 100644 include/sparse/rte_mbuf.h > > > > diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk > > index c6ced9387..2c1ef68fc 100644 > > --- a/include/sparse/automake.mk > > +++ b/include/sparse/automake.mk > > @@ -11,6 +11,7 @@ noinst_HEADERS += \ > > include/sparse/netpacket/packet.h \ > > include/sparse/pthread.h \ > > include/sparse/rte_atomic.h \ > > + include/sparse/rte_mbuf.h \ > > Indentation is not the same as the rest of this file. Will address this in next version. > > > > include/sparse/rte_memcpy.h \ > > include/sparse/rte_trace_point.h \ > > include/sparse/sys/socket.h \ diff --git > > a/include/sparse/rte_mbuf.h b/include/sparse/rte_mbuf.h new file mode > > 100644 index 000000000..61b53279f > > --- /dev/null > > +++ b/include/sparse/rte_mbuf.h > > @@ -0,0 +1,27 @@ > > +/* Copyright (c) 2020 Intel, Inc. > > + * > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > + * you may not use this file except in compliance with the License. > > + * You may obtain a copy of the License at: > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, > > +software > > + * distributed under the License is distributed on an "AS IS" BASIS, > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > implied. > > + * See the License for the specific language governing permissions > > +and > > + * limitations under the License. > > + */ > > + > > +#ifndef __CHECKER__ > > +#error "Use this header only with sparse. It is not a correct > > implementation." > > +#endif > > + > > +/* sparse doesn't know about gcc atomic builtins. */ #define > > +__ATOMIC_ACQ_REL 0 #define __ATOMIC_RELAXED 1 #define > > +__atomic_add_fetch(a, b, c) ((*a)=(*a)+b) #define __atomic_store_n(a, > > +b, c) (*a=b) > > a, b, c give no hint at what they refer to. > I'd rather use variable names like p, val and memorder. > Afaics in OVS, there is nothing preventing us from using spaces in macros. > And I may be paranoid but I would protect what is passed through those macros. > > So how about something like: > +#define __atomic_add_fetch(p, val, memorder) (*(p) = *(p) + (val)) > +#define __atomic_store_n(p, val, memorder) (*(p) = (val)) Agreed. Looks better. Will address this. > > > > + > > +/* Get actual <rte_mbuf.h> definitions for us to annotate and build > > +on. */ #include_next <rte_mbuf.h> > > > -- > David Marchand Thanks and regards, Sunil _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
