Netdev List
 help / color / mirror / Atom feed
* RE: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: Parshuram Raju Thombare @ 2019-02-23  6:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
	netdev@vger.kernel.org, f.fainelli@gmail.com,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222213953.GN5894@lunn.ch>

>>  	/* mask with MAC supported features */
>> -	if (macb_is_gem(bp) && bp->caps &
>MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>> -		phy_set_max_speed(phydev, SPEED_1000);
>> -	else
>> -		phy_set_max_speed(phydev, SPEED_100);
>> +	if (macb_is_gem(bp)) {
>> +		linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>> +		if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> +
>	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> +					 phydev->supported);
>> +	} else {
>> +		linkmode_copy(phydev->supported, PHY_BASIC_FEATURES);
>> +	}
>> +
>> +	linkmode_copy(phydev->advertising, phydev->supported);
>
>This is not correct. Just because the MAC can do 2.5G does not mean the PHY
>can. So you should not be adding links modes. Also, somebody might be using a
>PHY that can do 2.5G with a MAC which can only do 1G.
>
>The correct thing to do is call phy_set_max_speed() with the maximum speed the
>MAC can do.
Hi Andrew,

Ok, I think this should have been logical AND. I will modify to use phy_set_max_speed() 
instead of directly copying linkmodes.

Regards,
Parshuram Thombare

^ permalink raw reply

* RE: [PATCH 3/3] net: ethernet: add support for high speed mac and usxgmii pcs
From: Parshuram Raju Thombare @ 2019-02-23  6:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
	netdev@vger.kernel.org, f.fainelli@gmail.com,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222214404.GP5894@lunn.ch>

>>  	if (macb_is_gem(bp)) {
>> -		linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>> -		if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> -
>	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> -					 phydev->supported);
>> +		if (bp->caps & MACB_CAPS_HIGH_SPEED) {
>> +			linkmode_copy(phydev->supported,
>PHY_10GBIT_FEATURES);
>> +		} else {
>> +			u32 bitmask =
>ETHTOOL_LINK_MODE_2500baseT_Full_BIT;
>> +
>> +			linkmode_copy(phydev->supported,
>PHY_GBIT_FEATURES);
>> +			if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> +				linkmode_set_bit(bitmask, phydev->supported);
>> +		}
>
>Same issue again. Somebody could be using a 10G MAC with a 2.5G PHY.
>
>     Andrew
Hi Andrew,

Ok, I think this should have been logical AND. I will modify to use phy_set_max_speed() 
instead of directly copying linkmodes.

Regards,
Parshuram Thombare

^ permalink raw reply

* RE: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write functions.
From: Parshuram Raju Thombare @ 2019-02-23  6:27 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
	netdev@vger.kernel.org, f.fainelli@gmail.com,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222214140.GO5894@lunn.ch>



Regards,
Parshuram Thombare

>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Saturday, February 23, 2019 3:12 AM
>To: Parshuram Raju Thombare <pthombar@cadence.com>
>Cc: nicolas.ferre@microchip.com; davem@davemloft.net;
>netdev@vger.kernel.org; f.fainelli@gmail.com; hkallweit1@gmail.com; linux-
>kernel@vger.kernel.org; Rafal Ciepiela <rafalc@cadence.com>; Piotr Sroka
><piotrs@cadence.com>; Jan Kotas <jank@cadence.com>
>Subject: Re: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write
>functions.
>
>EXTERNAL MAIL
>
>
>On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
>> This patch modify MDIO read/write functions to support communication
>> with C45 PHY in Cadence ethernet controller driver.
>
>Hi Parshuram
>
>Are all versions of the MDIO controller capable of doing C45?
>
>    Andrew
Now driver support c22 and c45 PHY. 
Are you suggesting to add check for C45 PHY using is_c45 in phydev ?

Regards,
Parshuram Thombare

^ permalink raw reply

* [PATCH v2 0/6] RCU fixes for rcu_assign_pointer() usage
From: Joel Fernandes (Google) @ 2019-02-23  6:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
	Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
	Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
	John Fastabend, Josh Triplett, keescook, kernel-hardening,
	kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
	Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
	Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
	xdp-newbies, Yonghong Song

These patches fix various sparse errors found as a result of the recent check
to add rcu_check_sparse() to rcu_assign_pointer().  The errors in some cases
seem to either missing API usage, or missing annotations. The annotations added
in the series can also help avoid future incorrect usages and bugs so it is a
good idea to do in any case.

RFC v1 -> Patch v2:
Made changes to various scheduler patches (Peter Zijlstra)

Joel Fernandes (Google) (6):
net: rtnetlink: Fix incorrect RCU API usage
ixgbe: Fix incorrect RCU API usage
sched/cpufreq: Annotate cpufreq_update_util_data pointer with __rcu
sched_domain: Annotate RCU pointers properly
rcuwait: Annotate task_struct with __rcu
sched: Annotate perf_domain pointer with __rcu

drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 ++++++++++-----
include/linux/rcuwait.h                       |  2 +-
include/linux/sched/topology.h                |  4 ++--
kernel/sched/cpufreq.c                        |  2 +-
kernel/sched/sched.h                          | 18 +++++++++---------
kernel/sched/topology.c                       | 10 +++++-----
net/core/rtnetlink.c                          |  4 ++--
8 files changed, 32 insertions(+), 27 deletions(-)

--
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply

* [PATCH v2 1/6] net: rtnetlink: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-23  6:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
	Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
	Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
	John Fastabend, Josh Triplett, keescook, kernel-hardening,
	kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
	Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
	Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
	xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>

rtnl_register_internal() and rtnl_unregister_all tries to directly
dereference an RCU protected pointed outside RCU read side section.
While this is Ok to do since a lock is held, let us use the correct
API to avoid programmer bugs in the future.

This also fixes sparse warnings arising from not using RCU API.

