diff mbox series

[v9,35/37,TESTING] Kconfig: enable NET_LWIP by default except for SANDBOX

Message ID 1629f6845b448edcdead09a9350f1ae2ae7a4bb6.1724419624.git.jerome.forissier@linaro.org
State Superseded
Headers show
Series Introduce the lwIP network stack | expand

Commit Message

Jerome Forissier Aug. 23, 2024, 1:48 p.m. UTC
Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
NET_LWIP so default to NET in this case.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Simon Glass Aug. 29, 2024, 2:05 p.m. UTC | #1
Hi Jerome,

On Fri, 23 Aug 2024 at 07:50, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
> NET_LWIP so default to NET in this case.

Sandbox needs to support NET_LWIP.

>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Kconfig b/Kconfig
> index 6657b9e5e30..2b316afa542 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -749,6 +749,7 @@ menu Networking
>
>  choice
>         prompt "Networking stack"
> +       default NET_LWIP if !SANDBOX
>         default NET
>
>  config NO_NET
> --
> 2.40.1
>

Regards,
Simon
Jerome Forissier Aug. 29, 2024, 4:21 p.m. UTC | #2
On 8/29/24 16:05, Simon Glass wrote:
> Hi Jerome,
> 
> On Fri, 23 Aug 2024 at 07:50, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
>>
>> Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
>> NET_LWIP so default to NET in this case.
> 
> Sandbox needs to support NET_LWIP.

I agree in principle, but AFAICT it is not a trivial task. Does it have to
be done in this series or can it be dealt with later?

I would need to look into this more closely to give details, but if I
remember correctly the sandbox tests expect a precise sequence of network
packets. For example when doing a TFTP tests, it expects an ARP packet to
go out first, but when lwIP is used the ARP might not go out because the IP
may already be in the cache. So the sandbox is definitely not a black box
in that respect which makes things a bit more difficult.

Thanks,
Simon Glass Aug. 30, 2024, 12:59 a.m. UTC | #3
Hi Jerome,

On Thu, 29 Aug 2024 at 10:21, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
>
>
> On 8/29/24 16:05, Simon Glass wrote:
> > Hi Jerome,
> >
> > On Fri, 23 Aug 2024 at 07:50, Jerome Forissier
> > <jerome.forissier@linaro.org> wrote:
> >>
> >> Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
> >> NET_LWIP so default to NET in this case.
> >
> > Sandbox needs to support NET_LWIP.
>
> I agree in principle, but AFAICT it is not a trivial task. Does it have to
> be done in this series or can it be dealt with later?

Does it build OK with sandbox? We use it for almost all of our feature
development and testing, so I cannot imagine bringing this in if it
doesn't work. How did you develop this feature?

>
> I would need to look into this more closely to give details, but if I
> remember correctly the sandbox tests expect a precise sequence of network
> packets. For example when doing a TFTP tests, it expects an ARP packet to
> go out first, but when lwIP is used the ARP might not go out because the IP
> may already be in the cache. So the sandbox is definitely not a black box
> in that respect which makes things a bit more difficult.

The key thing is to have a way to reset the state. So long as you have
that, you should be able to put networking back to the initial state
before running a test.

Regards,
Simon
Ilias Apalodimas Aug. 30, 2024, 9:27 a.m. UTC | #4
Hi Simon,

On Fri, 30 Aug 2024 at 03:59, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Jerome,
>
> On Thu, 29 Aug 2024 at 10:21, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
> >
> >
> >
> > On 8/29/24 16:05, Simon Glass wrote:
> > > Hi Jerome,
> > >
> > > On Fri, 23 Aug 2024 at 07:50, Jerome Forissier
> > > <jerome.forissier@linaro.org> wrote:
> > >>
> > >> Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
> > >> NET_LWIP so default to NET in this case.
> > >
> > > Sandbox needs to support NET_LWIP.
> >
> > I agree in principle, but AFAICT it is not a trivial task. Does it have to
> > be done in this series or can it be dealt with later?
>
> Does it build OK with sandbox? We use it for almost all of our feature
> development and testing, so I cannot imagine bringing this in if it
> doesn't work. How did you develop this feature?

