Received-SPF: pass (google.com: domain of linux-kernel+bounces-101565-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Feedback-ID: i85e1472c:Fastmail
Content-Type: text/plain;
	charset=utf-8
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.300.61.1.2\))
Subject: Re: [PATCH] netpoll: support sending over raw IP interfaces
From: Mark <mark@yotsuba.nl>
In-Reply-To: <20240313133602.GA1263314@maili.marvell.com>
Date: Wed, 13 Mar 2024 14:53:23 +0100
Cc: netdev@vger.kernel.org,
 Hans de Goede <hdegoede@redhat.com>,
 Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>,
 Breno Leitao <leitao@debian.org>,
 Ingo Molnar <mingo@redhat.com>,
 "David S. Miller" <davem@davemloft.net>,
 Paolo Abeni <pabeni@redhat.com>,
 linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <7C42FC4B-D803-4194-8FBB-19A432D37124@yotsuba.nl>
References: <20240313124613.51399-1-mark@yotsuba.nl>
 <20240313133602.GA1263314@maili.marvell.com>
To: Ratheesh Kannoth <rkannoth@marvell.com>

Hi Ratheesh,

> Op 13 mrt 6 Reiwa, om 14:36 heeft Ratheesh Kannoth =
<rkannoth@marvell.com> het volgende geschreven:
>=20
> On 2024-03-13 at 18:16:13, Mark Cilissen (mark@yotsuba.nl) wrote:
>> [=E2=80=A6]
> Taking an assumption based on dev=E2=80=99s 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=E2=80=99s 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=3D=3D0 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=E2=80=99t 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.

> [=E2=80=A6]

Thanks and regards,
Mark=