Message ID | 20190306010117.GE25780@ovn.org |
---|---|
State | Rejected |
Headers | show |
Series | [ovs-dev] MUSL netpacket/packet.h | expand |
Bleep bloop. Greetings Ben Pfaff, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: Failed to merge in the changes. Patch failed at 0001 MUSL netpacket/packet.h The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
On Tue, Mar 05, 2019 at 08:57:22PM -0500, 0-day Robot wrote: > Bleep bloop. Greetings Ben Pfaff, I am a robot and I have tried out your patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > > git-am: > Failed to merge in the changes. > Patch failed at 0001 MUSL netpacket/packet.h > The copy of the patch that failed is found in: > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch Roboto-san: Ha ha ha! Have a nice day.
Hello, The musl-if_packet.patch will always be needed by Alpine due to musl libc / glibc incompatibilities. See: https://wiki.musl-libc.org/faq.html#Q:-Why-am-I-getting- Kind Regards, Stuart Cardall. On 2019-03-06 02:01, Ben Pfaff wrote: > Hmm, with the patch I suspect that compilation fails against some > systems with glibc, based on e.g. the appended commit. Probably there > is some way to resolve it with proper autoconf tests, but I don't know > what test is appropriate in this case. I tried building with > "musl-gcc" > on Debian, but that had a lot of problems other than this one. So > unless someone invests some time in solving the problem, maybe your > patch is an appropriate stopgap. > > On Wed, Mar 06, 2019 at 12:30:24AM +0000, Fernando Casas Schössow > wrote: >> I tried to build the package and it fails without the patch so I guess >> it is still needed to build on Alpine (I'm adding Stuart Cardall who >> is the package maintainer in case he can comment on this). >> >> Probably is only needed in Alpine, because of musl libc. >> >> On mié, mar 6, 2019 at 1:17 AM, Ben Pfaff <blp@ovn.org> wrote: >> On Wed, Mar 06, 2019 at 12:13:32AM +0000, Fernando Casas Schössow >> wrote: >> And besides the ifupdown scripts the only patch it applies seems to be >> required by musl libc: musl-if_packet.patch --- >> openvswitch-2.4.0/lib/netdev-linux.c 2015-08-20 00:33:42.960971996 >> +0000 +++ openvswitch-2.4.0/lib/netdev-linux.c.new 2015-08-22 >> 18:16:10.741115156 +0000 @@ -37,10 +37,9 @@ #include <sys/ioctl.h> >> #include <sys/socket.h> #include <sys/utsname.h> -#include >> <netpacket/packet.h> #include <net/if.h> #include <net/if_arp.h> >> -#include <net/if_packet.h> +#include <linux/if_packet.h> #include >> <net/route.h> #include <netinet/in.h> #include <poll.h> >> I'm going to assume that the 2.4.0 version number in the patch just >> means that it was originally applied to that version and later >> forward-ported. If this patch is still needed on master, let me >> know--we'll get it applied upstream. >> >> > > commit 55bc98d6cb340626eab68fa91eeffafe5e5a2339 > Author: Ben Pfaff <blp@nicira.com> > Date: Thu Jan 16 17:21:55 2014 -0800 > > netdev-linux: Fix build break on RHEL 6.1. > > Commit 73c85181d (netdev-linux: Read packet auxdata to obtain > vlan_tid) > added #include <linux/if_packet.h> to this file, to get the > definition > of PACKET_AUXDATA and some other definitions, but on RHEL 6.1 this > provoked compiler errors: > > In file included from /usr/include/linux/rtnetlink.h:5, > from lib/netdev-linux.c:34: > /usr/include/linux/netlink.h:34: error: expected > specifier-qualifier-list > before 'sa_family_t' > > Since the old #includes worked everywhere, and this file already > defined > its own versions of most of the new macros that it needed, this > commit just > reverts the old #includes and adds the one macro definition it > didn't > already have. > > (RHEL 6.1 isn't necessarily the only platform where this is a > problem, but > it's the first one for which we noticed the problem.) > > This switches the definition of sockaddr_ll used from the Linux > one, which > uses __be16 for sll_protocol, to the glibc one, which uses plain > "unsigned > short int". This makes sparse complain (rightly), so this commit > also > adds a sparse-specific header that uses ovs_be16 to prevent the > warning. > > Signed-off-by: Ben Pfaff <blp@nicira.com> > > diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk > index 45ae1f506062..572c7c2c737e 100644 > --- a/include/sparse/automake.mk > +++ b/include/sparse/automake.mk > @@ -4,6 +4,7 @@ noinst_HEADERS += \ > include/sparse/math.h \ > include/sparse/netinet/in.h \ > include/sparse/netinet/ip6.h \ > + include/sparse/netpacket/packet.h \ > include/sparse/pthread.h \ > include/sparse/sys/socket.h \ > include/sparse/sys/wait.h > diff --git a/include/sparse/netpacket/packet.h > b/include/sparse/netpacket/packet.h > new file mode 100644 > index 000000000000..21bdd2ea7087 > --- /dev/null > +++ b/include/sparse/netpacket/packet.h > @@ -0,0 +1,37 @@ > +/* > + * Copyright (c) 2014 Nicira, 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 > + > +#ifndef __NETPACKET_PACKET_SPARSE > +#define __NETPACKET_PACKET_SPARSE 1 > + > +#include "openvswitch/types.h" > + > +struct sockaddr_ll > + { > + unsigned short int sll_family; > + ovs_be16 sll_protocol; > + int sll_ifindex; > + unsigned short int sll_hatype; > + unsigned char sll_pkttype; > + unsigned char sll_halen; > + unsigned char sll_addr[8]; > + }; > + > +#endif /* <netpacket/packet.h> sparse */ > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 9c1a36db181e..e756d88a9458 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc. > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -20,11 +20,11 @@ > > #include <errno.h> > #include <fcntl.h> > +#include <arpa/inet.h> > #include <inttypes.h> > #include <linux/filter.h> > #include <linux/gen_stats.h> > #include <linux/if_ether.h> > -#include <linux/if_packet.h> > #include <linux/if_tun.h> > #include <linux/types.h> > #include <linux/ethtool.h> > @@ -37,8 +37,10 @@ > #include <sys/types.h> > #include <sys/ioctl.h> > #include <sys/socket.h> > +#include <netpacket/packet.h> > #include <net/if.h> > #include <net/if_arp.h> > +#include <net/if_packet.h> > #include <net/route.h> > #include <netinet/in.h> > #include <poll.h> > @@ -116,6 +118,9 @@ COVERAGE_DEFINE(netdev_set_ethtool); > * With all this churn it's easiest to unconditionally define a > replacement > * structure that has everything we want. > */ > +#ifndef PACKET_AUXDATA > +#define PACKET_AUXDATA 8 > +#endif > #ifndef TP_STATUS_VLAN_VALID > #define TP_STATUS_VLAN_VALID (1 << 4) > #endif
Thanks Stuart for confirming this. On mié, mar 6, 2019 at 10:30 AM, developer@it-offshore.co.uk wrote: Hello, The musl-if_packet.patch will always be needed by Alpine due to musl libc / glibc incompatibilities. See: https://wiki.musl-libc.org/faq.html#Q:-Why-am-I-getting- Kind Regards, Stuart Cardall.
diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk index 45ae1f506062..572c7c2c737e 100644 --- a/include/sparse/automake.mk +++ b/include/sparse/automake.mk @@ -4,6 +4,7 @@ noinst_HEADERS += \ include/sparse/math.h \ include/sparse/netinet/in.h \ include/sparse/netinet/ip6.h \ + include/sparse/netpacket/packet.h \ include/sparse/pthread.h \ include/sparse/sys/socket.h \ include/sparse/sys/wait.h diff --git a/include/sparse/netpacket/packet.h b/include/sparse/netpacket/packet.h new file mode 100644 index 000000000000..21bdd2ea7087 --- /dev/null +++ b/include/sparse/netpacket/packet.h @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2014 Nicira, 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 + +#ifndef __NETPACKET_PACKET_SPARSE +#define __NETPACKET_PACKET_SPARSE 1 + +#include "openvswitch/types.h" + +struct sockaddr_ll + { + unsigned short int sll_family; + ovs_be16 sll_protocol; + int sll_ifindex; + unsigned short int sll_hatype; + unsigned char sll_pkttype; + unsigned char sll_halen; + unsigned char sll_addr[8]; + }; + +#endif /* <netpacket/packet.h> sparse */ diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 9c1a36db181e..e756d88a9458 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc. + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,11 +20,11 @@ #include <errno.h> #include <fcntl.h> +#include <arpa/inet.h> #include <inttypes.h> #include <linux/filter.h> #include <linux/gen_stats.h> #include <linux/if_ether.h> -#include <linux/if_packet.h> #include <linux/if_tun.h> #include <linux/types.h> #include <linux/ethtool.h> @@ -37,8 +37,10 @@ #include <sys/types.h> #include <sys/ioctl.h> #include <sys/socket.h> +#include <netpacket/packet.h> #include <net/if.h> #include <net/if_arp.h> +#include <net/if_packet.h> #include <net/route.h> #include <netinet/in.h> #include <poll.h> @@ -116,6 +118,9 @@ COVERAGE_DEFINE(netdev_set_ethtool); * With all this churn it's easiest to unconditionally define a replacement * structure that has everything we want. */ +#ifndef PACKET_AUXDATA +#define PACKET_AUXDATA 8 +#endif #ifndef TP_STATUS_VLAN_VALID #define TP_STATUS_VLAN_VALID (1 << 4) #endif