Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] net/flow: fix fc->percpu NULL pointer dereference
From: Steffen Klassert @ 2017-06-09  8:32 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: David Miller, Xin Long, network dev
In-Reply-To: <CAPwn2JSeKnspZpnWTXsze7sCSXJ9mbq2K3-mu-muuy8iT4A1qw@mail.gmail.com>

On Fri, Jun 09, 2017 at 04:23:01PM +0800, Hangbin Liu wrote:
> Hi Steffen,
> 
> BTW, If we put the check in xfrm_policy_flush(), we can prevent it earlier.
> But If we put the check in flow_cache_percpu_empty(), we can prevent
> other functions set fc->percpu to NULL, although not much possible : )
> 
> So I'm not quite sure whether we should put the check in
> flow_cache_percpu_empty() or in xfrm_policy_flush().

Can't we just call xfrm_policy_fini() first and then flow_cache_fini()?

^ permalink raw reply

* Re: [PATCH net] net/flow: fix fc->percpu NULL pointer dereference
From: Xin Long @ 2017-06-09  8:43 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: Hangbin Liu, David Miller, network dev
In-Reply-To: <20170609083255.GR22049@secunet.com>

On Fri, Jun 9, 2017 at 4:32 PM, Steffen Klassert
<steffen.klassert@secunet.com> wrote:
> On Fri, Jun 09, 2017 at 04:23:01PM +0800, Hangbin Liu wrote:
>> Hi Steffen,
>>
>> BTW, If we put the check in xfrm_policy_flush(), we can prevent it earlier.
>> But If we put the check in flow_cache_percpu_empty(), we can prevent
>> other functions set fc->percpu to NULL, although not much possible : )
>>
>> So I'm not quite sure whether we should put the check in
>> flow_cache_percpu_empty() or in xfrm_policy_flush().
>
> Can't we just call xfrm_policy_fini() first and then flow_cache_fini()?
>
That would be a better fix. seems safe as what flow_cache_fini does
is only to free fcp->hash_table and stop timer, I didn't see  it has
any dependence on xfrm_policy stuff.

^ permalink raw reply

* [PATCH net RESEND] net: rps: send out pending IPI's on CPU hotplug
From: Ashwanth Goli @ 2017-06-09  8:54 UTC (permalink / raw)
  To: netdev; +Cc: eric.dumazet

IPI's from the victim cpu are not handled in dev_cpu_callback.
So these pending IPI's would be sent to the remote cpu only when
NET_RX is scheduled on the victim cpu and since this trigger is
unpredictable it would result in packet latencies on the remote cpu.

This patch add support to send the pending ipi's of victim cpu.

Signed-off-by: Ashwanth Goli <ashwanth@codeaurora.org>
---
 net/core/dev.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index fca407b..e6bfa54 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4948,6 +4948,19 @@ __sum16 __skb_gro_checksum_complete(struct sk_buff *skb)
 }
 EXPORT_SYMBOL(__skb_gro_checksum_complete);
 
+static void net_rps_send_ipi(struct softnet_data *remsd)
+{
+#ifdef CONFIG_RPS
+	while (remsd) {
+		struct softnet_data *next = remsd->rps_ipi_next;
+
+		if (cpu_online(remsd->cpu))
+			smp_call_function_single_async(remsd->cpu, &remsd->csd);
+		remsd = next;
+	}
+#endif
+}
+
 /*
  * net_rps_action_and_irq_enable sends any pending IPI's for rps.
  * Note: called with local irq disabled, but exits with local irq enabled.
@@ -4963,14 +4976,7 @@ static void net_rps_action_and_irq_enable(struct softnet_data *sd)
 		local_irq_enable();
 
 		/* Send pending IPI's to kick RPS processing on remote cpus. */
-		while (remsd) {
-			struct softnet_data *next = remsd->rps_ipi_next;
-
-			if (cpu_online(remsd->cpu))
-				smp_call_function_single_async(remsd->cpu,
-							   &remsd->csd);
-			remsd = next;
-		}
+		net_rps_send_ipi(remsd);
 	} else
 #endif
 		local_irq_enable();
@@ -8192,7 +8198,7 @@ static int dev_cpu_dead(unsigned int oldcpu)
 	struct sk_buff **list_skb;
 	struct sk_buff *skb;
 	unsigned int cpu;
-	struct softnet_data *sd, *oldsd;
+	struct softnet_data *sd, *oldsd, *remsd;
 
 	local_irq_disable();
 	cpu = smp_processor_id();
@@ -8233,6 +8239,13 @@ static int dev_cpu_dead(unsigned int oldcpu)
 	raise_softirq_irqoff(NET_TX_SOFTIRQ);
 	local_irq_enable();
 
+#ifdef CONFIG_RPS
+	remsd = oldsd->rps_ipi_list;
+	oldsd->rps_ipi_list = NULL;
+#endif
+	/* send out pending IPI's on offline CPU */
+	net_rps_send_ipi(remsd);
+
 	/* Process offline CPU's input_pkt_queue */
 	while ((skb = __skb_dequeue(&oldsd->process_queue))) {
 		netif_rx_ni(skb);
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH net-next] net: phy: use of_mdio_parse_addr
From: Michael Ellerman @ 2017-06-09  9:00 UTC (permalink / raw)
  To: Liviu Dudau, David Miller
  Cc: jon.mason, andrew, f.fainelli, netdev, linux-kernel,
	bcm-kernel-feedback-list
In-Reply-To: <20170607161854.GC4389@bart.dudau.co.uk>

Liviu Dudau <liviu@dudau.co.uk> writes:

