| Message ID | 1629f6845b448edcdead09a9350f1ae2ae7a4bb6.1724419624.git.jerome.forissier@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce the lwIP network stack | expand |
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
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,
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
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
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
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.
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
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(+)