Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Willem de Bruijn @ 2017-08-14 19:03 UTC (permalink / raw)
  To: Thiago Macieira; +Cc: Matthew Dawson, Paolo Abeni, Network Development
In-Reply-To: <1756525.aGoekHffaC@tjmaciei-mobl1>

On Mon, Aug 14, 2017 at 2:58 PM, Thiago Macieira
<thiago.macieira@intel.com> wrote:
> On Monday, 14 August 2017 11:46:42 PDT Willem de Bruijn wrote:
>> > By the way, what were the usecases for the peek offset feature?
>>
>> The idea was to be able to peek at application headers of upper
>> layer protocols and multiplex messages among threads. It proved
>> so complex even for UDP that we did not attempt the same feature
>> for TCP. Also, KCM implements demultiplexing using eBPF today.
>
> Interesting, but how would userspace coordinate like that? Suppose multiple
> threads are woken up by a datagram being received

This assumes a separate listener thread and worker threadpool.

^ permalink raw reply

* Re: [PATCH net] ipv6: release rt6->rt6i_idev properly during ifdown
From: David Ahern @ 2017-08-14 19:05 UTC (permalink / raw)
  To: Wei Wang, David Miller, netdev; +Cc: Martin KaFai Lau
In-Reply-To: <20170814174459.3569-1-tracywwnj@gmail.com>

On 8/14/17 11:44 AM, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
> 
> When a dst is created by addrconf_dst_alloc() for a host route or an
> anycast route, dst->dev points to loopback dev while rt6->rt6i_idev
> points to a real device.
> When the real device goes down, the current cleanup code only checks for
> dst->dev and assumes rt6->rt6i_idev->dev is the same. This causes the
> refcount leak on the real device in the above situation.
> This patch makes sure to always release the refcount taken on
> rt6->rt6i_idev during dst_dev_put().
> 
> Fixes: 587fea741134 ("ipv6: mark DST_NOGC and remove the operation of
> dst_free()")
> Reported-by: John Stultz <john.stultz@linaro.org>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Tested-by: Martin KaFai Lau <kafai@fb.com>
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>  net/ipv6/route.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH 5/6] [net-next]net: i40e: Clean up of cloud filters
From: Nambiar, Amritha @ 2017-08-14 19:06 UTC (permalink / raw)
  To: Shannon Nelson, intel-wired-lan, jeffrey.t.kirsher
  Cc: netdev, mitch.a.williams
In-Reply-To: <d0d01a6f-0a3b-cbc0-4ef6-a1c8af91ba93@oracle.com>

On 8/1/2017 12:16 PM, Shannon Nelson wrote:
> On 7/31/2017 5:38 PM, Amritha Nambiar wrote:
>> Introduce the cloud filter datastructure and cleanup of cloud
>> filters associated with the device.
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>> ---
>>   drivers/net/ethernet/intel/i40e/i40e.h      |   11 +++++++++++
>>   drivers/net/ethernet/intel/i40e/i40e_main.c |   27 +++++++++++++++++++++++++++
>>   2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>> index 1391e5d..5c0cad5 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -252,6 +252,14 @@ struct i40e_fdir_filter {
>>   	u32 fd_id;
>>   };
>>   
>> +struct i40e_cloud_filter {
>> +	struct hlist_node cloud_node;
>> +	/* cloud filter input set follows */
>> +	unsigned long cookie;
>> +	/* filter control */
>> +	u16 seid;
>> +};
> 
> This would be cleaner and more readable with the field comments off to 
> the side rather than in line with the fields.

Will fix in the next version of the series.

> 
>> +
>>   #define I40E_ETH_P_LLDP			0x88cc
>>   
>>   #define I40E_DCB_PRIO_TYPE_STRICT	0
>> @@ -419,6 +427,9 @@ struct i40e_pf {
>>   	struct i40e_udp_port_config udp_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
>>   	u16 pending_udp_bitmap;
>>   
>> +	struct hlist_head cloud_filter_list;
>> +	u16 num_cloud_filters;
>> +
>>   	enum i40e_interrupt_policy int_policy;
>>   	u16 rx_itr_default;
>>   	u16 tx_itr_default;
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index fdddd74..93f6fe2 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -6928,6 +6928,29 @@ static void i40e_fdir_filter_exit(struct i40e_pf *pf)
>>   }
>>   
>>   /**
>> + * i40e_cloud_filter_exit - Cleans up the Cloud Filters
>> + * @pf: Pointer to PF
>> + *
>> + * This function destroys the hlist where all the Cloud Filters
>> + * filters were saved.
>> + **/
>> +static void i40e_cloud_filter_exit(struct i40e_pf *pf)
>> +{
>> +	struct i40e_cloud_filter *cfilter;
>> +	struct hlist_node *node;
>> +
>> +	if (hlist_empty(&pf->cloud_filter_list))
>> +		return;
> 
> Is this check really necessary?  Doesn't hlist_for_each_entry_safe() 
> check for this?

That's right. Will fix in the next version of the series.

> 
>> +
>> +	hlist_for_each_entry_safe(cfilter, node,
>> +				  &pf->cloud_filter_list, cloud_node) {
>> +		hlist_del(&cfilter->cloud_node);
>> +		kfree(cfilter);
>> +	}
>> +	pf->num_cloud_filters = 0;
>> +}
>> +
>> +/**
>>    * i40e_close - Disables a network interface
>>    * @netdev: network interface device structure
>>    *
>> @@ -12137,6 +12160,7 @@ static int i40e_setup_pf_switch(struct i40e_pf *pf, bool reinit)
>>   			vsi = i40e_vsi_reinit_setup(pf->vsi[pf->lan_vsi]);
>>   		if (!vsi) {
>>   			dev_info(&pf->pdev->dev, "setup of MAIN VSI failed\n");
>> +			i40e_cloud_filter_exit(pf);
>>   			i40e_fdir_teardown(pf);
>>   			return -EAGAIN;
>>   		}
>> @@ -12961,6 +12985,8 @@ static void i40e_remove(struct pci_dev *pdev)
>>   	if (pf->vsi[pf->lan_vsi])
>>   		i40e_vsi_release(pf->vsi[pf->lan_vsi]);
>>   
>> +	i40e_cloud_filter_exit(pf);
>> +
>>   	/* remove attached clients */
>>   	if (pf->flags & I40E_FLAG_IWARP_ENABLED) {
>>   		ret_code = i40e_lan_del_device(pf);
>> @@ -13170,6 +13196,7 @@ static void i40e_shutdown(struct pci_dev *pdev)
>>   
>>   	del_timer_sync(&pf->service_timer);
>>   	cancel_work_sync(&pf->service_task);
>> +	i40e_cloud_filter_exit(pf);
>>   	i40e_fdir_teardown(pf);
>>   
>>   	/* Client close must be called explicitly here because the timer
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>>

^ permalink raw reply

* [patch net-next 0/2] mlxsw: Add support for nexthop group consolidation for IPv6
From: Jiri Pirko @ 2017-08-14 19:09 UTC (permalink / raw)
  To: netdev; +Cc: davem, arkadis, idosch, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Arkadi says:

Due to limited ASIC resources the maximum number of routes is limited by
the nexthop resource. In order to improve the routing scale nexthop
consolidation should be performed.

In case of IPv4, the kernel does the consolidation of nexthops in the form
of the fib_info struct. In that case, the driver uses the fib_info's
address as a key for the internal nexthop group representative struct
lookup. In case of IPv6, the kernel doesn't do consolidation, thus the
driver should implement it by itself.

The hash value is calculated based on the nexthop set, by performing
bitwise xor on the ifindexs of the nexthops, in a similar way to IPV4's
kernel implementation. In case of collision a full match is performed
between the sets which include address and ifindex comparison.

In order to use the same hash table in both cases (IPv4/6), the rhashtable
is changed to operate on variable length key.

Arkadi Sharshevsky (2):
  mlxsw: spectrum_router: Prepare nexthop group's hash table for IPv6
  mlxsw: spectrum_router: Add support for nexthop group consolidation
    for IPv6

 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 209 ++++++++++++++++++---
 1 file changed, 188 insertions(+), 21 deletions(-)

-- 
2.9.3

^ permalink raw reply

* [patch net-next 1/2] mlxsw: spectrum_router: Prepare nexthop group's hash table for IPv6
From: Jiri Pirko @ 2017-08-14 19:09 UTC (permalink / raw)
  To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170814190920.9576-1-jiri@resnulli.us>

From: Arkadi Sharshevsky <arkadis@mellanox.com>

This patch does preparation before introducing IPv6 nexthop group
consolidation. Currently the nexthop group hash table is used only by
IPv4 and uses fixed key size. In order to support the IPv6's variable
length key the current table is changed.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 69 ++++++++++++++++------
 1 file changed, 52 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 3d9be36..5100429 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1522,15 +1522,11 @@ struct mlxsw_sp_nexthop {
 	struct mlxsw_sp_neigh_entry *neigh_entry;
 };
 
-struct mlxsw_sp_nexthop_group_key {
-	struct fib_info *fi;
-};
-
 struct mlxsw_sp_nexthop_group {
+	void *priv;
 	struct rhash_head ht_node;
 	struct list_head fib_list; /* list of fib entries that use this group */
 	struct neigh_table *neigh_tbl;
-	struct mlxsw_sp_nexthop_group_key key;
 	u8 adj_index_valid:1,
 	   gateway:1; /* routes using the group use a gateway */
 	u32 adj_index;
@@ -1540,10 +1536,46 @@ struct mlxsw_sp_nexthop_group {
 #define nh_rif	nexthops[0].rif
 };
 
+static struct fib_info *
+mlxsw_sp_nexthop4_group_fi(const struct mlxsw_sp_nexthop_group *nh_grp)
+{
+	return nh_grp->priv;
+}
+
+struct mlxsw_sp_nexthop_group_cmp_arg {
+	struct fib_info *fi;
+};
+
+static int
+mlxsw_sp_nexthop_group_cmp(struct rhashtable_compare_arg *arg, const void *ptr)
+{
+	const struct mlxsw_sp_nexthop_group_cmp_arg *cmp_arg = arg->key;
+	const struct mlxsw_sp_nexthop_group *nh_grp = ptr;
+
+	return cmp_arg->fi != mlxsw_sp_nexthop4_group_fi(nh_grp);
+}
+
+static u32 mlxsw_sp_nexthop_group_hash_obj(const void *data, u32 len, u32 seed)
+{
+	const struct mlxsw_sp_nexthop_group *nh_grp = data;
+	struct fib_info *fi = mlxsw_sp_nexthop4_group_fi(nh_grp);
+
+	return jhash(&fi, sizeof(fi), seed);
+}
+
+static u32
+mlxsw_sp_nexthop_group_hash(const void *data, u32 len, u32 seed)
+{
+	const struct mlxsw_sp_nexthop_group_cmp_arg *cmp_arg = data;
+
+	return jhash(&cmp_arg->fi, sizeof(cmp_arg->fi), seed);
+}
+
 static const struct rhashtable_params mlxsw_sp_nexthop_group_ht_params = {
-	.key_offset = offsetof(struct mlxsw_sp_nexthop_group, key),
 	.head_offset = offsetof(struct mlxsw_sp_nexthop_group, ht_node),
-	.key_len = sizeof(struct mlxsw_sp_nexthop_group_key),
+	.hashfn	     = mlxsw_sp_nexthop_group_hash,
+	.obj_hashfn  = mlxsw_sp_nexthop_group_hash_obj,
+	.obj_cmpfn   = mlxsw_sp_nexthop_group_cmp,
 };
 
 static int mlxsw_sp_nexthop_group_insert(struct mlxsw_sp *mlxsw_sp,
@@ -1563,10 +1595,14 @@ static void mlxsw_sp_nexthop_group_remove(struct mlxsw_sp *mlxsw_sp,
 }
 
 static struct mlxsw_sp_nexthop_group *
-mlxsw_sp_nexthop_group_lookup(struct mlxsw_sp *mlxsw_sp,
-			      struct mlxsw_sp_nexthop_group_key key)
+mlxsw_sp_nexthop4_group_lookup(struct mlxsw_sp *mlxsw_sp,
+			       struct fib_info *fi)
 {
-	return rhashtable_lookup_fast(&mlxsw_sp->router->nexthop_group_ht, &key,
+	struct mlxsw_sp_nexthop_group_cmp_arg cmp_arg;
+
+	cmp_arg.fi = fi;
+	return rhashtable_lookup_fast(&mlxsw_sp->router->nexthop_group_ht,
+				      &cmp_arg,
 				      mlxsw_sp_nexthop_group_ht_params);
 }
 
@@ -2063,12 +2099,12 @@ mlxsw_sp_nexthop4_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
 	nh_grp = kzalloc(alloc_size, GFP_KERNEL);
 	if (!nh_grp)
 		return ERR_PTR(-ENOMEM);
+	nh_grp->priv = fi;
 	INIT_LIST_HEAD(&nh_grp->fib_list);
 	nh_grp->neigh_tbl = &arp_tbl;
 
 	nh_grp->gateway = fi->fib_nh->nh_scope == RT_SCOPE_LINK;
 	nh_grp->count = fi->fib_nhs;
-	nh_grp->key.fi = fi;
 	fib_info_hold(fi);
 	for (i = 0; i < nh_grp->count; i++) {
 		nh = &nh_grp->nexthops[i];
@@ -2089,7 +2125,7 @@ mlxsw_sp_nexthop4_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
 		nh = &nh_grp->nexthops[i];
 		mlxsw_sp_nexthop4_fini(mlxsw_sp, nh);
 	}
-	fib_info_put(nh_grp->key.fi);
+	fib_info_put(fi);
 	kfree(nh_grp);
 	return ERR_PTR(err);
 }
@@ -2108,7 +2144,7 @@ mlxsw_sp_nexthop4_group_destroy(struct mlxsw_sp *mlxsw_sp,
 	}
 	mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh_grp);
 	WARN_ON_ONCE(nh_grp->adj_index_valid);
-	fib_info_put(nh_grp->key.fi);
+	fib_info_put(mlxsw_sp_nexthop4_group_fi(nh_grp));
 	kfree(nh_grp);
 }
 
@@ -2116,11 +2152,9 @@ static int mlxsw_sp_nexthop4_group_get(struct mlxsw_sp *mlxsw_sp,
 				       struct mlxsw_sp_fib_entry *fib_entry,
 				       struct fib_info *fi)
 {
-	struct mlxsw_sp_nexthop_group_key key;
 	struct mlxsw_sp_nexthop_group *nh_grp;
 
-	key.fi = fi;
-	nh_grp = mlxsw_sp_nexthop_group_lookup(mlxsw_sp, key);
+	nh_grp = mlxsw_sp_nexthop4_group_lookup(mlxsw_sp, fi);
 	if (!nh_grp) {
 		nh_grp = mlxsw_sp_nexthop4_group_create(mlxsw_sp, fi);
 		if (IS_ERR(nh_grp))
@@ -2551,7 +2585,8 @@ mlxsw_sp_fib4_entry_lookup(struct mlxsw_sp *mlxsw_sp,
 		if (fib4_entry->tb_id == fen_info->tb_id &&
 		    fib4_entry->tos == fen_info->tos &&
 		    fib4_entry->type == fen_info->type &&
-		    fib4_entry->common.nh_group->key.fi == fen_info->fi) {
+		    mlxsw_sp_nexthop4_group_fi(fib4_entry->common.nh_group) ==
+		    fen_info->fi) {
 			return fib4_entry;
 		}
 	}
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 2/2] mlxsw: spectrum_router: Add support for nexthop group consolidation for IPv6
From: Jiri Pirko @ 2017-08-14 19:09 UTC (permalink / raw)
  To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170814190920.9576-1-jiri@resnulli.us>

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Due to limited ASIC resources the maximum number of routes is limited by
the nexthop resource. In order to improve the routing scale nexthop
consolidation should be performed.

This patch adds support for IPv6 neighbor consolidation. The hash value
is calculated based on the nexthop set, by performing bitwise xor on the
ifindexs of the nexthops, in a similar way to IPv4's kernel implementation.
In case of collision a full match is performed between the sets which
include address and ifindex comparison.

Non gateway nexthop groups are not inserted to the hash table due to
lack of nexthop device (ifindex).

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 150 +++++++++++++++++++--
 1 file changed, 141 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 5100429..16676ff 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1509,6 +1509,7 @@ struct mlxsw_sp_nexthop {
 	struct rhash_head ht_node;
 	struct mlxsw_sp_nexthop_key key;
 	unsigned char gw_addr[sizeof(struct in6_addr)];
+	int ifindex;
 	struct mlxsw_sp_rif *rif;
 	u8 should_offload:1, /* set indicates this neigh is connected and
 			      * should be put to KVD linear area of this group.
@@ -1543,24 +1544,115 @@ mlxsw_sp_nexthop4_group_fi(const struct mlxsw_sp_nexthop_group *nh_grp)
 }
 
 struct mlxsw_sp_nexthop_group_cmp_arg {
-	struct fib_info *fi;
+	enum mlxsw_sp_l3proto proto;
+	union {
+		struct fib_info *fi;
+		struct mlxsw_sp_fib6_entry *fib6_entry;
+	};
 };
 
+static bool
+mlxsw_sp_nexthop6_group_has_nexthop(const struct mlxsw_sp_nexthop_group *nh_grp,
+				    const struct in6_addr *gw, int ifindex)
+{
+	int i;
+
+	for (i = 0; i < nh_grp->count; i++) {
+		const struct mlxsw_sp_nexthop *nh;
+
+		nh = &nh_grp->nexthops[i];
+		if (nh->ifindex == ifindex &&
+		    ipv6_addr_equal(gw, (struct in6_addr *) nh->gw_addr))
+			return true;
+	}
+
+	return false;
+}
+
+static bool
+mlxsw_sp_nexthop6_group_cmp(const struct mlxsw_sp_nexthop_group *nh_grp,
+			    const struct mlxsw_sp_fib6_entry *fib6_entry)
+{
+	struct mlxsw_sp_rt6 *mlxsw_sp_rt6;
+
+	if (nh_grp->count != fib6_entry->nrt6)
+		return false;
+
+	list_for_each_entry(mlxsw_sp_rt6, &fib6_entry->rt6_list, list) {
+		struct in6_addr *gw;
+		int ifindex;
+
+		ifindex = mlxsw_sp_rt6->rt->dst.dev->ifindex;
+		gw = &mlxsw_sp_rt6->rt->rt6i_gateway;
+		if (!mlxsw_sp_nexthop6_group_has_nexthop(nh_grp, gw, ifindex))
+			return false;
+	}
+
+	return true;
+}
+
 static int
 mlxsw_sp_nexthop_group_cmp(struct rhashtable_compare_arg *arg, const void *ptr)
 {
 	const struct mlxsw_sp_nexthop_group_cmp_arg *cmp_arg = arg->key;
 	const struct mlxsw_sp_nexthop_group *nh_grp = ptr;
 
-	return cmp_arg->fi != mlxsw_sp_nexthop4_group_fi(nh_grp);
+	switch (cmp_arg->proto) {
+	case MLXSW_SP_L3_PROTO_IPV4:
+		return cmp_arg->fi != mlxsw_sp_nexthop4_group_fi(nh_grp);
+	case MLXSW_SP_L3_PROTO_IPV6:
+		return !mlxsw_sp_nexthop6_group_cmp(nh_grp,
+						    cmp_arg->fib6_entry);
+	default:
+		WARN_ON(1);
+		return 1;
+	}
+}
+
+static int
+mlxsw_sp_nexthop_group_type(const struct mlxsw_sp_nexthop_group *nh_grp)
+{
+	return nh_grp->neigh_tbl->family;
 }
 
 static u32 mlxsw_sp_nexthop_group_hash_obj(const void *data, u32 len, u32 seed)
 {
 	const struct mlxsw_sp_nexthop_group *nh_grp = data;
-	struct fib_info *fi = mlxsw_sp_nexthop4_group_fi(nh_grp);
+	const struct mlxsw_sp_nexthop *nh;
+	struct fib_info *fi;
+	unsigned int val;
+	int i;
+
+	switch (mlxsw_sp_nexthop_group_type(nh_grp)) {
+	case AF_INET:
+		fi = mlxsw_sp_nexthop4_group_fi(nh_grp);
+		return jhash(&fi, sizeof(fi), seed);
+	case AF_INET6:
+		val = nh_grp->count;
+		for (i = 0; i < nh_grp->count; i++) {
+			nh = &nh_grp->nexthops[i];
+			val ^= nh->ifindex;
+		}
+		return jhash(&val, sizeof(val), seed);
+	default:
+		WARN_ON(1);
+		return 0;
+	}
+}
+
+static u32
+mlxsw_sp_nexthop6_group_hash(struct mlxsw_sp_fib6_entry *fib6_entry, u32 seed)
+{
+	unsigned int val = fib6_entry->nrt6;
+	struct mlxsw_sp_rt6 *mlxsw_sp_rt6;
+	struct net_device *dev;
+
+	list_for_each_entry(mlxsw_sp_rt6, &fib6_entry->rt6_list, list) {
+		dev = mlxsw_sp_rt6->rt->dst.dev;
+		val ^= dev->ifindex;
+	}
 
-	return jhash(&fi, sizeof(fi), seed);
+	return jhash(&val, sizeof(val), seed);
 }
 
 static u32
@@ -1568,7 +1660,15 @@ mlxsw_sp_nexthop_group_hash(const void *data, u32 len, u32 seed)
 {
 	const struct mlxsw_sp_nexthop_group_cmp_arg *cmp_arg = data;
 
-	return jhash(&cmp_arg->fi, sizeof(cmp_arg->fi), seed);
+	switch (cmp_arg->proto) {
+	case MLXSW_SP_L3_PROTO_IPV4:
+		return jhash(&cmp_arg->fi, sizeof(cmp_arg->fi), seed);
+	case MLXSW_SP_L3_PROTO_IPV6:
+		return mlxsw_sp_nexthop6_group_hash(cmp_arg->fib6_entry, seed);
+	default:
+		WARN_ON(1);
+		return 0;
+	}
 }
 
 static const struct rhashtable_params mlxsw_sp_nexthop_group_ht_params = {
@@ -1581,6 +1681,10 @@ static const struct rhashtable_params mlxsw_sp_nexthop_group_ht_params = {
 static int mlxsw_sp_nexthop_group_insert(struct mlxsw_sp *mlxsw_sp,
 					 struct mlxsw_sp_nexthop_group *nh_grp)
 {
+	if (mlxsw_sp_nexthop_group_type(nh_grp) == AF_INET6 &&
+	    !nh_grp->gateway)
+		return 0;
+
 	return rhashtable_insert_fast(&mlxsw_sp->router->nexthop_group_ht,
 				      &nh_grp->ht_node,
 				      mlxsw_sp_nexthop_group_ht_params);
@@ -1589,6 +1693,10 @@ static int mlxsw_sp_nexthop_group_insert(struct mlxsw_sp *mlxsw_sp,
 static void mlxsw_sp_nexthop_group_remove(struct mlxsw_sp *mlxsw_sp,
 					  struct mlxsw_sp_nexthop_group *nh_grp)
 {
+	if (mlxsw_sp_nexthop_group_type(nh_grp) == AF_INET6 &&
+	    !nh_grp->gateway)
+		return;
+
 	rhashtable_remove_fast(&mlxsw_sp->router->nexthop_group_ht,
 			       &nh_grp->ht_node,
 			       mlxsw_sp_nexthop_group_ht_params);
@@ -1600,12 +1708,26 @@ mlxsw_sp_nexthop4_group_lookup(struct mlxsw_sp *mlxsw_sp,
 {
 	struct mlxsw_sp_nexthop_group_cmp_arg cmp_arg;
 
+	cmp_arg.proto = MLXSW_SP_L3_PROTO_IPV4;
 	cmp_arg.fi = fi;
 	return rhashtable_lookup_fast(&mlxsw_sp->router->nexthop_group_ht,
 				      &cmp_arg,
 				      mlxsw_sp_nexthop_group_ht_params);
 }
 
+static struct mlxsw_sp_nexthop_group *
+mlxsw_sp_nexthop6_group_lookup(struct mlxsw_sp *mlxsw_sp,
+			       struct mlxsw_sp_fib6_entry *fib6_entry)
+{
+	struct mlxsw_sp_nexthop_group_cmp_arg cmp_arg;
+
+	cmp_arg.proto = MLXSW_SP_L3_PROTO_IPV6;
+	cmp_arg.fib6_entry = fib6_entry;
+	return rhashtable_lookup_fast(&mlxsw_sp->router->nexthop_group_ht,
+				      &cmp_arg,
+				      mlxsw_sp_nexthop_group_ht_params);
+}
+
 static const struct rhashtable_params mlxsw_sp_nexthop_ht_params = {
 	.key_offset = offsetof(struct mlxsw_sp_nexthop, key),
 	.head_offset = offsetof(struct mlxsw_sp_nexthop, ht_node),
@@ -3197,6 +3319,7 @@ static int mlxsw_sp_nexthop6_init(struct mlxsw_sp *mlxsw_sp,
 
 	if (!dev)
 		return 0;
+	nh->ifindex = dev->ifindex;
 
 	rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
 	if (!rif)
@@ -3254,9 +3377,15 @@ mlxsw_sp_nexthop6_group_create(struct mlxsw_sp *mlxsw_sp,
 			goto err_nexthop6_init;
 		mlxsw_sp_rt6 = list_next_entry(mlxsw_sp_rt6, list);
 	}
+
+	err = mlxsw_sp_nexthop_group_insert(mlxsw_sp, nh_grp);
+	if (err)
+		goto err_nexthop_group_insert;
+
 	mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh_grp);
 	return nh_grp;
 
+err_nexthop_group_insert:
 err_nexthop6_init:
 	for (i--; i >= 0; i--) {
 		nh = &nh_grp->nexthops[i];
@@ -3273,6 +3402,7 @@ mlxsw_sp_nexthop6_group_destroy(struct mlxsw_sp *mlxsw_sp,
 	struct mlxsw_sp_nexthop *nh;
 	int i = nh_grp->count;
 
+	mlxsw_sp_nexthop_group_remove(mlxsw_sp, nh_grp);
 	for (i--; i >= 0; i--) {
 		nh = &nh_grp->nexthops[i];
 		mlxsw_sp_nexthop6_fini(mlxsw_sp, nh);
@@ -3287,10 +3417,12 @@ static int mlxsw_sp_nexthop6_group_get(struct mlxsw_sp *mlxsw_sp,
 {
 	struct mlxsw_sp_nexthop_group *nh_grp;
 
-	/* For now, don't consolidate nexthop groups */
-	nh_grp = mlxsw_sp_nexthop6_group_create(mlxsw_sp, fib6_entry);
-	if (IS_ERR(nh_grp))
-		return PTR_ERR(nh_grp);
+	nh_grp = mlxsw_sp_nexthop6_group_lookup(mlxsw_sp, fib6_entry);
+	if (!nh_grp) {
+		nh_grp = mlxsw_sp_nexthop6_group_create(mlxsw_sp, fib6_entry);
+		if (IS_ERR(nh_grp))
+			return PTR_ERR(nh_grp);
+	}
 
 	list_add_tail(&fib6_entry->common.nexthop_group_node,
 		      &nh_grp->fib_list);
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Thiago Macieira @ 2017-08-14 19:15 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: Matthew Dawson, Paolo Abeni, Network Development
In-Reply-To: <CAF=yD-K4HgOhMMT0tWycZgcqhU5OACQk_roPQEeRems1_Azhgw@mail.gmail.com>

On Monday, 14 August 2017 12:03:16 PDT Willem de Bruijn wrote:
> On Mon, Aug 14, 2017 at 2:58 PM, Thiago Macieira
> 
> <thiago.macieira@intel.com> wrote:
> > On Monday, 14 August 2017 11:46:42 PDT Willem de Bruijn wrote:
> >> > By the way, what were the usecases for the peek offset feature?
> >> 
> >> The idea was to be able to peek at application headers of upper
> >> layer protocols and multiplex messages among threads. It proved
> >> so complex even for UDP that we did not attempt the same feature
> >> for TCP. Also, KCM implements demultiplexing using eBPF today.
> > 
> > Interesting, but how would userspace coordinate like that? Suppose
> > multiple
> > threads are woken up by a datagram being received
> 
> This assumes a separate listener thread and worker threadpool.

The listener thread still needs to synchronise with the worker that got 
activated and wait for it to recv from the socket before the listener thread 
can go back to poll().

If we are really talking about threads in the same process, it might be easier 
for the listener to just read the datagram anyway and pass it on to the 
worker. That way, it can proceed immediately to the next datagram and not have 
to wait for the possibly slow worker.

If it is a separate process, then I don't see another way and this might be 
necessary.

By the way, what does recv with MSG_PEEK | MSG_TRUNC return? Is it the full 
datagram's size or is it the size minus the peek offset?

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

^ permalink raw reply

* [PATCH net-next] liquidio: fix issues with fw_type module parameter
From: Felix Manlunas @ 2017-08-14 19:17 UTC (permalink / raw)
  To: davem; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla

From: Derek Chickles <derek.chickles@cavium.com>

The fw_type module parameter isn't showing up in the
/sys/module/liquidio/parameters directory.  Fix it by setting the read
permission bits for user, group, other in module_param_string().  Revise
the description of fw_type.  Initialize the fw_type static char array with
the default value to conform to the module parameter description.

Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/lio_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index cbd6287..b8ba2c2 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -59,9 +59,9 @@ static int debug = -1;
 module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "NETIF_MSG debug bits");
 
-static char fw_type[LIO_MAX_FW_TYPE_LEN];
-module_param_string(fw_type, fw_type, sizeof(fw_type), 0000);
-MODULE_PARM_DESC(fw_type, "Type of firmware to be loaded. Default \"nic\"");
+static char fw_type[LIO_MAX_FW_TYPE_LEN] = LIO_FW_NAME_TYPE_NIC;
+module_param_string(fw_type, fw_type, sizeof(fw_type), 0444);
+MODULE_PARM_DESC(fw_type, "Type of firmware to be loaded. Default \"nic\".  Use \"none\" to load firmware from flash.");
 
 static u32 console_bitmask;
 module_param(console_bitmask, int, 0644);

^ permalink raw reply related

* Re: [Intel-wired-lan] [PATCH 6/6] [net-next]net: i40e: Enable cloud filters in i40e via tc/flower classifier
From: Nambiar, Amritha @ 2017-08-14 19:21 UTC (permalink / raw)
  To: Shannon Nelson, intel-wired-lan, jeffrey.t.kirsher
  Cc: netdev, mitch.a.williams
In-Reply-To: <84377bab-472f-7981-48df-cdf5b0528978@oracle.com>

On 8/1/2017 12:16 PM, Shannon Nelson wrote:
> On 7/31/2017 5:38 PM, Amritha Nambiar wrote:
>> This patch enables tc-flower based hardware offloads. tc/flower
>> filter provided by the kernel is configured as driver specific
>> cloud filter. The patch implements functions and admin queue
>> commands needed to support cloud filters in the driver and
>> adds cloud filters to configure these tc-flower filters.
>>
>> The only action supported is to redirect packets to a traffic class
>> on the same device.
>>
>> # tc qdisc add dev eth0 ingress
>> # ethtool -K eth0 hw-tc-offload on
>>
>> # tc filter add dev eth0 protocol ip parent ffff:\
>>    prio 1 flower dst_mac 3c:fd:fe:a0:d6:70 skip_sw indev eth0\
>>    action mirred ingress redirect dev eth0 tc 0
>>
>> # tc filter add dev eth0 protocol ip parent ffff:\
>>    prio 2 flower dst_ip 192.168.3.5/32\
>>    ip_proto udp dst_port 25 skip_sw indev eth0\
>>    action mirred ingress redirect dev eth0 tc 1
>>
>> # tc filter add dev eth0 protocol ipv6 parent ffff:\
>>    prio 3 flower dst_ip fe8::200:1\
>>    ip_proto udp dst_port 66 skip_sw indev eth0\
>>    action mirred ingress redirect dev eth0 tc 2
>>
>> Delete tc flower filter:
>> Example:
>>
>> # tc filter del dev eth0 parent ffff: prio 3 handle 0x1 flower
>> # tc filter del dev eth0 parent ffff:
>>
>> Flow Director Sideband is disabled while configuring cloud filters
>> via tc-flower.
> 
> Only while configuring, or the whole time there is a cloud filter?  This 
> is unclear here.

The entire time cloud filters exists. Will make the comment clearer in v2.

> 
>>
>> Unsupported matches when cloud filters are added using enhanced
>> big buffer cloud filter mode of underlying switch include:
>> 1. source port and source IP
>> 2. Combined MAC address and IP fields.
>> 3. Not specfying L4 port
> 
> s/specfying/specifying/

Will fix in v2.

> 
>>
>> These filter matches can however be used to redirect traffic to
>> the main VSI (tc 0) which does not require the enhanced big buffer
>> cloud filter support.
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
>> ---
>>   drivers/net/ethernet/intel/i40e/i40e.h           |   46 +
>>   drivers/net/ethernet/intel/i40e/i40e_common.c    |  180 ++++
>>   drivers/net/ethernet/intel/i40e/i40e_main.c      |  952 ++++++++++++++++++++++
>>   drivers/net/ethernet/intel/i40e/i40e_prototype.h |   17
>>   4 files changed, 1193 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>> index 5c0cad5..7288265 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -55,6 +55,8 @@
>>   #include <linux/net_tstamp.h>
>>   #include <linux/ptp_clock_kernel.h>
>>   #include <net/pkt_cls.h>
>> +#include <net/tc_act/tc_gact.h>
>> +#include <net/tc_act/tc_mirred.h>
>>   #include "i40e_type.h"
>>   #include "i40e_prototype.h"
>>   #include "i40e_client.h"
>> @@ -252,10 +254,51 @@ struct i40e_fdir_filter {
>>   	u32 fd_id;
>>   };
>>   
>> +#define I40E_CLOUD_FIELD_OMAC	0x01
>> +#define I40E_CLOUD_FIELD_IMAC	0x02
>> +#define I40E_CLOUD_FIELD_IVLAN	0x04
>> +#define I40E_CLOUD_FIELD_TEN_ID	0x08
>> +#define I40E_CLOUD_FIELD_IIP	0x10
>> +
>> +#define I40E_CLOUD_FILTER_FLAGS_OMAC	I40E_CLOUD_FIELD_OMAC
>> +#define I40E_CLOUD_FILTER_FLAGS_IMAC	I40E_CLOUD_FIELD_IMAC
>> +#define I40E_CLOUD_FILTER_FLAGS_IMAC_IVLAN	(I40E_CLOUD_FIELD_IMAC | \
>> +						 I40E_CLOUD_FIELD_IVLAN)
>> +#define I40E_CLOUD_FILTER_FLAGS_IMAC_TEN_ID	(I40E_CLOUD_FIELD_IMAC | \
>> +						 I40E_CLOUD_FIELD_TEN_ID)
>> +#define I40E_CLOUD_FILTER_FLAGS_OMAC_TEN_ID_IMAC (I40E_CLOUD_FIELD_OMAC | \
>> +						  I40E_CLOUD_FIELD_IMAC | \
>> +						  I40E_CLOUD_FIELD_TEN_ID)
>> +#define I40E_CLOUD_FILTER_FLAGS_IMAC_IVLAN_TEN_ID (I40E_CLOUD_FIELD_IMAC | \
>> +						   I40E_CLOUD_FIELD_IVLAN | \
>> +						   I40E_CLOUD_FIELD_TEN_ID)
>> +#define I40E_CLOUD_FILTER_FLAGS_IIP	I40E_CLOUD_FIELD_IIP
>> +
>>   struct i40e_cloud_filter {
>>   	struct hlist_node cloud_node;
>>   	/* cloud filter input set follows */
>>   	unsigned long cookie;
>> +	u8 dst_mac[ETH_ALEN];
>> +	u8 src_mac[ETH_ALEN];
>> +	__be16 vlan_id;
>> +	__be32 dst_ip[4];
>> +	__be32 src_ip[4];
>> +	u8 dst_ipv6[16];
>> +	u8 src_ipv6[16];
>> +	__be16 dst_port;
>> +	__be16 src_port;
>> +	/* matter only when IP based filtering is set */
>> +	bool is_ipv6;
>> +	/* IPPROTO value */
>> +	u8 ip_proto;
>> +	/* L4 port type: src or destination port */
>> +#define I40E_CLOUD_FILTER_PORT_SRC	0x01
>> +#define I40E_CLOUD_FILTER_PORT_DEST	0x02
>> +	u8 port_type;
>> +	u32 tenant_id;
>> +	u8 flags;
>> +#define I40E_CLOUD_TNL_TYPE_NONE	0xff
>> +	u8 tunnel_type;
>>   	/* filter control */
>>   	u16 seid;
>>   };
>> @@ -574,6 +617,9 @@ struct i40e_pf {
>>   	u16 phy_led_val;
>>   
>>   	u16 override_q_count;
>> +	u16 last_sw_conf_flags;
>> +	u16 last_sw_conf_valid_flags;
>> +
> 
> Unnecessary blank line
> 
>>   };
>>   
>>   /**
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
>> index d0e8138..bfbe304 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_common.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
>> @@ -5269,5 +5269,185 @@ i40e_add_pinfo_to_list(struct i40e_hw *hw,
>>   
>>   	status = i40e_aq_write_ppp(hw, (void *)sec, sec->data_end,
>>   				   track_id, &offset, &info, NULL);
>> +
>> +	return status;
>> +}
>> +
>> +/**
>> + * i40e_aq_add_cloud_filters
>> + * @hw: pointer to the hardware structure
>> + * @seid: VSI seid to add cloud filters from
>> + * @filters: Buffer which contains the filters to be added
>> + * @filter_count: number of filters contained in the buffer
>> + *
>> + * Set the cloud filters for a given VSI.  The contents of the
>> + * i40e_aqc_add_remove_cloud_filters_element_data are filled
>> + * in by the caller of the function.
>> + *
>> + **/
>> +enum i40e_status_code i40e_aq_add_cloud_filters(struct i40e_hw *hw,
>> +		u16 seid,
>> +		struct i40e_aqc_add_remove_cloud_filters_element_data *filters,
>> +		u8 filter_count)
>> +{
>> +	struct i40e_aq_desc desc;
>> +	struct i40e_aqc_add_remove_cloud_filters *cmd =
>> +	(struct i40e_aqc_add_remove_cloud_filters *)&desc.params.raw;
>> +	enum i40e_status_code status;
>> +	u16 buff_len;
>> +
>> +	i40e_fill_default_direct_cmd_desc(&desc,
>> +					  i40e_aqc_opc_add_cloud_filters);
>> +
>> +	buff_len = filter_count * sizeof(*filters);
>> +	desc.datalen = cpu_to_le16(buff_len);
>> +	desc.flags |= cpu_to_le16((u16)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
>> +	cmd->num_filters = filter_count;
>> +	cmd->seid = cpu_to_le16(seid);
>> +
>> +	status = i40e_asq_send_command(hw, &desc, filters, buff_len, NULL);
>> +
>> +	return status;
>> +}
>> +
>> +/**
>> + * i40e_aq_add_cloud_filters_big_buffer
>> + * @hw: pointer to the hardware structure
>> + * @seid: VSI seid to add cloud filters from
>> + * @filters: Buffer which contains the filters in big buffer to be added
>> + * @filter_count: number of filters contained in the buffer
>> + *
>> + * Set the cloud filters for a given VSI.  The contents of the
>> + * i40e_aqc_add_remove_cloud_filters_element_big_data are filled
>> + * in by the caller of the function.
>> + *
>> + **/
>> +i40e_status i40e_aq_add_cloud_filters_big_buffer(struct i40e_hw *hw,
>> +	u16 seid,
>> +	struct i40e_aqc_add_remove_cloud_filters_element_big_data *filters,
>> +	u8 filter_count)
>> +{
>> +	struct i40e_aq_desc desc;
>> +	struct i40e_aqc_add_remove_cloud_filters *cmd =
>> +	(struct i40e_aqc_add_remove_cloud_filters *)&desc.params.raw;
>> +	i40e_status status;
>> +	u16 buff_len;
>> +	int i;
>> +
>> +	i40e_fill_default_direct_cmd_desc(&desc,
>> +					  i40e_aqc_opc_add_cloud_filters);
>> +
>> +	buff_len = filter_count * sizeof(*filters);
>> +	desc.datalen = cpu_to_le16(buff_len);
>> +	desc.flags |= cpu_to_le16((u16)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
>> +	cmd->num_filters = filter_count;
>> +	cmd->seid = cpu_to_le16(seid);
>> +	cmd->big_buffer_flag = I40E_AQC_ADD_REM_CLOUD_CMD_BIG_BUFFER;
>> +
>> +	for (i = 0; i < filter_count; i++) {
>> +		u16 tnl_type;
>> +		u32 ti;
>> +
>> +		tnl_type = (le16_to_cpu(filters[i].element.flags) &
>> +			   I40E_AQC_ADD_CLOUD_TNL_TYPE_MASK) >>
>> +			   I40E_AQC_ADD_CLOUD_TNL_TYPE_SHIFT;
>> +		if (tnl_type == I40E_AQC_ADD_CLOUD_TNL_TYPE_GENEVE) {
>> +			ti = le32_to_cpu(filters[i].element.tenant_id);
>> +			filters[i].element.tenant_id = cpu_to_le32(ti << 8);
>> +		}
> 
> You might want to add a comment to explain this little hack.  I believe 
> this is for a bit of firmware weirdness?

Yes, this is to handle a firmware behavior specific to Geneve filters.
Will add the comment in v2.

> 
>> +	}
>> +
>> +	status = i40e_asq_send_command(hw, &desc, filters, buff_len, NULL);
>> +
>> +	return status;
>> +}
>> +
>> +/**
>> + * i40e_aq_remove_cloud_filters
>> + * @hw: pointer to the hardware structure
>> + * @seid: VSI seid to remove cloud filters from
>> + * @filters: Buffer which contains the filters to be removed
>> + * @filter_count: number of filters contained in the buffer
>> + *
>> + * Remove the cloud filters for a given VSI.  The contents of the
>> + * i40e_aqc_add_remove_cloud_filters_element_data are filled
>> + * in by the caller of the function.
>> + *
>> + **/
>> +enum i40e_status_code i40e_aq_remove_cloud_filters(struct i40e_hw *hw,
>> +		u16 seid,
>> +		struct i40e_aqc_add_remove_cloud_filters_element_data *filters,
>> +		u8 filter_count)
>> +{
>> +	struct i40e_aq_desc desc;
>> +	struct i40e_aqc_add_remove_cloud_filters *cmd =
>> +	(struct i40e_aqc_add_remove_cloud_filters *)&desc.params.raw;
>> +	enum i40e_status_code status;
>> +	u16 buff_len;
>> +
>> +	i40e_fill_default_direct_cmd_desc(&desc,
>> +					  i40e_aqc_opc_remove_cloud_filters);
>> +
>> +	buff_len = filter_count * sizeof(*filters);
>> +	desc.datalen = cpu_to_le16(buff_len);
>> +	desc.flags |= cpu_to_le16((u16)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
>> +	cmd->num_filters = filter_count;
>> +	cmd->seid = cpu_to_le16(seid);
>> +
>> +	status = i40e_asq_send_command(hw, &desc, filters, buff_len, NULL);
>> +
>> +	return status;
>> +}
>> +
>> +/**
>> + * i40e_aq_remove_cloud_filters_big_buffer
>> + * @hw: pointer to the hardware structure
>> + * @seid: VSI seid to remove cloud filters from
>> + * @filters: Buffer which contains the filters in big buffer to be removed
>> + * @filter_count: number of filters contained in the buffer
>> + *
>> + * Remove the cloud filters for a given VSI.  The contents of the
>> + * i40e_aqc_add_remove_cloud_filters_element_big_data are filled
>> + * in by the caller of the function.
>> + *
>> + **/
>> +i40e_status i40e_aq_remove_cloud_filters_big_buffer(
>> +	struct i40e_hw *hw,
>> +	u16 seid,
>> +	struct i40e_aqc_add_remove_cloud_filters_element_big_data *filters,
>> +	u8 filter_count)
>> +{
>> +	struct i40e_aq_desc desc;
>> +	struct i40e_aqc_add_remove_cloud_filters *cmd =
>> +	(struct i40e_aqc_add_remove_cloud_filters *)&desc.params.raw;
>> +	i40e_status status;
>> +	u16 buff_len;
>> +	int i;
>> +
>> +	i40e_fill_default_direct_cmd_desc(&desc,
>> +					  i40e_aqc_opc_remove_cloud_filters);
>> +
>> +	buff_len = filter_count * sizeof(*filters);
>> +	desc.datalen = cpu_to_le16(buff_len);
>> +	desc.flags |= cpu_to_le16((u16)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
>> +	cmd->num_filters = filter_count;
>> +	cmd->seid = cpu_to_le16(seid);
>> +	cmd->big_buffer_flag = I40E_AQC_ADD_REM_CLOUD_CMD_BIG_BUFFER;
>> +
>> +	for (i = 0; i < filter_count; i++) {
>> +		u16 tnl_type;
>> +		u32 ti;
>> +
>> +		tnl_type = (le16_to_cpu(filters[i].element.flags) &
>> +			   I40E_AQC_ADD_CLOUD_TNL_TYPE_MASK) >>
>> +			   I40E_AQC_ADD_CLOUD_TNL_TYPE_SHIFT;
>> +		if (tnl_type == I40E_AQC_ADD_CLOUD_TNL_TYPE_GENEVE) {
>> +			ti = le32_to_cpu(filters[i].element.tenant_id);
>> +			filters[i].element.tenant_id = cpu_to_le32(ti << 8);
>> +		}
> 
> Same comment
> 
>> +	}
>> +
>> +	status = i40e_asq_send_command(hw, &desc, filters, buff_len, NULL);
>> +
>>   	return status;
>>   }
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index 93f6fe2..65b284e 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -69,6 +69,12 @@ static int i40e_reset(struct i40e_pf *pf);
>>   static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired);
>>   static void i40e_fdir_sb_setup(struct i40e_pf *pf);
>>   static int i40e_veb_get_bw_info(struct i40e_veb *veb);
>> +static int i40e_add_del_cloud_filter(struct i40e_vsi *vsi,
>> +				     struct i40e_cloud_filter *filter,
>> +				     bool add);
>> +static int i40e_add_del_cloud_filter_big_buf(struct i40e_vsi *vsi,
>> +					     struct i40e_cloud_filter *filter,
>> +					     bool add);
>>   
>>   /* i40e_pci_tbl - PCI Device ID Table
>>    *
>> @@ -5482,7 +5488,11 @@ int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
>>    **/
>>   static void i40e_remove_queue_channels(struct i40e_vsi *vsi)
> 
> Hmmm... I'm looking at net-next code and I don't see this function, or 
> anything else with "i40e_channel" or "queue_channel".  What are you 
> applying this patch to?  Am I missing something?
> 
>>   {
>> +	enum i40e_admin_queue_err last_aq_status;
>> +	struct i40e_cloud_filter *cfilter;
>>   	struct i40e_channel *ch, *ch_tmp;
>> +	struct i40e_pf *pf = vsi->back;
>> +	struct hlist_node *node;
>>   	int ret, i;
>>   
>>   	/* Reset rss size that was stored when reconfiguring rss for
>> @@ -5523,6 +5533,29 @@ static void i40e_remove_queue_channels(struct i40e_vsi *vsi)
>>   				 "Failed to reset tx rate for ch->seid %u\n",
>>   				 ch->seid);
>>   
>> +		/* delete cloud filters associated with this channel */
>> +		hlist_for_each_entry_safe(cfilter, node,
>> +					  &pf->cloud_filter_list, cloud_node) {
>> +			if (cfilter->seid != ch->seid)
>> +				continue;
>> +
>> +			hash_del(&cfilter->cloud_node);
>> +			if (cfilter->dst_port)
>> +				ret = i40e_add_del_cloud_filter_big_buf(vsi,
>> +									cfilter,
>> +									false);
>> +			else
>> +				ret = i40e_add_del_cloud_filter(vsi, cfilter,
>> +								false);
>> +			last_aq_status = pf->hw.aq.asq_last_status;
>> +			if (ret)
>> +				dev_info(&pf->pdev->dev,
>> +					 "fail to delete cloud filter, err %s aq_err %s\n",
>> +					 i40e_stat_str(&pf->hw, ret),
>> +					 i40e_aq_str(&pf->hw, last_aq_status));
>> +			kfree(cfilter);
>> +		}
>> +
>>   		/* delete VSI from FW */
>>   		ret = i40e_aq_delete_element(&vsi->back->hw, ch->seid,
>>   					     NULL);
>> @@ -6004,6 +6037,131 @@ static bool i40e_setup_channel(struct i40e_pf *pf, struct i40e_vsi *vsi,
>>   }
>>   
>>   /**
>> + * i40e_get_device_capabilities - get device level info about the HW
>> + * @pf: the PF struct
>> + **/
>> +static int i40e_get_device_capabilities(struct i40e_pf *pf)
> 
> Since this is almost exactly the same as the original 
> i40e_get_capabilities(), it would be better to simply add list_type_opc 
> to i40e_get_capabilities() parameter list

Will fix this in v2.

> 
>> +{
>> +	struct i40e_aqc_list_capabilities_element_resp *cap_buf;
>> +	u16 data_size;
>> +	int buf_len;
>> +	int err;
>> +
>> +	buf_len = 40 * sizeof(struct i40e_aqc_list_capabilities_element_resp);
>> +
>> +	/* proceed with query device level capabilities */
>> +	do {
>> +		cap_buf = kzalloc(buf_len, GFP_KERNEL);
>> +		if (!cap_buf)
>> +			return -ENOMEM;
>> +
>> +		/* this loads the data into the hw struct for us */
>> +		err = i40e_aq_discover_capabilities(&pf->hw, cap_buf, buf_len,
>> +					     &data_size,
>> +					     i40e_aqc_opc_list_dev_capabilities,
>> +					     NULL);
>> +		/* data loaded, buffer no longer needed */
>> +		kfree(cap_buf);
>> +
>> +		if (pf->hw.aq.asq_last_status == I40E_AQ_RC_ENOMEM) {
>> +			/* retry with a larger buffer */
>> +			buf_len = data_size;
>> +		} else if (pf->hw.aq.asq_last_status != I40E_AQ_RC_OK) {
>> +			dev_dbg(&pf->pdev->dev,
>> +				"device capability discovery failed, err %s aq_err %s\n",
>> +				i40e_stat_str(&pf->hw, err),
>> +				i40e_aq_str(&pf->hw,
>> +					    pf->hw.aq.asq_last_status));
>> +			return -ENODEV;
>> +		}
>> +	} while (err);
>> +
>> +	if (pf->hw.debug_mask & I40E_DEBUG_USER) {
>> +		dev_info(&pf->pdev->dev,
>> +			 "switch_mode=0x%04x, function_valid=0x%08x\n",
>> +			 pf->hw.dev_caps.switch_mode,
>> +			 pf->hw.dev_caps.valid_functions);
>> +		dev_info(&pf->pdev->dev,
>> +			 "SR-IOV=%d, num_vfs for all function=%u\n",
>> +			 pf->hw.dev_caps.sr_iov_1_1, pf->hw.dev_caps.num_vfs);
>> +		dev_info(&pf->pdev->dev,
>> +			 "num_vsis=%u, num_rx:%u, num_tx=%u\n",
>> +			 pf->hw.dev_caps.num_vsis, pf->hw.dev_caps.num_rx_qp,
>> +			 pf->hw.dev_caps.num_tx_qp);
>> +	}
>> +	return 0;
>> +}
>> +
>> +/**
>> + * i40e_validate_and_set_switch_mode - sets up switch mode correctly
>> + * @vsi: ptr to VSI which has PF backing
>> + * @l4type: true for TCP ond false for UDP
>> + * @port_type: true if port is destination and false if port is source
>> + *
>> + * Sets up switch mode correctly if it needs to be changed and perform
>> + * what are allowed modes.
>> + **/
>> +static int i40e_validate_and_set_switch_mode(struct i40e_vsi *vsi, bool l4type,
>> +					     bool port_type)
>> +{
>> +	u8 mode;
>> +	struct i40e_pf *pf = vsi->back;
>> +	struct i40e_hw *hw = &pf->hw;
>> +	int ret;
>> +
>> +	ret = i40e_get_device_capabilities(pf);
>> +	if (ret)
>> +		return -EINVAL;
>> +
>> +	if (hw->dev_caps.switch_mode) {
>> +		/* if switch mode is set, support mode2 (non-tunneled for
>> +		 * cloud filter) for now
>> +		 */
>> +#define I40E_SWITCH_MODE_MASK	0xF /* largest cloud filter mode is 0x8 */
> 
> Why isn't this defined in the appropriate .h file?

Will move this in v2.

> 
>> +		u32 switch_mode = hw->dev_caps.switch_mode &
>> +							I40E_SWITCH_MODE_MASK;
>> +		if (switch_mode >= I40E_NVM_IMAGE_TYPE_MODE1) {
>> +			if (switch_mode == I40E_NVM_IMAGE_TYPE_MODE2)
>> +				return 0;
>> +			dev_err(&pf->pdev->dev,
>> +				"Invalid switch_mode (%d), only non-tunneled mode for cloud filter is supported\n",
>> +				hw->dev_caps.switch_mode);
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	/* port_type: true for destination port and false for source port
>> +	 * For now, supports only destination port type
>> +	 */
>> +	if (!port_type) {
>> +		dev_err(&pf->pdev->dev, "src port type not supported\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Set Bit 7 to be valid */
>> +	mode = I40E_AQ_SET_SWITCH_BIT7_VALID;
>> +
>> +	/* Set L4type to both TCP and UDP support */
>> +	mode |= I40E_AQ_SET_SWITCH_L4_TYPE_BOTH;
>> +
>> +	/* Set cloud filter mode */
>> +	mode |= I40E_AQ_SET_SWITCH_MODE_NON_TUNNEL;
>> +
>> +	/* Prep mode field for set_switch_config */
>> +	ret = i40e_aq_set_switch_config(hw, pf->last_sw_conf_flags,
>> +					pf->last_sw_conf_valid_flags,
>> +					mode, NULL);
>> +	if (ret && hw->aq.asq_last_status != I40E_AQ_RC_ESRCH)
>> +		dev_err(&pf->pdev->dev,
>> +			"couldn't set switch config bits, err %s aq_err %s\n",
>> +			i40e_stat_str(hw, ret),
>> +			i40e_aq_str(hw,
>> +				    hw->aq.asq_last_status));
>> +
>> +	return ret;
>> +}
>> +
>> +/**
>>    * i40e_create_queue_channel - function to create channel
>>    * @vsi: VSI to be configured
>>    * @ch: ptr to channel (it contains channel specific params)
>> @@ -6632,6 +6790,10 @@ static int i40e_setup_tc(struct net_device *netdev, struct tc_to_netdev *tc)
>>   	}
>>   	if (!hw) {
>>   		pf->flags &= ~I40E_FLAG_TC_MQPRIO;
>> +		if ((pf->hw.func_caps.fd_filters_guaranteed > 0) ||
>> +		    (pf->hw.func_caps.fd_filters_best_effort > 0))
>> +			pf->flags |= I40E_FLAG_FD_ATR_ENABLED;
>> +
>>   		if (tc->type == TC_SETUP_MQPRIO_EXT)
>>   			memcpy(&vsi->mqprio_qopt, tc->mqprio_qopt,
>>   			       sizeof(*tc->mqprio_qopt));
>> @@ -6682,6 +6844,11 @@ static int i40e_setup_tc(struct net_device *netdev, struct tc_to_netdev *tc)
>>   		       sizeof(*tc->mqprio_qopt));
>>   		pf->flags |= I40E_FLAG_TC_MQPRIO;
>>   		pf->flags &= ~I40E_FLAG_DCB_ENABLED;
>> +		if (pf->flags & I40E_FLAG_FD_ATR_ENABLED) {
>> +			pf->flags &= ~I40E_FLAG_FD_ATR_ENABLED;
>> +			dev_info(&pf->pdev->dev,
>> +				 "Disabling ATR in MQPRIO mode\n");
>> +		}
>>   		break;
>>   	default:
>>   		return -EINVAL;
>> @@ -6743,10 +6910,724 @@ static int i40e_setup_tc(struct net_device *netdev, struct tc_to_netdev *tc)
>>   	return ret;
>>   }
>>   
>> +/**
>> + * i40e_set_cld_element - sets cloud filter element data
>> + * @filter: cloud filter rule
>> + * @cld: ptr to cloud filter element data
>> + *
>> + * This is helper function to copy data into cloud filter element
>> + **/
>> +static inline void
>> +i40e_set_cld_element(struct i40e_cloud_filter *filter,
>> +		     struct i40e_aqc_add_remove_cloud_filters_element_data *cld)
>> +{
>> +	u8 *dest_ipaddr;
>> +	u32 ipaddr;
>> +	int i;
>> +
>> +	memset(cld, 0, sizeof(*cld));
>> +
>> +	ether_addr_copy(cld->outer_mac, filter->dst_mac);
>> +	ether_addr_copy(cld->inner_mac, filter->src_mac);
>> +
>> +	if (filter->is_ipv6) {
>> +		dest_ipaddr = (u8 *)&cld->ipaddr.v6.data;
>> +		for (i = ARRAY_SIZE(filter->dst_ipv6) - 1; i >= 0; i--) {
>> +			memcpy(dest_ipaddr, &filter->dst_ipv6[i], 1);
>> +			dest_ipaddr++;
>> +		}
>> +	} else {
>> +		ipaddr = be32_to_cpu(filter->dst_ip[0]);
>> +		memcpy(&cld->ipaddr.v4.data, &ipaddr, 4);
>> +	}
>> +
>> +	cld->inner_vlan = cpu_to_le16(ntohs(filter->vlan_id));
>> +	cld->tenant_id = cpu_to_le32(filter->tenant_id);
>> +}
>> +
>> +/**
>> + * i40e_add_del_cloud_filter - Add/del cloud filter
>> + * @vsi: pointer to VSI
>> + * @filter: cloud filter rule
>> + * @add: if true, add, if false, delete
>> + *
>> + * Add or delete a cloud filter for a specific flow spec.
>> + * Returns 0 if the filter were successfully added.
>> + **/
>> +static int i40e_add_del_cloud_filter(struct i40e_vsi *vsi,
>> +				     struct i40e_cloud_filter *filter, bool add)
>> +{
>> +	struct i40e_aqc_add_remove_cloud_filters_element_data cld_filter;
>> +	struct i40e_pf *pf = vsi->back;
>> +	int ret;
>> +	static const u16 flag_table[128] = {
>> +		[I40E_CLOUD_FILTER_FLAGS_OMAC]  =
>> +			I40E_AQC_ADD_CLOUD_FILTER_OMAC,
>> +		[I40E_CLOUD_FILTER_FLAGS_IMAC]  =
>> +			I40E_AQC_ADD_CLOUD_FILTER_IMAC,
>> +		[I40E_CLOUD_FILTER_FLAGS_IMAC_IVLAN]  =
>> +			I40E_AQC_ADD_CLOUD_FILTER_IMAC_IVLAN,
>> +		[I40E_CLOUD_FILTER_FLAGS_IMAC_TEN_ID] =
>> +			I40E_AQC_ADD_CLOUD_FILTER_IMAC_TEN_ID,
>> +		[I40E_CLOUD_FILTER_FLAGS_OMAC_TEN_ID_IMAC] =
>> +			I40E_AQC_ADD_CLOUD_FILTER_OMAC_TEN_ID_IMAC,
>> +		[I40E_CLOUD_FILTER_FLAGS_IMAC_IVLAN_TEN_ID] =
>> +			I40E_AQC_ADD_CLOUD_FILTER_IMAC_IVLAN_TEN_ID,
>> +		[I40E_CLOUD_FILTER_FLAGS_IIP] =
>> +			I40E_AQC_ADD_CLOUD_FILTER_IIP,
>> +	};
>> +
>> +	if (filter->flags >= ARRAY_SIZE(flag_table))
>> +		return I40E_ERR_CONFIG;
>> +
>> +	/* copy element needed to add cloud filter from filter */
>> +	i40e_set_cld_element(filter, &cld_filter);
>> +
>> +	if (filter->tunnel_type != I40E_CLOUD_TNL_TYPE_NONE)
>> +		cld_filter.flags = cpu_to_le16(filter->tunnel_type <<
>> +					     I40E_AQC_ADD_CLOUD_TNL_TYPE_SHIFT);
>> +
>> +	if (filter->is_ipv6)
>> +		cld_filter.flags |= cpu_to_le16(flag_table[filter->flags] |
>> +						I40E_AQC_ADD_CLOUD_FLAGS_IPV6);
>> +	else
>> +		cld_filter.flags |= cpu_to_le16(flag_table[filter->flags] |
>> +						I40E_AQC_ADD_CLOUD_FLAGS_IPV4);
>> +
>> +	if (add)
>> +		ret = i40e_aq_add_cloud_filters(&pf->hw, filter->seid,
>> +						&cld_filter, 1);
>> +	else
>> +		ret = i40e_aq_remove_cloud_filters(&pf->hw, filter->seid,
>> +						   &cld_filter, 1);
>> +	if (ret)
>> +		dev_dbg(&pf->pdev->dev,
>> +			"Failed to %s cloud filter using l4 port %u, err %d aq_err %d\n",
>> +			add ? "add" : "delete", filter->dst_port, ret,
>> +			pf->hw.aq.asq_last_status);
>> +
>> +	dev_info(&pf->pdev->dev,
>> +		 "%s cloud filter for VSI: %d\n", add ? "Added" : "Deleted",
>> +		 filter->seid);
> 
> If there was an error, this dev_info() is lying to the user.

Will fix this in v2.

> 
>> +	return ret;
>> +}
>> +
>> +/**
>> + * i40e_add_del_cloud_filter_big_buf - Add/del cloud filter using big_buf
>> + * @vsi: pointer to VSI
>> + * @filter: cloud filter rule
>> + * @add: if true, add, if false, delete
>> + *
>> + * Add or delete a cloud filter for a specific flow spec using big buffer.
>> + * Returns 0 if the filter were successfully added.
>> + **/
>> +static int i40e_add_del_cloud_filter_big_buf(struct i40e_vsi *vsi,
>> +					     struct i40e_cloud_filter *filter,
>> +					     bool add)
>> +{
>> +	struct i40e_aqc_add_remove_cloud_filters_element_big_data cld_filter;
>> +	struct i40e_pf *pf = vsi->back;
>> +	int ret;
>> +
>> +	/* Both (Outer/Inner) valid mac_addr are not supported */
>> +	if (is_valid_ether_addr(filter->dst_mac) &&
>> +	    is_valid_ether_addr(filter->src_mac))
>> +		return -EINVAL;
>> +
>> +	/* Make sure port is specified, otherwise bail out, for channel
>> +	 * specific cloud filter needs 'L4 port' to be non-zero
>> +	 */
>> +	if (!filter->dst_port)
>> +		return -EINVAL;
>> +
>> +	/* adding filter using src_port/src_ip is not supported at this stage */
>> +	if (filter->src_port || filter->src_ip[0])
>> +		return -EINVAL;
>> +
>> +	/* copy element needed to add cloud filter from filter */
>> +	i40e_set_cld_element(filter, &cld_filter.element);
>> +
>> +	if (is_valid_ether_addr(filter->dst_mac) ||
>> +	    is_valid_ether_addr(filter->src_mac) ||
>> +	    is_multicast_ether_addr(filter->dst_mac) ||
>> +	    is_multicast_ether_addr(filter->src_mac)) {
>> +		/* MAC + IP : unsupported mode */
>> +		if (filter->dst_ip[0])
>> +			return -EINVAL;
>> +
>> +		/* since we validated that L4 port must be valid before
>> +		 * we get here, start with respective "flags" value
>> +		 * and update if vlan is present or not
>> +		 */
>> +		cld_filter.element.flags =
>> +			cpu_to_le16(I40E_AQC_ADD_CLOUD_FILTER_MAC_PORT);
>> +
>> +		if (filter->vlan_id) {
>> +			cld_filter.element.flags =
>> +			cpu_to_le16(I40E_AQC_ADD_CLOUD_FILTER_MAC_VLAN_PORT);
>> +		}
> 
> 		cld_filter.element.flags = cpu_to_le16(filter->vlan_id
> 				? I40E_AQC_ADD_CLOUD_FILTER_MAC_VLAN_PORT;
> 				: I40E_AQC_ADD_CLOUD_FILTER_MAC_PORT);
> 
> 
>> +
>> +	} else if (filter->dst_ip[0] || filter->is_ipv6) {
>> +		cld_filter.element.flags =
>> +				cpu_to_le16(I40E_AQC_ADD_CLOUD_FILTER_IP_PORT);
>> +		if (filter->is_ipv6)
>> +			cld_filter.element.flags |=
>> +				cpu_to_le16(I40E_AQC_ADD_CLOUD_FLAGS_IPV6);
>> +		else
>> +			cld_filter.element.flags |=
>> +				cpu_to_le16(I40E_AQC_ADD_CLOUD_FLAGS_IPV4);
> 
> 		cld_filter.element.flags |= cpu_to_le16(filter->is_ipv6
> 					? I40E_AQC_ADD_CLOUD_FLAGS_IPV6
> 					: I40E_AQC_ADD_CLOUD_FLAGS_IPV4);
> 
> 
>> +	} else {
>> +		dev_err(&pf->pdev->dev,
>> +			"either mac or ip has to be valid for cloud filter\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Now copy L4 port in Byte 6..7 in general fields */
>> +	cld_filter.general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X16_WORD0] =
>> +						be16_to_cpu(filter->dst_port);
>> +
>> +	if (add) {
>> +		bool proto_type, port_type;
>> +
>> +		proto_type = (filter->ip_proto == IPPROTO_TCP) ? true : false;
>> +		port_type = (filter->port_type & I40E_CLOUD_FILTER_PORT_DEST) ?
>> +			     true : false;
>> +
>> +		/* For now, src port based cloud filter for channel is not
>> +		 * supported
>> +		 */
>> +		if (!port_type) {
>> +			dev_err(&pf->pdev->dev,
>> +				"unsupported port type (src port)\n");
>> +			return -EOPNOTSUPP;
>> +		}
>> +
>> +		/* Validate current device switch mode, change if necessary */
>> +		ret = i40e_validate_and_set_switch_mode(vsi, proto_type,
>> +							port_type);
>> +		if (ret) {
>> +			dev_err(&pf->pdev->dev,
>> +				"fail to and set switch mode, ret %d\n",
>> +				ret);
>> +			return ret;
>> +		}
>> +
>> +		ret = i40e_aq_add_cloud_filters_big_buffer(&pf->hw,
>> +							   filter->seid,
>> +							   &cld_filter, 1);
>> +	} else {
>> +		ret = i40e_aq_remove_cloud_filters_big_buffer(&pf->hw,
>> +							      filter->seid,
>> +							      &cld_filter, 1);
>> +	}
>> +
>> +	if (ret)
>> +		dev_dbg(&pf->pdev->dev,
>> +			"Failed to %s cloud filter(big buffer) err %d aq_err %d\n",
>> +			add ? "add" : "delete", ret, pf->hw.aq.asq_last_status);
>> +
>> +	dev_info(&pf->pdev->dev,
>> +		 "%s cloud filter for VSI: %d, L4 port: %d\n",
>> +		 add ? "add" : "delete", filter->seid, ntohs(filter->dst_port));
> 
> If there was an error, this dev_info is possibly lying to the user.

Will fix this in v2.

> 
>> +
>> +	return ret;
>> +}
>> +
>> +/**
>> + * i40e_parse_cls_flower - Parse tc flower filters provided by kernel
>> + * @vsi: Pointer to VSI
>> + * @cls_flower: Pointer to struct tc_cls_flower_offload
>> + * @filter: Pointer to cloud filter structure
>> + *
>> + **/
>> +static int i40e_parse_cls_flower(struct i40e_vsi *vsi,
>> +				 struct tc_cls_flower_offload *f,
>> +				 struct i40e_cloud_filter *filter)
>> +{
>> +	struct i40e_pf *pf = vsi->back;
>> +	u16 addr_type = 0;
>> +	u8 field_flags = 0;
>> +
>> +	if (f->dissector->used_keys &
>> +	    ~(BIT(FLOW_DISSECTOR_KEY_CONTROL) |
>> +	      BIT(FLOW_DISSECTOR_KEY_BASIC) |
>> +	      BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS) |
>> +	      BIT(FLOW_DISSECTOR_KEY_VLAN) |
>> +	      BIT(FLOW_DISSECTOR_KEY_IPV4_ADDRS) |
>> +	      BIT(FLOW_DISSECTOR_KEY_IPV6_ADDRS) |
>> +	      BIT(FLOW_DISSECTOR_KEY_PORTS) |
>> +	      BIT(FLOW_DISSECTOR_KEY_ENC_KEYID) |
>> +	      BIT(FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS) |
>> +	      BIT(FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS) |
>> +	      BIT(FLOW_DISSECTOR_KEY_ENC_PORTS)	|
>> +	      BIT(FLOW_DISSECTOR_KEY_ENC_CONTROL))) {
>> +		dev_err(&pf->pdev->dev, "Unsupported key used: 0x%x\n",
>> +			f->dissector->used_keys);
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_ENC_KEYID)) {
>> +		struct flow_dissector_key_keyid *key =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_ENC_KEYID,
>> +						  f->key);
>> +
>> +		struct flow_dissector_key_keyid *mask =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_ENC_KEYID,
>> +						  f->mask);
>> +
>> +		if (mask->keyid != 0)
>> +			field_flags |= I40E_CLOUD_FIELD_TEN_ID;
>> +
>> +		filter->tenant_id = be32_to_cpu(key->keyid);
>> +	}
>> +
>> +	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_BASIC)) {
>> +		struct flow_dissector_key_basic *key =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_BASIC,
>> +						  f->key);
>> +
>> +		filter->ip_proto = key->ip_proto;
>> +	}
>> +
>> +	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS)) {
>> +		struct flow_dissector_key_eth_addrs *key =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_ETH_ADDRS,
>> +						  f->key);
>> +
>> +		struct flow_dissector_key_eth_addrs *mask =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_ETH_ADDRS,
>> +						  f->mask);
>> +
>> +		/* use is_broadcast and is_zero to check for all 0xf or 0 */
>> +		if (!is_zero_ether_addr(mask->dst)) {
>> +			if (is_broadcast_ether_addr(mask->dst)) {
>> +				field_flags |= I40E_CLOUD_FIELD_OMAC;
>> +			} else {
>> +				dev_err(&pf->pdev->dev, "Bad ether dest mask %pM\n",
>> +					mask->dst);
>> +				return I40E_ERR_CONFIG;
>> +			}
>> +		}
>> +
>> +		if (!is_zero_ether_addr(mask->src)) {
>> +			if (is_broadcast_ether_addr(mask->src)) {
>> +				field_flags |= I40E_CLOUD_FIELD_IMAC;
>> +			} else {
>> +				dev_err(&pf->pdev->dev, "Bad ether src mask %pM\n",
>> +					mask->src);
>> +				return I40E_ERR_CONFIG;
>> +			}
>> +		}
>> +		ether_addr_copy(filter->dst_mac, key->dst);
>> +		ether_addr_copy(filter->src_mac, key->src);
>> +	}
>> +
>> +	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_VLAN)) {
>> +		struct flow_dissector_key_vlan *key =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_VLAN,
>> +						  f->key);
>> +		struct flow_dissector_key_vlan *mask =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_VLAN,
>> +						  f->mask);
>> +
>> +		if (mask->vlan_id) {
>> +			if (mask->vlan_id == VLAN_VID_MASK) {
>> +				field_flags |= I40E_CLOUD_FIELD_IVLAN;
>> +			} else {
>> +				dev_err(&pf->pdev->dev, "Bad vlan mask %u\n",
> 
> Should this be a %04x rather than %u?

That's right. Will fix this in v2.

> 
>> +					mask->vlan_id);
>> +				return I40E_ERR_CONFIG;
>> +			}
>> +		}
>> +
>> +		filter->vlan_id = cpu_to_be16(key->vlan_id);
>> +	}
>> +
>> +	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_CONTROL)) {
>> +		struct flow_dissector_key_control *key =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_CONTROL,
>> +						  f->key);
>> +
>> +		addr_type = key->addr_type;
>> +	}
>> +
>> +	if (addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
>> +		struct flow_dissector_key_ipv4_addrs *key =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_IPV4_ADDRS,
>> +						  f->key);
>> +		struct flow_dissector_key_ipv4_addrs *mask =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_IPV4_ADDRS,
>> +						  f->mask);
>> +
>> +		if (mask->dst) {
>> +			if (mask->dst == cpu_to_be32(0xffffffff)) {
>> +				field_flags |= I40E_CLOUD_FIELD_IIP;
>> +			} else {
>> +				dev_err(&pf->pdev->dev, "Bad ip dst mask 0x%08x\n",
>> +					be32_to_cpu(mask->dst));
>> +				return I40E_ERR_CONFIG;
>> +			}
>> +		}
>> +
>> +		if (mask->src) {
>> +			if (mask->src == cpu_to_be32(0xffffffff)) {
>> +				field_flags |= I40E_CLOUD_FIELD_IIP;
>> +			} else {
>> +				dev_err(&pf->pdev->dev, "Bad ip src mask 0x%08x\n",
>> +					be32_to_cpu(mask->dst));
>> +				return I40E_ERR_CONFIG;
>> +			}
>> +		}
>> +
>> +		if (field_flags & I40E_CLOUD_FIELD_TEN_ID) {
>> +			dev_err(&pf->pdev->dev, "Tenant id not allowed for ip filter\n");
>> +			return I40E_ERR_CONFIG;
>> +		}
>> +		filter->dst_ip[0] = key->dst;
>> +		filter->src_ip[0] = key->src;
>> +	}
>> +
>> +	if (addr_type == FLOW_DISSECTOR_KEY_IPV6_ADDRS) {
>> +		struct flow_dissector_key_ipv6_addrs *key =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_IPV6_ADDRS,
>> +						  f->key);
>> +		struct flow_dissector_key_ipv6_addrs *mask =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_IPV6_ADDRS,
>> +						  f->mask);
>> +
>> +		/* validate mask, make sure it is not IPV6_ADDR_ANY */
>> +		if (ipv6_addr_any(&mask->dst)) {
>> +			dev_err(&pf->pdev->dev, "Bad ipv6 dst mask 0x%02x\n",
>> +				IPV6_ADDR_ANY);
>> +			return I40E_ERR_CONFIG;
>> +		}
>> +
>> +		/* validate src and dest IPV6 address, make sure they are not
>> +		 * ANY (0:0:0:0:0:0:0:0) or LOOPBACK (0:0:0:0:0:0:0:1), which
>> +		 * can be represented as ::1
>> +		 */
>> +		if (ipv6_addr_any(&key->dst) || ipv6_addr_loopback(&key->dst)) {
>> +			dev_err(&pf->pdev->dev,
>> +				"Bad ipv6 dst addr is ANY or LOOPBACK\n");
>> +			return I40E_ERR_CONFIG;
>> +		}
>> +		if (ipv6_addr_loopback(&key->src)) {
>> +			dev_err(&pf->pdev->dev,
>> +				"Bad ipv6 src addr is ANY or LOOPBACK\n");
>> +			return I40E_ERR_CONFIG;
>> +		}
>> +		memcpy(&filter->src_ipv6, &key->src.s6_addr,
>> +		       ARRAY_SIZE(filter->src_ipv6));
>> +		memcpy(&filter->dst_ipv6, &key->dst.s6_addr,
>> +		       ARRAY_SIZE(filter->dst_ipv6));
>> +
>> +		/* mark it as IPv6 filter, to be used later */
>> +		filter->is_ipv6 = true;
>> +
>> +		/* and it is IP[4|6] filter type */
>> +		field_flags |= I40E_CLOUD_FIELD_IIP;
>> +	}
>> +
>> +	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_PORTS)) {
>> +		struct flow_dissector_key_ports *key =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_PORTS,
>> +						  f->key);
>> +		struct flow_dissector_key_ports *mask =
>> +			skb_flow_dissector_target(f->dissector,
>> +						  FLOW_DISSECTOR_KEY_PORTS,
>> +						  f->mask);
>> +
>> +		if (mask->src) {
>> +			if (mask->src == cpu_to_be16(0xffff)) {
>> +				field_flags |= I40E_CLOUD_FIELD_IIP;
>> +			} else {
>> +				dev_err(&pf->pdev->dev, "Bad src port mask %u\n",
> 
> Should this be a %04x rather than %u?

Will fix this in v2.

> 
>> +					be16_to_cpu(mask->src));
>> +				return I40E_ERR_CONFIG;
>> +			}
>> +		}
>> +
>> +		if (mask->dst) {
>> +			if (mask->dst == cpu_to_be16(0xffff)) {
>> +				field_flags |= I40E_CLOUD_FIELD_IIP;
>> +			} else {
>> +				dev_err(&pf->pdev->dev, "Bad dst port mask %u\n",
> 
> Should this be a %04x rather than %u?
> 
>> +					be16_to_cpu(mask->dst));
>> +				return I40E_ERR_CONFIG;
>> +			}
>> +		}
>> +
>> +		filter->dst_port = key->dst;
>> +		filter->src_port = key->src;
>> +
>> +		/* For now, only supports destination port*/
>> +		filter->port_type |= I40E_CLOUD_FILTER_PORT_DEST;
>> +
>> +		switch (filter->ip_proto) {
>> +		case IPPROTO_TCP:
>> +		case IPPROTO_UDP:
>> +			break;
>> +		default:
>> +			dev_err(&pf->pdev->dev,
>> +				"Only UDP and TCP transport are supported\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +	filter->flags = field_flags;
>> +	return 0;
>> +}
>> +
>> +/**
>> + * i40e_handle_redirect_action: Forward to a traffic class on the device
>> + * @vsi: Pointer to VSI
>> + * @ifindex: ifindex of the device to forwared to
>> + * @tc: traffic class index on the device
>> + * @filter: Pointer to cloud filter structure
>> + *
>> + **/
>> +static int i40e_handle_redirect_action(struct i40e_vsi *vsi, int ifindex, u8 tc,
>> +				       struct i40e_cloud_filter *filter)
>> +{
>> +	struct i40e_channel *ch, *ch_tmp;
>> +
>> +	/* redirect to a traffic class on the same device */
>> +	if (vsi->netdev->ifindex == ifindex) {
>> +		if (tc == 0) {
>> +			filter->seid = vsi->seid;
>> +			return 0;
>> +		} else if (vsi->tc_config.enabled_tc & BIT(tc)) {
>> +			if (!filter->dst_port) {
>> +				dev_err(&vsi->back->pdev->dev,
>> +					"Specify destination port to redirect to traffic class that is not default\n");
>> +				return -EINVAL;
>> +			}
>> +			if (list_empty(&vsi->ch_list))
>> +				return -EINVAL;
>> +			list_for_each_entry_safe(ch, ch_tmp, &vsi->ch_list,
>> +						 list) {
>> +				if (ch->seid == vsi->tc_seid_map[tc])
>> +					filter->seid = ch->seid;
>> +			}
>> +			return 0;
>> +		}
>> +	}
>> +	return -EINVAL;
>> +}
>> +
>> +/**
>> + * i40e_parse_tc_actions - Parse tc actions
>> + * @vsi: Pointer to VSI
>> + * @cls_flower: Pointer to struct tc_cls_flower_offload
>> + * @filter: Pointer to cloud filter structure
>> + *
>> + **/
>> +static int i40e_parse_tc_actions(struct i40e_vsi *vsi, struct tcf_exts *exts,
>> +				 struct i40e_cloud_filter *filter)
>> +{
>> +	const struct tc_action *a;
>> +	LIST_HEAD(actions);
>> +	int err;
>> +
>> +	if (tc_no_actions(exts))
>> +		return -EINVAL;
>> +
>> +	tcf_exts_to_list(exts, &actions);
>> +	list_for_each_entry(a, &actions, list) {
>> +		/* Drop action */
>> +		if (is_tcf_gact_shot(a)) {
>> +			dev_err(&vsi->back->pdev->dev,
>> +				"Cloud filters do not support the drop action.\n");
>> +			return -EOPNOTSUPP;
>> +		}
>> +
>> +		/* Redirect to a traffic class on the same device */
>> +		if (!is_tcf_mirred_egress_redirect(a)) {
>> +			int ifindex = tcf_mirred_ifindex(a);
>> +			int tc = tcf_mirred_tc(a);
>> +
>> +			err = i40e_handle_redirect_action(vsi, ifindex, tc,
>> +							  filter);
>> +			if (err == 0)
>> +				return err;
>> +		}
>> +	}
>> +	return -EINVAL;
>> +}
>> +
>> +/**
>> + * i40e_configure_clsflower - Configure tc flower filters
>> + * @vsi: Pointer to VSI
>> + * @cls_flower: Pointer to struct tc_cls_flower_offload
>> + *
>> + **/
>> +static int i40e_configure_clsflower(struct i40e_vsi *vsi,
>> +				    struct tc_cls_flower_offload *cls_flower)
>> +{
>> +	struct i40e_cloud_filter *filter = NULL;
>> +	struct i40e_pf *pf = vsi->back;
>> +	int err = 0;
>> +
>> +	if (test_bit(__I40E_RESET_RECOVERY_PENDING, pf->state) ||
>> +	    test_bit(__I40E_RESET_INTR_RECEIVED, pf->state))
>> +		return -EBUSY;
>> +
>> +	if (pf->fdir_pf_active_filters ||
>> +	    (!hlist_empty(&pf->fdir_filter_list))) {
>> +		dev_err(&vsi->back->pdev->dev,
>> +			"Flow Director Sideband filters exists, turn ntuple off to configure cloud filters\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (vsi->back->flags & I40E_FLAG_FD_SB_ENABLED) {
>> +		dev_err(&vsi->back->pdev->dev,
>> +			"Disable Flow Director Sideband while configuring Cloud filters via tc-flower\n");
>> +		vsi->back->flags &= ~I40E_FLAG_FD_SB_ENABLED;
>> +	}
>> +
>> +	filter = kzalloc(sizeof(*filter), GFP_KERNEL);
>> +	if (!filter)
>> +		return -ENOMEM;
>> +
>> +	filter->cookie = cls_flower->cookie;
>> +
>> +	err = i40e_parse_cls_flower(vsi, cls_flower, filter);
>> +	if (err < 0)
>> +		goto err;
>> +
>> +	err = i40e_parse_tc_actions(vsi, cls_flower->exts, filter);
>> +	if (err < 0)
>> +		goto err;
>> +
>> +	/* Add cloud filter */
>> +	if (filter->dst_port)
>> +		err = i40e_add_del_cloud_filter_big_buf(vsi, filter, true);
>> +	else
>> +		err = i40e_add_del_cloud_filter(vsi, filter, true);
>> +
>> +	if (err) {
>> +		dev_err(&pf->pdev->dev,
>> +			"Failed to add cloud filter, err %s\n",
>> +			i40e_stat_str(&pf->hw, err));
>> +		err = i40e_aq_rc_to_posix(err, pf->hw.aq.asq_last_status);
>> +		goto err;
>> +	}
>> +
>> +	/* add filter to the ordered list */
>> +	INIT_HLIST_NODE(&filter->cloud_node);
>> +
>> +	hlist_add_head(&filter->cloud_node, &pf->cloud_filter_list);
>> +
>> +	pf->num_cloud_filters++;
>> +
>> +	return err;
>> +err:
>> +	kfree(filter);
>> +	return err;
>> +}
>> +
>> +/**
>> + * i40e_find_cloud_filter - Find the could filter in the list
>> + * @vsi: Pointer to VSI
>> + * @cookie: filter specific cookie
>> + *
>> + **/
>> +static struct i40e_cloud_filter *i40e_find_cloud_filter(struct i40e_vsi *vsi,
>> +							unsigned long *cookie)
>> +{
>> +	struct i40e_cloud_filter *filter = NULL;
>> +	struct hlist_node *node2;
>> +
>> +	hlist_for_each_entry_safe(filter, node2,
>> +				  &vsi->back->cloud_filter_list, cloud_node)
>> +		if (!memcmp(cookie, &filter->cookie, sizeof(filter->cookie)))
>> +			return filter;
>> +	return NULL;
>> +}
>> +
>> +/**
>> + * i40e_delete_clsflower - Remove tc flower filters
>> + * @vsi: Pointer to VSI
>> + * @cls_flower: Pointer to struct tc_cls_flower_offload
>> + *
>> + **/
>> +static int i40e_delete_clsflower(struct i40e_vsi *vsi,
>> +				 struct tc_cls_flower_offload *cls_flower)
>> +{
>> +	struct i40e_cloud_filter *filter = NULL;
>> +	struct i40e_pf *pf = vsi->back;
>> +	int err = 0;
>> +
>> +	filter = i40e_find_cloud_filter(vsi, &cls_flower->cookie);
>> +
>> +	if (!filter)
>> +		return -EINVAL;
>> +
>> +	hash_del(&filter->cloud_node);
>> +
>> +	if (filter->dst_port)
>> +		err = i40e_add_del_cloud_filter_big_buf(vsi, filter, false);
>> +	else
>> +		err = i40e_add_del_cloud_filter(vsi, filter, false);
>> +	if (err) {
>> +		kfree(filter);
>> +		dev_err(&pf->pdev->dev,
>> +			"Failed to delete cloud filter, err %s\n",
>> +			i40e_stat_str(&pf->hw, err));
>> +		return i40e_aq_rc_to_posix(err, pf->hw.aq.asq_last_status);
> 
> Hmmm... okay, maybe those earlier functions are not really lying if they 
> have this followup to check them.
> 
>> +	}
>> +
>> +	kfree(filter);
>> +	pf->num_cloud_filters--;
>> +
>> +	if (!pf->num_cloud_filters) {
>> +		/* Re-enable FD-SB that was disabled while configuring cloud
>> +		 * filters
>> +		 */
>> +		if ((pf->hw.func_caps.fd_filters_guaranteed > 0) ||
>> +		    (pf->hw.func_caps.fd_filters_best_effort > 0)) {
>> +			if (!(pf->flags & I40E_FLAG_MFP_ENABLED &&
>> +			      pf->hw.num_partitions > 1))
>> +				pf->flags |= I40E_FLAG_FD_SB_ENABLED;
> 
> What if FD_SB was disabled for other reasons, like not enough msix 
> vectors or queues, or some other odd circumstance that turns them off?

I agree, this doesn't totally work to keep FD SB and Cloud filters
exclusive. I'll fix this in v2, perhaps with new PF flags to keep track
of the status of FD SB and Cloud filters.

> 
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static int __i40e_setup_tc(struct net_device *netdev, u32 handle,
>>   			   u32 chain_index, __be16 proto,
>>   			   struct tc_to_netdev *tc)
>>   {
>> +	struct i40e_netdev_priv *np = netdev_priv(netdev);
>> +	struct i40e_vsi *vsi = np->vsi;
>> +
>> +	if (TC_H_MAJ(handle) == TC_H_MAJ(TC_H_INGRESS) &&
>> +	    tc->type == TC_SETUP_CLSFLOWER) {
>> +		switch (tc->cls_flower->command) {
>> +		case TC_CLSFLOWER_REPLACE:
>> +			return i40e_configure_clsflower(vsi, tc->cls_flower);
>> +		case TC_CLSFLOWER_DESTROY:
>> +			return i40e_delete_clsflower(vsi, tc->cls_flower);
>> +		case TC_CLSFLOWER_STATS:
>> +			return -EOPNOTSUPP;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>>   	return i40e_setup_tc(netdev, tc);
>>   }
>>   
>> @@ -6948,6 +7829,16 @@ static void i40e_cloud_filter_exit(struct i40e_pf *pf)
>>   		kfree(cfilter);
>>   	}
>>   	pf->num_cloud_filters = 0;
>> +
>> +	/* Re-enable FD-SB that was disabled while configuring cloud
>> +	 * filters
>> +	 */
>> +	if ((pf->hw.func_caps.fd_filters_guaranteed > 0) ||
>> +	    (pf->hw.func_caps.fd_filters_best_effort > 0)) {
>> +		if (!(pf->flags & I40E_FLAG_MFP_ENABLED &&
>> +		      pf->hw.num_partitions > 1))
>> +			pf->flags |= I40E_FLAG_FD_SB_ENABLED;
> 
> What if FD_SB was disabled for other reasons, like not enough msix 
> vectors or queues, or some other odd circumstance that turns them off?
> 
> This is getting complex and repetative.  Maybe there needs to be a 
> little function that checks all the FD_SB conditions and can be used 
> from anywhere needed.
> 
>> +	}
>>   }
>>   
>>   /**
>> @@ -8157,6 +9048,48 @@ static void i40e_fdir_teardown(struct i40e_pf *pf)
>>   }
>>   
>>   /**
>> + * i40e_rebuild_cloud_filters - Rebuilds cloud filters for VSIs
>> + * @vsi: PF main vsi
>> + * @seid: seid of main or channel VSIs
>> + *
>> + * Rebuilds cloud filters associated with main VSI and channel VSIs if they
>> + * existed before reset
>> + **/
>> +static int i40e_rebuild_cloud_filters(struct i40e_vsi *vsi, u16 seid)
>> +{
>> +	struct i40e_cloud_filter *cfilter;
>> +	struct i40e_pf *pf = vsi->back;
>> +	struct hlist_node *node;
>> +	i40e_status ret;
>> +
>> +	/* Add cloud filters back if they exist */
>> +	if (hlist_empty(&pf->cloud_filter_list))
>> +		return 0;
>> +
>> +	hlist_for_each_entry_safe(cfilter, node, &pf->cloud_filter_list,
>> +				  cloud_node) {
>> +		if (cfilter->seid != seid)
>> +			continue;
>> +
>> +		if (cfilter->dst_port)
>> +			ret = i40e_add_del_cloud_filter_big_buf(vsi, cfilter,
>> +								true);
>> +		else
>> +			ret = i40e_add_del_cloud_filter(vsi, cfilter, true);
>> +
>> +		if (ret) {
>> +			dev_dbg(&pf->pdev->dev,
>> +				"Failed to rebuild cloud filter, err %s aq_err %s\n",
>> +				i40e_stat_str(&pf->hw, ret),
>> +				i40e_aq_str(&pf->hw,
>> +					    pf->hw.aq.asq_last_status));
>> +			return ret;
>> +		}
>> +	}
>> +	return 0;
>> +}
>> +
>> +/**
>>    * i40e_rebuild_channels - Rebuilds channel VSIs if they existed before reset
>>    * @vsi: PF main vsi
>>    *
>> @@ -8193,6 +9126,13 @@ static int i40e_rebuild_channels(struct i40e_vsi *vsi)
>>   						I40E_BW_CREDIT_DIVISOR,
>>   				ch->seid);
>>   		}
>> +		ret = i40e_rebuild_cloud_filters(vsi, ch->seid);
>> +		if (ret) {
>> +			dev_dbg(&vsi->back->pdev->dev,
>> +				"Failed to rebuild cloud filters for channel VSI %u\n",
>> +				ch->seid);
>> +			return ret;
>> +		}
>>   	}
>>   	return 0;
>>   }
>> @@ -8476,6 +9416,10 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
>>   			goto end_unlock;
>>   	}
>>   
>> +	ret = i40e_rebuild_cloud_filters(vsi, vsi->seid);
>> +	if (ret)
>> +		goto end_unlock;
>> +
>>   	/* PF Main VSI is rebuild by now, go ahead and rebuild channel VSIs
>>   	 * for this main VSI if they exist
>>   	 */
>> @@ -10846,7 +11790,8 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
>>   		netdev->hw_features |= NETIF_F_NTUPLE;
>>   	hw_features = hw_enc_features		|
>>   		      NETIF_F_HW_VLAN_CTAG_TX	|
>> -		      NETIF_F_HW_VLAN_CTAG_RX;
>> +		      NETIF_F_HW_VLAN_CTAG_RX	|
>> +		      NETIF_F_HW_TC;
>>   
>>   	netdev->hw_features |= hw_features;
>>   
>> @@ -12123,8 +13068,10 @@ static int i40e_setup_pf_switch(struct i40e_pf *pf, bool reinit)
>>   	*/
>>   
>>   	if ((pf->hw.pf_id == 0) &&
>> -	    !(pf->flags & I40E_FLAG_TRUE_PROMISC_SUPPORT))
>> +	    !(pf->flags & I40E_FLAG_TRUE_PROMISC_SUPPORT)) {
>>   		flags = I40E_AQ_SET_SWITCH_CFG_PROMISC;
>> +		pf->last_sw_conf_flags = flags;
>> +	}
>>   
>>   	if (pf->hw.pf_id == 0) {
>>   		u16 valid_flags;
>> @@ -12140,6 +13087,7 @@ static int i40e_setup_pf_switch(struct i40e_pf *pf, bool reinit)
>>   					     pf->hw.aq.asq_last_status));
>>   			/* not a fatal problem, just keep going */
>>   		}
>> +		pf->last_sw_conf_valid_flags = valid_flags;
>>   	}
>>   
>>   	/* first time setup */
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
>> index 9142d0d..e24f1ce 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
>> @@ -283,6 +283,23 @@ i40e_status i40e_aq_query_switch_comp_bw_config(struct i40e_hw *hw,
>>   		struct i40e_asq_cmd_details *cmd_details);
>>   i40e_status i40e_aq_resume_port_tx(struct i40e_hw *hw,
>>   				   struct i40e_asq_cmd_details *cmd_details);
>> +i40e_status i40e_aq_add_cloud_filters_big_buffer(struct i40e_hw *hw,
>> +	u16 seid,
>> +	struct i40e_aqc_add_remove_cloud_filters_element_big_data *filters,
>> +	u8 filter_count);
>> +enum i40e_status_code i40e_aq_add_cloud_filters(struct i40e_hw *hw,
>> +		u16 vsi,
>> +		struct i40e_aqc_add_remove_cloud_filters_element_data *filters,
>> +		u8 filter_count);
>> +
>> +enum i40e_status_code i40e_aq_remove_cloud_filters(struct i40e_hw *hw,
>> +		u16 vsi,
>> +		struct i40e_aqc_add_remove_cloud_filters_element_data *filters,
>> +		u8 filter_count);
>> +i40e_status i40e_aq_remove_cloud_filters_big_buffer(
>> +	struct i40e_hw *hw, u16 seid,
>> +	struct i40e_aqc_add_remove_cloud_filters_element_big_data *filters,
>> +	u8 filter_count);
>>   i40e_status i40e_read_lldp_cfg(struct i40e_hw *hw,
>>   			       struct i40e_lldp_variables *lldp_cfg);
>>   /* i40e_common */
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>>

^ permalink raw reply

* [PATCH net] udp: fix linear skb reception with PEEK_OFF
From: Paolo Abeni @ 2017-08-14 19:31 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Eric Dumazet, Sasha Levin, Al Viro

From: Al Viro <viro@ZenIV.linux.org.uk>

copy_linear_skb() is broken; both of its callers actually
expect 'len' to be the amount we are trying to copy,
not the offset of the end.
Fix it keeping the meanings of arguments in sync with what the
callers (both of them) expect.
Also restore a saner behavior on EFAULT (i.e. preserving
the iov_iter position in case of failure):

The commit fd851ba9caa9 ("udp: harden copy_linear_skb()")
avoids the more destructive effect of the buggy
copy_linear_skb(), e.g. no more invalid memory access, but
said function still behaves incorrectly: when peeking with
offset it can fail with EINVAL instead of copying the
appropriate amount of memory.

Reported-by: Sasha Levin <alexander.levin@verizon.com>
Fixes: b65ac44674dd ("udp: try to avoid 2 cache miss on dequeue")
Fixes: fd851ba9caa9 ("udp: harden copy_linear_skb()")
Signed-off-by: Al Viro <viro@ZenIV.linux.org.uk>
Acked-by: Paolo Abeni <pabeni@redhat.com>
Tested-by: Sasha Levin <alexander.levin@verizon.com>
---
This patch has been buried in a private email exchange for some
time.
I'm posting it on behalf of Al Viro, to avoid loosing this merge
window, since he is busy elsewhere.
---
 include/net/udp.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index e9b1d1eacb59..586de4b811b5 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -366,14 +366,13 @@ static inline bool udp_skb_is_linear(struct sk_buff *skb)
 static inline int copy_linear_skb(struct sk_buff *skb, int len, int off,
 				  struct iov_iter *to)
 {
-	int n, copy = len - off;
+	int n;
 
-	if (copy < 0)
-		return -EINVAL;
-	n = copy_to_iter(skb->data + off, copy, to);
-	if (n == copy)
+	n = copy_to_iter(skb->data + off, len, to);
+	if (n == len)
 		return 0;
 
+	iov_iter_revert(to, n);
 	return -EFAULT;
 }
 
-- 
2.13.5

^ permalink raw reply related

* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Willem de Bruijn @ 2017-08-14 19:39 UTC (permalink / raw)
  To: Thiago Macieira; +Cc: Matthew Dawson, Paolo Abeni, Network Development
In-Reply-To: <4013878.dizogNlyrN@tjmaciei-mobl1>

On Mon, Aug 14, 2017 at 3:15 PM, Thiago Macieira
<thiago.macieira@intel.com> wrote:
> On Monday, 14 August 2017 12:03:16 PDT Willem de Bruijn wrote:
>> On Mon, Aug 14, 2017 at 2:58 PM, Thiago Macieira
>>
>> <thiago.macieira@intel.com> wrote:
>> > On Monday, 14 August 2017 11:46:42 PDT Willem de Bruijn wrote:
>> >> > By the way, what were the usecases for the peek offset feature?
>> >>
>> >> The idea was to be able to peek at application headers of upper
>> >> layer protocols and multiplex messages among threads. It proved
>> >> so complex even for UDP that we did not attempt the same feature
>> >> for TCP. Also, KCM implements demultiplexing using eBPF today.
>> >
>> > Interesting, but how would userspace coordinate like that? Suppose
>> > multiple
>> > threads are woken up by a datagram being received
>>
>> This assumes a separate listener thread and worker threadpool.
>
> The listener thread still needs to synchronise with the worker that got
> activated and wait for it to recv from the socket before the listener thread
> can go back to poll().
>
> If we are really talking about threads in the same process, it might be easier
> for the listener to just read the datagram anyway and pass it on to the
> worker. That way, it can proceed immediately to the next datagram and not have
> to wait for the possibly slow worker.
>
> If it is a separate process, then I don't see another way and this might be
> necessary.
>
> By the way, what does recv with MSG_PEEK | MSG_TRUNC return? Is it the full
> datagram's size or is it the size minus the peek offset?

udp_recvmsg returns ulen if the flag is passed.

        if (flags & MSG_TRUNC)
                err = ulen;

This is computed earlier in the function as udp payload length and not
modified after.

        ulen = udp_skb_len(skb);

^ permalink raw reply

* Re: [iproute PATCH 51/51] lib/bpf: Check return value of write()
From: Daniel Borkmann @ 2017-08-14 20:35 UTC (permalink / raw)
  To: Phil Sutter, Stephen Hemminger, netdev
In-Reply-To: <20170814172526.GW16375@orbyte.nwl.cc>

On 08/14/2017 07:25 PM, Phil Sutter wrote:
[...]
> But I really think we shouldn't make such a fuss about it - writing to
> stderr either always works or we're in trouble everywhere. This patch
> was merely to shut gcc up, so no need to waste much energy on a scenario
> which won't happen anyway.

Yup, fair enough, makes sense.

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

^ permalink raw reply

* Re: ipv4: distinguish EHOSTUNREACH from the ENETUNREACH
From: Daniel Walker @ 2017-08-14 20:35 UTC (permalink / raw)
  To: Duan Jiong, David S. Miller, Poornima Ranganath,
	xe-linux-external, netdev

Hi,


It seems like commit cd0f0b is trying to add back these two errors 
values into ip_route_input_slow(). However, if you follow the code path 
further down you get to the two exit points of this function,

in net/ipv4/route.c:ip_route_input_slow()

if (rt_cache_valid(rth)) {
	skb_dst_set_noref(skb, &rth->dst);
	err = 0;
	goto out;
}

and

skb_dst_set(skb, &rth->dst);
err = 0;
goto out;

Both of these set "err" variable to 0. This effective destroys the 
return value which the patch seems to be adding. Am I missing something 
here?


Thanks,

Daniel

^ permalink raw reply

* Cycling Enthusiasts List
From: Jens Altmann @ 2017-08-14 18:49 UTC (permalink / raw)
  To: netdev


Hi,

Greetings of the day!

Would you be interested in acquiring an email list of "Cycling Enthusiasts" from USA?

We also have data for Hikign Enthusiasts, Running Enthusiasts, Camping and Outdoor Enthusiasts, Skiers List, Health and Fitness Enthusiasts, Tennis Enthusiasts, Boxing Enthusiasts, Travelers List, Frequent Travelers List, Basketball Enthusiasts, Golfers, Soccer Enthusiasts, Baseball Enthusiasts and many more.

Each record in the list contains Contact Name (First, Middle and Last Name), Mailing Address, List type and Opt-in email address.

All the contacts are opt-in verified, complete permission based and can be used for unlimited multi-channel marketing.

Please let me know your thoughts towards procuring the Cycling Enthusiasts List.

Best Regards,
Jens Altmann
Marketing Manager



We respect your privacy, if you do not wish to receive any further emails from our end, please reply with a subject “Leave Out”.

^ permalink raw reply

* Re: [PATCH net] udp: fix linear skb reception with PEEK_OFF
From: Eric Dumazet @ 2017-08-14 21:06 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, David S. Miller, Eric Dumazet, Sasha Levin, Al Viro
In-Reply-To: <18c1b112032a685518548760890547fdbf292b85.1502739025.git.pabeni@redhat.com>

On Mon, 2017-08-14 at 21:31 +0200, Paolo Abeni wrote:
> From: Al Viro <viro@ZenIV.linux.org.uk>
> 
> copy_linear_skb() is broken; both of its callers actually
> expect 'len' to be the amount we are trying to copy,
> not the offset of the end.
> Fix it keeping the meanings of arguments in sync with what the
> callers (both of them) expect.
> Also restore a saner behavior on EFAULT (i.e. preserving
> the iov_iter position in case of failure):
> 
> The commit fd851ba9caa9 ("udp: harden copy_linear_skb()")
> avoids the more destructive effect of the buggy
> copy_linear_skb(), e.g. no more invalid memory access, but
> said function still behaves incorrectly: when peeking with
> offset it can fail with EINVAL instead of copying the
> appropriate amount of memory.
> 
> Reported-by: Sasha Levin <alexander.levin@verizon.com>
> Fixes: b65ac44674dd ("udp: try to avoid 2 cache miss on dequeue")
> Fixes: fd851ba9caa9 ("udp: harden copy_linear_skb()")
> Signed-off-by: Al Viro <viro@ZenIV.linux.org.uk>
> Acked-by: Paolo Abeni <pabeni@redhat.com>
> Tested-by: Sasha Levin <alexander.levin@verizon.com>
> ---

Oh well.

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* [PATCH net] dccp: purge write queue in dccp_destroy_sock()
From: Eric Dumazet @ 2017-08-14 21:10 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Gerrit Renker

From: Eric Dumazet <edumazet@google.com>

syzkaller reported that DCCP could have a non empty
write queue at dismantle time.

WARNING: CPU: 1 PID: 2953 at net/core/stream.c:199 sk_stream_kill_queues+0x3ce/0x520 net/core/stream.c:199
Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 2953 Comm: syz-executor0 Not tainted 4.13.0-rc4+ #2
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:16 [inline]
 dump_stack+0x194/0x257 lib/dump_stack.c:52
 panic+0x1e4/0x417 kernel/panic.c:180
 __warn+0x1c4/0x1d9 kernel/panic.c:541
 report_bug+0x211/0x2d0 lib/bug.c:183
 fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:190
 do_trap_no_signal arch/x86/kernel/traps.c:224 [inline]
 do_trap+0x260/0x390 arch/x86/kernel/traps.c:273
 do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:310
 do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:323
 invalid_op+0x1e/0x30 arch/x86/entry/entry_64.S:846
RIP: 0010:sk_stream_kill_queues+0x3ce/0x520 net/core/stream.c:199
RSP: 0018:ffff8801d182f108 EFLAGS: 00010297
RAX: ffff8801d1144140 RBX: ffff8801d13cb280 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff85137b00 RDI: ffff8801d13cb280
RBP: ffff8801d182f148 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801d13cb4d0
R13: ffff8801d13cb3b8 R14: ffff8801d13cb300 R15: ffff8801d13cb3b8
 inet_csk_destroy_sock+0x175/0x3f0 net/ipv4/inet_connection_sock.c:835
 dccp_close+0x84d/0xc10 net/dccp/proto.c:1067
 inet_release+0xed/0x1c0 net/ipv4/af_inet.c:425
 sock_release+0x8d/0x1e0 net/socket.c:597
 sock_close+0x16/0x20 net/socket.c:1126
 __fput+0x327/0x7e0 fs/file_table.c:210
 ____fput+0x15/0x20 fs/file_table.c:246
 task_work_run+0x18a/0x260 kernel/task_work.c:116
 exit_task_work include/linux/task_work.h:21 [inline]
 do_exit+0xa32/0x1b10 kernel/exit.c:865
 do_group_exit+0x149/0x400 kernel/exit.c:969
 get_signal+0x7e8/0x17e0 kernel/signal.c:2330
 do_signal+0x94/0x1ee0 arch/x86/kernel/signal.c:808
 exit_to_usermode_loop+0x21c/0x2d0 arch/x86/entry/common.c:157
 prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
 syscall_return_slowpath+0x3a7/0x450 arch/x86/entry/common.c:263

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
---
 net/dccp/proto.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index 9fe25bf63296..86bc40ba6ba5 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -201,10 +201,7 @@ void dccp_destroy_sock(struct sock *sk)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
 
-	/*
-	 * DCCP doesn't use sk_write_queue, just sk_send_head
-	 * for retransmissions
-	 */
+	__skb_queue_purge(&sk->sk_write_queue);
 	if (sk->sk_send_head != NULL) {
 		kfree_skb(sk->sk_send_head);
 		sk->sk_send_head = NULL;

^ permalink raw reply related

* Re: [PATCH] net/sched: reset block pointer in tcf_block_put()
From: Cong Wang @ 2017-08-14 21:15 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Linux Kernel Network Developers, Jiri Pirko, David S. Miller,
	Jamal Hadi Salim
In-Reply-To: <7267faab-c089-e60b-0696-3734f801c1ba@yandex-team.ru>

On Mon, Aug 14, 2017 at 5:59 AM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
>
> This should work, I suppose.
>
> But this approach requires careful review for all qdisc, mine is completely
> mechanical.

Well, we don't have many classful qdisc's. Your patch actually
touches more qdisc's than mine, because you change an API, so
it is slightly harder to backport. ;)

^ permalink raw reply

* Re: [net-next 15/15] i40e: synchronize nvmupdate command and adminq subtask
From: Shannon Nelson @ 2017-08-14 21:40 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: David Miller, Sudheer Mogilappagari, netdev, nhorman,
	Stefan Assmann, jogreene
In-Reply-To: <20170812110848.18264-17-jeffrey.t.kirsher@intel.com>

On Sat, Aug 12, 2017 at 4:08 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
>
> During NVM update, state machine gets into unrecoverable state because
> i40e_clean_adminq_subtask can get scheduled after the admin queue
> command but before other state variables are updated. This causes
> incorrect input to i40e_nvmupd_check_wait_event and state transitions
> don't happen.
>
> This issue existed before but surfaced after commit 373149fc99a0
> ("i40e: Decrease the scope of rtnl lock")
>
> This fix adds locking around admin queue command and update of
> state variables so that adminq_subtask will have accurate information
> whenever it gets scheduled.
>
> Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_nvm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_nvm.c b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
> index 6fdecd70dcbc..2cf7db2dc7cd 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_nvm.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
> @@ -753,6 +753,11 @@ i40e_status i40e_nvmupd_command(struct i40e_hw *hw,
>                 hw->nvmupd_state = I40E_NVMUPD_STATE_INIT;
>         }
>
> +       /* Acquire lock to prevent race condition where adminq_task
> +        * can execute after i40e_nvmupd_nvm_read/write but before state
> +        * variables (nvm_wait_opcode, nvm_release_on_done) are updated
> +        */
> +       mutex_lock(&hw->aq.arq_mutex);
>         switch (hw->nvmupd_state) {
>         case I40E_NVMUPD_STATE_INIT:
>                 status = i40e_nvmupd_state_init(hw, cmd, bytes, perrno);
> @@ -788,6 +793,7 @@ i40e_status i40e_nvmupd_command(struct i40e_hw *hw,
>                 *perrno = -ESRCH;
>                 break;
>         }

Perhaps I missed a patch somewhere, but I think there is still a
return statement in the middle of this switch() (INIT_WAIT and
WRITE_WAIT) that means you can leave the mutex locked.  I thought I
had seen a newer version of this patch that had this fixed

sln

> +       mutex_unlock(&hw->aq.arq_mutex);
>         return status;
>  }
>
> --
> 2.14.0
>



-- 
==============================================
Mr. Shannon Nelson         Parents can't afford to be squeamish.

^ permalink raw reply

* RE: [net-next 08/15] i40e/i40evf: organize and re-number feature flags
From: Keller, Jacob E @ 2017-08-14 22:11 UTC (permalink / raw)
  To: David Miller, Kirsher, Jeffrey T
  Cc: netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
	jogreene@redhat.com
In-Reply-To: <20170812.130405.1462747317744893185.davem@davemloft.net>



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Saturday, August 12, 2017 1:04 PM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; netdev@vger.kernel.org;
> nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 08/15] i40e/i40evf: organize and re-number feature flags
> 
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Sat, 12 Aug 2017 04:08:41 -0700
> 
> > Also ensure that the flags variable is actually a u64 to guarantee
> > 64bits of space on all architectures.
> 
> Why?  You don't need 64-bits, you only need 27.
> 
> This will be unnecessarily expensive on 32-bit platforms.
> 
> Please don't do this.

I suppose a better method would be to switch to using a declare_bitmap instead, so that it automatically sizes based on the number of flags we have. The reason we chose 64bits is because we will add flags in the future, as we originally had more than 32 flags prior to this patch until we moved some into a separate field.

But now that I think about it, using DECLARE_BITMAP makes more sense, though it's a bit more invasive of the code.

Thanks,
Jake

^ permalink raw reply

* [PATCH net-next 00/11] net: dsa: add generic debugfs interface
From: Vivien Didelot @ 2017-08-14 22:22 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Volodymyr Bendiuga, Nikita Yushchenko,
	Maxime Hadjinlian, Chris Healy, Maxim Uvarov, Stefan Eichenberger,
	Jason Cobham, Juergen Borleis, Tobias Waldekranz, Vivien Didelot

This patch series adds a generic debugfs interface for the DSA
framework, so that all switch devices benefit from it, e.g. Marvell,
Broadcom, Microchip or any other DSA driver.

This is really convenient for debugging, especially CPU ports and DSA
links which are not exposed to userspace as net device. This interface
is currently the only way to easily inspect the hardware for such ports.

With the patch series, any switch device user is able to query the
hardware for the supported tagging protocol, the ports stats and
registers, as well as their FDB, MDB and VLAN entries.

This support is only compiled if CONFIG_DEBUG_FS is enabled. Below is
and example of usage of this interface on a multi-chip switch fabric:

    # mount -t debugfs none /sys/kernel/debug
    # cd /sys/kernel/debug/dsa/
    # ls
    switch0  switch1 switch2
    # ls -l switch0/
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port0
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port1
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port2
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port5
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port6
    -r--r--r-- 1 root root 0 Jan  1 00:00 tag_protocol
    -r--r--r-- 1 root root 0 Jan  1 00:00 tree
    # ls -l switch0/port6
    -r--r--r-- 1 root root 0 Jan  1 00:00 fdb
    -r--r--r-- 1 root root 0 Jan  1 00:00 mdb
    -r--r--r-- 1 root root 0 Jan  1 00:00 regs
    -r--r--r-- 1 root root 0 Jan  1 00:00 stats
    -r--r--r-- 1 root root 0 Jan  1 00:00 vlan
    # cat switch0/port2/vlan
    vid 42  pvid  untagged
    # cat switch0/port1/fdb
    vid 0    12:34:56:78:90:ab    static    unicast
    # pr -mt switch0/port{5,6}/stats
    in_good_octets      : 0             in_good_octets      : 13824
    in_bad_octets       : 0             in_bad_octets       : 0
    in_unicast          : 0             in_unicast          : 0
    in_broadcasts       : 0             in_broadcasts       : 216
    in_multicasts       : 0             in_multicasts       : 0
    in_pause            : 0             in_pause            : 0
    in_undersize        : 0             in_undersize        : 0
    ...
    # pr -mt switch0/port{5,6}/regs
     0: 4e07			     0: 4d04
     1: 403e			     1: 003d
     2: 0000			     2: 0000
     3: 3521			     3: 3521
     4: 0533			     4: 373f
     5: 8000			     5: 0000
     6: 005f			     6: 003f
     7: 002a			     7: 002a
    ...

where switch0 port5 and port6 are CPU and DSA ports of a ZII Rev B.

Vivien Didelot (11):
  net: dsa: legacy: assign dst->applied
  net: dsa: add debugfs interface
  net: dsa: debugfs: add tree
  net: dsa: debugfs: add tag_protocol
  net: dsa: debugfs: add port stats
  net: dsa: debugfs: add port registers
  net: dsa: debugfs: add port fdb
  net: dsa: restore mdb dump
  net: dsa: debugfs: add port mdb
  net: dsa: restore VLAN dump
  net: dsa: debugfs: add port vlan

 drivers/net/dsa/b53/b53_common.c       |  41 +++
 drivers/net/dsa/b53/b53_priv.h         |   2 +
 drivers/net/dsa/bcm_sf2.c              |   1 +
 drivers/net/dsa/dsa_loop.c             |  38 +++
 drivers/net/dsa/microchip/ksz_common.c |  41 +++
 drivers/net/dsa/mv88e6xxx/chip.c       |  82 +++++-
 include/net/dsa.h                      |  15 ++
 net/dsa/Kconfig                        |  14 +
 net/dsa/Makefile                       |   1 +
 net/dsa/debugfs.c                      | 453 +++++++++++++++++++++++++++++++++
 net/dsa/dsa.c                          |   3 +
 net/dsa/dsa2.c                         |   4 +
 net/dsa/dsa_priv.h                     |  13 +
 net/dsa/legacy.c                       |   7 +
 14 files changed, 707 insertions(+), 8 deletions(-)
 create mode 100644 net/dsa/debugfs.c

-- 
2.14.0

^ permalink raw reply

* [PATCH net-next 01/11] net: dsa: legacy: assign dst->applied
From: Vivien Didelot @ 2017-08-14 22:22 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Volodymyr Bendiuga, Nikita Yushchenko,
	Maxime Hadjinlian, Chris Healy, Maxim Uvarov, Stefan Eichenberger,
	Jason Cobham, Juergen Borleis, Tobias Waldekranz, Vivien Didelot
In-Reply-To: <20170814222242.10643-1-vivien.didelot@savoirfairelinux.com>

The "applied" boolean of the dsa_switch_tree is only set in the new
bindings. This patch sets it in the legacy code as well.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/legacy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 91e6f7981d39..a6a0849483d1 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -605,6 +605,7 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 	 */
 	wmb();
 	dev->dsa_ptr = dst;
+	dst->applied = true;
 
 	return 0;
 }
@@ -689,6 +690,8 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
 
 	dev_put(dst->cpu_dp->netdev);
+
+	dst->applied = false;
 }
 
 static int dsa_remove(struct platform_device *pdev)
-- 
2.14.0

^ permalink raw reply related

* [PATCH net-next 02/11] net: dsa: add debugfs interface
From: Vivien Didelot @ 2017-08-14 22:22 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Volodymyr Bendiuga, Nikita Yushchenko,
	Maxime Hadjinlian, Chris Healy, Maxim Uvarov, Stefan Eichenberger,
	Jason Cobham, Juergen Borleis, Tobias Waldekranz, Vivien Didelot
