Re: [U-Boot] Rework the network stack

[Joe Hershberger]

On Mon, Mar 23, 2015 at 3:04 PM, Simon Glass s...@chromium.org wrote:

 Hi,

 On 23 March 2015 at 13:55, Jörg Krause joerg.krause@embedded.rocks
wrote:
 
  Joe, Simon,
 
  On Mo, 2015-03-23 at 10:46 -0600, Simon Glass wrote:
   Hi Jörg,
  
   On 22 March 2015 at 14:37, Jörg Krause joerg.krause@embedded.rocks
wrote:
Hi Joe,

8--snip

I had a look at the patches and the code base is not entirely the
same.
But maybe we should just stick to the crucial points of the new
network
stack:
   
1) A network device gets initialized when it becomes the current
one and
gets brought down when it's not used anymore or prior booting to
Linux
so the connection remains active all the time.

I think that's a reasonable goal.

My thoughts about this one:
Bringing the device down can be done in eth_unregister().
Initializing
for the current device can be done in eth_current_changed(). What I
like
even better is to make eth_current_changed() the sole place for
calling
init() and halt(). It would have to be extend to take at least two
parameters, eth_current and new_current. If eth_current is null just
bring up the new device, if new_current is null just bring down the
current device.

This would fit into the driver model implementation differently. I'm not
sure we care to change it in the old Ethernet support. All the more reason
to upgrade to the DM implementation.

2) Replace the NetLoop by a connection list where each protocol
uses a
connection to send and receive packets.

I wonder if that is more complex than needed. It is the case that the only
concurrent operations we need is NetConsole and one other operation. ARP is
already handled in sequence and should not need to be considered concurrent
(even though it could be). Perhaps we can find a way to allow NetConsole to
operate outside of the typical packet handler'.

This induce to port all protocols to use the new network, of cause.

It would be nice to not need to change all protocols.

In my opinion it would be also good to do some clean ups and
restructuring of code. Put more functions into net-utils, is there a
need for eth_init and so on.

I certainly agree that there are more clean-ups to be done. I don't agree
that they should be connected to this change. The more meaningless renames
we have in the series the harder it is to see and grok the substantive
changes. This is very apparent in Sascha's patch series. It would be much
easier to read / port if not clouded by essentially unrelated refactoring.

   Joe has done a lot of work to move U-Boot's network stack over to
   driver model. This is now at u-boot-dm/next waiting for the merge
   window to open.
  
   The connection list idea does not sound like a hugely complex change.
   Are you thinking of trying it out?

Let's try to start simpler. We can refactor further if needed.

  I have just noticed today the big series of u-boot-dm patches of Joe. I
  will try it out. But before I will start I want to discuss the concept.
  What do you think about my thoughts about bringing up and down a network
  device?

I think it can be done when probed. In fact I'm not sure there's a reason
more than one cannot be started at a time. We can certainly have more than
one probed at a time. We just need to remove them before booting to
Linux. eth_pre_remove() already calls eth_get_ops(dev)-stop(dev).

 Joe is the expert here.

 With driver model the actual bring-up should be automatic (i.e. it is
 probed as soon as it is used).

As long as we move the call to eth_get_ops(current)-start(current); to the
probe function, then this is true.

 Regards,
 Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Rework the network stack

[Simon Glass]

Hi Jörg,

On 22 March 2015 at 14:37, Jörg Krause joerg.krause@embedded.rocks wrote:
 Hi Joe,

 On Sa, 2015-03-21 at 22:59 -0500, Joe Hershberger wrote:
 Hi Jörg,

 On Sat, Mar 21, 2015 at 3:33 AM, Jörg Krause joerg.krause@embedded.rocks
 wrote:
 
  Hi all,
 
  there is an issue with the current network stack using netconsole. It's
  impossible to use network commands as TFTP inside netconsole, because
  they try to run as atomic network commands.
 
  The issue was already reported by Stefano Babic in 2010:
  [U-Boot] NetConsole and network API
  http://lists.denx.de/pipermail/u-boot/2010-August/075535.html

 I worked around this problem here:

 http://lists.denx.de/pipermail/u-boot/2012-August/129913.html

 I know about this patch, and I understand the problem it tries to
 workaround. But it does not work for using netconsole with another
 protocol like ping. When for example ping enters the NetLoop() it will
 halt the network device.

 The problem for this is here:
 if (eth_is_on_demand_init() || protocol != NETCONS) {
 eth_halt();
 ...
 }

 eth_is_on_demand_init() returns correctly with false, because PING !=
 NETCONS. But the second expression results in true, because PING !=
 NETCONS.

  I run into the same problem:
  [U-Boot] netconsole: USB Ethernet connection dropping with ping or
  tftpboot
  http://lists.denx.de/pipermail/u-boot/2015-February/203838.html

 I didn't understand what about your case was not able to work given the
 workaround I implemented previously. What was different about it?

  I have looked at the current network stack. The stack is based on the
  concept of atomic network commands. The implementation for netconsole
  looks very confusing.

 There is no doubt that netconsole is quite confusing as it exists today.

 I tried to fix the issue, but it did not work properly.

  Sascha Hauer has reimplemented the network stack for Barebox:
  http://www.spinics.net/lists/u-boot-v2/msg00914.html
 
  Looking at the current implementation of net.c looks very clean and
  well-designed.

 Thanks for pointing this out. I hadn't gone to look at the network stack in
 barebox.

  What do you think about porting this to U-Boot?

 I can look into this. Naturally there are many other changes to u-boot
 network stack since the time barebox forked, so I expect such a port to be
 very intensive... most likely a near complete rewrite of Sascha's series,
 but I will investigate further.

 I had a look at the patches and the code base is not entirely the same.
 But maybe we should just stick to the crucial points of the new network
 stack:

 1) A network device gets initialized when it becomes the current one and
 gets brought down when it's not used anymore or prior booting to Linux
 so the connection remains active all the time.

 My thoughts about this one:
 Bringing the device down can be done in eth_unregister(). Initializing
 for the current device can be done in eth_current_changed(). What I like
 even better is to make eth_current_changed() the sole place for calling
 init() and halt(). It would have to be extend to take at least two
 parameters, eth_current and new_current. If eth_current is null just
 bring up the new device, if new_current is null just bring down the
 current device.

 2) Replace the NetLoop by a connection list where each protocol uses a
 connection to send and receive packets.

 This induce to port all protocols to use the new network, of cause.

 In my opinion it would be also good to do some clean ups and
 restructuring of code. Put more functions into net-utils, is there a
 need for eth_init and so on.

Joe has done a lot of work to move U-Boot's network stack over to
driver model. This is now at u-boot-dm/next waiting for the merge
window to open.

The connection list idea does not sound like a hugely complex change.
Are you thinking of trying it out?

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Rework the network stack

[Simon Glass]

Hi,

On 23 March 2015 at 13:55, Jörg Krause joerg.krause@embedded.rocks wrote:

 Joe, Simon,

 On Mo, 2015-03-23 at 10:46 -0600, Simon Glass wrote:
  Hi Jörg,
 
  On 22 March 2015 at 14:37, Jörg Krause joerg.krause@embedded.rocks wrote:
   Hi Joe,
  
   On Sa, 2015-03-21 at 22:59 -0500, Joe Hershberger wrote:
   Hi Jörg,
  
   On Sat, Mar 21, 2015 at 3:33 AM, Jörg Krause 
   joerg.krause@embedded.rocks
   wrote:
   
Hi all,
   
there is an issue with the current network stack using netconsole. It's
impossible to use network commands as TFTP inside netconsole, because
they try to run as atomic network commands.
   
The issue was already reported by Stefano Babic in 2010:
[U-Boot] NetConsole and network API
http://lists.denx.de/pipermail/u-boot/2010-August/075535.html
  
   I worked around this problem here:
  
   http://lists.denx.de/pipermail/u-boot/2012-August/129913.html
  
   I know about this patch, and I understand the problem it tries to
   workaround. But it does not work for using netconsole with another
   protocol like ping. When for example ping enters the NetLoop() it will
   halt the network device.
  
   The problem for this is here:
   if (eth_is_on_demand_init() || protocol != NETCONS) {
   eth_halt();
   ...
   }
  
   eth_is_on_demand_init() returns correctly with false, because PING !=
   NETCONS. But the second expression results in true, because PING !=
   NETCONS.
  
I run into the same problem:
[U-Boot] netconsole: USB Ethernet connection dropping with ping or
tftpboot
http://lists.denx.de/pipermail/u-boot/2015-February/203838.html
  
   I didn't understand what about your case was not able to work given the
   workaround I implemented previously. What was different about it?
  
I have looked at the current network stack. The stack is based on the
concept of atomic network commands. The implementation for netconsole
looks very confusing.
  
   There is no doubt that netconsole is quite confusing as it exists today.
  
   I tried to fix the issue, but it did not work properly.
  
Sascha Hauer has reimplemented the network stack for Barebox:
http://www.spinics.net/lists/u-boot-v2/msg00914.html
   
Looking at the current implementation of net.c looks very clean and
well-designed.
  
   Thanks for pointing this out. I hadn't gone to look at the network stack 
   in
   barebox.
  
What do you think about porting this to U-Boot?
  
   I can look into this. Naturally there are many other changes to u-boot
   network stack since the time barebox forked, so I expect such a port to 
   be
   very intensive... most likely a near complete rewrite of Sascha's series,
   but I will investigate further.
  
   I had a look at the patches and the code base is not entirely the same.
   But maybe we should just stick to the crucial points of the new network
   stack:
  
   1) A network device gets initialized when it becomes the current one and
   gets brought down when it's not used anymore or prior booting to Linux
   so the connection remains active all the time.
  
   My thoughts about this one:
   Bringing the device down can be done in eth_unregister(). Initializing
   for the current device can be done in eth_current_changed(). What I like
   even better is to make eth_current_changed() the sole place for calling
   init() and halt(). It would have to be extend to take at least two
   parameters, eth_current and new_current. If eth_current is null just
   bring up the new device, if new_current is null just bring down the
   current device.
  
   2) Replace the NetLoop by a connection list where each protocol uses a
   connection to send and receive packets.
  
   This induce to port all protocols to use the new network, of cause.
  
   In my opinion it would be also good to do some clean ups and
   restructuring of code. Put more functions into net-utils, is there a
   need for eth_init and so on.
 
  Joe has done a lot of work to move U-Boot's network stack over to
  driver model. This is now at u-boot-dm/next waiting for the merge
  window to open.
 
  The connection list idea does not sound like a hugely complex change.
  Are you thinking of trying it out?

 I have just noticed today the big series of u-boot-dm patches of Joe. I
 will try it out. But before I will start I want to discuss the concept.
 What do you think about my thoughts about bringing up and down a network
 device?

Joe is the expert here.

With driver model the actual bring-up should be automatic (i.e. it is
probed as soon as it is used).

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Rework the network stack

[Jörg Krause]

Joe, Simon,

