Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] hinic: add fw version query
From: Xue Chaojing @ 2019-07-05  2:40 UTC (permalink / raw)
  To: davem
  Cc: linux-kernel, netdev, luoshaokai, cloud.wangxiaoyun, xuechaojing,
	chiqijun, wulike1

This patch adds firmware version query in ethtool -i.

Signed-off-by: Xue Chaojing <xuechaojing@huawei.com>
---
 .../net/ethernet/huawei/hinic/hinic_ethtool.c |  8 +++++
 .../net/ethernet/huawei/hinic/hinic_hw_dev.h  |  2 ++
 .../net/ethernet/huawei/hinic/hinic_port.c    | 30 +++++++++++++++++++
 .../net/ethernet/huawei/hinic/hinic_port.h    | 14 +++++++++
 4 files changed, 54 insertions(+)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
index 73a20f01ad4c..60ec48fe4144 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
@@ -117,11 +117,19 @@ static void hinic_get_drvinfo(struct net_device *netdev,
 			      struct ethtool_drvinfo *info)
 {
 	struct hinic_dev *nic_dev = netdev_priv(netdev);
+	u8 mgmt_ver[HINIC_MGMT_VERSION_MAX_LEN] = {0};
 	struct hinic_hwdev *hwdev = nic_dev->hwdev;
 	struct hinic_hwif *hwif = hwdev->hwif;
+	int err;
 
 	strlcpy(info->driver, HINIC_DRV_NAME, sizeof(info->driver));
 	strlcpy(info->bus_info, pci_name(hwif->pdev), sizeof(info->bus_info));
+
+	err = hinic_get_mgmt_version(nic_dev, mgmt_ver);
+	if (err)
+		return;
+
+	snprintf(info->fw_version, sizeof(info->fw_version), "%s", mgmt_ver);
 }
 
 static void hinic_get_ringparam(struct net_device *netdev,
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
index 984c98f33258..b069045de416 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
@@ -77,6 +77,8 @@ enum hinic_port_cmd {
 
 	HINIC_PORT_CMD_FWCTXT_INIT      = 69,
 
+	HINIC_PORT_CMD_GET_MGMT_VERSION = 88,
+
 	HINIC_PORT_CMD_SET_FUNC_STATE   = 93,
 
 	HINIC_PORT_CMD_GET_GLOBAL_QPN   = 102,
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_port.c b/drivers/net/ethernet/huawei/hinic/hinic_port.c
index 1bbeb91be808..1e389a004e50 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_port.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_port.c
@@ -1038,3 +1038,33 @@ int hinic_get_phy_port_stats(struct hinic_dev *nic_dev,
 
 	return err;
 }
+
+int hinic_get_mgmt_version(struct hinic_dev *nic_dev, u8 *mgmt_ver)
+{
+	struct hinic_hwdev *hwdev = nic_dev->hwdev;
+	struct hinic_version_info up_ver = {0};
+	struct hinic_hwif *hwif;
+	struct pci_dev *pdev;
+	u16 out_size;
+	int err;
+
+	if (!hwdev)
+		return -EINVAL;
+
+	hwif = hwdev->hwif;
+	pdev = hwif->pdev;
+
+	err = hinic_port_msg_cmd(hwdev, HINIC_PORT_CMD_GET_MGMT_VERSION,
+				 &up_ver, sizeof(up_ver), &up_ver,
+				 &out_size);
+	if (err || !out_size || up_ver.status) {
+		dev_err(&pdev->dev,
+			"Failed to get mgmt version, err: %d, status: 0x%x, out size: 0x%x\n",
+			err, up_ver.status, out_size);
+		return -EINVAL;
+	}
+
+	snprintf(mgmt_ver, HINIC_MGMT_VERSION_MAX_LEN, "%s", up_ver.ver);
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_port.h b/drivers/net/ethernet/huawei/hinic/hinic_port.h
index 1bc47c7a5c00..44772fd47fc1 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_port.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_port.h
@@ -16,6 +16,18 @@
 #define HINIC_RSS_KEY_SIZE	40
 #define HINIC_RSS_INDIR_SIZE	256
 #define HINIC_PORT_STATS_VERSION	0
+#define HINIC_FW_VERSION_NAME	16
+#define HINIC_COMPILE_TIME_LEN	20
+#define HINIC_MGMT_VERSION_MAX_LEN	32
+
+struct hinic_version_info {
+	u8 status;
+	u8 version;
+	u8 rsvd[6];
+
+	u8 ver[HINIC_FW_VERSION_NAME];
+	u8 time[HINIC_COMPILE_TIME_LEN];
+};
 
 enum hinic_rx_mode {
 	HINIC_RX_MODE_UC        = BIT(0),
@@ -571,4 +583,6 @@ int hinic_get_vport_stats(struct hinic_dev *nic_dev,
 
 int hinic_set_rx_vlan_offload(struct hinic_dev *nic_dev, u8 en);
 
+int hinic_get_mgmt_version(struct hinic_dev *nic_dev, u8 *mgmt_ver);
+
 #endif
-- 
2.17.1


^ permalink raw reply related

* [PATCH] wl3501_cs: remove redundant variable ret
From: Colin King @ 2019-07-05 10:37 UTC (permalink / raw)
  To: Kalle Valo, David S . Miller, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

The variable ret is being initialized with a value that is never
read and it is being updated later with a new value that is returned.
The variable is redundant and can be replaced with a return 0 as
there are no other return points in this function.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/wl3501_cs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
index a25b17932edb..007bf6803293 100644
--- a/drivers/net/wireless/wl3501_cs.c
+++ b/drivers/net/wireless/wl3501_cs.c
@@ -1226,7 +1226,6 @@ static int wl3501_init_firmware(struct wl3501_card *this)
 static int wl3501_close(struct net_device *dev)
 {
 	struct wl3501_card *this = netdev_priv(dev);
-	int rc = -ENODEV;
 	unsigned long flags;
 	struct pcmcia_device *link;
 	link = this->p_dev;
@@ -1241,10 +1240,9 @@ static int wl3501_close(struct net_device *dev)
 	/* Mask interrupts from the SUTRO */
 	wl3501_block_interrupt(this);
 
-	rc = 0;
 	printk(KERN_INFO "%s: WL3501 closed\n", dev->name);
 	spin_unlock_irqrestore(&this->lock, flags);
-	return rc;
+	return 0;
 }
 
 /**
-- 
2.20.1


^ permalink raw reply related

* [net-next, PATCH, v3] net: netsec: Sync dma for device on buffer allocation
From: Ilias Apalodimas @ 2019-07-05 10:47 UTC (permalink / raw)
  To: netdev, jaswinder.singh; +Cc: brouer, ard.biesheuvel, arnd, Ilias Apalodimas

Quoting Arnd,
We have to do a sync_single_for_device /somewhere/ before the
buffer is given to the device. On a non-cache-coherent machine with
a write-back cache, there may be dirty cache lines that get written back
after the device DMA's data into it (e.g. from a previous memset
from before the buffer got freed), so you absolutely need to flush any
dirty cache lines on it first.

Since the coherency is configurable in this device make sure we cover
all configurations by explicitly syncing the allocated buffer for the
device before refilling it's descriptors

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Changes since v2:
- Only sync for the portion of the packet owned by the NIC as suggested by 
  Jesper

 drivers/net/ethernet/socionext/netsec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 5544a722543f..6b954ad88842 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -727,6 +727,7 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
 {
 
 	struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
+	enum dma_data_direction dma_dir;
 	struct page *page;
 
 	page = page_pool_dev_alloc_pages(dring->page_pool);
@@ -742,6 +743,8 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
 	 * cases and reserve enough space for headroom + skb_shared_info
 	 */
 	*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, *desc_len, dma_dir);
 
 	return page_address(page);
 }
-- 
2.20.1


^ permalink raw reply related

* [PATCH v3 0/3] vsock/virtio: several fixes in the .probe() and .remove()
From: Stefano Garzarella @ 2019-07-05 11:04 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, kvm, Michael S. Tsirkin, linux-kernel,
	Jason Wang, virtualization, Stefan Hajnoczi

During the review of "[PATCH] vsock/virtio: Initialize core virtio vsock
before registering the driver", Stefan pointed out some possible issues
in the .probe() and .remove() callbacks of the virtio-vsock driver.

This series tries to solve these issues:
- Patch 1 adds RCU critical sections to avoid use-after-free of
  'the_virtio_vsock' pointer.
- Patch 2 stops workers before to call vdev->config->reset(vdev) to
  be sure that no one is accessing the device.
- Patch 3 moves the works flush at the end of the .remove() to avoid
  use-after-free of 'vsock' object.

v3:
- Patch 1: use rcu_dereference_protected() to get the_virtio_vosck value in
           the virtio_vsock_probe() [Jason]

v2: https://patchwork.kernel.org/cover/11022343/

v1: https://patchwork.kernel.org/cover/10964733/

Before this series the guest crashes in a few second. After this series the
test runs (~12h) without issues.
Tested on an SMP guest (-smp 4 -monitor tcp:127.0.0.1:1234,server,nowait)
with these scripts to stress the .probe()/.remove() path:

- guest
  while true; do
      cat /dev/urandom | nc-vsock -l 4321 > /dev/null &
      cat /dev/urandom | nc-vsock -l 5321 > /dev/null &
      cat /dev/urandom | nc-vsock -l 6321 > /dev/null &
      cat /dev/urandom | nc-vsock -l 7321 > /dev/null &
      wait
  done

- host
  while true; do
      cat /dev/urandom | nc-vsock 3 4321 > /dev/null &
      cat /dev/urandom | nc-vsock 3 5321 > /dev/null &
      cat /dev/urandom | nc-vsock 3 6321 > /dev/null &
      cat /dev/urandom | nc-vsock 3 7321 > /dev/null &
      sleep 2
      echo "device_del v1" | nc 127.0.0.1 1234
      sleep 1
      echo "device_add vhost-vsock-pci,id=v1,guest-cid=3" | nc 127.0.0.1 1234
      sleep 1
  done

Stefano Garzarella (3):
  vsock/virtio: use RCU to avoid use-after-free on the_virtio_vsock
  vsock/virtio: stop workers during the .remove()
  vsock/virtio: fix flush of works during the .remove()

 net/vmw_vsock/virtio_transport.c | 134 ++++++++++++++++++++++++-------
 1 file changed, 104 insertions(+), 30 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH v3 3/3] vsock/virtio: fix flush of works during the .remove()
From: Stefano Garzarella @ 2019-07-05 11:04 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, kvm, Michael S. Tsirkin, linux-kernel,
	Jason Wang, virtualization, Stefan Hajnoczi
In-Reply-To: <20190705110454.95302-1-sgarzare@redhat.com>

This patch moves the flush of works after vdev->config->del_vqs(vdev),
because we need to be sure that no workers run before to free the
'vsock' object.

Since we stopped the workers using the [tx|rx|event]_run flags,
we are sure no one is accessing the device while we are calling
vdev->config->reset(vdev), so we can safely move the workers' flush.

Before the vdev->config->del_vqs(vdev), workers can be scheduled
by VQ callbacks, so we must flush them after del_vqs(), to avoid
use-after-free of 'vsock' object.

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 4dbdce7746bd..0815d1357861 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -681,12 +681,6 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 	rcu_assign_pointer(the_virtio_vsock, NULL);
 	synchronize_rcu();
 
-	flush_work(&vsock->loopback_work);
-	flush_work(&vsock->rx_work);
-	flush_work(&vsock->tx_work);
-	flush_work(&vsock->event_work);
-	flush_work(&vsock->send_pkt_work);
-
 	/* Reset all connected sockets when the device disappear */
 	vsock_for_each_connected_socket(virtio_vsock_reset_sock);
 
@@ -741,6 +735,15 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 	/* Delete virtqueues and flush outstanding callbacks if any */
 	vdev->config->del_vqs(vdev);
 
+	/* Other works can be queued before 'config->del_vqs()', so we flush
+	 * all works before to free the vsock object to avoid use after free.
+	 */
+	flush_work(&vsock->loopback_work);
+	flush_work(&vsock->rx_work);
+	flush_work(&vsock->tx_work);
+	flush_work(&vsock->event_work);
+	flush_work(&vsock->send_pkt_work);
+
 	mutex_unlock(&the_virtio_vsock_mutex);
 
 	kfree(vsock);
-- 
2.20.1


^ permalink raw reply related

* [PATCH v3 2/3] vsock/virtio: stop workers during the .remove()
From: Stefano Garzarella @ 2019-07-05 11:04 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, kvm, Michael S. Tsirkin, linux-kernel,
	Jason Wang, virtualization, Stefan Hajnoczi
In-Reply-To: <20190705110454.95302-1-sgarzare@redhat.com>

Before to call vdev->config->reset(vdev) we need to be sure that
no one is accessing the device, for this reason, we add new variables
in the struct virtio_vsock to stop the workers during the .remove().

This patch also add few comments before vdev->config->reset(vdev)
and vdev->config->del_vqs(vdev).

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport.c | 51 +++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 3eaec60aa64f..4dbdce7746bd 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -38,6 +38,7 @@ struct virtio_vsock {
 	 * must be accessed with tx_lock held.
 	 */
 	struct mutex tx_lock;
+	bool tx_run;
 
 	struct work_struct send_pkt_work;
 	spinlock_t send_pkt_list_lock;
@@ -53,6 +54,7 @@ struct virtio_vsock {
 	 * must be accessed with rx_lock held.
 	 */
 	struct mutex rx_lock;
+	bool rx_run;
 	int rx_buf_nr;
 	int rx_buf_max_nr;
 
@@ -60,6 +62,7 @@ struct virtio_vsock {
 	 * vqs[VSOCK_VQ_EVENT] must be accessed with event_lock held.
 	 */
 	struct mutex event_lock;
+	bool event_run;
 	struct virtio_vsock_event event_list[8];
 
 	u32 guest_cid;
@@ -94,6 +97,10 @@ static void virtio_transport_loopback_work(struct work_struct *work)
 	spin_unlock_bh(&vsock->loopback_list_lock);
 
 	mutex_lock(&vsock->rx_lock);
+
+	if (!vsock->rx_run)
+		goto out;
+
 	while (!list_empty(&pkts)) {
 		struct virtio_vsock_pkt *pkt;
 
@@ -102,6 +109,7 @@ static void virtio_transport_loopback_work(struct work_struct *work)
 
 		virtio_transport_recv_pkt(pkt);
 	}
+out:
 	mutex_unlock(&vsock->rx_lock);
 }
 
@@ -130,6 +138,9 @@ virtio_transport_send_pkt_work(struct work_struct *work)
 
 	mutex_lock(&vsock->tx_lock);
 
+	if (!vsock->tx_run)
+		goto out;
+
 	vq = vsock->vqs[VSOCK_VQ_TX];
 
 	for (;;) {
@@ -188,6 +199,7 @@ virtio_transport_send_pkt_work(struct work_struct *work)
 	if (added)
 		virtqueue_kick(vq);
 
+out:
 	mutex_unlock(&vsock->tx_lock);
 
 	if (restart_rx)
@@ -323,6 +335,10 @@ static void virtio_transport_tx_work(struct work_struct *work)
 
 	vq = vsock->vqs[VSOCK_VQ_TX];
 	mutex_lock(&vsock->tx_lock);
+
+	if (!vsock->tx_run)
+		goto out;
+
 	do {
 		struct virtio_vsock_pkt *pkt;
 		unsigned int len;
@@ -333,6 +349,8 @@ static void virtio_transport_tx_work(struct work_struct *work)
 			added = true;
 		}
 	} while (!virtqueue_enable_cb(vq));
+
+out:
 	mutex_unlock(&vsock->tx_lock);
 
 	if (added)
@@ -361,6 +379,9 @@ static void virtio_transport_rx_work(struct work_struct *work)
 
 	mutex_lock(&vsock->rx_lock);
 
+	if (!vsock->rx_run)
+		goto out;
+
 	do {
 		virtqueue_disable_cb(vq);
 		for (;;) {
@@ -470,6 +491,9 @@ static void virtio_transport_event_work(struct work_struct *work)
 
 	mutex_lock(&vsock->event_lock);
 
+	if (!vsock->event_run)
+		goto out;
+
 	do {
 		struct virtio_vsock_event *event;
 		unsigned int len;
@@ -484,7 +508,7 @@ static void virtio_transport_event_work(struct work_struct *work)
 	} while (!virtqueue_enable_cb(vq));
 
 	virtqueue_kick(vsock->vqs[VSOCK_VQ_EVENT]);
-
+out:
 	mutex_unlock(&vsock->event_lock);
 }
 
@@ -620,12 +644,18 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
 	INIT_WORK(&vsock->send_pkt_work, virtio_transport_send_pkt_work);
 	INIT_WORK(&vsock->loopback_work, virtio_transport_loopback_work);
 
+	mutex_lock(&vsock->tx_lock);
+	vsock->tx_run = true;
+	mutex_unlock(&vsock->tx_lock);
+
 	mutex_lock(&vsock->rx_lock);
 	virtio_vsock_rx_fill(vsock);
+	vsock->rx_run = true;
 	mutex_unlock(&vsock->rx_lock);
 
 	mutex_lock(&vsock->event_lock);
 	virtio_vsock_event_fill(vsock);
+	vsock->event_run = true;
 	mutex_unlock(&vsock->event_lock);
 
 	vdev->priv = vsock;
@@ -660,6 +690,24 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 	/* Reset all connected sockets when the device disappear */
 	vsock_for_each_connected_socket(virtio_vsock_reset_sock);
 
+	/* Stop all work handlers to make sure no one is accessing the device,
+	 * so we can safely call vdev->config->reset().
+	 */
+	mutex_lock(&vsock->rx_lock);
+	vsock->rx_run = false;
+	mutex_unlock(&vsock->rx_lock);
+
+	mutex_lock(&vsock->tx_lock);
+	vsock->tx_run = false;
+	mutex_unlock(&vsock->tx_lock);
+
+	mutex_lock(&vsock->event_lock);
+	vsock->event_run = false;
+	mutex_unlock(&vsock->event_lock);
+
+	/* Flush all device writes and interrupts, device will not use any
+	 * more buffers.
+	 */
 	vdev->config->reset(vdev);
 
 	mutex_lock(&vsock->rx_lock);
@@ -690,6 +738,7 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 	}
 	spin_unlock_bh(&vsock->loopback_list_lock);
 
+	/* Delete virtqueues and flush outstanding callbacks if any */
 	vdev->config->del_vqs(vdev);
 
 	mutex_unlock(&the_virtio_vsock_mutex);
-- 
2.20.1


^ permalink raw reply related

* [PATCH v3 1/3] vsock/virtio: use RCU to avoid use-after-free on the_virtio_vsock
From: Stefano Garzarella @ 2019-07-05 11:04 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, kvm, Michael S. Tsirkin, linux-kernel,
	Jason Wang, virtualization, Stefan Hajnoczi
In-Reply-To: <20190705110454.95302-1-sgarzare@redhat.com>

Some callbacks used by the upper layers can run while we are in the
.remove(). A potential use-after-free can happen, because we free
the_virtio_vsock without knowing if the callbacks are over or not.

To solve this issue we move the assignment of the_virtio_vsock at the
end of .probe(), when we finished all the initialization, and at the
beginning of .remove(), before to release resources.
For the same reason, we do the same also for the vdev->priv.

We use RCU to be sure that all callbacks that use the_virtio_vsock
ended before freeing it. This is not required for callbacks that
use vdev->priv, because after the vdev->config->del_vqs() we are sure
that they are ended and will no longer be invoked.

We also take the mutex during the .remove() to avoid that .probe() can
run while we are resetting the device.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport.c | 70 +++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 24 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 9c287e3e393c..3eaec60aa64f 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -65,19 +65,22 @@ struct virtio_vsock {
 	u32 guest_cid;
 };
 
-static struct virtio_vsock *virtio_vsock_get(void)
-{
-	return the_virtio_vsock;
-}
-
 static u32 virtio_transport_get_local_cid(void)
 {
-	struct virtio_vsock *vsock = virtio_vsock_get();
+	struct virtio_vsock *vsock;
+	u32 ret;
 
-	if (!vsock)
-		return VMADDR_CID_ANY;
+	rcu_read_lock();
+	vsock = rcu_dereference(the_virtio_vsock);
+	if (!vsock) {
+		ret = VMADDR_CID_ANY;
+		goto out_rcu;
+	}
 
-	return vsock->guest_cid;
+	ret = vsock->guest_cid;
+out_rcu:
+	rcu_read_unlock();
+	return ret;
 }
 
 static void virtio_transport_loopback_work(struct work_struct *work)
@@ -197,14 +200,18 @@ virtio_transport_send_pkt(struct virtio_vsock_pkt *pkt)
 	struct virtio_vsock *vsock;
 	int len = pkt->len;
 
-	vsock = virtio_vsock_get();
+	rcu_read_lock();
+	vsock = rcu_dereference(the_virtio_vsock);
 	if (!vsock) {
 		virtio_transport_free_pkt(pkt);
-		return -ENODEV;
+		len = -ENODEV;
+		goto out_rcu;
 	}
 
-	if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->guest_cid)
-		return virtio_transport_send_pkt_loopback(vsock, pkt);
+	if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->guest_cid) {
+		len = virtio_transport_send_pkt_loopback(vsock, pkt);
+		goto out_rcu;
+	}
 
 	if (pkt->reply)
 		atomic_inc(&vsock->queued_replies);
@@ -214,6 +221,9 @@ virtio_transport_send_pkt(struct virtio_vsock_pkt *pkt)
 	spin_unlock_bh(&vsock->send_pkt_list_lock);
 
 	queue_work(virtio_vsock_workqueue, &vsock->send_pkt_work);
+
+out_rcu:
+	rcu_read_unlock();
 	return len;
 }
 
@@ -222,12 +232,14 @@ virtio_transport_cancel_pkt(struct vsock_sock *vsk)
 {
 	struct virtio_vsock *vsock;
 	struct virtio_vsock_pkt *pkt, *n;
-	int cnt = 0;
+	int cnt = 0, ret;
 	LIST_HEAD(freeme);
 
-	vsock = virtio_vsock_get();
+	rcu_read_lock();
+	vsock = rcu_dereference(the_virtio_vsock);
 	if (!vsock) {
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_rcu;
 	}
 
 	spin_lock_bh(&vsock->send_pkt_list_lock);
@@ -255,7 +267,11 @@ virtio_transport_cancel_pkt(struct vsock_sock *vsk)
 			queue_work(virtio_vsock_workqueue, &vsock->rx_work);
 	}
 
-	return 0;
+	ret = 0;
+
+out_rcu:
+	rcu_read_unlock();
+	return ret;
 }
 
 static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
@@ -565,7 +581,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
 		return ret;
 
 	/* Only one virtio-vsock device per guest is supported */
-	if (the_virtio_vsock) {
+	if (rcu_dereference_protected(the_virtio_vsock,
+				lockdep_is_held(&the_virtio_vsock_mutex))) {
 		ret = -EBUSY;
 		goto out;
 	}
@@ -590,8 +607,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
 	vsock->rx_buf_max_nr = 0;
 	atomic_set(&vsock->queued_replies, 0);
 
-	vdev->priv = vsock;
-	the_virtio_vsock = vsock;
 	mutex_init(&vsock->tx_lock);
 	mutex_init(&vsock->rx_lock);
 	mutex_init(&vsock->event_lock);
@@ -613,6 +628,9 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
 	virtio_vsock_event_fill(vsock);
 	mutex_unlock(&vsock->event_lock);
 
+	vdev->priv = vsock;
+	rcu_assign_pointer(the_virtio_vsock, vsock);
+
 	mutex_unlock(&the_virtio_vsock_mutex);
 	return 0;
 
@@ -627,6 +645,12 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 	struct virtio_vsock *vsock = vdev->priv;
 	struct virtio_vsock_pkt *pkt;
 
+	mutex_lock(&the_virtio_vsock_mutex);
+
+	vdev->priv = NULL;
+	rcu_assign_pointer(the_virtio_vsock, NULL);
+	synchronize_rcu();
+
 	flush_work(&vsock->loopback_work);
 	flush_work(&vsock->rx_work);
 	flush_work(&vsock->tx_work);
@@ -666,12 +690,10 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 	}
 	spin_unlock_bh(&vsock->loopback_list_lock);
 