sandbox is deeply rooted into the old network stack. It fakes ACK
responses, handles the ping request/replies with the sandbox ethernet
driver etc. I was looking on adding LWIP support for it, but it's not
trivial and I don't want us to add another hacky 'glue layer'. Since
LWIP is a big feature on its own -- and is working on real hardware
and QEMU, we discussed adding it, so people can test and contribute
fixes while we fix sandbox.

>
> >
> > I would need to look into this more closely to give details, but if I
> > remember correctly the sandbox tests expect a precise sequence of network
> > packets. For example when doing a TFTP tests, it expects an ARP packet to
> > go out first, but when lwIP is used the ARP might not go out because the IP
> > may already be in the cache. So the sandbox is definitely not a black box
> > in that respect which makes things a bit more difficult.
>
> The key thing is to have a way to reset the state. So long as you have
> that, you should be able to put networking back to the initial state
> before running a test.

Thanks
/Ilias
>
> Regards,
> Simon
Simon Glass Sept. 1, 2024, 8:09 p.m. UTC | #5
Hi Ilias,

On Fri, 30 Aug 2024 at 03:27, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
> On Fri, 30 Aug 2024 at 03:59, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Jerome,
> >
> > On Thu, 29 Aug 2024 at 10:21, Jerome Forissier
> > <jerome.forissier@linaro.org> wrote:
> > >
> > >
> > >
> > > On 8/29/24 16:05, Simon Glass wrote:
> > > > Hi Jerome,
> > > >
> > > > On Fri, 23 Aug 2024 at 07:50, Jerome Forissier
> > > > <jerome.forissier@linaro.org> wrote:
> > > >>
> > > >> Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
> > > >> NET_LWIP so default to NET in this case.
> > > >
> > > > Sandbox needs to support NET_LWIP.
> > >
> > > I agree in principle, but AFAICT it is not a trivial task. Does it have to
> > > be done in this series or can it be dealt with later?
> >
> > Does it build OK with sandbox? We use it for almost all of our feature
> > development and testing, so I cannot imagine bringing this in if it
> > doesn't work. How did you develop this feature?
>
> sandbox is deeply rooted into the old network stack. It fakes ACK
> responses, handles the ping request/replies with the sandbox ethernet
> driver etc. I was looking on adding LWIP support for it, but it's not
> trivial and I don't want us to add another hacky 'glue layer'. Since
> LWIP is a big feature on its own -- and is working on real hardware
> and QEMU, we discussed adding it, so people can test and contribute
> fixes while we fix sandbox.

At the very least it needs to build on sandbox.

I am not sure what tests LWIP has. The U-Boot networking tests are
very simple on sandbox. The test provides a handler to generate a
packet that it wants, then does a networking operation. It should not
be hard to plumb that through.

I am nervous about this going in with an intention to fix it up later...

>
> >
> > >
> > > I would need to look into this more closely to give details, but if I
> > > remember correctly the sandbox tests expect a precise sequence of network
> > > packets. For example when doing a TFTP tests, it expects an ARP packet to
> > > go out first, but when lwIP is used the ARP might not go out because the IP
> > > may already be in the cache. So the sandbox is definitely not a black box
> > > in that respect which makes things a bit more difficult.
> >
> > The key thing is to have a way to reset the state. So long as you have
> > that, you should be able to put networking back to the initial state
> > before running a test.

