On Fri, Nov 9, 2018 at 5:21 AM Sunil Kovvuri <sunil.kovvuri@xxxxxxxxx> wrote:
>
> On Fri, Nov 9, 2018 at 2:13 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> >
> > On Thu, Nov 8, 2018 at 7:37 PM <sunil.kovvuri@xxxxxxxxx> wrote:
> > > @@ -666,4 +668,20 @@ struct npc_mcam_unmap_counter_req {
> > > u8 all; /* Unmap all entries using this counter ? */
> > > };
> > >
> > > +struct npc_mcam_alloc_and_write_entry_req {
> > > + struct mbox_msghdr hdr;
> > > + struct mcam_entry entry_data;
> > > + u16 ref_entry;
> > > + u8 priority; /* Lower or higher w.r.t ref_entry */
> > > + u8 intf; /* Rx or Tx interface */
> > > + u8 enable_entry;/* Enable this MCAM entry ? */
> > > + u8 alloc_cntr; /* Allocate counter and map ? */
> > > +};
> >
> > I noticed that this structure requires padding at the end because
> > struct mbox_msghdr has a 32-bit alignment requirement. For
> > data structures in an interface, I'd recommend avoiding that kind
> > of padding and adding reserved fields or widening the types
> > accordingly.
> >
>
> When there are multiple messages in the mailbox, each message starts
> at a 16byte aligned offset. So struct mbox_msghdr is always aligned.
> I think adding reserved fields is not needed here.
>
> ===
> struct mbox_msghdr *otx2_mbox_alloc_msg_rsp(struct otx2_mbox *mbox, int devid,
> int size, int size_rsp)
> {
> size = ALIGN(size, MBOX_MSG_ALIGN);
> ===
>
> Is this what you were referring to ?
>
No, I mean the padding at the end of the structure. An example
would be a structure like
struct s {
u16 a;
u32 b;
u16 c;
};
Since b is aligned to four bytes, you get padding between a and b.
On top of that, you also get padding after c to make the size of
structure itself be a multiple of its alignment. For interfaces, we
should avoid both kinds of padding. This can be done by marking
members as __packed (usually I don't recommend that), by
changing the size of members, or by adding explicit 'reserved'
fields in place of the padding.
> > I also noticed a similar problem in struct mbox_msghdr. Maybe
> > use the 'pahole' tool to check for this kind of padding in the
> > API structures.
Arnd
