net: batman-adv: use "__packed __aligned(2)" for each structure instead of "__packed(2)" region

Unfortunately, not all compilers assumes the structures within a pack region also need be packed (e.g. metag), so need add a pack explicitly to satisfy all compilers.

The related error (under metag with allmodconfig):

MODPOST 2952 modules
ERROR: "__compiletime_assert_431" [net/batman-adv/batman-adv.ko] undefined!
ERROR: "__compiletime_assert_432" [net/batman-adv/batman-adv.ko] undefined!
ERROR: "__compiletime_assert_429" [net/batman-adv/batman-adv.ko] undefined!
ERROR: "__compiletime_assert_428" [net/batman-adv/batman-adv.ko] undefined!
ERROR: "__compiletime_assert_423" [net/batman-adv/batman-adv.ko] undefined!

Signed-off-by Chen Gang <[email protected]>

---
 net/batman-adv/packet.h | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

net/batman-adv/packet.h +6 -10

index 0a381d1..9206b48 100644
@@ -154,7 +154,6 @@ enum batadv_tvlv_type {
 	BATADV_TVLV_ROAM	= 0x05,
 };
 
-#pragma pack(2)
 /* the destination hardware field in the ARP frame is used to
  * transport the claim type and the group id
  */

@@ -162,8 +161,7 @@ struct batadv_bla_claim_dst {
 	uint8_t magic[3];	/* FF:43:05 */
 	uint8_t type;		/* bla_claimframe */
 	__be16 group;		/* group id */
-};
-#pragma pack()
+} __packed __aligned(2);
 
 /**
  * struct batadv_ogm_packet - ogm (routing protocol) packet

@@ -281,7 +279,6 @@ struct batadv_icmp_packet_rr {
  * misalignment of the payload after the ethernet header. It may also lead to
  * leakage of information when the padding it not initialized before sending.
  */
-#pragma pack(2)
 
 /**
  * struct batadv_unicast_packet - unicast packet for network payload

@@ -300,7 +297,7 @@ struct batadv_unicast_packet {
 	/* "4 bytes boundary + 2 bytes" long to make the payload after the
 	 * following ethernet header again 4 bytes boundary aligned
 	 */
-};
+}  __packed __aligned(2);
 
 /**
  * struct batadv_unicast_4addr_packet - extended unicast packet

@@ -316,7 +313,7 @@ struct batadv_unicast_4addr_packet {
 	/* "4 bytes boundary + 2 bytes" long to make the payload after the
 	 * following ethernet header again 4 bytes boundary aligned
 	 */
-};
+}  __packed __aligned(2);
 
 /**
  * struct batadv_frag_packet - fragmented packet

@@ -347,7 +344,7 @@ struct batadv_frag_packet {
 	uint8_t orig[ETH_ALEN];
 	__be16  seqno;
 	__be16  total_size;
-};
+}  __packed __aligned(2);
 
 /**
  * struct batadv_bcast_packet - broadcast packet for network payload

@@ -368,7 +365,7 @@ struct batadv_bcast_packet {
 	/* "4 bytes boundary + 2 bytes" long to make the payload after the
 	 * following ethernet header again 4 bytes boundary aligned
 	 */
-};
+}  __packed __aligned(2);
 
 /**
  * struct batadv_coded_packet - network coded packet

@@ -404,9 +401,8 @@ struct batadv_coded_packet {
 	uint8_t  second_orig_dest[ETH_ALEN];
 	__be32   second_crc;
 	__be16   coded_len;
-};
+}  __packed __aligned(2);
 
-#pragma pack()
 
 /**
  * struct batadv_unicast_tvlv - generic unicast packet with tvlv payload

Quoted 18 lines · Chen Gang

Unfortunately, not all compilers assumes the structures within a pack region also need be packed (e.g. metag), so need add a pack explicitly to satisfy all compilers.

The related error (under metag with allmodconfig):
MODPOST 2952 modules

ERROR: "__compiletime_assert_431" [net/batman-adv/batman-adv.ko] undefined!
ERROR: "__compiletime_assert_432" [net/batman-adv/batman-adv.ko] undefined!
ERROR: "__compiletime_assert_429" [net/batman-adv/batman-adv.ko] undefined!
ERROR: "__compiletime_assert_428" [net/batman-adv/batman-adv.ko] undefined!
ERROR: "__compiletime_assert_423" [net/batman-adv/batman-adv.ko] undefined!

Quoted 1 line

Signed-off-by Chen Gang <[email protected]>

David, what do you think about this change?

Can "__packed __aligned(2)" generate a different structure padding than "#pragma pack(2)" ?

I am not really sure about the difference between the two. But if we have the possibility that the padding may change then this patch should go into net, otherwise we will have a protocol compatibility problem between 3.13 and 3.14.

It appears that the following gcc patch adds support for #pragma pack:
http://gcc.gnu.org/ml/gcc-patches/2006-10/msg01115.html

I gave it a quick spin on metag gcc (which is unfortunately stuck on an old version) and it seems to fix my simple test case so that #pragma pack(2) becomes equivalent to __packed __aligned(2) (for sizeof and __alignof__).

However, the __packed and __aligned are linux specific macros to abstract compiler details, whereas #pragma pack appears to be a compiler-specific WIN32 style equivalent to GCC's __attribute__((packed)) and __attribute__((aligned(2))) (these are what __packed and __aligned use in compiler-gcc.h).

Therefore I believe using the Linux abstractions is still more correct here.

Quoted 8 lines · James Hogan

It appears that the following gcc patch adds support for #pragma pack:
http://gcc.gnu.org/ml/gcc-patches/2006-10/msg01115.html

I gave it a quick spin on metag gcc (which is unfortunately stuck on an old version) and it seems to fix my simple test case so that #pragma pack(2) becomes equivalent to __packed __aligned(2) (for sizeof and __alignof__).

Then I personally think that it is better to fix metag gcc instead of changing the kernel.

Actually there are many different spots where "#pragma pack" is used. batman-adv is just the only one having compile time checks for structure sizes.

Quoted 7 lines

However, the __packed and __aligned are linux specific macros to abstract compiler details, whereas #pragma pack appears to be a compiler-specific WIN32 style equivalent to GCC's __attribute__((packed)) and __attribute__((aligned(2))) (these are what __packed and __aligned use in compiler-gcc.h).

Quoted 1 line

Therefore I believe using the Linux abstractions is still more correct here.

If you really think so, I'd suggest to grep in the kernel and catch all the other occurrences of "#pragma pack" and change them all (assuming that using __attribute__((aligned(2))) is the way to go).

Quoted 7 lines · James Hogan

It appears that the following gcc patch adds support for #pragma pack:
http://gcc.gnu.org/ml/gcc-patches/2006-10/msg01115.html

I gave it a quick spin on metag gcc (which is unfortunately stuck on an old version) and it seems to fix my simple test case so that #pragma pack(2) becomes equivalent to __packed __aligned(2) (for sizeof and __alignof__).

Then I personally think that it is better to fix metag gcc instead of changing the kernel.

Actually there are many different spots where "#pragma pack" is used. batman-adv is just the only one having compile time checks for structure sizes.