On Mo, 2015-03-23 at 10:46 -0600, Simon Glass wrote:
 Hi Jörg,
 
 On 22 March 2015 at 14:37, Jörg Krause joerg.krause@embedded.rocks wrote:
  Hi Joe,
 
  On Sa, 2015-03-21 at 22:59 -0500, Joe Hershberger wrote:
  Hi Jörg,
 
  On Sat, Mar 21, 2015 at 3:33 AM, Jörg Krause joerg.krause@embedded.rocks
  wrote:
  
   Hi all,
  
   there is an issue with the current network stack using netconsole. It's
   impossible to use network commands as TFTP inside netconsole, because
   they try to run as atomic network commands.
  
   The issue was already reported by Stefano Babic in 2010:
   [U-Boot] NetConsole and network API
   http://lists.denx.de/pipermail/u-boot/2010-August/075535.html
 
  I worked around this problem here:
 
  http://lists.denx.de/pipermail/u-boot/2012-August/129913.html
 
  I know about this patch, and I understand the problem it tries to
  workaround. But it does not work for using netconsole with another
  protocol like ping. When for example ping enters the NetLoop() it will
  halt the network device.
 
  The problem for this is here:
  if (eth_is_on_demand_init() || protocol != NETCONS) {
  eth_halt();
  ...
  }
 
  eth_is_on_demand_init() returns correctly with false, because PING !=
  NETCONS. But the second expression results in true, because PING !=
  NETCONS.
 
   I run into the same problem:
   [U-Boot] netconsole: USB Ethernet connection dropping with ping or
   tftpboot
   http://lists.denx.de/pipermail/u-boot/2015-February/203838.html
 
  I didn't understand what about your case was not able to work given the
  workaround I implemented previously. What was different about it?
 
   I have looked at the current network stack. The stack is based on the
   concept of atomic network commands. The implementation for netconsole
   looks very confusing.
 
  There is no doubt that netconsole is quite confusing as it exists today.
 
  I tried to fix the issue, but it did not work properly.
 
   Sascha Hauer has reimplemented the network stack for Barebox:
   http://www.spinics.net/lists/u-boot-v2/msg00914.html
  
   Looking at the current implementation of net.c looks very clean and
   well-designed.
 
  Thanks for pointing this out. I hadn't gone to look at the network stack in
  barebox.
 
   What do you think about porting this to U-Boot?
 
  I can look into this. Naturally there are many other changes to u-boot
  network stack since the time barebox forked, so I expect such a port to be
  very intensive... most likely a near complete rewrite of Sascha's series,
  but I will investigate further.
 
  I had a look at the patches and the code base is not entirely the same.
  But maybe we should just stick to the crucial points of the new network
  stack:
 
  1) A network device gets initialized when it becomes the current one and
  gets brought down when it's not used anymore or prior booting to Linux
  so the connection remains active all the time.
 
  My thoughts about this one:
  Bringing the device down can be done in eth_unregister(). Initializing
  for the current device can be done in eth_current_changed(). What I like
  even better is to make eth_current_changed() the sole place for calling
  init() and halt(). It would have to be extend to take at least two
  parameters, eth_current and new_current. If eth_current is null just
  bring up the new device, if new_current is null just bring down the
  current device.
 
  2) Replace the NetLoop by a connection list where each protocol uses a
  connection to send and receive packets.
 
  This induce to port all protocols to use the new network, of cause.
 
  In my opinion it would be also good to do some clean ups and
  restructuring of code. Put more functions into net-utils, is there a
  need for eth_init and so on.
 
 Joe has done a lot of work to move U-Boot's network stack over to
 driver model. This is now at u-boot-dm/next waiting for the merge
 window to open.
 
 The connection list idea does not sound like a hugely complex change.
 Are you thinking of trying it out?

I have just noticed today the big series of u-boot-dm patches of Joe. I
will try it out. But before I will start I want to discuss the concept. 
What do you think about my thoughts about bringing up and down a network
device?

Best regards,
Jörg

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Rework the network stack

[Jörg Krause]

Hi Joe,

On Sa, 2015-03-21 at 22:59 -0500, Joe Hershberger wrote:
 Hi Jörg,
 
 On Sat, Mar 21, 2015 at 3:33 AM, Jörg Krause joerg.krause@embedded.rocks
 wrote:
 
  Hi all,
 
  there is an issue with the current network stack using netconsole. It's
  impossible to use network commands as TFTP inside netconsole, because
  they try to run as atomic network commands.
 
  The issue was already reported by Stefano Babic in 2010:
  [U-Boot] NetConsole and network API
  http://lists.denx.de/pipermail/u-boot/2010-August/075535.html
 
 I worked around this problem here:
 
 http://lists.denx.de/pipermail/u-boot/2012-August/129913.html

I know about this patch, and I understand the problem it tries to
workaround. But it does not work for using netconsole with another
protocol like ping. When for example ping enters the NetLoop() it will
halt the network device.

The problem for this is here:
if (eth_is_on_demand_init() || protocol != NETCONS) {
eth_halt();
...
}

eth_is_on_demand_init() returns correctly with false, because PING !=
NETCONS. But the second expression results in true, because PING !=
NETCONS.

  I run into the same problem:
  [U-Boot] netconsole: USB Ethernet connection dropping with ping or
  tftpboot
  http://lists.denx.de/pipermail/u-boot/2015-February/203838.html
 
 I didn't understand what about your case was not able to work given the
 workaround I implemented previously. What was different about it?
 
  I have looked at the current network stack. The stack is based on the
  concept of atomic network commands. The implementation for netconsole
  looks very confusing.
 
 There is no doubt that netconsole is quite confusing as it exists today.

