Hi,
note, please use the --thread option in addition of the in-reply-to to
new series.
Small comments inline,
On Wed, Apr 12, 2017 at 05:02:24PM +0800, Wei Dai wrote:
>[...]
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index ff903e6..91fc4c4 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -4475,26 +4475,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
> 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 -ENOTSUP;
> (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 -EINVAL;
> + }
> + re = priv_mac_addr_add(priv, index,
> (const uint8_t (*)[ETHER_ADDR_LEN])
> mac_addr->addr_bytes);
Please keep the coding style consistency among the file, this last
snippet should be:
re = priv_mac_addr_add(priv, index,
(const uint8_t (*)[ETHER_ADDR_LEN])
mac_addr->addr_bytes);
> end:
> priv_unlock(priv);
> + return -re;
> }
>
> /**
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index d26d465..3c5de07 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -238,8 +238,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);
In the whole file, function prototypes does not provide variable names,
please keep it as is.
Same point about the indentation, the second line should be aligned with
the '('.
> 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..3cc6f8b 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 -ENOTSUP;
>
> (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 = -EINVAL;
> 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);
Same remark here about the indentation.
> end:
> priv_unlock(priv);
> + return -re;
> }
Thanks,
--
Nélio Laranjeiro
6WIND
