On Wed, 4 Nov 2020 22:23:14 +0530 rohit maheshwari wrote: > On 04/11/20 2:16 AM, Jakub Kicinski wrote: > > On Fri, 30 Oct 2020 23:32:20 +0530 Rohit Maheshwari wrote: > >> Creating SKB per tls record and freeing the original one causes > >> panic. There will be race if connection reset is requested. By > >> freeing original skb, refcnt will be decremented and that means, > >> there is no pending record to send, and so tls_dev_del will be > >> requested in control path while SKB of related connection is in > >> queue. > >> Better approach is to use same SKB to send one record (partial > >> data) at a time. We still have to create a new SKB when partial > >> last part of a record is requested. > >> This fix introduces new API cxgb4_write_partial_sgl() to send > >> partial part of skb. Present cxgb4_write_sgl can only provide > >> feasibility to start from an offset which limits to header only > >> and it can write sgls for the whole skb len. But this new API > >> will help in both. It can start from any offset and can end > >> writing in middle of the skb. > > You never replied to my question on v2. > > > > If the problem is that the socket gets freed, why don't you make the > > new skb take a reference on the socket? > > > > 650 LoC is really a rather large fix. > This whole skb alloc and copy record frags was written under the > assumption that there will be zero data copy (no linear skb was > expected) but that isn't correct. Continuing with the same change > requires more checks and will be more complicated. That's why I > made this change. I think using same SKB to send out multiple > records is better than allocating new SKB every time.
Bug fixes are not where code should be restructured. Please produce a minimal fix.