-	mutex_lock(&the_virtio_vsock_mutex);
-	the_virtio_vsock = NULL;
-	mutex_unlock(&the_virtio_vsock_mutex);
-
 	vdev->config->del_vqs(vdev);
 
+	mutex_unlock(&the_virtio_vsock_mutex);
+
 	kfree(vsock);
 }
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v7 net-next 5/5] net: ethernet: ti: cpsw: add XDP support
From: Jesper Dangaard Brouer @ 2019-07-05 11:13 UTC (permalink / raw)
  To: Ivan Khoronzhuk
  Cc: grygorii.strashko, davem, ast, linux-kernel, linux-omap,
	ilias.apalodimas, netdev, daniel, jakub.kicinski, john.fastabend,
	brouer
In-Reply-To: <20190704231406.27083-6-ivan.khoronzhuk@linaro.org>

On Fri,  5 Jul 2019 02:14:06 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:

> +static int cpsw_xdp_tx_frame(struct cpsw_priv *priv, struct xdp_frame *xdpf,
> +			     struct page *page)
> +{
> +	struct cpsw_common *cpsw = priv->cpsw;
> +	struct cpsw_meta_xdp *xmeta;
> +	struct cpdma_chan *txch;
> +	dma_addr_t dma;
> +	int ret, port;
> +
> +	xmeta = (void *)xdpf + CPSW_XMETA_OFFSET;
> +	xmeta->ndev = priv->ndev;
> +	xmeta->ch = 0;
> +	txch = cpsw->txv[0].ch;
> +
> +	port = priv->emac_port + cpsw->data.dual_emac;
> +	if (page) {
> +		dma = page_pool_get_dma_addr(page);
> +		dma += xdpf->data - (void *)xdpf;

This code is only okay because this only happens for XDP_TX, where you
know this head-room calculation will be true.  The "correct"
calculation of the head-room would be:

  dma += xdpf->headroom + sizeof(struct xdp_frame);

The reason behind not using xdpf pointer itself as "data_hard_start",
is to allow struct xdp_frame to be located in another memory area.
This will be useful for e.g. AF_XDP transmit, or other zero-copy
transmit to go through ndo_xdp_xmit() (as we don't want userspace to
be-able to e.g. "race" change xdpf->len during transmit/DMA-completion).


> +		ret = cpdma_chan_submit_mapped(txch, cpsw_xdpf_to_handle(xdpf),
> +					       dma, xdpf->len, port);
> +	} else {
> +		if (sizeof(*xmeta) > xdpf->headroom) {
> +			xdp_return_frame_rx_napi(xdpf);
> +			return -EINVAL;
> +		}
> +
> +		ret = cpdma_chan_submit(txch, cpsw_xdpf_to_handle(xdpf),
> +					xdpf->data, xdpf->len, port);
> +	}



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* [PATCH] selftests: txring_overwrite: fix incorrect test of mmap() return value
From: Frank de Brabander @ 2019-07-05 11:43 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Frank de Brabander

If mmap() fails it returns MAP_FAILED, which is defined as ((void *) -1).
The current if-statement incorrectly tests if *ring is NULL.

Signed-off-by: Frank de Brabander <debrabander@gmail.com>
---
 tools/testing/selftests/net/txring_overwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/txring_overwrite.c b/tools/testing/selftests/net/txring_overwrite.c
index fd8b1c6..7d9ea03 100644
--- a/tools/testing/selftests/net/txring_overwrite.c
+++ b/tools/testing/selftests/net/txring_overwrite.c
@@ -113,7 +113,7 @@ static int setup_tx(char **ring)
 
 	*ring = mmap(0, req.tp_block_size * req.tp_block_nr,
 		     PROT_READ | PROT_WRITE, MAP_SHARED, fdt, 0);
-	if (!*ring)
+	if (*ring == MAP_FAILED)
 		error(1, errno, "mmap");
 
 	return fdt;
-- 
2.7.4


^ permalink raw reply related

* Re: [net-next, PATCH, v3] net: netsec: Sync dma for device on buffer allocation
From: Jesper Dangaard Brouer @ 2019-07-05 11:46 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: netdev, jaswinder.singh, ard.biesheuvel, arnd, brouer
In-Reply-To: <1562323667-6945-1-git-send-email-ilias.apalodimas@linaro.org>

On Fri,  5 Jul 2019 13:47:47 +0300
Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:

> Quoting Arnd,
> We have to do a sync_single_for_device /somewhere/ before the
> buffer is given to the device. On a non-cache-coherent machine with
> a write-back cache, there may be dirty cache lines that get written back
> after the device DMA's data into it (e.g. from a previous memset
> from before the buffer got freed), so you absolutely need to flush any
> dirty cache lines on it first.
> 
> Since the coherency is configurable in this device make sure we cover
> all configurations by explicitly syncing the allocated buffer for the
> device before refilling it's descriptors
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> Changes since v2:
> - Only sync for the portion of the packet owned by the NIC as suggested by 
>   Jesper

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