Regards,
Simon
Tom Rini Sept. 4, 2024, 10:32 p.m. UTC | #6
On Sun, Sep 01, 2024 at 02:09:43PM -0600, Simon Glass wrote:
> Hi Ilias,
> 
> On Fri, 30 Aug 2024 at 03:27, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Fri, 30 Aug 2024 at 03:59, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Jerome,
> > >
> > > On Thu, 29 Aug 2024 at 10:21, Jerome Forissier
> > > <jerome.forissier@linaro.org> wrote:
> > > >
> > > >
> > > >
> > > > On 8/29/24 16:05, Simon Glass wrote:
> > > > > Hi Jerome,
> > > > >
> > > > > On Fri, 23 Aug 2024 at 07:50, Jerome Forissier
> > > > > <jerome.forissier@linaro.org> wrote:
> > > > >>
> > > > >> Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
> > > > >> NET_LWIP so default to NET in this case.
> > > > >
> > > > > Sandbox needs to support NET_LWIP.
> > > >
> > > > I agree in principle, but AFAICT it is not a trivial task. Does it have to
> > > > be done in this series or can it be dealt with later?
> > >
> > > Does it build OK with sandbox? We use it for almost all of our feature
> > > development and testing, so I cannot imagine bringing this in if it
> > > doesn't work. How did you develop this feature?
> >
> > sandbox is deeply rooted into the old network stack. It fakes ACK
> > responses, handles the ping request/replies with the sandbox ethernet
> > driver etc. I was looking on adding LWIP support for it, but it's not
> > trivial and I don't want us to add another hacky 'glue layer'. Since
> > LWIP is a big feature on its own -- and is working on real hardware
> > and QEMU, we discussed adding it, so people can test and contribute
> > fixes while we fix sandbox.
> 
> At the very least it needs to build on sandbox.

It builds with sandbox, one of my tests is to switch the global default
to lwIP and make sure nothing blows up.

> I am not sure what tests LWIP has. The U-Boot networking tests are
> very simple on sandbox. The test provides a handler to generate a
> packet that it wants, then does a networking operation. It should not
> be hard to plumb that through.
> 
> I am nervous about this going in with an intention to fix it up later...

We have two flavours of tests. One is the sandbox-specific state
machines, and the other is functional testing. This series passes
functional testing while adding another functional test (for wget). I'm
quite fine with having sandbox figure out lwIP be a later TODO.
Simon Glass Sept. 19, 2024, 2:14 p.m. UTC | #7
Hi Tom,

On Thu, 5 Sept 2024 at 00:32, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Sep 01, 2024 at 02:09:43PM -0600, Simon Glass wrote:
> > Hi Ilias,
> >
> > On Fri, 30 Aug 2024 at 03:27, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, 30 Aug 2024 at 03:59, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Jerome,
> > > >
> > > > On Thu, 29 Aug 2024 at 10:21, Jerome Forissier
> > > > <jerome.forissier@linaro.org> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 8/29/24 16:05, Simon Glass wrote:
> > > > > > Hi Jerome,
> > > > > >
> > > > > > On Fri, 23 Aug 2024 at 07:50, Jerome Forissier
> > > > > > <jerome.forissier@linaro.org> wrote:
> > > > > >>
> > > > > >> Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
> > > > > >> NET_LWIP so default to NET in this case.
> > > > > >
> > > > > > Sandbox needs to support NET_LWIP.
> > > > >
> > > > > I agree in principle, but AFAICT it is not a trivial task. Does it have to
> > > > > be done in this series or can it be dealt with later?
> > > >
> > > > Does it build OK with sandbox? We use it for almost all of our feature
> > > > development and testing, so I cannot imagine bringing this in if it
> > > > doesn't work. How did you develop this feature?
> > >
> > > sandbox is deeply rooted into the old network stack. It fakes ACK
> > > responses, handles the ping request/replies with the sandbox ethernet
> > > driver etc. I was looking on adding LWIP support for it, but it's not
> > > trivial and I don't want us to add another hacky 'glue layer'. Since
> > > LWIP is a big feature on its own -- and is working on real hardware
> > > and QEMU, we discussed adding it, so people can test and contribute
> > > fixes while we fix sandbox.
> >
> > At the very least it needs to build on sandbox.
>
> It builds with sandbox, one of my tests is to switch the global default
> to lwIP and make sure nothing blows up.
>
> > I am not sure what tests LWIP has. The U-Boot networking tests are
> > very simple on sandbox. The test provides a handler to generate a
> > packet that it wants, then does a networking operation. It should not
> > be hard to plumb that through.
> >
> > I am nervous about this going in with an intention to fix it up later...
>
> We have two flavours of tests. One is the sandbox-specific state
> machines, and the other is functional testing. This series passes
> functional testing while adding another functional test (for wget). I'm
> quite fine with having sandbox figure out lwIP be a later TODO.