I tried to fix the issue, but it did not work properly.

  Sascha Hauer has reimplemented the network stack for Barebox:
  http://www.spinics.net/lists/u-boot-v2/msg00914.html
 
  Looking at the current implementation of net.c looks very clean and
  well-designed.
 
 Thanks for pointing this out. I hadn't gone to look at the network stack in
 barebox.
 
  What do you think about porting this to U-Boot?
 
 I can look into this. Naturally there are many other changes to u-boot
 network stack since the time barebox forked, so I expect such a port to be
 very intensive... most likely a near complete rewrite of Sascha's series,
 but I will investigate further.

I had a look at the patches and the code base is not entirely the same.
But maybe we should just stick to the crucial points of the new network
stack:

1) A network device gets initialized when it becomes the current one and
gets brought down when it's not used anymore or prior booting to Linux
so the connection remains active all the time.

My thoughts about this one:
Bringing the device down can be done in eth_unregister(). Initializing
for the current device can be done in eth_current_changed(). What I like
even better is to make eth_current_changed() the sole place for calling
init() and halt(). It would have to be extend to take at least two
parameters, eth_current and new_current. If eth_current is null just
bring up the new device, if new_current is null just bring down the
current device.

2) Replace the NetLoop by a connection list where each protocol uses a
connection to send and receive packets.

This induce to port all protocols to use the new network, of cause.

In my opinion it would be also good to do some clean ups and
restructuring of code. Put more functions into net-utils, is there a
need for eth_init and so on.

Best regards
Jörg Krause

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] Rework the network stack

[Jörg Krause]

Hi all,

there is an issue with the current network stack using netconsole. It's
impossible to use network commands as TFTP inside netconsole, because
they try to run as atomic network commands.

The issue was already reported by Stefano Babic in 2010:
[U-Boot] NetConsole and network API
http://lists.denx.de/pipermail/u-boot/2010-August/075535.html

I run into the same problem:
[U-Boot] netconsole: USB Ethernet connection dropping with ping or
tftpboot
http://lists.denx.de/pipermail/u-boot/2015-February/203838.html

I have looked at the current network stack. The stack is based on the
concept of atomic network commands. The implementation for netconsole
looks very confusing.

Sascha Hauer has reimplemented the network stack for Barebox:
http://www.spinics.net/lists/u-boot-v2/msg00914.html

Looking at the current implementation of net.c looks very clean and
well-designed.

What do you think about porting this to U-Boot?

Best regards
Jörg Krause

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Rework the network stack

[Joe Hershberger]

Hi Jörg,

On Sat, Mar 21, 2015 at 3:33 AM, Jörg Krause joerg.krause@embedded.rocks
wrote:

 Hi all,

 there is an issue with the current network stack using netconsole. It's
 impossible to use network commands as TFTP inside netconsole, because
 they try to run as atomic network commands.

 The issue was already reported by Stefano Babic in 2010:
 [U-Boot] NetConsole and network API
 http://lists.denx.de/pipermail/u-boot/2010-August/075535.html

I worked around this problem here:

http://lists.denx.de/pipermail/u-boot/2012-August/129913.html

 I run into the same problem:
 [U-Boot] netconsole: USB Ethernet connection dropping with ping or
 tftpboot
 http://lists.denx.de/pipermail/u-boot/2015-February/203838.html

I didn't understand what about your case was not able to work given the
workaround I implemented previously. What was different about it?

 I have looked at the current network stack. The stack is based on the
 concept of atomic network commands. The implementation for netconsole
 looks very confusing.

There is no doubt that netconsole is quite confusing as it exists today.

 Sascha Hauer has reimplemented the network stack for Barebox:
 http://www.spinics.net/lists/u-boot-v2/msg00914.html

 Looking at the current implementation of net.c looks very clean and
 well-designed.

Thanks for pointing this out. I hadn't gone to look at the network stack in
barebox.

 What do you think about porting this to U-Boot?

I can look into this. Naturally there are many other changes to u-boot
network stack since the time barebox forked, so I expect such a port to be
very intensive... most likely a near complete rewrite of Sascha's series,
but I will investigate further.

Cheers,
-Joe
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot