[ovs-dev] MUSL netpacket/packet.h
diff mbox series

Message ID 20190306010117.GE25780@ovn.org
State New
Headers show
Series
  • [ovs-dev] MUSL netpacket/packet.h
Related show

Commit Message

Ben Pfaff March 6, 2019, 1:01 a.m. UTC
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>

Comments

0-day Robot March 6, 2019, 1:57 a.m. UTC | #1
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
Ben Pfaff March 6, 2019, 2:11 a.m. UTC | #2
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.
developer@it-offshore.co.uk March 6, 2019, 9:30 a.m. UTC | #3
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
Fernando Casas Schössow March 7, 2019, 8:10 a.m. UTC | #4
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.

Patch
diff mbox series

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