OK, well let's not leave it too long. It makes tests nice and simple
to write - just a function that can parse and generate any required
packets.

Also if someone can answer my first question (what tests LWIP has)
that would help.

Regards,
Simon
Jerome Forissier Sept. 24, 2024, 11:30 a.m. UTC | #8
On 9/19/24 16:14, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 5 Sept 2024 at 00:32, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Sun, Sep 01, 2024 at 02:09:43PM -0600, Simon Glass wrote:
>>> Hi Ilias,
>>>
>>> On Fri, 30 Aug 2024 at 03:27, Ilias Apalodimas
>>> <ilias.apalodimas@linaro.org> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On Fri, 30 Aug 2024 at 03:59, Simon Glass <sjg@chromium.org> wrote:
>>>>>
>>>>> Hi Jerome,
>>>>>
>>>>> On Thu, 29 Aug 2024 at 10:21, Jerome Forissier
>>>>> <jerome.forissier@linaro.org> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 8/29/24 16:05, Simon Glass wrote:
>>>>>>> Hi Jerome,
>>>>>>>
>>>>>>> On Fri, 23 Aug 2024 at 07:50, Jerome Forissier
>>>>>>> <jerome.forissier@linaro.org> wrote:
>>>>>>>>
>>>>>>>> Enable NET_LWIP by default for testing purposes. SANDBOX doesn't support
>>>>>>>> NET_LWIP so default to NET in this case.
>>>>>>>
>>>>>>> Sandbox needs to support NET_LWIP.
>>>>>>
>>>>>> I agree in principle, but AFAICT it is not a trivial task. Does it have to
>>>>>> be done in this series or can it be dealt with later?
>>>>>
>>>>> Does it build OK with sandbox? We use it for almost all of our feature
>>>>> development and testing, so I cannot imagine bringing this in if it
>>>>> doesn't work. How did you develop this feature?
>>>>
>>>> sandbox is deeply rooted into the old network stack. It fakes ACK
>>>> responses, handles the ping request/replies with the sandbox ethernet
>>>> driver etc. I was looking on adding LWIP support for it, but it's not
>>>> trivial and I don't want us to add another hacky 'glue layer'. Since
>>>> LWIP is a big feature on its own -- and is working on real hardware
>>>> and QEMU, we discussed adding it, so people can test and contribute
>>>> fixes while we fix sandbox.
>>>
>>> At the very least it needs to build on sandbox.
>>
>> It builds with sandbox, one of my tests is to switch the global default
>> to lwIP and make sure nothing blows up.
>>
>>> I am not sure what tests LWIP has. The U-Boot networking tests are
>>> very simple on sandbox. The test provides a handler to generate a
>>> packet that it wants, then does a networking operation. It should not
>>> be hard to plumb that through.
>>>
>>> I am nervous about this going in with an intention to fix it up later...
>>
>> We have two flavours of tests. One is the sandbox-specific state
>> machines, and the other is functional testing. This series passes
>> functional testing while adding another functional test (for wget). I'm
>> quite fine with having sandbox figure out lwIP be a later TODO.
> 
> OK, well let's not leave it too long. It makes tests nice and simple
> to write - just a function that can parse and generate any required
> packets.
> 
> Also if someone can answer my first question (what tests LWIP has)
> that would help.

There are unit and functional tests in lib/lwip/lwip/test but I have no
idea how to run them and even if they are run in some upstream CI.

That said, NET_LWIP is functionally tested via test/py, more specifically:

 test_net_dhcp()
 test_net_ping()
 test_net_tftpboot()
 test_efi_setup_dhcp()
 test_efi_helloworld_net_tftp()
 test_efi_helloworld_net_http()

Regards,
diff mbox series

Patch

diff --git a/Kconfig b/Kconfig
index 6657b9e5e30..2b316afa542 100644
--- a/Kconfig
+++ b/Kconfig
@@ -749,6 +749,7 @@  menu Networking
 
 choice
 	prompt "Networking stack"
+	default NET_LWIP if !SANDBOX
 	default NET
 
 config NO_NET