On 07/24/18 08:22 AM, Vakul Garg wrote: > > I don't think this patch is safe as-is. sgin_arr is a stack array of size > > MAX_SKB_FRAGS (+ overhead), while my read of skb_cow_data is that it > > walks the whole chain of skbs from skb->next, and can return any number of > > segments. Therefore we need to heap allocate. I think I copied the IPSEC > > code here. > > > > For perf though, we could use the stack array if skb_cow_data returns <= > > MAX_SKB_FRAGS. > > > > This code is slightly confusing though, since we don't heap allocate in the > > zerocopy case - what happens is that skb_to_sgvec returns -EMSGSIZE, and > > we fall back to the non-zerocopy case, and return again to this function, > > where we then hit the skb_cow_data path and heap allocate. > > Thanks for explaining. > I am missing the point why MAX_SKB_FRAGS sized local array > sgin has been used in tls_sw_recvmsg(). What is special about MAX_SKB_FRAGS so > that we used it as a size factor for 'sgin'?
There is nothing special about it, in the zerocopy-fastpath if we happen to fit in MAX_SKB_FRAGS we avoid any kmalloc whatsoever though. It could be renamed MAX_SC_FOR_FASTPATH or something. > Will it be a bad idea to get rid of array 'sgin' on stack and simply kmalloc > 'sgin' for > whatever the number the number of pages returned by iov_iter_npages()? > We can allocate for sgout too with the same kmalloc(). > > (Using a local array based 'sgin' is coming in the way to achieve sending > multiple async > record decryption requests to the accelerator without waiting for previous > one to complete.) Yes we could do this, and yes we would need to heap allocate if you want to support multiple outstanding decryption requests. I think async crypto prevents any sort of zerocopy-fastpath, however.