net/core/rtnetlink.c:332:13: warning: incorrect type in assignment
(different address spaces) net/core/rtnetlink.c:332:13:    expected
struct rtnl_link **tab net/core/rtnetlink.c:332:13:    got struct
rtnl_link *[noderef] <asn:4>*<noident>

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 net/core/rtnetlink.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5ea1bed08ede..98be4b4818a9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -188,7 +188,7 @@ static int rtnl_register_internal(struct module *owner,
 	msgindex = rtm_msgindex(msgtype);
 
 	rtnl_lock();
-	tab = rtnl_msg_handlers[protocol];
+	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
 	if (tab == NULL) {
 		tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
 		if (!tab)
@@ -329,7 +329,7 @@ void rtnl_unregister_all(int protocol)
 	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
 
 	rtnl_lock();
-	tab = rtnl_msg_handlers[protocol];
+	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
 	if (!tab) {
 		rtnl_unlock();
 		return;
-- 
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply related

* [PATCH v2 2/6] ixgbe: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-23  6:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
	Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
	Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
	John Fastabend, Josh Triplett, keescook, kernel-hardening,
	kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
	Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
	Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
	xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>

Recently, I added an RCU annotation check in rcu_assign_pointer. This
caused a sparse error to be reported by the ixgbe driver.

Further looking, it seems the adapter->xdp_prog pointer is not annotated
with __rcu. Annonating it fixed the error, but caused a bunch of other
warnings.

This patch tries to fix all warnings by using RCU API properly. This
makes sense to do because not using RCU properly can result in various
hard to find bugs. This is a best effort fix and is only build tested.
The sparse errors and warnings go away with the change. I request
maintainers / developers in this area to review / test it properly.

The sparse error fixed is:
ixgbe_main.c:10256:25: error: incompatible types in comparison expression

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  4 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 ++++++++++-----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 08d85e336bd4..3b14daf27516 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -311,7 +311,7 @@ struct ixgbe_ring {
 	struct ixgbe_ring *next;	/* pointer to next ring in q_vector */
 	struct ixgbe_q_vector *q_vector; /* backpointer to host q_vector */
 	struct net_device *netdev;	/* netdev ring belongs to */
-	struct bpf_prog *xdp_prog;
+	struct bpf_prog __rcu *xdp_prog;
 	struct device *dev;		/* device for DMA mapping */
 	void *desc;			/* descriptor ring memory */
 	union {
@@ -560,7 +560,7 @@ struct ixgbe_adapter {
 	unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
 	/* OS defined structs */
 	struct net_device *netdev;
-	struct bpf_prog *xdp_prog;
+	struct bpf_prog __rcu *xdp_prog;
 	struct pci_dev *pdev;
 	struct mii_bus *mii_bus;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index daff8183534b..408a312aa6ba 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2199,7 +2199,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
 	u32 act;
 
 	rcu_read_lock();
-	xdp_prog = READ_ONCE(rx_ring->xdp_prog);
+	xdp_prog = rcu_dereference(rx_ring->xdp_prog);
 
 	if (!xdp_prog)
 		goto xdp_out;
@@ -6547,7 +6547,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
 			     rx_ring->queue_index) < 0)
 		goto err;
 
-	rx_ring->xdp_prog = adapter->xdp_prog;
+	rcu_assign_pointer(rx_ring->xdp_prog, adapter->xdp_prog);
 
 	return 0;
 err:
@@ -10246,7 +10246,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 	if (nr_cpu_ids > MAX_XDP_QUEUES)
 		return -ENOMEM;
 
-	old_prog = xchg(&adapter->xdp_prog, prog);
+	old_prog = rcu_access_pointer(adapter->xdp_prog);
+	rcu_assign_pointer(adapter->xdp_prog, prog);
 
 	/* If transitioning XDP modes reconfigure rings */
 	if (!!prog != !!old_prog) {
@@ -10271,13 +10272,17 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	struct bpf_prog *prog;
 
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return ixgbe_xdp_setup(dev, xdp->prog);
 	case XDP_QUERY_PROG:
-		xdp->prog_id = adapter->xdp_prog ?
-			adapter->xdp_prog->aux->id : 0;
+		rcu_read_lock();
+		prog = rcu_dereference(adapter->xdp_prog);
+		xdp->prog_id = prog ? prog->aux->id : 0;
+		rcu_read_unlock();
+
 		return 0;
 	case XDP_QUERY_XSK_UMEM:
 		return ixgbe_xsk_umem_query(adapter, &xdp->xsk.umem,
-- 
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply related

* [PATCH v2 6/6] sched: Annotate perf_domain pointer with __rcu
From: Joel Fernandes (Google) @ 2019-02-23  6:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
	Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
	Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
	John Fastabend, Josh Triplett, keescook, kernel-hardening,
	kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
	Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
	Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
	xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>

This fixes the following sparse errors in sched/fair.c:

fair.c:6506:14: error: incompatible types in comparison expression
fair.c:8642:21: error: incompatible types in comparison expression

Using __rcu will also help sparse catch any future bugs.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/sched/sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index ca6a79f57e7a..c8e6514433a9 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -780,7 +780,7 @@ struct root_domain {
 	 * NULL-terminated list of performance domains intersecting with the
 	 * CPUs of the rd. Protected by RCU.
 	 */
-	struct perf_domain	*pd;
+	struct perf_domain __rcu *pd;
 };
 
 extern struct root_domain def_root_domain;
-- 
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply related

* [PATCH v2 5/6] rcuwait: Annotate task_struct with __rcu
From: Joel Fernandes (Google) @ 2019-02-23  6:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
	Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
	Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
	John Fastabend, Josh Triplett, keescook, kernel-hardening,
	kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
	Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
	Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
	xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>

This suppresses sparse error generated due to the recently added
rcu_assign_pointer sparse check.

percpu-rwsem.c:162:9: sparse: error: incompatible types in comparison expression
exit.c:316:16: sparse: error: incompatible types in comparison expression

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 include/linux/rcuwait.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/rcuwait.h b/include/linux/rcuwait.h
index 90bfa3279a01..563290fc194f 100644
--- a/include/linux/rcuwait.h
+++ b/include/linux/rcuwait.h
@@ -18,7 +18,7 @@
  * awoken.
  */
 struct rcuwait {
-	struct task_struct *task;
+	struct task_struct __rcu *task;
 };
 
 #define __RCUWAIT_INITIALIZER(name)		\
-- 
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply related

* [PATCH v2 4/6] sched_domain: Annotate RCU pointers properly
From: Joel Fernandes (Google) @ 2019-02-23  6:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
	Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
	Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
	John Fastabend, Josh Triplett, keescook, kernel-hardening,
	kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
	Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
	Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
	xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>

The scheduler uses RCU API in various places to access sched_domain
pointers. These cause sparse errors as below.

Many new errors show up because of an annotation check I added to
rcu_assign_pointer(). Let us annotate the pointers correctly which also
will help sparse catch any potential future bugs.

This fixes the following sparse errors:

rt.c:1681:9: error: incompatible types in comparison expression
deadline.c:1904:9: error: incompatible types in comparison expression
core.c:519:9: error: incompatible types in comparison expression
core.c:1634:17: error: incompatible types in comparison expression
fair.c:6193:14: error: incompatible types in comparison expression
fair.c:9883:22: error: incompatible types in comparison expression
fair.c:9897:9: error: incompatible types in comparison expression
sched.h:1287:9: error: incompatible types in comparison expression
topology.c:612:9: error: incompatible types in comparison expression
topology.c:615:9: error: incompatible types in comparison expression
sched.h:1300:9: error: incompatible types in comparison expression
topology.c:618:9: error: incompatible types in comparison expression
sched.h:1287:9: error: incompatible types in comparison expression
topology.c:621:9: error: incompatible types in comparison expression
sched.h:1300:9: error: incompatible types in comparison expression
topology.c:624:9: error: incompatible types in comparison expression
topology.c:671:9: error: incompatible types in comparison expression
stats.c:45:17: error: incompatible types in comparison expression
fair.c:5998:15: error: incompatible types in comparison expression
fair.c:5989:15: error: incompatible types in comparison expression
fair.c:5998:15: error: incompatible types in comparison expression
fair.c:5989:15: error: incompatible types in comparison expression
fair.c:6120:19: error: incompatible types in comparison expression
fair.c:6506:14: error: incompatible types in comparison expression
fair.c:6515:14: error: incompatible types in comparison expression
fair.c:6623:9: error: incompatible types in comparison expression
fair.c:5970:17: error: incompatible types in comparison expression
fair.c:8642:21: error: incompatible types in comparison expression
fair.c:9253:9: error: incompatible types in comparison expression
fair.c:9331:9: error: incompatible types in comparison expression
fair.c:9519:15: error: incompatible types in comparison expression
fair.c:9533:14: error: incompatible types in comparison expression
fair.c:9542:14: error: incompatible types in comparison expression
fair.c:9567:14: error: incompatible types in comparison expression
fair.c:9597:14: error: incompatible types in comparison expression
fair.c:9421:16: error: incompatible types in comparison expression
fair.c:9421:16: error: incompatible types in comparison expression

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 include/linux/sched/topology.h |  4 ++--
 kernel/sched/sched.h           | 14 +++++++-------
 kernel/sched/topology.c        | 10 +++++-----
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index c31d3a47a47c..4819c9e01e42 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -76,8 +76,8 @@ struct sched_domain_shared {
 
 struct sched_domain {
 	/* These fields must be setup */
-	struct sched_domain *parent;	/* top domain must be null terminated */
-	struct sched_domain *child;	/* bottom domain must be null terminated */
+	struct sched_domain __rcu *parent;	/* top domain must be null terminated */
+	struct sched_domain __rcu *child;	/* bottom domain must be null terminated */
 	struct sched_group *groups;	/* the balancing groups of the domain */
 	unsigned long min_interval;	/* Minimum balance interval ms */
 	unsigned long max_interval;	/* Maximum balance interval ms */
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 2ab545d40381..ca6a79f57e7a 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -866,8 +866,8 @@ struct rq {
 	atomic_t		nr_iowait;
 
 #ifdef CONFIG_SMP
-	struct root_domain	*rd;
-	struct sched_domain	*sd;
+	struct root_domain		*rd;
+	struct sched_domain __rcu	*sd;
 
 	unsigned long		cpu_capacity;
 	unsigned long		cpu_capacity_orig;
@@ -1305,13 +1305,13 @@ static inline struct sched_domain *lowest_flag_domain(int cpu, int flag)
 	return sd;
 }
 
-DECLARE_PER_CPU(struct sched_domain *, sd_llc);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_llc);
 DECLARE_PER_CPU(int, sd_llc_size);
 DECLARE_PER_CPU(int, sd_llc_id);
-DECLARE_PER_CPU(struct sched_domain_shared *, sd_llc_shared);
-DECLARE_PER_CPU(struct sched_domain *, sd_numa);
-DECLARE_PER_CPU(struct sched_domain *, sd_asym_packing);
-DECLARE_PER_CPU(struct sched_domain *, sd_asym_cpucapacity);
+DECLARE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_numa);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
 extern struct static_key_false sched_asym_cpucapacity;
 
 struct sched_group_capacity {
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 3f35ba1d8fde..0844ee757dad 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -586,13 +586,13 @@ static void destroy_sched_domains(struct sched_domain *sd)
  * the cpumask of the domain), this allows us to quickly tell if
  * two CPUs are in the same cache domain, see cpus_share_cache().
  */
-DEFINE_PER_CPU(struct sched_domain *, sd_llc);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
 DEFINE_PER_CPU(int, sd_llc_size);
 DEFINE_PER_CPU(int, sd_llc_id);
-DEFINE_PER_CPU(struct sched_domain_shared *, sd_llc_shared);
-DEFINE_PER_CPU(struct sched_domain *, sd_numa);
-DEFINE_PER_CPU(struct sched_domain *, sd_asym_packing);
-DEFINE_PER_CPU(struct sched_domain *, sd_asym_cpucapacity);
+DEFINE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_numa);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
 DEFINE_STATIC_KEY_FALSE(sched_asym_cpucapacity);
 
 static void update_top_cache_domain(int cpu)
-- 
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply related

* [PATCH v2 3/6] sched/cpufreq: Annotate cpufreq_update_util_data pointer with __rcu
From: Joel Fernandes (Google) @ 2019-02-23  6:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
	Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
	Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
	John Fastabend, Josh Triplett, keescook, kernel-hardening,
	kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
	Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
	Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
	xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>

Recently I added an RCU annotation check to rcu_assign_pointer(). All
pointers assigned to RCU protected data are to be annotated with __rcu
inorder to be able to use rcu_assign_pointer() similar to checks in
other RCU APIs.

This resulted in a sparse error: kernel//sched/cpufreq.c:41:9: sparse:
error: incompatible types in comparison expression (different address
spaces)

Fix this by annotating cpufreq_update_util_data pointer with __rcu. This
will also help sparse catch any future RCU misuage bugs.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/sched/cpufreq.c | 2 +-
 kernel/sched/sched.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c
index 22bd8980f32f..e316ee7bb2e5 100644
--- a/kernel/sched/cpufreq.c
+++ b/kernel/sched/cpufreq.c
@@ -7,7 +7,7 @@
  */
 #include "sched.h"
 
-DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
 
 /**
  * cpufreq_add_update_util_hook - Populate the CPU's update_util_data pointer.
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index d04530bf251f..2ab545d40381 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2166,7 +2166,7 @@ static inline u64 irq_time_read(int cpu)
 #endif /* CONFIG_IRQ_TIME_ACCOUNTING */
 
 #ifdef CONFIG_CPU_FREQ
-DECLARE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+DECLARE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
 
 /**
  * cpufreq_update_util - Take a note about CPU utilization changes.
-- 
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply related

* [PATCH] tools: testing: selftests: Remove duplicate headers
From: Souptick Joarder @ 2019-02-23  7:09 UTC (permalink / raw)
  To: bamv2005, shuah, davem, benh, paulus, mpe, adobriyan,
	mathieu.desnoyers, peterz, paulmck, boqun.feng, john.stultz, tglx,
	sboyd, akpm
  Cc: linux-gpio, linux-kselftest, linux-kernel, netdev, linuxppc-dev,
	linux-fsdevel, sabyasachi.linux

Remove duplicate headers which are included twice.

Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
---
 tools/testing/selftests/gpio/gpio-mockup-chardev.c          | 1 -
 tools/testing/selftests/net/udpgso.c                        | 1 -
 tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c | 1 -
 tools/testing/selftests/proc/proc-self-syscall.c            | 1 -
 tools/testing/selftests/rseq/rseq.h                         | 1 -
 tools/testing/selftests/timers/skew_consistency.c           | 1 -
 tools/testing/selftests/x86/mpx-dig.c                       | 2 --
 7 files changed, 8 deletions(-)

diff --git a/tools/testing/selftests/gpio/gpio-mockup-chardev.c b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
index aaa1e9f..d587c81 100644
--- a/tools/testing/selftests/gpio/gpio-mockup-chardev.c
+++ b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
@@ -12,7 +12,6 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <stdio.h>
 #include <errno.h>
 #include <string.h>
 #include <fcntl.h>
diff --git a/tools/testing/selftests/net/udpgso.c b/tools/testing/selftests/net/udpgso.c
index e279051..b8265ee 100644
--- a/tools/testing/selftests/net/udpgso.c
+++ b/tools/testing/selftests/net/udpgso.c
@@ -17,7 +17,6 @@
 #include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
 #include <sys/ioctl.h>
 #include <sys/socket.h>
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
index 167135b..af1b802 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
@@ -11,7 +11,6 @@
 #include <sys/wait.h>
 #include <unistd.h>
 #include <setjmp.h>
-#include <signal.h>
 
 #include "ebb.h"
 
diff --git a/tools/testing/selftests/proc/proc-self-syscall.c b/tools/testing/selftests/proc/proc-self-syscall.c
index 5ab5f48..3a4fec3 100644
--- a/tools/testing/selftests/proc/proc-self-syscall.c
+++ b/tools/testing/selftests/proc/proc-self-syscall.c
@@ -20,7 +20,6 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <errno.h>
-#include <unistd.h>
 #include <string.h>
 #include <stdio.h>
 
diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
index c72eb70..6c1126e7 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -16,7 +16,6 @@
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <sched.h>
 #include <linux/rseq.h>
 
 /*
diff --git a/tools/testing/selftests/timers/skew_consistency.c b/tools/testing/selftests/timers/skew_consistency.c
index 022b711..8066be9 100644
--- a/tools/testing/selftests/timers/skew_consistency.c
+++ b/tools/testing/selftests/timers/skew_consistency.c
@@ -32,7 +32,6 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
-#include <stdlib.h>
 #include <string.h>
 #include <sys/wait.h>
 #include "../kselftest.h"
diff --git a/tools/testing/selftests/x86/mpx-dig.c b/tools/testing/selftests/x86/mpx-dig.c
index c13607e..880fbf6 100644
--- a/tools/testing/selftests/x86/mpx-dig.c
+++ b/tools/testing/selftests/x86/mpx-dig.c
@@ -8,9 +8,7 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <errno.h>
-#include <sys/types.h>
 #include <sys/stat.h>
-#include <unistd.h>
 #include <sys/mman.h>
 #include <string.h>
 #include <fcntl.h>
-- 
1.9.1


^ permalink raw reply related

* [PATCH net-next v2 1/3] ipv4: icmp: use icmp_sk_exit()
From: Kefeng Wang @ 2019-02-23  7:28 UTC (permalink / raw)
  To: davem, netdev, eric.dumazet; +Cc: Kefeng Wang
In-Reply-To: <20190223072828.159975-1-wangkefeng.wang@huawei.com>

Simply use icmp_sk_exit() when inet_ctl_sock_create() fail in icmp_sk_init().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 net/ipv4/icmp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 065997f414e6..364cfe5e414b 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1245,9 +1245,7 @@ static int __net_init icmp_sk_init(struct net *net)
 	return 0;
 
 fail:
-	for_each_possible_cpu(i)
-		inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv4.icmp_sk, i));
-	free_percpu(net->ipv4.icmp_sk);
+	icmp_sk_exit(net);
 	return err;
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next v2 2/3] ipv6: icmp: use icmpv6_sk_exit()
From: Kefeng Wang @ 2019-02-23  7:28 UTC (permalink / raw)
  To: davem, netdev, eric.dumazet; +Cc: Kefeng Wang
In-Reply-To: <20190223072828.159975-1-wangkefeng.wang@huawei.com>

Simply use icmpv6_sk_exit() when inet_ctl_sock_create() fail
in icmpv6_sk_init().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 net/ipv6/icmp.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index bbcdfd299692..af520014def5 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -953,10 +953,19 @@ void icmpv6_flow_init(struct sock *sk, struct flowi6 *fl6,
 	security_sk_classify_flow(sk, flowi6_to_flowi(fl6));
 }
 
+static void __net_exit icmpv6_sk_exit(struct net *net)
+{
+	int i;
+
+	for_each_possible_cpu(i)
+		inet_ctl_sock_destroy(net->ipv6.icmp_sk[i]);
+	kfree(net->ipv6.icmp_sk);
+}
+
 static int __net_init icmpv6_sk_init(struct net *net)
 {
 	struct sock *sk;
-	int err, i, j;
+	int err, i;
 
 	net->ipv6.icmp_sk =
 		kcalloc(nr_cpu_ids, sizeof(struct sock *), GFP_KERNEL);
@@ -982,22 +991,10 @@ static int __net_init icmpv6_sk_init(struct net *net)
 	return 0;
 
  fail:
-	for (j = 0; j < i; j++)
-		inet_ctl_sock_destroy(net->ipv6.icmp_sk[j]);
-	kfree(net->ipv6.icmp_sk);
+	icmpv6_sk_exit(net);
 	return err;
 }
 
-static void __net_exit icmpv6_sk_exit(struct net *net)
-{
-	int i;
-
-	for_each_possible_cpu(i) {
-		inet_ctl_sock_destroy(net->ipv6.icmp_sk[i]);
-	}
-	kfree(net->ipv6.icmp_sk);
-}
-
 static struct pernet_operations icmpv6_sk_ops = {
 	.init = icmpv6_sk_init,
 	.exit = icmpv6_sk_exit,
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next v2 3/3] ipv6: icmp: use percpu allocation
From: Kefeng Wang @ 2019-02-23  7:28 UTC (permalink / raw)
  To: davem, netdev, eric.dumazet; +Cc: Kefeng Wang
In-Reply-To: <20190223072828.159975-1-wangkefeng.wang@huawei.com>

Use percpu allocation for the ipv6.icmp_sk.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 include/net/netns/ipv6.h |  2 +-
 net/ipv6/icmp.c          | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index ef1ed529f33c..b028a1dc150d 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -83,7 +83,7 @@ struct netns_ipv6 {
 	struct fib6_table       *fib6_local_tbl;
 	struct fib_rules_ops    *fib6_rules_ops;
 #endif
-	struct sock		**icmp_sk;
+	struct sock * __percpu	*icmp_sk;
 	struct sock             *ndisc_sk;
 	struct sock             *tcp_sk;
 	struct sock             *igmp_sk;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index af520014def5..802faa2fcc0e 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -81,7 +81,7 @@
  */
 static inline struct sock *icmpv6_sk(struct net *net)
 {
-	return net->ipv6.icmp_sk[smp_processor_id()];
+	return *this_cpu_ptr(net->ipv6.icmp_sk);
 }
 
 static int icmpv6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
@@ -958,8 +958,8 @@ static void __net_exit icmpv6_sk_exit(struct net *net)
 	int i;
 
 	for_each_possible_cpu(i)
-		inet_ctl_sock_destroy(net->ipv6.icmp_sk[i]);
-	kfree(net->ipv6.icmp_sk);
+		inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv6.icmp_sk, i));
+	free_percpu(net->ipv6.icmp_sk);
 }
 
 static int __net_init icmpv6_sk_init(struct net *net)
@@ -967,8 +967,7 @@ static int __net_init icmpv6_sk_init(struct net *net)
 	struct sock *sk;
 	int err, i;
 
-	net->ipv6.icmp_sk =
-		kcalloc(nr_cpu_ids, sizeof(struct sock *), GFP_KERNEL);
+	net->ipv6.icmp_sk = alloc_percpu(struct sock *);
 	if (!net->ipv6.icmp_sk)
 		return -ENOMEM;
 
@@ -981,7 +980,7 @@ static int __net_init icmpv6_sk_init(struct net *net)
 			goto fail;
 		}
 
-		net->ipv6.icmp_sk[i] = sk;
+		*per_cpu_ptr(net->ipv6.icmp_sk, i) = sk;
 
 		/* Enough space for 2 64K ICMP packets, including
 		 * sk_buff struct overhead.
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next v2 0/3] ipv4/v6: icmp: small cleanup and update
From: Kefeng Wang @ 2019-02-23  7:28 UTC (permalink / raw)
  To: davem, netdev, eric.dumazet; +Cc: Kefeng Wang

v2:
- Add cover letter and user proper patch subject-prefix suggested-by Eric Dumazet

This patch series contains some small cleanup and update,
1) use icmp/v6_sk_exit when icmp_sk_init fails instead of open-code
2) use new percpu allocation interface for the ipv6.icmp_sk

Kefeng Wang (3):
  ipv4: icmp: use icmp_sk_exit()
  ipv6: icmp: use icmpv6_sk_exit()
  ipv6: icmp: use percpu allocation

 include/net/netns/ipv6.h |  2 +-
 net/ipv4/icmp.c          |  4 +---
 net/ipv6/icmp.c          | 32 ++++++++++++++------------------
 3 files changed, 16 insertions(+), 22 deletions(-)

-- 
2.20.1


^ permalink raw reply

* Re: [PATCH v2 bpf-next 4/9] bpf: add bpf helper bpf_skb_ecn_set_ce
From: Martin Lau @ 2019-02-23  7:30 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Lawrence Brakmo, netdev, Alexei Starovoitov, Eric Dumazet,
	Kernel Team
In-Reply-To: <fdcd5fbb-0f51-18f9-a13a-52e8009d082e@iogearbox.net>

On Sat, Feb 23, 2019 at 02:14:26AM +0100, Daniel Borkmann wrote:
> On 02/23/2019 02:06 AM, brakmo wrote:
> > This patch adds a new bpf helper BPF_FUNC_skb_ecn_set_ce
> > "int bpf_skb_ecn_set_ce(struct sk_buff *skb)". It is added to
> > BPF_PROG_TYPE_CGROUP_SKB typed bpf_prog which currently can
> > be attached to the ingress and egress path. The helper is needed
> > because his type of bpf_prog cannot modify the skb directly.
> > 
> > This helper is used to set the ECN field of ECN capable IP packets to ce
> > (congestion encountered) in the IPv6 or IPv4 header of the skb. It can be
> > used by a bpf_prog to manage egress or ingress network bandwdith limit
> > per cgroupv2 by inducing an ECN response in the TCP sender.
> > This works best when using DCTCP.
> > 
> > Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> > ---
> >  include/uapi/linux/bpf.h | 10 +++++++++-
> >  net/core/filter.c        | 14 ++++++++++++++
> >  2 files changed, 23 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 95b5058fa945..fc646f3eaf9b 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2365,6 +2365,13 @@ union bpf_attr {
> >   *		Make a tcp_sock enter CWR state.
> >   *	Return
> >   *		0 on success, or a negative error in case of failure.
> > + *
> > + * int bpf_skb_ecn_set_ce(struct sk_buf *skb)
> > + *	Description
> > + *		Sets ECN of IP header to ce (congestion encountered) if
> > + *		current value is ect (ECN capable). Works with IPv6 and IPv4.
> > + *	Return
> > + *		1 if set, 0 if not set.
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)		\
> >  	FN(unspec),			\
> > @@ -2464,7 +2471,8 @@ union bpf_attr {
> >  	FN(spin_unlock),		\
> >  	FN(sk_fullsock),		\
> >  	FN(tcp_sock),			\
> > -	FN(tcp_enter_cwr),
> > +	FN(tcp_enter_cwr),		\
> > +	FN(skb_ecn_set_ce),
> >  
> >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> >   * function eBPF program intends to call
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index ca57ef25279c..955369c6ed30 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -5444,6 +5444,18 @@ static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
> >  	.ret_type    = RET_INTEGER,
> >  	.arg1_type    = ARG_PTR_TO_TCP_SOCK,
> >  };
> > +
> > +BPF_CALL_1(bpf_skb_ecn_set_ce, struct sk_buff *, skb)
> > +{
> > +	return INET_ECN_set_ce(skb);
> 
> Hm, but as mentioned last time, don't we have to ensure here that skb
> is writable (aka skb->data private to us before writing into it)?
INET_ECN_set_ce(skb) is also called from a few net/sched/sch_*.c
but I don't see how they ensure if a skb is writable.

May be I have missed something there that can also be borrowed and
reused here?

Thanks,
Martin

> 
> > +}
> > +
> > +static const struct bpf_func_proto bpf_skb_ecn_set_ce_proto = {
> > +	.func		= bpf_skb_ecn_set_ce,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_INTEGER,
> > +	.arg1_type	= ARG_PTR_TO_CTX,
> > +};
> >  #endif /* CONFIG_INET */
> >  
> >  bool bpf_helper_changes_pkt_data(void *func)
> > @@ -5610,6 +5622,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog)
> >  		} else {
> >  			return NULL;
> >  		}
> > +	case BPF_FUNC_skb_ecn_set_ce:
> > +		return &bpf_skb_ecn_set_ce_proto;
> >  #endif
> >  	default:
> >  		return sk_filter_func_proto(func_id, prog);
> > 
> 
> Thanks,
> Daniel

