From mboxrd@z Thu Jan 1 00:00:00 1970 From: Martin Schiller Subject: Re: [net v3] drivers/net/wan/lapbether: Use needed_headroom instead of hard_header_len Date: Wed, 05 Aug 2020 07:23:55 +0200 Message-ID: <9975370f14b8ddeafc8dec7bc6c0878a@dev.tdt.de> References: <20200802195046.402539-1-xie.he.0141@gmail.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Xie He Cc: "David S. Miller" , Jakub Kicinski , Linux Kernel Network Developers , LKML , Linux X25 , Willem de Bruijn , Brian Norris , netdev-owner@vger.kernel.org On 2020-08-04 21:20, Xie He wrote: > On Tue, Aug 4, 2020 at 5:43 AM Martin Schiller wrote: >> >> I'm not an expert in the field, but after reading the commit message >> and >> the previous comments, I'd say that makes sense. > > Thanks! > >> Shouldn't this kernel panic be intercepted by a skb_cow() before the >> skb_push() in lapbeth_data_transmit()? > > When a skb is passing down a protocol stack for transmission, there > might be several different skb_push calls to prepend different > headers. It would be the best (in terms of performance) if we can > allocate the needed header space in advance, so that we don't need to > reallocate the skb every time a new header needs to be prepended. Yes, I agree. > Adding skb_cow before these skb_push calls would indeed help > preventing kernel panics, but that might not be the essential issue > here, and it might also prevent us from discovering the real issue. (I > guess this is also the reason skb_cow is not included in skb_push > itself.) Well, you are right that the panic is "useful" to discover the real problem. But on the other hand, if it is possible to prevent a panic, I think we should do so. Maybe with adding a warning, when skb_cow() needs to reallocate memory. But this is getting a little bit off topic. For this patch I can say: LGTM. Reviewed-by: Martin Schiller