Re: [U-Boot] [PATCH v5 27/27] net: Improve error handling

On Wed, Mar 4, 2015 at 12:35 PM, Simon Glass sjg@chromium.org wrote:

Hi Joe,

On 3 March 2015 at 19:41, Joe Hershberger joe.hershberger@ni.com wrote:

...

Take a pass at plumbing errors through to the users of the network stack

Currently only the start() function errors will be returned from NetLoop(). recv() tends not to have errors, so that is likely not worth adding. send() certainly can return errors, but this patch does not attempt to plumb them yet. halt() is not expected to error.

Signed-off-by: Joe Hershberger joe.hershberger@ni.com

Nice patch!

Reviewed-by: Simon Glass sjg@chromium.org

...

---

Changes in v5: -New to v5

Changes in v4: None Changes in v3: None Changes in v2: None

include/net.h | 3 ++- net/eth.c | 56

++++++++++++++++++++++++++++++++++++++++++++++++--------

...

net/net.c | 26 ++++++++++++++++++-------- test/dm/eth.c | 4 ++-- 4 files changed, 70 insertions(+), 19 deletions(-)

[snip]

...

diff --git a/net/net.c b/net/net.c index afec443..69f38f7 100644 --- a/net/net.c +++ b/net/net.c @@ -84,6 +84,7 @@ #include <common.h> #include <command.h> #include <environment.h> +#include <errno.h> #include <net.h> #if defined(CONFIG_STATUS_LED) #include <miiphy.h> @@ -333,7 +334,7 @@ void net_init(void)

int NetLoop(enum proto_t protocol) {

•   int ret = -1;
  

•   int ret = -EINVAL;
  
    NetRestarted = 0;
    NetDevExists = 0;
  

@@ -345,9 +346,10 @@ int NetLoop(enum proto_t protocol) if (eth_is_on_demand_init() || protocol != NETCONS) { eth_halt(); eth_set_current();

•           if (eth_init() < 0) {
  

•           ret = eth_init();
  

•           if (ret < 0) {
                    eth_halt();
  

•                   return -1;
  

•                   return ret;
            }
    } else
            eth_init_state_only();
  

@@ -370,7 +372,7 @@ restart: case 1: /* network not configured */ eth_halt();

•           return -1;
  

•           return -ENODEV;
  
    case 2:
            /* network device not configured */
  

@@ -484,6 +486,8 @@ restart: /* * Check the ethernet for a new packet. The

ethernet

...

             *      receive routine will process it.

•            *      Most drivers return the most recent packet
  

size, but not

...

•            *      errors that may have happened.
             */
            eth_rx();
  
  

@@ -537,7 +541,7 @@ restart: }

            if (net_state == NETLOOP_FAIL)

•                   NetStartAgain();
  

•                   ret = NetStartAgain();
  

Where does this 'ret' get used?

This is part of the NetLoop() function, the top-most interface to the network stack in net.c.

If the net_state remains "NETLOOP_FAIL" after calling NetStartAgain, then the case structure following this if will "goto done".

At the end of done: is:

return ret;

We need to pass the return value out of NetStartAgain, since it can possibly call eth_ops->start() on a new device.

...

            switch (net_state) {

@@ -597,11 +601,12 @@ startAgainTimeout(void) net_set_state(NETLOOP_RESTART); }

-void NetStartAgain(void) +int NetStartAgain(void) { char *nretry; int retry_forever = 0; unsigned long retrycnt = 0;

•   int ret;
  
    nretry = getenv("netretry");
    if (nretry) {
  

@@ -621,7 +626,11 @@ void NetStartAgain(void) if ((!retry_forever) && (NetTryCount >= retrycnt)) { eth_halt(); net_set_state(NETLOOP_FAIL);

•           return;
  

•           /*
  

•            * We don't provide a way for the protocol to return an
  

error,

...

•            * but this is almost always the reason.
  

•            */
  

•           return -ETIMEDOUT;
    }
  
    NetTryCount++;
  

@@ -630,7 +639,7 @@ void NetStartAgain(void) #if !defined(CONFIG_NET_DO_NOT_TRY_ANOTHER) eth_try_another(!NetRestarted); #endif

•   eth_init();
  

•   ret = eth_init();
    if (NetRestartWrap) {
            NetRestartWrap = 0;
            if (NetDevExists) {
  

@@ -642,6 +651,7 @@ void NetStartAgain(void) } else { net_set_state(NETLOOP_RESTART); }

•   return ret;
  

}

/**********************************************************************/

[snip]