> On Fri, Jun 02, 2017 at 02:22:51PM -0400, David Miller wrote:
>> From: Jon Mason <jon.mason@broadcom.com>
>> Date: Wed, 31 May 2017 15:43:30 -0400
>> 
>> > use of_mdio_parse_addr() in place of an OF read of reg and a bounds
>> > check (which is litterally the exact same thing that
>> > of_mdio_parse_addr() does)
>> > 
>> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>> 
>> Applied, thanks Jon.
>
> This makes linux-next fail the modules_install target as depmod detects 2 circular
> dependencies. Reverting this patch fixes the issue.
>
> depmod: ERROR: Cycle detected: libphy -> of_mdio -> fixed_phy -> libphy
> depmod: ERROR: Cycle detected: libphy -> of_mdio -> libphy
> depmod: ERROR: Found 3 modules in dependency cycles!
> make[1]: *** [/home/dliviu/devel/kernel/Makefile:1245: _modinst_post] Error 1
>
> This is on an ARCH=arm build, build I doubt it makes a difference. Let me know if
> you need some .config values in order to reproduce.

Same happens on 32-bit powerpc.

cheers

^ permalink raw reply

* [PATCH 05/11] net: caif: convert to use DRIVER_ATTR_RO
From: Greg Kroah-Hartman @ 2017-06-09  9:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, Dmitry Tarnyagin, netdev
In-Reply-To: <20170609090314.13991-1-gregkh@linuxfoundation.org>

We are trying to get rid of DRIVER_ATTR(), and the caif driver's
attributes can be trivially changed to use DRIVER_ATTR_RO().

Cc: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>
Cc: <netdev@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/caif/caif_spi.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/caif/caif_spi.c b/drivers/net/caif/caif_spi.c
index 3a529fbe539f..3281a3e0c144 100644
--- a/drivers/net/caif/caif_spi.c
+++ b/drivers/net/caif/caif_spi.c
@@ -289,44 +289,44 @@ static LIST_HEAD(cfspi_list);
 static spinlock_t cfspi_list_lock;
 
 /* SPI uplink head alignment. */
-static ssize_t show_up_head_align(struct device_driver *driver, char *buf)
+static ssize_t up_head_align_show(struct device_driver *driver, char *buf)
 {
 	return sprintf(buf, "%d\n", spi_up_head_align);
 }
 
-static DRIVER_ATTR(up_head_align, S_IRUSR, show_up_head_align, NULL);
+static DRIVER_ATTR_RO(up_head_align);
 
 /* SPI uplink tail alignment. */
-static ssize_t show_up_tail_align(struct device_driver *driver, char *buf)
+static ssize_t up_tail_align_show(struct device_driver *driver, char *buf)
 {
 	return sprintf(buf, "%d\n", spi_up_tail_align);
 }
 
-static DRIVER_ATTR(up_tail_align, S_IRUSR, show_up_tail_align, NULL);
+static DRIVER_ATTR_RO(up_tail_align);
 
 /* SPI downlink head alignment. */
-static ssize_t show_down_head_align(struct device_driver *driver, char *buf)
+static ssize_t down_head_align_show(struct device_driver *driver, char *buf)
 {
 	return sprintf(buf, "%d\n", spi_down_head_align);
 }
 
-static DRIVER_ATTR(down_head_align, S_IRUSR, show_down_head_align, NULL);
+static DRIVER_ATTR_RO(down_head_align);
 
 /* SPI downlink tail alignment. */
-static ssize_t show_down_tail_align(struct device_driver *driver, char *buf)
+static ssize_t down_tail_align_show(struct device_driver *driver, char *buf)
 {
 	return sprintf(buf, "%d\n", spi_down_tail_align);
 }
 
-static DRIVER_ATTR(down_tail_align, S_IRUSR, show_down_tail_align, NULL);
+static DRIVER_ATTR_RO(down_tail_align);
 
 /* SPI frame alignment. */
-static ssize_t show_frame_align(struct device_driver *driver, char *buf)
+static ssize_t frame_align_show(struct device_driver *driver, char *buf)
 {
 	return sprintf(buf, "%d\n", spi_frm_align);
 }
 
-static DRIVER_ATTR(frame_align, S_IRUSR, show_frame_align, NULL);
+static DRIVER_ATTR_RO(frame_align);
 
 int cfspi_xmitfrm(struct cfspi *cfspi, u8 *buf, size_t len)
 {
-- 
2.13.1

^ permalink raw reply related

* [PATCH 06/11] net: ehea: convert to use DRIVER_ATTR_RO
From: Greg Kroah-Hartman @ 2017-06-09  9:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, Douglas Miller, netdev
In-Reply-To: <20170609090314.13991-1-gregkh@linuxfoundation.org>

We are trying to get rid of DRIVER_ATTR(), and the ehea driver's
attribute can be trivially changed to use DRIVER_ATTR_RO().

Cc: Douglas Miller <dougmill@linux.vnet.ibm.com>
Cc: <netdev@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/ethernet/ibm/ehea/ehea_main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index 1e53d7a82675..b9d310f20bcc 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -3553,14 +3553,12 @@ static int check_module_parm(void)
 	return ret;
 }
 
-static ssize_t ehea_show_capabilities(struct device_driver *drv,
-				      char *buf)
+static ssize_t capabilities_show(struct device_driver *drv, char *buf)
 {
 	return sprintf(buf, "%d", EHEA_CAPABILITIES);
 }
 
