RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: David Laight [mailto:David.Laight@xxxxxxxxxx]
> Sent: Friday, June 09, 2017 10:28 AM
> To: 'David Miller' <davem@xxxxxxxxxxxxx>; Mintz, Yuval
> <Yuval.Mintz@xxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx; Kalderon, Michal
> <Michal.Kalderon@xxxxxxxxxx>
> 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 <Yuval.Mintz@xxxxxxxxxx>
> > 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].
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux