net/mlx5: fix VF MAC address set over Bluefield

Message ID 20200722090755.26103-1-rasland@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: fix VF MAC address set over Bluefield |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS

Commit Message

Raslan Darawsheh July 22, 2020, 9:07 a.m. UTC
  When trying to set MAC address of an ethethet device and if it was
a representor, PMD sets the MAC over the corresponding VF instead.

For the case of HPF (Host PF representor on BluueField), PMD shouldn't
attempt to set it, since it doesn't have any corresponding VF and fails.

This will fix the issue by setting the MAC on the dev directly.

Fixes: 0d1d73170820 ("net/mlx5: set VF MAC address from host")
Cc: stable@dpdk.org

Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_mac.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
  

Comments

Raslan Darawsheh July 23, 2020, 8:57 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Raslan Darawsheh
> Sent: Wednesday, July 22, 2020 12:08 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Slava Ovsiienko <viacheslavo@mellanox.com>
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix VF MAC address set over Bluefield
> 
> When trying to set MAC address of an ethethet device and if it was
> a representor, PMD sets the MAC over the corresponding VF instead.
> 
> For the case of HPF (Host PF representor on BluueField), PMD shouldn't
> attempt to set it, since it doesn't have any corresponding VF and fails.
> 
> This will fix the issue by setting the MAC on the dev directly.
> 
> Fixes: 0d1d73170820 ("net/mlx5: set VF MAC address from host")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_mac.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
> index 2d808d629..c75b1a169 100644
> --- a/drivers/net/mlx5/mlx5_mac.c
> +++ b/drivers/net/mlx5/mlx5_mac.c
> @@ -157,8 +157,11 @@ mlx5_mac_addr_set(struct rte_eth_dev *dev, struct
> rte_ether_addr *mac_addr)
>  	uint16_t port_id;
>  	struct mlx5_priv *priv = dev->data->dev_private;
> 
> -	/* Configuring the VF instead of its representor. */
> -	if (priv->representor) {
> +	/*
> +	 * Configuring the VF instead of its representor,
> +	 * need to skip the special case of HPF on Bluefield.
> +	 */
> +	if (priv->representor && priv->representor >= 0) {
>  		DRV_LOG(DEBUG, "VF represented by port %u setting
> primary MAC address",
>  			dev->data->port_id);
>  		RTE_ETH_FOREACH_DEV_SIBLING(port_id, dev->data-
> >port_id) {
> --
> 2.27.0


Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh
  
Ferruh Yigit July 23, 2020, 10:43 a.m. UTC | #2
On 7/22/2020 10:07 AM, Raslan Darawsheh wrote:
> When trying to set MAC address of an ethethet device and if it was
> a representor, PMD sets the MAC over the corresponding VF instead.
> 
> For the case of HPF (Host PF representor on BluueField), PMD shouldn't
> attempt to set it, since it doesn't have any corresponding VF and fails.
> 
> This will fix the issue by setting the MAC on the dev directly.
> 
> Fixes: 0d1d73170820 ("net/mlx5: set VF MAC address from host")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_mac.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
> index 2d808d629..c75b1a169 100644
> --- a/drivers/net/mlx5/mlx5_mac.c
> +++ b/drivers/net/mlx5/mlx5_mac.c
> @@ -157,8 +157,11 @@ mlx5_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
>  	uint16_t port_id;
>  	struct mlx5_priv *priv = dev->data->dev_private;
>  
> -	/* Configuring the VF instead of its representor. */
> -	if (priv->representor) {
> +	/*
> +	 * Configuring the VF instead of its representor,
> +	 * need to skip the special case of HPF on Bluefield.
> +	 */
> +	if (priv->representor && priv->representor >= 0) {


'priv->representor' is 1 bit size bitfield (unsigned int representor:1), what is
the point of additional ">=0" check?
Can it be "representor_id >= 0" by any chance?
  
Raslan Darawsheh July 23, 2020, 10:55 a.m. UTC | #3
Hi,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, July 23, 2020 1:44 PM
> To: Raslan Darawsheh <rasland@mellanox.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Slava Ovsiienko <viacheslavo@mellanox.com>
> Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix VF MAC address set over
> Bluefield
> 
> On 7/22/2020 10:07 AM, Raslan Darawsheh wrote:
> > When trying to set MAC address of an ethethet device and if it was
> > a representor, PMD sets the MAC over the corresponding VF instead.
> >
> > For the case of HPF (Host PF representor on BluueField), PMD shouldn't
> > attempt to set it, since it doesn't have any corresponding VF and fails.
> >
> > This will fix the issue by setting the MAC on the dev directly.
> >
> > Fixes: 0d1d73170820 ("net/mlx5: set VF MAC address from host")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5_mac.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
> > index 2d808d629..c75b1a169 100644
> > --- a/drivers/net/mlx5/mlx5_mac.c
> > +++ b/drivers/net/mlx5/mlx5_mac.c
> > @@ -157,8 +157,11 @@ mlx5_mac_addr_set(struct rte_eth_dev *dev,
> struct rte_ether_addr *mac_addr)
> >  	uint16_t port_id;
> >  	struct mlx5_priv *priv = dev->data->dev_private;
> >
> > -	/* Configuring the VF instead of its representor. */
> > -	if (priv->representor) {
> > +	/*
> > +	 * Configuring the VF instead of its representor,
> > +	 * need to skip the special case of HPF on Bluefield.
> > +	 */
> > +	if (priv->representor && priv->representor >= 0) {
> 
> 
> 'priv->representor' is 1 bit size bitfield (unsigned int representor:1), what is
> the point of additional ">=0" check?
> Can it be "representor_id >= 0" by any chance?
Yes it should be representor_id >= 0, will fix 

Kindest regards
Raslan Darawsheh
  

Patch

diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index 2d808d629..c75b1a169 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -157,8 +157,11 @@  mlx5_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
 	uint16_t port_id;
 	struct mlx5_priv *priv = dev->data->dev_private;
 
-	/* Configuring the VF instead of its representor. */
-	if (priv->representor) {
+	/*
+	 * Configuring the VF instead of its representor,
+	 * need to skip the special case of HPF on Bluefield.
+	 */
+	if (priv->representor && priv->representor >= 0) {
 		DRV_LOG(DEBUG, "VF represented by port %u setting primary MAC address",
 			dev->data->port_id);
 		RTE_ETH_FOREACH_DEV_SIBLING(port_id, dev->data->port_id) {