* RE: [PATCH net-next v4 1/4] devlink: Refactor physical port attributes
From: Parav Pandit @ 2019-07-08 4:34 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev@vger.kernel.org, Jiri Pirko, Saeed Mahameed,
jakub.kicinski@netronome.com
In-Reply-To: <20190707194750.GA2306@nanopsycho.orion>
> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Monday, July 8, 2019 1:18 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: netdev@vger.kernel.org; Jiri Pirko <jiri@mellanox.com>; Saeed
> Mahameed <saeedm@mellanox.com>; jakub.kicinski@netronome.com
> Subject: Re: [PATCH net-next v4 1/4] devlink: Refactor physical port
> attributes
>
> Sat, Jul 06, 2019 at 08:23:47PM CEST, parav@mellanox.com wrote:
> >To support additional devlink port flavours and to support few common
> >and few different port attributes, make following changes.
> >
> >1. Move physical port attributes to a different structure 2. Return
> >such attritubes in netlink response only for physical ports (PHYSICAL,
> >CPU and DSA)
>
> 2 changes, 2 patches please.
Done in v5.
^ permalink raw reply
* Re: [PATCH] phy: added a PHY_BUSY state into phy_state_machine
From: Andrew Lunn @ 2019-07-08 4:42 UTC (permalink / raw)
To: kwangdo.yi; +Cc: netdev
In-Reply-To: <1562538732-20700-1-git-send-email-kwangdo.yi@gmail.com>
On Sun, Jul 07, 2019 at 06:32:12PM -0400, kwangdo.yi wrote:
> When mdio driver polling the phy state in the phy_state_machine,
> sometimes it results in -ETIMEDOUT and link is down. But the phy
> is still alive and just didn't meet the polling deadline.
> Closing the phy link in this case seems too radical. Failing to
> meet the deadline happens very rarely. When stress test runs for
> tens of hours with multiple target boards (Xilinx Zynq7000 with
> marvell 88E1512 PHY, Xilinx custom emac IP), it happens. This
> patch gives another chance to the phy_state_machine when polling
> timeout happens. Only two consecutive failing the deadline is
> treated as the real phy halt and close the connection.
Hi Kwangdo
I agree with Florian here. This does not seem like a PHY problem. It
is an MDIO bus problem. ETIMEDOUT is only returned from
xemaclite_mdio_wait().
What value are using for HZ? If you have 1000, jiffies + 2 could well
be too short.
Andrew
^ permalink raw reply
* [PATCH 1/2] e1000e: add workaround for possible stalled packet
From: Kai-Heng Feng @ 2019-07-08 4:55 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: sasha.neftin, intel-wired-lan, netdev, linux-kernel,
Kai-Heng Feng
Forwardport from http://mails.dpdk.org/archives/dev/2016-November/050657.html
This works around a possible stalled packet issue, which may occur due to
clock recovery from the PCH being too slow, when the LAN is transitioning
from K1 at 1G link speed.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204057
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 10 ++++++++++
drivers/net/ethernet/intel/e1000e/ich8lan.h | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 395b05701480..56f88a4e538c 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -1429,6 +1429,16 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
else
phy_reg |= 0xFA;
e1e_wphy_locked(hw, I217_PLL_CLOCK_GATE_REG, phy_reg);
+
+ if (speed == SPEED_1000) {
+ hw->phy.ops.read_reg_locked(hw, HV_PM_CTRL,
+ &phy_reg);
+
+ phy_reg |= HV_PM_CTRL_K1_CLK_REQ;
+
+ hw->phy.ops.write_reg_locked(hw, HV_PM_CTRL,
+ phy_reg);
+ }
}
hw->phy.ops.release(hw);
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.h b/drivers/net/ethernet/intel/e1000e/ich8lan.h
index eb09c755fa17..1502895eb45d 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.h
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.h
@@ -210,7 +210,7 @@
/* PHY Power Management Control */
#define HV_PM_CTRL PHY_REG(770, 17)
-#define HV_PM_CTRL_PLL_STOP_IN_K1_GIGA 0x100
+#define HV_PM_CTRL_K1_CLK_REQ 0x200
#define HV_PM_CTRL_K1_ENABLE 0x4000
#define I217_PLL_CLOCK_GATE_REG PHY_REG(772, 28)
--
2.17.1
^ permalink raw reply related
* [PATCH 2/2] e1000e: disable force K1-off feature
From: Kai-Heng Feng @ 2019-07-08 4:55 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: sasha.neftin, intel-wired-lan, netdev, linux-kernel,
Kai-Heng Feng
In-Reply-To: <20190708045546.30160-1-kai.heng.feng@canonical.com>
Forwardport from http://mails.dpdk.org/archives/dev/2016-November/050658.html
MAC-PHY desync may occur causing misdetection of link up event.
Disabling K1-off feature can work around the problem.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204057
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/net/ethernet/intel/e1000e/hw.h | 1 +
drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000e/hw.h b/drivers/net/ethernet/intel/e1000e/hw.h
index eff75bd8a8f0..e3c71fd093ee 100644
--- a/drivers/net/ethernet/intel/e1000e/hw.h
+++ b/drivers/net/ethernet/intel/e1000e/hw.h
@@ -662,6 +662,7 @@ struct e1000_dev_spec_ich8lan {
bool kmrn_lock_loss_workaround_enabled;
struct e1000_shadow_ram shadow_ram[E1000_ICH8_SHADOW_RAM_WORDS];
bool nvm_k1_enabled;
+ bool disable_k1_off;
bool eee_disable;
u16 eee_lp_ability;
enum e1000_ulp_state ulp_state;
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 56f88a4e538c..c1e0e03dc5cb 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -1538,6 +1538,9 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
fextnvm6 &= ~E1000_FEXTNVM6_K1_OFF_ENABLE;
}
+ if (hw->dev_spec.ich8lan.disable_k1_off == true)
+ fextnvm6 &= ~E1000_FEXTNVM6_K1_OFF_ENABLE;
+
ew32(FEXTNVM6, fextnvm6);
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next] hinic: add fw version query
From: David Miller @ 2019-07-08 5:15 UTC (permalink / raw)
To: xuechaojing
Cc: linux-kernel, netdev, luoshaokai, cloud.wangxiaoyun, chiqijun,
wulike1
In-Reply-To: <20190705024028.5768-1-xuechaojing@huawei.com>
From: Xue Chaojing <xuechaojing@huawei.com>
Date: Fri, 5 Jul 2019 02:40:28 +0000
> This patch adds firmware version query in ethtool -i.
>
> Signed-off-by: Xue Chaojing <xuechaojing@huawei.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] net: bpfilter: print umh messages to /dev/kmsg
From: David Miller @ 2019-07-08 5:17 UTC (permalink / raw)
To: GLin; +Cc: netdev, netfilter-devel, ast, daniel, FVogt
In-Reply-To: <20190705035357.3995-1-glin@suse.com>
From: Gary Lin <GLin@suse.com>
Date: Fri, 5 Jul 2019 03:54:58 +0000
> bpfilter_umh currently printed all messages to /dev/console and this
> might interfere the user activity(*).
>
> This commit changes the output device to /dev/kmsg so that the messages
> from bpfilter_umh won't show on the console directly.
>
> (*) https://bugzilla.suse.com/show_bug.cgi?id=1140221
>
> Signed-off-by: Gary Lin <glin@suse.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: hisilicon: Add an tx_desc to adapt HI13X1_GMAC
From: David Miller @ 2019-07-08 5:18 UTC (permalink / raw)
To: xiaojiangfeng
Cc: yisen.zhuang, salil.mehta, dingtianhong, robh+dt, mark.rutland,
netdev, devicetree, linux-kernel, leeyou.li, xiekunxun,
jianping.liu, nixiaoming
In-Reply-To: <1562307003-103516-1-git-send-email-xiaojiangfeng@huawei.com>
From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
Date: Fri, 5 Jul 2019 14:10:03 +0800
> HI13X1 changed the offsets and bitmaps for tx_desc
> registers in the same peripheral device on different
> models of the hip04_eth.
>
> Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH] phy: added a PHY_BUSY state into phy_state_machine
From: Heiner Kallweit @ 2019-07-08 6:03 UTC (permalink / raw)
To: Florian Fainelli, kwangdo.yi, netdev, Andrew Lunn
In-Reply-To: <539888f4-e5be-7ad5-53ce-63dd182708b1@gmail.com>
On 08.07.2019 05:07, Florian Fainelli wrote:
> +Andrew, Heiner (please CC PHY library maintainers).
>
> On 7/7/2019 3:32 PM, kwangdo.yi wrote:
>> When mdio driver polling the phy state in the phy_state_machine,
>> sometimes it results in -ETIMEDOUT and link is down. But the phy
>> is still alive and just didn't meet the polling deadline.
>> Closing the phy link in this case seems too radical. Failing to
>> meet the deadline happens very rarely. When stress test runs for
>> tens of hours with multiple target boards (Xilinx Zynq7000 with
>> marvell 88E1512 PHY, Xilinx custom emac IP), it happens. This
>> patch gives another chance to the phy_state_machine when polling
>> timeout happens. Only two consecutive failing the deadline is
>> treated as the real phy halt and close the connection.
>
In addition to what Florian said already there's at least one
issue apart from the quite hacky approach in general.
Let's say we are in interrupt mode and the timeout happens when
reading the PHY status after a link-down interrupt. When ignoring
the error we miss the transition and phylib will report a wrong
link status.
I also would prefer to first check for the root cause and try to
fix it, before adding hacks to upper layers for ignoring errors.
> How about simply increasing the MDIO polling timeout in the Xilinx EMAC
> driver instead? Or if the PHY is where the timeout needs to be
> increased, allow the PHY device drivers to advertise min/max timeouts
> such that the MDIO bus layer can use that information?
>
>>
>>
>> Signed-off-by: kwangdo.yi <kwangdo.yi@gmail.com>
>> ---
>> drivers/net/phy/phy.c | 6 ++++++
>> include/linux/phy.h | 1 +
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index e888542..9e8138b 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -919,7 +919,13 @@ void phy_state_machine(struct work_struct *work)
>> break;
>> case PHY_NOLINK:
>> case PHY_RUNNING:
>> + case PHY_BUSY:
>> err = phy_check_link_status(phydev);
>> + if (err == -ETIMEDOUT && old_state == PHY_RUNNING) {
>> + phy->state = PHY_BUSY;
>> + err = 0;
>> +
>> + }
>> break;
>> case PHY_FORCING:
>> err = genphy_update_link(phydev);
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 6424586..4a49401 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -313,6 +313,7 @@ enum phy_state {
>> PHY_RUNNING,
>> PHY_NOLINK,
>> PHY_FORCING,
>> + PHY_BUSY,
>> };
>>
>> /**
>>
>
^ permalink raw reply
* Re: [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Tiwei Bie @ 2019-07-08 6:16 UTC (permalink / raw)
To: Alex Williamson
Cc: Jason Wang, mst, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang
In-Reply-To: <20190705084946.67b8f9f5@x1.home>
On Fri, Jul 05, 2019 at 08:49:46AM -0600, Alex Williamson wrote:
> On Thu, 4 Jul 2019 14:21:34 +0800
> Tiwei Bie <tiwei.bie@intel.com> wrote:
> > On Thu, Jul 04, 2019 at 12:31:48PM +0800, Jason Wang wrote:
> > > On 2019/7/3 下午9:08, Tiwei Bie wrote:
> > > > On Wed, Jul 03, 2019 at 08:16:23PM +0800, Jason Wang wrote:
> > > > > On 2019/7/3 下午7:52, Tiwei Bie wrote:
> > > > > > On Wed, Jul 03, 2019 at 06:09:51PM +0800, Jason Wang wrote:
> > > > > > > On 2019/7/3 下午5:13, Tiwei Bie wrote:
> > > > > > > > Details about this can be found here:
> > > > > > > >
> > > > > > > > https://lwn.net/Articles/750770/
> > > > > > > >
> > > > > > > > What's new in this version
> > > > > > > > ==========================
> > > > > > > >
> > > > > > > > A new VFIO device type is introduced - vfio-vhost. This addressed
> > > > > > > > some comments from here:https://patchwork.ozlabs.org/cover/984763/
> > > > > > > >
> > > > > > > > Below is the updated device interface:
> > > > > > > >
> > > > > > > > Currently, there are two regions of this device: 1) CONFIG_REGION
> > > > > > > > (VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
> > > > > > > > device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
> > > > > > > > can be used to notify the device.
> > > > > > > >
> > > > > > > > 1. CONFIG_REGION
> > > > > > > >
> > > > > > > > The region described by CONFIG_REGION is the main control interface.
> > > > > > > > Messages will be written to or read from this region.
> > > > > > > >
> > > > > > > > The message type is determined by the `request` field in message
> > > > > > > > header. The message size is encoded in the message header too.
> > > > > > > > The message format looks like this:
> > > > > > > >
> > > > > > > > struct vhost_vfio_op {
> > > > > > > > __u64 request;
> > > > > > > > __u32 flags;
> > > > > > > > /* Flag values: */
> > > > > > > > #define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
> > > > > > > > __u32 size;
> > > > > > > > union {
> > > > > > > > __u64 u64;
> > > > > > > > struct vhost_vring_state state;
> > > > > > > > struct vhost_vring_addr addr;
> > > > > > > > } payload;
> > > > > > > > };
> > > > > > > >
> > > > > > > > The existing vhost-kernel ioctl cmds are reused as the message
> > > > > > > > requests in above structure.
> > > > > > > Still a comments like V1. What's the advantage of inventing a new protocol?
> > > > > > I'm trying to make it work in VFIO's way..
> > > > > >
> > > > > > > I believe either of the following should be better:
> > > > > > >
> > > > > > > - using vhost ioctl, we can start from SET_VRING_KICK/SET_VRING_CALL and
> > > > > > > extend it with e.g notify region. The advantages is that all exist userspace
> > > > > > > program could be reused without modification (or minimal modification). And
> > > > > > > vhost API hides lots of details that is not necessary to be understood by
> > > > > > > application (e.g in the case of container).
> > > > > > Do you mean reusing vhost's ioctl on VFIO device fd directly,
> > > > > > or introducing another mdev driver (i.e. vhost_mdev instead of
> > > > > > using the existing vfio_mdev) for mdev device?
> > > > > Can we simply add them into ioctl of mdev_parent_ops?
> > > > Right, either way, these ioctls have to be and just need to be
> > > > added in the ioctl of the mdev_parent_ops. But another thing we
> > > > also need to consider is that which file descriptor the userspace
> > > > will do the ioctl() on. So I'm wondering do you mean let the
> > > > userspace do the ioctl() on the VFIO device fd of the mdev
> > > > device?
> > > >
> > >
> > > Yes.
> >
> > Got it! I'm not sure what's Alex opinion on this. If we all
> > agree with this, I can do it in this way.
> >
> > > Is there any other way btw?
> >
> > Just a quick thought.. Maybe totally a bad idea. I was thinking
> > whether it would be odd to do non-VFIO's ioctls on VFIO's device
> > fd. So I was wondering whether it's possible to allow binding
> > another mdev driver (e.g. vhost_mdev) to the supported mdev
> > devices. The new mdev driver, vhost_mdev, can provide similar
> > ways to let userspace open the mdev device and do the vhost ioctls
> > on it. To distinguish with the vfio_mdev compatible mdev devices,
> > the device API of the new vhost_mdev compatible mdev devices
> > might be e.g. "vhost-net" for net?
> >
> > So in VFIO case, the device will be for passthru directly. And
> > in VHOST case, the device can be used to accelerate the existing
> > virtualized devices.
> >
> > How do you think?
>
> VFIO really can't prevent vendor specific ioctls on the device file
> descriptor for mdevs, but a) we'd want to be sure the ioctl address
> space can't collide with ioctls we'd use for vfio defined purposes and
> b) maybe the VFIO user API isn't what you want in the first place if
> you intend to mostly/entirely ignore the defined ioctl set and replace
> them with your own. In the case of the latter, you're also not getting
> the advantages of the existing VFIO userspace code, so why expose a
> VFIO device at all.
Yeah, I totally agree.
>
> The mdev interface does provide a general interface for creating and
> managing virtual devices, vfio-mdev is just one driver on the mdev
> bus. Parav (Mellanox) has been doing work on mdev-core to help clean
> out vfio-isms from the interface, aiui, with the intent of implementing
> another mdev bus driver for using the devices within the kernel.
Great to know this! I found below series after some searching:
https://lkml.org/lkml/2019/3/8/821
In above series, the new mlx5_core mdev driver will do the probe
by calling mlx5_get_core_dev() first on the parent device of the
mdev device. In vhost_mdev, maybe we can also keep track of all
the compatible mdev devices and use this info to do the probe.
But we also need a way to allow vfio_mdev driver to distinguish
and reject the incompatible mdev devices.
> It
> seems like this vhost-mdev driver might be similar, using mdev but not
> necessarily vfio-mdev to expose devices. Thanks,
Yeah, I also think so!
Thanks!
Tiwei
>
> Alex
^ permalink raw reply
* [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-08 6:32 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller
Cc: linux-wireless, netdev, linux-kernel, linux, Jian-Hong Pan,
Daniel Drake, stable
Testing with RTL8822BE hardware, when available memory is low, we
frequently see a kernel panic and system freeze.
First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
rx routine starvation
WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822 rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
[ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
Then we see a variety of different error conditions and kernel panics,
such as this one (trimmed):
rtw_pci 0000:02:00.0: pci bus timeout, check dma status
skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:105!
invalid opcode: 0000 [#1] SMP NOPTI
RIP: 0010:skb_panic+0x43/0x45
When skb allocation fails and the "rx routine starvation" is hit, the
function returns immediately without updating the RX ring. At this
point, the RX ring may continue referencing an old skb which was already
handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
bad things happen.
This patch allocates a new skb first in RX ISR. If we don't have memory
available, we discard the current frame, allowing the existing skb to be
reused in the ring. Otherwise, we simplify the code flow and just hand
over the RX-populated skb over to mac80211.
In addition, to fixing the kernel crash, the RX routine should now
generally behave better under low memory conditions.
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Reviewed-by: Daniel Drake <drake@endlessm.com>
Cc: <stable@vger.kernel.org>
---
drivers/net/wireless/realtek/rtw88/pci.c | 28 +++++++++++-------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index cfe05ba7280d..1bfc99ae6b84 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
rx_desc = skb->data;
chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
+ /* discard current skb if the new skb cannot be allocated as a
+ * new one in rx ring later
+ * */
+ new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
+ if (WARN(!new, "rx routine starvation\n")) {
+ new = skb;
+ goto next_rp;
+ }
+
/* offset from rx_desc to payload */
pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
pkt_stat.shift;
@@ -803,25 +812,14 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
skb_put(skb, pkt_stat.pkt_len);
skb_reserve(skb, pkt_offset);
- /* alloc a smaller skb to mac80211 */
- new = dev_alloc_skb(pkt_stat.pkt_len);
- if (!new) {
- new = skb;
- } else {
- skb_put_data(new, skb->data, skb->len);
- dev_kfree_skb_any(skb);
- }
/* TODO: merge into rx.c */
rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
- memcpy(new->cb, &rx_status, sizeof(rx_status));
- ieee80211_rx_irqsafe(rtwdev->hw, new);
+ memcpy(skb->cb, &rx_status, sizeof(rx_status));
+ ieee80211_rx_irqsafe(rtwdev->hw, skb);
}
- /* skb delivered to mac80211, alloc a new one in rx ring */
- new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
- if (WARN(!new, "rx routine starvation\n"))
- return;
-
+next_rp:
+ /* skb delivered to mac80211, attach the new one into rx ring */
ring->buf[cur_rp] = new;
rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
--
2.22.0
^ permalink raw reply related
* [PATCH] r8169: add enable_aspm parameter
From: AceLan Kao @ 2019-07-08 6:37 UTC (permalink / raw)
To: Realtek linux nic maintainers, Heiner Kallweit, David S. Miller,
netdev, linux-kernel
We have many commits in the driver which enable and then disable ASPM
function over and over again.
commit b75bb8a5b755 ("r8169: disable ASPM again")
commit 0866cd15029b ("r8169: enable ASPM on RTL8106E")
commit 94235460f9ea ("r8169: Align ASPM/CLKREQ setting function with vendor driver")
commit aa1e7d2c31ef ("r8169: enable ASPM on RTL8168E-VL")
commit f37658da21aa ("r8169: align ASPM entry latency setting with vendor driver")
commit a99790bf5c7f ("r8169: Reinstate ASPM Support")
commit 671646c151d4 ("r8169: Don't disable ASPM in the driver")
commit 4521e1a94279 ("Revert "r8169: enable internal ASPM and clock request settings".")
commit d64ec841517a ("r8169: enable internal ASPM and clock request settings")
This function is very important for production, and if we can't come out
a solution to make both happy, I'd suggest we add a parameter in the
driver to toggle it.
Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
drivers/net/ethernet/realtek/r8169.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index d06a61f00e78..f557cb36e2c6 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -702,10 +702,13 @@ struct rtl8169_private {
typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
+static int enable_aspm;
MODULE_AUTHOR("Realtek and the Linux r8169 crew <netdev@vger.kernel.org>");
MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver");
module_param_named(debug, debug.msg_enable, int, 0);
MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., 16=all)");
+module_param(enable_aspm, int, 0);
+MODULE_PARM_DESC(enable_aspm, "Enable ASPM support (0 = disable, 1 = enable");
MODULE_SOFTDEP("pre: realtek");
MODULE_LICENSE("GPL");
MODULE_FIRMWARE(FIRMWARE_8168D_1);
@@ -7163,10 +7166,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (rc)
return rc;
- /* Disable ASPM completely as that cause random device stop working
- * problems as well as full system hangs for some PCIe devices users.
- */
- pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
+ if (!enable_aspm) {
+ /* Disable ASPM completely as that cause random device stop working
+ * problems as well as full system hangs for some PCIe devices users.
+ */
+ pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
+ }
/* enable device (incl. PCI PM wakeup and hotplug setup) */
rc = pcim_enable_device(pdev);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v4 1/4] net/sched: Introduce action ct
From: Paul Blakey @ 2019-07-08 7:07 UTC (permalink / raw)
To: Florian Westphal
Cc: Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, netdev@vger.kernel.org, David Miller,
Aaron Conole, Zhike Wang, Rony Efraim, nst-kernel@redhat.com,
John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <20190707120455.6li4tfb5ppht4xy7@breakpoint.cc>
On 7/7/2019 3:04 PM, Florian Westphal wrote:
> Paul Blakey <paulb@mellanox.com> wrote:
>> +/* Determine whether skb->_nfct is equal to the result of conntrack lookup. */
>> +static bool tcf_ct_skb_nfct_cached(struct net *net, struct sk_buff *skb,
>> + u16 zone_id, bool force)
>> +{
>> + enum ip_conntrack_info ctinfo;
>> + struct nf_conn *ct;
>> +
>> + ct = nf_ct_get(skb, &ctinfo);
>> + if (!ct)
>> + return false;
>> + if (!net_eq(net, read_pnet(&ct->ct_net)))
>> + return false;
>> + if (nf_ct_zone(ct)->id != zone_id)
>> + return false;
>> +
>> + /* Force conntrack entry direction. */
>> + if (force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
>> + nf_conntrack_put(&ct->ct_general);
>> + nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
>> +
>> + if (nf_ct_is_confirmed(ct))
>> + nf_ct_kill(ct);
> This looks like a possible UAF:
> nf_conntrack_put() may free the conntrack entry.
>
> It seems better to do do:
> if (nf_ct_is_confirmed(ct))
> nf_ct_kill(ct);
>
> nf_conntrack_put(&ct->ct_general);
> nf_ct_set(skb, ...
Like if conntrack has just timed it out (or conntrack flushed), and skb
holds the last ref?
thanks, will reverse the order.
^ permalink raw reply
* general protection fault in send_hsr_supervision_frame
From: syzbot @ 2019-07-08 7:17 UTC (permalink / raw)
To: arvid.brodin, davem, linux-kernel, netdev, syzkaller-bugs,
xiyou.wangcong
Hello,
syzbot found the following crash on:
HEAD commit: 537de0c8 ipv4: Fix NULL pointer dereference in ipv4_neigh_..
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=16d2af63a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=90f5d2d9c1e7421c
dashboard link: https://syzkaller.appspot.com/bug?extid=097ef84cdc95843fbaa8
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14a9361da00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10a4c753a00000
The bug was bisected to:
commit b9a1e627405d68d475a3c1f35e685ccfb5bbe668
Author: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu Jul 4 00:21:13 2019 +0000
hsr: implement dellink to clean up resources
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=10b86c77a00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=12b86c77a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=14b86c77a00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+097ef84cdc95843fbaa8@syzkaller.appspotmail.com
Fixes: b9a1e627405d ("hsr: implement dellink to clean up resources")
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 10432 Comm: syz-executor357 Not tainted 5.2.0-rc6+ #76
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:send_hsr_supervision_frame+0x38/0xf20 net/hsr/hsr_device.c:255
Code: 89 fd 41 54 53 48 83 ec 50 89 75 bc e8 81 d2 5c fa 49 8d 45 10 48 89
c2 48 89 45 d0 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80> 3c 02 00 0f
85 dc 0c 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b
RSP: 0018:ffff8880ae809c50 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff871403d7
RDX: 0000000000000002 RSI: ffffffff8713f08f RDI: 0000000000000000
RBP: ffff8880ae809cc8 R08: ffff88809e014600 R09: ffffed1015d06c70
R10: ffffed1015d06c6f R11: ffff8880ae83637b R12: ffff888097eff000
R13: 0000000000000000 R14: 0000000000000000 R15: dffffc0000000000
FS: 00007f85a5cd2700(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000006de0a0 CR3: 00000000a1151000 CR4: 00000000001406f0
Call Trace:
<IRQ>
hsr_announce+0x12f/0x3b0 net/hsr/hsr_device.c:339
call_timer_fn+0x193/0x720 kernel/time/timer.c:1322
expire_timers kernel/time/timer.c:1366 [inline]
__run_timers kernel/time/timer.c:1685 [inline]
__run_timers kernel/time/timer.c:1653 [inline]
run_timer_softirq+0x66f/0x1740 kernel/time/timer.c:1698
__do_softirq+0x25c/0x94c kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x180/0x1d0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:536 [inline]
smp_apic_timer_interrupt+0x13b/0x550 arch/x86/kernel/apic/apic.c:1068
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:806
</IRQ>
RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:767
[inline]
RIP: 0010:lock_is_held_type+0x272/0x320 kernel/locking/lockdep.c:4343
Code: ff df c7 83 7c 08 00 00 00 00 00 00 48 c1 e8 03 80 3c 10 00 0f 85 88
00 00 00 48 83 3d 86 a0 5b 07 00 74 31 48 8b 7d c0 57 9d <0f> 1f 44 00 00
48 83 c4 20 44 89 e0 5b 41 5c 41 5d 41 5e 41 5f 5d
RSP: 0018:ffff8880a13f71e8 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
RAX: 1ffffffff1164e7e RBX: ffff88809e014600 RCX: ffff88809e014e80
RDX: dffffc0000000000 RSI: ffffffff88ba3700 RDI: 0000000000000286
RBP: ffff8880a13f7230 R08: ffff88809e014600 R09: ffffed1015d06c70
R10: ffffed1015d06c6f R11: ffff8880ae83637b R12: 0000000000000001
R13: ffff88809e014ef8 R14: ffffffff88ba3700 R15: 0000000000000003
lock_is_held include/linux/lockdep.h:356 [inline]
rcu_read_lock_held kernel/rcu/update.c:270 [inline]
rcu_read_lock_held+0xa3/0xd0 kernel/rcu/update.c:262
xa_head include/linux/xarray.h:1128 [inline]
xas_start+0x1ce/0x560 lib/xarray.c:187
xas_load+0x21/0x150 lib/xarray.c:232
find_get_entry+0x144/0x770 mm/filemap.c:1506
pagecache_get_page+0x4c/0x850 mm/filemap.c:1608
find_get_page_flags include/linux/pagemap.h:266 [inline]
ext4_mb_load_buddy_gfp+0x595/0x13e0 fs/ext4/mballoc.c:1190
ext4_mb_load_buddy fs/ext4/mballoc.c:1241 [inline]
ext4_mb_regular_allocator+0x7e0/0x1260 fs/ext4/mballoc.c:2190
ext4_mb_new_blocks+0x1881/0x3c10 fs/ext4/mballoc.c:4539
ext4_ext_map_blocks+0x2b83/0x5250 fs/ext4/extents.c:4414
ext4_map_blocks+0x8c5/0x18e0 fs/ext4/inode.c:640
ext4_alloc_file_blocks+0x287/0xac0 fs/ext4/extents.c:4603
ext4_fallocate+0x8ba/0x2060 fs/ext4/extents.c:4888
vfs_fallocate+0x4aa/0xa50 fs/open.c:309
ioctl_preallocate+0x197/0x210 fs/ioctl.c:490
file_ioctl fs/ioctl.c:506 [inline]
do_vfs_ioctl+0x1170/0x1380 fs/ioctl.c:696
ksys_ioctl+0xab/0xd0 fs/ioctl.c:713
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x448e19
Code: e8 dc e6 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 7b 05 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f85a5cd1d98 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000006e5a08 RCX: 0000000000448e19
RDX: 0000000020000080 RSI: 0000000040305828 RDI: 0000000000000003
RBP: 00000000006e5a00 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006e5a0c
R13: f4a25a5f72695f65 R14: 6761000000000000 R15: 00046c7465677568
Modules linked in:
---[ end trace 1a213132b72d6860 ]---
RIP: 0010:send_hsr_supervision_frame+0x38/0xf20 net/hsr/hsr_device.c:255
Code: 89 fd 41 54 53 48 83 ec 50 89 75 bc e8 81 d2 5c fa 49 8d 45 10 48 89
c2 48 89 45 d0 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80> 3c 02 00 0f
85 dc 0c 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b
RSP: 0018:ffff8880ae809c50 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff871403d7
RDX: 0000000000000002 RSI: ffffffff8713f08f RDI: 0000000000000000
RBP: ffff8880ae809cc8 R08: ffff88809e014600 R09: ffffed1015d06c70
R10: ffffed1015d06c6f R11: ffff8880ae83637b R12: ffff888097eff000
R13: 0000000000000000 R14: 0000000000000000 R15: dffffc0000000000
FS: 00007f85a5cd2700(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000006de0a0 CR3: 00000000a1151000 CR4: 00000000001406f0
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* RE: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: Tony Chuang @ 2019-07-08 7:23 UTC (permalink / raw)
To: Jian-Hong Pan, Kalle Valo, David S . Miller
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux@endlessm.com, Daniel Drake,
stable@vger.kernel.org
In-Reply-To: <20190708063252.4756-1-jian-hong@endlessm.com>
> Subject: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
nit, "rtw88: pci:" would be better.
>
>
> When skb allocation fails and the "rx routine starvation" is hit, the
> function returns immediately without updating the RX ring. At this
> point, the RX ring may continue referencing an old skb which was already
> handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> bad things happen.
>
> This patch allocates a new skb first in RX ISR. If we don't have memory
> available, we discard the current frame, allowing the existing skb to be
> reused in the ring. Otherwise, we simplify the code flow and just hand
> over the RX-populated skb over to mac80211.
>
> In addition, to fixing the kernel crash, the RX routine should now
> generally behave better under low memory conditions.
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Reviewed-by: Daniel Drake <drake@endlessm.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/net/wireless/realtek/rtw88/pci.c | 28 +++++++++++-------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> b/drivers/net/wireless/realtek/rtw88/pci.c
> index cfe05ba7280d..1bfc99ae6b84 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> struct rtw_pci *rtwpci,
> rx_desc = skb->data;
> chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
>
> + /* discard current skb if the new skb cannot be allocated as a
> + * new one in rx ring later
> + * */
nit, comment indentation.
> + new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> + if (WARN(!new, "rx routine starvation\n")) {
> + new = skb;
> + goto next_rp;
> + }
> +
> /* offset from rx_desc to payload */
> pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> pkt_stat.shift;
> @@ -803,25 +812,14 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> struct rtw_pci *rtwpci,
> skb_put(skb, pkt_stat.pkt_len);
> skb_reserve(skb, pkt_offset);
>
> - /* alloc a smaller skb to mac80211 */
> - new = dev_alloc_skb(pkt_stat.pkt_len);
> - if (!new) {
> - new = skb;
> - } else {
> - skb_put_data(new, skb->data, skb->len);
> - dev_kfree_skb_any(skb);
> - }
I am not sure if it's fine to deliver every huge SKB to mac80211.
Because it will then be delivered to TCP/IP stack.
Hence I think either it should be tested to know if the performance
would be impacted or find out a more efficient way to send
smaller SKB to mac80211 stack.
> /* TODO: merge into rx.c */
> rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> - memcpy(new->cb, &rx_status, sizeof(rx_status));
> - ieee80211_rx_irqsafe(rtwdev->hw, new);
> + memcpy(skb->cb, &rx_status, sizeof(rx_status));
> + ieee80211_rx_irqsafe(rtwdev->hw, skb);
> }
>
> - /* skb delivered to mac80211, alloc a new one in rx ring */
> - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> - if (WARN(!new, "rx routine starvation\n"))
> - return;
> -
> +next_rp:
> + /* skb delivered to mac80211, attach the new one into rx ring */
> ring->buf[cur_rp] = new;
> rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
>
--
Yan-Hsuan
^ permalink raw reply
* [PATCH] net: netsec: Sync dma for device on buffer allocation
From: Ilias Apalodimas @ 2019-07-08 7:25 UTC (permalink / raw)
To: netdev, jaswinder.singh, davem; +Cc: Ilias Apalodimas
cd1973a9215a ("net: netsec: Sync dma for device on buffer allocation")
was merged on it's v1 instead of the v3.
Merge the proper patch version
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
drivers/net/ethernet/socionext/netsec.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index f6e261c6a059..460777449cd9 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -743,9 +743,7 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
*/
*desc_len = PAGE_SIZE - NETSEC_RX_BUF_NON_DATA;
dma_dir = page_pool_get_dma_dir(dring->page_pool);
- dma_sync_single_for_device(priv->dev,
- *dma_handle - NETSEC_RXBUF_HEADROOM,
- PAGE_SIZE, dma_dir);
+ dma_sync_single_for_device(priv->dev, *dma_handle, *desc_len, dma_dir);
return page_address(page);
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-08 8:07 UTC (permalink / raw)
To: Tony Chuang
Cc: Kalle Valo, David S . Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux@endlessm.com, Daniel Drake, stable@vger.kernel.org
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D1861A6D@RTITMBSVM04.realtek.com.tw>
Tony Chuang <yhchuang@realtek.com> 於 2019年7月8日 週一 下午3:23寫道：
>
> > Subject: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
>
> nit, "rtw88: pci:" would be better.
Ok.
> >
> > When skb allocation fails and the "rx routine starvation" is hit, the
> > function returns immediately without updating the RX ring. At this
> > point, the RX ring may continue referencing an old skb which was already
> > handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> > bad things happen.
> >
> > This patch allocates a new skb first in RX ISR. If we don't have memory
> > available, we discard the current frame, allowing the existing skb to be
> > reused in the ring. Otherwise, we simplify the code flow and just hand
> > over the RX-populated skb over to mac80211.
> >
> > In addition, to fixing the kernel crash, the RX routine should now
> > generally behave better under low memory conditions.
> >
> > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> > Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> > Reviewed-by: Daniel Drake <drake@endlessm.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > drivers/net/wireless/realtek/rtw88/pci.c | 28 +++++++++++-------------
> > 1 file changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> > b/drivers/net/wireless/realtek/rtw88/pci.c
> > index cfe05ba7280d..1bfc99ae6b84 100644
> > --- a/drivers/net/wireless/realtek/rtw88/pci.c
> > +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> > @@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> > struct rtw_pci *rtwpci,
> > rx_desc = skb->data;
> > chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
> >
> > + /* discard current skb if the new skb cannot be allocated as a
> > + * new one in rx ring later
> > + * */
>
> nit, comment indentation.
Thanks. I will fix this.
> > + new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> > + if (WARN(!new, "rx routine starvation\n")) {
> > + new = skb;
> > + goto next_rp;
> > + }
> > +
> > /* offset from rx_desc to payload */
> > pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> > pkt_stat.shift;
> > @@ -803,25 +812,14 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> > struct rtw_pci *rtwpci,
> > skb_put(skb, pkt_stat.pkt_len);
> > skb_reserve(skb, pkt_offset);
> >
> > - /* alloc a smaller skb to mac80211 */
> > - new = dev_alloc_skb(pkt_stat.pkt_len);
> > - if (!new) {
> > - new = skb;
> > - } else {
> > - skb_put_data(new, skb->data, skb->len);
> > - dev_kfree_skb_any(skb);
> > - }
>
> I am not sure if it's fine to deliver every huge SKB to mac80211.
> Because it will then be delivered to TCP/IP stack.
> Hence I think either it should be tested to know if the performance
> would be impacted or find out a more efficient way to send
> smaller SKB to mac80211 stack.
I remember network stack only processes the skb with(in) pointers
(skb->data) and the skb->len for data part. It also checks real
buffer boundary (head and end) of the skb to prevent memory overflow.
Therefore, I think using the original skb is the most efficient way.
If I misunderstand something, please point out.
> > /* TODO: merge into rx.c */
> > rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> > - memcpy(new->cb, &rx_status, sizeof(rx_status));
> > - ieee80211_rx_irqsafe(rtwdev->hw, new);
> > + memcpy(skb->cb, &rx_status, sizeof(rx_status));
> > + ieee80211_rx_irqsafe(rtwdev->hw, skb);
> > }
> >
> > - /* skb delivered to mac80211, alloc a new one in rx ring */
> > - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> > - if (WARN(!new, "rx routine starvation\n"))
> > - return;
> > -
> > +next_rp:
> > + /* skb delivered to mac80211, attach the new one into rx ring */
> > ring->buf[cur_rp] = new;
> > rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
> >
>
> --
>
> Yan-Hsuan
^ permalink raw reply
* [net-next:master 342/422] drivers/net/dsa/qca8k.c:1050:21: error: implicit declaration of function 'devm_gpiod_get_optional'; did you mean 'devm_gpio_request_one'?
From: kbuild test robot @ 2019-07-08 8:15 UTC (permalink / raw)
To: Christian Lamparter; +Cc: kbuild-all, netdev
[-- Attachment #1: Type: text/plain, Size: 3464 bytes --]
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/davem/net-next.git master
head: 61a582be1a668a0c1407a46f779965bfeff88784
commit: a653f2f538f9d3e2d1f1445f74a47bfdace85c2e [342/422] net: dsa: qca8k: introduce reset via gpio feature
config: x86_64-randconfig-s2-07081539 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-9) 7.4.0
reproduce:
git checkout a653f2f538f9d3e2d1f1445f74a47bfdace85c2e
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/net/dsa/qca8k.c: In function 'qca8k_sw_probe':
>> drivers/net/dsa/qca8k.c:1050:21: error: implicit declaration of function 'devm_gpiod_get_optional'; did you mean 'devm_gpio_request_one'? [-Werror=implicit-function-declaration]
priv->reset_gpio = devm_gpiod_get_optional(priv->dev, "reset",
^~~~~~~~~~~~~~~~~~~~~~~
devm_gpio_request_one
>> drivers/net/dsa/qca8k.c:1051:10: error: 'GPIOD_ASIS' undeclared (first use in this function); did you mean 'GPIOF_IN'?
GPIOD_ASIS);
^~~~~~~~~~
GPIOF_IN
drivers/net/dsa/qca8k.c:1051:10: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/net/dsa/qca8k.c:1056:3: error: implicit declaration of function 'gpiod_set_value_cansleep'; did you mean 'gpio_set_value_cansleep'? [-Werror=implicit-function-declaration]
gpiod_set_value_cansleep(priv->reset_gpio, 1);
^~~~~~~~~~~~~~~~~~~~~~~~
gpio_set_value_cansleep
cc1: some warnings being treated as errors
vim +1050 drivers/net/dsa/qca8k.c
1033
1034 static int
1035 qca8k_sw_probe(struct mdio_device *mdiodev)
1036 {
1037 struct qca8k_priv *priv;
1038 u32 id;
1039
1040 /* allocate the private data struct so that we can probe the switches
1041 * ID register
1042 */
1043 priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL);
1044 if (!priv)
1045 return -ENOMEM;
1046
1047 priv->bus = mdiodev->bus;
1048 priv->dev = &mdiodev->dev;
1049
> 1050 priv->reset_gpio = devm_gpiod_get_optional(priv->dev, "reset",
> 1051 GPIOD_ASIS);
1052 if (IS_ERR(priv->reset_gpio))
1053 return PTR_ERR(priv->reset_gpio);
1054
1055 if (priv->reset_gpio) {
> 1056 gpiod_set_value_cansleep(priv->reset_gpio, 1);
1057 /* The active low duration must be greater than 10 ms
1058 * and checkpatch.pl wants 20 ms.
1059 */
1060 msleep(20);
1061 gpiod_set_value_cansleep(priv->reset_gpio, 0);
1062 }
1063
1064 /* read the switches ID register */
1065 id = qca8k_read(priv, QCA8K_REG_MASK_CTRL);
1066 id >>= QCA8K_MASK_CTRL_ID_S;
1067 id &= QCA8K_MASK_CTRL_ID_M;
1068 if (id != QCA8K_ID_QCA8337)
1069 return -ENODEV;
1070
1071 priv->ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
1072 if (!priv->ds)
1073 return -ENOMEM;
1074
1075 priv->ds->priv = priv;
1076 priv->ops = qca8k_switch_ops;
1077 priv->ds->ops = &priv->ops;
1078 mutex_init(&priv->reg_mutex);
1079 dev_set_drvdata(&mdiodev->dev, priv);
1080
1081 return dsa_register_switch(priv->ds);
1082 }
1083
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31075 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2 8/8] net: mscc: PTP Hardware Clock (PHC) support
From: Antoine Tenart @ 2019-07-08 8:17 UTC (permalink / raw)
To: Richard Cochran
Cc: Antoine Tenart, davem, alexandre.belloni, UNGLinuxDriver, ralf,
paul.burton, jhogan, netdev, linux-mips, thomas.petazzoni,
allan.nielsen
In-Reply-To: <20190705220224.5i2uy4uxx5o4raaw@localhost>
Hi Richard,
On Fri, Jul 05, 2019 at 03:02:24PM -0700, Richard Cochran wrote:
> On Fri, Jul 05, 2019 at 09:52:13PM +0200, Antoine Tenart wrote:
> > +static irqreturn_t ocelot_ptp_rdy_irq_handler(int irq, void *arg)
> > +{
> > + struct ocelot *ocelot = arg;
> > +
> > + do {
>
> > + /* Check if a timestamp can be retrieved */
> > + if (!(val & SYS_PTP_STATUS_PTP_MESS_VLD))
> > + break;
>
> As in my reply on v1, I suggest adding a sanity check on this ISR's
> infinite loop.
That's a good idea. I'll fix this in v3.
> > + } while (true);
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH] tipc: ensure skb->lock is initialised
From: Eric Dumazet @ 2019-07-08 8:18 UTC (permalink / raw)
To: Chris Packham, jon.maloy, ying.xue, davem
Cc: netdev, tipc-discussion, linux-kernel
In-Reply-To: <20190707225328.15852-1-chris.packham@alliedtelesis.co.nz>
On 7/8/19 12:53 AM, Chris Packham wrote:
> tipc_named_node_up() creates a skb list. It passes the list to
> tipc_node_xmit() which has some code paths that can call
> skb_queue_purge() which relies on the list->lock being initialised.
> Ensure tipc_named_node_up() uses skb_queue_head_init() so that the lock
> is explicitly initialised.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
I would rather change the faulty skb_queue_purge() to __skb_queue_purge()
^ permalink raw reply
* Re: [PATCH net-next 3/4] bnxt_en: optimized XDP_REDIRECT support
From: Ilias Apalodimas @ 2019-07-08 8:28 UTC (permalink / raw)
To: Michael Chan; +Cc: davem, gospo, netdev, hawk, ast
In-Reply-To: <1562398578-26020-4-git-send-email-michael.chan@broadcom.com>
Thanks Andy, Michael
> + if (event & BNXT_REDIRECT_EVENT)
> + xdp_do_flush_map();
> +
> if (event & BNXT_TX_EVENT) {
> struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
> u16 prod = txr->tx_prod;
> @@ -2254,9 +2257,23 @@ static void bnxt_free_tx_skbs(struct bnxt *bp)
>
> for (j = 0; j < max_idx;) {
> struct bnxt_sw_tx_bd *tx_buf = &txr->tx_buf_ring[j];
> - struct sk_buff *skb = tx_buf->skb;
> + struct sk_buff *skb;
> int k, last;
>
> + if (i < bp->tx_nr_rings_xdp &&
> + tx_buf->action == XDP_REDIRECT) {
> + dma_unmap_single(&pdev->dev,
> + dma_unmap_addr(tx_buf, mapping),
> + dma_unmap_len(tx_buf, len),
> + PCI_DMA_TODEVICE);
> + xdp_return_frame(tx_buf->xdpf);
> + tx_buf->action = 0;
> + tx_buf->xdpf = NULL;
> + j++;
> + continue;
> + }
> +
Can't see the whole file here and maybe i am missing something, but since you
optimize for that and start using page_pool, XDP_TX will be a re-synced (and
not remapped) buffer that can be returned to the pool and resynced for
device usage.
Is that happening later on the tx clean function?
> + skb = tx_buf->skb;
> if (!skb) {
> j++;
> continue;
> @@ -2517,6 +2534,13 @@ static int bnxt_alloc_rx_rings(struct bnxt *bp)
> if (rc < 0)
> return rc;
>
> + rc = xdp_rxq_info_reg_mem_model(&rxr->xdp_rxq,
> + MEM_TYPE_PAGE_SHARED, NULL);
> + if (rc) {
> + xdp_rxq_info_unreg(&rxr->xdp_rxq);
I think you can use page_pool_free directly here (and pge_pool_destroy once
Ivan's patchset gets nerged), that's what mlx5 does iirc. Can we keep that
common please?
If Ivan's patch get merged please note you'll have to explicitly
page_pool_destroy, after calling xdp_rxq_info_unreg() in the general unregister
case (not the error habdling here). Sorry for the confusion this might bring!
> + return rc;
> + }
> +
> rc = bnxt_alloc_ring(bp, &ring->ring_mem);
> if (rc)
> return rc;
> @@ -10233,6 +10257,7 @@ static const struct net_device_ops bnxt_netdev_ops = {
[...]
Thanks!
/Ilias
^ permalink raw reply
* [PATCH nf-next 1/3] netfilter: nf_nat_proto: add nf_nat_bridge_ops support
From: wenxu @ 2019-07-08 8:29 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
From: wenxu <wenxu@ucloud.cn>
Add nf_nat_bridge_ops to do nat in the bridge family
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nf_nat.h | 3 ++
net/netfilter/nf_nat_proto.c | 63 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
index 423cda2..0c2d326 100644
--- a/include/net/netfilter/nf_nat.h
+++ b/include/net/netfilter/nf_nat.h
@@ -101,6 +101,9 @@ int nf_nat_icmpv6_reply_translation(struct sk_buff *skb, struct nf_conn *ct,
int nf_nat_inet_register_fn(struct net *net, const struct nf_hook_ops *ops);
void nf_nat_inet_unregister_fn(struct net *net, const struct nf_hook_ops *ops);
+int nf_nat_bridge_register_fn(struct net *net, const struct nf_hook_ops *ops);
+void nf_nat_bridge_unregister_fn(struct net *net, const struct nf_hook_ops *ops);
+
unsigned int
nf_nat_inet_fn(void *priv, struct sk_buff *skb,
const struct nf_hook_state *state);
diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
index 888292e..652a71e 100644
--- a/net/netfilter/nf_nat_proto.c
+++ b/net/netfilter/nf_nat_proto.c
@@ -1035,3 +1035,66 @@ void nf_nat_inet_unregister_fn(struct net *net, const struct nf_hook_ops *ops)
}
EXPORT_SYMBOL_GPL(nf_nat_inet_unregister_fn);
#endif /* NFT INET NAT */
+
+#if defined(CONFIG_NF_TABLES_BRIDGE) && IS_ENABLED(CONFIG_NFT_NAT)
+static unsigned int
+nf_nat_bridge_in(void *priv, struct sk_buff *skb,
+ const struct nf_hook_state *state)
+{
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ return nf_nat_ipv4_in(priv, skb, state);
+ case htons(ETH_P_IPV6):
+ return nf_nat_ipv6_in(priv, skb, state);
+ default:
+ return NF_ACCEPT;
+ }
+}
+
+static unsigned int
+nf_nat_bridge_out(void *priv, struct sk_buff *skb,
+ const struct nf_hook_state *state)
+{
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ return nf_nat_ipv4_out(priv, skb, state);
+ case htons(ETH_P_IPV6):
+ return nf_nat_ipv6_out(priv, skb, state);
+ default:
+ return NF_ACCEPT;
+ }
+}
+
+const struct nf_hook_ops nf_nat_bridge_ops[] = {
+ /* Before packet filtering, change destination */
+ {
+ .hook = nf_nat_bridge_in,
+ .pf = NFPROTO_BRIDGE,
+ .hooknum = NF_INET_PRE_ROUTING,
+ .priority = NF_IP_PRI_NAT_DST,
+ },
+ /* After packet filtering, change source */
+ {
+ .hook = nf_nat_bridge_out,
+ .pf = NFPROTO_BRIDGE,
+ .hooknum = NF_INET_POST_ROUTING,
+ .priority = NF_IP_PRI_NAT_SRC,
+ },
+};
+
+int nf_nat_bridge_register_fn(struct net *net, const struct nf_hook_ops *ops)
+{
+ if (WARN_ON_ONCE(ops->pf != NFPROTO_BRIDGE))
+ return -EINVAL;
+
+ return nf_nat_register_fn(net, ops->pf, ops, nf_nat_bridge_ops,
+ ARRAY_SIZE(nf_nat_bridge_ops));
+}
+EXPORT_SYMBOL_GPL(nf_nat_bridge_register_fn);
+
+void nf_nat_bridge_unregister_fn(struct net *net, const struct nf_hook_ops *ops)
+{
+ nf_nat_unregister_fn(net, ops->pf, ops, ARRAY_SIZE(nf_nat_bridge_ops));
+}
+EXPORT_SYMBOL_GPL(nf_nat_bridge_unregister_fn);
+#endif
--
1.8.3.1
^ permalink raw reply related
* [PATCH nf-next 2/3] netfilter: nft_chain_nat: add nft_chain_nat_bridge support
From: wenxu @ 2019-07-08 8:29 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1562574567-8293-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
Add nft_chan_nat_bridge to handle nat rule in bridge family
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/netfilter/nft_chain_nat.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/net/netfilter/nft_chain_nat.c b/net/netfilter/nft_chain_nat.c
index 2f89bde..2ae3fbb 100644
--- a/net/netfilter/nft_chain_nat.c
+++ b/net/netfilter/nft_chain_nat.c
@@ -104,6 +104,23 @@ static void nft_nat_inet_unreg(struct net *net, const struct nf_hook_ops *ops)
};
#endif
+#ifdef CONFIG_NF_TABLES_BRIDGE
+static const struct nft_chain_type nft_chain_nat_bridge = {
+ .name = "nat",
+ .type = NFT_CHAIN_T_NAT,
+ .family = NFPROTO_BRIDGE,
+ .owner = THIS_MODULE,
+ .hook_mask = (1 << NF_INET_PRE_ROUTING) |
+ (1 << NF_INET_POST_ROUTING),
+ .hooks = {
+ [NF_INET_PRE_ROUTING] = nft_nat_do_chain,
+ [NF_INET_POST_ROUTING] = nft_nat_do_chain,
+ },
+ .ops_register = nf_nat_bridge_register_fn,
+ .ops_unregister = nf_nat_bridge_unregister_fn,
+};
+#endif
+
static int __init nft_chain_nat_init(void)
{
#ifdef CONFIG_NF_TABLES_IPV6
@@ -115,6 +132,9 @@ static int __init nft_chain_nat_init(void)
#ifdef CONFIG_NF_TABLES_INET
nft_register_chain_type(&nft_chain_nat_inet);
#endif
+#ifdef CONFIG_NF_TABLES_BRIDGE
+ nft_register_chain_type(&nft_chain_nat_bridge);
+#endif
return 0;
}
@@ -130,6 +150,9 @@ static void __exit nft_chain_nat_exit(void)
#ifdef CONFIG_NF_TABLES_INET
nft_unregister_chain_type(&nft_chain_nat_inet);
#endif
+#ifdef CONFIG_NF_TABLES_BRIDGE
+ nft_unregister_chain_type(&nft_chain_nat_bridge);
+#endif
}
module_init(nft_chain_nat_init);
@@ -142,3 +165,6 @@ static void __exit nft_chain_nat_exit(void)
#ifdef CONFIG_NF_TABLES_IPV6
MODULE_ALIAS_NFT_CHAIN(AF_INET6, "nat");
#endif
+#ifdef CONFIG_NF_TABLES_BRIDGE
+MODULE_ALIAS_NFT_CHAIN(AF_BRIDGE, "nat");
+#endif
--
1.8.3.1
^ permalink raw reply related
* [PATCH nf-next 3/3] netfilter: nft_nat: add nft_bridge_nat_type support
From: wenxu @ 2019-07-08 8:29 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1562574567-8293-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
Add nft_bridge_nat_type to configure nat rule in bridge family
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/netfilter/nft_nat.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c
index c3c93e9..ba396851 100644
--- a/net/netfilter/nft_nat.c
+++ b/net/netfilter/nft_nat.c
@@ -136,7 +136,9 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
return -EINVAL;
family = ntohl(nla_get_be32(tb[NFTA_NAT_FAMILY]));
- if (ctx->family != NFPROTO_INET && ctx->family != family)
+ if (ctx->family != NFPROTO_INET &&
+ ctx->family != NFPROTO_BRIDGE &&
+ ctx->family != family)
return -EOPNOTSUPP;
switch (family) {
@@ -318,6 +320,40 @@ static void nft_nat_inet_module_exit(void)
static void nft_nat_inet_module_exit(void) { }
#endif
+#ifdef CONFIG_NF_TABLES_BRIDGE
+static const struct nft_expr_ops nft_nat_bridge_ops = {
+ .type = &nft_nat_type,
+ .size = NFT_EXPR_SIZE(sizeof(struct nft_nat)),
+ .eval = nft_nat_eval,
+ .init = nft_nat_init,
+ .destroy = nft_nat_destroy,
+ .dump = nft_nat_dump,
+ .validate = nft_nat_validate,
+};
+
+static struct nft_expr_type nft_bridge_nat_type __read_mostly = {
+ .name = "nat",
+ .family = NFPROTO_BRIDGE,
+ .ops = &nft_nat_bridge_ops,
+ .policy = nft_nat_policy,
+ .maxattr = NFTA_NAT_MAX,
+ .owner = THIS_MODULE,
+};
+
+static int nft_nat_bridge_module_init(void)
+{
+ return nft_register_expr(&nft_bridge_nat_type);
+}
+
+static void nft_nat_bridge_module_exit(void)
+{
+ nft_unregister_expr(&nft_bridge_nat_type);
+}
+#else
+static int nft_nat_bridge_module_init(void) { return 0; }
+static void nft_nat_bridge_module_exit(void) { }
+#endif
+
static int __init nft_nat_module_init(void)
{
int ret = nft_nat_inet_module_init();
@@ -325,15 +361,24 @@ static int __init nft_nat_module_init(void)
if (ret)
return ret;
+ ret = nft_nat_bridge_module_init();
+ if (ret) {
+ nft_nat_inet_module_exit();
+ return ret;
+ }
+
ret = nft_register_expr(&nft_nat_type);
- if (ret)
+ if (ret) {
+ nft_nat_bridge_module_exit();
nft_nat_inet_module_exit();
+ }
return ret;
}
static void __exit nft_nat_module_exit(void)
{
+ nft_nat_bridge_module_exit();
nft_nat_inet_module_exit();
nft_unregister_expr(&nft_nat_type);
}
--
1.8.3.1
^ permalink raw reply related
* RE: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: David Laight @ 2019-07-08 8:36 UTC (permalink / raw)
To: 'Jian-Hong Pan', Yan-Hsuan Chuang, Kalle Valo,
David S . Miller
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux@endlessm.com, Daniel Drake,
stable@vger.kernel.org
In-Reply-To: <20190708063252.4756-1-jian-hong@endlessm.com>
From: Jian-Hong Pan
> Sent: 08 July 2019 07:33
> To: Yan-Hsuan Chuang; Kalle Valo; David S . Miller
>
> Testing with RTL8822BE hardware, when available memory is low, we
> frequently see a kernel panic and system freeze.
>
> First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
>
> rx routine starvation
> WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822
> rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> [ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
>
> Then we see a variety of different error conditions and kernel panics,
> such as this one (trimmed):
>
> rtw_pci 0000:02:00.0: pci bus timeout, check dma status
> skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f
> data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:105!
> invalid opcode: 0000 [#1] SMP NOPTI
> RIP: 0010:skb_panic+0x43/0x45
>
> When skb allocation fails and the "rx routine starvation" is hit, the
> function returns immediately without updating the RX ring. At this
> point, the RX ring may continue referencing an old skb which was already
> handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> bad things happen.
>
> This patch allocates a new skb first in RX ISR. If we don't have memory
> available, we discard the current frame, allowing the existing skb to be
> reused in the ring. Otherwise, we simplify the code flow and just hand
> over the RX-populated skb over to mac80211.
>
> In addition, to fixing the kernel crash, the RX routine should now
> generally behave better under low memory conditions.
Under low memory conditions it may be preferable to limit the amount
of memory assigned to the receive ring.
I also thought it was preferable (DM may correct me here) to do the
skb allocates from the 'bh' of the driver rather than from the hardware
interrupt.
It is also almost certainly preferable (especially on IOMMU systems)
to copy small frames into a new skb (of the right size) and then
reuse the skb (with its dma-mapped buffer) for a later frame.
Allocating a new skb before ay px processing just seems wrong...
David
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Reviewed-by: Daniel Drake <drake@endlessm.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/net/wireless/realtek/rtw88/pci.c | 28 +++++++++++-------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index cfe05ba7280d..1bfc99ae6b84 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> rx_desc = skb->data;
> chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
>
> + /* discard current skb if the new skb cannot be allocated as a
> + * new one in rx ring later
> + * */
> + new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> + if (WARN(!new, "rx routine starvation\n")) {
> + new = skb;
> + goto next_rp;
> + }
> +
> /* offset from rx_desc to payload */
> pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> pkt_stat.shift;
> @@ -803,25 +812,14 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> skb_put(skb, pkt_stat.pkt_len);
> skb_reserve(skb, pkt_offset);
>
> - /* alloc a smaller skb to mac80211 */
> - new = dev_alloc_skb(pkt_stat.pkt_len);
> - if (!new) {
> - new = skb;
> - } else {
> - skb_put_data(new, skb->data, skb->len);
> - dev_kfree_skb_any(skb);
> - }
> /* TODO: merge into rx.c */
> rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> - memcpy(new->cb, &rx_status, sizeof(rx_status));
> - ieee80211_rx_irqsafe(rtwdev->hw, new);
> + memcpy(skb->cb, &rx_status, sizeof(rx_status));
> + ieee80211_rx_irqsafe(rtwdev->hw, skb);
> }
>
> - /* skb delivered to mac80211, alloc a new one in rx ring */
> - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> - if (WARN(!new, "rx routine starvation\n"))
> - return;
> -
> +next_rp:
> + /* skb delivered to mac80211, attach the new one into rx ring */
> ring->buf[cur_rp] = new;
> rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
>
> --
> 2.22.0
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH net-next v2 8/8] net: mscc: PTP Hardware Clock (PHC) support
From: Antoine Tenart @ 2019-07-08 8:48 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Antoine Tenart, davem, richardcochran, alexandre.belloni,
UNGLinuxDriver, ralf, paul.burton, jhogan, netdev, linux-mips,
thomas.petazzoni, allan.nielsen
In-Reply-To: <20190705151038.0581a052@cakuba.netronome.com>
Hello Jakub,
On Fri, Jul 05, 2019 at 03:10:38PM -0700, Jakub Kicinski wrote:
> On Fri, 5 Jul 2019 21:52:13 +0200, Antoine Tenart wrote:
> > @@ -596,11 +606,50 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
> >
> > dev->stats.tx_packets++;
> > dev->stats.tx_bytes += skb->len;
> > - dev_kfree_skb_any(skb);
> > +
> > + if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP &&
> > + port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
> > + struct ocelot_skb *oskb =
> > + kzalloc(sizeof(struct ocelot_skb), GFP_KERNEL);
>
> I think this is the TX path, you can't use GFP_KERNEL here.
I'll fix it.
> > +static int ocelot_hwstamp_set(struct ocelot_port *port, struct ifreq *ifr)
> > +{
> > + struct ocelot *ocelot = port->ocelot;
> > + struct hwtstamp_config cfg;
> > +
> > + if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
> > + return -EFAULT;
> > +
> > + /* reserved for future extensions */
> > + if (cfg.flags)
> > + return -EINVAL;
> > +
> > + /* Tx type sanity check */
> > + switch (cfg.tx_type) {
> > + case HWTSTAMP_TX_ON:
> > + port->ptp_cmd = IFH_REW_OP_TWO_STEP_PTP;
> > + break;
> > + case HWTSTAMP_TX_ONESTEP_SYNC:
> > + /* IFH_REW_OP_ONE_STEP_PTP updates the correctional field, we
> > + * need to update the origin time.
> > + */
> > + port->ptp_cmd = IFH_REW_OP_ORIGIN_PTP;
> > + break;
> > + case HWTSTAMP_TX_OFF:
> > + port->ptp_cmd = 0;
> > + break;
> > + default:
> > + return -ERANGE;
> > + }
> > +
> > + mutex_lock(&ocelot->ptp_lock);
> > +
> > + switch (cfg.rx_filter) {
> > + case HWTSTAMP_FILTER_NONE:
> > + break;
> > + case HWTSTAMP_FILTER_ALL:
> > + case HWTSTAMP_FILTER_SOME:
> > + case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
> > + case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> > + case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
> > + case HWTSTAMP_FILTER_NTP_ALL:
> > + case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> > + case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
> > + case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> > + case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
> > + case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
> > + case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
> > + case HWTSTAMP_FILTER_PTP_V2_EVENT:
> > + case HWTSTAMP_FILTER_PTP_V2_SYNC:
> > + case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> > + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> > + break;
> > + default:
> > + mutex_unlock(&ocelot->ptp_lock);
> > + return -ERANGE;
> > + }
>
> No device reconfig, so the PTP RX stamping is always enabled? Perhaps
> consider setting
>
> ocelot->hwtstamp_config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT
>
> at probe?
That's right. Would set the ptp flag to 0 also be an option here (so
that we respect HWTSTAMP_FILTER_NONE at least in the driver)?
> > + /* Commit back the result & save it */
> > + memcpy(&ocelot->hwtstamp_config, &cfg, sizeof(cfg));
> > + mutex_unlock(&ocelot->ptp_lock);
> > +
> > + return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
> > +}
> >
> > +static int ocelot_get_ts_info(struct net_device *dev,
> > + struct ethtool_ts_info *info)
> > +{
> > + struct ocelot_port *ocelot_port = netdev_priv(dev);
> > + struct ocelot *ocelot = ocelot_port->ocelot;
> > + int ret;
> > +
> > + if (!ocelot->ptp)
> > + return -EOPNOTSUPP;
>
> Hmm.. why does software timestamping depend on PTP?
Because it depends on the "PTP" register bank (and the "PTP" interrupt)
being described and available. This is why I named the flag 'ptp', but
it could be named 'timestamp' or 'ts' as well.
Thanks,
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox