netdev - Re: [PATCH] netpoll: support sending over raw IP interfaces

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <7C42FC4B-D803-4194-8FBB-19A432D37124@yotsuba.nl>
Date: Wed, 13 Mar 2024 14:53:23 +0100
From: Mark <mark@...suba.nl>
To: Ratheesh Kannoth <rkannoth@...vell.com>
Cc: netdev@...r.kernel.org,
 Hans de Goede <hdegoede@...hat.com>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Breno Leitao <leitao@...ian.org>,
 Ingo Molnar <mingo@...hat.com>,
 "David S. Miller" <davem@...emloft.net>,
 Paolo Abeni <pabeni@...hat.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] netpoll: support sending over raw IP interfaces

Hi Ratheesh,

> Op 13 mrt 6 Reiwa, om 14:36 heeft Ratheesh Kannoth <rkannoth@...vell.com> het volgende geschreven:
> 
> On 2024-03-13 at 18:16:13, Mark Cilissen (mark@...suba.nl) wrote:
>> […]
> Taking an assumption based on dev’s lower layer does not look to be good.
> why not transmit packet from skb_network_header() in your driver (by making
> changes in your driver)

There’s two assumptions at play here:
- The lower layer is ethernet: this has always been present in netpoll, and is even
  documented in netconsole.rst. This comment just mentions it because we add a way
  to bypass the assumption; it is not an assumption this patch adds to the code.
- hard_header_len==0 means that there is no exposed link layer: this is a rather
  conservative assumption in my opinion, and is also mentioned in the definition
  of LL_RESERVED_SPACE:

> * Alternative is:
> *   dev->hard_header_len ? (dev->hard_header_len +
> *                            (HH_DATA_MOD - 1)) & ~(HH_DATA_MOD - 1) : 0

  The same assumption is also made in more places in the core network code, like af_packet:

>   - If the device has no dev->header_ops->create, there is no LL header
>     visible above the device. In this case, its hard_header_len should be 0.
>     The device may prepend its own header internally. In this case, its
>     needed_headroom should be set to the space needed for it to add its
>     internal header.

I could change it to, like af_packet, check `dev->header_ops` instead if that is preferred,
but I don’t think that patching every single raw IP driver to deal with skbs that managed
to somehow have link layer data would be preferred here, especially since netpoll is kind
of a special case to begin with. I am open to suggestions and ideas, though.

> […]

Thanks and regards,
Mark