-static DRIVER_ATTR(capabilities, S_IRUSR | S_IRGRP | S_IROTH,
-		   ehea_show_capabilities, NULL);
+static DRIVER_ATTR_RO(capabilities);
 
 static int __init ehea_module_init(void)
 {
-- 
2.13.1

^ permalink raw reply related

* [PATCH 07/11] wireless: ipw2x00: convert to use DRIVER_ATTR_RW
From: Greg Kroah-Hartman @ 2017-06-09  9:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Stanislav Yakovlev, Kalle Valo,
	linux-wireless, netdev
In-Reply-To: <20170609090314.13991-1-gregkh@linuxfoundation.org>

We are trying to get rid of DRIVER_ATTR(), and the ipw2x00 driver's
attributes can be trivially changed to use DRIVER_ATTR_RW().

Cc: Stanislav Yakovlev <stas.yakovlev@gmail.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: <linux-wireless@vger.kernel.org>
Cc: <netdev@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/wireless/intel/ipw2x00/ipw2100.c | 8 +++-----
 drivers/net/wireless/intel/ipw2x00/ipw2200.c | 8 +++-----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2100.c b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
index f922859acf40..aaaca4d08e2b 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
@@ -4160,12 +4160,12 @@ static ssize_t show_bssinfo(struct device *d, struct device_attribute *attr,
 static DEVICE_ATTR(bssinfo, S_IRUGO, show_bssinfo, NULL);
 
 #ifdef CONFIG_IPW2100_DEBUG
-static ssize_t show_debug_level(struct device_driver *d, char *buf)
+static ssize_t debug_level_show(struct device_driver *d, char *buf)
 {
 	return sprintf(buf, "0x%08X\n", ipw2100_debug_level);
 }
 
-static ssize_t store_debug_level(struct device_driver *d,
+static ssize_t debug_level_store(struct device_driver *d,
 				 const char *buf, size_t count)
 {
 	u32 val;
@@ -4179,9 +4179,7 @@ static ssize_t store_debug_level(struct device_driver *d,
 
 	return strnlen(buf, count);
 }
-
-static DRIVER_ATTR(debug_level, S_IWUSR | S_IRUGO, show_debug_level,
-		   store_debug_level);
+static DRIVER_ATTR_RW(debug_level);
 #endif				/* CONFIG_IPW2100_DEBUG */
 
 static ssize_t show_fatal_error(struct device *d,
diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
index bbc579b647b6..5b79e2ec3a16 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
@@ -1195,12 +1195,12 @@ static void ipw_led_shutdown(struct ipw_priv *priv)
  *
  * See the level definitions in ipw for details.
  */
-static ssize_t show_debug_level(struct device_driver *d, char *buf)
+static ssize_t debug_level_show(struct device_driver *d, char *buf)
 {
 	return sprintf(buf, "0x%08X\n", ipw_debug_level);
 }
 
-static ssize_t store_debug_level(struct device_driver *d, const char *buf,
+static ssize_t debug_level_store(struct device_driver *d, const char *buf,
 				 size_t count)
 {
 	char *p = (char *)buf;
@@ -1221,9 +1221,7 @@ static ssize_t store_debug_level(struct device_driver *d, const char *buf,
 
 	return strnlen(buf, count);
 }
-
-static DRIVER_ATTR(debug_level, S_IWUSR | S_IRUGO,
-		   show_debug_level, store_debug_level);
+static DRIVER_ATTR_RW(debug_level);
 
 static inline u32 ipw_get_event_log_len(struct ipw_priv *priv)
 {
-- 
2.13.1

^ permalink raw reply related

* Re: [PATCH net] net/flow: fix fc->percpu NULL pointer dereference
From: Hangbin Liu @ 2017-06-09  9:06 UTC (permalink / raw)
  To: Xin Long; +Cc: Steffen Klassert, David Miller, network dev
In-Reply-To: <CADvbK_dn9QMd8_yRsBMZnjstxjy_L-Pc_UPkyNbyZ=0OORu85A@mail.gmail.com>

2017-06-09 16:43 GMT+08:00 Xin Long <lucien.xin@gmail.com>:
> On Fri, Jun 9, 2017 at 4:32 PM, Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
>> On Fri, Jun 09, 2017 at 04:23:01PM +0800, Hangbin Liu wrote:
>>> Hi Steffen,
>>>
>>> BTW, If we put the check in xfrm_policy_flush(), we can prevent it earlier.
>>> But If we put the check in flow_cache_percpu_empty(), we can prevent
>>> other functions set fc->percpu to NULL, although not much possible : )
>>>
>>> So I'm not quite sure whether we should put the check in
>>> flow_cache_percpu_empty() or in xfrm_policy_flush().
>>
>> Can't we just call xfrm_policy_fini() first and then flow_cache_fini()?

Yes, that would be easy fix. I have been thinking about that.  But if we change
 the order in xfrm_net_exit(), do we also need to change the order in
 xfrm_net_init()? That would change a lot.

If no need, that would be good.

>>
> That would be a better fix. seems safe as what flow_cache_fini does
> is only to free fcp->hash_table and stop timer, I didn't see  it has
> any dependence on xfrm_policy stuff.

I'm not familiar about this part. So not sure about the influence if we free
the flow cache after xfrm_policy_fini(). I need do some test first.

I would also be appreciate if you or some one could make sure if it doesn't
influence anything.

Thanks
Hangbin

^ permalink raw reply

* [PATCH net-next 00/11] r8152: minor adjustment
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang

Adjust some code to make it reasonable or satisfy the suggestion from
the engineers.

Hayes Wang (11):
  r8152: add r8153_phy_status function
  r8152: adjust lpm settings for RTL8153
  r8152: adjust the settings about MAC clock speed down for RTL8153
  r8152: move the setting of rx aggregation
  r8152: adjust rtl8153_runtime_enable function
  r8152: adjust U2P3 for RTL8153
  r8152: move the default coalesce setting for RTL8153
  r8152: move the initialization to reset_resume function
  r8152: check if disabling ALDPS is finished
  r8152: avoid rx queue more than 1000 packets
  r8152: replace napi_complete with napi_complete_done

 drivers/net/usb/r8152.c | 181 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 130 insertions(+), 51 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH net-next 01/11] r8152: add r8153_phy_status function
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Use r8153_phy_status() to check phy status of RTL8153.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fd31fab..9239dfb 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -394,6 +394,7 @@
 
 /* OCP_PHY_STATUS */
 #define PHY_STAT_MASK		0x0007
+#define PHY_STAT_EXT_INIT	2
 #define PHY_STAT_LAN_ON		3
 #define PHY_STAT_PWRDN		5
 
@@ -2452,6 +2453,28 @@ static void r8153_u2p3en(struct r8152 *tp, bool enable)
 	ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
 }
 
+static u16 r8153_phy_status(struct r8152 *tp, u16 desired)
+{
+	u16 data;
+	int i;
+
+	for (i = 0; i < 500; i++) {
+		data = ocp_reg_read(tp, OCP_PHY_STATUS);
+		data &= PHY_STAT_MASK;
+		if (desired) {
+			if (data == desired)
+				break;
+		} else if (data == PHY_STAT_LAN_ON || data == PHY_STAT_PWRDN ||
+			   data == PHY_STAT_EXT_INIT) {
+			break;
+		}
+
+		msleep(20);
+	}
+
+	return data;
+}
+
 static void r8153_power_cut_en(struct r8152 *tp, bool enable)
 {
 	u32 ocp_data;
@@ -3420,12 +3443,7 @@ static void r8153_init(struct r8152 *tp)
 		msleep(20);
 	}
 
-	for (i = 0; i < 500; i++) {
-		ocp_data = ocp_reg_read(tp, OCP_PHY_STATUS) & PHY_STAT_MASK;
-		if (ocp_data == PHY_STAT_LAN_ON || ocp_data == PHY_STAT_PWRDN)
-			break;
-		msleep(20);
-	}
+	data = r8153_phy_status(tp, 0);
 
 	if (tp->version == RTL_VER_03 || tp->version == RTL_VER_04 ||
 	    tp->version == RTL_VER_05)
@@ -3437,12 +3455,7 @@ static void r8153_init(struct r8152 *tp)
 		r8152_mdio_write(tp, MII_BMCR, data);
 	}
 
-	for (i = 0; i < 500; i++) {
-		ocp_data = ocp_reg_read(tp, OCP_PHY_STATUS) & PHY_STAT_MASK;
-		if (ocp_data == PHY_STAT_LAN_ON)
-			break;
-		msleep(20);
-	}
+	data = r8153_phy_status(tp, PHY_STAT_LAN_ON);
 
 	usb_disable_lpm(tp->udev);
 	r8153_u2p3en(tp, false);
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 02/11] r8152: adjust lpm settings for RTL8153
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Enable lpm after r8153_init() and remove other enable/disable lpm.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 9239dfb..b8c904f 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2263,7 +2263,6 @@ static int rtl8153_enable(struct r8152 *tp)
 	if (test_bit(RTL8152_UNPLUG, &tp->flags))
 		return -ENODEV;
 
-	usb_disable_lpm(tp->udev);
 	set_tx_qlen(tp);
 	rtl_set_eee_plus(tp);
 	r8153_set_rx_early_timeout(tp);
@@ -3003,7 +3002,6 @@ static void rtl8153_disable(struct r8152 *tp)
 	rtl_disable(tp);
 	rtl_reset_bmu(tp);
 	r8153_aldps_en(tp, true);
-	usb_enable_lpm(tp->udev);
 }
 
 static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u16 speed, u8 duplex)
@@ -3127,7 +3125,6 @@ static void rtl8153_up(struct r8152 *tp)
 	r8153_aldps_en(tp, true);
 	r8153_u2p3en(tp, true);
 	r8153_u1u2en(tp, true);
-	usb_enable_lpm(tp->udev);
 }
 
 static void rtl8153_down(struct r8152 *tp)
