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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <063D6719AE5E284EB5DD2968C1650D6DD001B8C2@AcuExch.aculab.com>
Date:   Fri, 9 Jun 2017 08:23:30 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     "'Mintz, Yuval'" <Yuval.Mintz@...ium.com>,
        'David Miller' <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "Kalderon, Michal" <Michal.Kalderon@...ium.com>
Subject: RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx

From: Mintz, Yuval
> Sent: 09 June 2017 08:52
> > From: David Laight [mailto:David.Laight@...LAB.COM]
> > Sent: Friday, June 09, 2017 10:28 AM
> > To: 'David Miller' <davem@...emloft.net>; Mintz, Yuval
> > <Yuval.Mintz@...ium.com>
> > Cc: netdev@...r.kernel.org; linux-rdma@...r.kernel.org; Kalderon, Michal
> > <Michal.Kalderon@...ium.com>
> > 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@...ium.com>
> > > 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