Hi,
Please see below some comments,
On Sat, Apr 01, 2017 at 04:03:16PM +0800, Wei Dai wrote:
>[...]
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index 79efaaa..d61bb27 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -4630,26 +4630,30 @@ mlx4_mac_addr_remove(struct rte_eth_dev *dev,
> uint32_t index)
> * @param vmdq
> * VMDq pool index to associate address with (ignored).
> */
> -static void
> +static int
Please keep function documentation up to date.
> mlx4_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
> uint32_t index, uint32_t vmdq)
> {
> struct priv *priv = dev->data->dev_private;
> + int re;
>
> if (mlx4_is_secondary())
> - return;
> + return -1;
Should not it return ENOSUP instead?
> (void)vmdq;
> priv_lock(priv);
> DEBUG("%p: adding MAC address at index %" PRIu32,
> (void *)dev, index);
> /* Last array entry is reserved for broadcast. */
> - if (index >= (elemof(priv->mac) - 1))
> - goto end;
> - priv_mac_addr_add(priv, index,
> + if (index >= (elemof(priv->mac) - 1)) {
> + priv_unlock(priv);
> + return -2;
> + }
> + re = priv_mac_addr_add(priv, index,
> (const uint8_t (*)[ETHER_ADDR_LEN])
> mac_addr->addr_bytes);
You have an issue here, internal function of mlx drivers return
positives errno (as described in their functions documentation).
It should be negated before returning.
> end:
> priv_unlock(priv);
> + return re;
> }
>
> /**
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index 879da5e..cca7a36 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -229,8 +229,8 @@ int hash_rxq_mac_addrs_add(struct hash_rxq *);
> int priv_mac_addr_add(struct priv *, unsigned int,
> const uint8_t (*)[ETHER_ADDR_LEN]);
> int priv_mac_addrs_enable(struct priv *);
> -void mlx5_mac_addr_add(struct rte_eth_dev *, struct ether_addr *, uint32_t,
> - uint32_t);
> +int mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
> + uint32_t index, uint32_t vmdq);
> void mlx5_mac_addr_set(struct rte_eth_dev *, struct ether_addr *);
>
> /* mlx5_rss.c */
> diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
> index 4fcfd3b..db93f4e 100644
> --- a/drivers/net/mlx5/mlx5_mac.c
> +++ b/drivers/net/mlx5/mlx5_mac.c
> @@ -470,26 +470,30 @@ priv_mac_addrs_enable(struct priv *priv)
> * @param vmdq
> * VMDq pool index to associate address with (ignored).
> */
> -void
> +int
> mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
> uint32_t index, uint32_t vmdq)
> {
> struct priv *priv = dev->data->dev_private;
> + int re;
>
> if (mlx5_is_secondary())
> - return;
> + return -1;
>
> (void)vmdq;
> priv_lock(priv);
> DEBUG("%p: adding MAC address at index %" PRIu32,
> (void *)dev, index);
> - if (index >= RTE_DIM(priv->mac))
> + if (index >= RTE_DIM(priv->mac)) {
> + re = -2;
> goto end;
> - priv_mac_addr_add(priv, index,
> + }
> + re = priv_mac_addr_add(priv, index,
> (const uint8_t (*)[ETHER_ADDR_LEN])
> mac_addr->addr_bytes);
> end:
> priv_unlock(priv);
> + return re;
> }
>[...]
Same remarks here,
Thanks,
--
Nélio Laranjeiro
6WIND