@@ -3457,7 +3454,6 @@ static void r8153_init(struct r8152 *tp)
 
 	data = r8153_phy_status(tp, PHY_STAT_LAN_ON);
 
-	usb_disable_lpm(tp->udev);
 	r8153_u2p3en(tp, false);
 
 	if (tp->version == RTL_VER_04) {
@@ -3517,6 +3513,7 @@ static void r8153_init(struct r8152 *tp)
 
 	r8153_power_cut_en(tp, false);
 	r8153_u1u2en(tp, true);
+	usb_enable_lpm(tp->udev);
 
 	/* MAC clock speed down */
 	ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL, 0);
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 03/11] r8152: adjust the settings about MAC clock speed down for RTL8153
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

The MAC clock speed down could be enabled if the U1/U2 is disabled.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b8c904f..9a794db 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2428,6 +2428,29 @@ static void __rtl_set_wol(struct r8152 *tp, u32 wolopts)
 		device_set_wakeup_enable(&tp->udev->dev, false);
 }
 
+static void r8153_mac_clk_spd(struct r8152 *tp, bool enable)
+{
+	/* MAC clock speed down */
+	if (enable) {
+		ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL,
+			       ALDPS_SPDWN_RATIO);
+		ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL2,
+			       EEE_SPDWN_RATIO);
+		ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
+			       PKT_AVAIL_SPDWN_EN | SUSPEND_SPDWN_EN |
+			       U1U2_SPDWN_EN | L1_SPDWN_EN);
+		ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4,
+			       PWRSAVE_SPDWN_EN | RXDV_SPDWN_EN | TX10MIDLE_EN |
+			       TP100_SPDWN_EN | TP500_SPDWN_EN | EEE_SPDWN_EN |
+			       TP1000_SPDWN_EN);
+	} else {
+		ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL, 0);
+		ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL2, 0);
+		ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, 0);
+		ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4, 0);
+	}
+}
+
 static void r8153_u1u2en(struct r8152 *tp, bool enable)
 {
 	u8 u1u2[8];
@@ -2533,7 +2556,9 @@ static void rtl8153_runtime_enable(struct r8152 *tp, bool enable)
 	if (enable) {
 		r8153_u1u2en(tp, false);
 		r8153_u2p3en(tp, false);
+		r8153_mac_clk_spd(tp, true);
 	} else {
+		r8153_mac_clk_spd(tp, false);
 		r8153_u2p3en(tp, true);
 		r8153_u1u2en(tp, true);
 	}
@@ -2881,6 +2906,7 @@ static void r8153_first_init(struct r8152 *tp)
 	u32 ocp_data;
 	int i;
 
+	r8153_mac_clk_spd(tp, false);
 	rxdy_gated_en(tp, true);
 	r8153_teredo_off(tp);
 
@@ -2947,6 +2973,8 @@ static void r8153_enter_oob(struct r8152 *tp)
 	u32 ocp_data;
 	int i;
 
+	r8153_mac_clk_spd(tp, true);
+
 	ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL);
 	ocp_data &= ~NOW_IS_OOB;
 	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, ocp_data);
@@ -3513,13 +3541,9 @@ static void r8153_init(struct r8152 *tp)
 
 	r8153_power_cut_en(tp, false);
 	r8153_u1u2en(tp, true);
+	r8153_mac_clk_spd(tp, false);
 	usb_enable_lpm(tp->udev);
 
-	/* MAC clock speed down */
-	ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL, 0);
-	ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL2, 0);
-	ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, 0);
-	ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4, 0);
 
 	rtl_tally_reset(tp);
 	r8153_u2p3en(tp, true);
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 04/11] r8152: move the setting of rx aggregation
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Move the setting from r8153_first_init() to r8153_init(). It only needs to
be set once.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 9a794db..e569c48 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2961,11 +2961,6 @@ static void r8153_first_init(struct r8152 *tp)
 	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RXFIFO_CTRL2, RXFIFO_THR3_NORMAL);
 	/* TX share fifo free credit full threshold */
 	ocp_write_dword(tp, MCU_TYPE_PLA, PLA_TXFIFO_CTRL, TXFIFO_THR_NORMAL2);
-
-	/* rx aggregation */
-	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
-	ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
-	ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 }
 
 static void r8153_enter_oob(struct r8152 *tp)
@@ -3544,6 +3539,10 @@ static void r8153_init(struct r8152 *tp)
 	r8153_mac_clk_spd(tp, false);
 	usb_enable_lpm(tp->udev);
 
+	/* rx aggregation */
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
+	ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
+	ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 
 	rtl_tally_reset(tp);
 	r8153_u2p3en(tp, true);
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 05/11] r8152: adjust rtl8153_runtime_enable function
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Adjust the order of rtl8153_runtime_enable() according to the
suggestion from the engineer.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e569c48..32e83fd 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2551,13 +2551,13 @@ static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
 
 static void rtl8153_runtime_enable(struct r8152 *tp, bool enable)
 {
-	rtl_runtime_suspend_enable(tp, enable);
-
 	if (enable) {
 		r8153_u1u2en(tp, false);
 		r8153_u2p3en(tp, false);
 		r8153_mac_clk_spd(tp, true);
+		rtl_runtime_suspend_enable(tp, true);
 	} else {
+		rtl_runtime_suspend_enable(tp, false);
 		r8153_mac_clk_spd(tp, false);
 		r8153_u2p3en(tp, true);
 		r8153_u1u2en(tp, true);
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 06/11] r8152: adjust U2P3 for RTL8153
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Use another way to keep disabling the U2P3 for both RTL_VER_03 and
RTL_VER_04.

Move enabling U2P3 from r8153_init() to r8153_hw_phy_cfg(). The
engineer ask the setting should be done after PHY settings.

Disable U2P3 first in rtl8153_up().

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 32e83fd..565ac5b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2468,7 +2468,7 @@ static void r8153_u2p3en(struct r8152 *tp, bool enable)
 	u32 ocp_data;
 
 	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
-	if (enable && tp->version != RTL_VER_03 && tp->version != RTL_VER_04)
+	if (enable)
 		ocp_data |= U2P3_ENABLE;
 	else
 		ocp_data &= ~U2P3_ENABLE;
@@ -2559,7 +2559,18 @@ static void rtl8153_runtime_enable(struct r8152 *tp, bool enable)
 	} else {
 		rtl_runtime_suspend_enable(tp, false);
 		r8153_mac_clk_spd(tp, false);
-		r8153_u2p3en(tp, true);
+
+		switch (tp->version) {
+		case RTL_VER_03:
+		case RTL_VER_04:
+			break;
+		case RTL_VER_05:
+		case RTL_VER_06:
+		default:
+			r8153_u2p3en(tp, true);
+			break;
+		}
+
 		r8153_u1u2en(tp, true);
 	}
 }
@@ -2898,6 +2909,17 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
 	r8153_aldps_en(tp, true);
 	r8152b_enable_fc(tp);
 
+	switch (tp->version) {
+	case RTL_VER_03:
+	case RTL_VER_04:
+		break;
+	case RTL_VER_05:
+	case RTL_VER_06:
+	default:
+		r8153_u2p3en(tp, true);
+		break;
+	}
+
 	set_bit(PHY_RESET, &tp->flags);
 }
 
@@ -3143,10 +3165,22 @@ static void rtl8153_up(struct r8152 *tp)
 		return;
 
 	r8153_u1u2en(tp, false);
+	r8153_u2p3en(tp, false);
 	r8153_aldps_en(tp, false);
 	r8153_first_init(tp);
 	r8153_aldps_en(tp, true);
-	r8153_u2p3en(tp, true);
+
+	switch (tp->version) {
+	case RTL_VER_03:
+	case RTL_VER_04:
+		break;
+	case RTL_VER_05:
+	case RTL_VER_06:
+	default:
+		r8153_u2p3en(tp, true);
+		break;
+	}
+
 	r8153_u1u2en(tp, true);
 }
 
@@ -3545,7 +3579,6 @@ static void r8153_init(struct r8152 *tp)
 	ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 
 	rtl_tally_reset(tp);
-	r8153_u2p3en(tp, true);
 }
 
 static int rtl8152_pre_reset(struct usb_interface *intf)
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 07/11] r8152: move the default coalesce setting for RTL8153
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Only RTL8153 could set coalesce, so move the default setting for
rtl8152_probe() to r8153_init().

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 565ac5b..f78111c 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3579,6 +3579,19 @@ static void r8153_init(struct r8152 *tp)
 	ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 
 	rtl_tally_reset(tp);
+
+	switch (tp->udev->speed) {
+	case USB_SPEED_SUPER:
+	case USB_SPEED_SUPER_PLUS:
+		tp->coalesce = COALESCE_SUPER;
+		break;
+	case USB_SPEED_HIGH:
+		tp->coalesce = COALESCE_HIGH;
+		break;
+	default:
+		tp->coalesce = COALESCE_SLOW;
+		break;
+	}
 }
 
 static int rtl8152_pre_reset(struct usb_interface *intf)
@@ -4524,19 +4537,6 @@ static int rtl8152_probe(struct usb_interface *intf,
 	tp->mii.reg_num_mask = 0x1f;
 	tp->mii.phy_id = R8152_PHY_ID;
 
-	switch (udev->speed) {
-	case USB_SPEED_SUPER:
-	case USB_SPEED_SUPER_PLUS:
-		tp->coalesce = COALESCE_SUPER;
-		break;
-	case USB_SPEED_HIGH:
-		tp->coalesce = COALESCE_HIGH;
-		break;
-	default:
-		tp->coalesce = COALESCE_SLOW;
-		break;
-	}
-
 	tp->autoneg = AUTONEG_ENABLE;
 	tp->speed = tp->mii.supports_gmii ? SPEED_1000 : SPEED_100;
 	tp->duplex = DUPLEX_FULL;
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 08/11] r8152: move the initialization to reset_resume function
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Move tp->rtl_ops.init() from rtl8152_resume() to rtl8152_reset_resume().
The initialization is only necessary for reset_resume().

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f78111c..f43b7a8 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3776,11 +3776,8 @@ static int rtl8152_resume(struct usb_interface *intf)
 
 	mutex_lock(&tp->control);
 
-	if (!test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
-		tp->rtl_ops.init(tp);
-		queue_delayed_work(system_long_wq, &tp->hw_phy_work, 0);
+	if (!test_bit(SELECTIVE_SUSPEND, &tp->flags))
 		netif_device_attach(netdev);
-	}
 
 	if (netif_running(netdev) && netdev->flags & IFF_UP) {
 		if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
@@ -3826,6 +3823,10 @@ static int rtl8152_reset_resume(struct usb_interface *intf)
 	struct r8152 *tp = usb_get_intfdata(intf);
 
 	clear_bit(SELECTIVE_SUSPEND, &tp->flags);
+	mutex_lock(&tp->control);
+	tp->rtl_ops.init(tp);
+	queue_delayed_work(system_long_wq, &tp->hw_phy_work, 0);
+	mutex_unlock(&tp->control);
 	return rtl8152_resume(intf);
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 09/11] r8152: check if disabling ALDPS is finished
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Use PLA 0xe000 bit 8 to check if disabling ALDPS is finished.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f43b7a8..204f4b2 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2836,9 +2836,15 @@ static void r8153_aldps_en(struct r8152 *tp, bool enable)
 		data |= EN_ALDPS;
 		ocp_reg_write(tp, OCP_POWER_CFG, data);
 	} else {
+		int i;
+
 		data &= ~EN_ALDPS;
 		ocp_reg_write(tp, OCP_POWER_CFG, data);
-		msleep(20);
+		for (i = 0; i < 20; i++) {
+			usleep_range(1000, 2000);
+			if (ocp_read_word(tp, MCU_TYPE_PLA, 0xe000) & 0x0100)
+				break;
+		}
 	}
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 10/11] r8152: avoid rx queue more than 1000 packets
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Stop queuing rx packets if it is more than 1000.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 204f4b2..fa29583 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1813,6 +1813,10 @@ static int rx_bottom(struct r8152 *tp, int budget)
 			unsigned int pkt_len;
 			struct sk_buff *skb;
 
+			/* limite the skb numbers for rx_queue */
+			if (unlikely(skb_queue_len(&tp->rx_queue) >= 1000))
+				break;
+
 			pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
 			if (pkt_len < ETH_ZLEN)
 				break;
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 11/11] r8152: replace napi_complete with napi_complete_done
From: Hayes Wang @ 2017-06-09  9:11 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>

Change from using napi_complete to napi_complete_done to allow for the
use of gro_flush_timeout in tuning network processing.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fa29583..5a02053 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1938,7 +1938,8 @@ static int r8152_poll(struct napi_struct *napi, int budget)
 	bottom_half(tp);
 
 	if (work_done < budget) {
-		napi_complete(napi);
+		if (!napi_complete_done(napi, work_done))
+			goto out;
 		if (!list_empty(&tp->rx_done))
 			napi_schedule(napi);
 		else if (!skb_queue_empty(&tp->tx_queue) &&
@@ -1946,6 +1947,7 @@ static int r8152_poll(struct napi_struct *napi, int budget)
 			napi_schedule(napi);
 	}
 
+out:
 	return work_done;
 }
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v3 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Ding Tianhong @ 2017-06-09  9:13 UTC (permalink / raw)
  To: John Garry, leedom, ashok.raj, helgaas, werner, ganeshgr,
	asit.k.mallick, patrick.j.cramer, Suravee.Suthikulpanit, Bob.Shaw,
	l.stach, amira, gabriele.paoloni, David.Laight, jeffrey.t.kirsher,
	catalin.marinas, will.deacon, mark.rutland, robin.murphy, davem,
	alexander.duyck, linux-arm-kernel, netdev, linux-pci,
	linux-kernel
  Cc: Linuxarm
In-Reply-To: <a4b98567-286a-1fba-e935-f9ba50f1178d@huawei.com>


Hi John:

Thanks for the reviewing, I will fix it in next version.

Ding