^ permalink raw reply

* Re: [PATCH net-next 2/5] net/mlx5e: Make the log friendly when decapsulation offload not supported
From: Tonghao Zhang @ 2019-02-23  7:58 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: Saeed Mahameed, Linux Netdev List
In-Reply-To: <CAJ3xEMijFiNPZZjtKVvRVbUUFXqaZPP=K=cbQvdKcRNowGODwA@mail.gmail.com>

On Fri, Feb 22, 2019 at 5:07 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
>
> On Fri, Feb 22, 2019 at 9:49 AM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> >
> > On Fri, Feb 22, 2019 at 12:32 AM Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > >
> > > On Thu, Feb 21, 2019 at 3:42 PM <xiangxia.m.yue@gmail.com> wrote:
> > > >
> > > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > > >
> > > > If we try to offload decapsulation actions to VFs hw, we get the log [1].
> > >
> > > but the switching was on the tunnel  type (if (tunnel_type == [...]) -
> > Yes, but we try to offload tc flow to VF device. For example
> > the  p2p1_0 is VF, but not rep
>
> so this should go to the nic and not esw tc offload code path in en_tc.c
nic and esw will call parse_cls_flower() to parse the flower flows,
and more information is shown as below.
> and the nic path (look for parse_tc_nic_actions or a like) doesn't have
> a case for the tunnel key action - you should not have got to this code
> at all, please look deeper to realize what is going on there, maybe p2p1_0
> is a rep? what does ip -d link show gives on it?

#  ip -d link
14: eth0_p1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
UP mode DEFAULT group default qlen 1000
    link/ether 98:03:9b:06:d9:09 brd ff:ff:ff:ff:ff:ff promiscuity 0
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 128 numrxqueues 16
gso_max_size 65536 gso_max_segs 65535 portname p1 switchid
98039b06d909
    vf 0 MAC 00:11:22:33:44:00, spoof checking off, link-state auto,
trust off, query_rss off
    vf 1 MAC 00:11:22:33:44:01, spoof checking off, link-state auto,
trust off, query_rss off
15: eth0_pf1vf0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500
qdisc mq state UP mode DEFAULT group default qlen 1000
    link/ether 8a:26:c0:33:17:2c brd ff:ff:ff:ff:ff:ff promiscuity 1
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 16 numrxqueues 16
gso_max_size 65536 gso_max_segs 65535 portname pf1vf0 switchid
98039b06d909
16: eth0_pf1vf1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq
state UP mode DEFAULT group default qlen 1000
    link/ether 96:34:55:5b:42:80 brd ff:ff:ff:ff:ff:ff promiscuity 0
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 16 numrxqueues 16
gso_max_size 65536 gso_max_segs 65535 portname pf1vf1 switchid
98039b06d909
17: p2p1_0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
UP mode DEFAULT group default qlen 1000
    link/ether 00:11:22:33:44:00 brd ff:ff:ff:ff:ff:ff promiscuity 0
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 64 numrxqueues 8
gso_max_size 65536 gso_max_segs 65535
18: p2p1_1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
UP mode DEFAULT group default qlen 1000
    link/ether 00:11:22:33:44:01 brd ff:ff:ff:ff:ff:ff promiscuity 0
minmtu 68 maxmtu 9978 addrgenmode none numtxqueues 64 numrxqueues 8
gso_max_size 65536 gso_max_segs 65535

#  ethtool -i p2p1_0
driver: mlx5_core
version: 5.0-0
firmware-version: 14.24.1000 (MT_2420110034)
bus-info: 0000:05:01.2
supports-statistics: yes
supports-test: yes
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: yes

# ethtool -i eth0_pf1vf0
driver: mlx5e_rep
version: 5.0.0-rc7+
firmware-version:
bus-info:
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: no


the call trace:
[  586.669600]  dump_stack+0x5a/0x73
[  586.669651]  mlx5e_tc_tun_parse+0x2c6/0x3a0 [mlx5_core]
[  586.669682]  __parse_cls_flower.constprop.48+0x1b2/0xca0 [mlx5_core]
[  586.669746]  parse_cls_flower+0x5d/0x110 [mlx5_core]
[  586.669771]  mlx5e_configure_flower+0x40a/0x760 [mlx5_core]
[  586.669779]  tc_setup_cb_call+0x55/0x80
[  586.669787]  fl_change+0x12a1/0x16b8 [cls_flower]
[  586.669797]  tc_new_tfilter+0x570/0x890
[  586.669808]  rtnetlink_rcv_msg+0xed/0x320
[  586.669817]  netlink_rcv_skb+0xcb/0x100
[  586.669822]  netlink_unicast+0x17f/0x230
[  586.669827]  netlink_sendmsg+0x2d2/0x3d0

^ permalink raw reply

* [PATCHv2 net-next] tls: Return type of non-data records retrieved using MSG_PEEK in recvmsg
From: Vakul Garg @ 2019-02-23  8:39 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: borisp@mellanox.com, aviadye@mellanox.com, davejwatson@fb.com,
	davem@davemloft.net, doronrk@fb.com, Vakul Garg

The patch enables returning 'type' in msghdr for records that are
retrieved with MSG_PEEK in recvmsg. Further it prevents records peeked
from socket from getting clubbed with any other record of different
type when records are subsequently dequeued from strparser.

For each record, we now retain its type in sk_buff's control buffer
cb[]. Inside control buffer, record's full length and offset are already
stored by strparser in 'struct strp_msg'. We store record type after
'struct strp_msg' inside 'struct tls_msg'. For tls1.2, the type is
stored just after record dequeue. For tls1.3, the type is stored after
record has been decrypted.

Inside process_rx_list(), before processing a non-data record, we check
that we must be able to return back the record type to the user
application. If not, the decrypted records in tls context's rx_list is
left there without consuming any data.

Fixes: 692d7b5d1f912 ("tls: Fix recvmsg() to be able to peek across
multiple records)
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---

Changes in v2:
	- Modified 'Fixed:' line to use full commit header line.

 include/net/tls.h | 10 +++++++
 net/tls/tls_sw.c  | 78 +++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 77 insertions(+), 11 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index a8b37226a287..9f4117ae2297 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -129,6 +129,11 @@ struct tls_rec {
 	u8 aead_req_ctx[];
 };
 
+struct tls_msg {
+	struct strp_msg rxm;
+	u8 control;
+};
+
 struct tx_work {
 	struct delayed_work work;
 	struct sock *sk;
@@ -333,6 +338,11 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
 int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx,
 				   int flags, long *timeo);
 
+static inline struct tls_msg *tls_msg(struct sk_buff *skb)
+{
+	return (struct tls_msg *)strp_msg(skb);
+}
+
 static inline bool tls_is_pending_closed_record(struct tls_context *ctx)
 {
 	return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 71be8acfbc9b..1cc830582fa8 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1530,22 +1530,38 @@ static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
 }
 
 /* This function traverses the rx_list in tls receive context to copies the
- * decrypted data records into the buffer provided by caller zero copy is not
+ * decrypted records into the buffer provided by caller zero copy is not
  * true. Further, the records are removed from the rx_list if it is not a peek
  * case and the record has been consumed completely.
  */
 static int process_rx_list(struct tls_sw_context_rx *ctx,
 			   struct msghdr *msg,
+			   u8 *control,
+			   bool *cmsg,
 			   size_t skip,
 			   size_t len,
 			   bool zc,
 			   bool is_peek)
 {
 	struct sk_buff *skb = skb_peek(&ctx->rx_list);
+	u8 ctrl = *control;
+	u8 msgc = *cmsg;
+	struct tls_msg *tlm;
 	ssize_t copied = 0;
 
+	/* Set the record type in 'control' if caller didn't pass it */
+	if (!ctrl && skb) {
+		tlm = tls_msg(skb);
+		ctrl = tlm->control;
+	}
+
 	while (skip && skb) {
 		struct strp_msg *rxm = strp_msg(skb);
+		tlm = tls_msg(skb);
+
+		/* Cannot process a record of different type */
+		if (ctrl != tlm->control)
+			return 0;
 
 		if (skip < rxm->full_len)
 			break;
@@ -1559,6 +1575,27 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
 		struct strp_msg *rxm = strp_msg(skb);
 		int chunk = min_t(unsigned int, rxm->full_len - skip, len);
 
+		tlm = tls_msg(skb);
+
+		/* Cannot process a record of different type */
+		if (ctrl != tlm->control)
+			return 0;
+
+		/* Set record type if not already done. For a non-data record,
+		 * do not proceed if record type could not be copied.
+		 */
+		if (!msgc) {
+			int cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
+					    sizeof(ctrl), &ctrl);
+			msgc = true;
+			if (ctrl != TLS_RECORD_TYPE_DATA) {
+				if (cerr || msg->msg_flags & MSG_CTRUNC)
+					return -EIO;
+
+				*cmsg = msgc;
+			}
+		}
+
 		if (!zc || (rxm->full_len - skip) > len) {
 			int err = skb_copy_datagram_msg(skb, rxm->offset + skip,
 						    msg, chunk);
@@ -1597,6 +1634,7 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
 		skb = next_skb;
 	}
 
+	*control = ctrl;
 	return copied;
 }
 
@@ -1614,6 +1652,7 @@ int tls_sw_recvmsg(struct sock *sk,
 	unsigned char control = 0;
 	ssize_t decrypted = 0;
 	struct strp_msg *rxm;
+	struct tls_msg *tlm;
 	struct sk_buff *skb;
 	ssize_t copied = 0;
 	bool cmsg = false;
@@ -1632,7 +1671,8 @@ int tls_sw_recvmsg(struct sock *sk,
 	lock_sock(sk);
 
 	/* Process pending decrypted records. It must be non-zero-copy */
-	err = process_rx_list(ctx, msg, 0, len, false, is_peek);
+	err = process_rx_list(ctx, msg, &control, &cmsg, 0, len, false,
+			      is_peek);
 	if (err < 0) {
 		tls_err_abort(sk, err);
 		goto end;
@@ -1668,6 +1708,12 @@ int tls_sw_recvmsg(struct sock *sk,
 				}
 			}
 			goto recv_end;
+		} else {
+			tlm = tls_msg(skb);
+			if (prot->version == TLS_1_3_VERSION)
+				tlm->control = 0;
+			else
+				tlm->control = ctx->control;
 		}
 
 		rxm = strp_msg(skb);
@@ -1694,22 +1740,34 @@ int tls_sw_recvmsg(struct sock *sk,
 
 		if (err == -EINPROGRESS)
 			num_async++;
+		else if (prot->version == TLS_1_3_VERSION)
+			tlm->control = ctx->control;
+
+		/* If the type of records being processed is not known yet,
+		 * set it to record type just dequeued. If it is already known,
+		 * but does not match the record type just dequeued, go to end.
+		 * We always get record type here since for tls1.2, record type
+		 * is known just after record is dequeued from stream parser.
+		 * For tls1.3, we disable async.
+		 */
+
+		if (!control)
+			control = tlm->control;
+		else if (control != tlm->control)
+			goto recv_end;
 
 		if (!cmsg) {
 			int cerr;
 
 			cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
-					sizeof(ctx->control), &ctx->control);
+					sizeof(control), &control);
 			cmsg = true;
-			control = ctx->control;
-			if (ctx->control != TLS_RECORD_TYPE_DATA) {
+			if (control != TLS_RECORD_TYPE_DATA) {
 				if (cerr || msg->msg_flags & MSG_CTRUNC) {
 					err = -EIO;
 					goto recv_end;
 				}
 			}
-		} else if (control != ctx->control) {
-			goto recv_end;
 		}
 
 		if (async)
@@ -1784,18 +1842,16 @@ int tls_sw_recvmsg(struct sock *sk,
 
 		/* Drain records from the rx_list & copy if required */
 		if (is_peek || is_kvec)
-			err = process_rx_list(ctx, msg, copied,
+			err = process_rx_list(ctx, msg, &control, &cmsg, copied,
 					      decrypted, false, is_peek);
 		else
-			err = process_rx_list(ctx, msg, 0,
+			err = process_rx_list(ctx, msg, &control, &cmsg, 0,
 					      decrypted, true, is_peek);
 		if (err < 0) {
 			tls_err_abort(sk, err);
 			copied = 0;
 			goto end;
 		}
-
-		WARN_ON(decrypted != err);
 	}
 
 	copied += decrypted;
-- 
2.13.6


^ permalink raw reply related

* [PATCHv3 net-next] tls: Return type of non-data records retrieved using MSG_PEEK in recvmsg
From: Vakul Garg @ 2019-02-23  8:42 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: borisp@mellanox.com, aviadye@mellanox.com, davejwatson@fb.com,
	davem@davemloft.net, doronrk@fb.com, Vakul Garg

The patch enables returning 'type' in msghdr for records that are
retrieved with MSG_PEEK in recvmsg. Further it prevents records peeked
from socket from getting clubbed with any other record of different
type when records are subsequently dequeued from strparser.

For each record, we now retain its type in sk_buff's control buffer
cb[]. Inside control buffer, record's full length and offset are already
stored by strparser in 'struct strp_msg'. We store record type after
'struct strp_msg' inside 'struct tls_msg'. For tls1.2, the type is
stored just after record dequeue. For tls1.3, the type is stored after
record has been decrypted.

Inside process_rx_list(), before processing a non-data record, we check
that we must be able to return back the record type to the user
application. If not, the decrypted records in tls context's rx_list is
left there without consuming any data.

Fixes: 692d7b5d1f912 ("tls: Fix recvmsg() to be able to peek across
multiple records")
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---

Changes in v2:
	- Modified 'Fixed:' line to use full commit header line.
Changes in v3:
	- Added a missing ' " ' in Fixed: line.

 include/net/tls.h | 10 +++++++
 net/tls/tls_sw.c  | 78 +++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 77 insertions(+), 11 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index a8b37226a287..9f4117ae2297 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -129,6 +129,11 @@ struct tls_rec {
 	u8 aead_req_ctx[];
 };
 
+struct tls_msg {
+	struct strp_msg rxm;
+	u8 control;
+};
+
 struct tx_work {
 	struct delayed_work work;
 	struct sock *sk;
@@ -333,6 +338,11 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
 int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx,
 				   int flags, long *timeo);
 
+static inline struct tls_msg *tls_msg(struct sk_buff *skb)
+{
+	return (struct tls_msg *)strp_msg(skb);
+}
+
 static inline bool tls_is_pending_closed_record(struct tls_context *ctx)
 {
 	return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 71be8acfbc9b..1cc830582fa8 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1530,22 +1530,38 @@ static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
 }
 
 /* This function traverses the rx_list in tls receive context to copies the
- * decrypted data records into the buffer provided by caller zero copy is not
+ * decrypted records into the buffer provided by caller zero copy is not
  * true. Further, the records are removed from the rx_list if it is not a peek
  * case and the record has been consumed completely.
  */
 static int process_rx_list(struct tls_sw_context_rx *ctx,
 			   struct msghdr *msg,
+			   u8 *control,
+			   bool *cmsg,
 			   size_t skip,
 			   size_t len,
 			   bool zc,
 			   bool is_peek)
 {
 	struct sk_buff *skb = skb_peek(&ctx->rx_list);
+	u8 ctrl = *control;
+	u8 msgc = *cmsg;
+	struct tls_msg *tlm;
 	ssize_t copied = 0;
 
+	/* Set the record type in 'control' if caller didn't pass it */
+	if (!ctrl && skb) {
+		tlm = tls_msg(skb);
+		ctrl = tlm->control;
+	}
+
 	while (skip && skb) {
 		struct strp_msg *rxm = strp_msg(skb);
+		tlm = tls_msg(skb);
+
+		/* Cannot process a record of different type */
+		if (ctrl != tlm->control)
+			return 0;
 
 		if (skip < rxm->full_len)
 			break;
@@ -1559,6 +1575,27 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
 		struct strp_msg *rxm = strp_msg(skb);
 		int chunk = min_t(unsigned int, rxm->full_len - skip, len);
 
+		tlm = tls_msg(skb);
+
+		/* Cannot process a record of different type */
+		if (ctrl != tlm->control)
+			return 0;
+
+		/* Set record type if not already done. For a non-data record,
+		 * do not proceed if record type could not be copied.
+		 */
+		if (!msgc) {
+			int cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
+					    sizeof(ctrl), &ctrl);
+			msgc = true;
+			if (ctrl != TLS_RECORD_TYPE_DATA) {
+				if (cerr || msg->msg_flags & MSG_CTRUNC)
+					return -EIO;
+
+				*cmsg = msgc;
+			}
+		}
+
 		if (!zc || (rxm->full_len - skip) > len) {
 			int err = skb_copy_datagram_msg(skb, rxm->offset + skip,
 						    msg, chunk);
@@ -1597,6 +1634,7 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
 		skb = next_skb;
 	}
 
+	*control = ctrl;
 	return copied;
 }
 
@@ -1614,6 +1652,7 @@ int tls_sw_recvmsg(struct sock *sk,
 	unsigned char control = 0;
 	ssize_t decrypted = 0;
 	struct strp_msg *rxm;
+	struct tls_msg *tlm;
 	struct sk_buff *skb;
 	ssize_t copied = 0;
 	bool cmsg = false;
@@ -1632,7 +1671,8 @@ int tls_sw_recvmsg(struct sock *sk,
 	lock_sock(sk);
 
 	/* Process pending decrypted records. It must be non-zero-copy */
-	err = process_rx_list(ctx, msg, 0, len, false, is_peek);
+	err = process_rx_list(ctx, msg, &control, &cmsg, 0, len, false,
+			      is_peek);
 	if (err < 0) {
 		tls_err_abort(sk, err);
 		goto end;
@@ -1668,6 +1708,12 @@ int tls_sw_recvmsg(struct sock *sk,
 				}
 			}
 			goto recv_end;
+		} else {
+			tlm = tls_msg(skb);
+			if (prot->version == TLS_1_3_VERSION)
+				tlm->control = 0;
+			else
+				tlm->control = ctx->control;
 		}
 
 		rxm = strp_msg(skb);
@@ -1694,22 +1740,34 @@ int tls_sw_recvmsg(struct sock *sk,
 
 		if (err == -EINPROGRESS)
 			num_async++;
+		else if (prot->version == TLS_1_3_VERSION)
+			tlm->control = ctx->control;
+
+		/* If the type of records being processed is not known yet,
+		 * set it to record type just dequeued. If it is already known,
+		 * but does not match the record type just dequeued, go to end.
+		 * We always get record type here since for tls1.2, record type
+		 * is known just after record is dequeued from stream parser.
+		 * For tls1.3, we disable async.
+		 */
+
+		if (!control)
+			control = tlm->control;
+		else if (control != tlm->control)
+			goto recv_end;
 
 		if (!cmsg) {
 			int cerr;
 
 			cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
-					sizeof(ctx->control), &ctx->control);
+					sizeof(control), &control);
 			cmsg = true;
-			control = ctx->control;
-			if (ctx->control != TLS_RECORD_TYPE_DATA) {
+			if (control != TLS_RECORD_TYPE_DATA) {
 				if (cerr || msg->msg_flags & MSG_CTRUNC) {
 					err = -EIO;
 					goto recv_end;
 				}
 			}
-		} else if (control != ctx->control) {
-			goto recv_end;
 		}
 
 		if (async)
@@ -1784,18 +1842,16 @@ int tls_sw_recvmsg(struct sock *sk,
 
 		/* Drain records from the rx_list & copy if required */
 		if (is_peek || is_kvec)
-			err = process_rx_list(ctx, msg, copied,
+			err = process_rx_list(ctx, msg, &control, &cmsg, copied,
 					      decrypted, false, is_peek);
 		else
-			err = process_rx_list(ctx, msg, 0,
+			err = process_rx_list(ctx, msg, &control, &cmsg, 0,
 					      decrypted, true, is_peek);
 		if (err < 0) {
 			tls_err_abort(sk, err);
 			copied = 0;
 			goto end;
 		}
-
-		WARN_ON(decrypted != err);
 	}
 
 	copied += decrypted;
-- 
2.13.6


^ permalink raw reply related

* RE: [PATCH net] staging: fsl-dpaa2: ethsw: Add missing netdevice check
From: Ioana Ciornei @ 2019-02-23  8:45 UTC (permalink / raw)
  To: Florian Fainelli, netdev@vger.kernel.org
  Cc: Ioana Ciocoi Radulescu, Greg Kroah-Hartman,
	open list:DPAA2 ETHERNET SWITCH DRIVER,
	open list:STAGING SUBSYSTEM
In-Reply-To: <20190222220214.25401-1-f.fainelli@gmail.com>


> Subject: [PATCH net] staging: fsl-dpaa2: ethsw: Add missing netdevice check
> 
> port_switchdev_event() does not check that the target network device is
> actually backed by the ethsw driver, this could be problematic in a stacked
> environment case.
> 

Just FYI, I sent a patch set containing a similar patch verifying if the netdev is backed by the ethsw:
https://lkml.org/lkml/2019/2/6/216

I sent the entire patch set against the staging tree.

Ioana C

> Fixes: 44baaa43d7cc ("staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet
> Switch driver")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-
> dpaa2/ethsw/ethsw.c
> index daabaceeea52..2edd82f5229a 100644
> --- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> @@ -1047,6 +1047,9 @@ static int port_switchdev_event(struct notifier_block
> *unused,
>  	struct ethsw_switchdev_event_work *switchdev_work;
>  	struct switchdev_notifier_fdb_info *fdb_info = ptr;
> 
> +	if (!ethsw_port_dev_check(dev))
> +		return NOTIFY_DONE;
> +
>  	switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
>  	if (!switchdev_work)
>  		return NOTIFY_BAD;
> --
> 2.17.1


^ permalink raw reply

* [PATCH 0/3] soc: fsl: dpio: enable and configure cache stashing
From: Ioana Ciornei @ 2019-02-23  8:48 UTC (permalink / raw)
  To: Leo Li
  Cc: Roy Pledge, Ioana Ciocoi Radulescu, Laurentiu Tudor, Horia Geanta,
	brouer@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ioana Ciornei

The first two patches enable cache stashing and configure the core cluster
destination per software portal while the third patch is the one
configuring the amount of stashing on a queue.

Ioana Ciornei (3):
  soc: fsl: dpio: enable frame data cache stashing per software portal
  soc: fsl: dpio: configure cache stashing destination
  dpaa2-eth: configure the cache stashing amount on a queue

 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c |  7 +++-
 drivers/soc/fsl/Kconfig                          |  1 +
 drivers/soc/fsl/dpio/dpio-cmd.h                  |  5 +++
 drivers/soc/fsl/dpio/dpio-driver.c               | 52 ++++++++++++++++++++++++
 drivers/soc/fsl/dpio/dpio.c                      | 16 ++++++++
 drivers/soc/fsl/dpio/dpio.h                      |  5 +++
 drivers/soc/fsl/dpio/qbman-portal.c              |  4 +-
 7 files changed, 87 insertions(+), 3 deletions(-)

-- 
1.9.1


^ permalink raw reply

* [PATCH 1/3] soc: fsl: dpio: enable frame data cache stashing per software portal
From: Ioana Ciornei @ 2019-02-23  8:48 UTC (permalink / raw)
  To: Leo Li
  Cc: Roy Pledge, Ioana Ciocoi Radulescu, Laurentiu Tudor, Horia Geanta,
	brouer@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ioana Ciornei
In-Reply-To: <20190223084816.28985-1-ioana.ciornei@nxp.com>

Enable cache stashing on the frame data dequeued using this software
portal. Also, enable dropping a stash request transaction when the
target request queue is almost full.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/soc/fsl/dpio/qbman-portal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
index 5a73397..d020135 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.c
+++ b/drivers/soc/fsl/dpio/qbman-portal.c
@@ -169,9 +169,9 @@ struct qbman_swp *qbman_swp_init(const struct qbman_swp_desc *d)
 				3, /* RPM: Valid bit mode, RCR in array mode */
 				2, /* DCM: Discrete consumption ack mode */
 				3, /* EPM: Valid bit mode, EQCR in array mode */
-				0, /* mem stashing drop enable == FALSE */
+				1, /* mem stashing drop enable == TRUE */
 				1, /* mem stashing priority == TRUE */
-				0, /* mem stashing enable == FALSE */
+				1, /* mem stashing enable == TRUE */
 				1, /* dequeue stashing priority == TRUE */
 				0, /* dequeue stashing enable == FALSE */
 				0); /* EQCR_CI stashing priority == FALSE */
-- 
1.9.1


^ permalink raw reply related

* [PATCH 2/3] soc: fsl: dpio: configure cache stashing destination
From: Ioana Ciornei @ 2019-02-23  8:48 UTC (permalink / raw)
  To: Leo Li
  Cc: Roy Pledge, Ioana Ciocoi Radulescu, Laurentiu Tudor, Horia Geanta,
	brouer@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ioana Ciornei
In-Reply-To: <20190223084816.28985-1-ioana.ciornei@nxp.com>

Depending on the SoC version and the CPU id, configure the cache
stashing destination for a specific dpio.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/soc/fsl/Kconfig            |  1 +
 drivers/soc/fsl/dpio/dpio-cmd.h    |  5 ++++
 drivers/soc/fsl/dpio/dpio-driver.c | 52 ++++++++++++++++++++++++++++++++++++++
 drivers/soc/fsl/dpio/dpio.c        | 16 ++++++++++++
 drivers/soc/fsl/dpio/dpio.h        |  5 ++++
 5 files changed, 79 insertions(+)

diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
index 8f80e8b..61f8e14 100644
--- a/drivers/soc/fsl/Kconfig
+++ b/drivers/soc/fsl/Kconfig
@@ -22,6 +22,7 @@ config FSL_GUTS
 config FSL_MC_DPIO
         tristate "QorIQ DPAA2 DPIO driver"
         depends on FSL_MC_BUS
+        select SOC_BUS
         help
 	  Driver for the DPAA2 DPIO object.  A DPIO provides queue and
 	  buffer management facilities for software to interact with
diff --git a/drivers/soc/fsl/dpio/dpio-cmd.h b/drivers/soc/fsl/dpio/dpio-cmd.h
index 5814d2f..e13fd3a 100644
--- a/drivers/soc/fsl/dpio/dpio-cmd.h
+++ b/drivers/soc/fsl/dpio/dpio-cmd.h
@@ -26,6 +26,7 @@
 #define DPIO_CMDID_DISABLE				DPIO_CMD(0x003)
 #define DPIO_CMDID_GET_ATTR				DPIO_CMD(0x004)
 #define DPIO_CMDID_RESET				DPIO_CMD(0x005)
+#define DPIO_CMDID_SET_STASHING_DEST			DPIO_CMD(0x120)
 
 struct dpio_cmd_open {
 	__le32 dpio_id;
@@ -47,4 +48,8 @@ struct dpio_rsp_get_attr {
 	__le32 qbman_version;
 };
 
+struct dpio_stashing_dest {
+	u8 sdest;
+};
+
 #endif /* _FSL_DPIO_CMD_H */
diff --git a/drivers/soc/fsl/dpio/dpio-driver.c b/drivers/soc/fsl/dpio/dpio-driver.c
index a28799b..c0cdc89 100644
--- a/drivers/soc/fsl/dpio/dpio-driver.c
+++ b/drivers/soc/fsl/dpio/dpio-driver.c
@@ -14,6 +14,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/sys_soc.h>
 
 #include <linux/fsl/mc.h>
 #include <soc/fsl/dpaa2-io.h>
@@ -32,6 +33,46 @@ struct dpio_priv {
 
 static cpumask_var_t cpus_unused_mask;
 
+static const struct soc_device_attribute ls1088a_soc[] = {
+	{.family = "QorIQ LS1088A"},
+	{ /* sentinel */ }
+};
+
+static const struct soc_device_attribute ls2080a_soc[] = {
+	{.family = "QorIQ LS2080A"},
+	{ /* sentinel */ }
+};
+
+static const struct soc_device_attribute ls2088a_soc[] = {
+	{.family = "QorIQ LS2088A"},
+	{ /* sentinel */ }
+};
+
+static const struct soc_device_attribute lx2160a_soc[] = {
+	{.family = "QorIQ LX2160A"},
+	{ /* sentinel */ }
+};
+
+static int dpaa2_dpio_get_cluster_sdest(struct fsl_mc_device *dpio_dev, int cpu)
+{
+	int cluster_base, cluster_size;
+
+	if (soc_device_match(ls1088a_soc)) {
+		cluster_base = 2;
+		cluster_size = 4;
+	} else if (soc_device_match(ls2080a_soc) ||
+		   soc_device_match(ls2088a_soc) ||
+		   soc_device_match(lx2160a_soc)) {
+		cluster_base = 0;
+		cluster_size = 2;
+	} else {
+		dev_err(&dpio_dev->dev, "unknown SoC version\n");
+		return -1;
+	}
+
+	return cluster_base + cpu / cluster_size;
+}
+
 static irqreturn_t dpio_irq_handler(int irq_num, void *arg)
 {
 	struct device *dev = (struct device *)arg;
@@ -89,6 +130,7 @@ static int dpaa2_dpio_probe(struct fsl_mc_device *dpio_dev)
 	int err = -ENOMEM;
 	struct device *dev = &dpio_dev->dev;
 	int possible_next_cpu;
+	int sdest;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -145,6 +187,16 @@ static int dpaa2_dpio_probe(struct fsl_mc_device *dpio_dev)
 	desc.cpu = possible_next_cpu;
 	cpumask_clear_cpu(possible_next_cpu, cpus_unused_mask);
 
+	sdest = dpaa2_dpio_get_cluster_sdest(dpio_dev, desc.cpu);
+	if (sdest >= 0) {
+		err = dpio_set_stashing_destination(dpio_dev->mc_io, 0,
+						    dpio_dev->mc_handle,
+						    sdest);
+		if (err)
+			dev_err(dev, "dpio_set_stashing_destination failed for cpu%d\n",
+				desc.cpu);
+	}
+
 	/*
 	 * Set the CENA regs to be the cache inhibited area of the portal to
 	 * avoid coherency issues if a user migrates to another core.
diff --git a/drivers/soc/fsl/dpio/dpio.c b/drivers/soc/fsl/dpio/dpio.c
index 521bc69..af74c59 100644
--- a/drivers/soc/fsl/dpio/dpio.c
+++ b/drivers/soc/fsl/dpio/dpio.c
@@ -166,6 +166,22 @@ int dpio_get_attributes(struct fsl_mc_io *mc_io,
 	return 0;
 }
 
+int dpio_set_stashing_destination(struct fsl_mc_io *mc_io,
+				  u32 cmd_flags,
+				  u16 token,
+				  u8 sdest)
+{
+	struct fsl_mc_command cmd = { 0 };
+	struct dpio_stashing_dest *dpio_cmd;
+
+	cmd.header = mc_encode_cmd_header(DPIO_CMDID_SET_STASHING_DEST,
+					  cmd_flags, token);
+	dpio_cmd = (struct dpio_stashing_dest *)cmd.params;
+	dpio_cmd->sdest = sdest;
+
+	return mc_send_command(mc_io, &cmd);
+}
+
 /**
  * dpio_get_api_version - Get Data Path I/O API version
  * @mc_io:	Pointer to MC portal's DPIO object
diff --git a/drivers/soc/fsl/dpio/dpio.h b/drivers/soc/fsl/dpio/dpio.h
index b2ac4ba..da06f72 100644
--- a/drivers/soc/fsl/dpio/dpio.h
+++ b/drivers/soc/fsl/dpio/dpio.h
@@ -75,6 +75,11 @@ int dpio_get_attributes(struct fsl_mc_io	*mc_io,
 			u16		token,
 			struct dpio_attr	*attr);
 
+int dpio_set_stashing_destination(struct fsl_mc_io *mc_io,
+				  u32 cmd_flags,
+				  u16 token,
+				  u8 dest);
+
 int dpio_get_api_version(struct fsl_mc_io *mc_io,
 			 u32 cmd_flags,
 			 u16 *major_ver,
-- 
1.9.1


^ permalink raw reply related

* [PATCH 3/3] dpaa2-eth: configure the cache stashing amount on a queue
From: Ioana Ciornei @ 2019-02-23  8:48 UTC (permalink / raw)
  To: Leo Li
  Cc: Roy Pledge, Ioana Ciocoi Radulescu, Laurentiu Tudor, Horia Geanta,
	brouer@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ioana Ciornei
In-Reply-To: <20190223084816.28985-1-ioana.ciornei@nxp.com>

Configure the amount of 64 bytes of frame, annotation and context data
that will be cache stashed for a specific frame queue.  Since the frame
context is not used, configure that only 64 bytes of frame data and 64
bytes of annotation will be stashed.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index c500ea7..3c03fca8 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -2303,9 +2303,14 @@ static int setup_rx_flow(struct dpaa2_eth_priv *priv,
 	queue.destination.type = DPNI_DEST_DPCON;
 	queue.destination.priority = 1;
 	queue.user_context = (u64)(uintptr_t)fq;
+	queue.flc.stash_control = 1;
+	queue.flc.value &= 0xFFFFFFFFFFFFFFC0;
+	/* 01 01 00 - data, annotation, flow context */
+	queue.flc.value |= 0x14;
 	err = dpni_set_queue(priv->mc_io, 0, priv->mc_token,
 			     DPNI_QUEUE_RX, 0, fq->flowid,
-			     DPNI_QUEUE_OPT_USER_CTX | DPNI_QUEUE_OPT_DEST,
+			     DPNI_QUEUE_OPT_USER_CTX | DPNI_QUEUE_OPT_DEST |
+			     DPNI_QUEUE_OPT_FLC,
 			     &queue);
 	if (err) {
 		dev_err(dev, "dpni_set_queue(RX) failed\n");
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH] net: phy: realtek: Dummy IRQ calls for RTL8366RB
From: Heiner Kallweit @ 2019-02-23  9:00 UTC (permalink / raw)
  To: Linus Walleij, Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20190223023639.27038-1-linus.walleij@linaro.org>

On 23.02.2019 03:36, Linus Walleij wrote:
> This fixes a regression introduced by
> commit 0d2e778e38e0ddffab4bb2b0e9ed2ad5165c4bf7
> "net: phy: replace PHY_HAS_INTERRUPT with a check for
> config_intr and ack_interrupt".
> 
> This assumes that a PHY cannot trigger interrupt unless
> it has .config_intr() or .ack_interrupt() implemented.
> A later patch makes the code assume both need to be
> implemented for interrupts to be present.
> 
> But this PHY (which is inside a DSA) will happily
> fire interrupts without either callback.
> 
> Implement a dummy callback for .config_intr() and
> .ack_interrupt() as a workaround.
> 
> Tested on the RTL8366RB on D-Link DIR-685.
> 
> Fixes: 0d2e778e38e0 ("net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt")
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/net/phy/realtek.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index c6010fb1aa0f..31372be44570 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -211,6 +211,17 @@ static int rtl8366rb_config_init(struct phy_device *phydev)
>  	return ret;
>  }
>  
> +static int rtl8366rb_ack_interrupt(struct phy_device *phydev)
> +{
> +	return 0;
> +}
> +
> +static int rtl8366rb_config_intr(struct phy_device *phydev)
> +{
> +	/* Dummy function to make sure we get interrupts */
> +	return 0;
> +}
> +

Maybe we should place these dummy calls in the core. We have this for
other cases already, see e.g. genphy_no_soft_reset. Then these dummies
can be re-used by other drivers that may have the need to do so.
Names could be:
genphy_no_ack_interrupt
genphy_no_config_intr

>  static struct phy_driver realtek_drvs[] = {
>  	{
>  		PHY_ID_MATCH_EXACT(0x00008201),
> @@ -282,6 +293,8 @@ static struct phy_driver realtek_drvs[] = {
>  		.name		= "RTL8366RB Gigabit Ethernet",
>  		.features	= PHY_GBIT_FEATURES,
>  		.config_init	= &rtl8366rb_config_init,
> +		.ack_interrupt	= &rtl8366rb_ack_interrupt,
> +		.config_intr	= &rtl8366rb_config_intr,
>  		.suspend	= genphy_suspend,
>  		.resume		= genphy_resume,
>  	},
> 

^ permalink raw reply


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