batctl-2014.4.0 fails to build with musl, mostly because of somewhat weird netinet/*.h headers musl provides, but the problem turns out to be easy to fix.
musl does not allow including netinet/* and linux/* headers together. batctl includes netinet/if_ether.h indirectly via net/ethernet.h, so netinet/if_ether.h must be used instead of linux/if_ether.h.
__be16 and __be32 are linux-specific typedefs for uint16_t and uint32_t with __attribute__((bitwise)) that has no effect outside of the kernel. Replacing them with uint16_t and uint32_t removes dependency on linux/types.h.
Signed-off-by: Alex Suykov alex.suykov@gmail.com
--- batctl-2014.4.0/ping.c +++ batctl-2014.4.0/ping.c @@ -34,7 +34,7 @@ #include <stdint.h> #include <sys/select.h> #include <sys/time.h> -#include <linux/if_ether.h> +#include <netinet/if_ether.h>
#include "main.h" #include "ping.h" --- batctl-2014.4.0/tcpdump.h +++ batctl-2014.4.0/tcpdump.h @@ -23,7 +23,7 @@ #define _BATCTL_TCPDUMP_H
#include <netpacket/packet.h> -#include <linux/if_ether.h> +#include <netinet/if_ether.h> #include <net/if_arp.h> #include <sys/types.h> #include "main.h" --- batctl-2014.4.0/traceroute.c +++ batctl-2014.4.0/traceroute.c @@ -28,7 +28,7 @@ #include <unistd.h> #include <fcntl.h> #include <string.h> -#include <linux/if_ether.h> +#include <netinet/if_ether.h> #include <stddef.h> #include <sys/select.h> #include <sys/time.h> --- batctl-2014.4.0/packet.h +++ batctl-2014.4.0/packet.h @@ -196,7 +196,7 @@ struct batadv_bla_claim_dst { uint8_t magic[3]; /* FF:43:05 */ uint8_t type; /* bla_claimframe */ - __be16 group; /* group id */ + uint16_t group; /* group id */ }; #pragma pack()
@@ -213,12 +213,12 @@ uint8_t version; uint8_t ttl; uint8_t flags; - __be32 seqno; + uint32_t seqno; uint8_t orig[ETH_ALEN]; uint8_t prev_sender[ETH_ALEN]; uint8_t reserved; uint8_t tq; - __be16 tvlv_len; + uint16_t tvlv_len; /* __packed is not needed as the struct size is divisible by 4, * and the largest data type in this struct has a size of 4. */ @@ -273,7 +273,7 @@ uint8_t orig[ETH_ALEN]; uint8_t uid; uint8_t reserved; - __be16 seqno; + uint16_t seqno; };
#define BATADV_RR_LEN 16 @@ -300,7 +300,7 @@ uint8_t orig[ETH_ALEN]; uint8_t uid; uint8_t rr_cur; - __be16 seqno; + uint16_t seqno; uint8_t rr[BATADV_RR_LEN][ETH_ALEN]; };
@@ -380,8 +380,8 @@ #endif uint8_t dest[ETH_ALEN]; uint8_t orig[ETH_ALEN]; - __be16 seqno; - __be16 total_size; + uint16_t seqno; + uint16_t total_size; };
/** @@ -398,7 +398,7 @@ uint8_t version; /* batman version field */ uint8_t ttl; uint8_t reserved; - __be32 seqno; + uint32_t seqno; uint8_t orig[ETH_ALEN]; /* "4 bytes boundary + 2 bytes" long to make the payload after the * following ethernet header again 4 bytes boundary aligned @@ -431,14 +431,14 @@ /* uint8_t first_dest[ETH_ALEN]; - saved in mac header destination */ uint8_t first_source[ETH_ALEN]; uint8_t first_orig_dest[ETH_ALEN]; - __be32 first_crc; + uint32_t first_crc; uint8_t second_ttl; uint8_t second_ttvn; uint8_t second_dest[ETH_ALEN]; uint8_t second_source[ETH_ALEN]; uint8_t second_orig_dest[ETH_ALEN]; - __be32 second_crc; - __be16 coded_len; + uint32_t second_crc; + uint16_t coded_len; };
#pragma pack() @@ -461,7 +461,7 @@ uint8_t reserved; uint8_t dst[ETH_ALEN]; uint8_t src[ETH_ALEN]; - __be16 tvlv_len; + uint16_t tvlv_len; uint16_t align; };
@@ -474,7 +474,7 @@ struct batadv_tvlv_hdr { uint8_t type; uint8_t version; - __be16 len; + uint16_t len; };
/** @@ -484,8 +484,8 @@ * @bandwidth_up: advertised uplink upload bandwidth */ struct batadv_tvlv_gateway_data { - __be32 bandwidth_down; - __be32 bandwidth_up; + uint32_t bandwidth_down; + uint32_t bandwidth_up; };
/** @@ -498,7 +498,7 @@ struct batadv_tvlv_tt_data { uint8_t flags; uint8_t ttvn; - __be16 num_vlan; + uint16_t num_vlan; };
/** @@ -509,8 +509,8 @@ * @reserved: unused, useful for alignment purposes */ struct batadv_tvlv_tt_vlan_data { - __be32 crc; - __be16 vid; + uint32_t crc; + uint16_t vid; uint16_t reserved; };
@@ -526,7 +526,7 @@ uint8_t flags; uint8_t reserved[3]; uint8_t addr[ETH_ALEN]; - __be16 vid; + uint16_t vid; };
/** @@ -536,7 +536,7 @@ */ struct batadv_tvlv_roam_adv { uint8_t client[ETH_ALEN]; - __be16 vid; + uint16_t vid; };
/**
On Wednesday 01 April 2015 20:18:21 Alex Suykov wrote:
batctl-2014.4.0 fails to build with musl, mostly because of somewhat weird netinet/*.h headers musl provides, but the problem turns out to be easy to fix.
musl does not allow including netinet/* and linux/* headers together. batctl includes netinet/if_ether.h indirectly via net/ethernet.h, so netinet/if_ether.h must be used instead of linux/if_ether.h.
__be16 and __be32 are linux-specific typedefs for uint16_t and uint32_t with __attribute__((bitwise)) that has no effect outside of the kernel. Replacing them with uint16_t and uint32_t removes dependency on linux/types.h.
Signed-off-by: Alex Suykov alex.suykov@gmail.com
Nacked-by: Sven Eckelmann sven@narfation.org
The __be32, __be16 are from the kernel and used there to check if data was correctly converted from host byte order to big endian (and the other way around). batctl just uses the packet.h from the kernel module. Removing them from the userspace would mean that it gets removed from the kernel module. This would mean that someone has send this "weird" commit to the next linux kernel maintainer. And I am quite happy that I will not be this person.
See https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-March/012926.html for a proposal how to provide headers which are not yet part of linux-libc-dev (linux kernel uapi headers).
Kind regards, Sven
Wed, Apr 01, 2015 at 07:36:10PM +0200, Sven Eckelmann wrote:
Nacked-by: Sven Eckelmann sven@narfation.org
The __be32, __be16 are from the kernel and used there to check if data was correctly converted from host byte order to big endian (and the other way around). batctl just uses the packet.h from the kernel module.
Ah, I see, I wasn't aware packet.h is shared between batctl and the batman kernel module.
See https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-March/012926.html for a proposal how to provide headers which are not yet part of linux-libc-dev (linux kernel uapi headers).
It's not that linux headers are not available in musl, they are. The problem is that musl provides its own standalone <netinet/if_ether.h> instead of including <linux/if_ether.h>, and if both get included at the same time, gcc complains about duplicate definitions.
Can you please take a look at the patch below? That is enough to get a musl build, too, and it keeps packet.h intact.
In glibc and uclibc at least, <netinet/if_ether.h> includes <linux/if_ether.h>, so those can be interchanged freely, and batctl already depends on netinet/* headers. With musl, that would break bitwise attribute of course, but only outside of the kernel, and that would be a musl issue anyway.
ping.c | 3 ++- tcpdump.c | 1 + tcpdump.h | 2 +- traceroute.c | 3 ++- 4 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/ping.c b/ping.c index bdca222..9ec6745 100644 --- a/ping.c +++ b/ping.c @@ -34,7 +34,8 @@ #include <stdint.h> #include <sys/select.h> #include <sys/time.h> -#include <linux/if_ether.h> +#include <netinet/if_ether.h> +#include <linux/types.h>
#include "main.h" #include "ping.h" diff --git a/tcpdump.c b/tcpdump.c index b994977..cfeb4cc 100644 --- a/tcpdump.c +++ b/tcpdump.c @@ -43,6 +43,7 @@ #include <stdint.h> #include <sys/select.h> #include <sys/socket.h> +#include <linux/types.h>
#include "tcpdump.h" #include "packet.h" diff --git a/tcpdump.h b/tcpdump.h index 5d936f2..3c9126c 100644 --- a/tcpdump.h +++ b/tcpdump.h @@ -23,7 +23,7 @@ #define _BATCTL_TCPDUMP_H
#include <netpacket/packet.h> -#include <linux/if_ether.h> +#include <netinet/if_ether.h> #include <net/if_arp.h> #include <sys/types.h> #include "main.h" diff --git a/traceroute.c b/traceroute.c index 4ebfec2..5b58d9d 100644 --- a/traceroute.c +++ b/traceroute.c @@ -22,16 +22,17 @@
#include <netinet/in.h> +#include <netinet/if_ether.h> #include <errno.h> #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <fcntl.h> #include <string.h> -#include <linux/if_ether.h> #include <stddef.h> #include <sys/select.h> #include <sys/time.h> +#include <linux/types.h>
#include "main.h" #include "traceroute.h"
On Wednesday 01 April 2015 22:35:50 Alex Suykov wrote: [...]
It's not that linux headers are not available in musl, they are. The problem is that musl provides its own standalone <netinet/if_ether.h> instead of including <linux/if_ether.h>, and if both get included at the same time, gcc complains about duplicate definitions.
Can you please take a look at the patch below? That is enough to get a musl build, too, and it keeps packet.h intact.
In glibc and uclibc at least, <netinet/if_ether.h> includes <linux/if_ether.h>, so those can be interchanged freely, and batctl already depends on netinet/* headers. With musl, that would break bitwise attribute of course, but only outside of the kernel, and that would be a musl issue anyway.
The linux/if_ether.h -> netinet/if_ether.h seems to be understandable (but rather unfortunate).
I am not sure why why you include linux/types.h to the different source files. Most likely because this include should actually be in packet.h as mentioned in https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-March/012930.html https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-March/012942.html
Kind regards, Sven
Wed, Apr 01, 2015 at 10:02:14PM +0200, Sven Eckelmann wrote:
The linux/if_ether.h -> netinet/if_ether.h seems to be understandable (but rather unfortunate).
I am not sure why why you include linux/types.h to the different source files. Most likely because this include should actually be in packet.h as mentioned in https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-March/012930.html https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-March/012942.html
I assume any changes in packet.h must be done in batman-adv first, then ported to batctl, and preferably not the other way around. And since I was only concerned with batctl, I saw packet.h as read-only.
Moving linux/types.h to packet.h does make sense I think. However, it also brings linux/if_ether.h there, which immediately makes musl support difficult. Maybe even impractical.
I'm not saying that's wrong, because I'm not trying to say batctl needs musl support in the first place. Just another point to consider.
On Thursday 02 April 2015 00:53:02 Alex Suykov wrote:
Wed, Apr 01, 2015 at 10:02:14PM +0200, Sven Eckelmann wrote:
The linux/if_ether.h -> netinet/if_ether.h seems to be understandable (but rather unfortunate).
I am not sure why why you include linux/types.h to the different source files. Most likely because this include should actually be in packet.h as mentioned in
https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-March/012930.html https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-March/012942.html
I assume any changes in packet.h must be done in batman-adv first, then ported to batctl, and preferably not the other way around. And since I was only concerned with batctl, I saw packet.h as read-only.
Yes, this is the preferable way to modify packet.h. I was asking to understand if these are needed for something else which I may have missed.
Maybe you can remove the type.h stuff and ask to get it queued in after my patches are in. At least if this would work
Moving linux/types.h to packet.h does make sense I think. However, it also brings linux/if_ether.h there, which immediately makes musl support difficult. Maybe even impractical.
The maintainers rejected the use of __KERNEL__ checks when Markus Pargmann wanted to use them [1]. So I may have to modify my patch to move bitops and if_ether.h back to main.h. Maybe you can try to modify your packet.h as seen in the new version of "[PATCHv3 2/4] batman-adv: Add required to includes to all files" which I will send in some minutes. Would be nice when this version + your linux/if_ether.h -> netinet/if_ether.h would work with musl.
I'm not saying that's wrong, because I'm not trying to say batctl needs musl support in the first place. Just another point to consider.
We can at least try to get support in. But when we have two problems to solve then it may takes some more rounds and extra discussion :)
Kind regards, Sven
[1] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2014-December/012699.html
Thu, Apr 02, 2015 at 08:39:32AM +0200, Sven Eckelmann wrote:
Moving linux/types.h to packet.h does make sense I think. However, it also brings linux/if_ether.h there, which immediately makes musl support difficult. Maybe even impractical.
The maintainers rejected the use of __KERNEL__ checks when Markus Pargmann wanted to use them [1]. So I may have to modify my patch to move bitops and if_ether.h back to main.h. Maybe you can try to modify your packet.h as seen in the new version of "[PATCHv3 2/4] batman-adv: Add required to includes to all files" which I will send in some minutes. Would be nice when this version
- your linux/if_ether.h -> netinet/if_ether.h would work with musl.
Not only it works, it also turns out to be shortest musl-fixes patch so far. With linux/types.h in packet.h, I only need to replace linux/if_ether.h with netinet/if_ether.h to get a successful musl build.
b.a.t.m.a.n@lists.open-mesh.org
-
Alex Suykov -
Sven Eckelmann