Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 5/8] net: mscc: Add initial Ocelot switch support
From: Andrew Lunn @ 2018-03-30 14:50 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Florian Fainelli, David S . Miller, Allan Nielsen,
	razvan.stefanescu, po.liu, Thomas Petazzoni, netdev, devicetree,
	linux-kernel, linux-mips
In-Reply-To: <20180330141634.GD14180@piout.net>

On Fri, Mar 30, 2018 at 04:16:34PM +0200, Alexandre Belloni wrote:
> On 30/03/2018 at 15:54:22 +0200, Andrew Lunn wrote:
> > > > All of this sounds like it should be moved into the br_join/leave, this
> > > > does not appear to be the right place to do that.
> > > > 
> > > 
> > > No, I've triple checked because this is a comment that both Andrew and
> > > you had. Once a port is added to the PGID MASK, it will start forwarding
> > > frames so we really want that to happen only when the port is in
> > > BR_STATE_FORWARDING state. Else, we may forward frames between the
> > > addition of the port to the bridge and setting the port to the
> > > BR_STATE_BLOCKING state.
> > 
> > Hi Alexandre
> > 
> > Interesting observation. I took a look at some of the other join
> > implementations. mv88e6xxx does the join immediately. mt7539 does it
> > immediately, if the port is enabled. lan9303 does it immediately.
> > qca8k does it immediately. b53 does it immediately.
> > 
> 
> I had a look at b53, my impression was that b53_br_join() adds the port
> to the bridge but b53_br_set_stp_state() actually enables forwarding. So
> as long as the default on the port is PORT_CTRL_DIS_STATE, the port will
> not be forwarding frames. And this is the case because b53_enable_port()
> does put 0 in B53_PORT_CTRL.

https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/b53/b53_regs.h#L71

It seems like, 0 means no STP at all. Which to me would mean, forward
all packets. But i could be wrong. Florian?

> The fact is that ocelot doesn't have separate controls. The port is
> either forwarding or not. If it is not forwarding, then there is nothing
> to tell the HW to do.

Think about the following sequence:

ip link set lan0 up

After this command, i expect to see packets on lan0 arrive at the
host, tcpdump to work, etc. This probably means the port is in
'forwarding' mode, or for B53, STP is disabled.

ip link add name br0 type bridge
ip link set dev br0 up
ip link set dev lan0 master br0

When the port is added to the bridge, there is a window of time
between the join and the STP change to blocking/learning, when the
port is in forwarding mode. You avoid this window. But the other
drivers don't appear to.

So i would like to fix this of every driver. I'm not sure how yet...

   Andrew

^ permalink raw reply

* Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration
From: David Ahern @ 2018-03-30 14:45 UTC (permalink / raw)
  To: Ido Schimmel, netdev; +Cc: davem, jiri, petrm, mlxsw
In-Reply-To: <20180329203334.25575-10-idosch@mellanox.com>

On 3/29/18 2:33 PM, Ido Schimmel wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> This resolves race during initialization where the resources with
> ops are registered before driver and the structures used by occ_get
> op is initialized. So keep occ_get callbacks registered only when
> all structs are initialized.

Why can't the occ_get handler look at some flag in an mlxsw struct to
know if the system has initialized?

Separate registration here is awkward. You register a resource and then
register its op later.

Also, this should be a standalone patch rather than embedded in a
'mlxsw: Various cleanups' set.


> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 24 ++-----
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  1 -
>  .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    | 67 ++++++++++++--------
>  include/net/devlink.h                              | 40 ++++++++----
>  net/core/devlink.c                                 | 74 +++++++++++++++++++---
>  5 files changed, 134 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index b831af38e0a1..0d95d2cb73e3 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -3805,18 +3805,6 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
>  	},
>  };
>  
> -static u64 mlxsw_sp_resource_kvd_linear_occ_get(struct devlink *devlink)
> -{
> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> -
> -	return mlxsw_sp_kvdl_occ_get(mlxsw_sp);
> -}
> -
> -static const struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
> -	.occ_get = mlxsw_sp_resource_kvd_linear_occ_get,
> -};
> -
>  static void
>  mlxsw_sp_resource_size_params_prepare(struct mlxsw_core *mlxsw_core,
>  				      struct devlink_resource_size_params *kvd_size_params,
> @@ -3877,8 +3865,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>  	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD,
>  					kvd_size, MLXSW_SP_RESOURCE_KVD,
>  					DEVLINK_RESOURCE_ID_PARENT_TOP,
> -					&kvd_size_params,
> -					NULL);
> +					&kvd_size_params);
>  	if (err)
>  		return err;
>  
> @@ -3887,8 +3874,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>  					linear_size,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
>  					MLXSW_SP_RESOURCE_KVD,
> -					&linear_size_params,
> -					&mlxsw_sp_resource_kvd_linear_ops);
> +					&linear_size_params);
>  	if (err)
>  		return err;
>  
> @@ -3905,8 +3891,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>  					double_size,
>  					MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
>  					MLXSW_SP_RESOURCE_KVD,
> -					&hash_double_size_params,
> -					NULL);
> +					&hash_double_size_params);
>  	if (err)
>  		return err;
>  
> @@ -3915,8 +3900,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>  					single_size,
>  					MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
>  					MLXSW_SP_RESOURCE_KVD,
> -					&hash_single_size_params,
> -					NULL);
> +					&hash_single_size_params);
>  	if (err)
>  		return err;
>  
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> index 21bee8f19894..c59a0d7d81d5 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> @@ -442,7 +442,6 @@ void mlxsw_sp_kvdl_free(struct mlxsw_sp *mlxsw_sp, int entry_index);
>  int mlxsw_sp_kvdl_alloc_size_query(struct mlxsw_sp *mlxsw_sp,
>  				   unsigned int entry_count,
>  				   unsigned int *p_alloc_size);
> -u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp);
>  int mlxsw_sp_kvdl_resources_register(struct devlink *devlink);
>  
>  struct mlxsw_sp_acl_rule_info {
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
> index 7b28f65d6407..1b7280168e6b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
> @@ -315,8 +315,9 @@ static u64 mlxsw_sp_kvdl_part_occ(struct mlxsw_sp_kvdl_part *part)
>  	return occ;
>  }
>  
> -u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
> +static u64 mlxsw_sp_kvdl_occ_get(void *priv)
>  {
> +	const struct mlxsw_sp *mlxsw_sp = priv;
>  	u64 occ = 0;
>  	int i;
>  
> @@ -326,48 +327,33 @@ u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
>  	return occ;
>  }
>  
> -static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
> +static u64 mlxsw_sp_kvdl_single_occ_get(void *priv)
>  {
> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> +	const struct mlxsw_sp *mlxsw_sp = priv;
>  	struct mlxsw_sp_kvdl_part *part;
>  
>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_SINGLE];
>  	return mlxsw_sp_kvdl_part_occ(part);
>  }
>  
> -static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
> +static u64 mlxsw_sp_kvdl_chunks_occ_get(void *priv)
>  {
> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> +	const struct mlxsw_sp *mlxsw_sp = priv;
>  	struct mlxsw_sp_kvdl_part *part;
>  
>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_CHUNKS];
>  	return mlxsw_sp_kvdl_part_occ(part);
>  }
>  
> -static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
> +static u64 mlxsw_sp_kvdl_large_chunks_occ_get(void *priv)
>  {
> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> +	const struct mlxsw_sp *mlxsw_sp = priv;
>  	struct mlxsw_sp_kvdl_part *part;
>  
>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS];
>  	return mlxsw_sp_kvdl_part_occ(part);
>  }
>  
> -static const struct devlink_resource_ops mlxsw_sp_kvdl_single_ops = {
> -	.occ_get = mlxsw_sp_kvdl_single_occ_get,
> -};
> -
> -static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_ops = {
> -	.occ_get = mlxsw_sp_kvdl_chunks_occ_get,
> -};
> -
> -static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_large_ops = {
> -	.occ_get = mlxsw_sp_kvdl_large_chunks_occ_get,
> -};
> -
>  int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>  {
>  	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> @@ -386,8 +372,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>  					MLXSW_SP_KVDL_SINGLE_SIZE,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
> -					&size_params,
> -					&mlxsw_sp_kvdl_single_ops);
> +					&size_params);
>  	if (err)
>  		return err;
>  
> @@ -398,8 +383,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>  					MLXSW_SP_KVDL_CHUNKS_SIZE,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
> -					&size_params,
> -					&mlxsw_sp_kvdl_chunks_ops);
> +					&size_params);
>  	if (err)
>  		return err;
>  
> @@ -410,13 +394,13 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>  					MLXSW_SP_KVDL_LARGE_CHUNKS_SIZE,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
> -					&size_params,
> -					&mlxsw_sp_kvdl_chunks_large_ops);
> +					&size_params);
>  	return err;
>  }
>  
>  int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>  {
> +	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
>  	struct mlxsw_sp_kvdl *kvdl;
>  	int err;
>  
> @@ -429,6 +413,23 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>  	if (err)
>  		goto err_kvdl_parts_init;
>  
> +	devlink_resource_occ_get_register(devlink,
> +					  MLXSW_SP_RESOURCE_KVD_LINEAR,
> +					  mlxsw_sp_kvdl_occ_get,
> +					  mlxsw_sp);
> +	devlink_resource_occ_get_register(devlink,
> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
> +					  mlxsw_sp_kvdl_single_occ_get,
> +					  mlxsw_sp);
> +	devlink_resource_occ_get_register(devlink,
> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
> +					  mlxsw_sp_kvdl_chunks_occ_get,
> +					  mlxsw_sp);
> +	devlink_resource_occ_get_register(devlink,
> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
> +					  mlxsw_sp_kvdl_large_chunks_occ_get,
> +					  mlxsw_sp);
> +
>  	return 0;
>  
>  err_kvdl_parts_init:
> @@ -438,6 +439,16 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>  
>  void mlxsw_sp_kvdl_fini(struct mlxsw_sp *mlxsw_sp)
>  {
> +	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
> +
> +	devlink_resource_occ_get_unregister(devlink,
> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS);
> +	devlink_resource_occ_get_unregister(devlink,
> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS);
> +	devlink_resource_occ_get_unregister(devlink,
> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE);
> +	devlink_resource_occ_get_unregister(devlink,
> +					    MLXSW_SP_RESOURCE_KVD_LINEAR);
>  	mlxsw_sp_kvdl_parts_fini(mlxsw_sp);
>  	kfree(mlxsw_sp->kvdl);
>  }
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index e21d8cadd480..2e4f71e16e95 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -231,14 +231,6 @@ struct devlink_dpipe_headers {
>  	unsigned int headers_count;
>  };
>  
> -/**
> - * struct devlink_resource_ops - resource ops
> - * @occ_get: get the occupied size
> - */
> -struct devlink_resource_ops {
> -	u64 (*occ_get)(struct devlink *devlink);
> -};
> -
>  /**
>   * struct devlink_resource_size_params - resource's size parameters
>   * @size_min: minimum size which can be set
> @@ -265,6 +257,8 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
>  	size_params->unit = unit;
>  }
>  
> +typedef u64 devlink_resource_occ_get_t(void *priv);
> +
>  /**
>   * struct devlink_resource - devlink resource
>   * @name: name of the resource
> @@ -277,7 +271,6 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
>   * @size_params: size parameters
>   * @list: parent list
>   * @resource_list: list of child resources
> - * @resource_ops: resource ops
>   */
>  struct devlink_resource {
>  	const char *name;
> @@ -289,7 +282,8 @@ struct devlink_resource {
>  	struct devlink_resource_size_params size_params;
>  	struct list_head list;
>  	struct list_head resource_list;
> -	const struct devlink_resource_ops *resource_ops;
> +	devlink_resource_occ_get_t *occ_get;
> +	void *occ_get_priv;
>  };
>  
>  #define DEVLINK_RESOURCE_ID_PARENT_TOP 0
> @@ -409,8 +403,7 @@ int devlink_resource_register(struct devlink *devlink,
>  			      u64 resource_size,
>  			      u64 resource_id,
>  			      u64 parent_resource_id,
> -			      const struct devlink_resource_size_params *size_params,
> -			      const struct devlink_resource_ops *resource_ops);
> +			      const struct devlink_resource_size_params *size_params);
>  void devlink_resources_unregister(struct devlink *devlink,
>  				  struct devlink_resource *resource);
>  int devlink_resource_size_get(struct devlink *devlink,
> @@ -419,6 +412,12 @@ int devlink_resource_size_get(struct devlink *devlink,
>  int devlink_dpipe_table_resource_set(struct devlink *devlink,
>  				     const char *table_name, u64 resource_id,
>  				     u64 resource_units);
> +void devlink_resource_occ_get_register(struct devlink *devlink,
> +				       u64 resource_id,
> +				       devlink_resource_occ_get_t *occ_get,
> +				       void *occ_get_priv);
> +void devlink_resource_occ_get_unregister(struct devlink *devlink,
> +					 u64 resource_id);
>  
>  #else
>  
> @@ -562,8 +561,7 @@ devlink_resource_register(struct devlink *devlink,
>  			  u64 resource_size,
>  			  u64 resource_id,
>  			  u64 parent_resource_id,
> -			  const struct devlink_resource_size_params *size_params,
> -			  const struct devlink_resource_ops *resource_ops)
> +			  const struct devlink_resource_size_params *size_params)
>  {
>  	return 0;
>  }
> @@ -589,6 +587,20 @@ devlink_dpipe_table_resource_set(struct devlink *devlink,
>  	return -EOPNOTSUPP;
>  }
>  
> +static inline void
> +devlink_resource_occ_get_register(struct devlink *devlink,
> +				  u64 resource_id,
> +				  devlink_resource_occ_get_t *occ_get,
> +				  void *occ_get_priv)
> +{
> +}
> +
> +static inline void
> +devlink_resource_occ_get_unregister(struct devlink *devlink,
> +				    u64 resource_id)
> +{
> +}
> +
>  #endif
>  
>  #endif /* _NET_DEVLINK_H_ */
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 9236e421bd62..ad1317376798 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -2405,6 +2405,16 @@ devlink_resource_size_params_put(struct devlink_resource *resource,
>  	return 0;
>  }
>  
> +static int devlink_resource_occ_put(struct devlink_resource *resource,
> +				    struct sk_buff *skb)
> +{
> +	if (!resource->occ_get)
> +		return 0;
> +	return nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
> +				 resource->occ_get(resource->occ_get_priv),
> +				 DEVLINK_ATTR_PAD);
> +}
> +
>  static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
>  				struct devlink_resource *resource)
>  {
> @@ -2425,11 +2435,8 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
>  	if (resource->size != resource->size_new)
>  		nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_SIZE_NEW,
>  				  resource->size_new, DEVLINK_ATTR_PAD);
> -	if (resource->resource_ops && resource->resource_ops->occ_get)
> -		if (nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
> -				      resource->resource_ops->occ_get(devlink),
> -				      DEVLINK_ATTR_PAD))
> -			goto nla_put_failure;
> +	if (devlink_resource_occ_put(resource, skb))
> +		goto nla_put_failure;
>  	if (devlink_resource_size_params_put(resource, skb))
>  		goto nla_put_failure;
>  	if (list_empty(&resource->resource_list))
> @@ -3162,15 +3169,13 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
>   *	@resource_id: resource's id
>   *	@parent_reosurce_id: resource's parent id
>   *	@size params: size parameters
> - *	@resource_ops: resource ops
>   */
>  int devlink_resource_register(struct devlink *devlink,
>  			      const char *resource_name,
>  			      u64 resource_size,
>  			      u64 resource_id,
>  			      u64 parent_resource_id,
> -			      const struct devlink_resource_size_params *size_params,
> -			      const struct devlink_resource_ops *resource_ops)
> +			      const struct devlink_resource_size_params *size_params)
>  {
>  	struct devlink_resource *resource;
>  	struct list_head *resource_list;
> @@ -3213,7 +3218,6 @@ int devlink_resource_register(struct devlink *devlink,
>  	resource->size = resource_size;
>  	resource->size_new = resource_size;
>  	resource->id = resource_id;
> -	resource->resource_ops = resource_ops;
>  	resource->size_valid = true;
>  	memcpy(&resource->size_params, size_params,
>  	       sizeof(resource->size_params));
> @@ -3315,6 +3319,58 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink,
>  }
>  EXPORT_SYMBOL_GPL(devlink_dpipe_table_resource_set);
>  
> +/**
> + *	devlink_resource_occ_get_register - register occupancy getter
> + *
> + *	@devlink: devlink
> + *	@resource_id: resource id
> + *	@occ_get: occupancy getter callback
> + *	@occ_get_priv: occupancy getter callback priv
> + */
> +void devlink_resource_occ_get_register(struct devlink *devlink,
> +				       u64 resource_id,
> +				       devlink_resource_occ_get_t *occ_get,
> +				       void *occ_get_priv)
> +{
> +	struct devlink_resource *resource;
> +
> +	mutex_lock(&devlink->lock);
> +	resource = devlink_resource_find(devlink, NULL, resource_id);
> +	if (WARN_ON(!resource))
> +		goto out;
> +	WARN_ON(resource->occ_get);
> +
> +	resource->occ_get = occ_get;
> +	resource->occ_get_priv = occ_get_priv;
> +out:
> +	mutex_unlock(&devlink->lock);
> +}
> +EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register);
> +
> +/**
> + *	devlink_resource_occ_get_unregister - unregister occupancy getter
> + *
> + *	@devlink: devlink
> + *	@resource_id: resource id
> + */
> +void devlink_resource_occ_get_unregister(struct devlink *devlink,
> +					 u64 resource_id)
> +{
> +	struct devlink_resource *resource;
> +
> +	mutex_lock(&devlink->lock);
> +	resource = devlink_resource_find(devlink, NULL, resource_id);
> +	if (WARN_ON(!resource))
> +		goto out;
> +	WARN_ON(!resource->occ_get);
> +
> +	resource->occ_get = NULL;
> +	resource->occ_get_priv = NULL;
> +out:
> +	mutex_unlock(&devlink->lock);
> +}
> +EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
> +
>  static int __init devlink_module_init(void)
>  {
>  	return genl_register_family(&devlink_nl_family);
> 

^ permalink raw reply

* Re: [PATCH v4 0/2] of_net: Implement of_get_nvmem_mac_address helper
From: David Miller @ 2018-03-30 14:40 UTC (permalink / raw)
  To: mike.looijmans
  Cc: netdev, linux-kernel, devicetree, andrew, f.fainelli, robh+dt,
	frowand.list
In-Reply-To: <1522301389-11034-1-git-send-email-mike.looijmans@topic.nl>

From: Mike Looijmans <mike.looijmans@topic.nl>
Date: Thu, 29 Mar 2018 07:29:47 +0200

> Posted this as a small set now, with an (optional) second patch that shows
> how the changes work and what I've used to test the code on a Topic Miami board.
> I've taken the liberty to add appropriate "Acked" and "Review" tags.
> 
> v4: Replaced "6" with ETH_ALEN
> 
> v3: Add patch that implements mac in nvmem for the Cadence MACB controller
>     Remove the integrated of_get_mac_address call
> 
> v2: Use of_nvmem_cell_get to avoid needing the assiciated device
>     Use void* instead of char*
>     Add devicetree binding doc

Series applied to net-next, thank you.

^ permalink raw reply

* Re: [PATCH net-next 0/2] nfp: flower: handle MTU changes
From: David Miller @ 2018-03-30 14:39 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20180329015007.19496-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed, 28 Mar 2018 18:50:05 -0700

> This set improves MTU handling for flower offload.  The max MTU is
> correctly capped and physical port MTU is communicated to the FW
> (and indirectly HW).

Series applied, thanks Jakub.

^ permalink raw reply

* Re: [PATCH net-next 0/9] devlink: Add support for region access
From: Andrew Lunn @ 2018-03-30 14:34 UTC (permalink / raw)
  To: Alex Vesker; +Cc: David S. Miller, netdev, Tariq Toukan, Jiri Pirko
In-Reply-To: <28b99a08-1967-3044-4010-0faa5d6bfc14@mellanox.com>

> >And it seems to want contiguous pages. How well does that work after
> >the system has been running for a while and memory is fragmented?
> 
> The allocation can be changed, there is no read need for contiguous pages.
> It is important to note that we the amount of snapshots is limited by the
> driver
> this can be based on the dump size or expected frequency of collection.
> I also prefer not to pre-allocate this memory.

The driver code also asks for a 1MB contiguous chunk of memory!  You
really should think about this API, how can you avoid double memory
allocations. And can kvmalloc be used. But then you get into the
problem for DMA'ing the memory from the device...

This API also does not scale. 1MB is actually quite small. I'm sure
there is firmware running on CPUs with a lot more than 1MB of RAM.
How well does with API work with 64MB? Say i wanted to snapshot my
GPU? Or the MC/BMC?

> Dump in devlink means provide all the data, saying dump address x length y
> sounds
> confusing.  Do you see this as a critical issue?

No, i don't. But nearly every set of tools i've used has one command.
eg uboot, coreboot, gdb, od, hexdump. Even ethtool has [offset N] [length N]

How many tools can you name which have two different command, rather
than one with options?

      Andrew

^ permalink raw reply

* Re: [net-next V7 PATCH 12/16] page_pool: refurbish version of page_pool code
From: kbuild test robot @ 2018-03-30 14:25 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: kbuild-all, netdev, BjörnTöpel, magnus.karlsson,
	eugenia, Jason Wang, John Fastabend, Eran Ben Elisha,
	Saeed Mahameed, galp, Jesper Dangaard Brouer, Daniel Borkmann,
	Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152234291739.17048.7135649249513438321.stgit@firesoul>

[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]

Hi Jesper,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jesper-Dangaard-Brouer/XDP-redirect-memory-return-API/20180330-203122
config: score-spct6600_defconfig (attached as .config)
compiler: score-elf-gcc (GCC) 4.9.4
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=score 

All errors (new ones prefixed by >>):

   init/main.o: In function `try_to_run_init_process':
   main.c:(.text+0x40): relocation truncated to fit: R_SCORE_24 against `run_init_process'
   main.c:(.text+0x78): relocation truncated to fit: R_SCORE_24 against `.L36'
   init/main.o: In function `.L132':
   main.c:(.text+0x190): relocation truncated to fit: R_SCORE_24 against `.L129'
   main.c:(.text+0x200): relocation truncated to fit: R_SCORE_24 against `.L132'
   init/main.o: In function `loglevel':
   main.c:(.init.text+0xa4): relocation truncated to fit: R_SCORE_24 against `get_option'
   init/main.o: In function `.L15':
   main.c:(.init.text+0x110): relocation truncated to fit: R_SCORE_24 against `strcmp'
   main.c:(.init.text+0x124): relocation truncated to fit: R_SCORE_24 against `parameq'
   main.c:(.init.text+0x14c): relocation truncated to fit: R_SCORE_24 against `printk'
   init/main.o: In function `.L31':
   main.c:(.init.text+0x160): relocation truncated to fit: R_SCORE_24 against `strcmp'
   init/main.o: In function `.L21':
   main.c:(.init.text+0x170): relocation truncated to fit: R_SCORE_24 against `.L15'
   init/main.o: In function `initcall_blacklist':
   main.c:(.init.text+0x198): additional relocation overflows omitted from the output
   net/core/page_pool.o: In function `__page_pool_alloc_pages_slow':
>> page_pool.c:(.text.__page_pool_alloc_pages_slow+0x74): undefined reference to `bad_dma_ops'
   page_pool.c:(.text.__page_pool_alloc_pages_slow+0x78): undefined reference to `bad_dma_ops'
   page_pool.c:(.text.__page_pool_alloc_pages_slow+0x8c): undefined reference to `bad_dma_ops'
   page_pool.c:(.text.__page_pool_alloc_pages_slow+0x90): undefined reference to `bad_dma_ops'
   net/core/page_pool.o: In function `__page_pool_clean_page.isra.14':
>> page_pool.c:(.text.__page_pool_clean_page.isra.14+0x2c): undefined reference to `bad_dma_ops'
   net/core/page_pool.o:page_pool.c:(.text.__page_pool_clean_page.isra.14+0x30): more undefined references to `bad_dma_ops' follow

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7999 bytes --]

^ permalink raw reply

* Re: [PATCH 0/2] rhashtable_walk fixes
From: David Miller @ 2018-03-30 14:18 UTC (permalink / raw)
  To: neilb; +Cc: tgraf, herbert, netdev, linux-kernel
In-Reply-To: <152228607974.16370.14544827502467836789.stgit@noble>

From: NeilBrown <neilb@suse.com>
Date: Thu, 29 Mar 2018 12:19:09 +1100

> These two patches apply on top of my previous "rhashtable: reset iter
> when rhashtable_walk_start sees new table" patch.
> 
> The first fixes a bug that I found in rhltable_insert().
> 
> The second is an alternate to my "rhashtable: allow a walk of the hash
> table without missing object."
> This version doesn't require an API change and should be reliable for
> rhltables too (my first version didn't handle these correctly).

Neil, please don't mix and match patches.

Also when you need to change a patch in a series, please post the entire
new series not just the patch that changes.

Patch #1 in this series is unnecessary.  As Herbert explained this has
been fixed already.

So please repost freshly the patches that are relevant and you want me
to consider for inclusion.  Also be explicit and clear about which of
my two networking trees you are targetting these changes.

Thank you.

^ permalink raw reply

* Re: [PATCH net-next 5/8] net: mscc: Add initial Ocelot switch support
From: Alexandre Belloni @ 2018-03-30 14:16 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, David S . Miller, Allan Nielsen,
	razvan.stefanescu, po.liu, Thomas Petazzoni, netdev, devicetree,
	linux-kernel, linux-mips
In-Reply-To: <20180330135422.GA28244@lunn.ch>

On 30/03/2018 at 15:54:22 +0200, Andrew Lunn wrote:
> > > All of this sounds like it should be moved into the br_join/leave, this
> > > does not appear to be the right place to do that.
> > > 
> > 
> > No, I've triple checked because this is a comment that both Andrew and
> > you had. Once a port is added to the PGID MASK, it will start forwarding
> > frames so we really want that to happen only when the port is in
> > BR_STATE_FORWARDING state. Else, we may forward frames between the
> > addition of the port to the bridge and setting the port to the
> > BR_STATE_BLOCKING state.
> 
> Hi Alexandre
> 
> Interesting observation. I took a look at some of the other join
> implementations. mv88e6xxx does the join immediately. mt7539 does it
> immediately, if the port is enabled. lan9303 does it immediately.
> qca8k does it immediately. b53 does it immediately.
> 

I had a look at b53, my impression was that b53_br_join() adds the port
to the bridge but b53_br_set_stp_state() actually enables forwarding. So
as long as the default on the port is PORT_CTRL_DIS_STATE, the port will
not be forwarding frames. And this is the case because b53_enable_port()
does put 0 in B53_PORT_CTRL.

The fact is that ocelot doesn't have separate controls. The port is
either forwarding or not. If it is not forwarding, then there is nothing
to tell the HW to do.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH] fix typo in command value in drivers/net/phy/mdio-bitbang.
From: Andrew Lunn @ 2018-03-30 14:14 UTC (permalink / raw)
  To: Frans Meulenbroeks; +Cc: torvalds, linux-kernel, netdev
In-Reply-To: <CACW_hTauBL6but9143Kio-ZU5jsMKx2uTEUv3kHFaTGuwdyHZA@mail.gmail.com>

On Fri, Mar 30, 2018 at 02:03:38PM +0200, Frans Meulenbroeks wrote:
> mdio-bitbang mentioned 10 for both read and write.
> However mdio read opcode is 10 and write opcode is 01
> Fixed comment.
> 
> Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>

Hi Frans

The correct place to send this patch is <netdev@vger.kernel.org> and
David Miller <davem@davemloft.net>. Please can you submit it again, to
these addresses.

In general, if you don't know where to send a patch,
scripts/get_maintainers.pl will tell you.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew


> ---
>  drivers/net/phy/mdio-bitbang.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/mdio-bitbang.c b/drivers/net/phy/mdio-bitbang.c
> index 61a543c..403b085 100644
> --- a/drivers/net/phy/mdio-bitbang.c
> +++ b/drivers/net/phy/mdio-bitbang.c
> @@ -113,7 +113,7 @@ static void mdiobb_cmd(struct mdiobb_ctrl *ctrl, int
> op, u8 phy, u8 reg)
>         for (i = 0; i < 32; i++)
>                 mdiobb_send_bit(ctrl, 1);
> 
> -       /* send the start bit (01) and the read opcode (10) or write (10).
> +       /* send the start bit (01) and the read opcode (10) or write (01).
>            Clause 45 operation uses 00 for the start and 11, 10 for
>            read/write */
>         mdiobb_send_bit(ctrl, 0);
> -- 
> 2.7.4

^ permalink raw reply

* Re: [PATCH] atm: iphase: fix spelling mistake: "Receiverd" -> "Received"
From: David Miller @ 2018-03-30 14:11 UTC (permalink / raw)
  To: colin.king
  Cc: 3chas3, linux-atm-general, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180328231853.28461-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Thu, 29 Mar 2018 00:18:53 +0100

> From: Colin Ian King <colin.king@canonical.com>
> 
> Trivial fix to spelling mistake in message text
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks Colin.

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] phylink: API changes
From: David Miller @ 2018-03-30 14:11 UTC (permalink / raw)
  To: f.fainelli
  Cc: netdev, thomas.petazzoni, andrew, rmk+kernel, linux-kernel,
	antoine.tenart, ymarkman, stefanc, maxime.chevallier,
	miquel.raynal, mw
In-Reply-To: <20180328224416.6981-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 28 Mar 2018 15:44:14 -0700

> This patch series contains two API changes to PHYLINK which will later be used
> by DSA to migrate to PHYLINK. Because these are API changes that impact other
> outstanding work (e.g: MVPP2) I would rather get them included sooner to minimize
> conflicts.

Series applied, thanks Florian.

^ permalink raw reply

* Re: [PATCH net-next] MAINTAINERS: update vmxnet3 driver maintainer
From: David Miller @ 2018-03-30 14:05 UTC (permalink / raw)
  To: doshir
  Cc: netdev, skhare, mchehab, gregkh, linus.walleij, akpm, rdunlap,
	linux-kernel
In-Reply-To: <20180328223834.5452-1-doshir@vmware.com>

From: Ronak Doshi <doshir@vmware.com>
Date: Wed, 28 Mar 2018 15:38:19 -0700

> Shrikrishna Khare would no longer maintain the vmxnet3 driver. Taking
> over the role of vmxnet3 maintainer.
> 
> Signed-off-by: Ronak Doshi <doshir@vmware.com>
> Signed-off-by: Shrikrishna Khare <skhare@vmware.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next v2 0/3] net: Broadcom drivers coalescing fixes
From: David Miller @ 2018-03-30 14:04 UTC (permalink / raw)
  To: f.fainelli
  Cc: netdev, jaedon.shin, pgynther, opendmb, michael.chan, gospo,
	talgi, saeedm
In-Reply-To: <20180328221538.29290-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 28 Mar 2018 15:15:35 -0700

> Following Tal's review of the adaptive RX/TX coalescing feature added to the
> SYSTEMPORT and GENET driver a number of things showed up:
> 
> - adaptive TX coalescing is not actually a good idea with the current way
>   the estimator will program the ring, this results in a higher CPU load, NAPI
>   on TX already does a reasonably good job at maintaining the interrupt count low
> 
> - both SYSTEMPORT and GENET would suffer from the same issues while configuring
>   coalescing parameters where the values would just not be applied correctly
>   based on user settings, so we fix that too
> 
> Tal, thanks again for your feedback, I would appreciate if you could review that
> the new behavior appears to be implemented correctly.
 ...
> Changes in v2:
> 
> - added Tal's reviewed-by to the first patch
> - split DIM initialization from coalescing parameters initialization
> - avoid duplicating the same code in bcmgenet_set_coalesce() when configuring RX rings
> - fixed the condition where default DIM parameters would be applied when
>   adaptive RX coalescing would be enabled, do this only if it was disabled before

Series applied, thanks Florian.

^ permalink raw reply

* Re: [PATCH net-next] net: Call add/kill vid ndo on vlan filter feature toggling
From: David Miller @ 2018-03-30 13:59 UTC (permalink / raw)
  To: galp; +Cc: netdev, tariqt
In-Reply-To: <1522248414-32539-1-git-send-email-galp@mellanox.com>

From: Gal Pressman <galp@mellanox.com>
Date: Wed, 28 Mar 2018 17:46:54 +0300

> NETIF_F_HW_VLAN_[CS]TAG_FILTER features require more than just a bit
> flip in dev->features in order to keep the driver in a consistent state.
> These features notify the driver of each added/removed vlan, but toggling
> of vlan-filter does not notify the driver accordingly for each of the
> existing vlans.
> 
> This patch implements a similar solution to NETIF_F_RX_UDP_TUNNEL_PORT
> behavior (which notifies the driver about UDP ports in the same manner
> that vids are reported).
> 
> Each toggling of the features propagates to the 8021q module, which
> iterates over the vlans and call add/kill ndo accordingly.
> 
> Signed-off-by: Gal Pressman <galp@mellanox.com>
> Reviewed-by: Tariq Toukan <tariqt@mellanox.com>

Looks good, applied, thank you.

^ permalink raw reply

* Re: [PATCH] tipc: avoid possible string overflow
From: David Miller @ 2018-03-30 13:54 UTC (permalink / raw)
  To: arnd
  Cc: jon.maloy, ying.xue, parthasarathy.bhuvaragan, netdev,
	tipc-discussion, linux-kernel
In-Reply-To: <20180328140302.2594031-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 28 Mar 2018 16:02:04 +0200

> gcc points out that the combined length of the fixed-length inputs to
> l->name is larger than the destination buffer size:
> 
> net/tipc/link.c: In function 'tipc_link_create':
> net/tipc/link.c:465:26: error: '%s' directive writing up to 32 bytes into a region of size between 26 and 58 [-Werror=format-overflow=]
>   sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
>                           ^~                              ~~~~~~~~
> net/tipc/link.c:465:2: note: 'sprintf' output 11 or more bytes (assuming 75) into a destination of size 60
>   sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
> 
> Using snprintf() ensures that the destination is still a nul-terminated
> string in all cases. It's still theoretically possible that the string
> gets trunctated though, so this patch should be carefully reviewed to
> ensure that either truncation is impossible in practice, or that we're
> ok with the truncation.
> 
> Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash values")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Based upon the discussion here, it looks like Jon will fix this in a different
way by increasing the destination buffer size.

^ permalink raw reply

* Re: [PATCH net-next 5/8] net: mscc: Add initial Ocelot switch support
From: Andrew Lunn @ 2018-03-30 13:54 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Florian Fainelli, David S . Miller, Allan Nielsen,
	razvan.stefanescu, po.liu, Thomas Petazzoni, netdev, devicetree,
	linux-kernel, linux-mips
In-Reply-To: <20180330124537.GC14180@piout.net>

> > All of this sounds like it should be moved into the br_join/leave, this
> > does not appear to be the right place to do that.
> > 
> 
> No, I've triple checked because this is a comment that both Andrew and
> you had. Once a port is added to the PGID MASK, it will start forwarding
> frames so we really want that to happen only when the port is in
> BR_STATE_FORWARDING state. Else, we may forward frames between the
> addition of the port to the bridge and setting the port to the
> BR_STATE_BLOCKING state.

Hi Alexandre

Interesting observation. I took a look at some of the other join
implementations. mv88e6xxx does the join immediately. mt7539 does it
immediately, if the port is enabled. lan9303 does it immediately.
qca8k does it immediately. b53 does it immediately.

Either they all get it wrong, or we make the assumption the bridge
sets the STP state first, then has the port join the bridge.

Looking at the code, br_add_if() it calls
netdev_master_upper_dev_link() and then later

        if (netif_running(dev) && netif_oper_up(dev) &&
            (br->dev->flags & IFF_UP))
                br_stp_enable_port(p);

So it does look like there is a window of time between the port
joining and the STP state being set.

I know in the past, we have run into the opposite problem. A port
leaves the bridge, while in blocked state. The port should then
becomes an individual port, so the STP state needs setting to
forwarding. I don't remember where we fix this.

I don't like that this new driver is different, but it also looks like
we have a real problem here. More digging needed.

   Andrew

^ permalink raw reply

* Re: [PATCH net-next] cxgb4: fix error return code in adap_init0()
From: David Miller @ 2018-03-30 13:52 UTC (permalink / raw)
  To: weiyongjun1; +Cc: ganeshgr, kumaras, netdev, kernel-janitors
In-Reply-To: <1522241469-77623-1-git-send-email-weiyongjun1@huawei.com>

From: Wei Yongjun <weiyongjun1@huawei.com>
Date: Wed, 28 Mar 2018 12:51:09 +0000

> Fix to return a negative error code from the hash filter init error
> handling case instead of 0, as done elsewhere in this function.
> 
> Fixes: 5c31254e35a8 ("cxgb4: initialize hash-filter configuration")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>

Applied, thanks Wei.

^ permalink raw reply

* Re: r8152 livelocks during pm_runtime_suspend
From: Jiri Slaby @ 2018-03-30 13:37 UTC (permalink / raw)
  To: hayeswang; +Cc: USB list, ML netdev, Rafael J. Wysocki, Linux PM list
In-Reply-To: <d1e1a702-753f-ec83-ed71-7e3649b20757@suse.cz>

On 03/30/2018, 03:17 PM, Jiri Slaby wrote:
> Hi,
> 
> I have seen r8152 from my docking station to kill my box several times
> in the last few days. The notebook is new, so I don't know if this is a
> regression.

Forgot to add, I am seeing this in dmesg:
[   13.353239] r8152 4-1.2:1.0 (unnamed net_device) (uninitialized):
Using pass-thru MAC addr d8:9e:f3:f6:6d:0c
[   13.365082] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[   13.390990] r8152 4-1.2:1.0 eth1: v1.09.9
[   13.399314] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[   13.552529] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready
[   13.561268] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready
[   13.736582] nf_conntrack version 0.5.0 (65536 buckets, 262144 max)
[   14.198646] bridge: filtering via arp/ip/ip6tables is no longer
available by default. Update your scripts to load br_netfilter if you
need this.
[   14.537233] Netfilter messages via NETLINK v0.30.
[   14.544740] ip_set: protocol 6
[   16.697657] r8152 4-1.2:1.0 eth1: carrier on
[   16.697724] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
[   16.847718] NET: Registered protocol family 17
[   20.629344] fuse init (API version 7.26)
[   21.371334] Bluetooth: RFCOMM TTY layer initialized
[   21.371343] Bluetooth: RFCOMM socket layer initialized
[   21.371353] Bluetooth: RFCOMM ver 1.11
[   21.753577] tun: Universal TUN/TAP device driver, 1.6
[   25.992820] retire_capture_urb: 2491 callbacks suppressed
[  118.054869] NOHZ: local_softirq_pending 08
[  118.056988] NOHZ: local_softirq_pending 08
[  185.562659] NOHZ: local_softirq_pending 08

08 seems to be NET_RX_SOFTIRQ. So is this related?

Kernel version: 4.15.14

> I have the NIC connected all the time. And when I return to the notebook
> after a while, the networking is dead. Looking at the stack traces, it
> is clear, that r8152 was attempted to be autosuspended and waits in
> napi_disable for NAPI_STATE_SCHED bit to be cleared:
> [22001.018437] kworker/2:0     D    0 16267      2 0x80000000
> [22001.018441] Workqueue: pm pm_runtime_work
> [22001.018443] Call Trace:
> [22001.018453]  schedule+0x2f/0x90
> [22001.018455]  schedule_timeout+0x1ce/0x540
> [22001.018474]  msleep+0x29/0x30
> [22001.018477]  napi_disable+0x25/0x60
> [22001.018483]  rtl8152_suspend+0x20a/0x2d0 [r8152]
> [22001.018493]  usb_suspend_both+0x8d/0x200 [usbcore]
> [22001.018510]  usb_runtime_suspend+0x2a/0x70 [usbcore]
> [22001.018514]  __rpm_callback+0xbc/0x1f0
> [22001.018519]  rpm_callback+0x4f/0x70
> [22001.018526]  rpm_suspend+0x11d/0x6d0
> [22001.018532]  pm_runtime_work+0x73/0xb0
> [22001.018535]  process_one_work+0x269/0x6c0
> [22001.018541]  worker_thread+0x2b/0x3d0
> [22001.018547]  kthread+0x113/0x130
> [22001.018556]  ret_from_fork+0x24/0x50
> 
> The assembly:
>> ffffffff81716730 <napi_disable>:
>> ffffffff81716730:       e8 eb b7 2e 00          callq  ffffffff81a01f20 <__fentry__>
>> ffffffff81716735:       55                      push   %rbp
>> ffffffff81716736:       48 89 fd                mov    %rdi,%rbp
>> ffffffff81716739:       53                      push   %rbx
>> ffffffff8171673a:       48 8d 5f 10             lea    0x10(%rdi),%rbx
>> ffffffff8171673e:       f0 80 4f 10 04          lock orb $0x4,0x10(%rdi)
>> ffffffff81716743:       f0 0f ba 6f 10 00       lock btsl $0x0,0x10(%rdi)
>> ffffffff81716749:       73 11                   jae    ffffffff8171675c <napi_disable+0x2c>
>> ffffffff8171674b:       bf 01 00 00 00          mov    $0x1,%edi
>> ffffffff81716750:       e8 ab ac a0 ff          callq  ffffffff81121400 <msleep>
>> ffffffff81716755:       f0 0f ba 2b 00          lock btsl $0x0,(%rbx)
>> ffffffff8171675a:       72 ef                   jb     ffffffff8171674b <napi_disable+0x1b>
> 
> 
> 
> 
> 
> There are other tasks in D state, of course, like these, waiting for the
> device to become pm-up:
> [22001.018749] kworker/3:1     D    0 16798      2 0x80000000
> [22001.018753] Workqueue: events rtl_work_func_t [r8152]
> [22001.018755] Call Trace:
> [22001.018767]  schedule+0x2f/0x90
> [22001.018769]  rpm_resume+0xf9/0x860
> [22001.018777]  rpm_resume+0x592/0x860
> [22001.018783]  __pm_runtime_resume+0x3a/0x50
> [22001.018789]  usb_autopm_get_interface+0x1d/0x50 [usbcore]
> [22001.018793]  rtl_work_func_t+0x3e/0x405 [r8152]
> [22001.018801]  process_one_work+0x269/0x6c0
> [22001.018807]  worker_thread+0x2b/0x3d0
> [22001.018813]  kthread+0x113/0x130
> [22001.018822]  ret_from_fork+0x24/0x50
> [22001.019713] tcpdump         D    0 17119   4265 0x00000004
> [22001.019716] Call Trace:
> [22001.019728]  schedule+0x2f/0x90
> [22001.019730]  rpm_resume+0xf9/0x860
> [22001.019738]  rpm_resume+0x592/0x860
> [22001.019744]  __pm_runtime_resume+0x3a/0x50
> [22001.019750]  usb_autopm_get_interface+0x1d/0x50 [usbcore]
> [22001.019754]  rtl8152_ioctl+0x30/0x140 [r8152]
> [22001.019758]  dev_ifsioc+0x115/0x3f0
> [22001.019763]  dev_ioctl+0x14b/0x680
> [22001.019775]  sock_do_ioctl+0x41/0x50
> [22001.019778]  sock_ioctl+0x1c2/0x2f0
> [22001.019781]  do_vfs_ioctl+0x91/0x680
> [22001.019789]  SyS_ioctl+0x74/0x80
> [22001.019794]  do_syscall_64+0x76/0x1c0
> 
> ...
> 
>> Showing all locks held in the system: 
>> 1 lock held by in:imklog/1371:
>>  #0:  (&f->f_pos_lock){+.+.}, at: [<00000000a0b38807>] __fdget_pos+0x3f/0x50
>> 1 lock held by Qt bearer threa/3003:
>>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
>> 1 lock held by Qt bearer threa/2825:
>>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
>> 1 lock held by DNS Res~ver #40/17041:
>>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
>> 1 lock held by Qt bearer threa/3110:
>>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
>> 1 lock held by DNS Res~ver #16/17044:
>>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
>> 2 locks held by bash/4561:
>>  #0:  (&tty->ldisc_sem){++++}, at: [<00000000e3d76e61>] tty_ldisc_ref_wait+0x24/0x50
>>  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<0000000091462d05>] n_tty_read+0xc3/0x850
>> 3 locks held by kworker/2:0/16267:
>>  #0:  ((wq_completion)"pm"){+.+.}, at: [<00000000b9dc0832>] process_one_work+0x1e3/0x6c0
>>  #1:  ((work_completion)(&dev->power.work)){+.+.}, at: [<00000000b9dc0832>] process_one_work+0x1e3/0x6c0
>>  #2:  (&tp->control){+.+.}, at: [<00000000ca575b90>] rtl8152_suspend+0x2b/0x2d0 [r8152]
>> 2 locks held by kworker/3:1/16798:
>>  #0:  ((wq_completion)"events"){+.+.}, at: [<00000000b9dc0832>] process_one_work+0x1e3/0x6c0
>>  #1:  ((work_completion)(&(&tp->schedule)->work)){+.+.}, at: [<00000000b9dc0832>] process_one_work+0x1e3/0x6c0
>> 1 lock held by tcpdump/17119: 
>>  #0:  (rtnl_mutex){+.+.}, at: [<0000000023a6461d>] dev_ioctl+0x13d/0x680
>> 2 locks held by less/17187: 
>>  #0:  (&tty->ldisc_sem){++++}, at: [<00000000e3d76e61>] tty_ldisc_ref_wait+0x24/0x50
>>  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<0000000091462d05>] n_tty_read+0xc3/0x850
> 
> 
> For now, I disabled pm-runtime on the device by:
> echo on > /sys/bus/usb/devices/4-1.2/power/control
> 
> Any ideas what's wrong? napi_disable from runtime suspend? Double
> napi_disable on the path? Some missing pm_runtime_get_sync somewhere?
> 
> Full dmesg after sysrq-t:
> https://www.fi.muni.cz/~xslaby/sklad/panics/r8152.txt
> 
> thanks,
> 


-- 
js
suse labs

^ permalink raw reply

* Re: [PATCH net-next 4/6] inet: frags: use rhashtables for reassembly units
From: Eric Dumazet @ 2018-03-30 13:30 UTC (permalink / raw)
  To: Herbert Xu, Eric Dumazet
  Cc: David S . Miller, netdev, Florian Westphal, Thomas Graf,
	Jesper Dangaard Brouer, Alexander Aring, Stefan Schmidt,
	Eric Dumazet, Nikolay Aleksandrov
In-Reply-To: <20180330071814.GA27025@gondor.apana.org.au>



On 03/30/2018 12:18 AM, Herbert Xu wrote:

> Instead of using the walk interface, how about
> rhashtable_free_and_destroy?

Oh, I was not aware of this beast ;)

I guess I will need to add a cond_resched() in it, right ?

My rough guess is that freeing 30 million frags would take few seconds ;)

> 
>>  void inet_frag_kill(struct inet_frag_queue *fq)
>>  {
>>  	if (del_timer(&fq->timer))
>>  		refcount_dec(&fq->refcnt);
>>  
>>  	if (!(fq->flags & INET_FRAG_COMPLETE)) {
>> -		fq_unlink(fq);
>> +		struct netns_frags *nf = fq->net;
>> +
>> +		fq->flags |= INET_FRAG_COMPLETE;
>> +		rhashtable_remove_fast(&nf->rhashtable, &fq->node, nf->f->rhash_params);
>>  		refcount_dec(&fq->refcnt);
>>  	}
>>  }
> 
> This means that the hash won't inline properly.  Don't know big
> of an issue it is to you.  But you could fix it by doing the same
> hack as rhashtable by making inet_frag_kill an inline function and
> take the rhash_params as an explicit argument.

I will investigate this, thanks !

^ permalink raw reply

* r8152 livelocks during pm_runtime_suspend
From: Jiri Slaby @ 2018-03-30 13:17 UTC (permalink / raw)
  To: hayeswang; +Cc: USB list, ML netdev, Rafael J. Wysocki, Linux PM list

Hi,

I have seen r8152 from my docking station to kill my box several times
in the last few days. The notebook is new, so I don't know if this is a
regression.

I have the NIC connected all the time. And when I return to the notebook
after a while, the networking is dead. Looking at the stack traces, it
is clear, that r8152 was attempted to be autosuspended and waits in
napi_disable for NAPI_STATE_SCHED bit to be cleared:
[22001.018437] kworker/2:0     D    0 16267      2 0x80000000
[22001.018441] Workqueue: pm pm_runtime_work
[22001.018443] Call Trace:
[22001.018453]  schedule+0x2f/0x90
[22001.018455]  schedule_timeout+0x1ce/0x540
[22001.018474]  msleep+0x29/0x30
[22001.018477]  napi_disable+0x25/0x60
[22001.018483]  rtl8152_suspend+0x20a/0x2d0 [r8152]
[22001.018493]  usb_suspend_both+0x8d/0x200 [usbcore]
[22001.018510]  usb_runtime_suspend+0x2a/0x70 [usbcore]
[22001.018514]  __rpm_callback+0xbc/0x1f0
[22001.018519]  rpm_callback+0x4f/0x70
[22001.018526]  rpm_suspend+0x11d/0x6d0
[22001.018532]  pm_runtime_work+0x73/0xb0
[22001.018535]  process_one_work+0x269/0x6c0
[22001.018541]  worker_thread+0x2b/0x3d0
[22001.018547]  kthread+0x113/0x130
[22001.018556]  ret_from_fork+0x24/0x50

The assembly:
> ffffffff81716730 <napi_disable>:
> ffffffff81716730:       e8 eb b7 2e 00          callq  ffffffff81a01f20 <__fentry__>
> ffffffff81716735:       55                      push   %rbp
> ffffffff81716736:       48 89 fd                mov    %rdi,%rbp
> ffffffff81716739:       53                      push   %rbx
> ffffffff8171673a:       48 8d 5f 10             lea    0x10(%rdi),%rbx
> ffffffff8171673e:       f0 80 4f 10 04          lock orb $0x4,0x10(%rdi)
> ffffffff81716743:       f0 0f ba 6f 10 00       lock btsl $0x0,0x10(%rdi)
> ffffffff81716749:       73 11                   jae    ffffffff8171675c <napi_disable+0x2c>
> ffffffff8171674b:       bf 01 00 00 00          mov    $0x1,%edi
> ffffffff81716750:       e8 ab ac a0 ff          callq  ffffffff81121400 <msleep>
> ffffffff81716755:       f0 0f ba 2b 00          lock btsl $0x0,(%rbx)
> ffffffff8171675a:       72 ef                   jb     ffffffff8171674b <napi_disable+0x1b>





There are other tasks in D state, of course, like these, waiting for the
device to become pm-up:
[22001.018749] kworker/3:1     D    0 16798      2 0x80000000
[22001.018753] Workqueue: events rtl_work_func_t [r8152]
[22001.018755] Call Trace:
[22001.018767]  schedule+0x2f/0x90
[22001.018769]  rpm_resume+0xf9/0x860
[22001.018777]  rpm_resume+0x592/0x860
[22001.018783]  __pm_runtime_resume+0x3a/0x50
[22001.018789]  usb_autopm_get_interface+0x1d/0x50 [usbcore]
[22001.018793]  rtl_work_func_t+0x3e/0x405 [r8152]
[22001.018801]  process_one_work+0x269/0x6c0
[22001.018807]  worker_thread+0x2b/0x3d0
[22001.018813]  kthread+0x113/0x130
[22001.018822]  ret_from_fork+0x24/0x50
[22001.019713] tcpdump         D    0 17119   4265 0x00000004
[22001.019716] Call Trace:
[22001.019728]  schedule+0x2f/0x90
[22001.019730]  rpm_resume+0xf9/0x860
[22001.019738]  rpm_resume+0x592/0x860
[22001.019744]  __pm_runtime_resume+0x3a/0x50
[22001.019750]  usb_autopm_get_interface+0x1d/0x50 [usbcore]
[22001.019754]  rtl8152_ioctl+0x30/0x140 [r8152]
[22001.019758]  dev_ifsioc+0x115/0x3f0
[22001.019763]  dev_ioctl+0x14b/0x680
[22001.019775]  sock_do_ioctl+0x41/0x50
[22001.019778]  sock_ioctl+0x1c2/0x2f0
[22001.019781]  do_vfs_ioctl+0x91/0x680
[22001.019789]  SyS_ioctl+0x74/0x80
[22001.019794]  do_syscall_64+0x76/0x1c0

...

> Showing all locks held in the system: 
> 1 lock held by in:imklog/1371:
>  #0:  (&f->f_pos_lock){+.+.}, at: [<00000000a0b38807>] __fdget_pos+0x3f/0x50
> 1 lock held by Qt bearer threa/3003:
>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
> 1 lock held by Qt bearer threa/2825:
>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
> 1 lock held by DNS Res~ver #40/17041:
>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
> 1 lock held by Qt bearer threa/3110:
>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
> 1 lock held by DNS Res~ver #16/17044:
>  #0:  (rtnl_mutex){+.+.}, at: [<0000000021e0bca0>] __netlink_dump_start+0x4c/0x1b0
> 2 locks held by bash/4561:
>  #0:  (&tty->ldisc_sem){++++}, at: [<00000000e3d76e61>] tty_ldisc_ref_wait+0x24/0x50
>  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<0000000091462d05>] n_tty_read+0xc3/0x850
> 3 locks held by kworker/2:0/16267:
>  #0:  ((wq_completion)"pm"){+.+.}, at: [<00000000b9dc0832>] process_one_work+0x1e3/0x6c0
>  #1:  ((work_completion)(&dev->power.work)){+.+.}, at: [<00000000b9dc0832>] process_one_work+0x1e3/0x6c0
>  #2:  (&tp->control){+.+.}, at: [<00000000ca575b90>] rtl8152_suspend+0x2b/0x2d0 [r8152]
> 2 locks held by kworker/3:1/16798:
>  #0:  ((wq_completion)"events"){+.+.}, at: [<00000000b9dc0832>] process_one_work+0x1e3/0x6c0
>  #1:  ((work_completion)(&(&tp->schedule)->work)){+.+.}, at: [<00000000b9dc0832>] process_one_work+0x1e3/0x6c0
> 1 lock held by tcpdump/17119: 
>  #0:  (rtnl_mutex){+.+.}, at: [<0000000023a6461d>] dev_ioctl+0x13d/0x680
> 2 locks held by less/17187: 
>  #0:  (&tty->ldisc_sem){++++}, at: [<00000000e3d76e61>] tty_ldisc_ref_wait+0x24/0x50
>  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<0000000091462d05>] n_tty_read+0xc3/0x850


For now, I disabled pm-runtime on the device by:
echo on > /sys/bus/usb/devices/4-1.2/power/control

Any ideas what's wrong? napi_disable from runtime suspend? Double
napi_disable on the path? Some missing pm_runtime_get_sync somewhere?

Full dmesg after sysrq-t:
https://www.fi.muni.cz/~xslaby/sklad/panics/r8152.txt

thanks,
-- 
js
suse labs

^ permalink raw reply

* Re: [PATCH net-next 5/8] net: mscc: Add initial Ocelot switch support
From: Alexandre Belloni @ 2018-03-30 12:45 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: David S . Miller, Allan Nielsen, razvan.stefanescu, po.liu,
	Thomas Petazzoni, Andrew Lunn, netdev, devicetree, linux-kernel,
	linux-mips
In-Reply-To: <1df0a932-f7c1-f1b5-9a35-3c16d0c551e5@gmail.com>

On 23/03/2018 at 14:41:25 -0700, Florian Fainelli wrote:
> On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> > Add a driver for Microsemi Ocelot Ethernet switch support.
> > 
> > This makes two modules:
> > mscc_ocelot_common handles all the common features that doesn't depend on
> > how the switch is integrated in the SoC. Currently, it handles offloading
> > bridging to the hardware. ocelot_io.c handles register accesses. This is
> > unfortunately needed because the register layout is packed and then depends
> > on the number of ports available on the switch. The register definition
> > files are automatically generated.
> > 
> > ocelot_board handles the switch integration on the SoC and on the board.
> > 
> > Frame injection and extraction to/from the CPU port is currently done using
> > register accesses which is quite slow. DMA is possible but the port is not
> > able to absorb the whole switch bandwidth.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
> Random drive by comments because this is quite a number of lines to review!
> 
> Overall, looks quite good for a first version. Out of curiosity, is
> there a particular switch test you ran this driver against? LNST?
> 

We have a really small custom test suite.

> > +	/* Add dummy CRC */
> > +	ocelot_write_rix(ocelot, 0, QS_INJ_WR, grp);
> > +	skb_tx_timestamp(skb);
> > +
> > +	dev->stats.tx_packets++;
> > +	dev->stats.tx_bytes += skb->len;
> > +	dev_kfree_skb_any(skb);
> 
> No interrupt to indicate transmit completion?
> 

No, unfortunately, the TX interrupts only indicates there is room to
start injecting frames, not that they have been transmitted.

> 
> > +static int ocelot_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
> > +			  struct net_device *dev, const unsigned char *addr,
> > +			  u16 vid, u16 flags)
> > +{
> > +	struct ocelot_port *port = netdev_priv(dev);
> > +	struct ocelot *ocelot = port->ocelot;
> > +
> > +	if (!vid) {
> > +		if (!port->vlan_aware)
> > +			/* If the bridge is not VLAN aware and no VID was
> > +			 * provided, set it to 1 as bridges have a default VID
> > +			 * of 1. Otherwise the MAC entry wouldn't match incoming
> > +			 * packets as the VID would differ (0 != 1).
> > +			 */
> > +			vid = 1;
> > +		else
> > +			/* If the bridge is VLAN aware a VID must be provided as
> > +			 * otherwise the learnt entry wouldn't match any frame.
> > +			 */
> > +			return -EINVAL;
> > +	}
> 
> So if we are targeting vid = 0 we end-up with vid = 1 possibly?
> 

I've removed that part that is not needed for now and will rework when
sending VLAN support.

> > +	ocelot_write_gix(ocelot, port_cfg, ANA_PORT_PORT_CFG,
> > +			 ocelot_port->chip_port);
> > +
> > +	/* Apply FWD mask. The loop is needed to add/remove the current port as
> > +	 * a source for the other ports.
> > +	 */
> > +	for (port = 0; port < ocelot->num_phys_ports; port++) {
> > +		if (ocelot->bridge_fwd_mask & BIT(port)) {
> > +			unsigned long mask = ocelot->bridge_fwd_mask & ~BIT(port);
> > +
> > +			for (i = 0; i < ocelot->num_phys_ports; i++) {
> > +				unsigned long bond_mask = ocelot->lags[i];
> > +
> > +				if (!bond_mask)
> > +					continue;
> > +
> > +				if (bond_mask & BIT(port)) {
> > +					mask &= ~bond_mask;
> > +					break;
> > +				}
> > +			}
> > +
> > +			ocelot_write_rix(ocelot,
> > +					 BIT(ocelot->num_phys_ports) | mask,
> > +					 ANA_PGID_PGID, PGID_SRC + port);
> > +		} else {
> > +			/* Only the CPU port, this is compatible with link
> > +			 * aggregation.
> > +			 */
> > +			ocelot_write_rix(ocelot,
> > +					 BIT(ocelot->num_phys_ports),
> > +					 ANA_PGID_PGID, PGID_SRC + port);
> > +		}
> 
> All of this sounds like it should be moved into the br_join/leave, this
> does not appear to be the right place to do that.
> 

No, I've triple checked because this is a comment that both Andrew and
you had. Once a port is added to the PGID MASK, it will start forwarding
frames so we really want that to happen only when the port is in
BR_STATE_FORWARDING state. Else, we may forward frames between the
addition of the port to the bridge and setting the port to the
BR_STATE_BLOCKING state.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* [PATCH v2 7/7] net: thunderx: add ndo_set_rx_mode callback implementation for VF
From: Vadim Lomovtsev @ 2018-03-30 11:59 UTC (permalink / raw)
  To: sgoutham, sunil.kovvuri, rric, linux-arm-kernel, netdev,
	linux-kernel, davem
  Cc: dnelson, ynorov, Vadim Lomovtsev
In-Reply-To: <20180330115953.17154-1-Vadim.Lomovtsev@caviumnetworks.com>

From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>

The ndo_set_rx_mode() is called from atomic context which causes
messages response timeouts while VF to PF communication via MSIx.
To get rid of that we're copy passed mc list, parse flags and queue
handling of kernel request to ordered workqueue.

Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 110 ++++++++++++++++++++++-
 1 file changed, 109 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 7d9c5ffbd041..c8a8faaf17e9 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -21,6 +21,7 @@
 #include <linux/bpf_trace.h>
 #include <linux/filter.h>
 #include <linux/net_tstamp.h>
+#include <linux/workqueue.h>
 
 #include "nic_reg.h"
 #include "nic.h"
@@ -67,6 +68,9 @@ module_param(cpi_alg, int, S_IRUGO);
 MODULE_PARM_DESC(cpi_alg,
 		 "PFC algorithm (0=none, 1=VLAN, 2=VLAN16, 3=IP Diffserv)");
 
+/* workqueue for handling kernel ndo_set_rx_mode() calls */
+static struct workqueue_struct *nicvf_rx_mode_wq;
+
 static inline u8 nicvf_netdev_qidx(struct nicvf *nic, u8 qidx)
 {
 	if (nic->sqs_mode)
@@ -1919,6 +1923,100 @@ static int nicvf_ioctl(struct net_device *netdev, struct ifreq *req, int cmd)
 	}
 }
 
+static void nicvf_set_rx_mode_task(struct work_struct *work_arg)
+{
+	struct nicvf_work *vf_work = container_of(work_arg, struct nicvf_work,
+						  work.work);
+	struct nicvf *nic = container_of(vf_work, struct nicvf, rx_mode_work);
+	union nic_mbx mbx = {};
+	struct xcast_addr *xaddr, *next;
+
+	if (!vf_work)
+		return;
+
+	/* From the inside of VM code flow we have only 128 bits memory
+	 * available to send message to host's PF, so send all mc addrs
+	 * one by one, starting from flush command in case if kernel
+	 * requests to configure specific MAC filtering
+	 */
+
+	/* flush DMAC filters and reset RX mode */
+	mbx.xcast.msg = NIC_MBOX_MSG_RESET_XCAST;
+	nicvf_send_msg_to_pf(nic, &mbx);
+
+	if (vf_work->mode & BGX_XCAST_MCAST_FILTER) {
+		/* once enabling filtering, we need to signal to PF to add
+		 * its' own LMAC to the filter to accept packets for it.
+		 */
+		mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
+		mbx.xcast.data.mac = 0;
+		nicvf_send_msg_to_pf(nic, &mbx);
+	}
+
+	/* check if we have any specific MACs to be added to PF DMAC filter */
+	if (vf_work->mc) {
+		/* now go through kernel list of MACs and add them one by one */
+		list_for_each_entry_safe(xaddr, next,
+					 &vf_work->mc->list, list) {
+			mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
+			mbx.xcast.data.mac = xaddr->addr;
+			nicvf_send_msg_to_pf(nic, &mbx);
+
+			/* after receiving ACK from PF release memory */
+			list_del(&xaddr->list);
+			kfree(xaddr);
+			vf_work->mc->count--;
+		}
+		kfree(vf_work->mc);
+	}
+
+	/* and finally set rx mode for PF accordingly */
+	mbx.xcast.msg = NIC_MBOX_MSG_SET_XCAST;
+	mbx.xcast.data.mode = vf_work->mode;
+
+	nicvf_send_msg_to_pf(nic, &mbx);
+}
+
+static void nicvf_set_rx_mode(struct net_device *netdev)
+{
+	struct nicvf *nic = netdev_priv(netdev);
+	struct netdev_hw_addr *ha;
+	struct xcast_addr_list *mc_list = NULL;
+	u8 mode = 0;
+
+	if (netdev->flags & IFF_PROMISC) {
+		mode = BGX_XCAST_BCAST_ACCEPT | BGX_XCAST_MCAST_ACCEPT;
+	} else {
+		if (netdev->flags & IFF_BROADCAST)
+			mode |= BGX_XCAST_BCAST_ACCEPT;
+
+		if (netdev->flags & IFF_ALLMULTI) {
+			mode |= BGX_XCAST_MCAST_ACCEPT;
+		} else if (netdev->flags & IFF_MULTICAST) {
+			mode |= BGX_XCAST_MCAST_FILTER;
+			/* here we need to copy mc addrs */
+			if (netdev_mc_count(netdev)) {
+				struct xcast_addr *xaddr;
+
+				mc_list = kmalloc(sizeof(*mc_list), GFP_ATOMIC);
+				INIT_LIST_HEAD(&mc_list->list);
+				netdev_hw_addr_list_for_each(ha, &netdev->mc) {
+					xaddr = kmalloc(sizeof(*xaddr),
+							GFP_ATOMIC);
+					xaddr->addr =
+						ether_addr_to_u64(ha->addr);
+					list_add_tail(&xaddr->list,
+						      &mc_list->list);
+					mc_list->count++;
+				}
+			}
+		}
+	}
+	nic->rx_mode_work.mc = mc_list;
+	nic->rx_mode_work.mode = mode;
+	queue_delayed_work(nicvf_rx_mode_wq, &nic->rx_mode_work.work, 2 * HZ);
+}
+
 static const struct net_device_ops nicvf_netdev_ops = {
 	.ndo_open		= nicvf_open,
 	.ndo_stop		= nicvf_stop,
@@ -1931,6 +2029,7 @@ static const struct net_device_ops nicvf_netdev_ops = {
 	.ndo_set_features       = nicvf_set_features,
 	.ndo_bpf		= nicvf_xdp,
 	.ndo_do_ioctl           = nicvf_ioctl,
+	.ndo_set_rx_mode        = nicvf_set_rx_mode,
 };
 
 static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
@@ -2071,6 +2170,8 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	INIT_WORK(&nic->reset_task, nicvf_reset_task);
 
+	INIT_DELAYED_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task);
+
 	err = register_netdev(netdev);
 	if (err) {
 		dev_err(dev, "Failed to register netdevice\n");
@@ -2109,6 +2210,8 @@ static void nicvf_remove(struct pci_dev *pdev)
 	nic = netdev_priv(netdev);
 	pnetdev = nic->pnicvf->netdev;
 
+	cancel_delayed_work_sync(&nic->rx_mode_work.work);
+
 	/* Check if this Qset is assigned to different VF.
 	 * If yes, clean primary and all secondary Qsets.
 	 */
@@ -2140,12 +2243,17 @@ static struct pci_driver nicvf_driver = {
 static int __init nicvf_init_module(void)
 {
 	pr_info("%s, ver %s\n", DRV_NAME, DRV_VERSION);
-
+	nicvf_rx_mode_wq = alloc_ordered_workqueue("nicvf_generic",
+						   WQ_MEM_RECLAIM);
 	return pci_register_driver(&nicvf_driver);
 }
 
 static void __exit nicvf_cleanup_module(void)
 {
+	if (nicvf_rx_mode_wq) {
+		destroy_workqueue(nicvf_rx_mode_wq);
+		nicvf_rx_mode_wq = NULL;
+	}
 	pci_unregister_driver(&nicvf_driver);
 }
 
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 6/7] net: thunderx: add workqueue control structures for handle ndo_set_rx_mode request
From: Vadim Lomovtsev @ 2018-03-30 11:59 UTC (permalink / raw)
  To: sgoutham, sunil.kovvuri, rric, linux-arm-kernel, netdev,
	linux-kernel, davem
  Cc: dnelson, ynorov, Vadim Lomovtsev
In-Reply-To: <20180330115953.17154-1-Vadim.Lomovtsev@caviumnetworks.com>

From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>

The kernel calls ndo_set_rx_mode() callback from atomic context which
causes messaging timeouts between VF and PF (as they’re implemented via
MSIx). So in order to handle ndo_set_rx_mode() we need to get rid of it.

This commit implements necessary workqueue related structures to let VF
queue kernel request processing in non-atomic context later.

Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
---
 drivers/net/ethernet/cavium/thunder/nic.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 069289b4f968..5fc46c5a4f36 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -265,6 +265,22 @@ struct nicvf_drv_stats {
 
 struct cavium_ptp;
 
+struct xcast_addr {
+	struct list_head list;
+	u64              addr;
+};
+
+struct xcast_addr_list {
+	struct list_head list;
+	int              count;
+};
+
+struct nicvf_work {
+	struct delayed_work    work;
+	u8                     mode;
+	struct xcast_addr_list *mc;
+};
+
 struct nicvf {
 	struct nicvf		*pnicvf;
 	struct net_device	*netdev;
@@ -313,6 +329,7 @@ struct nicvf {
 	struct nicvf_pfc	pfc;
 	struct tasklet_struct	qs_err_task;
 	struct work_struct	reset_task;
+	struct nicvf_work       rx_mode_work;
 
 	/* PTP timestamp */
 	struct cavium_ptp	*ptp_clock;
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 5/7] net: thunderx: add XCAST messages handlers for PF
From: Vadim Lomovtsev @ 2018-03-30 11:59 UTC (permalink / raw)
  To: sgoutham, sunil.kovvuri, rric, linux-arm-kernel, netdev,
	linux-kernel, davem
  Cc: dnelson, ynorov, Vadim Lomovtsev
In-Reply-To: <20180330115953.17154-1-Vadim.Lomovtsev@caviumnetworks.com>

From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>

This commit is to add message handling for ndo_set_rx_mode()
callback at PF side.

Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
---
 drivers/net/ethernet/cavium/thunder/nic_main.c | 45 +++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 7ff66a8194e2..55af04fa03a7 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -21,6 +21,8 @@
 #define DRV_NAME	"nicpf"
 #define DRV_VERSION	"1.0"
 
+#define NIC_VF_PER_MBX_REG      64
+
 struct hw_info {
 	u8		bgx_cnt;
 	u8		chans_per_lmac;
@@ -1072,6 +1074,40 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
 	case NIC_MBOX_MSG_PTP_CFG:
 		nic_config_timestamp(nic, vf, &mbx.ptp);
 		break;
+	case NIC_MBOX_MSG_RESET_XCAST:
+		if (vf >= nic->num_vf_en) {
+			ret = -1; /* NACK */
+			break;
+		}
+		bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+		lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+		bgx_reset_xcast_mode(nic->node, bgx, lmac,
+				     vf < NIC_VF_PER_MBX_REG ? vf :
+				     vf - NIC_VF_PER_MBX_REG);
+		break;
+
+	case NIC_MBOX_MSG_ADD_MCAST:
+		if (vf >= nic->num_vf_en) {
+			ret = -1; /* NACK */
+			break;
+		}
+		bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+		lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+		bgx_set_dmac_cam_filter(nic->node, bgx, lmac,
+					mbx.xcast.data.mac,
+					vf < NIC_VF_PER_MBX_REG ? vf :
+					vf - NIC_VF_PER_MBX_REG);
+		break;
+
+	case NIC_MBOX_MSG_SET_XCAST:
+		if (vf >= nic->num_vf_en) {
+			ret = -1; /* NACK */
+			break;
+		}
+		bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+		lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+		bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.data.mode);
+		break;
 	default:
 		dev_err(&nic->pdev->dev,
 			"Invalid msg from VF%d, msg 0x%x\n", vf, mbx.msg.msg);
@@ -1094,7 +1130,7 @@ static irqreturn_t nic_mbx_intr_handler(int irq, void *nic_irq)
 	struct nicpf *nic = (struct nicpf *)nic_irq;
 	int mbx;
 	u64 intr;
-	u8  vf, vf_per_mbx_reg = 64;
+	u8  vf;
 
 	if (irq == pci_irq_vector(nic->pdev, NIC_PF_INTR_ID_MBOX0))
 		mbx = 0;
@@ -1103,12 +1139,13 @@ static irqreturn_t nic_mbx_intr_handler(int irq, void *nic_irq)
 
 	intr = nic_reg_read(nic, NIC_PF_MAILBOX_INT + (mbx << 3));
 	dev_dbg(&nic->pdev->dev, "PF interrupt Mbox%d 0x%llx\n", mbx, intr);
-	for (vf = 0; vf < vf_per_mbx_reg; vf++) {
+	for (vf = 0; vf < NIC_VF_PER_MBX_REG; vf++) {
 		if (intr & (1ULL << vf)) {
 			dev_dbg(&nic->pdev->dev, "Intr from VF %d\n",
-				vf + (mbx * vf_per_mbx_reg));
+				vf + (mbx * NIC_VF_PER_MBX_REG));
 
-			nic_handle_mbx_intr(nic, vf + (mbx * vf_per_mbx_reg));
+			nic_handle_mbx_intr(nic, vf +
+					    (mbx * NIC_VF_PER_MBX_REG));
 			nic_clear_mbx_intr(nic, vf, mbx);
 		}
 	}
-- 
2.14.3

^ permalink raw reply related

* [PATCH v2 4/7] net: thunderx: add new messages for handle ndo_set_rx_mode callback
From: Vadim Lomovtsev @ 2018-03-30 11:59 UTC (permalink / raw)
  To: sgoutham, sunil.kovvuri, rric, linux-arm-kernel, netdev,
	linux-kernel, davem
  Cc: dnelson, ynorov, Vadim Lomovtsev
In-Reply-To: <20180330115953.17154-1-Vadim.Lomovtsev@caviumnetworks.com>

From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>

The kernel calls ndo_set_rx_mode() callback supplying it will all necessary
info, such as device state flags, multicast mac addresses list and so on.
Since we have only 128 bits to communicate with PF we need to initiate
several requests to PF with small/short operation each based on input data.

So this commit implements following PF messages codes along with new
data structures for them:
NIC_MBOX_MSG_RESET_XCAST to flush all filters configured for this
                          particular network interface (VF)
NIC_MBOX_MSG_ADD_MCAST   to add new MAC address to DMAC filter registers
                          for this particular network interface (VF)
NIC_MBOX_MSG_SET_XCAST   to apply filtering configuration to filter control
                          register

Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
---
 drivers/net/ethernet/cavium/thunder/nic.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 4cacce5d2b16..069289b4f968 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -403,6 +403,9 @@ struct nicvf {
 #define	NIC_MBOX_MSG_PTP_CFG		0x19	/* HW packet timestamp */
 #define	NIC_MBOX_MSG_CFG_DONE		0xF0	/* VF configuration done */
 #define	NIC_MBOX_MSG_SHUTDOWN		0xF1	/* VF is being shutdown */
+#define	NIC_MBOX_MSG_RESET_XCAST	0xF2    /* Reset DCAM filtering mode */
+#define	NIC_MBOX_MSG_ADD_MCAST		0xF3    /* Add MAC to DCAM filters */
+#define	NIC_MBOX_MSG_SET_XCAST		0xF4    /* Set MCAST/BCAST RX mode */
 
 struct nic_cfg_msg {
 	u8    msg;
@@ -556,6 +559,14 @@ struct set_ptp {
 	bool  enable;
 };
 
+struct xcast {
+	u8    msg;
+	union {
+		u8    mode;
+		u64   mac;
+	} data;
+};
+
 /* 128 bit shared memory between PF and each VF */
 union nic_mbx {
 	struct { u8 msg; }	msg;
@@ -576,6 +587,7 @@ union nic_mbx {
 	struct reset_stat_cfg	reset_stat;
 	struct pfc		pfc;
 	struct set_ptp		ptp;
+	struct xcast            xcast;
 };
 
 #define NIC_NODE_ID_MASK	0x03
-- 
2.14.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox