Skip to contentSkip to conversation

fm10k: correctly pack TLV structures and explain reasoning

PATCH New Jacob Keller 3 participants 4 messages Nov 9, 2015 – Dec 16, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 fm10k: correctly pack TLV structures and explain reasoning

The TLV format for little endian structures is actually 4 byte aligned copy. To this end, we need to add an additional __aligned(4) marker along with __packed to ensure that these structures are actually 4 byte aligned and packed correctly. Use of just __packed will not work as this will result in 1byte alignment which is incorrect. Add a comment explaining the reasoning behind why these structures need the special treatment.

---
- v3
* use __aligned(4) instead of __attribute__(aligned(4))

Note: this patch replaces both TLV patches currently on the queue as it looks like Jeff had forgotten to remove the earlier one when the subject changed.

 drivers/net/ethernet/intel/fm10k/fm10k_pf.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k_pf.h +9 -4
index a8fc512a2416..337859260b9b 100644
@@ -74,6 +74,11 @@ enum fm10k_pf_tlv_attr_id_v1 {
 #define FM10K_MSG_UPDATE_PVID_PVID_SHIFT	16
 #define FM10K_MSG_UPDATE_PVID_PVID_SIZE		16
 
+/* The following data structures are overlayed directly onto TLV mailbox
+ * messages, and must not break 4 byte alignment. Ensure the structures line
+ * up correctly as per their TLV definition.
+ */
+
 struct fm10k_mac_update {
 	__le32	mac_lower;
 	__le16	mac_upper;
@@ -81,26 +86,26 @@ struct fm10k_mac_update {
 	__le16	glort;
 	u8	flags;
 	u8	action;
-} __packed;
+} __aligned(4) __packed;
 
 struct fm10k_global_table_data {
 	__le32	used;
 	__le32	avail;
-} __packed;
+} __aligned(4) __packed;
 
 struct fm10k_swapi_error {
 	__le32				status;
 	struct fm10k_global_table_data	mac;
 	struct fm10k_global_table_data	nexthop;
 	struct fm10k_global_table_data	ffu;
-} __packed;
+} __aligned(4) __packed;
 
 struct fm10k_swapi_1588_timestamp {
 	__le64 egress;
 	__le64 ingress;
 	__le16 dglort;
 	__le16 sglort;
-} __packed;
+} __aligned(4) __packed;
 
 s32 fm10k_msg_lport_map_pf(struct fm10k_hw *, u32 **, struct fm10k_mbx_info *);
 extern const struct fm10k_tlv_attr fm10k_lport_map_msg_attr[];
Signature · 1 line
2.6.1.264.gbab76a9
#1 lore raw

PATCH v3 1/1 fm10k: correctly pack TLV structures and explain reasoning

Quoted 16 lines · Jacob Keller

The TLV format for little endian structures is actually 4 byte aligned
copy. To this end, we need to add an additional __aligned(4) marker along with __packed to ensure that these structures are actually 4 byte
aligned and packed correctly. Use of just __packed will not work as this
will result in 1byte alignment which is incorrect. Add a comment explaining the reasoning behind why these structures need the special treatment.

---
- v3
* use __aligned(4) instead of __attribute__(aligned(4))

Quoted 5 lines

Note: this patch replaces both TLV patches currently on the queue as it
looks like Jeff had forgotten to remove the earlier one when the subject
changed.

Nope, I just applied the last submitted version of this patch. ?Might
be helpful if the version actually incremented when you re-submit a
patch. ?This is the third version of "v3", so techincally this should
have been v5. :-)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151110/7cf5337d/attachment.asc>

#2 lore raw

PATCH v3 1/1 fm10k: correctly pack TLV structures and explain reasoning

Quoted 25 lines · Jeff Kirsher
Quoted 17 lines · Jacob Keller

The TLV format for little endian structures is actually 4 byte aligned
copy. To this end, we need to add an additional __aligned(4) marker along with __packed to ensure that these structures are actually 4 byte
aligned and packed correctly. Use of just __packed will not work as this
will result in 1byte alignment which is incorrect. Add a comment explaining the reasoning behind why these structures need the special
treatment.

---
- v3
* use __aligned(4) instead of __attribute__(aligned(4))

Quoted 6 lines

Note: this patch replaces both TLV patches currently on the queue as
it
looks like Jeff had forgotten to remove the earlier one when the subject
changed.

Quoted 4 lines

Nope, I just applied the last submitted version of this patch. ?Might be helpful if the version actually incremented when you re-submit a patch. ?This is the third version of "v3", so techincally this should have been v5. :-)

At one point there were two different versions on the queue somehow.

It is difficult to remember the version because git doesn't make it easy to keep track of the version. I'm currently working on how to integrate git-notes into my workflow for this sort of thing.

Regards,
Jake

#3 lore raw

PATCH v3 1/1 fm10k: correctly pack TLV structures and explain reasoning

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Keller, Jacob E
Sent: Tuesday, November 10, 2015 9:38 AM
To: Kirsher, Jeffrey T <[email protected]>; intel-wired-lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next-queue PATCH v3] fm10k: correctly pack TLV structures and explain reasoning