patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>
To: Wei Dai <wei.dai@intel.com>
Cc: dev@dpdk.org, thomas.monjalon@6wind.com, harish.patil@cavium.com,
	rasesh.mody@cavium.com, stephen.hurd@broadcom.com,
	ajit.khaparde@broadcom.com, wenzhuo.lu@intel.com,
	helin.zhang@intel.com, konstantin.ananyev@intel.com,
	jingjing.wu@intel.com, jing.d.chen@intel.com,
	adrien.mazarguil@6wind.com, bruce.richardson@intel.com,
	yuanhan.liu@linux.intel.com, maxime.coquelin@redhat.com,
	stable@dpdk.org
Subject: Re: [dpdk-stable] [PATCH v3 1/3] ethdev: fix adding invalid MAC addr
Date: Wed, 12 Apr 2017 11:30:20 +0200	[thread overview]
Message-ID: <20170412093020.GC16796@autoinstall.dev.6wind.com> (raw)
In-Reply-To: <1491987746-10155-2-git-send-email-wei.dai@intel.com>

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

  reply	other threads:[~2017-04-12  9:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1492071245.git.wei.dai@intel.com>
     [not found] ` <1491987746-10155-1-git-send-email-wei.dai@intel.com>
2017-04-12  9:02   ` Wei Dai
2017-04-12  9:30     ` Nélio Laranjeiro [this message]
2017-04-13  8:21   ` [dpdk-stable] [PATCH v4 " Wei Dai
2017-04-13  8:44     ` Nélio Laranjeiro
2017-04-13  9:22       ` Dai, Wei
2017-04-20  5:31     ` Yuanhan Liu
2017-04-20 21:43     ` Thomas Monjalon
2017-04-21  6:43     ` Lu, Wenzhuo
2017-04-29  6:09       ` Dai, Wei
2017-05-02  1:21         ` Lu, Wenzhuo
2017-05-02  1:51           ` Dai, Wei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170412093020.GC16796@autoinstall.dev.6wind.com \
    --to=nelio.laranjeiro@6wind.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=harish.patil@cavium.com \
    --cc=helin.zhang@intel.com \
    --cc=jing.d.chen@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=rasesh.mody@cavium.com \
    --cc=stable@dpdk.org \
    --cc=stephen.hurd@broadcom.com \
    --cc=thomas.monjalon@6wind.com \
    --cc=wei.dai@intel.com \
    --cc=wenzhuo.lu@intel.com \
    --cc=yuanhan.liu@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).