From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f194.google.com (mail-wr0-f194.google.com [209.85.128.194]) by dpdk.org (Postfix) with ESMTP id D79DF1B1DA for ; Fri, 16 Feb 2018 15:26:55 +0100 (CET) Received: by mail-wr0-f194.google.com with SMTP id v65so3062538wrc.11 for ; Fri, 16 Feb 2018 06:26:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=tpyVr8DPbD3ft5B75kNbwnh408jecacK2rIurphANHQ=; b=GxEN6uRHrg319xjGaFE6jHW8kp08n9z3q+67zUr6VF0g/j/tL4+cMudkcptkyAbYtl rtG8QBbvlLtTwjHQOnYzD2obqYMs0xMoBzpvof6yyx5TVMYvSjZs/lYl0+8U6RedQbCa uALFIdE332Oe8kT2ZGZmYZHVnCeZj5Fb9APoCc8mWaxKcLdkXPzO/X9RVoKPks0UgarS at5fq8iUtMK6LkUcpSYadfJ23x7cZosrbV8b3ahDfwwh0v+doXhcm452aA08HeObYI9q hblIlf403GsjSArGopzPosmKRvabDPsWTcX3cCV7p3c5tbOoXgeEiE0O/g4WmujQRhHr /3wg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=tpyVr8DPbD3ft5B75kNbwnh408jecacK2rIurphANHQ=; b=PwmMnCPoIO7nrLZO2RO68r7sNerYq7+u7NBKY0+Iysa8mFk1/sHmxMRd3y8N8xaW/X SzI3WDuPwQscJ5xHD56YNY7O2Bi87i1rBQIWxuOr+GlBb6ppNJ54evCED52JQEi+hIXQ Dkikh8kBhrEZKKM+tZ3ldd+ZPN3AOUMIy4bvlREUUR/Iqn/TCgB9bpJubmaitHiZLS3F Z50MQkYnpK8fyamXYkkL2ab0W9Dm7YAUSLxTHaAGePB0ghArWmuOiLnutNKUBj0JBfGs 8/rGyFknY1LYE2wwOlPzmBTJX6UUMRfKjtC599j4uaPUPRnboV2eRkCo0Fu6eGbhglrj oLmw== X-Gm-Message-State: APf1xPC78P61M/wm/j9AM/ckRsaeDCYWSGy2sPz8FJzA/llBRteG11m9 T9olEvlaTuwfgZscb4ENHHut8jZh X-Google-Smtp-Source: AH8x226UWbQXpg95HvclxvoFsMVpQ8U0bNWO6fllvvKUPb7312dbbyzEU6Hmf54jcMJWk93vYov0vw== X-Received: by 10.223.195.147 with SMTP id p19mr6089577wrf.224.1518791215561; Fri, 16 Feb 2018 06:26:55 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id v72sm15574845wmd.12.2018.02.16.06.26.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Feb 2018 06:26:54 -0800 (PST) Date: Fri, 16 Feb 2018 15:26:42 +0100 From: Adrien Mazarguil To: Nelio Laranjeiro Cc: dev@dpdk.org, Yongseok Koh Message-ID: <20180216142642.GF4256@6wind.com> References: <3c6d213784a58de85b30ff5053b0c35bcfd33b57.1518686930.git.nelio.laranjeiro@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3c6d213784a58de85b30ff5053b0c35bcfd33b57.1518686930.git.nelio.laranjeiro@6wind.com> Subject: Re: [dpdk-dev] [PATCH 2/3] net/mlx5: convert return errno to negative ones X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Feb 2018 14:26:56 -0000 How about reusing the title/commit log of its mlx4 counterpart: net/mlx5: standardize on negative errno values (see 9d14b27308a0 "net/mlx4: standardize on negative errno values") More below. On Thu, Feb 15, 2018 at 10:29:26AM +0100, Nelio Laranjeiro wrote: > Signed-off-by: Nelio Laranjeiro > Acked-by: Yongseok Koh > --- > drivers/net/mlx5/mlx5.c | 19 ++++----- > drivers/net/mlx5/mlx5_ethdev.c | 30 ++++++++------ > drivers/net/mlx5/mlx5_flow.c | 92 +++++++++++++++++++++-------------------- > drivers/net/mlx5/mlx5_mac.c | 7 ++-- > drivers/net/mlx5/mlx5_mr.c | 4 +- > drivers/net/mlx5/mlx5_rss.c | 16 +++---- > drivers/net/mlx5/mlx5_rxq.c | 20 ++++----- > drivers/net/mlx5/mlx5_socket.c | 41 ++++++++++++------ > drivers/net/mlx5/mlx5_trigger.c | 27 ++++++------ > drivers/net/mlx5/mlx5_txq.c | 14 +++---- > drivers/net/mlx5/mlx5_vlan.c | 2 +- > 11 files changed, 145 insertions(+), 127 deletions(-) > > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c > index f52edf74f..d24f2a37c 100644 > --- a/drivers/net/mlx5/mlx5.c > +++ b/drivers/net/mlx5/mlx5.c > @@ -413,7 +413,7 @@ mlx5_args_check(const char *key, const char *val, void *opaque) > * Device arguments structure. > * > * @return > - * 0 on success, errno value on failure. > + * 0 on success, negative errno value on failure. How about s/on \(failure\|error\)/otherwise/ here and everywhere else? Documentation should be identical for all relevant functions. In mlx5_ethdev.c, priv_get_ifname() is still documented to return "0 on success, -1 on failure and errno is set". You must get rid of the reliance on external errno as part of this commit; you can optionally set rte_errno, but for consistency all int-returning functions must return a valid negative errno value, never -1. The same applies to: - priv_sysfs_read - priv_sysfs_write - priv_get_sysfs_ulong - priv_set_sysfs_ulong - priv_ifreq - priv_get_num_vfs - priv_get_mtu - priv_get_cntr_sysfs - priv_set_mtu - priv_set_flags - mlx5_link_update (lacks documentation) - mlx5_ibv_device_to_pci_addr - priv_dev_set_link - mlx5_flow_item_validate - mlx5_flow_create_* (unsure) - priv_rx_intr_vec_enable ("negative" what?) - mlx5_rx_intr_enable (ditto) - mlx5_rx_intr_disable (ditto) - check_cqe (returning a valid errno wouldn't impact performance) - priv_read_dev_counters ("negative" what?) - priv_ethtool_get_stats_n - priv_xstats_get ("negative" what?) - mlx5_stats_get (lacks documentation) - mlx5_xstats_get ("negative" what?) - mlx5_xstats_get_names (lacks documentation) - priv_dev_traffic_disable (no error defined?) - mlx5_priv_txq_ibv_releasable (lacks documentation) - mlx5_priv_txq_ibv_verify (ditto) - mlx5_vlan_offload_set (ditto, to be checked) Also, some of them additionally set rte_errno while most of them do not. I'd suggest to *always* set rte_errno in case of error then add the "...and rte_errno is set" to documentation. mlx4 approach to errors: if (boom) { rte_errno = ECRAP; return -rte_errno; } Shorter, also valid but frowned upon: if (boom) return -(rte_errno = EBOOM); Alternatively when calling another rte_errno-aware function: ret = boom(); if (ret) return ret; -- Adrien Mazarguil 6WIND