On 2017/6/8 1:55, John Garry wrote:
> On 07/06/2017 10:16, Ding Tianhong wrote:
> 
> Hi Ding,
> 
> A few general style comments:
> 
>> The PCIe Device Control Register use the bit 4 to indicate that
>> whether the device is permitted to enable relaxed ordering or not.
>> But relaxed ordering is not safe for some platform which could only
>> use strong write ordering, so devices are allowed (but not required)
>> to enable relaxed ordering bit by default.
>>
>> If a PCIe device didn't enable the relaxed ordering attribute default,
>> we should not do anything in the PCIe configuration, otherwise we
>> should check if any of the devices above us do not support relaxed
>> ordering by the PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag, then base on
>> the result if we get a return that indicate that the relaxed ordering
>> is not supported we should update our device to disable relaxed ordering
>> in configuration space. If the device above us doesn't exist or isn't
>> the PCIe device, we shouldn't do anything and skip updating relaxed ordering
>> because we are probably running in a guest.
> 
> A guest machine/environment
> 
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>>  drivers/pci/pci.c   | 29 +++++++++++++++++++++++++++++
>>  drivers/pci/probe.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/pci.h |  2 ++
>>  3 files changed, 74 insertions(+)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index b01bd5b..3d42b38 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -4878,6 +4878,35 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
>>  EXPORT_SYMBOL(pcie_set_mps);
>>
>>  /**
>> + * pcie_clear_relaxed_ordering - clear PCI Express relexed ordering bit
>> + * @dev: PCI device to query
>> + *
>> + * If possible clear relaxed ordering
>> + */
>> +int pcie_clear_relaxed_ordering(struct pci_dev *dev)
>> +{
>> +    return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
>> +                      PCI_EXP_DEVCTL_RELAX_EN);
>> +}
>> +EXPORT_SYMBOL(pcie_clear_relaxed_ordering);
>> +
>> +/**
>> + * pcie_get_relaxed_ordering - check PCI Express relexed ordering bit
> 
> s/relexed/relaxed/
> 
> Check what on relaxed ordering bit?
> 
> And the function name is inconsistent with this discription.
> 
>> + * @dev: PCI device to query
>> + *
>> + * Returns true if relaxed ordering is been set
> 
> If you want to return true/false, then use !!, below in the function
> 
>> + */
>> +int pcie_get_relaxed_ordering(struct pci_dev *dev)
>> +{
>> +    u16 v;
>> +
>> +    pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
>> +
>> +    return (v & PCI_EXP_DEVCTL_RELAX_EN) >> 4;
>> +}
>> +EXPORT_SYMBOL(pcie_get_relaxed_ordering);
>> +
>> +/**
>>   * pcie_get_minimum_link - determine minimum link settings of a PCI device
>>   * @dev: PCI device to query
>>   * @speed: storage for minimum speed
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 19c8950..0c94c80 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1701,6 +1701,48 @@ static void pci_configure_extended_tags(struct pci_dev *dev)
>>                       PCI_EXP_DEVCTL_EXT_TAG);
>>  }
>>
>> +/**
>> + * pci_dev_disable_relaxed_ordering - check if the PCI device
>> + * should disable the relaxed ordering attribute.
> 
> I think that we need a more accurate description. I know some people think a function which just "checks" is vague.
> 
>> + * @dev: PCI device
>> + *
>> + * Return true if any of the PCI devices above us do not support
>> + * relaxed ordering.
>> + */
>> +static int pci_dev_disable_relaxed_ordering(struct pci_dev *dev)
> 
> The function name implies an action - disabling - but this function does nothing except return a value
> 
>> +{
>> +    int ro_disabled = 0;
>> +
>> +    while(dev) {
> 
> Did you run checkpatch?
> 
>> +        if (dev->dev_flags & PCI_DEV_FLAGS_NO_RELAXED_ORDERING) {
>> +            ro_disabled = 1;
> 
> just return true, and return false at the bottom, so you can do away with ro_disabled (which is not a bool)
> 
>> +            break;
>> +        }
>> +        dev = dev->bus->self;
>> +    }
>> +
>> +    return ro_disabled;
>> +}
>> +
>> +static void pci_configure_relaxed_ordering(struct pci_dev *dev)
>> +{
>> +    struct pci_dev *bridge = pci_upstream_bridge(dev);
>> +    int origin_ero;
> 
> You don't need this variable
> 
>> +
>> +    if (!pci_is_pcie(dev) || !bridge || !pci_is_pcie(bridge))
>> +        return;
>> +
>> +    origin_ero = pcie_get_relaxed_ordering(dev);
>> +    /* If the releaxed ordering enable bit is not set, do nothing. */
>> +    if (!origin_ero)
>> +        return;
>> +
>> +    if (pci_dev_disable_relaxed_ordering(dev)) {
>> +        pcie_clear_relaxed_ordering(dev);
>> +        dev_info(&dev->dev, "Disable Relaxed Ordering\n");
>> +    }
>> +}
>> +
>>  static void pci_configure_device(struct pci_dev *dev)
>>  {
>>      struct hotplug_params hpp;
>> @@ -1708,6 +1750,7 @@ static void pci_configure_device(struct pci_dev *dev)
>>
>>      pci_configure_mps(dev);
>>      pci_configure_extended_tags(dev);
>> +    pci_configure_relaxed_ordering(dev);
>>
>>      memset(&hpp, 0, sizeof(hpp));
>>      ret = pci_get_hp_params(dev, &hpp);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index e1e8428..299d2f3 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1105,6 +1105,8 @@ int __pci_enable_wake(struct pci_dev *dev, pci_power_t state,
>>  void pci_pme_wakeup_bus(struct pci_bus *bus);
>>  void pci_d3cold_enable(struct pci_dev *dev);
>>  void pci_d3cold_disable(struct pci_dev *dev);
>> +int pcie_clear_relaxed_ordering(struct pci_dev *dev);
>> +int pcie_get_relaxed_ordering(struct pci_dev *dev);
>>
>>  static inline int pci_enable_wake(struct pci_dev *dev, pci_power_t state,
>>                    bool enable)
>>
> 
> 
> 
> .
> 

^ permalink raw reply

* Re: [PATCH 07/11] wireless: ipw2x00: convert to use DRIVER_ATTR_RW
From: Kalle Valo @ 2017-06-09  9:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Stanislav Yakovlev, linux-wireless, netdev
In-Reply-To: <20170609090314.13991-7-gregkh@linuxfoundation.org>

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> We are trying to get rid of DRIVER_ATTR(), and the ipw2x00 driver's
> attributes can be trivially changed to use DRIVER_ATTR_RW().
>
> Cc: Stanislav Yakovlev <stas.yakovlev@gmail.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: <linux-wireless@vger.kernel.org>
> Cc: <netdev@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Looks good to me. I assume that I should take this, but let me know if
you are planning to push this via your trees instead. Either way is fine
for me, I'm not expecting any conflicts here.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH 07/11] wireless: ipw2x00: convert to use DRIVER_ATTR_RW
From: Greg Kroah-Hartman @ 2017-06-09  9:34 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-kernel, Stanislav Yakovlev, linux-wireless, netdev
In-Reply-To: <87h8zp4inb.fsf@codeaurora.org>

On Fri, Jun 09, 2017 at 12:25:44PM +0300, Kalle Valo wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> 
> > We are trying to get rid of DRIVER_ATTR(), and the ipw2x00 driver's
> > attributes can be trivially changed to use DRIVER_ATTR_RW().
> >
> > Cc: Stanislav Yakovlev <stas.yakovlev@gmail.com>
> > Cc: Kalle Valo <kvalo@codeaurora.org>
> > Cc: <linux-wireless@vger.kernel.org>
> > Cc: <netdev@vger.kernel.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Looks good to me. I assume that I should take this, but let me know if
> you are planning to push this via your trees instead. Either way is fine
> for me, I'm not expecting any conflicts here.

I can take it, thanks!

greg k-h

^ permalink raw reply

* [PATCH v4.11 -stable] esp4: Fix udpencap for local TCP packets.
From: Steffen Klassert @ 2017-06-09  9:35 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Locally generated TCP packets are usually cloned, so we
do skb_cow_data() on this packets. After that we need to
reload the pointer to the esp header. On udpencap this
header has an offset to skb_transport_header, so take this
offset into account.

This is a backport of:
commit 0e78a87306a ("esp4: Fix udpencap for local TCP packets.")

Fixes: 67d349ed603 ("net/esp4: Fix invalid esph pointer crash")
Fixes: fca11ebde3f0 ("esp4: Reorganize esp_output")
Reported-by: Don Bowman <db@donbowman.ca>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index b1e2444..9708a32 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -212,6 +212,7 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 	u8 *iv;
 	u8 *tail;
 	u8 *vaddr;
+	int esph_offset;
 	int blksize;
 	int clen;
 	int alen;
@@ -392,12 +393,14 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 	}
 
 cow:
+	esph_offset = (unsigned char *)esph - skb_transport_header(skb);
+
 	err = skb_cow_data(skb, tailen, &trailer);
 	if (err < 0)
 		goto error;
 	nfrags = err;
 	tail = skb_tail_pointer(trailer);
-	esph = ip_esp_hdr(skb);
+	esph = (struct ip_esp_hdr *)(skb_transport_header(skb) + esph_offset);
 
 skip_cow:
 	esp_output_fill_trailer(tail, tfclen, plen, proto);
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net] net/flow: fix fc->percpu NULL pointer dereference
From: Xin Long @ 2017-06-09  9:49 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: Steffen Klassert, David Miller, network dev
In-Reply-To: <CAPwn2JR8MUauO44qAeb3tw93PuQHEbO_8Tocs-ZW8Y9ZK+Ln8A@mail.gmail.com>

