netdev - Re: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20171208153734.GB6955@hmswarspite.think-freely.org>
Date:   Fri, 8 Dec 2017 10:37:34 -0500
From:   Neil Horman <nhorman@...driver.com>
To:     Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc:     David Laight <David.Laight@...LAB.COM>,
        'Xin Long' <lucien.xin@...il.com>,
        network dev <netdev@...r.kernel.org>,
        "linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for
 sctp_stream_interleave

On Fri, Dec 08, 2017 at 12:56:30PM -0200, Marcelo Ricardo Leitner wrote:
> On Fri, Dec 08, 2017 at 02:06:04PM +0000, David Laight wrote:
> > From: Xin Long
> > > Sent: 08 December 2017 13:04
> > ...
> > > @@ -264,8 +264,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> > >  				frag |= SCTP_DATA_SACK_IMM;
> > >  		}
> > > 
> > > -		chunk = sctp_make_datafrag_empty(asoc, sinfo, len, frag,
> > > -						 0, GFP_KERNEL);
> > > +		chunk = asoc->stream.si->make_datafrag(asoc, sinfo, len, frag,
> > > +						       GFP_KERNEL);
> > 
> > I know that none of the sctp code is very optimised, but that indirect
> > call is going to be horrid.
> 
> Yeah.. but there is no way to avoid the double derreference
> considering we only have the asoc pointer in there and we have to
> reach the contents of the data chunk operations struct, and the .si
> part is the same as 'stream' part as it's a constant offset.
> 
> Due to the for() in there, we could add a variable to store
> asoc->stream.si outside the for and then we can do only a single deref
> inside it. Xin, can you please try and see if the generated code is
> different?
> 
> Other suggestions?
> 
Is it worth replacing the si struct with an index/enum value, and indexing an
array of method pointer structs?  That would save you at least one dereference.

Alternatively you could preform the dereference in two steps (i.e. declare an si
pointer on the stack and set it equal to asoc->stream.si, then deref
si->make_datafrag at call time.  That will at least give the compiler an
opportunity to preload the first pointer.

Neil

>   Marcelo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>