* [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
* Request for backport of 96125bf9985a75db00496dd2bc9249b777d2b19b
From: Loganaden Velvindron @ 2019-07-05 14:15 UTC (permalink / raw)
To: netdev; +Cc: Dave Taht
Hi folks,
I read the guidelines for LTS/stable.
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
Although this is not a bugfix, I am humbly submitting a request so
that commit id
-- 96125bf9985a75db00496dd2bc9249b777d2b19b Allow 0.0.0.0/8 as a valid
address range -- is backported to all LTS kernels.
My motivation for such a request is that we need this patch to be as
widely deployed as possible and as early as possible for interop and
hopefully move into better utilization of ipv4 addresses space. Hence
my request for it be added to -stable.
Kind regards,
//Logan
^ 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 14:15 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Alexandre Belloni, Allan W. Nielsen, Claudiu Manoil,
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: <CA+h21hqU1H1PefBWKjnsmkMsLhx0p0HJTsp-UYrSgmVnsfqULA@mail.gmail.com>
> Let me see if I get your point.
> The D is optional, and the S is optional. So what's left? :)
A collection of interfaces.
switchdev and by extension DSA is all about using hardware as an
accelerator. Linux can do switching in software. It can do routing in
software, it can do bonding in software, etc. switchdev gives you an
API to offload this to hardware. And if the hardware or the driver
does not support it, linux will just keep on doing it in software.
Once you get the idea it is just an accelerator, the rest falls into
place. Why there are no new configuration tools, why we expect network
daemons to just work, why users don't need to learn anything new. It
is all just linux networking as normal.
> Also, there's a big difference between "the hardware can't do it" and
> "the driver doesn't implement it". If I follow your argument, would
> you write a DSA driver for a device that doesn't do L2 switching?
Sure i would. Such a device is a port multiplexor. The user sees the
user ports as linux interfaces. They can use those interfaces just
like any other linux interfaces. Configure them using iproute2, add
them to bridges, run bonding, etc. All just linux as normal.
> Along that same line, what benefit does the DSA model bring to a
> switch that can't do cascading, compared to switchdev? I'm asking this
> as a user, not as a developer.
DSA builds on top of switchdev. switchdev gives you an API to offload
things which are happening in software to the hardware. It is the glue
which allows you to configure the accelerator.
There is also a common pattern for some switches. They connect a
switch port MAC to a host port MAC, so that frames can be passed from
the CPU to the switch. This pattern is common enough that
infrastructure has been put in place to support this model. That
infrastructure is DSA.
But that is mostly about details for the driver writer. From the users
perspective, you have a bunch of interfaces which you just use as
normal, and some stuff can get accelerated by the hardware. We don't
want the user to have to known about, or do anything, with the host
port or the switch port it is connected to. DSA very nearly takes care
of everything to do with those two. The only thing you need to do is
configure the master interface up. Then things just work as a bunch of
Linux interfaces.
Now think about a pure switchdev switch, with a port connected to the
host. The model breaks down. How do you use that link to the switch?
It is just a plane link. You would not have tagging in operation. So
you cannot send frames out specific ports. In order to do that, you
need to add vlans, and configure the switch to map vlans to ports,
etc. You also then have two linux interfaces representing one
port. The pure switchdev interface, and the VLAN interface. That is
going to confuse the user. You SNMP agent is not just going to
work. You get the statistics from the pure switchdev interface, but
the IP configuration from the vlan interface? How do you bridge two
ports together? You need to put the same VLAN on two interfaces. Where
as the DSA model you just use linux as normal and create a bridge, add
the two interfaces, and then the stack transparently offloads it to
the accelerator. How does STP work?
Using DSA without using the D means switch ports just work as linux
interfaces.
>
> > > So my conclusion is that DSA for Felix/Ocelot doesn't make a lot of
> > > sense if the whole purpose is to hide the CPU-facing netdev.
> >
> > You actually convinced me the exact opposite. You described the
> > headers which are needed to implement DSA. The switch sounds like it
> > can do what DSA requires. So DSA is the correct model.
> >
> > Andrew
>
> Somebody actually asked, with the intention of building a board, if
> it's possible to cascade the LS1028A embedded switch (Felix) with
> discrete SJA1105 devices - Felix being at the top of the switch tree.
Florian has done something very similar. He used a Broadcom SoC with
an in built SF2 switch, and an external B53 roboswitch connected to
one of the SF2 ports. But in that setup, the master interface for the
b53 is a slave port of the SF2. Configure everything in device tree,
bring up the SoC master interface, then the SF2 port acting as a
master interface for the B53, and everything just worked. You have a
bunch of Linux interfaces you can just use as normal.
This is not using D in DSA. It is two instances of DSA. But because
the model is that you get normal linux interfaces, it just works. I
don't see why you cannot do the same with a LS1028A and a SJA1105.
Andrew
^ permalink raw reply
* Re: NEIGH: BUG, double timer add, state is 8
From: David Ahern @ 2019-07-05 14:28 UTC (permalink / raw)
To: Marek Majkowski, David Miller; +Cc: netdev, kernel-team
In-Reply-To: <CAJPywTJWQ9ACrp0naDn0gikU4P5-xGcGrZ6ZOKUeeC3S-k9+MA@mail.gmail.com>
On 7/4/19 3:59 PM, Marek Majkowski wrote:
> I found a way to hit an obscure BUG in the
> net/core/neighbour.c:neigh_add_timer(), by piping two carefully
> crafted messages into AF_NETLINK socket.
>
> https://github.com/torvalds/linux/blob/v5.2-rc7/net/core/neighbour.c#L259
>
> if (unlikely(mod_timer(&n->timer, when))) {
> printk("NEIGH: BUG, double timer add, state is %x\n", n->nud_state);
> dump_stack();
> }
>
> The repro is here:
> https://gist.github.com/majek/d70297b9d72bc2e2b82145e122722a0c
>
> wget https://gist.githubusercontent.com/majek/d70297b9d72bc2e2b82145e122722a0c/raw/9e140bcedecc28d722022f1da142a379a9b7a7b0/double_timer_add_bug.c
Thanks for the report - and the reproducer. I am on PTO through Monday;
I will take a look next week if no one else does.
^ permalink raw reply
* Re: [net-next 14/14] net/mlx5e: Add kTLS TX HW offload support
From: Tariq Toukan @ 2019-07-05 14:31 UTC (permalink / raw)
To: Jakub Kicinski, Saeed Mahameed
Cc: David S. Miller, netdev@vger.kernel.org, Eran Ben Elisha,
Boris Pismenny
In-Reply-To: <20190704131237.239bfa56@cakuba.netronome.com>
On 7/4/2019 11:12 PM, Jakub Kicinski wrote:
> On Thu, 4 Jul 2019 18:16:15 +0000, Saeed Mahameed wrote:
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> index 483d321d2151..6854f132d505 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> @@ -50,6 +50,15 @@ static const struct counter_desc sw_stats_desc[] = {
>> #ifdef CONFIG_MLX5_EN_TLS
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_tls_ooo) },
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_tls_resync_bytes) },
>> +
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_ooo) },
>
> Why do you call this stat tx_ktls_ooo, and not tx_tls_ooo (extra 'k')?
>
> For nfp I used the stats' names from mlx5 FPGA to make sure we are all
> consistent. I've added them to the tls-offload.rst doc and Boris has
> reviewed it.
>
> * ``rx_tls_decrypted`` - number of successfully decrypted TLS segments
> * ``tx_tls_encrypted`` - number of in-order TLS segments passed to device
> for encryption
> * ``tx_tls_ooo`` - number of TX packets which were part of a TLS stream
> but did not arrive in the expected order
> * ``tx_tls_drop_no_sync_data`` - number of TX packets dropped because
> they arrived out of order and associated record could not be found
>
> Why can't you use the same names for the stats as you used for your mlx5
> FPGA?
>
Agree. Fixing.
What about having stats both for packets and bytes?
tx_tls_encrypted_packets
tx_tls_encrypted_bytes
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_ooo_drop_no_sync_data) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_ooo_drop_bypass_req) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_ooo_dump_bytes) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_ooo_dump_packets) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_enc_packets) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_enc_bytes) },
>> + { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_ctx) },
>> #endif
>>
>> { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_lro_packets) },
>
> Dave, please don't apply this, I will review in depth once I get
> through the earlier 200 emails ;)
>
^ permalink raw reply
* Re: [PATCH bpf-next v3] libbpf: add xsk_ring_prod__nb_free() function
From: Daniel Borkmann @ 2019-07-05 14:35 UTC (permalink / raw)
To: Eelco Chaudron, netdev
Cc: ast, kafai, songliubraving, yhs, andrii.nakryiko, magnus.karlsson
In-Reply-To: <ea49f66f73aedcdade979605dab6b2474e2dc4cb.1562145300.git.echaudro@redhat.com>
On 07/03/2019 02:52 PM, Eelco Chaudron wrote:
> When an AF_XDP application received X packets, it does not mean X
> frames can be stuffed into the producer ring. To make it easier for
> AF_XDP applications this API allows them to check how many frames can
> be added into the ring.
>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
The commit log as it is along with the code is a bit too confusing for
readers. After all you only do a rename below. It would need to additionally
state that the rename is as per libbpf convention (xyz__ prefix) in order to
denote that this API is exposed to be used by applications.
Given you are doing this for xsk_prod_nb_free(), should we do the same for
xsk_cons_nb_avail() as well? Extending XDP sample app would be reasonable
addition as well in this context.
> ---
>
> v2 -> v3
> - Removed cache by pass option
>
> v1 -> v2
> - Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
> - Add caching so it will only touch global state when needed
>
> tools/lib/bpf/xsk.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 82ea71a0f3ec..3411556e04d9 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -76,7 +76,7 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
> return &descs[idx & rx->mask];
> }
>
> -static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
> +static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32 nb)
> {
> __u32 free_entries = r->cached_cons - r->cached_prod;
>
> @@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
> static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
> size_t nb, __u32 *idx)
> {
> - if (xsk_prod_nb_free(prod, nb) < nb)
> + if (xsk_prod__nb_free(prod, nb) < nb)
> return 0;
>
> *idx = prod->cached_prod;
>
^ 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