Some general comments below.

>  drivers/net/ethernet/socionext/netsec.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index 5544a722543f..6b954ad88842 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -727,6 +727,7 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
>  {
>  
>  	struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
> +	enum dma_data_direction dma_dir;
>  	struct page *page;
>  
>  	page = page_pool_dev_alloc_pages(dring->page_pool);
> @@ -742,6 +743,8 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
>  	 * cases and reserve enough space for headroom + skb_shared_info
>  	 */
>  	*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, *desc_len, dma_dir);

Following the API this seems to turn into a noop if dev_is_dma_coherent().

Thus, I don't think it is worth optimizing further, as I suggested
earlier, with only sync of previous packet length.   This sync of the
"full" possible payload-data area (without headroom) is likely the best
and simplest option.  I don't think we should extend and complicate
the API for optimizing for non-coherent DMA hardware.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH v7 net-next 5/5] net: ethernet: ti: cpsw: add XDP support
From: Ivan Khoronzhuk @ 2019-07-05 12:01 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: grygorii.strashko, davem, ast, linux-kernel, linux-omap,
	ilias.apalodimas, netdev, daniel, jakub.kicinski, john.fastabend
In-Reply-To: <20190705131354.15a9313c@carbon>

On Fri, Jul 05, 2019 at 01:13:54PM +0200, Jesper Dangaard Brouer wrote:
>On Fri,  5 Jul 2019 02:14:06 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> +static int cpsw_xdp_tx_frame(struct cpsw_priv *priv, struct xdp_frame *xdpf,
>> +			     struct page *page)
>> +{
>> +	struct cpsw_common *cpsw = priv->cpsw;
>> +	struct cpsw_meta_xdp *xmeta;
>> +	struct cpdma_chan *txch;
>> +	dma_addr_t dma;
>> +	int ret, port;
>> +
>> +	xmeta = (void *)xdpf + CPSW_XMETA_OFFSET;
>> +	xmeta->ndev = priv->ndev;
>> +	xmeta->ch = 0;
>> +	txch = cpsw->txv[0].ch;
>> +
>> +	port = priv->emac_port + cpsw->data.dual_emac;
>> +	if (page) {
>> +		dma = page_pool_get_dma_addr(page);
>> +		dma += xdpf->data - (void *)xdpf;
>
>This code is only okay because this only happens for XDP_TX, where you
>know this head-room calculation will be true.  The "correct"
>calculation of the head-room would be:
>
>  dma += xdpf->headroom + sizeof(struct xdp_frame);
>
>The reason behind not using xdpf pointer itself as "data_hard_start",
>is to allow struct xdp_frame to be located in another memory area.

My assumption was based on:

struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
{
	...
	xdp_frame = xdp->data_hard_start;
	...

	xdp_frame->headroom = headroom - sizeof(*xdp_frame);
	...
}

But agree, it doesn't contradict the reason in question.
So, better use proposed variant. Will check and do this in v8 a little later:

dma += xdpf->headroom + sizeof(struct xdp_frame);

>This will be useful for e.g. AF_XDP transmit, or other zero-copy
>transmit to go through ndo_xdp_xmit() (as we don't want userspace to
>be-able to e.g. "race" change xdpf->len during transmit/DMA-completion).
>
>
>> +		ret = cpdma_chan_submit_mapped(txch, cpsw_xdpf_to_handle(xdpf),
>> +					       dma, xdpf->len, port);
>> +	} else {
>> +		if (sizeof(*xmeta) > xdpf->headroom) {
>> +			xdp_return_frame_rx_napi(xdpf);
>> +			return -EINVAL;
>> +		}
>> +
>> +		ret = cpdma_chan_submit(txch, cpsw_xdpf_to_handle(xdpf),
>> +					xdpf->data, xdpf->len, port);
>> +	}
>
>
>
>-- 
>Best regards,
>  Jesper Dangaard Brouer
>  MSc.CS, Principal Kernel Engineer at Red Hat
>  LinkedIn: http://www.linkedin.com/in/brouer

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* [PATCH net-next 1/2] net: mvpp2: cls: Report an error for unsupported flow types
From: Maxime Chevallier @ 2019-07-05 12:09 UTC (permalink / raw)
  To: davem
  Cc: Maxime Chevallier, netdev, linux-kernel, linux-arm-kernel,
	Antoine Tenart, thomas.petazzoni, gregory.clement, miquel.raynal,
	nadavh, stefanc, mw
In-Reply-To: <20190705120913.25013-1-maxime.chevallier@bootlin.com>

Add a missing check to detect flow types that we don't support, so that
user can be informed of this.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
index b195fb5d61f4..6c088c903c15 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
@@ -1373,6 +1373,10 @@ int mvpp2_ethtool_cls_rule_ins(struct mvpp2_port *port,
 
 	efs->rule.flow = ethtool_rule->rule;
 	efs->rule.flow_type = mvpp2_cls_ethtool_flow_to_type(info->fs.flow_type);
+	if (efs->rule.flow_type < 0) {
+		ret = efs->rule.flow_type;
+		goto clean_rule;
+	}
 
 	ret = mvpp2_cls_rfs_parse_rule(&efs->rule);
 	if (ret)
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 2/2] net: mvpp2: cls: Add support for ETHER_FLOW
From: Maxime Chevallier @ 2019-07-05 12:09 UTC (permalink / raw)
  To: davem
  Cc: Maxime Chevallier, netdev, linux-kernel, linux-arm-kernel,
	Antoine Tenart, thomas.petazzoni, gregory.clement, miquel.raynal,
	nadavh, stefanc, mw
In-Reply-To: <20190705120913.25013-1-maxime.chevallier@bootlin.com>

Users can specify classification actions based on the 'ether' flow type.
In that case, this will apply to all ethernet traffic, superseeding
flows such as 'udp4' or 'tcp6'.

Add support for this flow type in the PPv2 classifier, by mapping the
ETHER_FLOW value to the corresponding entries in the classifier.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
index 6c088c903c15..35478cba2aa5 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
@@ -548,6 +548,8 @@ void mvpp2_cls_c2_read(struct mvpp2 *priv, int index,
 static int mvpp2_cls_ethtool_flow_to_type(int flow_type)
 {
 	switch (flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS)) {
+	case ETHER_FLOW:
+		return MVPP22_FLOW_ETHERNET;
 	case TCP_V4_FLOW:
 		return MVPP22_FLOW_TCP4;
 	case TCP_V6_FLOW:
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 0/2] net: mvpp2: Add classification based on the ETHER flow
From: Maxime Chevallier @ 2019-07-05 12:09 UTC (permalink / raw)
  To: davem
  Cc: Maxime Chevallier, netdev, linux-kernel, linux-arm-kernel,
	Antoine Tenart, thomas.petazzoni, gregory.clement, miquel.raynal,
	nadavh, stefanc, mw

Hello everyone,

This series adds support for classification of the ETHER flow in the
mvpp2 driver.

The first patch allows detecting when a user specifies a flow_type that
isn't supported by the driver, while the second adds support for this
flow_type by adding the mapping between the ETHER_FLOW enum value and
the relevant classifier flow entries.

Thanks,

Maxime

Maxime Chevallier (2):
  net: mvpp2: cls: Report an error for unsupported flow types
  net: mvpp2: cls: Add support for ETHER_FLOW

 drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.20.1


^ permalink raw reply

* [PATCHv2] tools bpftool: Fix json dump crash on powerpc
From: Jiri Olsa @ 2019-07-05 12:10 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Michael Petlan,
	netdev, bpf, Martin KaFai Lau
In-Reply-To: <20190704134210.17b8407c@cakuba.netronome.com>

On Thu, Jul 04, 2019 at 01:42:10PM -0700, Jakub Kicinski wrote:
> On Thu,  4 Jul 2019 10:58:56 +0200, Jiri Olsa wrote:
> > Michael reported crash with by bpf program in json mode on powerpc:
> > 
> >   # bpftool prog -p dump jited id 14
> >   [{
> >         "name": "0xd00000000a9aa760",
> >         "insns": [{
> >                 "pc": "0x0",
> >                 "operation": "nop",
> >                 "operands": [null
> >                 ]
> >             },{
> >                 "pc": "0x4",
> >                 "operation": "nop",
> >                 "operands": [null
> >                 ]
> >             },{
> >                 "pc": "0x8",
> >                 "operation": "mflr",
> >   Segmentation fault (core dumped)
> > 
> > The code is assuming char pointers in format, which is not always
> > true at least for powerpc. Fixing this by dumping the whole string
> > into buffer based on its format.
> > 
> > Please note that libopcodes code does not check return values from
> > fprintf callback, so there's no point to return error in case of
> > allocation failure.
> 
> Well, it doesn't check it today, it may perhaps do it in the future?
> Let's flip the question - since it doesn't check it today, why not
> propagate the error? :)  We should stay close to how fprintf would
> behave, IMHO.
> 
> Fixes: 107f041212c1 ("tools: bpftool: add JSON output for `bpftool prog dump jited *` command")

ok fair enough, v2 attached

thanks,
jirka


---
Michael reported crash with by bpf program in json mode on powerpc:

  # bpftool prog -p dump jited id 14
  [{
        "name": "0xd00000000a9aa760",
        "insns": [{
                "pc": "0x0",
                "operation": "nop",
                "operands": [null
                ]
            },{
                "pc": "0x4",
                "operation": "nop",
                "operands": [null
                ]
            },{
                "pc": "0x8",
                "operation": "mflr",
  Segmentation fault (core dumped)

The code is assuming char pointers in format, which is not always
true at least for powerpc. Fixing this by dumping the whole string
into buffer based on its format.

Please note that libopcodes code does not check return values from
fprintf callback, but as per Jakub suggestion returning -1 on allocation
failure so we do the best effort to propagate the error. 

Reported-by: Michael Petlan <mpetlan@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpftool/jit_disasm.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 3ef3093560ba..bfed711258ce 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -11,6 +11,8 @@
  * Licensed under the GNU General Public License, version 2.0 (GPLv2)
  */
 
+#define _GNU_SOURCE
+#include <stdio.h>
 #include <stdarg.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -44,11 +46,13 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	char *s;
 
 	va_start(ap, fmt);
+	if (vasprintf(&s, fmt, ap) < 0)
+		return -1;
+	va_end(ap);
+
 	if (!oper_count) {
 		int i;
 
-		s = va_arg(ap, char *);
-
 		/* Strip trailing spaces */
 		i = strlen(s) - 1;
 		while (s[i] == ' ')
@@ -61,11 +65,10 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	} else if (!strcmp(fmt, ",")) {
 		   /* Skip */
 	} else {
-		s = va_arg(ap, char *);
 		jsonw_string(json_wtr, s);
 		oper_count++;
 	}
-	va_end(ap);
+	free(s);
 	return 0;
 }
 
-- 
2.21.0


^ permalink raw reply related

* Re: kernel BUG at net/rxrpc/local_object.c:LINE!
From: Dmitry Vyukov @ 2019-07-05 12:12 UTC (permalink / raw)
  To: David Howells
  Cc: syzbot, Eric Biggers, David Miller, linux-afs, LKML, netdev,
	syzkaller-bugs
In-Reply-To: <24282.1562074644@warthog.procyon.org.uk>

,On Tue, Jul 2, 2019 at 3:37 PM David Howells <dhowells@redhat.com> wrote:
>
> syzbot <syzbot+1e0edc4b8b7494c28450@syzkaller.appspotmail.com> wrote:
>
> I *think* the reproducer boils down to the attached, but I can't get syzkaller
> to work and the attached sample does not cause the oops to occur.  Can you try
> it in your environment?
>
> > The bug was bisected to:
> >
> > commit 46894a13599a977ac35411b536fb3e0b2feefa95
> > Author: David Howells <dhowells@redhat.com>
> > Date:   Thu Oct 4 08:32:28 2018 +0000
> >
> >     rxrpc: Use IPv4 addresses throught the IPv6
>
> This might not be the correct bisection point.  If you look at the attached
> sample, you're mixing AF_INET and AF_INET6.  If you try AF_INET throughout,
> that might get a different point.  On the other hand, since you've bound the
> socket, the AF_INET6 passed to socket() should be ignored.
>
> David
> ---
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <sys/socket.h>
> #include <arpa/inet.h>
> #include <linux/rxrpc.h>
>
> static const unsigned char inet4_addr[4] = {
>         0xe0, 0x00, 0x00, 0x01
> };
>
> int main(void)
> {
>         struct sockaddr_rxrpc srx;
>         int fd;
>
>         memset(&srx, 0, sizeof(srx));
>         srx.srx_family                  = AF_RXRPC;
>         srx.srx_service                 = 0;
>         srx.transport_type              = AF_INET;
>         srx.transport_len               = sizeof(srx.transport.sin);
>         srx.transport.sin.sin_family    = AF_INET;
>         srx.transport.sin.sin_port      = htons(0x4e21);
>         memcpy(&srx.transport.sin.sin_addr, inet4_addr, 4);
>
>         fd = socket(AF_RXRPC, SOCK_DGRAM, AF_INET6);
>         if (fd == -1) {
>                 perror("socket");
>                 exit(1);
>         }
>
>         if (bind(fd, (struct sockaddr *)&srx, sizeof(srx)) == -1) {
>                 perror("bind");
>                 exit(1);
>         }
>
>         sleep(20);
>
>         // Whilst sleeping, hit with:
>         // echo -e '\0\0\0\0\0\0\0\0' | ncat -4u --send-only 224.0.0.1 20001
>
>         return 0;
> }

Hi David,

I can't re-reproduce it locally in qemu either. Though, syzbot managed
to re-reproduce it reliably during bisection (maybe there is some
difference in hardware and as the result the injected ethernet packet
would need some different values). Let's try to ask it again to make
sure:
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master

Re bisection, I don't know if there are some more subtle things as
play (you are in the better position to judge that), but bisection log
looks good, it tracked the target crash throughout and wasn't
distracted by any unrelated bugs, etc. So I don't see any obvious
reasons to not trust it.

^ permalink raw reply

* Re: kernel BUG at net/rxrpc/local_object.c:LINE!
From: Dmitry Vyukov @ 2019-07-05 12:15 UTC (permalink / raw)
  To: David Howells
  Cc: syzbot, Eric Biggers, David Miller, linux-afs, LKML, netdev,
	syzkaller-bugs
In-Reply-To: <CACT4Y+YjdV8CqX5=PzKsHnLsJOzsydqiq3igYDm_=nSdmFo2YQ@mail.gmail.com>

On Fri, Jul 5, 2019 at 2:12 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> > syzbot <syzbot+1e0edc4b8b7494c28450@syzkaller.appspotmail.com> wrote:
> >
> > I *think* the reproducer boils down to the attached, but I can't get syzkaller
> > to work and the attached sample does not cause the oops to occur.  Can you try
> > it in your environment?
> >
> > > The bug was bisected to:
> > >
> > > commit 46894a13599a977ac35411b536fb3e0b2feefa95
> > > Author: David Howells <dhowells@redhat.com>
> > > Date:   Thu Oct 4 08:32:28 2018 +0000
> > >
> > >     rxrpc: Use IPv4 addresses throught the IPv6
> >
> > This might not be the correct bisection point.  If you look at the attached
> > sample, you're mixing AF_INET and AF_INET6.  If you try AF_INET throughout,
> > that might get a different point.  On the other hand, since you've bound the
> > socket, the AF_INET6 passed to socket() should be ignored.
> >
> > David
> > ---
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <unistd.h>
> > #include <sys/socket.h>
> > #include <arpa/inet.h>
> > #include <linux/rxrpc.h>
> >
> > static const unsigned char inet4_addr[4] = {
> >         0xe0, 0x00, 0x00, 0x01
> > };
> >
> > int main(void)
> > {
> >         struct sockaddr_rxrpc srx;
> >         int fd;
> >
> >         memset(&srx, 0, sizeof(srx));
> >         srx.srx_family                  = AF_RXRPC;
> >         srx.srx_service                 = 0;
> >         srx.transport_type              = AF_INET;
> >         srx.transport_len               = sizeof(srx.transport.sin);
> >         srx.transport.sin.sin_family    = AF_INET;
> >         srx.transport.sin.sin_port      = htons(0x4e21);
> >         memcpy(&srx.transport.sin.sin_addr, inet4_addr, 4);
> >
> >         fd = socket(AF_RXRPC, SOCK_DGRAM, AF_INET6);
> >         if (fd == -1) {
> >                 perror("socket");
> >                 exit(1);
> >         }
> >
> >         if (bind(fd, (struct sockaddr *)&srx, sizeof(srx)) == -1) {
> >                 perror("bind");
> >                 exit(1);
> >         }
> >
> >         sleep(20);
> >
> >         // Whilst sleeping, hit with:
> >         // echo -e '\0\0\0\0\0\0\0\0' | ncat -4u --send-only 224.0.0.1 20001
> >
> >         return 0;
> > }
>
> Hi David,
>
> I can't re-reproduce it locally in qemu either. Though, syzbot managed
> to re-reproduce it reliably during bisection (maybe there is some
> difference in hardware and as the result the injected ethernet packet
> would need some different values). Let's try to ask it again to make
> sure:
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> master
>
> Re bisection, I don't know if there are some more subtle things as
> play (you are in the better position to judge that), but bisection log
> looks good, it tracked the target crash throughout and wasn't
> distracted by any unrelated bugs, etc. So I don't see any obvious
> reasons to not trust it.

FWIW here is a more complete translation of the syzkaller repro to C using:

$ syz-prog2c -prog /tmp/prog -threaded -collide -repeat=0 -procs=6
-sandbox=namespace -enable=tun,net_dev,net_reset,cgroups,close_fds
-tmpdir -segv

https://gist.githubusercontent.com/dvyukov/f712ca7c3a0d355ce63823d7882c2934/raw/7a72635b99e5a85599a6bcf9b7901fa9d8c494d4/repro.c

However, both syzbot and me won't able to repro with this C program,
so it is expected that it does not reproduce the crash for some
reason.

^ permalink raw reply

* Re: i.mx6ul with DSA in multi chip addressing mode - no MDIO access
From: Benjamin Beckmeyer @ 2019-07-05 12:41 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20190704155347.GJ18473@lunn.ch>

>> &mdio0 {
>>         interrupt-parent = <&gpio1>;
>>         interrupts = <3 IRQ_TYPE_LEVEL_HIGH>;
>>
>>         switch0: switch0@2 {
>>                 compatible = "marvell,mv88e6190";
>>                 reg = <2>;
>>                 pinctrl-0 = <&pinctrl_gpios>;
>>                 reset-gpios = <&gpio4 16 GPIO_ACTIVE_LOW>;
>>                 dsa,member = <0 0>;
> This is wrong. The interrupt is a switch property, not an MDIO bus
> property. So it belongs inside the switch node.
>
> 	  Andrew

Hi Andrew,

in the documentation for Marvell DSA the interrupt properties are in 
the MDIO part. Maybe the documentation for device tree is wrong or 
unclear?

I switched to the kernel 5.1.16 to take advantage of your new code.
At the moment I deleted all interrupt properties from my device tree 
and if I get you right now the access should be trigger all 100ms but 
I have accesses within the tracing about 175 times a second.

Here is a snip from my trace without IRQ
2188000.etherne-223   [000] ....   109.932406: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x01 val:0x40a8
 2188000.etherne-223   [000] ....   109.932501: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b64
 2188000.etherne-223   [000] ....   109.933113: mdio_access: 2188000.ethernet-1 write phy:0x02 reg:0x00 val:0x9b60
 2188000.etherne-223   [000] ....   109.933261: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b60
 2188000.etherne-223   [000] ....   109.933359: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x01 val:0xc801
 2188000.etherne-223   [000] ....   110.041683: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b60
 2188000.etherne-223   [000] ....   110.041817: mdio_access: 2188000.ethernet-1 write phy:0x02 reg:0x00 val:0x9b60
 2188000.etherne-223   [000] ....   110.041919: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b60
 2188000.etherne-223   [000] ....   110.042025: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x01 val:0xc801

Am I doing it right with the tracing points? I run just

echo 1 > /sys/kernel/debug/tracing/events/mdio/mdio_access/enable
cat /sys/kernel/debug/tracing/trace

Here is the another device tree I tried, but with this I get accesses 
on the bus in about every 50 microseconds!

--snip
&mdio0 {
        switch0: switch0@2 {
                compatible = "marvell,mv88e6190";
                reg = <2>;
                pinctrl-0 = <&pinctrl_switch_irq>;
                interrupt-parent = <&gpio1>;
                interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
                interrupt-controller;
                #interrupt-cells = <2>;
                dsa,member = <0 0>;

                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;
--snip

Here is a snip from my trace with IRQ.
irq/54-2188000.-223   [000] ....   958.940744: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b64
 irq/54-2188000.-223   [000] ....   958.940800: mdio_access: 2188000.ethernet-1 write phy:0x02 reg:0x00 val:0x9b60
 irq/54-2188000.-223   [000] ....   958.940857: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b60
 irq/54-2188000.-223   [000] ....   958.940914: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x01 val:0xc801
 irq/54-2188000.-223   [000] ....   958.940984: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b60
 irq/54-2188000.-223   [000] ....   958.941043: mdio_access: 2188000.ethernet-1 write phy:0x02 reg:0x00 val:0x9b60
 irq/54-2188000.-223   [000] ....   958.941100: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b60
 irq/54-2188000.-223   [000] ....   958.941158: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x01 val:0xc801
 irq/54-2188000.-223   [000] ....   958.941218: mdio_access: 2188000.ethernet-1 read  phy:0x02 reg:0x00 val:0x1b60
 irq/54-2188000.-223   [000] ....   958.941276: mdio_access: 2188000.ethernet-1 write phy:0x02 reg:0x00 val:0x9b64

Thanks,
Benny


^ permalink raw reply

* [PATCH net-next] MAINTAINERS: Add page_pool maintainer entry
From: Jesper Dangaard Brouer @ 2019-07-05 12:42 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, ilias.apalodimas, netdev
  Cc: daniel, jakub.kicinski, john.fastabend, ast

In this release cycle the number of NIC drivers using page_pool
will likely reach 4 drivers.  It is about time to add a maintainer
entry.  Add myself.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 MAINTAINERS |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 449e7cdb3303..1a8e0a01bf03 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11902,6 +11902,13 @@ F:	kernel/padata.c
 F:	include/linux/padata.h
 F:	Documentation/padata.txt
 
+PAGE POOL
+M:	Jesper Dangaard Brouer <hawk@kernel.org>
+L:	netdev@vger.kernel.org
+S:	Supported
+F:	net/core/page_pool.c
+F:	include/net/page_pool.h
+
 PANASONIC LAPTOP ACPI EXTRAS DRIVER
 M:	Harald Welte <laforge@gnumonks.org>
 L:	platform-driver-x86@vger.kernel.org


^ permalink raw reply related

* [PATCH net-next V2] MAINTAINERS: Add page_pool maintainer entry
From: Jesper Dangaard Brouer @ 2019-07-05 12:57 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, ilias.apalodimas, netdev
  Cc: daniel, jakub.kicinski, john.fastabend, ast

In this release cycle the number of NIC drivers using page_pool
will likely reach 4 drivers.  It is about time to add a maintainer
entry.  Add myself and Ilias.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
V2: Ilias also volunteered to co-maintain over IRC

 MAINTAINERS |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 449e7cdb3303..22655aa84a46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11902,6 +11902,14 @@ F:	kernel/padata.c
 F:	include/linux/padata.h
 F:	Documentation/padata.txt
 
+PAGE POOL
+M:	Jesper Dangaard Brouer <hawk@kernel.org>
+M:	Ilias Apalodimas <ilias.apalodimas@linaro.org>
+L:	netdev@vger.kernel.org
+S:	Supported
+F:	net/core/page_pool.c
+F:	include/net/page_pool.h
+
 PANASONIC LAPTOP ACPI EXTRAS DRIVER
 M:	Harald Welte <laforge@gnumonks.org>
 L:	platform-driver-x86@vger.kernel.org


^ permalink raw reply related

* Re: INFO: rcu detected stall in ext4_write_checks
From: Dmitry Vyukov @ 2019-07-05 13:18 UTC (permalink / raw)
  To: Theodore Ts'o, syzbot, Andreas Dilger, David Miller, eladr,
	Ido Schimmel, Jiri Pirko, John Stultz, linux-ext4, LKML, netdev,
	syzkaller-bugs, Thomas Gleixner
  Cc: syzkaller
In-Reply-To: <20190626184251.GE3116@mit.edu>

On Wed, Jun 26, 2019 at 8:43 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Wed, Jun 26, 2019 at 10:27:08AM -0700, syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:    abf02e29 Merge tag 'pm-5.2-rc6' of git://git.kernel.org/pu..
> > git tree:       upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=1435aaf6a00000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=e5c77f8090a3b96b
> > dashboard link: https://syzkaller.appspot.com/bug?extid=4bfbbf28a2e50ab07368
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11234c41a00000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=15d7f026a00000
> >
> > The bug was bisected to:
> >
> > commit 0c81ea5db25986fb2a704105db454a790c59709c
> > Author: Elad Raz <eladr@mellanox.com>
> > Date:   Fri Oct 28 19:35:58 2016 +0000
> >
> >     mlxsw: core: Add port type (Eth/IB) set API
>
> Um, so this doesn't pass the laugh test.
>
> > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=10393a89a00000
>
> It looks like the automated bisection machinery got confused by two
> failures getting triggered by the same repro; the symptoms changed
> over time.  Initially, the failure was:
>
> crashed: INFO: rcu detected stall in {sys_sendfile64,ext4_file_write_iter}
>
> Later, the failure changed to something completely different, and much
> earlier (before the test was even started):
>
> run #5: basic kernel testing failed: failed to copy test binary to VM: failed to run ["scp" "-P" "22" "-F" "/dev/null" "-o" "UserKnownHostsFile=/dev/null" "-o" "BatchMode=yes" "-o" "IdentitiesOnly=yes" "-o" "StrictHostKeyChecking=no" "-o" "ConnectTimeout=10" "-i" "/syzkaller/jobs/linux/workdir/image/key" "/tmp/syz-executor216456474" "root@10.128.15.205:./syz-executor216456474"]: exit status 1
> Connection timed out during banner exchange
> lost connection
>
> Looks like an opportunity to improve the bisection engine?

Hi Ted,

Yes, these infrastructure errors plague bisections episodically.
That's https://github.com/google/syzkaller/issues/1250

It did not confuse bisection explicitly as it understands that these
are infrastructure failures rather then a kernel crash, e.g. here you
may that it correctly identified that this run was OK and started
bisection in v4.10 v4.9 range besides 2 scp failures:

testing release v4.9
testing commit 69973b830859bc6529a7a0468ba0d80ee5117826 with gcc (GCC) 5.5.0
run #0: basic kernel testing failed: failed to copy test binary to VM:
failed to run ["scp" ...]: exit status 1
Connection timed out during banner exchange
run #1: basic kernel testing failed: failed to copy test binary to VM:
failed to run ["scp" ....]: exit status 1
Connection timed out during banner exchange
run #2: OK
run #3: OK
run #4: OK
run #5: OK
run #6: OK
run #7: OK
run #8: OK
run #9: OK
# git bisect start v4.10 v4.9

Though, of course, it may confuse bisection indirectly by reducing
number of tests per commit.

So far I wasn't able to gather any significant info about these
failures. We gather console logs, but on these runs they are empty.
It's easy to blame everything onto GCE but I don't have any bit of
information that would point either way. These failures just appear
randomly in production and usually in batches...

^ permalink raw reply

* Re: [PATCH net-next 4/6] arm64: dts: fsl: ls1028a: Add Felix switch port DT node
From: Andrew Lunn @ 2019-07-05 13:19 UTC (permalink / raw)
  To: Claudiu Manoil
  Cc: Vladimir Oltean, Alexandre Belloni, Allan W. Nielsen,
	David S . Miller, devicetree@vger.kernel.org,
	netdev@vger.kernel.org, Alexandru Marginean,
	linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com,
	Allan Nielsen, Rob Herring, linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB4880DEA9D7836A68E0EE141396F50@VI1PR04MB4880.eurprd04.prod.outlook.com>

> Nice discussion, again, but there's a missing point that has not been
> brought up yet.  We actually intend to support the following hardware
> configuration: a single PCI device consisting of the Microsemi's switch core
> and our DMA rings.
> The hardware supports this configuration into a single PCI function (PF), 
> with a unique PCI function id (0xe111), so that the same driver has access to 
> both switch registers and DMA rings connected to the CPU port.  This device
> would qualify  as a  switchdev device, and we can simply reuse the existing
> ocelot code for the switch core part.  The initial patch set was the first step in
> supporting the switch core on our platform, we just need to add the support
> for the DMA rings part, to make it a complete switchdev solution.

Hi Claudiu

It sound like in the end you will have a core library and then two
drivers wrapped around it, giving a pure switchdev device with polled
IO or DMA, and a DSA driver using a CPU port.

   Andrew

^ permalink raw reply

* Re: INFO: rcu detected stall in ext4_write_checks
From: Dmitry Vyukov @ 2019-07-05 13:24 UTC (permalink / raw)
  To: Theodore Ts'o, syzbot, Andreas Dilger, David Miller, eladr,
	Ido Schimmel, Jiri Pirko, John Stultz, linux-ext4, LKML, netdev,
	syzkaller-bugs, Thomas Gleixner, Peter Zijlstra, Ingo Molnar,
	Paul E. McKenney
In-Reply-To: <20190626224709.GH3116@mit.edu>

On Thu, Jun 27, 2019 at 12:47 AM Theodore Ts'o <tytso@mit.edu> wrote:
>
> More details about what is going on.  First, it requires root, because
> one of that is required is using sched_setattr (which is enough to
> shoot yourself in the foot):
>
> sched_setattr(0, {size=0, sched_policy=0x6 /* SCHED_??? */, sched_flags=0, sched_nice=0, sched_priority=0, sched_runtime=2251799813724439, sched_deadline=4611686018427453437, sched_period=0}, 0) = 0
>
> This is setting the scheduler policy to be SCHED_DEADLINE, with a
> runtime parameter of 2251799.813724439 seconds (or 26 days) and a
> deadline of 4611686018.427453437 seconds (or 146 *years*).  This means
> a particular kernel thread can run for up to 26 **days** before it is
> scheduled away, and if a kernel reads gets woken up or sent a signal,
> no worries, it will wake up roughly seven times the interval that Rip
> Van Winkle spent snoozing in a cave in the Catskill Mountains (in
> Washington Irving's short story).
>
> We then kick off a half-dozen threads all running:
>
>    sendfile(fd, fd, &pos, 0x8080fffffffe);
>
> (and since count is a ridiculously large number, this gets cut down to):
>
>    sendfile(fd, fd, &pos, 2147479552);
>
> Is it any wonder that we are seeing RCU stalls?   :-)

+Peter, Ingo for sched_setattr and +Paul for rcu

First of all: is it a semi-intended result of a root (CAP_SYS_NICE)
doing local DoS abusing sched_setattr? It would perfectly reasonable
to starve other processes, but I am not sure about rcu. In the end the
high prio process can use rcu itself, and then it will simply blow
system memory by stalling rcu. So it seems that rcu stalls should not
happen as a result of weird sched_setattr values. If that is the case,
what needs to be fixed? sched_setattr? rcu? sendfile?

If this is semi-intended, the only option I see is to disable
something in syzkaller: sched_setattr entirely, or drop CAP_SYS_NICE,
or ...? Any preference either way?

^ permalink raw reply

* Re: [PATCH net-next v3 3/3] net: stmmac: Introducing support for Page Pool
From: Ilias Apalodimas @ 2019-07-05 13:29 UTC (permalink / raw)
  To: Jose Abreu
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel, Joao Pinto,
	David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
	Jesper Dangaard Brouer, Arnd Bergmann
In-Reply-To: <384dab52828c4b65596ef4202562a574eed93b91.1562311299.git.joabreu@synopsys.com>

Hi Jose,

I think this look ok for now. One request though, on page_pool_free 

On Fri, Jul 05, 2019 at 09:23:00AM +0200, Jose Abreu wrote:
> Mapping and unmapping DMA region is an high bottleneck in stmmac driver,
> specially in the RX path.
> 
> This commit introduces support for Page Pool API and uses it in all RX
> queues. With this change, we get more stable troughput and some increase
> of banwidth with iperf:
> 	- MAC1000 - 950 Mbps
> 	- XGMAC: 9.22 Gbps
> 
> Changes from v2:
> 	- Uncoditionally call page_pool_free() (Jesper)
> Changes from v1:
> 	- Use page_pool_get_dma_addr() (Jesper)
> 	- Add a comment (Jesper)
> 	- Add page_pool_free() call (Jesper)
> 	- Reintroduce sync_single_for_device (Arnd / Ilias)
> 
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/stmicro/stmmac/Kconfig       |   1 +
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h      |  10 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 203 +++++++---------------
>  3 files changed, 70 insertions(+), 144 deletions(-)
> 

[...]
> @@ -1498,8 +1480,11 @@ static void free_dma_rx_desc_resources(struct stmmac_priv *priv)
>  					  sizeof(struct dma_extended_desc),
>  					  rx_q->dma_erx, rx_q->dma_rx_phy);
>  
> -		kfree(rx_q->rx_skbuff_dma);
> -		kfree(rx_q->rx_skbuff);
> +		kfree(rx_q->buf_pool);
> +		if (rx_q->page_pool) {
> +			page_pool_request_shutdown(rx_q->page_pool);
> +			page_pool_free(rx_q->page_pool);

A patch currently under review will slightly change that [1] and [2]
Can you defer this a bit till that one gets merged?
The only thing you'll have to do is respin this and replace page_pool_free()
with page_pool_destroy()

[1] https://lore.kernel.org/netdev/20190705094346.13b06da6@carbon/
[2] https://lore.kernel.org/netdev/156225871578.1603.6630229522953924907.stgit@firesoul/

^ permalink raw reply

* Re: [PATCH net-next 1/8] Documentation/bindings: net: ocelot: document the PTP bank
From: Antoine Tenart @ 2019-07-05 13:30 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Antoine Tenart, davem, richardcochran, alexandre.belloni,
	UNGLinuxDriver, ralf, paul.burton, jhogan, netdev, linux-mips,
	thomas.petazzoni, allan.nielsen
In-Reply-To: <20190701135214.GD25795@lunn.ch>

Hi Andrew,

On Mon, Jul 01, 2019 at 03:52:14PM +0200, Andrew Lunn wrote:
> On Mon, Jul 01, 2019 at 12:03:20PM +0200, Antoine Tenart wrote:
> > One additional register range needs to be described within the Ocelot
> > device tree node: the PTP. This patch documents the binding needed to do
> > so.
> 
> Are there any more register banks? Maybe just add them all?

I checked and there are (just a few) more. I also saw your other comment
about interrupts, and it's also true there.

Those definitions aren't related to the PHC so I'll prepare a patch for
a following series to add all the missing parts.

> Also, you should probably add a comment that despite it being in the
> Required part of the binding, it is actually optional.

I'm not sure about this: optional properties means some parts of the h/w
can be missing or not wired. It's not the case here, it's "optional" in
the driver only for dt compatibility (so that an older dt blob can work
with a newer kernel image), but it's now mandatory in the binding.

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply


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