Subject: Re: [PATCH net-next v1] flow_dissector: Add support for HSR

From: Kurt Kanzenbach <[email-masked]>
Date: Thu, 03 Mar 2022 09:08:35 +0100

Hi Jakub,

On Wed Mar 02 2022, Jakub Kicinski wrote:
> On Mon, 28 Feb 2022 20:58:56 +0100 Kurt Kanzenbach wrote:
>> Network drivers such as igb or igc call eth_get_headlen() to determine the
>> header length for their to be constructed skbs in receive path.
>> 
>> When running HSR on top of these drivers, it results in triggering BUG_ON() in
>> skb_pull(). The reason is the skb headlen is not sufficient for HSR to work
>> correctly. skb_pull() notices that.
>
> Should that also be fixed? BUG_ON() seems pretty drastic.

It's this statement here:

 https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/include/linux/skbuff.h#n2483

I tried to look up, why is this a BUG_ON() in Thomas' history tree
[1]. Couldn't find an explanation. It's been introduced by this commit:

|commit 1a0153507ffae9cf3350e76c12d441788c0191e1 (HEAD)
|Author: Linus Torvalds <[email-masked]>
|Date:   Mon Feb 4 18:11:38 2002 -0800
|
|    v2.4.3.2 -> v2.4.3.3
|    
|      - Hui-Fen Hsu: sis900 driver update
|      - NIIBE Yutaka: Super-H update
|      - Alan Cox: more resyncs (ARM down, but more to go)
|      - David Miller: network zerocopy, Sparc sync, qlogic,FC fix, etc.
|      - David Miller/me: get rid of various drivers hacks to do mmap
|      alignment behind the back of the VM layer. Create a real
|      protocol for it.

It seems like BUG/BUG_ON() is the error handling practice in case of
unavailable memory. Even though most functions such as skb_push() or
skb_put() use asserts or skb_over_panic() which also result in BUG() at
the end.

Thanks,
Kurt

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/