* [PATCH net] sctp: count data bundling sack chunk for outctrlchunks
From: Xin Long @ 2019-07-03 10:20 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman
Now all ctrl chunks are counted for asoc stats.octrlchunks and net
SCTP_MIB_OUTCTRLCHUNKS either after queuing up or bundling, other
than the chunk maked and bundled in sctp_packet_bundle_sack, which
caused 'outctrlchunks' not consistent with 'inctrlchunks' in peer.
This issue exists since very beginning, here to fix it by increasing
both net SCTP_MIB_OUTCTRLCHUNKS and asoc stats.octrlchunks when sack
chunk is maked and bundled in sctp_packet_bundle_sack.
Reported-by: Ja Ram Jeon <jajeon@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/output.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/sctp/output.c b/net/sctp/output.c
index e0c2747..dbda7e7 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -282,6 +282,9 @@ static enum sctp_xmit sctp_packet_bundle_sack(struct sctp_packet *pkt,
sctp_chunk_free(sack);
goto out;
}
+ SCTP_INC_STATS(sock_net(asoc->base.sk),
+ SCTP_MIB_OUTCTRLCHUNKS);
+ asoc->stats.octrlchunks++;
asoc->peer.sack_needed = 0;
if (del_timer(timer))
sctp_association_put(asoc);
--
2.1.0
^ permalink raw reply related
* [PATCH v6 net-next 0/5] net: ethernet: ti: cpsw: Add XDP support
From: Ivan Khoronzhuk @ 2019-07-03 10:18 UTC (permalink / raw)
To: grygorii.strashko, hawk, davem
Cc: ast, linux-kernel, linux-omap, xdp-newbies, ilias.apalodimas,
netdev, daniel, jakub.kicinski, john.fastabend, Ivan Khoronzhuk
This patchset adds XDP support for TI cpsw driver and base it on
page_pool allocator. It was verified on af_xdp socket drop,
af_xdp l2f, ebpf XDP_DROP, XDP_REDIRECT, XDP_PASS, XDP_TX.
It was verified with following configs enabled:
CONFIG_JIT=y
CONFIG_BPFILTER=y
CONFIG_BPF_SYSCALL=y
CONFIG_XDP_SOCKETS=y
CONFIG_BPF_EVENTS=y
CONFIG_HAVE_EBPF_JIT=y
CONFIG_BPF_JIT=y
CONFIG_CGROUP_BPF=y
Link on previous v5:
https://lkml.org/lkml/2019/6/30/89
Also regular tests with iperf2 were done in order to verify impact on
regular netstack performance, compared with base commit:
https://pastebin.com/JSMT0iZ4
v5..v6:
- do changes that is rx_dev while redirect/flush cycle is kept the same
- dropped net: ethernet: ti: davinci_cpdma: return handler status
- other changes desc in patches
v4..v5:
- added two plreliminary patches:
net: ethernet: ti: davinci_cpdma: allow desc split while down
net: ethernet: ti: cpsw_ethtool: allow res split while down
- added xdp alocator refcnt on xdp level, avoiding page pool refcnt
- moved flush status as separate argument for cpdma_chan_process
- reworked cpsw code according to last changes to allocator
- added missed statistic counter
v3..v4:
- added page pool user counter
- use same pool for ndevs in dual mac
- restructured page pool create/destroy according to the last changes in API
v2..v3:
- each rxq and ndev has its own page pool
v1..v2:
- combined xdp_xmit functions
- used page allocation w/o refcnt juggle
- unmapped page for skb netstack
- moved rxq/page pool allocation to open/close pair
- added several preliminary patches:
net: page_pool: add helper function to retrieve dma addresses
net: page_pool: add helper function to unmap dma addresses
net: ethernet: ti: cpsw: use cpsw as drv data
net: ethernet: ti: cpsw_ethtool: simplify slave loops
Ivan Khoronzhuk (5):
xdp: allow same allocator usage
net: ethernet: ti: davinci_cpdma: add dma mapped submit
net: ethernet: ti: davinci_cpdma: allow desc split while down
net: ethernet: ti: cpsw_ethtool: allow res split while down
net: ethernet: ti: cpsw: add XDP support
drivers/net/ethernet/ti/Kconfig | 1 +
drivers/net/ethernet/ti/cpsw.c | 485 +++++++++++++++++++++---
drivers/net/ethernet/ti/cpsw_ethtool.c | 76 +++-
drivers/net/ethernet/ti/cpsw_priv.h | 7 +
drivers/net/ethernet/ti/davinci_cpdma.c | 99 ++++-
drivers/net/ethernet/ti/davinci_cpdma.h | 7 +-
include/net/xdp_priv.h | 2 +
net/core/xdp.c | 55 +++
8 files changed, 656 insertions(+), 76 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v6 net-next 1/5] xdp: allow same allocator usage
From: Ivan Khoronzhuk @ 2019-07-03 10:18 UTC (permalink / raw)
To: grygorii.strashko, hawk, davem
Cc: ast, linux-kernel, linux-omap, xdp-newbies, ilias.apalodimas,
netdev, daniel, jakub.kicinski, john.fastabend, Ivan Khoronzhuk
In-Reply-To: <20190703101903.8411-1-ivan.khoronzhuk@linaro.org>
First of all, it is an absolute requirement that each RX-queue have
their own page_pool object/allocator. And this change is intendant
to handle special case, where a single RX-queue can receive packets
from two different net_devices.
In order to protect against using same allocator for 2 different rx
queues, add queue_index to xdp_mem_allocator to catch the obvious
mistake where queue_index mismatch, as proposed by Jesper Dangaard
Brouer.
Adding this on xdp allocator level allows drivers with such dependency
change the allocators w/o modifications.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
include/net/xdp_priv.h | 2 ++
net/core/xdp.c | 55 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
index 6a8cba6ea79a..9858a4057842 100644
--- a/include/net/xdp_priv.h
+++ b/include/net/xdp_priv.h
@@ -18,6 +18,8 @@ struct xdp_mem_allocator {
struct rcu_head rcu;
struct delayed_work defer_wq;
unsigned long defer_warn;
+ unsigned long refcnt;
+ u32 queue_index;
};
#endif /* __LINUX_NET_XDP_PRIV_H__ */
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 829377cc83db..4f0ddbb3717a 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -98,6 +98,18 @@ static bool __mem_id_disconnect(int id, bool force)
WARN(1, "Request remove non-existing id(%d), driver bug?", id);
return true;
}
+
+ /* to avoid calling hash lookup twice, decrement refcnt here till it
+ * reaches zero, then it can be called from workqueue afterwards.
+ */
+ if (xa->refcnt)
+ xa->refcnt--;
+
+ if (xa->refcnt) {
+ mutex_unlock(&mem_id_lock);
+ return true;
+ }
+
xa->disconnect_cnt++;
/* Detects in-flight packet-pages for page_pool */
@@ -312,6 +324,33 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
return true;
}
+static struct xdp_mem_allocator *xdp_allocator_find(void *allocator)
+{
+ struct xdp_mem_allocator *xae, *xa = NULL;
+ struct rhashtable_iter iter;
+
+ if (!allocator)
+ return xa;
+
+ rhashtable_walk_enter(mem_id_ht, &iter);
+ do {
+ rhashtable_walk_start(&iter);
+
+ while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
+ if (xae->allocator == allocator) {
+ xa = xae;
+ break;
+ }
+ }
+
+ rhashtable_walk_stop(&iter);
+
+ } while (xae == ERR_PTR(-EAGAIN));
+ rhashtable_walk_exit(&iter);
+
+ return xa;
+}
+
int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
enum xdp_mem_type type, void *allocator)
{
@@ -347,6 +386,20 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
}
}
+ mutex_lock(&mem_id_lock);
+ xdp_alloc = xdp_allocator_find(allocator);
+ if (xdp_alloc) {
+ /* One allocator per queue is supposed only */
+ if (xdp_alloc->queue_index != xdp_rxq->queue_index)
+ return -EINVAL;
+
+ xdp_rxq->mem.id = xdp_alloc->mem.id;
+ xdp_alloc->refcnt++;
+ mutex_unlock(&mem_id_lock);
+ return 0;
+ }
+ mutex_unlock(&mem_id_lock);
+
xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
if (!xdp_alloc)
return -ENOMEM;
@@ -360,6 +413,8 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
xdp_rxq->mem.id = id;
xdp_alloc->mem = xdp_rxq->mem;
xdp_alloc->allocator = allocator;
+ xdp_alloc->refcnt = 1;
+ xdp_alloc->queue_index = xdp_rxq->queue_index;
/* Insert allocator into ID lookup table */
ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
--
2.17.1
^ permalink raw reply related
* [PATCH v6 net-next 2/5] net: ethernet: ti: davinci_cpdma: add dma mapped submit
From: Ivan Khoronzhuk @ 2019-07-03 10:19 UTC (permalink / raw)
To: grygorii.strashko, hawk, davem
Cc: ast, linux-kernel, linux-omap, xdp-newbies, ilias.apalodimas,
netdev, daniel, jakub.kicinski, john.fastabend, Ivan Khoronzhuk
In-Reply-To: <20190703101903.8411-1-ivan.khoronzhuk@linaro.org>
In case if dma mapped packet needs to be sent, like with XDP
page pool, the "mapped" submit can be used. This patch adds dma
mapped submit based on regular one.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
drivers/net/ethernet/ti/davinci_cpdma.c | 89 ++++++++++++++++++++++---
drivers/net/ethernet/ti/davinci_cpdma.h | 4 ++
2 files changed, 83 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 5cf1758d425b..8da46394c0e7 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -139,6 +139,7 @@ struct submit_info {
int directed;
void *token;
void *data;
+ int flags;
int len;
};
@@ -184,6 +185,8 @@ static struct cpdma_control_info controls[] = {
(directed << CPDMA_TO_PORT_SHIFT)); \
} while (0)
+#define CPDMA_DMA_EXT_MAP BIT(16)
+
static void cpdma_desc_pool_destroy(struct cpdma_ctlr *ctlr)
{
struct cpdma_desc_pool *pool = ctlr->pool;
@@ -1015,6 +1018,7 @@ static int cpdma_chan_submit_si(struct submit_info *si)
struct cpdma_chan *chan = si->chan;
struct cpdma_ctlr *ctlr = chan->ctlr;
int len = si->len;
+ int swlen = len;
struct cpdma_desc __iomem *desc;
dma_addr_t buffer;
u32 mode;
@@ -1036,16 +1040,22 @@ static int cpdma_chan_submit_si(struct submit_info *si)
chan->stats.runt_transmit_buff++;
}
- buffer = dma_map_single(ctlr->dev, si->data, len, chan->dir);
- ret = dma_mapping_error(ctlr->dev, buffer);
- if (ret) {
- cpdma_desc_free(ctlr->pool, desc, 1);
- return -EINVAL;
- }
-
mode = CPDMA_DESC_OWNER | CPDMA_DESC_SOP | CPDMA_DESC_EOP;
cpdma_desc_to_port(chan, mode, si->directed);
+ if (si->flags & CPDMA_DMA_EXT_MAP) {
+ buffer = (u32)si->data;
+ dma_sync_single_for_device(ctlr->dev, buffer, len, chan->dir);
+ swlen |= CPDMA_DMA_EXT_MAP;
+ } else {
+ buffer = dma_map_single(ctlr->dev, si->data, len, chan->dir);
+ ret = dma_mapping_error(ctlr->dev, buffer);
+ if (ret) {
+ cpdma_desc_free(ctlr->pool, desc, 1);
+ return -EINVAL;
+ }
+ }
+
/* Relaxed IO accessors can be used here as there is read barrier
* at the end of write sequence.
*/
@@ -1055,7 +1065,7 @@ static int cpdma_chan_submit_si(struct submit_info *si)
writel_relaxed(mode | len, &desc->hw_mode);
writel_relaxed((uintptr_t)si->token, &desc->sw_token);
writel_relaxed(buffer, &desc->sw_buffer);
- writel_relaxed(len, &desc->sw_len);
+ writel_relaxed(swlen, &desc->sw_len);
desc_read(desc, sw_len);
__cpdma_chan_submit(chan, desc);
@@ -1079,6 +1089,32 @@ int cpdma_chan_idle_submit(struct cpdma_chan *chan, void *token, void *data,
si.data = data;
si.len = len;
si.directed = directed;
+ si.flags = 0;
+
+ spin_lock_irqsave(&chan->lock, flags);
+ if (chan->state == CPDMA_STATE_TEARDOWN) {
+ spin_unlock_irqrestore(&chan->lock, flags);
+ return -EINVAL;
+ }
+
+ ret = cpdma_chan_submit_si(&si);
+ spin_unlock_irqrestore(&chan->lock, flags);
+ return ret;
+}
+
+int cpdma_chan_idle_submit_mapped(struct cpdma_chan *chan, void *token,
+ dma_addr_t data, int len, int directed)
+{
+ struct submit_info si;
+ unsigned long flags;
+ int ret;
+
+ si.chan = chan;
+ si.token = token;
+ si.data = (void *)(u32)data;
+ si.len = len;
+ si.directed = directed;
+ si.flags = CPDMA_DMA_EXT_MAP;
spin_lock_irqsave(&chan->lock, flags);
if (chan->state == CPDMA_STATE_TEARDOWN) {
@@ -1103,6 +1139,32 @@ int cpdma_chan_submit(struct cpdma_chan *chan, void *token, void *data,
si.data = data;
si.len = len;
si.directed = directed;
+ si.flags = 0;
+
+ spin_lock_irqsave(&chan->lock, flags);
+ if (chan->state != CPDMA_STATE_ACTIVE) {
+ spin_unlock_irqrestore(&chan->lock, flags);
+ return -EINVAL;
+ }
+
+ ret = cpdma_chan_submit_si(&si);
+ spin_unlock_irqrestore(&chan->lock, flags);
+ return ret;
+}
+
+int cpdma_chan_submit_mapped(struct cpdma_chan *chan, void *token,
+ dma_addr_t data, int len, int directed)
+{
+ struct submit_info si;
+ unsigned long flags;
+ int ret;
+
+ si.chan = chan;
+ si.token = token;
+ si.data = (void *)(u32)data;
+ si.len = len;
+ si.directed = directed;
+ si.flags = CPDMA_DMA_EXT_MAP;
spin_lock_irqsave(&chan->lock, flags);
if (chan->state != CPDMA_STATE_ACTIVE) {
@@ -1140,10 +1202,17 @@ static void __cpdma_chan_free(struct cpdma_chan *chan,
uintptr_t token;
token = desc_read(desc, sw_token);
- buff_dma = desc_read(desc, sw_buffer);
origlen = desc_read(desc, sw_len);
- dma_unmap_single(ctlr->dev, buff_dma, origlen, chan->dir);
+ buff_dma = desc_read(desc, sw_buffer);
+ if (origlen & CPDMA_DMA_EXT_MAP) {
+ origlen &= ~CPDMA_DMA_EXT_MAP;
+ dma_sync_single_for_cpu(ctlr->dev, buff_dma, origlen,
+ chan->dir);
+ } else {
+ dma_unmap_single(ctlr->dev, buff_dma, origlen, chan->dir);
+ }
+
cpdma_desc_free(pool, desc, 1);
(*chan->handler)((void *)token, outlen, status);
}
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.h b/drivers/net/ethernet/ti/davinci_cpdma.h
index 9343c8c73c1b..0271a20c2e09 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.h
+++ b/drivers/net/ethernet/ti/davinci_cpdma.h
@@ -77,8 +77,12 @@ int cpdma_chan_stop(struct cpdma_chan *chan);
int cpdma_chan_get_stats(struct cpdma_chan *chan,
struct cpdma_chan_stats *stats);
+int cpdma_chan_submit_mapped(struct cpdma_chan *chan, void *token,
+ dma_addr_t data, int len, int directed);
int cpdma_chan_submit(struct cpdma_chan *chan, void *token, void *data,
int len, int directed);
+int cpdma_chan_idle_submit_mapped(struct cpdma_chan *chan, void *token,
+ dma_addr_t data, int len, int directed);
int cpdma_chan_idle_submit(struct cpdma_chan *chan, void *token, void *data,
int len, int directed);
int cpdma_chan_process(struct cpdma_chan *chan, int quota);
--
2.17.1
^ permalink raw reply related
* [PATCH v6 net-next 5/5] net: ethernet: ti: cpsw: add XDP support
From: Ivan Khoronzhuk @ 2019-07-03 10:19 UTC (permalink / raw)
To: grygorii.strashko, hawk, davem
Cc: ast, linux-kernel, linux-omap, xdp-newbies, ilias.apalodimas,
netdev, daniel, jakub.kicinski, john.fastabend, Ivan Khoronzhuk
In-Reply-To: <20190703101903.8411-1-ivan.khoronzhuk@linaro.org>
Add XDP support based on rx page_pool allocator, one frame per page.
Page pool allocator is used with assumption that only one rx_handler
is running simultaneously. DMA map/unmap is reused from page pool
despite there is no need to map whole page.
Due to specific of cpsw, the same TX/RX handler can be used by 2
network devices, so special fields in buffer are added to identify
an interface the frame is destined to. Thus XDP works for both
interfaces, that allows to test xdp redirect between two interfaces
easily. Aslo, each rx queue have own page pools, but common for both
netdevs.
XDP prog is common for all channels till appropriate changes are added
in XDP infrastructure. Also, once page_pool recycling becomes part of
skb netstack some simplifications can be added, like removing
page_pool_release_page() before skb receive.
In order to keep rx_dev while redirect, that can be somehow used in
future, do flush in rx_handler, that allows to keep rx dev the same
while reidrect. It allows to conform with tracing rx_dev pointed
by Jesper.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
drivers/net/ethernet/ti/Kconfig | 1 +
drivers/net/ethernet/ti/cpsw.c | 485 ++++++++++++++++++++++---
drivers/net/ethernet/ti/cpsw_ethtool.c | 66 +++-
drivers/net/ethernet/ti/cpsw_priv.h | 7 +
4 files changed, 502 insertions(+), 57 deletions(-)
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index a800d3417411..834afca3a019 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -50,6 +50,7 @@ config TI_CPSW
depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
select TI_DAVINCI_MDIO
select MFD_SYSCON
+ select PAGE_POOL
select REGMAP
---help---
This driver supports TI's CPSW Ethernet Switch.
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 32b7b3b74a6b..6e9be22035a9 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -31,6 +31,10 @@
#include <linux/if_vlan.h>
#include <linux/kmemleak.h>
#include <linux/sys_soc.h>
+#include <net/page_pool.h>
+#include <linux/bpf.h>
+#include <linux/bpf_trace.h>
+#include <linux/filter.h>
#include <linux/pinctrl/consumer.h>
#include <net/pkt_cls.h>
@@ -60,6 +64,10 @@ static int descs_pool_size = CPSW_CPDMA_DESCS_POOL_SIZE_DEFAULT;
module_param(descs_pool_size, int, 0444);
MODULE_PARM_DESC(descs_pool_size, "Number of CPDMA CPPI descriptors in pool");
+/* The buf includes headroom compatible with both skb and xdpf */
+#define CPSW_HEADROOM_NA (max(XDP_PACKET_HEADROOM, NET_SKB_PAD) + NET_IP_ALIGN)
+#define CPSW_HEADROOM ALIGN(CPSW_HEADROOM_NA, sizeof(long))
+
#define for_each_slave(priv, func, arg...) \
do { \
struct cpsw_slave *slave; \
@@ -74,6 +82,11 @@ MODULE_PARM_DESC(descs_pool_size, "Number of CPDMA CPPI descriptors in pool");
(func)(slave++, ##arg); \
} while (0)
+#define CPSW_XMETA_OFFSET ALIGN(sizeof(struct xdp_frame), sizeof(long))
+
+#define CPSW_XDP_CONSUMED 1
+#define CPSW_XDP_PASS 0
+
static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
__be16 proto, u16 vid);
@@ -337,24 +350,58 @@ void cpsw_intr_disable(struct cpsw_common *cpsw)
return;
}
+static int cpsw_is_xdpf_handle(void *handle)
+{
+ return (unsigned long)handle & BIT(0);
+}
+
+static void *cpsw_xdpf_to_handle(struct xdp_frame *xdpf)
+{
+ return (void *)((unsigned long)xdpf | BIT(0));
+}
+
+static struct xdp_frame *cpsw_handle_to_xdpf(void *handle)
+{
+ return (struct xdp_frame *)((unsigned long)handle & ~BIT(0));
+}
+
+struct __aligned(sizeof(long)) cpsw_meta_xdp {
+ struct net_device *ndev;
+ int ch;
+};
+
void cpsw_tx_handler(void *token, int len, int status)
{
+ struct cpsw_meta_xdp *xmeta;
+ struct xdp_frame *xdpf;
+ struct net_device *ndev;
struct netdev_queue *txq;
- struct sk_buff *skb = token;
- struct net_device *ndev = skb->dev;
- struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
+ struct sk_buff *skb;
+ int ch;
+
+ if (cpsw_is_xdpf_handle(token)) {
+ xdpf = cpsw_handle_to_xdpf(token);
+ xmeta = (void *)xdpf + CPSW_XMETA_OFFSET;
+ ndev = xmeta->ndev;
+ ch = xmeta->ch;
+ xdp_return_frame(xdpf);
+ } else {
+ skb = token;
+ ndev = skb->dev;
+ ch = skb_get_queue_mapping(skb);
+ cpts_tx_timestamp(ndev_to_cpsw(ndev)->cpts, skb);
+ dev_kfree_skb_any(skb);
+ }
/* Check whether the queue is stopped due to stalled tx dma, if the
* queue is stopped then start the queue as we have free desc for tx
*/
- txq = netdev_get_tx_queue(ndev, skb_get_queue_mapping(skb));
+ txq = netdev_get_tx_queue(ndev, ch);
if (unlikely(netif_tx_queue_stopped(txq)))
netif_tx_wake_queue(txq);
- cpts_tx_timestamp(cpsw->cpts, skb);
ndev->stats.tx_packets++;
ndev->stats.tx_bytes += len;
- dev_kfree_skb_any(skb);
}
static void cpsw_rx_vlan_encap(struct sk_buff *skb)
@@ -400,24 +447,236 @@ static void cpsw_rx_vlan_encap(struct sk_buff *skb)
}
}
+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;
+ 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);
+ }
+
+ if (ret) {
+ priv->ndev->stats.tx_dropped++;
+ xdp_return_frame_rx_napi(xdpf);
+ }
+
+ return ret;
+}
+
+static int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,
+ struct page *page)
+{
+ struct cpsw_common *cpsw = priv->cpsw;
+ struct net_device *ndev = priv->ndev;
+ int ret = CPSW_XDP_CONSUMED;
+ struct xdp_frame *xdpf;
+ struct bpf_prog *prog;
+ u32 act;
+
+ rcu_read_lock();
+
+ prog = READ_ONCE(priv->xdp_prog);
+ if (!prog) {
+ ret = CPSW_XDP_PASS;
+ goto out;
+ }
+
+ act = bpf_prog_run_xdp(prog, xdp);
+ switch (act) {
+ case XDP_PASS:
+ ret = CPSW_XDP_PASS;
+ break;
+ case XDP_TX:
+ xdpf = convert_to_xdp_frame(xdp);
+ if (unlikely(!xdpf))
+ goto drop;
+
+ cpsw_xdp_tx_frame(priv, xdpf, page);
+ break;
+ case XDP_REDIRECT:
+ if (xdp_do_redirect(ndev, xdp, prog))
+ goto drop;
+
+ /* as flush requires rx_dev to be per NAPI handle and there
+ * is can be two devices putting packets on bulk queue,
+ * do flush here avoid this just for sure.
+ */
+ xdp_do_flush_map();
+ break;
+ default:
+ bpf_warn_invalid_xdp_action(act);
+ /* fall through */
+ case XDP_ABORTED:
+ trace_xdp_exception(ndev, prog, act);
+ /* fall through -- handle aborts by dropping packet */
+ case XDP_DROP:
+ goto drop;
+ }
+out:
+ rcu_read_unlock();
+ return ret;
+drop:
+ rcu_read_unlock();
+ page_pool_recycle_direct(cpsw->page_pool[ch], page);
+ return ret;
+}
+
+static unsigned int cpsw_rxbuf_total_len(unsigned int len)
+{
+ len += CPSW_HEADROOM;
+ len += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+
+ return SKB_DATA_ALIGN(len);
+}
+
+static struct page_pool *cpsw_create_page_pool(struct cpsw_common *cpsw,
+ int size)
+{
+ struct page_pool_params pp_params;
+ struct page_pool *pool;
+
+ pp_params.order = 0;
+ pp_params.flags = PP_FLAG_DMA_MAP;
+ pp_params.pool_size = size;
+ pp_params.nid = NUMA_NO_NODE;
+ pp_params.dma_dir = DMA_BIDIRECTIONAL;
+ pp_params.dev = cpsw->dev;
+
+ pool = page_pool_create(&pp_params);
+ if (IS_ERR(pool))
+ dev_err(cpsw->dev, "cannot create rx page pool\n");
+
+ return pool;
+}
+
+static int cpsw_create_rx_pool(struct cpsw_common *cpsw, int ch)
+{
+ struct page_pool *pool;
+ int ret = 0, pool_size;
+
+ pool_size = cpdma_chan_get_rx_buf_num(cpsw->rxv[ch].ch);
+ pool = cpsw_create_page_pool(cpsw, pool_size);
+ if (IS_ERR(pool))
+ ret = PTR_ERR(pool);
+ else
+ cpsw->page_pool[ch] = pool;
+
+ return ret;
+}
+
+static int cpsw_ndev_create_xdp_rxq(struct cpsw_priv *priv, int ch)
+{
+ struct cpsw_common *cpsw = priv->cpsw;
+ int ret, new_pool = false;
+ struct xdp_rxq_info *rxq;
+
+ rxq = &priv->xdp_rxq[ch];
+
+ ret = xdp_rxq_info_reg(rxq, priv->ndev, ch);
+ if (ret)
+ return ret;
+
+ if (!cpsw->page_pool[ch]) {
+ ret = cpsw_create_rx_pool(cpsw, ch);
+ if (ret)
+ goto err_rxq;
+
+ new_pool = true;
+ }
+
+ ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_PAGE_POOL,
+ cpsw->page_pool[ch]);
+ if (!ret)
+ return 0;
+
+ if (new_pool) {
+ page_pool_free(cpsw->page_pool[ch]);
+ cpsw->page_pool[ch] = NULL;
+ }
+
+err_rxq:
+ xdp_rxq_info_unreg(rxq);
+ return ret;
+}
+
+void cpsw_ndev_destroy_xdp_rxqs(struct cpsw_priv *priv)
+{
+ struct cpsw_common *cpsw = priv->cpsw;
+ struct xdp_rxq_info *rxq;
+ int i;
+
+ for (i = 0; i < cpsw->rx_ch_num; i++) {
+ rxq = &priv->xdp_rxq[i];
+ if (xdp_rxq_info_is_reg(rxq))
+ xdp_rxq_info_unreg(rxq);
+ }
+}
+
+int cpsw_ndev_create_xdp_rxqs(struct cpsw_priv *priv)
+{
+ struct cpsw_common *cpsw = priv->cpsw;
+ int i, ret;
+
+ for (i = 0; i < cpsw->rx_ch_num; i++) {
+ ret = cpsw_ndev_create_xdp_rxq(priv, i);
+ if (ret)
+ goto err_cleanup;
+ }
+
+ return 0;
+
+err_cleanup:
+ cpsw_ndev_destroy_xdp_rxqs(priv);
+
+ return ret;
+}
+
static void cpsw_rx_handler(void *token, int len, int status)
{
- struct cpdma_chan *ch;
- struct sk_buff *skb = token;
- struct sk_buff *new_skb;
- struct net_device *ndev = skb->dev;
- int ret = 0, port;
- struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
+ struct page *new_page, *page = token;
+ void *pa = page_address(page);
+ struct cpsw_meta_xdp *xmeta = pa + CPSW_XMETA_OFFSET;
+ struct cpsw_common *cpsw = ndev_to_cpsw(xmeta->ndev);
+ int pkt_size = cpsw->rx_packet_max;
+ int ret = 0, port, ch = xmeta->ch;
+ int headroom = CPSW_HEADROOM;
+ struct net_device *ndev = xmeta->ndev;
struct cpsw_priv *priv;
+ struct page_pool *pool;
+ struct sk_buff *skb;
+ struct xdp_buff xdp;
+ dma_addr_t dma;
- if (cpsw->data.dual_emac) {
+ if (cpsw->data.dual_emac && status >= 0) {
port = CPDMA_RX_SOURCE_PORT(status);
- if (port) {
+ if (port)
ndev = cpsw->slaves[--port].ndev;
- skb->dev = ndev;
- }
}
+ priv = netdev_priv(ndev);
+ pool = cpsw->page_pool[ch];
if (unlikely(status < 0) || unlikely(!netif_running(ndev))) {
/* In dual emac mode check for all interfaces */
if (cpsw->data.dual_emac && cpsw->usage_count &&
@@ -426,43 +685,87 @@ static void cpsw_rx_handler(void *token, int len, int status)
* is already down and the other interface is up
* and running, instead of freeing which results
* in reducing of the number of rx descriptor in
- * DMA engine, requeue skb back to cpdma.
+ * DMA engine, requeue page back to cpdma.
*/
- new_skb = skb;
+ new_page = page;
goto requeue;
}
- /* the interface is going down, skbs are purged */
- dev_kfree_skb_any(skb);
+ /* the interface is going down, pages are purged */
+ page_pool_recycle_direct(pool, page);
return;
}
- new_skb = netdev_alloc_skb_ip_align(ndev, cpsw->rx_packet_max);
- if (new_skb) {
- skb_copy_queue_mapping(new_skb, skb);
- skb_put(skb, len);
- if (status & CPDMA_RX_VLAN_ENCAP)
- cpsw_rx_vlan_encap(skb);
- priv = netdev_priv(ndev);
- if (priv->rx_ts_enabled)
- cpts_rx_timestamp(cpsw->cpts, skb);
- skb->protocol = eth_type_trans(skb, ndev);
- netif_receive_skb(skb);
- ndev->stats.rx_bytes += len;
- ndev->stats.rx_packets++;
- kmemleak_not_leak(new_skb);
- } else {
+ new_page = page_pool_dev_alloc_pages(pool);
+ if (unlikely(!new_page)) {
+ new_page = page;
ndev->stats.rx_dropped++;
- new_skb = skb;
+ goto requeue;
}
+ if (priv->xdp_prog) {
+ if (status & CPDMA_RX_VLAN_ENCAP) {
+ xdp.data = pa + CPSW_HEADROOM +
+ CPSW_RX_VLAN_ENCAP_HDR_SIZE;
+ xdp.data_end = xdp.data + len -
+ CPSW_RX_VLAN_ENCAP_HDR_SIZE;
+ } else {
+ xdp.data = pa + CPSW_HEADROOM;
+ xdp.data_end = xdp.data + len;
+ }
+
+ xdp_set_data_meta_invalid(&xdp);
+
+ xdp.data_hard_start = pa;
+ xdp.rxq = &priv->xdp_rxq[ch];
+
+ ret = cpsw_run_xdp(priv, ch, &xdp, page);
+ if (ret != CPSW_XDP_PASS)
+ goto requeue;
+
+ /* XDP prog might have changed packet data and boundaries */
+ len = xdp.data_end - xdp.data;
+ headroom = xdp.data - xdp.data_hard_start;
+
+ /* XDP prog can modify vlan tag, so can't use encap header */
+ status &= ~CPDMA_RX_VLAN_ENCAP;
+ }
+
+ /* pass skb to netstack if no XDP prog or returned XDP_PASS */
+ skb = build_skb(pa, cpsw_rxbuf_total_len(pkt_size));
+ if (!skb) {
+ ndev->stats.rx_dropped++;
+ page_pool_recycle_direct(pool, page);
+ goto requeue;
+ }
+
+ skb_reserve(skb, headroom);
+ skb_put(skb, len);
+ skb->dev = ndev;
+ if (status & CPDMA_RX_VLAN_ENCAP)
+ cpsw_rx_vlan_encap(skb);
+ if (priv->rx_ts_enabled)
+ cpts_rx_timestamp(cpsw->cpts, skb);
+ skb->protocol = eth_type_trans(skb, ndev);
+
+ /* unmap page as no netstack skb page recycling */
+ page_pool_release_page(pool, page);
+ netif_receive_skb(skb);
+
+ ndev->stats.rx_bytes += len;
+ ndev->stats.rx_packets++;
+
requeue:
- ch = cpsw->rxv[skb_get_queue_mapping(new_skb)].ch;
- ret = cpdma_chan_submit(ch, new_skb, new_skb->data,
- skb_tailroom(new_skb), 0);
+ xmeta = page_address(new_page) + CPSW_XMETA_OFFSET;
+ xmeta->ndev = ndev;
+ xmeta->ch = ch;
+
+ dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM;
+ ret = cpdma_chan_submit_mapped(cpsw->rxv[ch].ch, new_page, dma,
+ pkt_size, 0);
if (ret < 0) {
WARN_ON(ret == -ENOMEM);
- dev_kfree_skb_any(new_skb);
+ page_pool_recycle_direct(pool, new_page);
}
}
@@ -1032,33 +1335,39 @@ static void cpsw_init_host_port(struct cpsw_priv *priv)
int cpsw_fill_rx_channels(struct cpsw_priv *priv)
{
struct cpsw_common *cpsw = priv->cpsw;
- struct sk_buff *skb;
+ struct cpsw_meta_xdp *xmeta;
+ struct page_pool *pool;
+ struct page *page;
int ch_buf_num;
int ch, i, ret;
+ dma_addr_t dma;
for (ch = 0; ch < cpsw->rx_ch_num; ch++) {
+ pool = cpsw->page_pool[ch];
ch_buf_num = cpdma_chan_get_rx_buf_num(cpsw->rxv[ch].ch);
for (i = 0; i < ch_buf_num; i++) {
- skb = __netdev_alloc_skb_ip_align(priv->ndev,
- cpsw->rx_packet_max,
- GFP_KERNEL);
- if (!skb) {
- cpsw_err(priv, ifup, "cannot allocate skb\n");
+ page = page_pool_dev_alloc_pages(pool);
+ if (!page) {
+ cpsw_err(priv, ifup, "allocate rx page err\n");
return -ENOMEM;
}
- skb_set_queue_mapping(skb, ch);
- ret = cpdma_chan_idle_submit(cpsw->rxv[ch].ch, skb,
- skb->data,
- skb_tailroom(skb), 0);
+ xmeta = page_address(page) + CPSW_XMETA_OFFSET;
+ xmeta->ndev = priv->ndev;
+ xmeta->ch = ch;
+
+ dma = page_pool_get_dma_addr(page) + CPSW_HEADROOM;
+ ret = cpdma_chan_idle_submit_mapped(cpsw->rxv[ch].ch,
+ page, dma,
+ cpsw->rx_packet_max,
+ 0);
if (ret < 0) {
cpsw_err(priv, ifup,
- "cannot submit skb to channel %d rx, error %d\n",
+ "cannot submit page to channel %d rx, error %d\n",
ch, ret);
- kfree_skb(skb);
+ page_pool_recycle_direct(pool, page);
return ret;
}
- kmemleak_not_leak(skb);
}
cpsw_info(priv, ifup, "ch %d rx, submitted %d descriptors\n",
@@ -1370,6 +1679,10 @@ static int cpsw_ndo_open(struct net_device *ndev)
cpsw_ale_add_vlan(cpsw->ale, cpsw->data.default_vlan,
ALE_ALL_PORTS, ALE_ALL_PORTS, 0, 0);
+ ret = cpsw_ndev_create_xdp_rxqs(priv);
+ if (ret)
+ goto err_cleanup;
+
/* initialize shared resources for every ndev */
if (!cpsw->usage_count) {
/* disable priority elevation */
@@ -1422,9 +1735,10 @@ static int cpsw_ndo_open(struct net_device *ndev)
err_cleanup:
if (!cpsw->usage_count) {
cpdma_ctlr_stop(cpsw->dma);
- for_each_slave(priv, cpsw_slave_stop, cpsw);
+ memset(cpsw->page_pool, 0, sizeof(cpsw->page_pool));
}
+ for_each_slave(priv, cpsw_slave_stop, cpsw);
pm_runtime_put_sync(cpsw->dev);
netif_carrier_off(priv->ndev);
return ret;
@@ -1447,9 +1761,12 @@ static int cpsw_ndo_stop(struct net_device *ndev)
cpsw_intr_disable(cpsw);
cpdma_ctlr_stop(cpsw->dma);
cpsw_ale_stop(cpsw->ale);
+ memset(cpsw->page_pool, 0, sizeof(cpsw->page_pool));
}
for_each_slave(priv, cpsw_slave_stop, cpsw);
+ cpsw_ndev_destroy_xdp_rxqs(priv);
+
if (cpsw_need_resplit(cpsw))
cpsw_split_res(cpsw);
@@ -2004,6 +2321,64 @@ static int cpsw_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
}
}
+static int cpsw_xdp_prog_setup(struct cpsw_priv *priv, struct netdev_bpf *bpf)
+{
+ struct bpf_prog *prog = bpf->prog;
+
+ if (!priv->xdpi.prog && !prog)
+ return 0;
+
+ if (!xdp_attachment_flags_ok(&priv->xdpi, bpf))
+ return -EBUSY;
+
+ WRITE_ONCE(priv->xdp_prog, prog);
+
+ xdp_attachment_setup(&priv->xdpi, bpf);
+
+ return 0;
+}
+
+static int cpsw_ndo_bpf(struct net_device *ndev, struct netdev_bpf *bpf)
+{
+ struct cpsw_priv *priv = netdev_priv(ndev);
+
+ switch (bpf->command) {
+ case XDP_SETUP_PROG:
+ return cpsw_xdp_prog_setup(priv, bpf);
+
+ case XDP_QUERY_PROG:
+ return xdp_attachment_query(&priv->xdpi, bpf);
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int cpsw_ndo_xdp_xmit(struct net_device *ndev, int n,
+ struct xdp_frame **frames, u32 flags)
+{
+ struct cpsw_priv *priv = netdev_priv(ndev);
+ struct xdp_frame *xdpf;
+ int i, drops = 0;
+
+ if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
+ return -EINVAL;
+
+ for (i = 0; i < n; i++) {
+ xdpf = frames[i];
+ if (xdpf->len < CPSW_MIN_PACKET_SIZE) {
+ xdp_return_frame_rx_napi(xdpf);
+ drops++;
+ continue;
+ }
+
+ if (cpsw_xdp_tx_frame(priv, xdpf, NULL))
+ drops++;
+ }
+
+ return n - drops;
+}
+
#ifdef CONFIG_NET_POLL_CONTROLLER
static void cpsw_ndo_poll_controller(struct net_device *ndev)
{
@@ -2032,6 +2407,8 @@ static const struct net_device_ops cpsw_netdev_ops = {
.ndo_vlan_rx_add_vid = cpsw_ndo_vlan_rx_add_vid,
.ndo_vlan_rx_kill_vid = cpsw_ndo_vlan_rx_kill_vid,
.ndo_setup_tc = cpsw_ndo_setup_tc,
+ .ndo_bpf = cpsw_ndo_bpf,
+ .ndo_xdp_xmit = cpsw_ndo_xdp_xmit,
};
static void cpsw_get_drvinfo(struct net_device *ndev,
diff --git a/drivers/net/ethernet/ti/cpsw_ethtool.c b/drivers/net/ethernet/ti/cpsw_ethtool.c
index fa4d75f5548e..b39a598cb094 100644
--- a/drivers/net/ethernet/ti/cpsw_ethtool.c
+++ b/drivers/net/ethernet/ti/cpsw_ethtool.c
@@ -578,6 +578,48 @@ static int cpsw_update_channels_res(struct cpsw_priv *priv, int ch_num, int rx,
return 0;
}
+static void cpsw_destroy_xdp_rxqs(struct cpsw_common *cpsw)
+{
+ struct net_device *ndev;
+ struct cpsw_priv *priv;
+ int i;
+
+ for (i = 0; i < cpsw->data.slaves; i++) {
+ ndev = cpsw->slaves[i].ndev;
+ if (!ndev || !netif_running(ndev))
+ continue;
+
+ priv = netdev_priv(ndev);
+ cpsw_ndev_destroy_xdp_rxqs(priv);
+ }
+
+ memset(cpsw->page_pool, 0, sizeof(cpsw->page_pool));
+}
+
+static int cpsw_create_xdp_rxqs(struct cpsw_common *cpsw)
+{
+ struct net_device *ndev;
+ struct cpsw_priv *priv;
+ int i, ret;
+
+ for (i = 0; i < cpsw->data.slaves; i++) {
+ ndev = cpsw->slaves[i].ndev;
+ if (!ndev || !netif_running(ndev))
+ continue;
+
+ priv = netdev_priv(ndev);
+ ret = cpsw_ndev_create_xdp_rxqs(priv);
+ if (ret)
+ goto err_cleanup;
+ }
+
+ return 0;
+
+err_cleanup:
+ cpsw_destroy_xdp_rxqs(cpsw);
+ return ret;
+}
+
int cpsw_set_channels_common(struct net_device *ndev,
struct ethtool_channels *chs,
cpdma_handler_fn rx_handler)
@@ -585,7 +627,7 @@ int cpsw_set_channels_common(struct net_device *ndev,
struct cpsw_priv *priv = netdev_priv(ndev);
struct cpsw_common *cpsw = priv->cpsw;
struct net_device *sl_ndev;
- int i, ret;
+ int i, new_pools, ret;
ret = cpsw_check_ch_settings(cpsw, chs);
if (ret < 0)
@@ -593,6 +635,10 @@ int cpsw_set_channels_common(struct net_device *ndev,
cpsw_suspend_data_pass(ndev);
+ new_pools = (chs->rx_count != cpsw->rx_ch_num) && cpsw->usage_count;
+ if (new_pools)
+ cpsw_destroy_xdp_rxqs(cpsw);
+
ret = cpsw_update_channels_res(priv, chs->rx_count, 1, rx_handler);
if (ret)
goto err;
@@ -622,6 +668,12 @@ int cpsw_set_channels_common(struct net_device *ndev,
cpsw_split_res(cpsw);
+ if (new_pools) {
+ ret = cpsw_create_xdp_rxqs(cpsw);
+ if (ret)
+ goto err;
+ }
+
ret = cpsw_resume_data_pass(ndev);
if (!ret)
return 0;
@@ -647,8 +699,7 @@ void cpsw_get_ringparam(struct net_device *ndev,
int cpsw_set_ringparam(struct net_device *ndev,
struct ethtool_ringparam *ering)
{
- struct cpsw_priv *priv = netdev_priv(ndev);
- struct cpsw_common *cpsw = priv->cpsw;
+ struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
int ret;
/* ignore ering->tx_pending - only rx_pending adjustment is supported */
@@ -663,10 +714,19 @@ int cpsw_set_ringparam(struct net_device *ndev,
cpsw_suspend_data_pass(ndev);
+ if (cpsw->usage_count)
+ cpsw_destroy_xdp_rxqs(cpsw);
+
ret = cpdma_set_num_rx_descs(cpsw->dma, ering->rx_pending);
if (ret)
goto err;
+ if (cpsw->usage_count) {
+ ret = cpsw_create_xdp_rxqs(cpsw);
+ if (ret)
+ goto err;
+ }
+
ret = cpsw_resume_data_pass(ndev);
if (!ret)
return 0;
diff --git a/drivers/net/ethernet/ti/cpsw_priv.h b/drivers/net/ethernet/ti/cpsw_priv.h
index 04795b97ee71..da68764e7f87 100644
--- a/drivers/net/ethernet/ti/cpsw_priv.h
+++ b/drivers/net/ethernet/ti/cpsw_priv.h
@@ -346,6 +346,7 @@ struct cpsw_common {
int rx_ch_num, tx_ch_num;
int speed;
int usage_count;
+ struct page_pool *page_pool[CPSW_MAX_QUEUES];
};
struct cpsw_priv {
@@ -360,6 +361,10 @@ struct cpsw_priv {
int shp_cfg_speed;
int tx_ts_enabled;
int rx_ts_enabled;
+ struct bpf_prog *xdp_prog;
+ struct xdp_rxq_info xdp_rxq[CPSW_MAX_QUEUES];
+ struct xdp_attachment_info xdpi;
+
u32 emac_port;
struct cpsw_common *cpsw;
};
@@ -391,6 +396,8 @@ int cpsw_fill_rx_channels(struct cpsw_priv *priv);
void cpsw_intr_enable(struct cpsw_common *cpsw);
void cpsw_intr_disable(struct cpsw_common *cpsw);
void cpsw_tx_handler(void *token, int len, int status);
+int cpsw_ndev_create_xdp_rxqs(struct cpsw_priv *priv);
+void cpsw_ndev_destroy_xdp_rxqs(struct cpsw_priv *priv);
/* ethtool */
u32 cpsw_get_msglevel(struct net_device *ndev);
--
2.17.1
^ permalink raw reply related
* [PATCH v6 net-next 4/5] net: ethernet: ti: cpsw_ethtool: allow res split while down
From: Ivan Khoronzhuk @ 2019-07-03 10:19 UTC (permalink / raw)
To: grygorii.strashko, hawk, davem
Cc: ast, linux-kernel, linux-omap, xdp-newbies, ilias.apalodimas,
netdev, daniel, jakub.kicinski, john.fastabend, Ivan Khoronzhuk
In-Reply-To: <20190703101903.8411-1-ivan.khoronzhuk@linaro.org>
That's possible to set channel num while interfaces are down. When
interface gets up it should resplit budget. This resplit can happen
after phy is up but only if speed is changed, so should be set before
this, for this allow it to happen while changing number of channels,
when interfaces are down.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
drivers/net/ethernet/ti/cpsw_ethtool.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw_ethtool.c b/drivers/net/ethernet/ti/cpsw_ethtool.c
index 08d7aaee8299..fa4d75f5548e 100644
--- a/drivers/net/ethernet/ti/cpsw_ethtool.c
+++ b/drivers/net/ethernet/ti/cpsw_ethtool.c
@@ -620,8 +620,7 @@ int cpsw_set_channels_common(struct net_device *ndev,
}
}
- if (cpsw->usage_count)
- cpsw_split_res(cpsw);
+ cpsw_split_res(cpsw);
ret = cpsw_resume_data_pass(ndev);
if (!ret)
--
2.17.1
^ permalink raw reply related
* [PATCH v6 net-next 3/5] net: ethernet: ti: davinci_cpdma: allow desc split while down
From: Ivan Khoronzhuk @ 2019-07-03 10:19 UTC (permalink / raw)
To: grygorii.strashko, hawk, davem
Cc: ast, linux-kernel, linux-omap, xdp-newbies, ilias.apalodimas,
netdev, daniel, jakub.kicinski, john.fastabend, Ivan Khoronzhuk
In-Reply-To: <20190703101903.8411-1-ivan.khoronzhuk@linaro.org>
That's possible to set ring params while interfaces are down. When
interface gets up it uses number of descs to fill rx queue and on
later on changes to create rx pools. Usually, this resplit can happen
after phy is up, but it can be needed before this, so allow it to
happen while setting number of rx descs, when interfaces are down.
Also, if no dependency on intf state, move it to cpdma layer, where
it should be.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
drivers/net/ethernet/ti/cpsw_ethtool.c | 9 ++++-----
drivers/net/ethernet/ti/davinci_cpdma.c | 10 +++++++++-
drivers/net/ethernet/ti/davinci_cpdma.h | 3 +--
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw_ethtool.c b/drivers/net/ethernet/ti/cpsw_ethtool.c
index f60dc1dfc443..08d7aaee8299 100644
--- a/drivers/net/ethernet/ti/cpsw_ethtool.c
+++ b/drivers/net/ethernet/ti/cpsw_ethtool.c
@@ -664,15 +664,14 @@ int cpsw_set_ringparam(struct net_device *ndev,
cpsw_suspend_data_pass(ndev);
- cpdma_set_num_rx_descs(cpsw->dma, ering->rx_pending);
-
- if (cpsw->usage_count)
- cpdma_chan_split_pool(cpsw->dma);
+ ret = cpdma_set_num_rx_descs(cpsw->dma, ering->rx_pending);
+ if (ret)
+ goto err;
ret = cpsw_resume_data_pass(ndev);
if (!ret)
return 0;
-
+err:
dev_err(cpsw->dev, "cannot set ring params, closing device\n");
dev_close(ndev);
return ret;
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 8da46394c0e7..4167b0b77c8e 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -1423,8 +1423,16 @@ int cpdma_get_num_tx_descs(struct cpdma_ctlr *ctlr)
return ctlr->num_tx_desc;
}
-void cpdma_set_num_rx_descs(struct cpdma_ctlr *ctlr, int num_rx_desc)
+int cpdma_set_num_rx_descs(struct cpdma_ctlr *ctlr, int num_rx_desc)
{
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&ctlr->lock, flags);
ctlr->num_rx_desc = num_rx_desc;
ctlr->num_tx_desc = ctlr->pool->num_desc - ctlr->num_rx_desc;
+ ret = cpdma_chan_split_pool(ctlr);
+ spin_unlock_irqrestore(&ctlr->lock, flags);
+
+ return ret;
}
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.h b/drivers/net/ethernet/ti/davinci_cpdma.h
index 0271a20c2e09..d3cfe234d16a 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.h
+++ b/drivers/net/ethernet/ti/davinci_cpdma.h
@@ -116,8 +116,7 @@ enum cpdma_control {
int cpdma_control_get(struct cpdma_ctlr *ctlr, int control);
int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value);
int cpdma_get_num_rx_descs(struct cpdma_ctlr *ctlr);
-void cpdma_set_num_rx_descs(struct cpdma_ctlr *ctlr, int num_rx_desc);
+int cpdma_set_num_rx_descs(struct cpdma_ctlr *ctlr, int num_rx_desc);
int cpdma_get_num_tx_descs(struct cpdma_ctlr *ctlr);
-int cpdma_chan_split_pool(struct cpdma_ctlr *ctlr);
#endif
--
2.17.1
^ permalink raw reply related
* Re: [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Jason Wang @ 2019-07-03 10:09 UTC (permalink / raw)
To: Tiwei Bie, mst, alex.williamson, maxime.coquelin
Cc: linux-kernel, kvm, virtualization, netdev, dan.daly,
cunming.liang, zhihong.wang
In-Reply-To: <20190703091339.1847-1-tiwei.bie@intel.com>
On 2019/7/3 下午5:13, Tiwei Bie wrote:
> Details about this can be found here:
>
> https://lwn.net/Articles/750770/
>
> What's new in this version
> ==========================
>
> A new VFIO device type is introduced - vfio-vhost. This addressed
> some comments from here: https://patchwork.ozlabs.org/cover/984763/
>
> Below is the updated device interface:
>
> Currently, there are two regions of this device: 1) CONFIG_REGION
> (VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
> device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
> can be used to notify the device.
>
> 1. CONFIG_REGION
>
> The region described by CONFIG_REGION is the main control interface.
> Messages will be written to or read from this region.
>
> The message type is determined by the `request` field in message
> header. The message size is encoded in the message header too.
> The message format looks like this:
>
> struct vhost_vfio_op {
> __u64 request;
> __u32 flags;
> /* Flag values: */
> #define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
> __u32 size;
> union {
> __u64 u64;
> struct vhost_vring_state state;
> struct vhost_vring_addr addr;
> } payload;
> };
>
> The existing vhost-kernel ioctl cmds are reused as the message
> requests in above structure.
Still a comments like V1. What's the advantage of inventing a new
protocol? I believe either of the following should be better:
- using vhost ioctl, we can start from SET_VRING_KICK/SET_VRING_CALL
and extend it with e.g notify region. The advantages is that all exist
userspace program could be reused without modification (or minimal
modification). And vhost API hides lots of details that is not necessary
to be understood by application (e.g in the case of container).
- using PCI layout, then you don't even need to re-invent notifiy region
at all and we can pass-through them to guest.
Personally, I prefer vhost ioctl.
>
> Each message will be written to or read from this region at offset 0:
>
> int vhost_vfio_write(struct vhost_dev *dev, struct vhost_vfio_op *op)
> {
> int count = VHOST_VFIO_OP_HDR_SIZE + op->size;
> struct vhost_vfio *vfio = dev->opaque;
> int ret;
>
> ret = pwrite64(vfio->device_fd, op, count, vfio->config_offset);
> if (ret != count)
> return -1;
>
> return 0;
> }
>
> int vhost_vfio_read(struct vhost_dev *dev, struct vhost_vfio_op *op)
> {
> int count = VHOST_VFIO_OP_HDR_SIZE + op->size;
> struct vhost_vfio *vfio = dev->opaque;
> uint64_t request = op->request;
> int ret;
>
> ret = pread64(vfio->device_fd, op, count, vfio->config_offset);
> if (ret != count || request != op->request)
> return -1;
>
> return 0;
> }
>
> It's quite straightforward to set things to the device. Just need to
> write the message to device directly:
>
> int vhost_vfio_set_features(struct vhost_dev *dev, uint64_t features)
> {
> struct vhost_vfio_op op;
>
> op.request = VHOST_SET_FEATURES;
> op.flags = 0;
> op.size = sizeof(features);
> op.payload.u64 = features;
>
> return vhost_vfio_write(dev, &op);
> }
>
> To get things from the device, two steps are needed.
> Take VHOST_GET_FEATURE as an example:
>
> int vhost_vfio_get_features(struct vhost_dev *dev, uint64_t *features)
> {
> struct vhost_vfio_op op;
> int ret;
>
> op.request = VHOST_GET_FEATURES;
> op.flags = VHOST_VFIO_NEED_REPLY;
> op.size = 0;
>
> /* Just need to write the header */
> ret = vhost_vfio_write(dev, &op);
> if (ret != 0)
> goto out;
>
> /* `op` wasn't changed during write */
> op.flags = 0;
> op.size = sizeof(*features);
>
> ret = vhost_vfio_read(dev, &op);
> if (ret != 0)
> goto out;
>
> *features = op.payload.u64;
> out:
> return ret;
> }
>
> 2. NOTIFIY_REGION (mmap-able)
>
> The region described by NOTIFY_REGION will be used to notify
> the device.
>
> Each queue will have a page for notification, and it can be mapped
> to VM (if hardware also supports), and the virtio driver in the VM
> will be able to notify the device directly.
>
> The region described by NOTIFY_REGION is also write-able. If
> the accelerator's notification register(s) cannot be mapped to
> the VM, write() can also be used to notify the device. Something
> like this:
>
> void notify_relay(void *opaque)
> {
> ......
> offset = host_page_size * queue_idx;
>
> ret = pwrite64(vfio->device_fd, &queue_idx, sizeof(queue_idx),
> vfio->notify_offset + offset);
> ......
> }
>
> 3. VFIO interrupt ioctl API
>
> VFIO interrupt ioctl API is used to setup device interrupts.
> IRQ-bypass can also be supported.
>
> Currently, the data path interrupt can be configured via the
> VFIO_VHOST_VQ_IRQ_INDEX with virtqueue's callfd.
How about DMA API? Do you expect to use VFIO IOMMU API or using vhost
SET_MEM_TABLE? VFIO IOMMU API is more generic for sure but with
SET_MEM_TABLE DMA can be done at the level of parent device which means
it can work for e.g the card with on-chip IOMMU.
And what's the plan for vIOMMU?
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> drivers/vhost/Makefile | 2 +
> drivers/vhost/vdpa.c | 770 +++++++++++++++++++++++++++++++++++++
> include/linux/vdpa_mdev.h | 72 ++++
> include/uapi/linux/vfio.h | 19 +
> include/uapi/linux/vhost.h | 25 ++
> 5 files changed, 888 insertions(+)
> create mode 100644 drivers/vhost/vdpa.c
> create mode 100644 include/linux/vdpa_mdev.h
We probably need some sample parent device implementation. It could be a
software datapath like e.g we can start from virtio-net device in guest
or a vhost/tap on host.
Thanks
>
> diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
> index 6c6df24f770c..cabb71095940 100644
> --- a/drivers/vhost/Makefile
> +++ b/drivers/vhost/Makefile
> @@ -10,4 +10,6 @@ vhost_vsock-y := vsock.o
>
> obj-$(CONFIG_VHOST_RING) += vringh.o
>
> +obj-$(CONFIG_VHOST_VFIO) += vdpa.o
> +
> obj-$(CONFIG_VHOST) += vhost.o
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> new file mode 100644
> index 000000000000..5c9426e2a091
> --- /dev/null
> +++ b/drivers/vhost/vdpa.c
> @@ -0,0 +1,770 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018-2019 Intel Corporation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/vfio.h>
> +#include <linux/vhost.h>
> +#include <linux/mdev.h>
> +#include <linux/vdpa_mdev.h>
> +#include <asm/uaccess.h>
> +
> +#define VDPA_CONFIG_SIZE 0x1000000
> +
> +#define VDPA_VFIO_VHOST_OFFSET_SHIFT 40
> +#define VDPA_VFIO_VHOST_OFFSET_MASK \
> + ((1ULL << VDPA_VFIO_VHOST_OFFSET_SHIFT) - 1)
> +#define VDPA_VFIO_VHOST_OFFSET_TO_INDEX(offset) \
> + ((offset) >> VDPA_VFIO_VHOST_OFFSET_SHIFT)
> +#define VDPA_VFIO_VHOST_INDEX_TO_OFFSET(index) \
> + ((u64)(index) << VDPA_VFIO_VHOST_OFFSET_SHIFT)
> +#define VDPA_VFIO_VHOST_REGION_OFFSET(offset) \
> + ((offset) & VDPA_VFIO_VHOST_OFFSET_MASK)
> +
> +struct vdpa_dev *vdpa_alloc(struct mdev_device *mdev, void *private,
> + int max_vrings)
> +{
> + struct vdpa_dev *vdpa;
> + size_t size;
> +
> + size = sizeof(struct vdpa_dev) + max_vrings *
> + sizeof(struct vdpa_vring_info);
> +
> + vdpa = kzalloc(size, GFP_KERNEL);
> + if (vdpa == NULL)
> + return NULL;
> +
> + mutex_init(&vdpa->ops_lock);
> +
> + vdpa->mdev = mdev;
> + vdpa->private = private;
> + vdpa->max_vrings = max_vrings;
> +
> + return vdpa;
> +}
> +EXPORT_SYMBOL(vdpa_alloc);
> +
> +void vdpa_free(struct vdpa_dev *vdpa)
> +{
> + struct mdev_device *mdev;
> +
> + mdev = vdpa->mdev;
> +
> + vdpa->ops->stop(vdpa);
> + mdev_set_drvdata(mdev, NULL);
> + mutex_destroy(&vdpa->ops_lock);
> + kfree(vdpa);
> +}
> +EXPORT_SYMBOL(vdpa_free);
> +
> +static ssize_t vdpa_handle_config_read(struct mdev_device *mdev,
> + char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct vdpa_dev *vdpa;
> + struct vhost_vfio_op *op = NULL;
> + loff_t pos = *ppos;
> + loff_t offset;
> + int ret;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
> + if (offset != 0) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (!vdpa->pending_reply) {
> + ret = 0;
> + goto out;
> + }
> +
> + vdpa->pending_reply = false;
> +
> + op = kzalloc(VHOST_VFIO_OP_HDR_SIZE + VHOST_VFIO_OP_PAYLOAD_MAX_SIZE,
> + GFP_KERNEL);
> + if (op == NULL) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + op->request = vdpa->pending.request;
> +
> + switch (op->request) {
> + case VHOST_GET_VRING_BASE:
> + op->payload.state = vdpa->pending.payload.state;
> + op->size = sizeof(op->payload.state);
> + break;
> + case VHOST_GET_FEATURES:
> + op->payload.u64 = vdpa->pending.payload.u64;
> + op->size = sizeof(op->payload.u64);
> + break;
> + default:
> + ret = -EINVAL;
> + goto out_free;
> + }
> +
> + if (op->size + VHOST_VFIO_OP_HDR_SIZE != count) {
> + ret = -EINVAL;
> + goto out_free;
> + }
> +
> + if (copy_to_user(buf, op, count)) {
> + ret = -EFAULT;
> + goto out_free;
> + }
> +
> + ret = count;
> +
> +out_free:
> + kfree(op);
> +out:
> + return ret;
> +}
> +
> +ssize_t vdpa_read(struct mdev_device *mdev, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + int done = 0;
> + unsigned int index;
> + loff_t pos = *ppos;
> + struct vdpa_dev *vdpa;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + mutex_lock(&vdpa->ops_lock);
> +
> + index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
> +
> + switch (index) {
> + case VFIO_VHOST_CONFIG_REGION_INDEX:
> + done = vdpa_handle_config_read(mdev, buf, count, ppos);
> + break;
> + }
> +
> + if (done > 0)
> + *ppos += done;
> +
> + mutex_unlock(&vdpa->ops_lock);
> +
> + return done;
> +}
> +EXPORT_SYMBOL(vdpa_read);
> +
> +static int vhost_set_vring_addr(struct mdev_device *mdev,
> + struct vhost_vring_addr *addr)
> +{
> + struct vdpa_dev *vdpa;
> + int qid = addr->index;
> + struct vdpa_vring_info *vring;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + if (qid >= vdpa->max_vrings)
> + return -EINVAL;
> +
> + if (qid >= vdpa->nr_vring)
> + vdpa->nr_vring = qid + 1;
> +
> + vring = &vdpa->vring_info[qid];
> +
> + vring->desc_user_addr = addr->desc_user_addr;
> + vring->used_user_addr = addr->used_user_addr;
> + vring->avail_user_addr = addr->avail_user_addr;
> + vring->log_guest_addr = addr->log_guest_addr;
> +
> + return 0;
> +}
> +
> +static int vhost_set_vring_num(struct mdev_device *mdev,
> + struct vhost_vring_state *num)
> +{
> + struct vdpa_dev *vdpa;
> + int qid = num->index;
> + struct vdpa_vring_info *vring;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + if (qid >= vdpa->max_vrings)
> + return -EINVAL;
> +
> + vring = &vdpa->vring_info[qid];
> +
> + vring->size = num->num;
> +
> + return 0;
> +}
> +
> +static int vhost_set_vring_base(struct mdev_device *mdev,
> + struct vhost_vring_state *base)
> +{
> + struct vdpa_dev *vdpa;
> + int qid = base->index;
> + struct vdpa_vring_info *vring;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + if (qid >= vdpa->max_vrings)
> + return -EINVAL;
> +
> + vring = &vdpa->vring_info[qid];
> +
> + vring->base = base->num;
> +
> + return 0;
> +}
> +
> +static int vhost_get_vring_base(struct mdev_device *mdev,
> + struct vhost_vring_state *base)
> +{
> + struct vdpa_dev *vdpa;
> + int qid = base->index;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + vdpa->pending_reply = true;
> + vdpa->pending.request = VHOST_GET_VRING_BASE;
> + vdpa->pending.payload.state.index = qid;
> + vdpa->pending.payload.state.num = vdpa->vring_info[qid].base;
> +
> + return 0;
> +}
> +
> +static int vhost_set_log_base(struct mdev_device *mdev, u64 *log_base)
> +{
> + struct vdpa_dev *vdpa;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + vdpa->log_base = *log_base;
> + return 0;
> +}
> +
> +static int vhost_set_features(struct mdev_device *mdev, u64 *features)
> +{
> + struct vdpa_dev *vdpa;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + vdpa->features = *features;
> + vdpa->ops->set_features(vdpa);
> +
> + return 0;
> +}
> +
> +static int vhost_get_features(struct mdev_device *mdev, u64 *features)
> +{
> + struct vdpa_dev *vdpa;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + vdpa->pending_reply = true;
> + vdpa->pending.request = VHOST_GET_FEATURES;
> + vdpa->pending.payload.u64 =
> + vdpa->ops->supported_features(vdpa);
> +
> + return 0;
> +}
> +
> +static int vhost_set_owner(struct mdev_device *mdev)
> +{
> + // TODO
> + return 0;
> +}
> +
> +static int vhost_reset_owner(struct mdev_device *mdev)
> +{
> + // TODO
> + return 0;
> +}
> +
> +static int vhost_set_state(struct mdev_device *mdev, u64 *state)
> +{
> + struct vdpa_dev *vdpa;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + if (*state >= VHOST_DEVICE_S_MAX)
> + return -EINVAL;
> +
> + if (vdpa->state == *state)
> + return 0;
> +
> + vdpa->state = *state;
> +
> + switch (vdpa->state) {
> + case VHOST_DEVICE_S_RUNNING:
> + vdpa->ops->start(vdpa);
> + break;
> + case VHOST_DEVICE_S_STOPPED:
> + vdpa->ops->stop(vdpa);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t vdpa_handle_config_write(struct mdev_device *mdev,
> + const char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct vhost_vfio_op *op = NULL;
> + loff_t pos = *ppos;
> + loff_t offset;
> + int ret;
> +
> + offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
> + if (offset != 0) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (count < VHOST_VFIO_OP_HDR_SIZE) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + op = kzalloc(VHOST_VFIO_OP_HDR_SIZE + VHOST_VFIO_OP_PAYLOAD_MAX_SIZE,
> + GFP_KERNEL);
> + if (op == NULL) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + if (copy_from_user(op, buf, VHOST_VFIO_OP_HDR_SIZE)) {
> + ret = -EINVAL;
> + goto out_free;
> + }
> +
> + if (op->size > VHOST_VFIO_OP_PAYLOAD_MAX_SIZE ||
> + op->size + VHOST_VFIO_OP_HDR_SIZE != count) {
> + ret = -EINVAL;
> + goto out_free;
> + }
> +
> + if (copy_from_user(&op->payload, buf + VHOST_VFIO_OP_HDR_SIZE,
> + op->size)) {
> + ret = -EFAULT;
> + goto out_free;
> + }
> +
> + switch (op->request) {
> + case VHOST_SET_LOG_BASE:
> + vhost_set_log_base(mdev, &op->payload.u64);
> + break;
> + case VHOST_SET_VRING_ADDR:
> + vhost_set_vring_addr(mdev, &op->payload.addr);
> + break;
> + case VHOST_SET_VRING_NUM:
> + vhost_set_vring_num(mdev, &op->payload.state);
> + break;
> + case VHOST_SET_VRING_BASE:
> + vhost_set_vring_base(mdev, &op->payload.state);
> + break;
> + case VHOST_GET_VRING_BASE:
> + vhost_get_vring_base(mdev, &op->payload.state);
> + break;
> + case VHOST_SET_FEATURES:
> + vhost_set_features(mdev, &op->payload.u64);
> + break;
> + case VHOST_GET_FEATURES:
> + vhost_get_features(mdev, &op->payload.u64);
> + break;
> + case VHOST_SET_OWNER:
> + vhost_set_owner(mdev);
> + break;
> + case VHOST_RESET_OWNER:
> + vhost_reset_owner(mdev);
> + break;
> + case VHOST_DEVICE_SET_STATE:
> + vhost_set_state(mdev, &op->payload.u64);
> + break;
> + default:
> + break;
> + }
> +
> + ret = count;
> +
> +out_free:
> + kfree(op);
> +out:
> + return ret;
> +}
> +
> +static ssize_t vdpa_handle_notify_write(struct mdev_device *mdev,
> + const char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct vdpa_dev *vdpa;
> + int qid;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + if (count < sizeof(qid))
> + return -EINVAL;
> +
> + if (copy_from_user(&qid, buf, sizeof(qid)))
> + return -EINVAL;
> +
> + vdpa->ops->notify(vdpa, qid);
> +
> + return count;
> +}
> +
> +ssize_t vdpa_write(struct mdev_device *mdev, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + int done = 0;
> + unsigned int index;
> + loff_t pos = *ppos;
> + struct vdpa_dev *vdpa;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + mutex_lock(&vdpa->ops_lock);
> +
> + index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
> +
> + switch (index) {
> + case VFIO_VHOST_CONFIG_REGION_INDEX:
> + done = vdpa_handle_config_write(mdev, buf, count, ppos);
> + break;
> + case VFIO_VHOST_NOTIFY_REGION_INDEX:
> + done = vdpa_handle_notify_write(mdev, buf, count, ppos);
> + break;
> + }
> +
> + if (done > 0)
> + *ppos += done;
> +
> + mutex_unlock(&vdpa->ops_lock);
> +
> + return done;
> +}
> +EXPORT_SYMBOL(vdpa_write);
> +
> +static int vdpa_get_region_info(struct mdev_device *mdev,
> + struct vfio_region_info *region_info,
> + u16 *cap_type_id, void **cap_type)
> +{
> + struct vdpa_dev *vdpa;
> + u32 index, flags;
> + u64 size = 0;
> +
> + if (!mdev)
> + return -EINVAL;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -EINVAL;
> +
> + index = region_info->index;
> + if (index >= VFIO_VHOST_NUM_REGIONS)
> + return -EINVAL;
> +
> + mutex_lock(&vdpa->ops_lock);
> +
> + flags = VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_WRITE;
> +
> + switch (index) {
> + case VFIO_VHOST_CONFIG_REGION_INDEX:
> + size = VDPA_CONFIG_SIZE;
> + break;
> + case VFIO_VHOST_NOTIFY_REGION_INDEX:
> + size = (u64)vdpa->max_vrings << PAGE_SHIFT;
> + flags |= VFIO_REGION_INFO_FLAG_MMAP;
> + break;
> + default:
> + size = 0;
> + break;
> + }
> +
> + region_info->size = size;
> + region_info->offset = VDPA_VFIO_VHOST_INDEX_TO_OFFSET(index);
> + region_info->flags = flags;
> + mutex_unlock(&vdpa->ops_lock);
> + return 0;
> +}
> +
> +static int vdpa_reset(struct mdev_device *mdev)
> +{
> + struct vdpa_dev *vdpa;
> +
> + if (!mdev)
> + return -EINVAL;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int vdpa_get_device_info(struct mdev_device *mdev,
> + struct vfio_device_info *dev_info)
> +{
> + struct vdpa_dev *vdpa;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + dev_info->flags = VFIO_DEVICE_FLAGS_VHOST | VFIO_DEVICE_RESET;
> + dev_info->num_regions = VFIO_VHOST_NUM_REGIONS;
> + dev_info->num_irqs = VFIO_VHOST_NUM_IRQS;
> +
> + return 0;
> +}
> +
> +static int vdpa_get_irq_info(struct mdev_device *mdev,
> + struct vfio_irq_info *info)
> +{
> + struct vdpa_dev *vdpa;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + if (info->index != VFIO_VHOST_VQ_IRQ_INDEX)
> + return -EINVAL;
> +
> + info->flags = VFIO_IRQ_INFO_EVENTFD;
> + info->count = vdpa->max_vrings;
> +
> + return 0;
> +}
> +
> +static int vdpa_set_irqs(struct mdev_device *mdev, uint32_t flags,
> + unsigned int index, unsigned int start,
> + unsigned int count, void *data)
> +{
> + struct vdpa_dev *vdpa;
> + int *fd = data, i;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -EINVAL;
> +
> + if (index != VFIO_VHOST_VQ_IRQ_INDEX)
> + return -ENOTSUPP;
> +
> + for (i = 0; i < count; i++)
> + vdpa->ops->set_eventfd(vdpa, start + i,
> + (flags & VFIO_IRQ_SET_DATA_EVENTFD) ? fd[i] : -1);
> +
> + return 0;
> +}
> +
> +long vdpa_ioctl(struct mdev_device *mdev, unsigned int cmd, unsigned long arg)
> +{
> + int ret = 0;
> + unsigned long minsz;
> + struct vdpa_dev *vdpa;
> +
> + if (!mdev)
> + return -EINVAL;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + switch (cmd) {
> + case VFIO_DEVICE_GET_INFO:
> + {
> + struct vfio_device_info info;
> +
> + minsz = offsetofend(struct vfio_device_info, num_irqs);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + ret = vdpa_get_device_info(mdev, &info);
> + if (ret)
> + return ret;
> +
> + if (copy_to_user((void __user *)arg, &info, minsz))
> + return -EFAULT;
> +
> + return 0;
> + }
> + case VFIO_DEVICE_GET_REGION_INFO:
> + {
> + struct vfio_region_info info;
> + u16 cap_type_id = 0;
> + void *cap_type = NULL;
> +
> + minsz = offsetofend(struct vfio_region_info, offset);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + ret = vdpa_get_region_info(mdev, &info, &cap_type_id,
> + &cap_type);
> + if (ret)
> + return ret;
> +
> + if (copy_to_user((void __user *)arg, &info, minsz))
> + return -EFAULT;
> +
> + return 0;
> + }
> + case VFIO_DEVICE_GET_IRQ_INFO:
> + {
> + struct vfio_irq_info info;
> +
> + minsz = offsetofend(struct vfio_irq_info, count);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz || info.index >= vdpa->max_vrings)
> + return -EINVAL;
> +
> + ret = vdpa_get_irq_info(mdev, &info);
> + if (ret)
> + return ret;
> +
> + if (copy_to_user((void __user *)arg, &info, minsz))
> + return -EFAULT;
> +
> + return 0;
> + }
> + case VFIO_DEVICE_SET_IRQS:
> + {
> + struct vfio_irq_set hdr;
> + size_t data_size = 0;
> + u8 *data = NULL;
> +
> + minsz = offsetofend(struct vfio_irq_set, count);
> +
> + if (copy_from_user(&hdr, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + ret = vfio_set_irqs_validate_and_prepare(&hdr, vdpa->max_vrings,
> + VFIO_VHOST_NUM_IRQS,
> + &data_size);
> + if (ret)
> + return ret;
> +
> + if (data_size) {
> + data = memdup_user((void __user *)(arg + minsz),
> + data_size);
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> + }
> +
> + ret = vdpa_set_irqs(mdev, hdr.flags, hdr.index, hdr.start,
> + hdr.count, data);
> +
> + kfree(data);
> + return ret;
> + }
> + case VFIO_DEVICE_RESET:
> + return vdpa_reset(mdev);
> + }
> + return -ENOTTY;
> +}
> +EXPORT_SYMBOL(vdpa_ioctl);
> +
> +static const struct vm_operations_struct vdpa_mm_ops = {
> +#ifdef CONFIG_HAVE_IOREMAP_PROT
> + .access = generic_access_phys
> +#endif
> +};
> +
> +int vdpa_mmap(struct mdev_device *mdev, struct vm_area_struct *vma)
> +{
> + struct vdpa_dev *vdpa;
> + unsigned int index;
> + loff_t pos;
> + loff_t offset;
> + int qid, ret;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + pos = vma->vm_pgoff << PAGE_SHIFT;
> +
> + index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
> + offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
> +
> + qid = offset >> PAGE_SHIFT;
> +
> + if (vma->vm_end < vma->vm_start)
> + return -EINVAL;
> + if ((vma->vm_flags & VM_SHARED) == 0)
> + return -EINVAL;
> + if (index != VFIO_VHOST_NOTIFY_REGION_INDEX)
> + return -EINVAL;
> + if (qid < 0 || qid >= vdpa->max_vrings)
> + return -EINVAL;
> +
> + if (vma->vm_end - vma->vm_start > PAGE_SIZE)
> + return -EINVAL;
> +
> + if (vdpa->ops->get_notify_addr == NULL)
> + return -ENOTSUPP;
> +
> + mutex_lock(&vdpa->ops_lock);
> +
> + vma->vm_ops = &vdpa_mm_ops;
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +
> + vma->vm_pgoff = vdpa->ops->get_notify_addr(vdpa, qid) >> PAGE_SHIFT;
> +
> + ret = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> + vma->vm_end - vma->vm_start, vma->vm_page_prot);
> +
> + mutex_unlock(&vdpa->ops_lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(vdpa_mmap);
> +
> +int vdpa_open(struct mdev_device *mdev)
> +{
> + return 0;
> +}
> +EXPORT_SYMBOL(vdpa_open);
> +
> +void vdpa_close(struct mdev_device *mdev)
> +{
> +}
> +EXPORT_SYMBOL(vdpa_close);
> +
> +MODULE_VERSION("0.0.0");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Hardware vhost accelerator abstraction");
> diff --git a/include/linux/vdpa_mdev.h b/include/linux/vdpa_mdev.h
> new file mode 100644
> index 000000000000..4bbdf7e2e712
> --- /dev/null
> +++ b/include/linux/vdpa_mdev.h
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018-2019 Intel Corporation.
> + */
> +
> +#ifndef VDPA_MDEV_H
> +#define VDPA_MDEV_H
> +
> +struct mdev_device;
> +struct vdpa_dev;
> +
> +/*
> + * XXX: Any comments about the vDPA API design for drivers
> + * would be appreciated!
> + */
> +
> +typedef int (*vdpa_start_device_t)(struct vdpa_dev *vdpa);
> +typedef int (*vdpa_stop_device_t)(struct vdpa_dev *vdpa);
> +typedef int (*vdpa_set_features_t)(struct vdpa_dev *vdpa);
> +typedef int (*vdpa_set_eventfd_t)(struct vdpa_dev *vdpa, int queue_idx, int fd);
> +typedef u64 (*vdpa_supported_features_t)(struct vdpa_dev *vdpa);
> +typedef void (*vdpa_notify_device_t)(struct vdpa_dev *vdpa, int queue_idx);
> +typedef u64 (*vdpa_get_notify_addr_t)(struct vdpa_dev *vdpa, int queue_idx);
> +
> +struct vdpa_device_ops {
> + vdpa_start_device_t start;
> + vdpa_stop_device_t stop;
> + vdpa_set_eventfd_t set_eventfd;
> + vdpa_supported_features_t supported_features;
> + vdpa_notify_device_t notify;
> + vdpa_get_notify_addr_t get_notify_addr;
> + vdpa_set_features_t set_features;
> +};
> +
> +struct vdpa_vring_info {
> + u64 desc_user_addr;
> + u64 used_user_addr;
> + u64 avail_user_addr;
> + u64 log_guest_addr;
> + u16 size;
> + u16 base;
> +};
> +
> +struct vdpa_dev {
> + struct mdev_device *mdev;
> + struct mutex ops_lock;
> + int nr_vring;
> + u64 features;
> + u64 state;
> + bool pending_reply;
> + struct vhost_vfio_op pending;
> + const struct vdpa_device_ops *ops;
> + void *private;
> + int max_vrings;
> + uint64_t log_base;
> + uint64_t log_size;
> + struct vdpa_vring_info vring_info[0];
> +};
> +
> +struct vdpa_dev *vdpa_alloc(struct mdev_device *mdev, void *private,
> + int max_vrings);
> +void vdpa_free(struct vdpa_dev *vdpa);
> +ssize_t vdpa_read(struct mdev_device *mdev, char __user *buf,
> + size_t count, loff_t *ppos);
> +ssize_t vdpa_write(struct mdev_device *mdev, const char __user *buf,
> + size_t count, loff_t *ppos);
> +long vdpa_ioctl(struct mdev_device *mdev, unsigned int cmd, unsigned long arg);
> +int vdpa_mmap(struct mdev_device *mdev, struct vm_area_struct *vma);
> +int vdpa_open(struct mdev_device *mdev);
> +void vdpa_close(struct mdev_device *mdev);
> +
> +#endif /* VDPA_MDEV_H */
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 8f10748dac79..6c5718ab7eeb 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -201,6 +201,7 @@ struct vfio_device_info {
> #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */
> #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */
> #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */
> +#define VFIO_DEVICE_FLAGS_VHOST (1 << 6) /* vfio-vhost device */
> __u32 num_regions; /* Max region index + 1 */
> __u32 num_irqs; /* Max IRQ index + 1 */
> };
> @@ -217,6 +218,7 @@ struct vfio_device_info {
> #define VFIO_DEVICE_API_AMBA_STRING "vfio-amba"
> #define VFIO_DEVICE_API_CCW_STRING "vfio-ccw"
> #define VFIO_DEVICE_API_AP_STRING "vfio-ap"
> +#define VFIO_DEVICE_API_VHOST_STRING "vfio-vhost"
>
> /**
> * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
> @@ -573,6 +575,23 @@ enum {
> VFIO_CCW_NUM_IRQS
> };
>
> +/*
> + * The vfio-vhost bus driver makes use of the following fixed region and
> + * IRQ index mapping. Unimplemented regions return a size of zero.
> + * Unimplemented IRQ types return a count of zero.
> + */
> +
> +enum {
> + VFIO_VHOST_CONFIG_REGION_INDEX,
> + VFIO_VHOST_NOTIFY_REGION_INDEX,
> + VFIO_VHOST_NUM_REGIONS
> +};
> +
> +enum {
> + VFIO_VHOST_VQ_IRQ_INDEX,
> + VFIO_VHOST_NUM_IRQS
> +};
> +
> /**
> * VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IORW(VFIO_TYPE, VFIO_BASE + 12,
> * struct vfio_pci_hot_reset_info)
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index 40d028eed645..ad95b90c5c05 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -116,4 +116,29 @@
> #define VHOST_VSOCK_SET_GUEST_CID _IOW(VHOST_VIRTIO, 0x60, __u64)
> #define VHOST_VSOCK_SET_RUNNING _IOW(VHOST_VIRTIO, 0x61, int)
>
> +/* VHOST_DEVICE specific defines */
> +
> +#define VHOST_DEVICE_SET_STATE _IOW(VHOST_VIRTIO, 0x70, __u64)
> +
> +#define VHOST_DEVICE_S_STOPPED 0
> +#define VHOST_DEVICE_S_RUNNING 1
> +#define VHOST_DEVICE_S_MAX 2
> +
> +struct vhost_vfio_op {
> + __u64 request;
> + __u32 flags;
> + /* Flag values: */
> +#define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
> + __u32 size;
> + union {
> + __u64 u64;
> + struct vhost_vring_state state;
> + struct vhost_vring_addr addr;
> + } payload;
> +};
> +
> +#define VHOST_VFIO_OP_HDR_SIZE \
> + ((unsigned long)&((struct vhost_vfio_op *)NULL)->payload)
> +#define VHOST_VFIO_OP_PAYLOAD_MAX_SIZE 1024 /* FIXME TBD */
> +
> #endif
^ permalink raw reply
* Re: [PATCH] bpf: Replace a seq_printf() call by seq_puts() in btf_enum_seq_show()
From: Daniel Borkmann @ 2019-07-03 10:09 UTC (permalink / raw)
To: Markus Elfring, bpf, netdev, Alexei Starovoitov, Martin KaFai Lau,
Song Liu, Yonghong Song
Cc: LKML, kernel-janitors
In-Reply-To: <93898abe-9a7d-0c64-0856-094b62e07ba2@web.de>
On 07/02/2019 07:13 PM, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 2 Jul 2019 19:04:08 +0200
>
> A string which did not contain a data format specification should be put
> into a sequence. Thus use the corresponding function “seq_puts”.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
The code is fine as is, I'm not applying this.
^ permalink raw reply
* Re: [PATCH v2 0/3] vsock/virtio: several fixes in the .probe() and .remove()
From: Stefano Garzarella @ 2019-07-03 10:07 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190703091453.GA11844@stefanha-x1.localdomain>
On Wed, Jul 03, 2019 at 10:14:53AM +0100, Stefan Hajnoczi wrote:
> On Mon, Jul 01, 2019 at 07:03:57PM +0200, Stefano Garzarella wrote:
> > On Mon, Jul 01, 2019 at 04:11:13PM +0100, Stefan Hajnoczi wrote:
> > > On Fri, Jun 28, 2019 at 02:36:56PM +0200, Stefano Garzarella wrote:
> > > > 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.
> > > >
> > > > v2:
> > > > - Patch 1: use RCU to protect 'the_virtio_vsock' pointer
> > > > - Patch 2: no changes
> > > > - Patch 3: flush works only at the end of .remove()
> > > > - Removed patch 4 because virtqueue_detach_unused_buf() returns all the buffers
> > > > allocated.
> > > >
> > > > v1: https://patchwork.kernel.org/cover/10964733/
> > >
> > > This looks good to me.
> >
> > Thanks for the review!
> >
> > >
> > > Did you run any stress tests? For example an SMP guest constantly
> > > connecting and sending packets together with a script that
> > > hotplug/unplugs vhost-vsock-pci from the host side.
> >
> > Yes, I started an SMP guest (-smp 4 -monitor tcp:127.0.0.1:1234,server,nowait)
> > and I run 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
> >
> > Do you think is enough or is better to have a test more accurate?
>
> That's good when left running overnight so that thousands of hotplug
> events are tested.
Honestly I run the test for ~30 mins (because without the patch the
crash happens in a few seconds), but of course, I'll run it this night :)
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Jiri Pirko @ 2019-07-03 10:04 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190702163437.GE20101@unicorn.suse.cz>
Tue, Jul 02, 2019 at 06:34:37PM CEST, mkubecek@suse.cz wrote:
>On Tue, Jul 02, 2019 at 03:05:15PM +0200, Jiri Pirko wrote:
>> Tue, Jul 02, 2019 at 01:50:04PM CEST, mkubecek@suse.cz wrote:
>> >
>> >+/* request header */
>> >+
>> >+/* use compact bitsets in reply */
>> >+#define ETHTOOL_RF_COMPACT (1 << 0)
>>
>> "COMPACT_BITSETS"?
>>
>> >+/* provide optional reply for SET or ACT requests */
>> >+#define ETHTOOL_RF_REPLY (1 << 1)
>>
>> "OPTIONAL_REPLY"?
>
>OK
>
>> >+ ret = nla_parse_nested(tb, ETHTOOL_A_HEADER_MAX, nest,
>> >+ policy ?: dflt_header_policy, extack);
>> >+ if (ret < 0)
>>
>> if (ret)
>>
>> Same remark goes to the rest of the code (also the rest of the patches),
>> in case called function cannot return positive values.
>
>The "if (ret < 0)" idiom for "on error do ..." is so ubiquitous through
>the whole kernel that I don't think it's worth it to carefully check
>which function can return a positive value and which cannot and risk
>that one day I overlook that some function. And yet another question is
>what exactly "cannot return" means: is it whenever the function does not
>return a positive value or only if it's explicitly documented not to?
>
>Looking at existing networking code, e.g. net/netfilter (except ipvs),
>net/sched or net/core/rtnetlink.c are using "if (ret < 0)" rather
>uniformly. And (as you objected to the check of genl_register_family()
>previous patch) even genetlink itself has
>
> err = genl_register_family(&genl_ctrl);
> if (err < 0)
> goto problem;
>
>in genl_init().
>
>>
>>
>> >+ return ret;
>> >+ devname_attr = tb[ETHTOOL_A_HEADER_DEV_NAME];
>> >+
>> >+ if (tb[ETHTOOL_A_HEADER_DEV_INDEX]) {
>> >+ u32 ifindex = nla_get_u32(tb[ETHTOOL_A_HEADER_DEV_INDEX]);
>> >+
>> >+ dev = dev_get_by_index(net, ifindex);
>> >+ if (!dev) {
>> >+ NL_SET_ERR_MSG_ATTR(extack,
>> >+ tb[ETHTOOL_A_HEADER_DEV_INDEX],
>> >+ "no device matches ifindex");
>> >+ return -ENODEV;
>> >+ }
>> >+ /* if both ifindex and ifname are passed, they must match */
>> >+ if (devname_attr &&
>> >+ strncmp(dev->name, nla_data(devname_attr), IFNAMSIZ)) {
>> >+ dev_put(dev);
>> >+ NL_SET_ERR_MSG_ATTR(extack, nest,
>> >+ "ifindex and name do not match");
>> >+ return -ENODEV;
>> >+ }
>> >+ } else if (devname_attr) {
>> >+ dev = dev_get_by_name(net, nla_data(devname_attr));
>> >+ if (!dev) {
>> >+ NL_SET_ERR_MSG_ATTR(extack, devname_attr,
>> >+ "no device matches name");
>> >+ return -ENODEV;
>> >+ }
>> >+ } else if (require_dev) {
>> >+ NL_SET_ERR_MSG_ATTR(extack, nest,
>> >+ "neither ifindex nor name specified");
>> >+ return -EINVAL;
>> >+ }
>> >+
>> >+ if (dev && !netif_device_present(dev)) {
>> >+ dev_put(dev);
>> >+ NL_SET_ERR_MSG(extack, "device not present");
>> >+ return -ENODEV;
>> >+ }
>> >+
>> >+ req_info->dev = dev;
>> >+ ethnl_update_u32(&req_info->req_mask, tb[ETHTOOL_A_HEADER_INFOMASK]);
>> >+ ethnl_update_u32(&req_info->global_flags, tb[ETHTOOL_A_HEADER_GFLAGS]);
>> >+ ethnl_update_u32(&req_info->req_flags, tb[ETHTOOL_A_HEADER_RFLAGS]);
>>
>> Just:
>> req_info->req_mask = nla_get_u32(tb[ETHTOOL_A_HEADER_INFOMASK];
>> ...
>>
>> Not sure what ethnl_update_u32() is good for, but it is not needed here.
>
>That would result in null pointer dereference if the attribute is
>missing. So you would need at least
>
> if (tb[ETHTOOL_A_HEADER_INFOMASK])
> req_info->req_mask = nla_get_u32(tb[ETHTOOL_A_HEADER_INFOMASK]);
> if (tb[ETHTOOL_A_HEADER_GFLAGS])
> req_info->global_flags =
> nla_get_u32(tb[ETHTOOL_A_HEADER_GFLAGS]);
> if (tb[ETHTOOL_A_HEADER_RFLAGS])
> req_info->req_flags = nla_get_u32(tb[ETHTOOL_A_HEADER_RFLAGS]);
Yeah, sure.
>
>I don't think it looks better.
Better than hiding something inside a helper in my opinion - helper that
is there for different reason moreover. Much easier to read the code
and follow.
>
>> >+
>> >+ return 0;
>> >+}
>> >+
>> >+/**
>> >+ * ethnl_fill_reply_header() - Put standard header into a reply message
>> >+ * @skb: skb with the message
>> >+ * @dev: network device to describe in header
>> >+ * @attrtype: attribute type to use for the nest
>> >+ *
>> >+ * Create a nested attribute with attributes describing given network device.
>> >+ * Clean up on error.
>>
>> Cleanup is obvious, no need to mention it in API docs.
>
>OK
>
>> >+ *
>> >+ * Return: 0 on success, error value (-EMSGSIZE only) on error
>> >+ */
>> >+int ethnl_fill_reply_header(struct sk_buff *skb, struct net_device *dev,
>> >+ u16 attrtype)
>> >+{
>> >+ struct nlattr *nest;
>> >+
>> >+ if (!dev)
>> >+ return 0;
>> >+ nest = nla_nest_start(skb, attrtype);
>> >+ if (!nest)
>> >+ return -EMSGSIZE;
>> >+
>> >+ if (nla_put_u32(skb, ETHTOOL_A_HEADER_DEV_INDEX, (u32)dev->ifindex) ||
>> >+ nla_put_string(skb, ETHTOOL_A_HEADER_DEV_NAME, dev->name))
>> >+ goto nla_put_failure;
>> >+ /* If more attributes are put into reply header, ethnl_header_size()
>> >+ * must be updated to account for them.
>> >+ */
>> >+
>> >+ nla_nest_end(skb, nest);
>> >+ return 0;
>> >+
>> >+nla_put_failure:
>> >+ nla_nest_cancel(skb, nest);
>> >+ return -EMSGSIZE;
>> >+}
>> >+
>> >+/**
>> >+ * ethnl_reply_init() - Create skb for a reply and fill device identification
>> >+ * @payload: payload length (without netlink and genetlink header)
>> >+ * @dev: device the reply is about (may be null)
>> >+ * @cmd: ETHTOOL_MSG_* message type for reply
>> >+ * @info: genetlink info of the received packet we respond to
>> >+ * @ehdrp: place to store payload pointer returned by genlmsg_new()
>> >+ *
>> >+ * Return: pointer to allocated skb on success, NULL on error
>> >+ */
>> >+struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
>> >+ u16 hdr_attrtype, struct genl_info *info,
>> >+ void **ehdrp)
>> >+{
>> >+ struct sk_buff *skb;
>> >+
>> >+ skb = genlmsg_new(payload, GFP_KERNEL);
>> >+ if (!skb)
>> >+ goto err;
>> >+ *ehdrp = genlmsg_put_reply(skb, info, ðtool_genl_family, 0, cmd);
>> >+ if (!*ehdrp)
>> >+ goto err_free;
>> >+
>> >+ if (dev) {
>> >+ int ret;
>> >+
>> >+ ret = ethnl_fill_reply_header(skb, dev, hdr_attrtype);
>> >+ if (ret < 0)
>> >+ goto err;
>> >+ }
>> >+ return skb;
>> >+
>> >+err_free:
>> >+ nlmsg_free(skb);
>> >+ if (info)
>> >+ GENL_SET_ERR_MSG(info, "failed to setup reply message");
>> >+err:
>>
>> Why also not fillup extack msg here?
>
>Right, err label should be right below the nlmsg_free(skb), thanks. And
>now I noticed another mistake: on ethnl_fill_reply_header() failure, we
>should go to err_free, not err.
>
>> >+static inline int ethnl_str_size(const char *s)
>>
>> If you really need this helper, put it into netlink code. There's nothing
>> ethtool-specific about this.
>
>OK, I'll look into it. I've been already thinking about some kind of
>NLA_SIZEOF() macro as about 1/3 of all uses of nla_total_size() follow
>the nla_total_size(sizeof(...)) pattern (and lot more should follow it
>but are written like e.g. nla_total_size(4) instead). This is another
>common pattern.
>
>> >+/* The ethnl_update_* helpers set value pointed to by @dst to the value of
>> >+ * netlink attribute @attr (if attr is not null). They return true if *dst
>> >+ * value was changed, false if not.
>> >+ */
>> >+static inline bool ethnl_update_u32(u32 *dst, struct nlattr *attr)
>>
>> I'm still not sure I'm convinced about these "update helpers" :)
>
>Just imagine what would e.g.
>
> if (ethnl_update_u32(&data.rx_pending, tb[ETHTOOL_A_RING_RX_PENDING]))
> mod = true;
> if (ethnl_update_u32(&data.rx_mini_pending,
> tb[ETHTOOL_A_RING_RX_MINI_PENDING]))
> mod = true;
> if (ethnl_update_u32(&data.rx_jumbo_pending,
> tb[ETHTOOL_A_RING_RX_JUMBO_PENDING]))
> mod = true;
> if (ethnl_update_u32(&data.tx_pending, tb[ETHTOOL_A_RING_TX_PENDING]))
> mod = true;
> if (!mod)
> return 0;
>
>look like without them. And coalescing parameters would be much worse
>(22 attributes / struct members).
No, I understand your motivation, don't get me wrong. I just wonder that
no other netlink implementation need such mechanism. Maybe I'm not
looking close enough. But if it does, should be rathe netlink helper.
Regarding the example code you have here. It is prefered to store
function result in a variable "if check" that variable. But in your,
code, couldn't this be done without ifs?
bool mod = false;
ethnl_update_u32(&mod, &data.rx_pending, tb[ETHTOOL_A_RING_RX_PENDING]))
ethnl_update_u32(&mod, &data.rx_mini_pending,
tb[ETHTOOL_A_RING_RX_MINI_PENDING]))
ethnl_update_u32(&mod, &data.rx_jumbo_pending,
tb[ETHTOOL_A_RING_RX_JUMBO_PENDING]))
ethnl_update_u32(&mod, &data.tx_pending, tb[ETHTOOL_A_RING_TX_PENDING]))
if (!mod)
return 0;
>
>> >+{
>> >+ u32 val;
>> >+
>> >+ if (!attr)
>> >+ return false;
>> >+ val = nla_get_u32(attr);
>> >+ if (*dst == val)
>> >+ return false;
>> >+
>> >+ *dst = val;
>> >+ return true;
>> >+}
>...
>> >+static inline bool ethnl_update_binary(u8 *dst, unsigned int len,
>>
>> void *dst
>
>OK.
>
>> >+/**
>> >+ * ethnl_is_privileged() - check if request has sufficient privileges
>> >+ * @skb: skb with client request
>> >+ *
>> >+ * Checks if client request has CAP_NET_ADMIN in its netns. Unlike the flags
>> >+ * in genl_ops, this allows finer access control, e.g. allowing or denying
>> >+ * the request based on its contents or witholding only part of the data
>> >+ * from unprivileged users.
>> >+ *
>> >+ * Return: true if request is privileged, false if not
>> >+ */
>> >+static inline bool ethnl_is_privileged(struct sk_buff *skb)
>>
>> I wonder why you need this helper. Genetlink uses
>> ops->flags & GENL_ADMIN_PERM for this.
>
>It's explained in the function description. Sometimes we need finer
>control than by request message type. An example is the WoL password:
>ETHTOOL_GWOL is privileged because of it but I believe there si no
>reason why unprivileged user couldn't see enabled WoL modes, we can
>simply omit the password for him. (Also, it allows to combine query for
>WoL settings with other unprivileged settings.)
Why can't we have rather:
ETHTOOL_WOL_GET for all
ETHTOOL_WOL_PASSWORD_GET with GENL_ADMIN_PERM
?
Better to stick with what we have in gennetlink rather then to bend the
implementation from the very beginning I think.
>
>> >+/**
>> >+ * ethnl_reply_header_size() - total size of reply header
>> >+ *
>> >+ * This is an upper estimate so that we do not need to hold RTNL lock longer
>> >+ * than necessary (to prevent rename between size estimate and composing the
>>
>> I guess this description is not relevant anymore. I don't see why to
>> hold rtnl mutex for this function...
>
>You don't need it for this function, it's the other way around: unless
>you hold RTNL lock for the whole time covering both checking needed
>message size and filling the message - and we don't - the device could
>be renamed in between. Thus if we returned size based on current device
>name, it might not be sufficient at the time the header is filled.
>That's why this function returns maximum possible size (which is
>actually a constant).
I suggest to avoid the description. It is misleading. Perhaps something
to have in a patch description but not here in code.
>
>Michal
>
>> >+ * message). Accounts only for device ifindex and name as those are the only
>> >+ * attributes ethnl_fill_reply_header() puts into the reply header.
>> >+ */
>> >+static inline unsigned int ethnl_reply_header_size(void)
>> >+{
>> >+ return nla_total_size(nla_total_size(sizeof(u32)) +
>> >+ nla_total_size(IFNAMSIZ));
>> >+}
^ permalink raw reply
* Re: [PATCH v2 1/3] vsock/virtio: use RCU to avoid use-after-free on the_virtio_vsock
From: Jason Wang @ 2019-07-03 9:53 UTC (permalink / raw)
To: Stefano Garzarella, netdev
Cc: kvm, virtualization, Stefan Hajnoczi, Michael S. Tsirkin,
David S. Miller, linux-kernel
In-Reply-To: <20190628123659.139576-2-sgarzare@redhat.com>
On 2019/6/28 下午8:36, Stefano Garzarella wrote:
> 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 | 67 +++++++++++++++++++++-----------
> 1 file changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 9c287e3e393c..7ad510ec12e0 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)
> @@ -590,8 +606,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 +627,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);
You probably need to use rcu_dereference_protected() to access
the_virtio_vsock in the function in order to survive from sparse.
> +
> mutex_unlock(&the_virtio_vsock_mutex);
> return 0;
>
> @@ -627,6 +644,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);
This is still suspicious, can we access the_virtio_vsock through
vdev->priv? If yes, we may still get use-after-free since it was not
protected by RCU.
Another more interesting question, I believe we will do singleton for
virtio_vsock structure. Then what's the point of using vdev->priv to
access the_virtio_vsock? It looks to me we can it brings extra troubles
for doing synchronization.
Thanks
> + synchronize_rcu();
> +
> flush_work(&vsock->loopback_work);
> flush_work(&vsock->rx_work);
> flush_work(&vsock->tx_work);
> @@ -666,12 +689,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);
> }
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix precision tracking
From: Daniel Borkmann @ 2019-07-03 9:50 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: netdev, bpf, kernel-team
In-Reply-To: <20190628162409.2513499-1-ast@kernel.org>
On 06/28/2019 06:24 PM, Alexei Starovoitov wrote:
> When equivalent state is found the current state needs to propagate precision marks.
> Otherwise the verifier will prune the search incorrectly.
>
> There is a price for correctness:
> before before broken fixed
> cnst spill precise precise
> bpf_lb-DLB_L3.o 1923 8128 1863 1898
> bpf_lb-DLB_L4.o 3077 6707 2468 2666
> bpf_lb-DUNKNOWN.o 1062 1062 544 544
> bpf_lxc-DDROP_ALL.o 166729 380712 22629 36823
> bpf_lxc-DUNKNOWN.o 174607 440652 28805 45325
> bpf_netdev.o 8407 31904 6801 7002
> bpf_overlay.o 5420 23569 4754 4858
> bpf_lxc_jit.o 39389 359445 50925 69631
> Overall precision tracking is still very effective.
>
> Fixes: b5dc0163d8fd ("bpf: precise scalar_value tracking")
> Reported-by: Lawrence Brakmo <brakmo@fb.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf 1/3] bpf, x32: Fix bug with ALU64 {LSH,RSH,ARSH} BPF_X shift by 0
From: Daniel Borkmann @ 2019-07-03 9:49 UTC (permalink / raw)
To: Luke Nelson, linux-kernel
Cc: Luke Nelson, Xi Wang, Wang YanQing, David S. Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, x86, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Yonghong Song, Shuah Khan,
Jakub Kicinski, Jiong Wang, Björn Töpel, netdev, bpf,
linux-kselftest
In-Reply-To: <20190629055759.28365-1-luke.r.nels@gmail.com>
On 06/29/2019 07:57 AM, Luke Nelson wrote:
> The current x32 BPF JIT for shift operations is not correct when the
> shift amount in a register is 0. The expected behavior is a no-op, whereas
> the current implementation changes bits in the destination register.
>
> The following example demonstrates the bug. The expected result of this
> program is 1, but the current JITed code returns 2.
>
> r0 = 1
> r1 = 1
> r2 = 0
> r1 <<= r2
> if r1 == 1 goto end
> r0 = 2
> end:
> exit
>
> The bug is caused by an incorrect assumption by the JIT that a shift by
> 32 clear the register. On x32 however, shifts use the lower 5 bits of
> the source, making a shift by 32 equivalent to a shift by 0.
>
> This patch fixes the bug using double-precision shifts, which also
> simplifies the code.
>
> Fixes: 03f5781be2c7 ("bpf, x86_32: add eBPF JIT compiler for ia32")
> Co-developed-by: Xi Wang <xi.wang@gmail.com>
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
Series applied, thanks!
^ permalink raw reply
* [PATCH bpf-next v3] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-07-03 9:45 UTC (permalink / raw)
To: netdev
Cc: ast, daniel, kafai, songliubraving, yhs, andrii.nakryiko,
magnus.karlsson
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>
---
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;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v4 bpf-next 0/4] libbpf: add perf buffer abstraction and API
From: Daniel Borkmann @ 2019-07-03 9:36 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko, ast, kernel-team, bpf, netdev,
jakub.kicinski
In-Reply-To: <20190630065109.1794420-1-andriin@fb.com>
On 06/30/2019 08:51 AM, Andrii Nakryiko wrote:
> This patchset adds a high-level API for setting up and polling perf buffers
> associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
> described in corresponding commit.
>
> Patch #1 adds a set of APIs to set up and work with perf buffer.
> Patch #2 enhances libbpf to support auto-setting PERF_EVENT_ARRAY map size.
> Patch #3 adds test.
> Patch #4 converts bpftool map event_pipe to new API.
>
> v3->v4:
> - fixed bpftool event_pipe cmd error handling (Jakub);
>
> v2->v3:
> - added perf_buffer__new_raw for more low-level control;
> - converted bpftool map event_pipe to new API (Daniel);
> - fixed bug with error handling in create_maps (Song);
>
> v1->v2:
> - add auto-sizing of PERF_EVENT_ARRAY maps;
>
> Andrii Nakryiko (4):
> libbpf: add perf buffer API
> libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
> selftests/bpf: test perf buffer API
> tools/bpftool: switch map event_pipe to libbpf's perf_buffer
>
> tools/bpf/bpftool/map_perf_ring.c | 201 +++------
> tools/lib/bpf/libbpf.c | 397 +++++++++++++++++-
> tools/lib/bpf/libbpf.h | 49 +++
> tools/lib/bpf/libbpf.map | 4 +
> .../selftests/bpf/prog_tests/perf_buffer.c | 94 +++++
> .../selftests/bpf/progs/test_perf_buffer.c | 29 ++
> 6 files changed, 630 insertions(+), 144 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
Hm, set looks good, but this does not apply cleanly. Please rebase against
bpf-next and resubmit. Please also update tools/lib/bpf/README.rst with regards
to the perf_buffer__ prefix. While at it, you could also address Jakub's comment.
Thanks,
Daniel
^ permalink raw reply
* Re: Shall we add some note info for tcp_min_snd_mss?
From: Eric Dumazet @ 2019-07-03 9:34 UTC (permalink / raw)
To: ZhangXiao, edumazet, davem; +Cc: netdev
In-Reply-To: <5D1C68C8.6020408@windriver.com>
On 7/3/19 1:35 AM, ZhangXiao wrote:
> Hi David & Eric,
>
> Commit 5f3e2bf0 (tcp: add tcp_min_snd_mss sysctl) add a new interface to
> adjust network. While if this variable been set too large, for example
> larger then (MTU - 40), the net link maybe damaged. So, how about adding
> some warning messages for the operator/administrator? In document, or in
> source code.
What kind of warning message do you
envision exactly ?
The sysctl is global, devices MTU can be quite arbitrary.
Like almost all sysctls, sysadmins are supposed to know what they are doing.
^ permalink raw reply
* Re: [PATCH mlx5-next 4/5] net/mlx5: Introduce TLS TX offload hardware bits and structures
From: leon @ 2019-07-03 9:27 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
Eran Ben Elisha, Tariq Toukan
In-Reply-To: <20190703073909.14965-5-saeedm@mellanox.com>
On Wed, Jul 03, 2019 at 07:39:32AM +0000, Saeed Mahameed wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
>
> Add TLS offload related IFC structs, layouts and enumerations.
>
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> include/linux/mlx5/device.h | 14 +++++
> include/linux/mlx5/mlx5_ifc.h | 104 ++++++++++++++++++++++++++++++++--
> 2 files changed, 114 insertions(+), 4 deletions(-)
<...>
> @@ -2725,7 +2739,8 @@ struct mlx5_ifc_traffic_counter_bits {
>
> struct mlx5_ifc_tisc_bits {
> u8 strict_lag_tx_port_affinity[0x1];
> - u8 reserved_at_1[0x3];
> + u8 tls_en[0x1];
> + u8 reserved_at_1[0x2];
It should be reserved_at_2.
Thanks
^ permalink raw reply
* Re: [PATCH net] tcp: refine memory limit test in tcp_fragment()
From: Eric Dumazet @ 2019-07-03 9:19 UTC (permalink / raw)
To: Tony Lu, Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Christoph Paasch,
oliver.yang, xlpang, dust.li
In-Reply-To: <20190703032718.GC55248@TonyMac-Alibaba>
On 7/2/19 8:27 PM, Tony Lu wrote:
> Hello Eric,
>
> We have applied that commit e358f4af19db ("tcp: tcp_fragment() should apply sane memory limits")
> as a hotpatch in production environment. We found that it will make
> tcp long connection reset during sending out packet when applying
> that commit.
>
> Our applications which in A/B test have suffered that
> and made them retransmit large data, and then caused retransmission
> storm and lower the performance and increase RT.
>
> Therefore we discontinued to apply this hotpatch in A/B test.
>
> After invesgation, we found this patch already fix this issue in
> stable. Before applying this patch, we have some questions:
>
Which stable version are you referring to exactly ?
> 1. This commit in stable hard coded a magic number 0x20000. I am
> wondering this value and if there any better solution.
0x20000 is two times 64KB, please read the changelog for the rationale.
> 2. Is there any known or unknown side effect? If any, we could test
> it in some suspicious scenarios before testing in prod env.
No known side effect.
Honestly, applications setting small SO_SNDBUF values can not expect good TCP performance anyway.
>
> Thanks.
>
> Cheers,
> Tony Lu
>
> On Fri, Jun 21, 2019 at 06:09:55AM -0700, Eric Dumazet wrote:
>> tcp_fragment() might be called for skbs in the write queue.
>>
>> Memory limits might have been exceeded because tcp_sendmsg() only
>> checks limits at full skb (64KB) boundaries.
>>
>> Therefore, we need to make sure tcp_fragment() wont punish applications
>> that might have setup very low SO_SNDBUF values.
>>
>> Fixes: f070ef2ac667 ("tcp: tcp_fragment() should apply sane memory limits")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Reported-by: Christoph Paasch <cpaasch@apple.com>
>> ---
>> net/ipv4/tcp_output.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>> index 00c01a01b547ec67c971dc25a74c9258563cf871..0ebc33d1c9e5099d163a234930e213ee35e9fbd1 100644
>> --- a/net/ipv4/tcp_output.c
>> +++ b/net/ipv4/tcp_output.c
>> @@ -1296,7 +1296,8 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
>> if (nsize < 0)
>> nsize = 0;
>>
>> - if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf)) {
>> + if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf &&
>> + tcp_queue != TCP_FRAG_IN_WRITE_QUEUE)) {
>> NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPWQUEUETOOBIG);
>> return -ENOMEM;
>> }
>> --
>> 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Tiwei Bie @ 2019-07-03 9:13 UTC (permalink / raw)
To: mst, jasowang, alex.williamson, maxime.coquelin
Cc: linux-kernel, kvm, virtualization, netdev, dan.daly,
cunming.liang, zhihong.wang, tiwei.bie
Details about this can be found here:
https://lwn.net/Articles/750770/
What's new in this version
==========================
A new VFIO device type is introduced - vfio-vhost. This addressed
some comments from here: https://patchwork.ozlabs.org/cover/984763/
Below is the updated device interface:
Currently, there are two regions of this device: 1) CONFIG_REGION
(VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
can be used to notify the device.
1. CONFIG_REGION
The region described by CONFIG_REGION is the main control interface.
Messages will be written to or read from this region.
The message type is determined by the `request` field in message
header. The message size is encoded in the message header too.
The message format looks like this:
struct vhost_vfio_op {
__u64 request;
__u32 flags;
/* Flag values: */
#define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
__u32 size;
union {
__u64 u64;
struct vhost_vring_state state;
struct vhost_vring_addr addr;
} payload;
};
The existing vhost-kernel ioctl cmds are reused as the message
requests in above structure.
Each message will be written to or read from this region at offset 0:
int vhost_vfio_write(struct vhost_dev *dev, struct vhost_vfio_op *op)
{
int count = VHOST_VFIO_OP_HDR_SIZE + op->size;
struct vhost_vfio *vfio = dev->opaque;
int ret;
ret = pwrite64(vfio->device_fd, op, count, vfio->config_offset);
if (ret != count)
return -1;
return 0;
}
int vhost_vfio_read(struct vhost_dev *dev, struct vhost_vfio_op *op)
{
int count = VHOST_VFIO_OP_HDR_SIZE + op->size;
struct vhost_vfio *vfio = dev->opaque;
uint64_t request = op->request;
int ret;
ret = pread64(vfio->device_fd, op, count, vfio->config_offset);
if (ret != count || request != op->request)
return -1;
return 0;
}
It's quite straightforward to set things to the device. Just need to
write the message to device directly:
int vhost_vfio_set_features(struct vhost_dev *dev, uint64_t features)
{
struct vhost_vfio_op op;
op.request = VHOST_SET_FEATURES;
op.flags = 0;
op.size = sizeof(features);
op.payload.u64 = features;
return vhost_vfio_write(dev, &op);
}
To get things from the device, two steps are needed.
Take VHOST_GET_FEATURE as an example:
int vhost_vfio_get_features(struct vhost_dev *dev, uint64_t *features)
{
struct vhost_vfio_op op;
int ret;
op.request = VHOST_GET_FEATURES;
op.flags = VHOST_VFIO_NEED_REPLY;
op.size = 0;
/* Just need to write the header */
ret = vhost_vfio_write(dev, &op);
if (ret != 0)
goto out;
/* `op` wasn't changed during write */
op.flags = 0;
op.size = sizeof(*features);
ret = vhost_vfio_read(dev, &op);
if (ret != 0)
goto out;
*features = op.payload.u64;
out:
return ret;
}
2. NOTIFIY_REGION (mmap-able)
The region described by NOTIFY_REGION will be used to notify
the device.
Each queue will have a page for notification, and it can be mapped
to VM (if hardware also supports), and the virtio driver in the VM
will be able to notify the device directly.
The region described by NOTIFY_REGION is also write-able. If
the accelerator's notification register(s) cannot be mapped to
the VM, write() can also be used to notify the device. Something
like this:
void notify_relay(void *opaque)
{
......
offset = host_page_size * queue_idx;
ret = pwrite64(vfio->device_fd, &queue_idx, sizeof(queue_idx),
vfio->notify_offset + offset);
......
}
3. VFIO interrupt ioctl API
VFIO interrupt ioctl API is used to setup device interrupts.
IRQ-bypass can also be supported.
Currently, the data path interrupt can be configured via the
VFIO_VHOST_VQ_IRQ_INDEX with virtqueue's callfd.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
drivers/vhost/Makefile | 2 +
drivers/vhost/vdpa.c | 770 +++++++++++++++++++++++++++++++++++++
include/linux/vdpa_mdev.h | 72 ++++
include/uapi/linux/vfio.h | 19 +
include/uapi/linux/vhost.h | 25 ++
5 files changed, 888 insertions(+)
create mode 100644 drivers/vhost/vdpa.c
create mode 100644 include/linux/vdpa_mdev.h
diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
index 6c6df24f770c..cabb71095940 100644
--- a/drivers/vhost/Makefile
+++ b/drivers/vhost/Makefile
@@ -10,4 +10,6 @@ vhost_vsock-y := vsock.o
obj-$(CONFIG_VHOST_RING) += vringh.o
+obj-$(CONFIG_VHOST_VFIO) += vdpa.o
+
obj-$(CONFIG_VHOST) += vhost.o
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
new file mode 100644
index 000000000000..5c9426e2a091
--- /dev/null
+++ b/drivers/vhost/vdpa.c
@@ -0,0 +1,770 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019 Intel Corporation.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/vfio.h>
+#include <linux/vhost.h>
+#include <linux/mdev.h>
+#include <linux/vdpa_mdev.h>
+#include <asm/uaccess.h>
+
+#define VDPA_CONFIG_SIZE 0x1000000
+
+#define VDPA_VFIO_VHOST_OFFSET_SHIFT 40
+#define VDPA_VFIO_VHOST_OFFSET_MASK \
+ ((1ULL << VDPA_VFIO_VHOST_OFFSET_SHIFT) - 1)
+#define VDPA_VFIO_VHOST_OFFSET_TO_INDEX(offset) \
+ ((offset) >> VDPA_VFIO_VHOST_OFFSET_SHIFT)
+#define VDPA_VFIO_VHOST_INDEX_TO_OFFSET(index) \
+ ((u64)(index) << VDPA_VFIO_VHOST_OFFSET_SHIFT)
+#define VDPA_VFIO_VHOST_REGION_OFFSET(offset) \
+ ((offset) & VDPA_VFIO_VHOST_OFFSET_MASK)
+
+struct vdpa_dev *vdpa_alloc(struct mdev_device *mdev, void *private,
+ int max_vrings)
+{
+ struct vdpa_dev *vdpa;
+ size_t size;
+
+ size = sizeof(struct vdpa_dev) + max_vrings *
+ sizeof(struct vdpa_vring_info);
+
+ vdpa = kzalloc(size, GFP_KERNEL);
+ if (vdpa == NULL)
+ return NULL;
+
+ mutex_init(&vdpa->ops_lock);
+
+ vdpa->mdev = mdev;
+ vdpa->private = private;
+ vdpa->max_vrings = max_vrings;
+
+ return vdpa;
+}
+EXPORT_SYMBOL(vdpa_alloc);
+
+void vdpa_free(struct vdpa_dev *vdpa)
+{
+ struct mdev_device *mdev;
+
+ mdev = vdpa->mdev;
+
+ vdpa->ops->stop(vdpa);
+ mdev_set_drvdata(mdev, NULL);
+ mutex_destroy(&vdpa->ops_lock);
+ kfree(vdpa);
+}
+EXPORT_SYMBOL(vdpa_free);
+
+static ssize_t vdpa_handle_config_read(struct mdev_device *mdev,
+ char __user *buf, size_t count, loff_t *ppos)
+{
+ struct vdpa_dev *vdpa;
+ struct vhost_vfio_op *op = NULL;
+ loff_t pos = *ppos;
+ loff_t offset;
+ int ret;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
+ if (offset != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!vdpa->pending_reply) {
+ ret = 0;
+ goto out;
+ }
+
+ vdpa->pending_reply = false;
+
+ op = kzalloc(VHOST_VFIO_OP_HDR_SIZE + VHOST_VFIO_OP_PAYLOAD_MAX_SIZE,
+ GFP_KERNEL);
+ if (op == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ op->request = vdpa->pending.request;
+
+ switch (op->request) {
+ case VHOST_GET_VRING_BASE:
+ op->payload.state = vdpa->pending.payload.state;
+ op->size = sizeof(op->payload.state);
+ break;
+ case VHOST_GET_FEATURES:
+ op->payload.u64 = vdpa->pending.payload.u64;
+ op->size = sizeof(op->payload.u64);
+ break;
+ default:
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (op->size + VHOST_VFIO_OP_HDR_SIZE != count) {
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (copy_to_user(buf, op, count)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ ret = count;
+
+out_free:
+ kfree(op);
+out:
+ return ret;
+}
+
+ssize_t vdpa_read(struct mdev_device *mdev, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ int done = 0;
+ unsigned int index;
+ loff_t pos = *ppos;
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ mutex_lock(&vdpa->ops_lock);
+
+ index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
+
+ switch (index) {
+ case VFIO_VHOST_CONFIG_REGION_INDEX:
+ done = vdpa_handle_config_read(mdev, buf, count, ppos);
+ break;
+ }
+
+ if (done > 0)
+ *ppos += done;
+
+ mutex_unlock(&vdpa->ops_lock);
+
+ return done;
+}
+EXPORT_SYMBOL(vdpa_read);
+
+static int vhost_set_vring_addr(struct mdev_device *mdev,
+ struct vhost_vring_addr *addr)
+{
+ struct vdpa_dev *vdpa;
+ int qid = addr->index;
+ struct vdpa_vring_info *vring;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (qid >= vdpa->max_vrings)
+ return -EINVAL;
+
+ if (qid >= vdpa->nr_vring)
+ vdpa->nr_vring = qid + 1;
+
+ vring = &vdpa->vring_info[qid];
+
+ vring->desc_user_addr = addr->desc_user_addr;
+ vring->used_user_addr = addr->used_user_addr;
+ vring->avail_user_addr = addr->avail_user_addr;
+ vring->log_guest_addr = addr->log_guest_addr;
+
+ return 0;
+}
+
+static int vhost_set_vring_num(struct mdev_device *mdev,
+ struct vhost_vring_state *num)
+{
+ struct vdpa_dev *vdpa;
+ int qid = num->index;
+ struct vdpa_vring_info *vring;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (qid >= vdpa->max_vrings)
+ return -EINVAL;
+
+ vring = &vdpa->vring_info[qid];
+
+ vring->size = num->num;
+
+ return 0;
+}
+
+static int vhost_set_vring_base(struct mdev_device *mdev,
+ struct vhost_vring_state *base)
+{
+ struct vdpa_dev *vdpa;
+ int qid = base->index;
+ struct vdpa_vring_info *vring;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (qid >= vdpa->max_vrings)
+ return -EINVAL;
+
+ vring = &vdpa->vring_info[qid];
+
+ vring->base = base->num;
+
+ return 0;
+}
+
+static int vhost_get_vring_base(struct mdev_device *mdev,
+ struct vhost_vring_state *base)
+{
+ struct vdpa_dev *vdpa;
+ int qid = base->index;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ vdpa->pending_reply = true;
+ vdpa->pending.request = VHOST_GET_VRING_BASE;
+ vdpa->pending.payload.state.index = qid;
+ vdpa->pending.payload.state.num = vdpa->vring_info[qid].base;
+
+ return 0;
+}
+
+static int vhost_set_log_base(struct mdev_device *mdev, u64 *log_base)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ vdpa->log_base = *log_base;
+ return 0;
+}
+
+static int vhost_set_features(struct mdev_device *mdev, u64 *features)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ vdpa->features = *features;
+ vdpa->ops->set_features(vdpa);
+
+ return 0;
+}
+
+static int vhost_get_features(struct mdev_device *mdev, u64 *features)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ vdpa->pending_reply = true;
+ vdpa->pending.request = VHOST_GET_FEATURES;
+ vdpa->pending.payload.u64 =
+ vdpa->ops->supported_features(vdpa);
+
+ return 0;
+}
+
+static int vhost_set_owner(struct mdev_device *mdev)
+{
+ // TODO
+ return 0;
+}
+
+static int vhost_reset_owner(struct mdev_device *mdev)
+{
+ // TODO
+ return 0;
+}
+
+static int vhost_set_state(struct mdev_device *mdev, u64 *state)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (*state >= VHOST_DEVICE_S_MAX)
+ return -EINVAL;
+
+ if (vdpa->state == *state)
+ return 0;
+
+ vdpa->state = *state;
+
+ switch (vdpa->state) {
+ case VHOST_DEVICE_S_RUNNING:
+ vdpa->ops->start(vdpa);
+ break;
+ case VHOST_DEVICE_S_STOPPED:
+ vdpa->ops->stop(vdpa);
+ break;
+ }
+
+ return 0;
+}
+
+static ssize_t vdpa_handle_config_write(struct mdev_device *mdev,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct vhost_vfio_op *op = NULL;
+ loff_t pos = *ppos;
+ loff_t offset;
+ int ret;
+
+ offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
+ if (offset != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (count < VHOST_VFIO_OP_HDR_SIZE) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ op = kzalloc(VHOST_VFIO_OP_HDR_SIZE + VHOST_VFIO_OP_PAYLOAD_MAX_SIZE,
+ GFP_KERNEL);
+ if (op == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (copy_from_user(op, buf, VHOST_VFIO_OP_HDR_SIZE)) {
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (op->size > VHOST_VFIO_OP_PAYLOAD_MAX_SIZE ||
+ op->size + VHOST_VFIO_OP_HDR_SIZE != count) {
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (copy_from_user(&op->payload, buf + VHOST_VFIO_OP_HDR_SIZE,
+ op->size)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ switch (op->request) {
+ case VHOST_SET_LOG_BASE:
+ vhost_set_log_base(mdev, &op->payload.u64);
+ break;
+ case VHOST_SET_VRING_ADDR:
+ vhost_set_vring_addr(mdev, &op->payload.addr);
+ break;
+ case VHOST_SET_VRING_NUM:
+ vhost_set_vring_num(mdev, &op->payload.state);
+ break;
+ case VHOST_SET_VRING_BASE:
+ vhost_set_vring_base(mdev, &op->payload.state);
+ break;
+ case VHOST_GET_VRING_BASE:
+ vhost_get_vring_base(mdev, &op->payload.state);
+ break;
+ case VHOST_SET_FEATURES:
+ vhost_set_features(mdev, &op->payload.u64);
+ break;
+ case VHOST_GET_FEATURES:
+ vhost_get_features(mdev, &op->payload.u64);
+ break;
+ case VHOST_SET_OWNER:
+ vhost_set_owner(mdev);
+ break;
+ case VHOST_RESET_OWNER:
+ vhost_reset_owner(mdev);
+ break;
+ case VHOST_DEVICE_SET_STATE:
+ vhost_set_state(mdev, &op->payload.u64);
+ break;
+ default:
+ break;
+ }
+
+ ret = count;
+
+out_free:
+ kfree(op);
+out:
+ return ret;
+}
+
+static ssize_t vdpa_handle_notify_write(struct mdev_device *mdev,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct vdpa_dev *vdpa;
+ int qid;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (count < sizeof(qid))
+ return -EINVAL;
+
+ if (copy_from_user(&qid, buf, sizeof(qid)))
+ return -EINVAL;
+
+ vdpa->ops->notify(vdpa, qid);
+
+ return count;
+}
+
+ssize_t vdpa_write(struct mdev_device *mdev, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ int done = 0;
+ unsigned int index;
+ loff_t pos = *ppos;
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ mutex_lock(&vdpa->ops_lock);
+
+ index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
+
+ switch (index) {
+ case VFIO_VHOST_CONFIG_REGION_INDEX:
+ done = vdpa_handle_config_write(mdev, buf, count, ppos);
+ break;
+ case VFIO_VHOST_NOTIFY_REGION_INDEX:
+ done = vdpa_handle_notify_write(mdev, buf, count, ppos);
+ break;
+ }
+
+ if (done > 0)
+ *ppos += done;
+
+ mutex_unlock(&vdpa->ops_lock);
+
+ return done;
+}
+EXPORT_SYMBOL(vdpa_write);
+
+static int vdpa_get_region_info(struct mdev_device *mdev,
+ struct vfio_region_info *region_info,
+ u16 *cap_type_id, void **cap_type)
+{
+ struct vdpa_dev *vdpa;
+ u32 index, flags;
+ u64 size = 0;
+
+ if (!mdev)
+ return -EINVAL;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -EINVAL;
+
+ index = region_info->index;
+ if (index >= VFIO_VHOST_NUM_REGIONS)
+ return -EINVAL;
+
+ mutex_lock(&vdpa->ops_lock);
+
+ flags = VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_WRITE;
+
+ switch (index) {
+ case VFIO_VHOST_CONFIG_REGION_INDEX:
+ size = VDPA_CONFIG_SIZE;
+ break;
+ case VFIO_VHOST_NOTIFY_REGION_INDEX:
+ size = (u64)vdpa->max_vrings << PAGE_SHIFT;
+ flags |= VFIO_REGION_INFO_FLAG_MMAP;
+ break;
+ default:
+ size = 0;
+ break;
+ }
+
+ region_info->size = size;
+ region_info->offset = VDPA_VFIO_VHOST_INDEX_TO_OFFSET(index);
+ region_info->flags = flags;
+ mutex_unlock(&vdpa->ops_lock);
+ return 0;
+}
+
+static int vdpa_reset(struct mdev_device *mdev)
+{
+ struct vdpa_dev *vdpa;
+
+ if (!mdev)
+ return -EINVAL;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int vdpa_get_device_info(struct mdev_device *mdev,
+ struct vfio_device_info *dev_info)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ dev_info->flags = VFIO_DEVICE_FLAGS_VHOST | VFIO_DEVICE_RESET;
+ dev_info->num_regions = VFIO_VHOST_NUM_REGIONS;
+ dev_info->num_irqs = VFIO_VHOST_NUM_IRQS;
+
+ return 0;
+}
+
+static int vdpa_get_irq_info(struct mdev_device *mdev,
+ struct vfio_irq_info *info)
+{
+ struct vdpa_dev *vdpa;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ if (info->index != VFIO_VHOST_VQ_IRQ_INDEX)
+ return -EINVAL;
+
+ info->flags = VFIO_IRQ_INFO_EVENTFD;
+ info->count = vdpa->max_vrings;
+
+ return 0;
+}
+
+static int vdpa_set_irqs(struct mdev_device *mdev, uint32_t flags,
+ unsigned int index, unsigned int start,
+ unsigned int count, void *data)
+{
+ struct vdpa_dev *vdpa;
+ int *fd = data, i;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -EINVAL;
+
+ if (index != VFIO_VHOST_VQ_IRQ_INDEX)
+ return -ENOTSUPP;
+
+ for (i = 0; i < count; i++)
+ vdpa->ops->set_eventfd(vdpa, start + i,
+ (flags & VFIO_IRQ_SET_DATA_EVENTFD) ? fd[i] : -1);
+
+ return 0;
+}
+
+long vdpa_ioctl(struct mdev_device *mdev, unsigned int cmd, unsigned long arg)
+{
+ int ret = 0;
+ unsigned long minsz;
+ struct vdpa_dev *vdpa;
+
+ if (!mdev)
+ return -EINVAL;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ switch (cmd) {
+ case VFIO_DEVICE_GET_INFO:
+ {
+ struct vfio_device_info info;
+
+ minsz = offsetofend(struct vfio_device_info, num_irqs);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ ret = vdpa_get_device_info(mdev, &info);
+ if (ret)
+ return ret;
+
+ if (copy_to_user((void __user *)arg, &info, minsz))
+ return -EFAULT;
+
+ return 0;
+ }
+ case VFIO_DEVICE_GET_REGION_INFO:
+ {
+ struct vfio_region_info info;
+ u16 cap_type_id = 0;
+ void *cap_type = NULL;
+
+ minsz = offsetofend(struct vfio_region_info, offset);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ ret = vdpa_get_region_info(mdev, &info, &cap_type_id,
+ &cap_type);
+ if (ret)
+ return ret;
+
+ if (copy_to_user((void __user *)arg, &info, minsz))
+ return -EFAULT;
+
+ return 0;
+ }
+ case VFIO_DEVICE_GET_IRQ_INFO:
+ {
+ struct vfio_irq_info info;
+
+ minsz = offsetofend(struct vfio_irq_info, count);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz || info.index >= vdpa->max_vrings)
+ return -EINVAL;
+
+ ret = vdpa_get_irq_info(mdev, &info);
+ if (ret)
+ return ret;
+
+ if (copy_to_user((void __user *)arg, &info, minsz))
+ return -EFAULT;
+
+ return 0;
+ }
+ case VFIO_DEVICE_SET_IRQS:
+ {
+ struct vfio_irq_set hdr;
+ size_t data_size = 0;
+ u8 *data = NULL;
+
+ minsz = offsetofend(struct vfio_irq_set, count);
+
+ if (copy_from_user(&hdr, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ ret = vfio_set_irqs_validate_and_prepare(&hdr, vdpa->max_vrings,
+ VFIO_VHOST_NUM_IRQS,
+ &data_size);
+ if (ret)
+ return ret;
+
+ if (data_size) {
+ data = memdup_user((void __user *)(arg + minsz),
+ data_size);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+ }
+
+ ret = vdpa_set_irqs(mdev, hdr.flags, hdr.index, hdr.start,
+ hdr.count, data);
+
+ kfree(data);
+ return ret;
+ }
+ case VFIO_DEVICE_RESET:
+ return vdpa_reset(mdev);
+ }
+ return -ENOTTY;
+}
+EXPORT_SYMBOL(vdpa_ioctl);
+
+static const struct vm_operations_struct vdpa_mm_ops = {
+#ifdef CONFIG_HAVE_IOREMAP_PROT
+ .access = generic_access_phys
+#endif
+};
+
+int vdpa_mmap(struct mdev_device *mdev, struct vm_area_struct *vma)
+{
+ struct vdpa_dev *vdpa;
+ unsigned int index;
+ loff_t pos;
+ loff_t offset;
+ int qid, ret;
+
+ vdpa = mdev_get_drvdata(mdev);
+ if (!vdpa)
+ return -ENODEV;
+
+ pos = vma->vm_pgoff << PAGE_SHIFT;
+
+ index = VDPA_VFIO_VHOST_OFFSET_TO_INDEX(pos);
+ offset = VDPA_VFIO_VHOST_REGION_OFFSET(pos);
+
+ qid = offset >> PAGE_SHIFT;
+
+ if (vma->vm_end < vma->vm_start)
+ return -EINVAL;
+ if ((vma->vm_flags & VM_SHARED) == 0)
+ return -EINVAL;
+ if (index != VFIO_VHOST_NOTIFY_REGION_INDEX)
+ return -EINVAL;
+ if (qid < 0 || qid >= vdpa->max_vrings)
+ return -EINVAL;
+
+ if (vma->vm_end - vma->vm_start > PAGE_SIZE)
+ return -EINVAL;
+
+ if (vdpa->ops->get_notify_addr == NULL)
+ return -ENOTSUPP;
+
+ mutex_lock(&vdpa->ops_lock);
+
+ vma->vm_ops = &vdpa_mm_ops;
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+
+ vma->vm_pgoff = vdpa->ops->get_notify_addr(vdpa, qid) >> PAGE_SHIFT;
+
+ ret = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+ vma->vm_end - vma->vm_start, vma->vm_page_prot);
+
+ mutex_unlock(&vdpa->ops_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(vdpa_mmap);
+
+int vdpa_open(struct mdev_device *mdev)
+{
+ return 0;
+}
+EXPORT_SYMBOL(vdpa_open);
+
+void vdpa_close(struct mdev_device *mdev)
+{
+}
+EXPORT_SYMBOL(vdpa_close);
+
+MODULE_VERSION("0.0.0");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Hardware vhost accelerator abstraction");
diff --git a/include/linux/vdpa_mdev.h b/include/linux/vdpa_mdev.h
new file mode 100644
index 000000000000..4bbdf7e2e712
--- /dev/null
+++ b/include/linux/vdpa_mdev.h
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019 Intel Corporation.
+ */
+
+#ifndef VDPA_MDEV_H
+#define VDPA_MDEV_H
+
+struct mdev_device;
+struct vdpa_dev;
+
+/*
+ * XXX: Any comments about the vDPA API design for drivers
+ * would be appreciated!
+ */
+
+typedef int (*vdpa_start_device_t)(struct vdpa_dev *vdpa);
+typedef int (*vdpa_stop_device_t)(struct vdpa_dev *vdpa);
+typedef int (*vdpa_set_features_t)(struct vdpa_dev *vdpa);
+typedef int (*vdpa_set_eventfd_t)(struct vdpa_dev *vdpa, int queue_idx, int fd);
+typedef u64 (*vdpa_supported_features_t)(struct vdpa_dev *vdpa);
+typedef void (*vdpa_notify_device_t)(struct vdpa_dev *vdpa, int queue_idx);
+typedef u64 (*vdpa_get_notify_addr_t)(struct vdpa_dev *vdpa, int queue_idx);
+
+struct vdpa_device_ops {
+ vdpa_start_device_t start;
+ vdpa_stop_device_t stop;
+ vdpa_set_eventfd_t set_eventfd;
+ vdpa_supported_features_t supported_features;
+ vdpa_notify_device_t notify;
+ vdpa_get_notify_addr_t get_notify_addr;
+ vdpa_set_features_t set_features;
+};
+
+struct vdpa_vring_info {
+ u64 desc_user_addr;
+ u64 used_user_addr;
+ u64 avail_user_addr;
+ u64 log_guest_addr;
+ u16 size;
+ u16 base;
+};
+
+struct vdpa_dev {
+ struct mdev_device *mdev;
+ struct mutex ops_lock;
+ int nr_vring;
+ u64 features;
+ u64 state;
+ bool pending_reply;
+ struct vhost_vfio_op pending;
+ const struct vdpa_device_ops *ops;
+ void *private;
+ int max_vrings;
+ uint64_t log_base;
+ uint64_t log_size;
+ struct vdpa_vring_info vring_info[0];
+};
+
+struct vdpa_dev *vdpa_alloc(struct mdev_device *mdev, void *private,
+ int max_vrings);
+void vdpa_free(struct vdpa_dev *vdpa);
+ssize_t vdpa_read(struct mdev_device *mdev, char __user *buf,
+ size_t count, loff_t *ppos);
+ssize_t vdpa_write(struct mdev_device *mdev, const char __user *buf,
+ size_t count, loff_t *ppos);
+long vdpa_ioctl(struct mdev_device *mdev, unsigned int cmd, unsigned long arg);
+int vdpa_mmap(struct mdev_device *mdev, struct vm_area_struct *vma);
+int vdpa_open(struct mdev_device *mdev);
+void vdpa_close(struct mdev_device *mdev);
+
+#endif /* VDPA_MDEV_H */
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 8f10748dac79..6c5718ab7eeb 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -201,6 +201,7 @@ struct vfio_device_info {
#define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */
#define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */
#define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */
+#define VFIO_DEVICE_FLAGS_VHOST (1 << 6) /* vfio-vhost device */
__u32 num_regions; /* Max region index + 1 */
__u32 num_irqs; /* Max IRQ index + 1 */
};
@@ -217,6 +218,7 @@ struct vfio_device_info {
#define VFIO_DEVICE_API_AMBA_STRING "vfio-amba"
#define VFIO_DEVICE_API_CCW_STRING "vfio-ccw"
#define VFIO_DEVICE_API_AP_STRING "vfio-ap"
+#define VFIO_DEVICE_API_VHOST_STRING "vfio-vhost"
/**
* VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
@@ -573,6 +575,23 @@ enum {
VFIO_CCW_NUM_IRQS
};
+/*
+ * The vfio-vhost bus driver makes use of the following fixed region and
+ * IRQ index mapping. Unimplemented regions return a size of zero.
+ * Unimplemented IRQ types return a count of zero.
+ */
+
+enum {
+ VFIO_VHOST_CONFIG_REGION_INDEX,
+ VFIO_VHOST_NOTIFY_REGION_INDEX,
+ VFIO_VHOST_NUM_REGIONS
+};
+
+enum {
+ VFIO_VHOST_VQ_IRQ_INDEX,
+ VFIO_VHOST_NUM_IRQS
+};
+
/**
* VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IORW(VFIO_TYPE, VFIO_BASE + 12,
* struct vfio_pci_hot_reset_info)
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 40d028eed645..ad95b90c5c05 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -116,4 +116,29 @@
#define VHOST_VSOCK_SET_GUEST_CID _IOW(VHOST_VIRTIO, 0x60, __u64)
#define VHOST_VSOCK_SET_RUNNING _IOW(VHOST_VIRTIO, 0x61, int)
+/* VHOST_DEVICE specific defines */
+
+#define VHOST_DEVICE_SET_STATE _IOW(VHOST_VIRTIO, 0x70, __u64)
+
+#define VHOST_DEVICE_S_STOPPED 0
+#define VHOST_DEVICE_S_RUNNING 1
+#define VHOST_DEVICE_S_MAX 2
+
+struct vhost_vfio_op {
+ __u64 request;
+ __u32 flags;
+ /* Flag values: */
+#define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
+ __u32 size;
+ union {
+ __u64 u64;
+ struct vhost_vring_state state;
+ struct vhost_vring_addr addr;
+ } payload;
+};
+
+#define VHOST_VFIO_OP_HDR_SIZE \
+ ((unsigned long)&((struct vhost_vfio_op *)NULL)->payload)
+#define VHOST_VFIO_OP_PAYLOAD_MAX_SIZE 1024 /* FIXME TBD */
+
#endif
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 0/3] vsock/virtio: several fixes in the .probe() and .remove()
From: Stefan Hajnoczi @ 2019-07-03 9:14 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190701170357.jtuhy3ank7mv6izb@steredhat>
[-- Attachment #1: Type: text/plain, Size: 2545 bytes --]
On Mon, Jul 01, 2019 at 07:03:57PM +0200, Stefano Garzarella wrote:
> On Mon, Jul 01, 2019 at 04:11:13PM +0100, Stefan Hajnoczi wrote:
> > On Fri, Jun 28, 2019 at 02:36:56PM +0200, Stefano Garzarella wrote:
> > > 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.
> > >
> > > v2:
> > > - Patch 1: use RCU to protect 'the_virtio_vsock' pointer
> > > - Patch 2: no changes
> > > - Patch 3: flush works only at the end of .remove()
> > > - Removed patch 4 because virtqueue_detach_unused_buf() returns all the buffers
> > > allocated.
> > >
> > > v1: https://patchwork.kernel.org/cover/10964733/
> >
> > This looks good to me.
>
> Thanks for the review!
>
> >
> > Did you run any stress tests? For example an SMP guest constantly
> > connecting and sending packets together with a script that
> > hotplug/unplugs vhost-vsock-pci from the host side.
>
> Yes, I started an SMP guest (-smp 4 -monitor tcp:127.0.0.1:1234,server,nowait)
> and I run 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
>
> Do you think is enough or is better to have a test more accurate?
That's good when left running overnight so that thousands of hotplug
events are tested.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v2 4/4] net: dsa: vsc73xx: Assert reset if iCPU is enabled
From: Pawel Dembicki @ 2019-07-03 9:10 UTC (permalink / raw)
Cc: paweldembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-4-paweldembicki@gmail.com>
Driver allow to use devices with disabled iCPU only.
Some devices have pre-initialised iCPU by bootloader.
That state make switch unmanaged. This patch force reset
if device is in unmanaged state. In the result chip lost
internal firmware from RAM and it can be managed.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
Changes in v2:
- rebase commit after changes 1-3/4 patches
drivers/net/dsa/vitesse-vsc73xx-core.c | 36 ++++++++++++--------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 10063f31d9a3..4525702faf68 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -417,22 +417,8 @@ static int vsc73xx_detect(struct vsc73xx *vsc)
}
if (val == 0xffffffff) {
- dev_info(vsc->dev, "chip seems dead, assert reset\n");
- gpiod_set_value_cansleep(vsc->reset, 1);
- /* Reset pulse should be 20ns minimum, according to datasheet
- * table 245, so 10us should be fine
- */
- usleep_range(10, 100);
- gpiod_set_value_cansleep(vsc->reset, 0);
- /* Wait 20ms according to datasheet table 245 */
- msleep(20);
-
- ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
- VSC73XX_ICPU_MBOX_VAL, &val);
- if (val == 0xffffffff) {
- dev_err(vsc->dev, "seems not to help, giving up\n");
- return -ENODEV;
- }
+ dev_info(vsc->dev, "chip seems dead.\n");
+ return -EAGAIN;
}
ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
@@ -483,9 +469,8 @@ static int vsc73xx_detect(struct vsc73xx *vsc)
}
if (icpu_si_boot_en && !icpu_pi_en) {
dev_err(vsc->dev,
- "iCPU enabled boots from SI, no external memory\n");
- dev_err(vsc->dev, "no idea how to deal with this\n");
- return -ENODEV;
+ "iCPU enabled boots from PI/SI, no external memory\n");
+ return -EAGAIN;
}
if (!icpu_si_boot_en && icpu_pi_en) {
dev_err(vsc->dev,
@@ -1158,6 +1143,19 @@ int vsc73xx_probe(struct vsc73xx *vsc)
msleep(20);
ret = vsc73xx_detect(vsc);
+ if (ret == -EAGAIN) {
+ dev_err(vsc->dev,
+ "Chip seams to be out of control. Assert reset and try again.\n");
+ gpiod_set_value_cansleep(vsc->reset, 1);
+ /* Reset pulse should be 20ns minimum, according to datasheet
+ * table 245, so 10us should be fine
+ */
+ usleep_range(10, 100);
+ gpiod_set_value_cansleep(vsc->reset, 0);
+ /* Wait 20ms according to datasheet table 245 */
+ msleep(20);
+ ret = vsc73xx_detect(vsc);
+ }
if (ret) {
dev_err(dev, "no chip found (%d)\n", ret);
return -ENODEV;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next v2] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-07-03 9:10 UTC (permalink / raw)
To: Magnus Karlsson
Cc: Network Development, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko
In-Reply-To: <CAJ8uoz3BoLiM04WW=91wYryrVBqj5GDsL5mvDaAyBAv-6MNbsQ@mail.gmail.com>
On 28 Jun 2019, at 12:14, Magnus Karlsson wrote:
> On Wed, Jun 26, 2019 at 10:33 AM Eelco Chaudron <echaudro@redhat.com>
> 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>
>> ---
>>
>> 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 | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>> index 82ea71a0f3ec..6acb81102346 100644
>> --- a/tools/lib/bpf/xsk.h
>> +++ b/tools/lib/bpf/xsk.h
>> @@ -76,11 +76,11 @@ 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;
>>
>> - if (free_entries >= nb)
>> + if (free_entries >= nb && nb != 0)
>> return free_entries;
>
> Thanks Eelco for the patch. Is the test nb != 0 introduced here so
> that the function will continue with the refresh from the global state
> when nb is set to 0? If so, could a user not instead just set the nb
> parameter to the size of the ring? This would always trigger a
> refresh, except when the number of free entries is equal to the size
> of the ring, but then we do not need the refresh anyway. This would
> eliminate the nb != 0 test that you introduced from the fast path.
Will remove this change from the fast path, and your suggestion can be
used if circumvention of the cache is needed. Will sent out a v3 soon...
>> /* Refresh the local tail pointer.
>> @@ -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;
>> --
>> 2.20.1
>>
^ permalink raw reply
* [PATCH v2 3/4] net: dsa: vsc73xx: add support for parallel mode
From: Pawel Dembicki @ 2019-07-03 9:07 UTC (permalink / raw)
Cc: paweldembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-3-paweldembicki@gmail.com>
This patch add platform part of vsc73xx driver.
It allows to use chip connected by parallel interface.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
Changes in v2:
- drop mutex
- fix indentation
drivers/net/dsa/Kconfig | 8 ++
drivers/net/dsa/Makefile | 1 +
drivers/net/dsa/vitesse-vsc73xx-platform.c | 160 +++++++++++++++++++++
3 files changed, 169 insertions(+)
create mode 100644 drivers/net/dsa/vitesse-vsc73xx-platform.c
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 4ab2aa09e2e4..80965808949d 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -116,4 +116,12 @@ config NET_DSA_VITESSE_VSC73XX_SPI
---help---
This enables support for the Vitesse VSC7385, VSC7388, VSC7395
and VSC7398 SparX integrated ethernet switches in SPI managed mode.
+
+config NET_DSA_VITESSE_VSC73XX_PLATFORM
+ tristate "Vitesse VSC7385/7388/7395/7398 Platform mode support"
+ depends on HAS_IOMEM
+ select NET_DSA_VITESSE_VSC73XX
+ ---help---
+ This enables support for the Vitesse VSC7385, VSC7388, VSC7395
+ and VSC7398 SparX integrated ethernet switches in Platform managed mode.
endmenu
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 117bf78be211..d5e4c668ac03 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_I2C) += lan9303_i2c.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_MDIO) += lan9303_mdio.o
obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx-core.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_PLATFORM) += vitesse-vsc73xx-platform.o
obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_SPI) += vitesse-vsc73xx-spi.o
obj-y += b53/
obj-y += microchip/
diff --git a/drivers/net/dsa/vitesse-vsc73xx-platform.c b/drivers/net/dsa/vitesse-vsc73xx-platform.c
new file mode 100644
index 000000000000..149d921b8499
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx-platform.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0
+/* DSA driver for:
+ * Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
+ *
+ * This driver takes control of the switch chip over Platform and
+ * configures it to route packages around when connected to a CPU port.
+ *
+ * Copyright (C) 2019 pawel Dembicki <paweldembicki@gmail.com>
+ * Based on vitesse-vsc-spi.c by:
+ * Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
+ * Includes portions of code from the firmware uploader by:
+ * Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "vitesse-vsc73xx.h"
+
+#define VSC73XX_CMD_PLATFORM_BLOCK_SHIFT 14
+#define VSC73XX_CMD_PLATFORM_BLOCK_MASK 0x7
+#define VSC73XX_CMD_PLATFORM_SUBBLOCK_SHIFT 10
+#define VSC73XX_CMD_PLATFORM_SUBBLOCK_MASK 0xf
+#define VSC73XX_CMD_PLATFORM_REGISTER_SHIFT 2
+
+/**
+ * struct vsc73xx_platform - VSC73xx Platform state container
+ */
+struct vsc73xx_platform {
+ struct platform_device *pdev;
+ void __iomem *base_addr;
+ struct vsc73xx vsc;
+};
+
+static const struct vsc73xx_ops vsc73xx_platform_ops;
+
+static u32 vsc73xx_make_addr(u8 block, u8 subblock, u8 reg)
+{
+ u32 ret;
+
+ ret = (block & VSC73XX_CMD_PLATFORM_BLOCK_MASK)
+ << VSC73XX_CMD_PLATFORM_BLOCK_SHIFT;
+ ret |= (subblock & VSC73XX_CMD_PLATFORM_SUBBLOCK_MASK)
+ << VSC73XX_CMD_PLATFORM_SUBBLOCK_SHIFT;
+ ret |= reg << VSC73XX_CMD_PLATFORM_REGISTER_SHIFT;
+
+ return ret;
+}
+
+static int vsc73xx_platform_read(struct vsc73xx *vsc, u8 block, u8 subblock,
+ u8 reg, u32 *val)
+{
+ struct vsc73xx_platform *vsc_platform = vsc->priv;
+ u32 offset;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ offset = vsc73xx_make_addr(block, subblock, reg);
+ *val = ioread32be(vsc_platform->base_addr + offset);
+
+ return 0;
+}
+
+static int vsc73xx_platform_write(struct vsc73xx *vsc, u8 block, u8 subblock,
+ u8 reg, u32 val)
+{
+ struct vsc73xx_platform *vsc_platform = vsc->priv;
+ u32 offset;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ offset = vsc73xx_make_addr(block, subblock, reg);
+ iowrite32be(val, vsc_platform->base_addr + offset);
+
+ return 0;
+}
+
+static int vsc73xx_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct vsc73xx_platform *vsc_platform;
+ struct resource *res = NULL;
+ int ret;
+
+ vsc_platform = devm_kzalloc(dev, sizeof(*vsc_platform), GFP_KERNEL);
+ if (!vsc_platform)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, vsc_platform);
+ vsc_platform->pdev = pdev;
+ vsc_platform->vsc.dev = dev;
+ vsc_platform->vsc.priv = vsc_platform;
+ vsc_platform->vsc.ops = &vsc73xx_platform_ops;
+
+ /* obtain I/O memory space */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "cannot obtain I/O memory space\n");
+ ret = -ENXIO;
+ return ret;
+ }
+
+ vsc_platform->base_addr = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(vsc_platform->base_addr)) {
+ dev_err(&pdev->dev, "cannot request I/O memory space\n");
+ ret = -ENXIO;
+ return ret;
+ }
+
+ return vsc73xx_probe(&vsc_platform->vsc);
+}
+
+static int vsc73xx_platform_remove(struct platform_device *pdev)
+{
+ struct vsc73xx_platform *vsc_platform = platform_get_drvdata(pdev);
+
+ return vsc73xx_remove(&vsc_platform->vsc);
+}
+
+static const struct vsc73xx_ops vsc73xx_platform_ops = {
+ .read = vsc73xx_platform_read,
+ .write = vsc73xx_platform_write,
+};
+
+static const struct of_device_id vsc73xx_of_match[] = {
+ {
+ .compatible = "vitesse,vsc7385",
+ },
+ {
+ .compatible = "vitesse,vsc7388",
+ },
+ {
+ .compatible = "vitesse,vsc7395",
+ },
+ {
+ .compatible = "vitesse,vsc7398",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
+
+static struct platform_driver vsc73xx_platform_driver = {
+ .probe = vsc73xx_platform_probe,
+ .remove = vsc73xx_platform_remove,
+ .driver = {
+ .name = "vsc73xx-platform",
+ .of_match_table = vsc73xx_of_match,
+ },
+};
+module_platform_driver(vsc73xx_platform_driver);
+
+MODULE_AUTHOR("Pawel Dembicki <paweldembicki@gmail.com>");
+MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 Platform driver");
+MODULE_LICENSE("GPL v2");
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/4] net: dsa: vsc73xx: Split vsc73xx driver
From: Pawel Dembicki @ 2019-07-03 9:02 UTC (permalink / raw)
Cc: paweldembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-2-paweldembicki@gmail.com>
This driver (currently) only takes control of the switch chip over
SPI and configures it to route packages around when connected to a
CPU port. But Vitesse chip support also parallel interface.
This patch split driver into two parts: core and spi. It is required
for add support to another managing interface.
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
Changes in v2:
- Make patch less invasive
- Move mutex to SPI part of driver
- v2 also tested by Linus
drivers/net/dsa/Kconfig | 11 +-
drivers/net/dsa/Makefile | 3 +-
...tesse-vsc73xx.c => vitesse-vsc73xx-core.c} | 170 +--------------
drivers/net/dsa/vitesse-vsc73xx-spi.c | 203 ++++++++++++++++++
drivers/net/dsa/vitesse-vsc73xx.h | 29 +++
5 files changed, 254 insertions(+), 162 deletions(-)
rename drivers/net/dsa/{vitesse-vsc73xx.c => vitesse-vsc73xx-core.c} (91%)
create mode 100644 drivers/net/dsa/vitesse-vsc73xx-spi.c
create mode 100644 drivers/net/dsa/vitesse-vsc73xx.h
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index b91e78e3598f..4ab2aa09e2e4 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -99,8 +99,8 @@ config NET_DSA_SMSC_LAN9303_MDIO
for MDIO managed mode.
config NET_DSA_VITESSE_VSC73XX
- tristate "Vitesse VSC7385/7388/7395/7398 support"
- depends on OF && SPI
+ tristate
+ depends on OF
depends on NET_DSA
select FIXED_PHY
select VITESSE_PHY
@@ -109,4 +109,11 @@ config NET_DSA_VITESSE_VSC73XX
This enables support for the Vitesse VSC7385, VSC7388,
VSC7395 and VSC7398 SparX integrated ethernet switches.
+config NET_DSA_VITESSE_VSC73XX_SPI
+ tristate "Vitesse VSC7385/7388/7395/7398 SPI mode support"
+ depends on SPI
+ select NET_DSA_VITESSE_VSC73XX
+ ---help---
+ This enables support for the Vitesse VSC7385, VSC7388, VSC7395
+ and VSC7398 SparX integrated ethernet switches in SPI managed mode.
endmenu
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index fefb6aaa82ba..117bf78be211 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -14,7 +14,8 @@ realtek-objs := realtek-smi.o rtl8366.o rtl8366rb.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_I2C) += lan9303_i2c.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_MDIO) += lan9303_mdio.o
-obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx-core.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_SPI) += vitesse-vsc73xx-spi.o
obj-y += b53/
obj-y += microchip/
obj-y += mv88e6xxx/
diff --git a/drivers/net/dsa/vitesse-vsc73xx.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
similarity index 91%
rename from drivers/net/dsa/vitesse-vsc73xx.c
rename to drivers/net/dsa/vitesse-vsc73xx-core.c
index d4780610ea8a..10063f31d9a3 100644
--- a/drivers/net/dsa/vitesse-vsc73xx.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -10,10 +10,6 @@
* handling the switch in a memory-mapped manner by connecting to that external
* CPU's memory bus.
*
- * This driver (currently) only takes control of the switch chip over SPI and
- * configures it to route packages around when connected to a CPU port. The
- * chip has embedded PHYs and VLAN support so we model it using DSA.
- *
* Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
* Includes portions of code from the firmware uploader by:
* Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
@@ -24,8 +20,6 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_mdio.h>
-#include <linux/platform_device.h>
-#include <linux/spi/spi.h>
#include <linux/bitops.h>
#include <linux/if_bridge.h>
#include <linux/etherdevice.h>
@@ -34,6 +28,8 @@
#include <linux/random.h>
#include <net/dsa.h>
+#include "vitesse-vsc73xx.h"
+
#define VSC73XX_BLOCK_MAC 0x1 /* Subblocks 0-4, 6 (CPU port) */
#define VSC73XX_BLOCK_ANALYZER 0x2 /* Only subblock 0 */
#define VSC73XX_BLOCK_MII 0x3 /* Subblocks 0 and 1 */
@@ -255,13 +251,6 @@
#define VSC73XX_GLORESET_PHY_RESET BIT(1)
#define VSC73XX_GLORESET_MASTER_RESET BIT(0)
-#define VSC73XX_CMD_MODE_READ 0
-#define VSC73XX_CMD_MODE_WRITE 1
-#define VSC73XX_CMD_MODE_SHIFT 4
-#define VSC73XX_CMD_BLOCK_SHIFT 5
-#define VSC73XX_CMD_BLOCK_MASK 0x7
-#define VSC73XX_CMD_SUBBLOCK_MASK 0xf
-
#define VSC7385_CLOCK_DELAY ((3 << 4) | 3)
#define VSC7385_CLOCK_DELAY_MASK ((3 << 4) | 3)
@@ -274,20 +263,6 @@
VSC73XX_ICPU_CTRL_CLK_EN | \
VSC73XX_ICPU_CTRL_SRST)
-/**
- * struct vsc73xx - VSC73xx state container
- */
-struct vsc73xx {
- struct device *dev;
- struct gpio_desc *reset;
- struct spi_device *spi;
- struct dsa_switch *ds;
- struct gpio_chip gc;
- u16 chipid;
- u8 addr[ETH_ALEN];
- struct mutex lock; /* Protects SPI traffic */
-};
-
#define IS_7385(a) ((a)->chipid == VSC73XX_CHIPID_ID_7385)
#define IS_7388(a) ((a)->chipid == VSC73XX_CHIPID_ID_7388)
#define IS_7395(a) ((a)->chipid == VSC73XX_CHIPID_ID_7395)
@@ -365,7 +340,7 @@ static const struct vsc73xx_counter vsc73xx_tx_counters[] = {
{ 29, "TxQoSClass3" }, /* non-standard counter */
};
-static int vsc73xx_is_addr_valid(u8 block, u8 subblock)
+int vsc73xx_is_addr_valid(u8 block, u8 subblock)
{
switch (block) {
case VSC73XX_BLOCK_MAC:
@@ -396,96 +371,18 @@ static int vsc73xx_is_addr_valid(u8 block, u8 subblock)
return 0;
}
-
-static u8 vsc73xx_make_addr(u8 mode, u8 block, u8 subblock)
-{
- u8 ret;
-
- ret = (block & VSC73XX_CMD_BLOCK_MASK) << VSC73XX_CMD_BLOCK_SHIFT;
- ret |= (mode & 1) << VSC73XX_CMD_MODE_SHIFT;
- ret |= subblock & VSC73XX_CMD_SUBBLOCK_MASK;
-
- return ret;
-}
+EXPORT_SYMBOL(vsc73xx_is_addr_valid);
static int vsc73xx_read(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
u32 *val)
{
- struct spi_transfer t[2];
- struct spi_message m;
- u8 cmd[4];
- u8 buf[4];
- int ret;
-
- if (!vsc73xx_is_addr_valid(block, subblock))
- return -EINVAL;
-
- spi_message_init(&m);
-
- memset(&t, 0, sizeof(t));
-
- t[0].tx_buf = cmd;
- t[0].len = sizeof(cmd);
- spi_message_add_tail(&t[0], &m);
-
- t[1].rx_buf = buf;
- t[1].len = sizeof(buf);
- spi_message_add_tail(&t[1], &m);
-
- cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_MODE_READ, block, subblock);
- cmd[1] = reg;
- cmd[2] = 0;
- cmd[3] = 0;
-
- mutex_lock(&vsc->lock);
- ret = spi_sync(vsc->spi, &m);
- mutex_unlock(&vsc->lock);
-
- if (ret)
- return ret;
-
- *val = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
-
- return 0;
+ return vsc->ops->read(vsc, block, subblock, reg, val);
}
static int vsc73xx_write(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
u32 val)
{
- struct spi_transfer t[2];
- struct spi_message m;
- u8 cmd[2];
- u8 buf[4];
- int ret;
-
- if (!vsc73xx_is_addr_valid(block, subblock))
- return -EINVAL;
-
- spi_message_init(&m);
-
- memset(&t, 0, sizeof(t));
-
- t[0].tx_buf = cmd;
- t[0].len = sizeof(cmd);
- spi_message_add_tail(&t[0], &m);
-
- t[1].tx_buf = buf;
- t[1].len = sizeof(buf);
- spi_message_add_tail(&t[1], &m);
-
- cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_MODE_WRITE, block, subblock);
- cmd[1] = reg;
-
- buf[0] = (val >> 24) & 0xff;
- buf[1] = (val >> 16) & 0xff;
- buf[2] = (val >> 8) & 0xff;
- buf[3] = val & 0xff;
-
- mutex_lock(&vsc->lock);
- ret = spi_sync(vsc->spi, &m);
- mutex_unlock(&vsc->lock);
-
- return ret;
+ return vsc->ops->write(vsc, block, subblock, reg, val);
}
static int vsc73xx_update_bits(struct vsc73xx *vsc, u8 block, u8 subblock,
@@ -1245,21 +1142,11 @@ static int vsc73xx_gpio_probe(struct vsc73xx *vsc)
return 0;
}
-static int vsc73xx_probe(struct spi_device *spi)
+int vsc73xx_probe(struct vsc73xx *vsc)
{
- struct device *dev = &spi->dev;
- struct vsc73xx *vsc;
+ struct device *dev = vsc->dev;
int ret;
- vsc = devm_kzalloc(dev, sizeof(*vsc), GFP_KERNEL);
- if (!vsc)
- return -ENOMEM;
-
- spi_set_drvdata(spi, vsc);
- vsc->spi = spi_dev_get(spi);
- vsc->dev = dev;
- mutex_init(&vsc->lock);
-
/* Release reset, if any */
vsc->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(vsc->reset)) {
@@ -1270,14 +1157,6 @@ static int vsc73xx_probe(struct spi_device *spi)
/* Wait 20ms according to datasheet table 245 */
msleep(20);
- spi->mode = SPI_MODE_0;
- spi->bits_per_word = 8;
- ret = spi_setup(spi);
- if (ret < 0) {
- dev_err(dev, "spi setup failed.\n");
- return ret;
- }
-
ret = vsc73xx_detect(vsc);
if (ret) {
dev_err(dev, "no chip found (%d)\n", ret);
@@ -1321,43 +1200,16 @@ static int vsc73xx_probe(struct spi_device *spi)
return 0;
}
+EXPORT_SYMBOL(vsc73xx_probe);
-static int vsc73xx_remove(struct spi_device *spi)
+int vsc73xx_remove(struct vsc73xx *vsc)
{
- struct vsc73xx *vsc = spi_get_drvdata(spi);
-
dsa_unregister_switch(vsc->ds);
gpiod_set_value(vsc->reset, 1);
return 0;
}
-
-static const struct of_device_id vsc73xx_of_match[] = {
- {
- .compatible = "vitesse,vsc7385",
- },
- {
- .compatible = "vitesse,vsc7388",
- },
- {
- .compatible = "vitesse,vsc7395",
- },
- {
- .compatible = "vitesse,vsc7398",
- },
- { },
-};
-MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
-
-static struct spi_driver vsc73xx_driver = {
- .probe = vsc73xx_probe,
- .remove = vsc73xx_remove,
- .driver = {
- .name = "vsc73xx",
- .of_match_table = vsc73xx_of_match,
- },
-};
-module_spi_driver(vsc73xx_driver);
+EXPORT_SYMBOL(vsc73xx_remove);
MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 driver");
diff --git a/drivers/net/dsa/vitesse-vsc73xx-spi.c b/drivers/net/dsa/vitesse-vsc73xx-spi.c
new file mode 100644
index 000000000000..0fe1e6175a4a
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx-spi.c
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: GPL-2.0
+/* DSA driver for:
+ * Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
+ *
+ * This driver takes control of the switch chip over SPI and
+ * configures it to route packages around when connected to a CPU port.
+ *
+ * Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
+ * Includes portions of code from the firmware uploader by:
+ * Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/spi/spi.h>
+
+#include "vitesse-vsc73xx.h"
+
+#define VSC73XX_CMD_SPI_MODE_READ 0
+#define VSC73XX_CMD_SPI_MODE_WRITE 1
+#define VSC73XX_CMD_SPI_MODE_SHIFT 4
+#define VSC73XX_CMD_SPI_BLOCK_SHIFT 5
+#define VSC73XX_CMD_SPI_BLOCK_MASK 0x7
+#define VSC73XX_CMD_SPI_SUBBLOCK_MASK 0xf
+
+/**
+ * struct vsc73xx_spi - VSC73xx SPI state container
+ */
+struct vsc73xx_spi {
+ struct spi_device *spi;
+ struct mutex lock; /* Protects SPI traffic */
+ struct vsc73xx vsc;
+};
+
+static const struct vsc73xx_ops vsc73xx_spi_ops;
+
+static u8 vsc73xx_make_addr(u8 mode, u8 block, u8 subblock)
+{
+ u8 ret;
+
+ ret = (block & VSC73XX_CMD_SPI_BLOCK_MASK)
+ << VSC73XX_CMD_SPI_BLOCK_SHIFT;
+ ret |= (mode & 1) << VSC73XX_CMD_SPI_MODE_SHIFT;
+ ret |= subblock & VSC73XX_CMD_SPI_SUBBLOCK_MASK;
+
+ return ret;
+}
+
+static int vsc73xx_spi_read(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 *val)
+{
+ struct vsc73xx_spi *vsc_spi = vsc->priv;
+ struct spi_transfer t[2];
+ struct spi_message m;
+ u8 cmd[4];
+ u8 buf[4];
+ int ret;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ spi_message_init(&m);
+
+ memset(&t, 0, sizeof(t));
+
+ t[0].tx_buf = cmd;
+ t[0].len = sizeof(cmd);
+ spi_message_add_tail(&t[0], &m);
+
+ t[1].rx_buf = buf;
+ t[1].len = sizeof(buf);
+ spi_message_add_tail(&t[1], &m);
+
+ cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_SPI_MODE_READ, block, subblock);
+ cmd[1] = reg;
+ cmd[2] = 0;
+ cmd[3] = 0;
+
+ mutex_lock(&vsc_spi->lock);
+ ret = spi_sync(vsc_spi->spi, &m);
+ mutex_unlock(&vsc_spi->lock);
+
+ if (ret)
+ return ret;
+
+ *val = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
+
+ return 0;
+}
+
+static int vsc73xx_spi_write(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 val)
+{
+ struct vsc73xx_spi *vsc_spi = vsc->priv;
+ struct spi_transfer t[2];
+ struct spi_message m;
+ u8 cmd[2];
+ u8 buf[4];
+ int ret;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ spi_message_init(&m);
+
+ memset(&t, 0, sizeof(t));
+
+ t[0].tx_buf = cmd;
+ t[0].len = sizeof(cmd);
+ spi_message_add_tail(&t[0], &m);
+
+ t[1].tx_buf = buf;
+ t[1].len = sizeof(buf);
+ spi_message_add_tail(&t[1], &m);
+
+ cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_SPI_MODE_WRITE, block, subblock);
+ cmd[1] = reg;
+
+ buf[0] = (val >> 24) & 0xff;
+ buf[1] = (val >> 16) & 0xff;
+ buf[2] = (val >> 8) & 0xff;
+ buf[3] = val & 0xff;
+
+ mutex_lock(&vsc_spi->lock);
+ ret = spi_sync(vsc_spi->spi, &m);
+ mutex_unlock(&vsc_spi->lock);
+
+ return ret;
+}
+
+static int vsc73xx_spi_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct vsc73xx_spi *vsc_spi;
+ int ret;
+
+ vsc_spi = devm_kzalloc(dev, sizeof(*vsc_spi), GFP_KERNEL);
+ if (!vsc_spi)
+ return -ENOMEM;
+
+ spi_set_drvdata(spi, vsc_spi);
+ vsc_spi->spi = spi_dev_get(spi);
+ vsc_spi->vsc.dev = dev;
+ vsc_spi->vsc.priv = vsc_spi;
+ vsc_spi->vsc.ops = &vsc73xx_spi_ops;
+ mutex_init(&vsc_spi->lock);
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ ret = spi_setup(spi);
+ if (ret < 0) {
+ dev_err(dev, "spi setup failed.\n");
+ return ret;
+ }
+
+ return vsc73xx_probe(&vsc_spi->vsc);
+}
+
+static int vsc73xx_spi_remove(struct spi_device *spi)
+{
+ struct vsc73xx_spi *vsc_spi = spi_get_drvdata(spi);
+
+ return vsc73xx_remove(&vsc_spi->vsc);
+}
+
+static const struct vsc73xx_ops vsc73xx_spi_ops = {
+ .read = vsc73xx_spi_read,
+ .write = vsc73xx_spi_write,
+};
+
+static const struct of_device_id vsc73xx_of_match[] = {
+ {
+ .compatible = "vitesse,vsc7385",
+ },
+ {
+ .compatible = "vitesse,vsc7388",
+ },
+ {
+ .compatible = "vitesse,vsc7395",
+ },
+ {
+ .compatible = "vitesse,vsc7398",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
+
+static struct spi_driver vsc73xx_spi_driver = {
+ .probe = vsc73xx_spi_probe,
+ .remove = vsc73xx_spi_remove,
+ .driver = {
+ .name = "vsc73xx-spi",
+ .of_match_table = vsc73xx_of_match,
+ },
+};
+module_spi_driver(vsc73xx_spi_driver);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 SPI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
new file mode 100644
index 000000000000..7478f8d4e0a9
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/device.h>
+#include <linux/etherdevice.h>
+#include <linux/gpio/driver.h>
+
+/**
+ * struct vsc73xx - VSC73xx state container
+ */
+struct vsc73xx {
+ struct device *dev;
+ struct gpio_desc *reset;
+ struct dsa_switch *ds;
+ struct gpio_chip gc;
+ u16 chipid;
+ u8 addr[ETH_ALEN];
+ const struct vsc73xx_ops *ops;
+ void *priv;
+};
+
+struct vsc73xx_ops {
+ int (*read)(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 *val);
+ int (*write)(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 val);
+};
+
+int vsc73xx_is_addr_valid(u8 block, u8 subblock);
+int vsc73xx_probe(struct vsc73xx *vsc);
+int vsc73xx_remove(struct vsc73xx *vsc);
--
2.20.1
^ permalink raw reply related
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