On Fri, Jun 9, 2017 at 5:06 PM, Hangbin Liu <liuhangbin@gmail.com> wrote:
> 2017-06-09 16:43 GMT+08:00 Xin Long <lucien.xin@gmail.com>:
>> On Fri, Jun 9, 2017 at 4:32 PM, Steffen Klassert
>> <steffen.klassert@secunet.com> wrote:
>>> On Fri, Jun 09, 2017 at 04:23:01PM +0800, Hangbin Liu wrote:
>>>> Hi Steffen,
>>>>
>>>> BTW, If we put the check in xfrm_policy_flush(), we can prevent it earlier.
>>>> But If we put the check in flow_cache_percpu_empty(), we can prevent
>>>> other functions set fc->percpu to NULL, although not much possible : )
>>>>
>>>> So I'm not quite sure whether we should put the check in
>>>> flow_cache_percpu_empty() or in xfrm_policy_flush().
>>>
>>> Can't we just call xfrm_policy_fini() first and then flow_cache_fini()?
>
> Yes, that would be easy fix. I have been thinking about that.  But if we change
>  the order in xfrm_net_exit(), do we also need to change the order in
>  xfrm_net_init()? That would change a lot.
>
> If no need, that would be good.
>
>>>
>> That would be a better fix. seems safe as what flow_cache_fini does
>> is only to free fcp->hash_table and stop timer, I didn't see  it has
>> any dependence on xfrm_policy stuff.
>
> I'm not familiar about this part. So not sure about the influence if we free
> the flow cache after xfrm_policy_fini(). I need do some test first.
>
> I would also be appreciate if you or some one could make sure if it doesn't
> influence anything.
>
another fix is to move xfrm_garbage_collect out of xfrm_policy_flush.
I could only see two places need to call it.
something like:

--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2755,6 +2755,8 @@ static int pfkey_spdflush(struct sock *sk,
struct sk_buff *skb, const struct sad
        int err, err2;

        err = xfrm_policy_flush(net, XFRM_POLICY_TYPE_MAIN, true);
+       if (!err)
+               xfrm_garbage_collect(net);
        err2 = unicast_flush_resp(sk, hdr);
        if (err || err2) {
                if (err == -ESRCH) /* empty table - old silent behavior */
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index ed4e52d..89343a3 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1007,9 +1007,6 @@ int xfrm_policy_flush(struct net *net, u8 type,
bool task_valid)
 out:
        spin_unlock_bh(&net->xfrm.xfrm_policy_lock);

-       if (cnt)
-               xfrm_garbage_collect(net);
-
        return err;
 }
 EXPORT_SYMBOL(xfrm_policy_flush);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 38614df..86116e9 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2027,6 +2027,7 @@ static int xfrm_flush_policy(struct sk_buff
*skb, struct nlmsghdr *nlh,
                        return 0;
                return err;
        }
+       xfrm_garbage_collect(net);

^ 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