| Message ID | 1447106648-5591-1-git-send-email-jacob.e.keller@intel.com |
|---|---|
| State | Accepted |
| Delegated to: | Jeff Kirsher |
| Headers | show |
On Mon, 2015-11-09 at 14:04 -0800, Jacob Keller wrote: > 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. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > --- > - 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. 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. :-)
On Tue, 2015-11-10 at 08:27 -0800, Jeff Kirsher wrote: > On Mon, 2015-11-09 at 14:04 -0800, Jacob Keller wrote: > > 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. > > > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > > --- > > - 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. > > 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
-----Original Message----- From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Keller, Jacob E Sent: Tuesday, November 10, 2015 9:38 AM To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; intel-wired-lan@lists.osuosl.org Subject: Re: [Intel-wired-lan] [next-queue PATCH v3] fm10k: correctly pack TLV structures and explain reasoning Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.h b/drivers/net/ethernet/intel/fm10k/fm10k_pf.h index a8fc512a2416..337859260b9b 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.h +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.h @@ -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[];
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. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- - 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(-)