Re: [PATCH v2 1/3] net: Get pxe config file from dhcp option 209

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3402 bytes --]

On Tue, Nov 07, 2023 at 03:50:06PM -0800, Sean Edmond wrote:
> 
> On 2023-11-04 12:53 a.m., Heinrich Schuchardt wrote:
> > On 11/4/23 03:03, Sean Edmond wrote:
> > > 
> > > On 2023-10-23 10:54 p.m., Heinrich Schuchardt wrote:
> > > > On 10/24/23 02:21, seanedmond@linux.microsoft.com wrote:
> > > > > From: Sean Edmond <seanedmond@microsoft.com>
> > > > > 
> > > > > Allow dhcp server pass pxe config file full path by using option 209
> > > > > 
> > > > > Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
> > > > > ---
> > > > >   cmd/Kconfig |  4 ++++
> > > > >   cmd/pxe.c   | 10 ++++++++++
> > > > >   net/bootp.c | 21 +++++++++++++++++++++
> > > > >   3 files changed, 35 insertions(+)
> > > > > 
> > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > > index 5bc0a92d57..adbb1a6187 100644
> > > > > --- a/cmd/Kconfig
> > > > > +++ b/cmd/Kconfig
> > > > > @@ -1826,6 +1826,10 @@ config BOOTP_PXE_CLIENTARCH
> > > > >       default 0x15 if ARM
> > > > >       default 0x0 if X86
> > > > > 
> > > > > +config BOOTP_PXE_DHCP_OPTION
> > > > > +    bool "Request & store 'pxe_configfile' from BOOTP/DHCP server"
> > > > > +    depends on BOOTP_PXE
> > > > 
> > > > Why should this be disabled by default?
> > > > 
> > > > Do we really want a separate config variable?
> > > > 
> > > I expect most won't use this option to get the file path (they'll use
> > > the default paths as per the PXE specification).  It makes more sense
> > > for me to keep it optional, like many of the other options?
> > 
> > RFC 5701 seems to require this option. Hence we should make it default
> > yes. Boards that have a build size issue can opt out.
> 
> The PXELINUX specification
> (https://wiki.syslinux.org/wiki/index.php?title=PXELINUX) doesn't state that
> option 209 is required. In the abense of option 209, PXELINUX will try the
> following default configuration files (this example is provided on the wiki
> link):
>  /mybootdir/pxelinux.cfg/b8945908-d6a6-41a9-611d-74a6ab80b83d
>  /mybootdir/pxelinux.cfg/01-88-99-aa-bb-cc-dd
>  /mybootdir/pxelinux.cfg/C0A8025B
>  /mybootdir/pxelinux.cfg/C0A8025
>  /mybootdir/pxelinux.cfg/C0A802
>  /mybootdir/pxelinux.cfg/C0A80
>  /mybootdir/pxelinux.cfg/C0A8
>  /mybootdir/pxelinux.cfg/C0A
>  /mybootdir/pxelinux.cfg/C0
>  /mybootdir/pxelinux.cfg/C
>  /mybootdir/pxelinux.cfg/default
> 
> If 209 is requested/provided it tries the provided "config file" before the
> default paths.  RFC 5071 should be seen as an extension for PXELINUX (not a
> requirement).  RFC 5071 only states "The Config File Option MUST be supplied
> by the DHCP server if it appears on the Parameter Request List".

We also want to be careful about overall size growth on common options,
even if someone can opt-out. Those are usually last-ditch stop-gaps and
it's better to make sure we really need functionality X/Y/Z by default,
for everyone. Especially with all of the work going on to make HTTP(s)
based network installs a viable option, how much more do we want to
change the PXE case, for everyone. I'm personally somewhat looking for
the use case to be well defined for a "this must be default". Network
provisioning is a thing, yes, but for whom/what, and in turn how broadly
do we need this to be in the vast majority of our boards (since it would
come in via BOOT*DEFAULTS or DISTRO_DEFAULTS).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]