On Wed, Dec 16, 2015 at 03:45:34PM +0200, Petri Savolainen wrote:
> From: Matias Elo <matias....@nokia.com>
>
> Added a separate function for determining if the netmap link
> is up.
>
> Signed-off-by: Matias Elo <matias....@nokia.com>
> ---
> platform/linux-generic/pktio/netmap.c | 48
> +++++++++++++++++++++++++----------
> 1 file changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/platform/linux-generic/pktio/netmap.c
> b/platform/linux-generic/pktio/netmap.c
> index e3aa7f1..544afaa 100644
> --- a/platform/linux-generic/pktio/netmap.c
> +++ b/platform/linux-generic/pktio/netmap.c
> @@ -66,7 +66,7 @@ static int netmap_do_ioctl(pktio_entry_t *pktio_entry,
> unsigned long cmd,
> break;
> case SIOCETHTOOL:
> if (subcmd == ETHTOOL_GLINK)
> - return !eval.data;
> + return eval.data;
> break;
> default:
> break;
> @@ -259,6 +259,38 @@ static int netmap_close(pktio_entry_t *pktio_entry)
> return 0;
> }
>
> +/**
> + * Determine netmap link status
This description nor the function name match what it actually does.
> + *
> + * @param pktio_entry Packet IO handle
A packet IO handle would be an odp_pktio_t rather than a pktio_entry_t
> + *
> + * @retval 1 link is up
> + * @retval 0 link is down
> + * @retval <0 on failure
> + */
> +static inline int netmap_link_status(pktio_entry_t *pktio_entry)
> +{
> + int i;
> + int ret;
> +
> + /* Wait for the link to come up */
> + for (i = 0; i < NM_OPEN_RETRIES; i++) {
> + ret = netmap_do_ioctl(pktio_entry, SIOCETHTOOL, ETHTOOL_GLINK);
> + if (ret == -1)
> + return -1;
> + /* nm_open() causes the physical link to reset. When using a
> + * direct attached loopback cable there may be a small delay
> + * until the opposing end's interface comes back up again. In
> + * this case without the additional sleep pktio validation
> + * tests fail. */
> + sleep(1);
> + if (ret == 1)
> + return 1;
> + }
> + ODP_DBG("%s link is down\n", pktio_entry->s.name);
> + return 0;
> +}
> +
I think it's better to leave the sleep/retry logic in netmap_open(), or
in another function named netmap_wait_for_link() or some such, and have
netmap_link_status() just return the link status (I'm assuming the
intention is for this to used later for the odp_pktio_link_status() API).
> static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry,
> const char *netdev, odp_pool_t pool)
> {
> @@ -359,7 +391,6 @@ static int netmap_start(pktio_entry_t *pktio_entry)
> pkt_netmap_t *pkt_nm = &pktio_entry->s.pkt_nm;
> netmap_ring_t *desc_ring;
> struct nm_desc base_desc;
> - int err;
> unsigned i;
> unsigned j;
> uint64_t flags;
> @@ -434,18 +465,7 @@ static int netmap_start(pktio_entry_t *pktio_entry)
> }
> }
> /* Wait for the link to come up */
> - for (i = 0; i < NM_OPEN_RETRIES; i++) {
> - err = netmap_do_ioctl(pktio_entry, SIOCETHTOOL, ETHTOOL_GLINK);
> - /* nm_open() causes the physical link to reset. When using a
> - * direct attached loopback cable there may be a small delay
> - * until the opposing end's interface comes back up again. In
> - * this case without the additional sleep pktio validation
> - * tests fail. */
> - sleep(1);
> - if (err == 0)
> - return 0;
> - }
> - ODP_ERR("%s didn't come up\n", pktio_entry->s.name);
> + return (netmap_link_status(pktio_entry) == 1) ? 0 : -1;
>
> error:
> netmap_close(pktio_entry);
> --
> 2.6.3
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
