From: Mintz, Yuval
> Sent: 09 June 2017 08:52
> > From: David Laight [mailto:[email protected]]
> > Sent: Friday, June 09, 2017 10:28 AM
> > To: 'David Miller' <[email protected]>; Mintz, Yuval
> > <[email protected]>
> > Cc: [email protected]; [email protected]; Kalderon, Michal
> > <[email protected]>
> > Subject: RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx
> >
> > From: David Miller
> > > Sent: 09 June 2017 00:24
> > >
> > > From: Yuval Mintz <[email protected]>
> > > Date: Thu, 8 Jun 2017 19:13:16 +0300
> > >
> > > > @@ -67,6 +79,21 @@ struct qed_ll2_stats {
> > > > u64 sent_bcast_pkts;
> > > > };
> > > >
> > > > +struct qed_ll2_tx_pkt_info {
> > > > + u8 num_of_bds;
> > > > + u16 vlan;
> > > > + u8 bd_flags;
> > > > + u16 l4_hdr_offset_w; /* from start of packet */
> > > > + enum qed_ll2_tx_dest tx_dest;
> > > > + enum qed_ll2_roce_flavor_type qed_roce_flavor;
> > > > + dma_addr_t first_frag;
> > > > + u16 first_frag_len;
> > > > + bool enable_ip_cksum;
> > > > + bool enable_l4_cksum;
> > > > + bool calc_ip_len;
> > > > + void *cookie;
> > > > +};
> > > > +
> > >
> > > This layout is extremely inefficient, with lots of padding in between
> > > struct members.
> > >
> > > Group small u8 members and u16 members together so that they consume
> > > full 32-bit areas so you can eliminate all of the padding.
> >
> > I'd also query the use of u16 sizes/lengths, any arithmetic on u16 (and u8)
> > variables is likely to generate extra code (on non-x86).
> > You are using 32 bits for the 'enum' - I bet the values fit in 8 bits, so
> > aren't
> > really worried about size.
> >
> > If size did matter you can easily get the above down to 32 bytes.
>
> You're right, and that's exactly the point - since this is not data-path
> critical
> I don't see why the size/efficiency should matter [greatly].
It is just good practise so that it happens automatically when it does matter.
Just swapping 'vlan' and 'bd_flags' would make it look much better.
David