In-Reply-To: <20170814222242.10643-1-vivien.didelot@savoirfairelinux.com>

This commit adds a DEBUG_FS dependent DSA core file creating a generic
debug filesystem interface for the DSA switch devices.

The interface can be mounted with:

    # mount -t debugfs none /sys/kernel/debug

The dsa directory contains one directory per switch chip:

    # cd /sys/kernel/debug/dsa/
    # ls
    switch0  switch1 switch2

Each chip directory contains one directory per port:

    # ls -l switch0/
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port0
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port1
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port2
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port5
    drwxr-xr-x 2 root root 0 Jan  1 00:00 port6

Future patches will add entry files to these directories.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h  |   7 ++++
 net/dsa/Kconfig    |  14 +++++++
 net/dsa/Makefile   |   1 +
 net/dsa/debugfs.c  | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/dsa/dsa.c      |   3 ++
 net/dsa/dsa2.c     |   4 ++
 net/dsa/dsa_priv.h |  13 ++++++
 net/dsa/legacy.c   |   4 ++
 8 files changed, 167 insertions(+)
 create mode 100644 net/dsa/debugfs.c

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7f46b521313e..4ef5d38755d9 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -212,6 +212,13 @@ struct dsa_switch {
 	 */
 	void *priv;
 
+#ifdef CONFIG_NET_DSA_DEBUGFS
+	/*
+	 * Debugfs interface.
+	 */
+	struct dentry *debugfs_dir;
+#endif
+
 	/*
 	 * Configuration data for this switch.
 	 */
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index cc5f8f971689..0f05a1e59dd2 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -15,6 +15,20 @@ config NET_DSA
 
 if NET_DSA
 
+config NET_DSA_DEBUGFS
+	bool "Distributed Switch Architecture debugfs interface"
+	depends on DEBUG_FS
+	---help---
+	  Enable creation of debugfs files for the DSA core.
+
+	  These debugfs files provide per-switch information, such as the tag
+	  protocol in use and ports connectivity. They also allow querying the
+	  hardware directly through the switch operations for debugging instead
+	  of going through the bridge, switchdev and DSA layers.
+
+	  This is also a way to inspect the stats and FDB, MDB or VLAN entries
+	  of CPU and DSA links, since they are not exposed to userspace.
+
 # tagging formats
 config NET_DSA_TAG_BRCM
 	bool
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index fcce25da937c..7f60c6dfaffb 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -1,6 +1,7 @@
 # the core
 obj-$(CONFIG_NET_DSA) += dsa_core.o
 dsa_core-y += dsa.o dsa2.o legacy.o port.o slave.o switch.o
+dsa_core-$(CONFIG_NET_DSA_DEBUGFS) += debugfs.o
 
 # tagging formats
 dsa_core-$(CONFIG_NET_DSA_TAG_BRCM) += tag_brcm.o
diff --git a/net/dsa/debugfs.c b/net/dsa/debugfs.c
new file mode 100644
index 000000000000..68caf5a2c0c3
--- /dev/null
+++ b/net/dsa/debugfs.c
@@ -0,0 +1,121 @@
+/*
+ * net/dsa/debugfs.c - DSA debugfs interface
+ * Copyright (c) 2017 Savoir-faire Linux, Inc.
+ *	Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/debugfs.h>
+
+#include "dsa_priv.h"
+
+#define DSA_SWITCH_FMT	"switch%d"
+#define DSA_PORT_FMT	"port%d"
+
+/* DSA module debugfs directory */
+static struct dentry *dsa_debugfs_dir;
+
+static int dsa_debugfs_create_port(struct dsa_switch *ds, int port)
+{
+	struct dentry *dir;
+	char name[32];
+
+	snprintf(name, sizeof(name), DSA_PORT_FMT, port);
+
+	dir = debugfs_create_dir(name, ds->debugfs_dir);
+	if (IS_ERR_OR_NULL(dir))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int dsa_debugfs_create_switch(struct dsa_switch *ds)
+{
+	char name[32];
+	int i;
+
+	/* skip if there is no debugfs support */
+	if (!dsa_debugfs_dir)
+		return 0;
+
+	snprintf(name, sizeof(name), DSA_SWITCH_FMT, ds->index);
+
+	ds->debugfs_dir = debugfs_create_dir(name, dsa_debugfs_dir);
+	if (IS_ERR_OR_NULL(ds->debugfs_dir))
+		return -EFAULT;
+
+	for (i = 0; i < ds->num_ports; i++) {
+		if (!ds->ports[i].dn)
+			continue;
+
+		err = dsa_debugfs_create_port(ds, i);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static void dsa_debugfs_destroy_switch(struct dsa_switch *ds)
+{
+	/* handles NULL */
+	debugfs_remove_recursive(ds->debugfs_dir);
+}
+
+void dsa_debugfs_create_tree(struct dsa_switch_tree *dst)
+{
+	struct dsa_switch *ds;
+	int i, err;
+
+	WARN_ON(!dst->applied);
+
+	for (i = 0; i < DSA_MAX_SWITCHES; i++) {
+		ds = dst->ds[i];
+		if (!ds)
+			continue;
+
+		err = dsa_debugfs_create_switch(ds);
+		if (err) {
+			pr_warn("DSA: failed to create debugfs interface for switch %d (%d)\n",
+				ds->index, err);
+			dsa_debugfs_destroy_tree(dst);
+			break;
+		}
+	}
+}
+
+void dsa_debugfs_destroy_tree(struct dsa_switch_tree *dst)
+{
+	struct dsa_switch *ds;
+	int i;
+
+	for (i = 0; i < DSA_MAX_SWITCHES; i++) {
+		ds = dst->ds[i];
+		if (!ds)
+			continue;
+
+		dsa_debugfs_destroy_switch(ds);
+	}
+}
+
+void dsa_debugfs_create_module(void)
+{
+	dsa_debugfs_dir = debugfs_create_dir("dsa", NULL);
+	if (IS_ERR(dsa_debugfs_dir)) {
+		pr_warn("DSA: failed to create debugfs interface\n");
+		dsa_debugfs_dir = NULL;
+	}
+
+	if (dsa_debugfs_dir)
+		pr_info("DSA: debugfs interface created\n");
+}
+
+void dsa_debugfs_destroy_module(void)
+{
+	/* handles NULL */
+	debugfs_remove_recursive(dsa_debugfs_dir);
+}
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 99e38af85fc5..62e49ff6d737 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -308,12 +308,15 @@ static int __init dsa_init_module(void)
 
 	dev_add_pack(&dsa_pack_type);
 
+	dsa_debugfs_create_module();
+
 	return 0;
 }
 module_init(dsa_init_module);
 
 static void __exit dsa_cleanup_module(void)
 {
+	dsa_debugfs_destroy_module();
 	dsa_slave_unregister_notifier();
 	dev_remove_pack(&dsa_pack_type);
 	dsa_legacy_unregister();
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index cceaa4dd9f53..5912618ad63d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -447,6 +447,8 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
 	dst->cpu_dp->netdev->dsa_ptr = dst;
 	dst->applied = true;
 
+	dsa_debugfs_create_tree(dst);
+
 	return 0;
 }
 
@@ -458,6 +460,8 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	if (!dst->applied)
 		return;
 
+	dsa_debugfs_destroy_tree(dst);
+
 	dst->cpu_dp->netdev->dsa_ptr = NULL;
 
 	/* If we used a tagging format that doesn't have an ethertype
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9c3eeb72462d..84ca3a50a58b 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -93,6 +93,19 @@ struct dsa_slave_priv {
 	struct list_head	mall_tc_list;
 };
 
+/* debugfs.c */
+#ifdef CONFIG_NET_DSA_DEBUGFS
+void dsa_debugfs_create_module(void);
+void dsa_debugfs_destroy_module(void);
+void dsa_debugfs_create_tree(struct dsa_switch_tree *dst);
+void dsa_debugfs_destroy_tree(struct dsa_switch_tree *dst);
+#else
+static inline void dsa_debugfs_create_module(void) { }
+static inline void dsa_debugfs_destroy_module(void) { }
+static inline void dsa_debugfs_create_tree(struct dsa_switch_tree *dst) { }
+static inline void dsa_debugfs_destroy_tree(struct dsa_switch_tree *dst) { }
+#endif
+
 /* dsa.c */
 int dsa_cpu_dsa_setup(struct dsa_port *port);
 void dsa_cpu_dsa_destroy(struct dsa_port *dport);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index a6a0849483d1..007034ebd218 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -607,6 +607,8 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 	dev->dsa_ptr = dst;
 	dst->applied = true;
 
+	dsa_debugfs_create_tree(dst);
+
 	return 0;
 }
 
@@ -672,6 +674,8 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 {
 	int i;
 
+	dsa_debugfs_destroy_tree(dst);
+
 	dst->cpu_dp->netdev->dsa_ptr = NULL;
 
 	/* If we used a tagging format that doesn't have an ethertype
-- 
2.14.0

^ permalink raw reply related

* [PATCH net-next 03/11] net: dsa: debugfs: add tree
From: Vivien Didelot @ 2017-08-14 22:22 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Volodymyr Bendiuga, Nikita Yushchenko,
	Maxime Hadjinlian, Chris Healy, Maxim Uvarov, Stefan Eichenberger,
	Jason Cobham, Juergen Borleis, Tobias Waldekranz, Vivien Didelot
In-Reply-To: <20170814222242.10643-1-vivien.didelot@savoirfairelinux.com>

This commit adds the boiler plate to create a DSA related debug
filesystem entry as well as a "tree" file, containing the tree index.

    # cat switch1/tree
    0

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/debugfs.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/net/dsa/debugfs.c b/net/dsa/debugfs.c
index 68caf5a2c0c3..5607efdb924d 100644
--- a/net/dsa/debugfs.c
+++ b/net/dsa/debugfs.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/debugfs.h>
+#include <linux/seq_file.h>
 
 #include "dsa_priv.h"
 
@@ -19,6 +20,107 @@
 /* DSA module debugfs directory */
 static struct dentry *dsa_debugfs_dir;
 
+struct dsa_debugfs_ops {
+	int (*read)(struct dsa_switch *ds, int id, struct seq_file *seq);
+	int (*write)(struct dsa_switch *ds, int id, char *buf);
+};
+
+struct dsa_debugfs_priv {
+	const struct dsa_debugfs_ops *ops;
+	struct dsa_switch *ds;
+	int id;
+};
+
+static int dsa_debugfs_show(struct seq_file *seq, void *p)
+{
+	struct dsa_debugfs_priv *priv = seq->private;
+	struct dsa_switch *ds = priv->ds;
+
+	/* Somehow file mode is bypassed... Double check here */
+	if (!priv->ops->read)
+		return -EOPNOTSUPP;
+
+	return priv->ops->read(ds, priv->id, seq);
+}
+
+static ssize_t dsa_debugfs_write(struct file *file, const char __user *user_buf,
+				 size_t count, loff_t *ppos)
+{
+	struct seq_file *seq = file->private_data;
+	struct dsa_debugfs_priv *priv = seq->private;
+	struct dsa_switch *ds = priv->ds;
+	char buf[count + 1];
+	int err;
+
+	/* Somehow file mode is bypassed... Double check here */
+	if (!priv->ops->write)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(buf, user_buf, count))
+		return -EFAULT;
+
+	buf[count] = '\0';
+
+	err = priv->ops->write(ds, priv->id, buf);
+
+	return err ? err : count;
+}
+
+static int dsa_debugfs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, dsa_debugfs_show, inode->i_private);
+}
+
+static const struct file_operations dsa_debugfs_fops = {
+	.open = dsa_debugfs_open,
+	.read = seq_read,
+	.write = dsa_debugfs_write,
+	.llseek = no_llseek,
+	.release = single_release,
+	.owner = THIS_MODULE,
+};
+
+static int dsa_debugfs_create_file(struct dsa_switch *ds, struct dentry *dir,
+				   char *name, int id,
+				   const struct dsa_debugfs_ops *ops)
+{
+	struct dsa_debugfs_priv *priv;
+	struct dentry *entry;
+	umode_t mode;
+
+	priv = devm_kzalloc(ds->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->ops = ops;
+	priv->ds = ds;
+	priv->id = id;
+
+	mode = 0;
+	if (ops->read)
+		mode |= 0444;
+	if (ops->write)
+		mode |= 0200;
+
+	entry = debugfs_create_file(name, mode, dir, priv, &dsa_debugfs_fops);
+	if (IS_ERR_OR_NULL(entry))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int dsa_debugfs_tree_read(struct dsa_switch *ds, int id,
+				 struct seq_file *seq)
+{
+	seq_printf(seq, "%d\n", ds->dst->tree);
+
+	return 0;
+}
+
+static const struct dsa_debugfs_ops dsa_debugfs_tree_ops = {
+	.read = dsa_debugfs_tree_read,
+};
+
 static int dsa_debugfs_create_port(struct dsa_switch *ds, int port)
 {
 	struct dentry *dir;
@@ -36,6 +138,7 @@ static int dsa_debugfs_create_port(struct dsa_switch *ds, int port)
 static int dsa_debugfs_create_switch(struct dsa_switch *ds)
 {
 	char name[32];
+	int err;
 	int i;
 
 	/* skip if there is no debugfs support */
@@ -48,6 +151,11 @@ static int dsa_debugfs_create_switch(struct dsa_switch *ds)
 	if (IS_ERR_OR_NULL(ds->debugfs_dir))
 		return -EFAULT;
 
+	err = dsa_debugfs_create_file(ds, ds->debugfs_dir, "tree", -1,
+				      &dsa_debugfs_tree_ops);
+	if (err)
+		return err;
+
 	for (i = 0; i < ds->num_ports; i++) {
 		if (!ds->ports[i].dn)
 			continue;
-- 
2.14.0

^ permalink raw reply related

* [PATCH net-next 05/11] net: dsa: debugfs: add port stats
From: Vivien Didelot @ 2017-08-14 22:22 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Volodymyr Bendiuga, Nikita Yushchenko,
	Maxime Hadjinlian, Chris Healy, Maxim Uvarov, Stefan Eichenberger,
	Jason Cobham, Juergen Borleis, Tobias Waldekranz, Vivien Didelot
In-Reply-To: <20170814222242.10643-1-vivien.didelot@savoirfairelinux.com>

Add a debug filesystem "stats" entry to query a port's hardware
statistics through the DSA switch .get_sset_count, .get_strings and
.get_ethtool_stats operations.

This allows one to get statistics about DSA links interconnecting
switches, which is very convenient because this kind of port is not
exposed to userspace.

Here are the stats of a zii-rev-b DSA and CPU ports:

    # pr -mt switch0/port{5,6}/stats
    in_good_octets      : 0             in_good_octets      : 13824
    in_bad_octets       : 0             in_bad_octets       : 0
    in_unicast          : 0             in_unicast          : 0
    in_broadcasts       : 0             in_broadcasts       : 216
    in_multicasts       : 0             in_multicasts       : 0
    in_pause            : 0             in_pause            : 0
    in_undersize        : 0             in_undersize        : 0
    in_fragments        : 0             in_fragments        : 0
    in_oversize         : 0             in_oversize         : 0
    in_jabber           : 0             in_jabber           : 0
    in_rx_error         : 0             in_rx_error         : 0
    in_fcs_error        : 0             in_fcs_error        : 0
    out_octets          : 9216          out_octets          : 0
    out_unicast         : 0             out_unicast         : 0
    out_broadcasts      : 144           out_broadcasts      : 0
    out_multicasts      : 0             out_multicasts      : 0
    out_pause           : 0             out_pause           : 0
    excessive           : 0             excessive           : 0
    collisions          : 0             collisions          : 0
    deferred            : 0             deferred            : 0
    single              : 0             single              : 0
    multiple            : 0             multiple            : 0
    out_fcs_error       : 0             out_fcs_error       : 0
    late                : 0             late                : 0
    hist_64bytes        : 0             hist_64bytes        : 0
    hist_65_127bytes    : 0             hist_65_127bytes    : 0
    hist_128_255bytes   : 0             hist_128_255bytes   : 0
    hist_256_511bytes   : 0             hist_256_511bytes   : 0
    hist_512_1023bytes  : 0             hist_512_1023bytes  : 0
    hist_1024_max_bytes : 0             hist_1024_max_bytes : 0
    sw_in_discards      : 0             sw_in_discards      : 0
    sw_in_filtered      : 0             sw_in_filtered      : 0
    sw_out_filtered     : 0             sw_out_filtered     : 216

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/debugfs.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/net/dsa/debugfs.c b/net/dsa/debugfs.c
index 30a732e86161..5f91b4423404 100644
--- a/net/dsa/debugfs.c
+++ b/net/dsa/debugfs.c
@@ -109,6 +109,43 @@ static int dsa_debugfs_create_file(struct dsa_switch *ds, struct dentry *dir,
 	return 0;
 }
 
+static void dsa_debugfs_stats_read_count(struct dsa_switch *ds, int id,
+					 struct seq_file *seq, int count)
+{
+	u8 strings[count * ETH_GSTRING_LEN];
+	u64 stats[count];
+	int i;
+
+	ds->ops->get_strings(ds, id, strings);
+	ds->ops->get_ethtool_stats(ds, id, stats);
+
+	for (i = 0; i < count; i++)
+		seq_printf(seq, "%-20s: %lld\n", strings + i * ETH_GSTRING_LEN,
+			   stats[i]);
+}
+
+static int dsa_debugfs_stats_read(struct dsa_switch *ds, int id,
+				  struct seq_file *seq)
+{
+	int count;
+
+	if (!ds->ops->get_sset_count || !ds->ops->get_strings ||
+	    !ds->ops->get_ethtool_stats)
+		return -EOPNOTSUPP;
+
+	count = ds->ops->get_sset_count(ds);
+	if (count < 0)
+		return count;
+
+	dsa_debugfs_stats_read_count(ds, id, seq, count);
+
+	return 0;
+}
+
+static const struct dsa_debugfs_ops dsa_debugfs_stats_ops = {
+	.read = dsa_debugfs_stats_read,
+};
+
 static int dsa_debugfs_tag_protocol_read(struct dsa_switch *ds, int id,
 					 struct seq_file *seq)
 {
@@ -174,6 +211,7 @@ static int dsa_debugfs_create_port(struct dsa_switch *ds, int port)
 {
 	struct dentry *dir;
 	char name[32];
+	int err;
 
 	snprintf(name, sizeof(name), DSA_PORT_FMT, port);
 
@@ -181,6 +219,11 @@ static int dsa_debugfs_create_port(struct dsa_switch *ds, int port)
 	if (IS_ERR_OR_NULL(dir))
 		return -EFAULT;
 
+	err = dsa_debugfs_create_file(ds, dir, "stats", port,
+				      &dsa_debugfs_stats_ops);
+	if (err)
+		return err;
+
 	return 0;
 }
 
-- 
2.14.0

^ permalink raw reply related

* [PATCH net-next 06/11] net: dsa: debugfs: add port registers
From: Vivien Didelot @ 2017-08-14 22:22 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Volodymyr Bendiuga, Nikita Yushchenko,
	Maxime Hadjinlian, Chris Healy, Maxim Uvarov, Stefan Eichenberger,
	Jason Cobham, Juergen Borleis, Tobias Waldekranz, Vivien Didelot
In-Reply-To: <20170814222242.10643-1-vivien.didelot@savoirfairelinux.com>

Add a debug filesystem "regs" entry to query a port's hardware registers
through the .get_regs_len and .get_regs_len switch operations.

This is very convenient because it allows one to dump the registers of
DSA links, which are not exposed to userspace.

Here are the registers of a zii-rev-b CPU and DSA ports:

    # pr -mt switch0/port{5,6}/regs
     0: 4e07			     0: 4d04
     1: 403e			     1: 003d
     2: 0000			     2: 0000
     3: 3521			     3: 3521
     4: 0533			     4: 373f
     5: 8000			     5: 0000
     6: 005f			     6: 003f
     7: 002a			     7: 002a
     8: 2080			     8: 2080
     9: 0001			     9: 0001
    10: 0000			    10: 0000
    11: 0020			    11: 0000
    12: 0000			    12: 0000
    13: 0000			    13: 0000
    14: 0000			    14: 0000
    15: 9100			    15: dada
    16: 0000			    16: 0000
    17: 0000			    17: 0000
    18: 0000			    18: 0000
    19: 0000			    19: 00d8
    20: 0000			    20: 0000
    21: 0000			    21: 0000
    22: 0022			    22: 0000
    23: 0000			    23: 0000
    24: 3210			    24: 3210
    25: 7654			    25: 7654
    26: 0000			    26: 0000
    27: 8000			    27: 8000
    28: 0000			    28: 0000
    29: 0000			    29: 0000
    30: 0000			    30: 0000
    31: 0000			    31: 0000

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/debugfs.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/net/dsa/debugfs.c b/net/dsa/debugfs.c
index 5f91b4423404..012fcf466cc1 100644
--- a/net/dsa/debugfs.c
+++ b/net/dsa/debugfs.c
@@ -109,6 +109,40 @@ static int dsa_debugfs_create_file(struct dsa_switch *ds, struct dentry *dir,
 	return 0;
 }
 
+static void dsa_debugfs_regs_read_count(struct dsa_switch *ds, int id,
+					struct seq_file *seq, int count)
+{
+	u16 data[count * ETH_GSTRING_LEN];
+	struct ethtool_regs regs;
+	int i;
+
+	ds->ops->get_regs(ds, id, &regs, data);
+
+	for (i = 0; i < count / 2; i++)
+		seq_printf(seq, "%2d: %04x\n", i, data[i]);
+}
+
+static int dsa_debugfs_regs_read(struct dsa_switch *ds, int id,
+				 struct seq_file *seq)
+{
+	int count;
+
+	if (!ds->ops->get_regs_len || !ds->ops->get_regs)
+		return -EOPNOTSUPP;
+
+	count = ds->ops->get_regs_len(ds, id);
+	if (count < 0)
+		return count;
+
+	dsa_debugfs_regs_read_count(ds, id, seq, count);
+
+	return 0;
+}
+
+static const struct dsa_debugfs_ops dsa_debugfs_regs_ops = {
+	.read = dsa_debugfs_regs_read,
+};
+
 static void dsa_debugfs_stats_read_count(struct dsa_switch *ds, int id,
 					 struct seq_file *seq, int count)
 {
@@ -219,6 +253,11 @@ static int dsa_debugfs_create_port(struct dsa_switch *ds, int port)
 	if (IS_ERR_OR_NULL(dir))
 		return -EFAULT;
 
+	err = dsa_debugfs_create_file(ds, dir, "regs", port,
+				      &dsa_debugfs_regs_ops);
+	if (err)
+		return err;
+
 	err = dsa_debugfs_create_file(ds, dir, "stats", port,
 				      &dsa_debugfs_stats_ops);
 	if (err)
-- 
2.14.0

^ 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