netdev - Re: Generic sk_buff contract documentation ?

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <cfdd904f-f9cb-4a6f-81c8-53b378626daf@lunn.ch>
Date: Thu, 16 Jan 2025 21:29:25 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Alexandre Ferrieux <alexandre.ferrieux@...il.com>
Cc: netdev@...r.kernel.org
Subject: Re: Generic sk_buff contract documentation ?

On Thu, Jan 16, 2025 at 08:51:26AM +0100, Alexandre Ferrieux wrote:
> Hi,
> 
> Recently digging into a driver-specific issue with sk_buff->protocol, I
> discovered (with surprise) that the computation of this field (which is *always*
> necessary for protocol handlers to kick in) was somehow "delegated" to
> individual network interface drivers. This can be seen by looking for callers of
> eth_type_trans(), e.g:
> 
> - tg3: called from tg3_poll_work
> - ixgbe: called from ixgbe_clean_rx_irq
> - veth: called from veth_xmit (though indirectly via __dev_forward_skb)
> 
> This is a surprise as one would naively expect this ubiquitous behavior to be
> triggered from generic code, depending only on the L2 header structure (but not
> on the specific NIC hardware at hand). Another surprise was *not* to find any
> mention of this "contract" in Documentation/*.
> 
> So, is it an unspoken tradition for NIC driver developers, to
> "just know" that prior to emitting an skb from the rx path, they must fill
> skb->protocol (along with who knows how many other metadata items) ?

Ideally, a new driver is simply a copy/paste of some existing driver,
which already has all this code. Trying to write a driver from scratch
without copying code is likely to run into a number of issues like
this, and why would you pass up so much usable code which has passed
review and has been heavily tested and debugged.

Could this be moved into the bottom of the stack? Maybe. But do you
want to modify 327 files?

	Andrew