| Message ID | 20190423234232.21799-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Commit | 0cdd5b13de91b989dc92246e20ee6d528417df97 |
| Headers | show |
| Series | [ovs-dev] sparse: Configure target operating system and fix fallout. | expand |
On 24.04.2019 2:42, Ben Pfaff wrote: > cgcc, the "sparse" wrapper that OVS uses, can be told the host architecture > or the host OS or both. Until now, OVS has told it the host architecture > because it is fairly common that it doesn't guess it automatically. Until > now, OS has not told it the host OS, assuming that it would get it right. > However, it doesn't--if you tell it the host OS or the host architecture, > it doesn't really have a default for the other. This means that on Linux > (presumably the only OS where sparse works properly for OVS), it was not > defining __linux__, which caused some weird behavior. > > This commit adds a flag to the cgcc invocation to make it define __linux__ > on Linux, and it fixes some errors that this would otherwise cause. > > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- LGTM, Acked-by: Ilya Maximets <i.maximets@samsung.com> One minor comment inline. > acinclude.m4 | 6 +++--- > include/sparse/automake.mk | 1 + > include/sparse/linux/if_packet.h | 30 ++++++++++++++++++++++++++++++ > lib/perf-counter.c | 4 ++-- > 4 files changed, 36 insertions(+), 5 deletions(-) > create mode 100644 include/sparse/linux/if_packet.h > > diff --git a/acinclude.m4 b/acinclude.m4 > index 1607d5f4b1d9..301aeb70d82a 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -1,6 +1,6 @@ > # -*- autoconf -*- > > -# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc. > +# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2019 Nicira, Inc. > # > # Licensed under the Apache License, Version 2.0 (the "License"); > # you may not use this file except in compliance with the License. > @@ -1107,8 +1107,8 @@ AC_DEFUN([OVS_CHECK_SPARSE_TARGET], > [x86_64-*], [ac_cv_sparse_target=x86_64], > [ac_cv_sparse_target=other])]) > AS_CASE([$ac_cv_sparse_target], > - [x86], [SPARSEFLAGS= CGCCFLAGS=-target=i86], > - [x86_64], [SPARSEFLAGS=-m64 CGCCFLAGS=-target=x86_64], > + [x86], [SPARSEFLAGS= CGCCFLAGS="-target=i86 -target=host_os_specs"], > + [x86_64], [SPARSEFLAGS=-m64 CGCCFLAGS="-target=x86_64 -target=host_os_specs"], > [SPARSEFLAGS= CGCCFLAGS=]) > AC_SUBST([SPARSEFLAGS]) > AC_SUBST([CGCCFLAGS])]) > diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk > index 4c7b17783fd5..8d96d0346f48 100644 > --- a/include/sparse/automake.mk > +++ b/include/sparse/automake.mk > @@ -27,4 +27,5 @@ noinst_HEADERS += \ > include/sparse/sys/types.h \ > include/sparse/sys/wait.h \ > include/sparse/threads.h \ > + include/sparse/linux/if_packet.h \ > include/sparse/linux/tc_act/tc_pedit.h > diff --git a/include/sparse/linux/if_packet.h b/include/sparse/linux/if_packet.h > new file mode 100644 > index 000000000000..eab1be9d247d > --- /dev/null > +++ b/include/sparse/linux/if_packet.h > @@ -0,0 +1,30 @@ > +#ifndef FIX_LINUX_IF_PACKET_H > +#define FIX_LINUX_IF_PACKET_H > + > +#ifndef __CHECKER__ > +#error "Use this header only with sparse. It is not a correct implementation." > +#endif > + > +#include_next <linux/if_packet.h> > + > +/* Fix endianness of 'spkt_protocol' and 'sll_protocol' members. */ > + > +#define sockaddr_pkt rpl_sockaddr_pkt > +struct sockaddr_pkt { > + unsigned short spkt_family; > + unsigned char spkt_device[14]; > + ovs_be16 spkt_protocol; > +}; > + > +#define sockaddr_ll rpl_sockaddr_ll > +struct sockaddr_ll { > + unsigned short sll_family; > + ovs_be16 sll_protocol; It'll be better to align the name with other structure fields. > + int sll_ifindex; > + unsigned short sll_hatype; > + unsigned char sll_pkttype; > + unsigned char sll_halen; > + unsigned char sll_addr[8]; > +}; > + > +#endif > diff --git a/lib/perf-counter.c b/lib/perf-counter.c > index c4458d2f5fa9..402fabe1775b 100644 > --- a/lib/perf-counter.c > +++ b/lib/perf-counter.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2015, 2016 Nicira, Inc. > + * Copyright (c) 2015, 2016, 2019 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -17,7 +17,7 @@ > /* This implementation only applies to the Linux platform. */ > > #include <config.h> > -#if defined(__linux__) && defined(HAVE_LINUX_PERF_EVENT_H) > +#if defined(__linux__) && defined(HAVE_LINUX_PERF_EVENT_H) && !__CHECKER__ > > #include <stddef.h> > #include <sys/types.h> >
On Wed, Apr 24, 2019 at 03:40:21PM +0300, Ilya Maximets wrote: > On 24.04.2019 2:42, Ben Pfaff wrote: > > cgcc, the "sparse" wrapper that OVS uses, can be told the host architecture > > or the host OS or both. Until now, OVS has told it the host architecture > > because it is fairly common that it doesn't guess it automatically. Until > > now, OS has not told it the host OS, assuming that it would get it right. > > However, it doesn't--if you tell it the host OS or the host architecture, > > it doesn't really have a default for the other. This means that on Linux > > (presumably the only OS where sparse works properly for OVS), it was not > > defining __linux__, which caused some weird behavior. > > > > This commit adds a flag to the cgcc invocation to make it define __linux__ > > on Linux, and it fixes some errors that this would otherwise cause. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > LGTM, > Acked-by: Ilya Maximets <i.maximets@samsung.com> > > One minor comment inline. Thanks, I fixed that and applied this to master.
diff --git a/acinclude.m4 b/acinclude.m4 index 1607d5f4b1d9..301aeb70d82a 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -1,6 +1,6 @@ # -*- autoconf -*- -# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc. +# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2019 Nicira, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -1107,8 +1107,8 @@ AC_DEFUN([OVS_CHECK_SPARSE_TARGET], [x86_64-*], [ac_cv_sparse_target=x86_64], [ac_cv_sparse_target=other])]) AS_CASE([$ac_cv_sparse_target], - [x86], [SPARSEFLAGS= CGCCFLAGS=-target=i86], - [x86_64], [SPARSEFLAGS=-m64 CGCCFLAGS=-target=x86_64], + [x86], [SPARSEFLAGS= CGCCFLAGS="-target=i86 -target=host_os_specs"], + [x86_64], [SPARSEFLAGS=-m64 CGCCFLAGS="-target=x86_64 -target=host_os_specs"], [SPARSEFLAGS= CGCCFLAGS=]) AC_SUBST([SPARSEFLAGS]) AC_SUBST([CGCCFLAGS])]) diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk index 4c7b17783fd5..8d96d0346f48 100644 --- a/include/sparse/automake.mk +++ b/include/sparse/automake.mk @@ -27,4 +27,5 @@ noinst_HEADERS += \ include/sparse/sys/types.h \ include/sparse/sys/wait.h \ include/sparse/threads.h \ + include/sparse/linux/if_packet.h \ include/sparse/linux/tc_act/tc_pedit.h diff --git a/include/sparse/linux/if_packet.h b/include/sparse/linux/if_packet.h new file mode 100644 index 000000000000..eab1be9d247d --- /dev/null +++ b/include/sparse/linux/if_packet.h @@ -0,0 +1,30 @@ +#ifndef FIX_LINUX_IF_PACKET_H +#define FIX_LINUX_IF_PACKET_H + +#ifndef __CHECKER__ +#error "Use this header only with sparse. It is not a correct implementation." +#endif + +#include_next <linux/if_packet.h> + +/* Fix endianness of 'spkt_protocol' and 'sll_protocol' members. */ + +#define sockaddr_pkt rpl_sockaddr_pkt +struct sockaddr_pkt { + unsigned short spkt_family; + unsigned char spkt_device[14]; + ovs_be16 spkt_protocol; +}; + +#define sockaddr_ll rpl_sockaddr_ll +struct sockaddr_ll { + unsigned short sll_family; + ovs_be16 sll_protocol; + int sll_ifindex; + unsigned short sll_hatype; + unsigned char sll_pkttype; + unsigned char sll_halen; + unsigned char sll_addr[8]; +}; + +#endif diff --git a/lib/perf-counter.c b/lib/perf-counter.c index c4458d2f5fa9..402fabe1775b 100644 --- a/lib/perf-counter.c +++ b/lib/perf-counter.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2016 Nicira, Inc. + * Copyright (c) 2015, 2016, 2019 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,7 @@ /* This implementation only applies to the Linux platform. */ #include <config.h> -#if defined(__linux__) && defined(HAVE_LINUX_PERF_EVENT_H) +#if defined(__linux__) && defined(HAVE_LINUX_PERF_EVENT_H) && !__CHECKER__ #include <stddef.h> #include <sys/types.h>
cgcc, the "sparse" wrapper that OVS uses, can be told the host architecture or the host OS or both. Until now, OVS has told it the host architecture because it is fairly common that it doesn't guess it automatically. Until now, OS has not told it the host OS, assuming that it would get it right. However, it doesn't--if you tell it the host OS or the host architecture, it doesn't really have a default for the other. This means that on Linux (presumably the only OS where sparse works properly for OVS), it was not defining __linux__, which caused some weird behavior. This commit adds a flag to the cgcc invocation to make it define __linux__ on Linux, and it fixes some errors that this would otherwise cause. Signed-off-by: Ben Pfaff <blp@ovn.org> --- acinclude.m4 | 6 +++--- include/sparse/automake.mk | 1 + include/sparse/linux/if_packet.h | 30 ++++++++++++++++++++++++++++++ lib/perf-counter.c | 4 ++-- 4 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 include/sparse/linux/if_packet.h