* [PATCH bpf-next V6 10/16] net/mlx5e: Refactor struct mlx5e_xdp_info
From: Tariq Toukan @ 2019-06-26 14:35 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, bjorn.topel, Magnus Karlsson
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Jonathan Lemon,
David S. Miller, Maxim Mikityanskiy, Tariq Toukan
In-Reply-To: <1561559738-4213-1-git-send-email-tariqt@mellanox.com>
From: Maxim Mikityanskiy <maximmi@mellanox.com>
Currently, struct mlx5e_xdp_info has some issues that have to be cleaned
up before the upcoming AF_XDP support makes things too complicated and
messy. This structure is used both when sending the packet and on
completion. Moreover, the cleanup procedure on completion depends on the
origin of the packet (XDP_REDIRECT, XDP_TX). Adding AF_XDP support will
add new flows that use this structure even differently. To avoid
overcomplicating the code, this commit refactors the usage of this
structure in the following ways:
1. struct mlx5e_xdp_info is split into two different structures. One is
struct mlx5e_xdp_xmit_data, a transient structure that doesn't need to
be stored and is only used while sending the packet. The other is still
struct mlx5e_xdp_info that is stored in a FIFO and contains the fields
needed on completion.
2. The fields of struct mlx5e_xdp_info that are used in different flows
are put into a union. A special enum indicates the cleanup mode and
helps choose the right union member. This approach is clear and
explicit. Although it could be possible to "guess" the mode by looking
at the values of the fields and at the XDP SQ type, it wouldn't be that
clear and extendable and would require looking through the whole chain
to understand what's going on.
For the reference, there are the fields of struct mlx5e_xdp_info that
are used in different flows (including AF_XDP ones):
Packet origin | Fields used on completion | Cleanup steps
-----------------------+---------------------------+------------------
XDP_REDIRECT, | xdpf, dma_addr | DMA unmap and
XDP_TX from XSK RQ | | xdp_return_frame.
-----------------------+---------------------------+------------------
XDP_TX from regular RQ | di | Recycle page.
-----------------------+---------------------------+------------------
AF_XDP TX | (none) | Increment the
| | producer index in
| | Completion Ring.
On send, the same set of mlx5e_xdp_xmit_data fields is used in all
flows: DMA and virtual addresses and length.
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 46 ++++++++++++--
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 81 +++++++++++++++---------
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h | 11 ++--
3 files changed, 97 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index c7676635bc8e..b3c2d7ef1287 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -403,10 +403,44 @@ struct mlx5e_dma_info {
dma_addr_t addr;
};
+/* XDP packets can be transmitted in different ways. On completion, we need to
+ * distinguish between them to clean up things in a proper way.
+ */
+enum mlx5e_xdp_xmit_mode {
+ /* An xdp_frame was transmitted due to either XDP_REDIRECT from another
+ * device or XDP_TX from an XSK RQ. The frame has to be unmapped and
+ * returned.
+ */
+ MLX5E_XDP_XMIT_MODE_FRAME,
+
+ /* The xdp_frame was created in place as a result of XDP_TX from a
+ * regular RQ. No DMA remapping happened, and the page belongs to us.
+ */
+ MLX5E_XDP_XMIT_MODE_PAGE,
+
+ /* No xdp_frame was created at all, the transmit happened from a UMEM
+ * page. The UMEM Completion Ring producer pointer has to be increased.
+ */
+ MLX5E_XDP_XMIT_MODE_XSK,
+};
+
struct mlx5e_xdp_info {
- struct xdp_frame *xdpf;
- dma_addr_t dma_addr;
- struct mlx5e_dma_info di;
+ enum mlx5e_xdp_xmit_mode mode;
+ union {
+ struct {
+ struct xdp_frame *xdpf;
+ dma_addr_t dma_addr;
+ } frame;
+ struct {
+ struct mlx5e_dma_info di;
+ } page;
+ };
+};
+
+struct mlx5e_xdp_xmit_data {
+ dma_addr_t dma_addr;
+ void *data;
+ u32 len;
};
struct mlx5e_xdp_info_fifo {
@@ -432,8 +466,10 @@ struct mlx5e_xdp_mpwqe {
};
struct mlx5e_xdpsq;
-typedef bool (*mlx5e_fp_xmit_xdp_frame)(struct mlx5e_xdpsq*,
- struct mlx5e_xdp_info*);
+typedef bool (*mlx5e_fp_xmit_xdp_frame)(struct mlx5e_xdpsq *,
+ struct mlx5e_xdp_xmit_data *,
+ struct mlx5e_xdp_info *);
+
struct mlx5e_xdpsq {
/* data path */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
index 5a900b70b203..89f6eb1109cf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
@@ -57,17 +57,27 @@ int mlx5e_xdp_max_mtu(struct mlx5e_params *params)
mlx5e_xmit_xdp_buff(struct mlx5e_xdpsq *sq, struct mlx5e_dma_info *di,
struct xdp_buff *xdp)
{
+ struct mlx5e_xdp_xmit_data xdptxd;
struct mlx5e_xdp_info xdpi;
+ struct xdp_frame *xdpf;
+ dma_addr_t dma_addr;
- xdpi.xdpf = convert_to_xdp_frame(xdp);
- if (unlikely(!xdpi.xdpf))
+ xdpf = convert_to_xdp_frame(xdp);
+ if (unlikely(!xdpf))
return false;
- xdpi.dma_addr = di->addr + (xdpi.xdpf->data - (void *)xdpi.xdpf);
- dma_sync_single_for_device(sq->pdev, xdpi.dma_addr,
- xdpi.xdpf->len, DMA_TO_DEVICE);
- xdpi.di = *di;
- return sq->xmit_xdp_frame(sq, &xdpi);
+ xdptxd.data = xdpf->data;
+ xdptxd.len = xdpf->len;
+
+ xdpi.mode = MLX5E_XDP_XMIT_MODE_PAGE;
+
+ dma_addr = di->addr + (xdpf->data - (void *)xdpf);
+ dma_sync_single_for_device(sq->pdev, dma_addr, xdptxd.len, DMA_TO_DEVICE);
+
+ xdptxd.dma_addr = dma_addr;
+ xdpi.page.di = *di;
+
+ return sq->xmit_xdp_frame(sq, &xdptxd, &xdpi);
}
/* returns true if packet was consumed by xdp */
@@ -184,14 +194,13 @@ static void mlx5e_xdp_mpwqe_complete(struct mlx5e_xdpsq *sq)
}
static bool mlx5e_xmit_xdp_frame_mpwqe(struct mlx5e_xdpsq *sq,
+ struct mlx5e_xdp_xmit_data *xdptxd,
struct mlx5e_xdp_info *xdpi)
{
struct mlx5e_xdp_mpwqe *session = &sq->mpwqe;
struct mlx5e_xdpsq_stats *stats = sq->stats;
- struct xdp_frame *xdpf = xdpi->xdpf;
-
- if (unlikely(sq->hw_mtu < xdpf->len)) {
+ if (unlikely(xdptxd->len > sq->hw_mtu)) {
stats->err++;
return false;
}
@@ -208,7 +217,7 @@ static bool mlx5e_xmit_xdp_frame_mpwqe(struct mlx5e_xdpsq *sq,
mlx5e_xdp_mpwqe_session_start(sq);
}
- mlx5e_xdp_mpwqe_add_dseg(sq, xdpi, stats);
+ mlx5e_xdp_mpwqe_add_dseg(sq, xdptxd, stats);
if (unlikely(session->complete ||
session->ds_count == session->max_ds_count))
@@ -219,7 +228,9 @@ static bool mlx5e_xmit_xdp_frame_mpwqe(struct mlx5e_xdpsq *sq,
return true;
}
-static bool mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xdp_info *xdpi)
+static bool mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq,
+ struct mlx5e_xdp_xmit_data *xdptxd,
+ struct mlx5e_xdp_info *xdpi)
{
struct mlx5_wq_cyc *wq = &sq->wq;
u16 pi = mlx5_wq_cyc_ctr2ix(wq, sq->pc);
@@ -229,9 +240,8 @@ static bool mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xdp_info *
struct mlx5_wqe_eth_seg *eseg = &wqe->eth;
struct mlx5_wqe_data_seg *dseg = wqe->data;
- struct xdp_frame *xdpf = xdpi->xdpf;
- dma_addr_t dma_addr = xdpi->dma_addr;
- unsigned int dma_len = xdpf->len;
+ dma_addr_t dma_addr = xdptxd->dma_addr;
+ u32 dma_len = xdptxd->len;
struct mlx5e_xdpsq_stats *stats = sq->stats;
@@ -253,7 +263,7 @@ static bool mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xdp_info *
/* copy the inline part if required */
if (sq->min_inline_mode != MLX5_INLINE_MODE_NONE) {
- memcpy(eseg->inline_hdr.start, xdpf->data, MLX5E_XDP_MIN_INLINE);
+ memcpy(eseg->inline_hdr.start, xdptxd->data, MLX5E_XDP_MIN_INLINE);
eseg->inline_hdr.sz = cpu_to_be16(MLX5E_XDP_MIN_INLINE);
dma_len -= MLX5E_XDP_MIN_INLINE;
dma_addr += MLX5E_XDP_MIN_INLINE;
@@ -286,14 +296,19 @@ static void mlx5e_free_xdpsq_desc(struct mlx5e_xdpsq *sq,
for (i = 0; i < wi->num_pkts; i++) {
struct mlx5e_xdp_info xdpi = mlx5e_xdpi_fifo_pop(xdpi_fifo);
- if (rq) {
- /* XDP_TX */
- mlx5e_page_release(rq, &xdpi.di, recycle);
- } else {
+ switch (xdpi.mode) {
+ case MLX5E_XDP_XMIT_MODE_FRAME:
/* XDP_REDIRECT */
- dma_unmap_single(sq->pdev, xdpi.dma_addr,
- xdpi.xdpf->len, DMA_TO_DEVICE);
- xdp_return_frame(xdpi.xdpf);
+ dma_unmap_single(sq->pdev, xdpi.frame.dma_addr,
+ xdpi.frame.xdpf->len, DMA_TO_DEVICE);
+ xdp_return_frame(xdpi.frame.xdpf);
+ break;
+ case MLX5E_XDP_XMIT_MODE_PAGE:
+ /* XDP_TX */
+ mlx5e_page_release(rq, &xdpi.page.di, recycle);
+ break;
+ default:
+ WARN_ON_ONCE(true);
}
}
}
@@ -398,21 +413,27 @@ int mlx5e_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
for (i = 0; i < n; i++) {
struct xdp_frame *xdpf = frames[i];
+ struct mlx5e_xdp_xmit_data xdptxd;
struct mlx5e_xdp_info xdpi;
- xdpi.dma_addr = dma_map_single(sq->pdev, xdpf->data, xdpf->len,
- DMA_TO_DEVICE);
- if (unlikely(dma_mapping_error(sq->pdev, xdpi.dma_addr))) {
+ xdptxd.data = xdpf->data;
+ xdptxd.len = xdpf->len;
+ xdptxd.dma_addr = dma_map_single(sq->pdev, xdptxd.data,
+ xdptxd.len, DMA_TO_DEVICE);
+
+ if (unlikely(dma_mapping_error(sq->pdev, xdptxd.dma_addr))) {
xdp_return_frame_rx_napi(xdpf);
drops++;
continue;
}
- xdpi.xdpf = xdpf;
+ xdpi.mode = MLX5E_XDP_XMIT_MODE_FRAME;
+ xdpi.frame.xdpf = xdpf;
+ xdpi.frame.dma_addr = xdptxd.dma_addr;
- if (unlikely(!sq->xmit_xdp_frame(sq, &xdpi))) {
- dma_unmap_single(sq->pdev, xdpi.dma_addr,
- xdpf->len, DMA_TO_DEVICE);
+ if (unlikely(!sq->xmit_xdp_frame(sq, &xdptxd, &xdpi))) {
+ dma_unmap_single(sq->pdev, xdptxd.dma_addr,
+ xdptxd.len, DMA_TO_DEVICE);
xdp_return_frame_rx_napi(xdpf);
drops++;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
index 8b537a4b0840..2a5158993349 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
@@ -97,15 +97,14 @@ static inline void mlx5e_xdp_update_inline_state(struct mlx5e_xdpsq *sq)
}
static inline void
-mlx5e_xdp_mpwqe_add_dseg(struct mlx5e_xdpsq *sq, struct mlx5e_xdp_info *xdpi,
+mlx5e_xdp_mpwqe_add_dseg(struct mlx5e_xdpsq *sq,
+ struct mlx5e_xdp_xmit_data *xdptxd,
struct mlx5e_xdpsq_stats *stats)
{
struct mlx5e_xdp_mpwqe *session = &sq->mpwqe;
- dma_addr_t dma_addr = xdpi->dma_addr;
- struct xdp_frame *xdpf = xdpi->xdpf;
struct mlx5_wqe_data_seg *dseg =
(struct mlx5_wqe_data_seg *)session->wqe + session->ds_count;
- u16 dma_len = xdpf->len;
+ u32 dma_len = xdptxd->len;
session->pkt_count++;
@@ -124,7 +123,7 @@ static inline void mlx5e_xdp_update_inline_state(struct mlx5e_xdpsq *sq)
}
inline_dseg->byte_count = cpu_to_be32(dma_len | MLX5_INLINE_SEG);
- memcpy(inline_dseg->data, xdpf->data, dma_len);
+ memcpy(inline_dseg->data, xdptxd->data, dma_len);
session->ds_count += ds_cnt;
stats->inlnw++;
@@ -132,7 +131,7 @@ static inline void mlx5e_xdp_update_inline_state(struct mlx5e_xdpsq *sq)
}
no_inline:
- dseg->addr = cpu_to_be64(dma_addr);
+ dseg->addr = cpu_to_be64(xdptxd->dma_addr);
dseg->byte_count = cpu_to_be32(dma_len);
dseg->lkey = sq->mkey_be;
session->ds_count++;
--
1.8.3.1
^ permalink raw reply related
* [PATCH bpf-next V6 05/16] xsk: Change the default frame size to 4096 and allow controlling it
From: Tariq Toukan @ 2019-06-26 14:35 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, bjorn.topel, Magnus Karlsson
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Jonathan Lemon,
David S. Miller, Maxim Mikityanskiy, Tariq Toukan
In-Reply-To: <1561559738-4213-1-git-send-email-tariqt@mellanox.com>
From: Maxim Mikityanskiy <maximmi@mellanox.com>
The typical XDP memory scheme is one packet per page. Change the AF_XDP
frame size in libbpf to 4096, which is the page size on x86, to allow
libbpf to be used with the drivers with the packet-per-page scheme.
Add a command line option -f to xdpsock to allow to specify a custom
frame size.
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
---
samples/bpf/xdpsock_user.c | 44 ++++++++++++++++++++++++++++----------------
tools/lib/bpf/xsk.h | 2 +-
2 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 0f5eb0d7f2df..93eaaf7239b2 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -68,6 +68,7 @@ enum benchmark_type {
static int opt_poll;
static int opt_interval = 1;
static u32 opt_xdp_bind_flags;
+static int opt_xsk_frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
static __u32 prog_id;
struct xsk_umem_info {
@@ -276,6 +277,12 @@ static size_t gen_eth_frame(struct xsk_umem_info *umem, u64 addr)
static struct xsk_umem_info *xsk_configure_umem(void *buffer, u64 size)
{
struct xsk_umem_info *umem;
+ struct xsk_umem_config cfg = {
+ .fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
+ .comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS,
+ .frame_size = opt_xsk_frame_size,
+ .frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM,
+ };
int ret;
umem = calloc(1, sizeof(*umem));
@@ -283,7 +290,7 @@ static struct xsk_umem_info *xsk_configure_umem(void *buffer, u64 size)
exit_with_error(errno);
ret = xsk_umem__create(&umem->umem, buffer, size, &umem->fq, &umem->cq,
- NULL);
+ &cfg);
if (ret)
exit_with_error(-ret);
@@ -323,11 +330,9 @@ static struct xsk_socket_info *xsk_configure_socket(struct xsk_umem_info *umem)
&idx);
if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
exit_with_error(-ret);
- for (i = 0;
- i < XSK_RING_PROD__DEFAULT_NUM_DESCS *
- XSK_UMEM__DEFAULT_FRAME_SIZE;
- i += XSK_UMEM__DEFAULT_FRAME_SIZE)
- *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx++) = i;
+ for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++)
+ *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx++) =
+ i * opt_xsk_frame_size;
xsk_ring_prod__submit(&xsk->umem->fq,
XSK_RING_PROD__DEFAULT_NUM_DESCS);
@@ -346,6 +351,7 @@ static struct xsk_socket_info *xsk_configure_socket(struct xsk_umem_info *umem)
{"interval", required_argument, 0, 'n'},
{"zero-copy", no_argument, 0, 'z'},
{"copy", no_argument, 0, 'c'},
+ {"frame-size", required_argument, 0, 'f'},
{0, 0, 0, 0}
};
@@ -365,8 +371,9 @@ static void usage(const char *prog)
" -n, --interval=n Specify statistics update interval (default 1 sec).\n"
" -z, --zero-copy Force zero-copy mode.\n"
" -c, --copy Force copy mode.\n"
+ " -f, --frame-size=n Set the frame size (must be a power of two, default is %d).\n"
"\n";
- fprintf(stderr, str, prog);
+ fprintf(stderr, str, prog, XSK_UMEM__DEFAULT_FRAME_SIZE);
exit(EXIT_FAILURE);
}
@@ -377,7 +384,7 @@ static void parse_command_line(int argc, char **argv)
opterr = 0;
for (;;) {
- c = getopt_long(argc, argv, "Frtli:q:psSNn:cz", long_options,
+ c = getopt_long(argc, argv, "Frtli:q:psSNn:czf:", long_options,
&option_index);
if (c == -1)
break;
@@ -420,6 +427,9 @@ static void parse_command_line(int argc, char **argv)
case 'F':
opt_xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
break;
+ case 'f':
+ opt_xsk_frame_size = atoi(optarg);
+ break;
default:
usage(basename(argv[0]));
}
@@ -432,6 +442,11 @@ static void parse_command_line(int argc, char **argv)
usage(basename(argv[0]));
}
+ if (opt_xsk_frame_size & (opt_xsk_frame_size - 1)) {
+ fprintf(stderr, "--frame-size=%d is not a power of two\n",
+ opt_xsk_frame_size);
+ usage(basename(argv[0]));
+ }
}
static void kick_tx(struct xsk_socket_info *xsk)
@@ -583,8 +598,7 @@ static void tx_only(struct xsk_socket_info *xsk)
for (i = 0; i < BATCH_SIZE; i++) {
xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->addr
- = (frame_nb + i) <<
- XSK_UMEM__DEFAULT_FRAME_SHIFT;
+ = (frame_nb + i) * opt_xsk_frame_size;
xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->len =
sizeof(pkt_data) - 1;
}
@@ -661,21 +675,19 @@ int main(int argc, char **argv)
}
ret = posix_memalign(&bufs, getpagesize(), /* PAGE_SIZE aligned */
- NUM_FRAMES * XSK_UMEM__DEFAULT_FRAME_SIZE);
+ NUM_FRAMES * opt_xsk_frame_size);
if (ret)
exit_with_error(ret);
/* Create sockets... */
- umem = xsk_configure_umem(bufs,
- NUM_FRAMES * XSK_UMEM__DEFAULT_FRAME_SIZE);
+ umem = xsk_configure_umem(bufs, NUM_FRAMES * opt_xsk_frame_size);
xsks[num_socks++] = xsk_configure_socket(umem);
if (opt_bench == BENCH_TXONLY) {
int i;
- for (i = 0; i < NUM_FRAMES * XSK_UMEM__DEFAULT_FRAME_SIZE;
- i += XSK_UMEM__DEFAULT_FRAME_SIZE)
- (void)gen_eth_frame(umem, i);
+ for (i = 0; i < NUM_FRAMES; i++)
+ (void)gen_eth_frame(umem, i * opt_xsk_frame_size);
}
signal(SIGINT, int_exit);
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 82ea71a0f3ec..833a6e60d065 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -167,7 +167,7 @@ static inline void *xsk_umem__get_data(void *umem_area, __u64 addr)
#define XSK_RING_CONS__DEFAULT_NUM_DESCS 2048
#define XSK_RING_PROD__DEFAULT_NUM_DESCS 2048
-#define XSK_UMEM__DEFAULT_FRAME_SHIFT 11 /* 2048 bytes */
+#define XSK_UMEM__DEFAULT_FRAME_SHIFT 12 /* 4096 bytes */
#define XSK_UMEM__DEFAULT_FRAME_SIZE (1 << XSK_UMEM__DEFAULT_FRAME_SHIFT)
#define XSK_UMEM__DEFAULT_FRAME_HEADROOM 0
--
1.8.3.1
^ permalink raw reply related
* [PATCH bpf-next V6 13/16] net/mlx5e: Consider XSK in XDP MTU limit calculation
From: Tariq Toukan @ 2019-06-26 14:35 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, bjorn.topel, Magnus Karlsson
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Jonathan Lemon,
David S. Miller, Maxim Mikityanskiy, Tariq Toukan
In-Reply-To: <1561559738-4213-1-git-send-email-tariqt@mellanox.com>
From: Maxim Mikityanskiy <maximmi@mellanox.com>
Use the existing mlx5e_get_linear_rq_headroom function to calculate the
headroom for mlx5e_xdp_max_mtu. This function takes the XSK headroom
into consideration, which will be used in the following patches.
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en/params.c | 4 ++--
drivers/net/ethernet/mellanox/mlx5/core/en/params.h | 2 ++
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 5 +++--
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h | 3 ++-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 ++--
5 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
index 50a458dc3836..0de908b12fcc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
@@ -9,8 +9,8 @@ static inline bool mlx5e_rx_is_xdp(struct mlx5e_params *params,
return params->xdp_prog || xsk;
}
-static inline u16 mlx5e_get_linear_rq_headroom(struct mlx5e_params *params,
- struct mlx5e_xsk_param *xsk)
+u16 mlx5e_get_linear_rq_headroom(struct mlx5e_params *params,
+ struct mlx5e_xsk_param *xsk)
{
u16 headroom = NET_IP_ALIGN;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.h b/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
index ed420f3efe52..7f29b82dd8c2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
@@ -11,6 +11,8 @@ struct mlx5e_xsk_param {
u16 chunk_size;
};
+u16 mlx5e_get_linear_rq_headroom(struct mlx5e_params *params,
+ struct mlx5e_xsk_param *xsk);
u32 mlx5e_rx_get_linear_frag_sz(struct mlx5e_params *params,
struct mlx5e_xsk_param *xsk);
u8 mlx5e_mpwqe_log_pkts_per_wqe(struct mlx5e_params *params);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
index 1364bdff702c..ee99efde9143 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
@@ -32,10 +32,11 @@
#include <linux/bpf_trace.h>
#include "en/xdp.h"
+#include "en/params.h"
-int mlx5e_xdp_max_mtu(struct mlx5e_params *params)
+int mlx5e_xdp_max_mtu(struct mlx5e_params *params, struct mlx5e_xsk_param *xsk)
{
- int hr = NET_IP_ALIGN + XDP_PACKET_HEADROOM;
+ int hr = mlx5e_get_linear_rq_headroom(params, xsk);
/* Let S := SKB_DATA_ALIGN(sizeof(struct skb_shared_info)).
* The condition checked in mlx5e_rx_is_linear_skb is:
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
index 86db5ad49a42..9200cb9f499b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
@@ -39,7 +39,8 @@
(sizeof(struct mlx5e_tx_wqe) / MLX5_SEND_WQE_DS)
#define MLX5E_XDP_TX_DS_COUNT (MLX5E_XDP_TX_EMPTY_DS_COUNT + 1 /* SG DS */)
-int mlx5e_xdp_max_mtu(struct mlx5e_params *params);
+struct mlx5e_xsk_param;
+int mlx5e_xdp_max_mtu(struct mlx5e_params *params, struct mlx5e_xsk_param *xsk);
bool mlx5e_xdp_handle(struct mlx5e_rq *rq, struct mlx5e_dma_info *di,
void *va, u16 *rx_headroom, u32 *len);
bool mlx5e_poll_xdpsq_cq(struct mlx5e_cq *cq);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index abc1d0f6cf53..96fb3fa32d63 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3722,7 +3722,7 @@ int mlx5e_change_mtu(struct net_device *netdev, int new_mtu,
if (params->xdp_prog &&
!mlx5e_rx_is_linear_skb(&new_channels.params)) {
netdev_err(netdev, "MTU(%d) > %d is not allowed while XDP enabled\n",
- new_mtu, mlx5e_xdp_max_mtu(params));
+ new_mtu, mlx5e_xdp_max_mtu(params, NULL));
err = -EINVAL;
goto out;
}
@@ -4167,7 +4167,7 @@ static int mlx5e_xdp_allowed(struct mlx5e_priv *priv, struct bpf_prog *prog)
if (!mlx5e_rx_is_linear_skb(&new_channels.params)) {
netdev_warn(netdev, "XDP is not allowed with MTU(%d) > %d\n",
new_channels.params.sw_mtu,
- mlx5e_xdp_max_mtu(&new_channels.params));
+ mlx5e_xdp_max_mtu(&new_channels.params, NULL));
return -EINVAL;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH bpf-next V6 12/16] net/mlx5e: XDP_TX from UMEM support
From: Tariq Toukan @ 2019-06-26 14:35 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, bjorn.topel, Magnus Karlsson
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Jonathan Lemon,
David S. Miller, Maxim Mikityanskiy, Tariq Toukan
In-Reply-To: <1561559738-4213-1-git-send-email-tariqt@mellanox.com>
From: Maxim Mikityanskiy <maximmi@mellanox.com>
When an XDP program returns XDP_TX, and the RQ is XSK-enabled, it
requires careful handling, because convert_to_xdp_frame creates a new
page and copies the data there, while our driver expects the xdp_frame
to point to the same memory as the xdp_buff. Handle this case
separately: map the page, and in the end unmap it and call
xdp_return_frame.
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 50 ++++++++++++++++++++----
1 file changed, 42 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
index b3e118fc4521..1364bdff702c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
@@ -69,14 +69,48 @@ int mlx5e_xdp_max_mtu(struct mlx5e_params *params)
xdptxd.data = xdpf->data;
xdptxd.len = xdpf->len;
- xdpi.mode = MLX5E_XDP_XMIT_MODE_PAGE;
+ if (xdp->rxq->mem.type == MEM_TYPE_ZERO_COPY) {
+ /* The xdp_buff was in the UMEM and was copied into a newly
+ * allocated page. The UMEM page was returned via the ZCA, and
+ * this new page has to be mapped at this point and has to be
+ * unmapped and returned via xdp_return_frame on completion.
+ */
+
+ /* Prevent double recycling of the UMEM page. Even in case this
+ * function returns false, the xdp_buff shouldn't be recycled,
+ * as it was already done in xdp_convert_zc_to_xdp_frame.
+ */
+ __set_bit(MLX5E_RQ_FLAG_XDP_XMIT, rq->flags); /* non-atomic */
+
+ xdpi.mode = MLX5E_XDP_XMIT_MODE_FRAME;
- dma_addr = di->addr + (xdpf->data - (void *)xdpf);
- dma_sync_single_for_device(sq->pdev, dma_addr, xdptxd.len, DMA_TO_DEVICE);
+ dma_addr = dma_map_single(sq->pdev, xdptxd.data, xdptxd.len,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(sq->pdev, dma_addr)) {
+ xdp_return_frame(xdpf);
+ return false;
+ }
- xdptxd.dma_addr = dma_addr;
- xdpi.page.rq = rq;
- xdpi.page.di = *di;
+ xdptxd.dma_addr = dma_addr;
+ xdpi.frame.xdpf = xdpf;
+ xdpi.frame.dma_addr = dma_addr;
+ } else {
+ /* Driver assumes that convert_to_xdp_frame returns an xdp_frame
+ * that points to the same memory region as the original
+ * xdp_buff. It allows to map the memory only once and to use
+ * the DMA_BIDIRECTIONAL mode.
+ */
+
+ xdpi.mode = MLX5E_XDP_XMIT_MODE_PAGE;
+
+ dma_addr = di->addr + (xdpf->data - (void *)xdpf);
+ dma_sync_single_for_device(sq->pdev, dma_addr, xdptxd.len,
+ DMA_TO_DEVICE);
+
+ xdptxd.dma_addr = dma_addr;
+ xdpi.page.rq = rq;
+ xdpi.page.di = *di;
+ }
return sq->xmit_xdp_frame(sq, &xdptxd, &xdpi);
}
@@ -298,13 +332,13 @@ static void mlx5e_free_xdpsq_desc(struct mlx5e_xdpsq *sq,
switch (xdpi.mode) {
case MLX5E_XDP_XMIT_MODE_FRAME:
- /* XDP_REDIRECT */
+ /* XDP_TX from the XSK RQ and XDP_REDIRECT */
dma_unmap_single(sq->pdev, xdpi.frame.dma_addr,
xdpi.frame.xdpf->len, DMA_TO_DEVICE);
xdp_return_frame(xdpi.frame.xdpf);
break;
case MLX5E_XDP_XMIT_MODE_PAGE:
- /* XDP_TX */
+ /* XDP_TX from the regular RQ */
mlx5e_page_release(xdpi.page.rq, &xdpi.page.di, recycle);
break;
default:
--
1.8.3.1
^ permalink raw reply related
* [PATCH bpf-next V6 08/16] net/mlx5e: Calculate linear RX frag size considering XSK
From: Tariq Toukan @ 2019-06-26 14:35 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, bjorn.topel, Magnus Karlsson
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Jonathan Lemon,
David S. Miller, Maxim Mikityanskiy, Tariq Toukan
In-Reply-To: <1561559738-4213-1-git-send-email-tariqt@mellanox.com>
From: Maxim Mikityanskiy <maximmi@mellanox.com>
Additional conditions introduced:
- XSK implies XDP.
- Headroom includes the XSK headroom if it exists.
- No space is reserved for struct shared_skb_info in XSK mode.
- Fragment size smaller than the XSK chunk size is not allowed.
A new auxiliary function mlx5e_get_linear_rq_headroom with the support
for XSK is introduced. Use this function in the implementation of
mlx5e_get_rq_headroom. Change headroom to u32 to match the headroom
field in struct xdp_umem.
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/en/params.c | 65 +++++++++++++++-------
.../net/ethernet/mellanox/mlx5/core/en/params.h | 8 ++-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
3 files changed, 52 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
index d3744bffbae3..50a458dc3836 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
@@ -3,33 +3,62 @@
#include "en/params.h"
-u32 mlx5e_rx_get_linear_frag_sz(struct mlx5e_params *params)
+static inline bool mlx5e_rx_is_xdp(struct mlx5e_params *params,
+ struct mlx5e_xsk_param *xsk)
{
- u16 hw_mtu = MLX5E_SW2HW_MTU(params, params->sw_mtu);
- u16 linear_rq_headroom = params->xdp_prog ?
- XDP_PACKET_HEADROOM : MLX5_RX_HEADROOM;
- u32 frag_sz;
+ return params->xdp_prog || xsk;
+}
+
+static inline u16 mlx5e_get_linear_rq_headroom(struct mlx5e_params *params,
+ struct mlx5e_xsk_param *xsk)
+{
+ u16 headroom = NET_IP_ALIGN;
+
+ if (mlx5e_rx_is_xdp(params, xsk)) {
+ headroom += XDP_PACKET_HEADROOM;
+ if (xsk)
+ headroom += xsk->headroom;
+ } else {
+ headroom += MLX5_RX_HEADROOM;
+ }
+
+ return headroom;
+}
+
+u32 mlx5e_rx_get_linear_frag_sz(struct mlx5e_params *params,
+ struct mlx5e_xsk_param *xsk)
+{
+ u32 hw_mtu = MLX5E_SW2HW_MTU(params, params->sw_mtu);
+ u16 linear_rq_headroom = mlx5e_get_linear_rq_headroom(params, xsk);
+ u32 frag_sz = linear_rq_headroom + hw_mtu;
- linear_rq_headroom += NET_IP_ALIGN;
+ /* AF_XDP doesn't build SKBs in place. */
+ if (!xsk)
+ frag_sz = MLX5_SKB_FRAG_SZ(frag_sz);
- frag_sz = MLX5_SKB_FRAG_SZ(linear_rq_headroom + hw_mtu);
+ /* XDP in mlx5e doesn't support multiple packets per page. */
+ if (mlx5e_rx_is_xdp(params, xsk))
+ frag_sz = max_t(u32, frag_sz, PAGE_SIZE);
- if (params->xdp_prog && frag_sz < PAGE_SIZE)
- frag_sz = PAGE_SIZE;
+ /* Even if we can go with a smaller fragment size, we must not put
+ * multiple packets into a single frame.
+ */
+ if (xsk)
+ frag_sz = max_t(u32, frag_sz, xsk->chunk_size);
return frag_sz;
}
u8 mlx5e_mpwqe_log_pkts_per_wqe(struct mlx5e_params *params)
{
- u32 linear_frag_sz = mlx5e_rx_get_linear_frag_sz(params);
+ u32 linear_frag_sz = mlx5e_rx_get_linear_frag_sz(params, NULL);
return MLX5_MPWRQ_LOG_WQE_SZ - order_base_2(linear_frag_sz);
}
bool mlx5e_rx_is_linear_skb(struct mlx5e_params *params)
{
- u32 frag_sz = mlx5e_rx_get_linear_frag_sz(params);
+ u32 frag_sz = mlx5e_rx_get_linear_frag_sz(params, NULL);
return !params->lro_en && frag_sz <= PAGE_SIZE;
}
@@ -39,7 +68,7 @@ bool mlx5e_rx_is_linear_skb(struct mlx5e_params *params)
bool mlx5e_rx_mpwqe_is_linear_skb(struct mlx5_core_dev *mdev,
struct mlx5e_params *params)
{
- u32 frag_sz = mlx5e_rx_get_linear_frag_sz(params);
+ u32 frag_sz = mlx5e_rx_get_linear_frag_sz(params, NULL);
s8 signed_log_num_strides_param;
u8 log_num_strides;
@@ -75,7 +104,7 @@ u8 mlx5e_mpwqe_get_log_stride_size(struct mlx5_core_dev *mdev,
struct mlx5e_params *params)
{
if (mlx5e_rx_mpwqe_is_linear_skb(mdev, params))
- return order_base_2(mlx5e_rx_get_linear_frag_sz(params));
+ return order_base_2(mlx5e_rx_get_linear_frag_sz(params, NULL));
return MLX5_MPWRQ_DEF_LOG_STRIDE_SZ(mdev);
}
@@ -90,15 +119,9 @@ u8 mlx5e_mpwqe_get_log_num_strides(struct mlx5_core_dev *mdev,
u16 mlx5e_get_rq_headroom(struct mlx5_core_dev *mdev,
struct mlx5e_params *params)
{
- u16 linear_rq_headroom = params->xdp_prog ?
- XDP_PACKET_HEADROOM : MLX5_RX_HEADROOM;
- bool is_linear_skb;
-
- linear_rq_headroom += NET_IP_ALIGN;
-
- is_linear_skb = (params->rq_wq_type == MLX5_WQ_TYPE_CYCLIC) ?
+ bool is_linear_skb = (params->rq_wq_type == MLX5_WQ_TYPE_CYCLIC) ?
mlx5e_rx_is_linear_skb(params) :
mlx5e_rx_mpwqe_is_linear_skb(mdev, params);
- return is_linear_skb ? linear_rq_headroom : 0;
+ return is_linear_skb ? mlx5e_get_linear_rq_headroom(params, NULL) : 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.h b/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
index b106a0236f36..ed420f3efe52 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
@@ -6,7 +6,13 @@
#include "en.h"
-u32 mlx5e_rx_get_linear_frag_sz(struct mlx5e_params *params);
+struct mlx5e_xsk_param {
+ u16 headroom;
+ u16 chunk_size;
+};
+
+u32 mlx5e_rx_get_linear_frag_sz(struct mlx5e_params *params,
+ struct mlx5e_xsk_param *xsk);
u8 mlx5e_mpwqe_log_pkts_per_wqe(struct mlx5e_params *params);
bool mlx5e_rx_is_linear_skb(struct mlx5e_params *params);
bool mlx5e_rx_mpwqe_is_linear_skb(struct mlx5_core_dev *mdev,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 1b427d7fab42..837a973b3507 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -1954,7 +1954,7 @@ static void mlx5e_build_rq_frags_info(struct mlx5_core_dev *mdev,
if (mlx5e_rx_is_linear_skb(params)) {
int frag_stride;
- frag_stride = mlx5e_rx_get_linear_frag_sz(params);
+ frag_stride = mlx5e_rx_get_linear_frag_sz(params, NULL);
frag_stride = roundup_pow_of_two(frag_stride);
info->arr[0].frag_size = byte_count;
--
1.8.3.1
^ permalink raw reply related
* [PATCH bpf-next V6 04/16] libbpf: Support getsockopt XDP_OPTIONS
From: Tariq Toukan @ 2019-06-26 14:35 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, bjorn.topel, Magnus Karlsson
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Jonathan Lemon,
David S. Miller, Maxim Mikityanskiy, Tariq Toukan
In-Reply-To: <1561559738-4213-1-git-send-email-tariqt@mellanox.com>
From: Maxim Mikityanskiy <maximmi@mellanox.com>
Query XDP_OPTIONS in libbpf to determine if the zero-copy mode is active
or not.
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
Acked-by: Björn Töpel <bjorn.topel@intel.com>
---
tools/lib/bpf/xsk.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index 7ef6293b4fd7..bf15a80a37c2 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -65,6 +65,7 @@ struct xsk_socket {
int xsks_map_fd;
__u32 queue_id;
char ifname[IFNAMSIZ];
+ bool zc;
};
struct xsk_nl_info {
@@ -480,6 +481,7 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
void *rx_map = NULL, *tx_map = NULL;
struct sockaddr_xdp sxdp = {};
struct xdp_mmap_offsets off;
+ struct xdp_options opts;
struct xsk_socket *xsk;
socklen_t optlen;
int err;
@@ -597,6 +599,16 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
}
xsk->prog_fd = -1;
+
+ optlen = sizeof(opts);
+ err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts, &optlen);
+ if (err) {
+ err = -errno;
+ goto out_mmap_tx;
+ }
+
+ xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
+
if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
err = xsk_setup_xdp_prog(xsk);
if (err)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next] xdp: Make __mem_id_disconnect static
From: Daniel Borkmann @ 2019-06-26 14:33 UTC (permalink / raw)
To: YueHaibing, davem, ast, jakub.kicinski, hawk, john.fastabend
Cc: linux-kernel, netdev, xdp-newbies, bpf
In-Reply-To: <20190625023137.29272-1-yuehaibing@huawei.com>
On 06/25/2019 04:31 AM, YueHaibing wrote:
> Fix sparse warning:
>
> net/core/xdp.c:88:6: warning:
> symbol '__mem_id_disconnect' was not declared. Should it be static?
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2] samples: bpf: make the use of xdp samples consistent
From: Daniel Borkmann @ 2019-06-26 14:33 UTC (permalink / raw)
To: Daniel T. Lee, Alexei Starovoitov; +Cc: bpf, netdev
In-Reply-To: <20190625005536.2516-1-danieltimlee@gmail.com>
On 06/25/2019 02:55 AM, Daniel T. Lee wrote:
> Currently, each xdp samples are inconsistent in the use.
> Most of the samples fetch the interface with it's name.
> (ex. xdp1, xdp2skb, xdp_redirect_cpu, xdp_sample_pkts, etc.)
>
> But some of the xdp samples are fetching the interface with
> ifindex by command argument.
>
> This commit enables xdp samples to fetch interface with it's name
> without changing the original index interface fetching.
> (<ifname|ifindex> fetching in the same way as xdp_sample_pkts_user.c does.)
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix compiler warning with CONFIG_MODULES=n
From: Daniel Borkmann @ 2019-06-26 14:32 UTC (permalink / raw)
To: Yonghong Song, ast, netdev, bpf; +Cc: kernel-team, arnd
In-Reply-To: <20190626003503.1985698-1-yhs@fb.com>
On 06/26/2019 02:35 AM, Yonghong Song wrote:
> With CONFIG_MODULES=n, the following compiler warning occurs:
> /data/users/yhs/work/net-next/kernel/trace/bpf_trace.c:605:13: warning:
> ‘do_bpf_send_signal’ defined but not used [-Wunused-function]
> static void do_bpf_send_signal(struct irq_work *entry)
>
> The __init function send_signal_irq_work_init(), which calls
> do_bpf_send_signal(), is defined under CONFIG_MODULES. Hence,
> when CONFIG_MODULES=n, nobody calls static function do_bpf_send_signal(),
> hence the warning.
>
> The init function send_signal_irq_work_init() should work without
> CONFIG_MODULES. Moving it out of CONFIG_MODULES
> code section fixed the compiler warning, and also make bpf_send_signal()
> helper work without CONFIG_MODULES.
>
> Fixes: 8b401f9ed244 ("bpf: implement bpf_send_signal() helper")
> Reported-By: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Yonghong Song <yhs@fb.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: build tests with debug info
From: Daniel Borkmann @ 2019-06-26 14:32 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko, ast, netdev, bpf
In-Reply-To: <20190625225628.3129845-1-andriin@fb.com>
On 06/26/2019 12:56 AM, Andrii Nakryiko wrote:
> Non-BPF (user land) part of selftests is built without debug info making
> occasional debugging with gdb terrible. Build with debug info always.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2 bpf-next] libbpf: fix max() type mismatch for 32bit
From: Daniel Borkmann @ 2019-06-26 14:32 UTC (permalink / raw)
To: Ivan Khoronzhuk, ast, netdev; +Cc: bpf, linux-kernel
In-Reply-To: <20190626103837.6455-1-ivan.khoronzhuk@linaro.org>
On 06/26/2019 12:38 PM, Ivan Khoronzhuk wrote:
> It fixes build error for 32bit caused by type mismatch
> size_t/unsigned long.
>
> Fixes: bf82927125dd ("libbpf: refactor map initialization")
> Acked-by: Song Liu <songliubraving@fb.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf] bpf: fix BPF_ALU32 | BPF_ARSH on BE arches
From: Daniel Borkmann @ 2019-06-26 14:31 UTC (permalink / raw)
To: Jiong Wang, alexei.starovoitov; +Cc: yauheni.kaliuta, bpf, netdev, oss-drivers
In-Reply-To: <1561480910-23543-1-git-send-email-jiong.wang@netronome.com>
On 06/25/2019 06:41 PM, Jiong Wang wrote:
> Yauheni reported the following code do not work correctly on BE arches:
>
> ALU_ARSH_X:
> DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
> CONT;
> ALU_ARSH_K:
> DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
> CONT;
>
> and are causing failure of test_verifier test 'arsh32 on imm 2' on BE
> arches.
>
> The code is taking address and interpreting memory directly, so is not
> endianness neutral. We should instead perform standard C type casting on
> the variable. A u64 to s32 conversion will drop the high 32-bit and reserve
> the low 32-bit as signed integer, this is all we want.
>
> Fixes: 2dc6b100f928 ("bpf: interpreter support BPF_ALU | BPF_ARSH")
> Reported-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf] tools: bpftool: use correct argument in cgroup errors
From: Daniel Borkmann @ 2019-06-26 14:30 UTC (permalink / raw)
To: Jakub Kicinski, alexei.starovoitov
Cc: netdev, bpf, oss-drivers, guro, sdf, Quentin Monnet
In-Reply-To: <20190625165631.18928-1-jakub.kicinski@netronome.com>
On 06/25/2019 06:56 PM, Jakub Kicinski wrote:
> cgroup code tries to use argv[0] as the cgroup path,
> but if it fails uses argv[1] to report errors.
>
> Fixes: 5ccda64d38cc ("bpftool: implement cgroup bpf operations")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] xsk: Properly terminate assignment in xskq_produce_flush_desc
From: Daniel Borkmann @ 2019-06-26 14:30 UTC (permalink / raw)
To: Nathan Chancellor, Björn Töpel, Magnus Karlsson,
David S. Miller, Alexei Starovoitov, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend
Cc: netdev, bpf, xdp-newbies, linux-kernel, clang-built-linux,
Nick Desaulniers, Nathan Huckleberry
In-Reply-To: <20190625182352.13918-1-natechancellor@gmail.com>
On 06/25/2019 08:23 PM, Nathan Chancellor wrote:
> Clang warns:
>
> In file included from net/xdp/xsk_queue.c:10:
> net/xdp/xsk_queue.h:292:2: warning: expression result unused
> [-Wunused-value]
> WRITE_ONCE(q->ring->producer, q->prod_tail);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/compiler.h:284:6: note: expanded from macro 'WRITE_ONCE'
> __u.__val; \
> ~~~ ^~~~~
> 1 warning generated.
>
> The q->prod_tail assignment has a comma at the end, not a semi-colon.
> Fix that so clang no longer warns and everything works as expected.
>
> Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
> Link: https://github.com/ClangBuiltLinux/linux/issues/544
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2 bpf-next 3/7] libbpf: add kprobe/uprobe attach API
From: Daniel Borkmann @ 2019-06-26 14:25 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko, ast, sdf, bpf, netdev,
kernel-team
In-Reply-To: <20190621045555.4152743-4-andriin@fb.com>
On 06/21/2019 06:55 AM, Andrii Nakryiko wrote:
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 207 +++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 8 ++
> tools/lib/bpf/libbpf.map | 2 +
> 3 files changed, 217 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2bb1fa008be3..d506772df350 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3969,6 +3969,213 @@ int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
> return 0;
> }
>
> +static int parse_uint(const char *buf)
> +{
> + int ret;
> +
> + errno = 0;
> + ret = (int)strtol(buf, NULL, 10);
> + if (errno) {
> + ret = -errno;
> + pr_debug("failed to parse '%s' as unsigned int\n", buf);
> + return ret;
> + }
> + if (ret < 0) {
> + pr_debug("failed to parse '%s' as unsigned int\n", buf);
> + return -EINVAL;
> + }
> + return ret;
> +}
> +
> +static int parse_uint_from_file(const char* file)
> +{
> + char buf[STRERR_BUFSIZE];
> + int fd, ret;
> +
> + fd = open(file, O_RDONLY);
> + if (fd < 0) {
> + ret = -errno;
> + pr_debug("failed to open '%s': %s\n", file,
> + libbpf_strerror_r(ret, buf, sizeof(buf)));
> + return ret;
> + }
> + ret = read(fd, buf, sizeof(buf));
> + ret = ret < 0 ? -errno : ret;
> + close(fd);
> + if (ret < 0) {
> + pr_debug("failed to read '%s': %s\n", file,
> + libbpf_strerror_r(ret, buf, sizeof(buf)));
> + return ret;
> + }
> + if (ret == 0 || ret >= sizeof(buf)) {
> + buf[sizeof(buf) - 1] = 0;
> + pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> + return -EINVAL;
> + }
> + return parse_uint(buf);
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/type";
> + return parse_uint_from_file(file);
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/type";
> + return parse_uint_from_file(file);
> +}
> +
> +static int parse_config_from_file(const char *file)
> +{
> + char buf[STRERR_BUFSIZE];
> + int fd, ret;
> +
> + fd = open(file, O_RDONLY);
> + if (fd < 0) {
> + ret = -errno;
> + pr_debug("failed to open '%s': %s\n", file,
> + libbpf_strerror_r(ret, buf, sizeof(buf)));
> + return ret;
> + }
> + ret = read(fd, buf, sizeof(buf));
> + ret = ret < 0 ? -errno : ret;
> + close(fd);
> + if (ret < 0) {
> + pr_debug("failed to read '%s': %s\n", file,
> + libbpf_strerror_r(ret, buf, sizeof(buf)));
> + return ret;
> + }
> + if (ret == 0 || ret >= sizeof(buf)) {
> + buf[sizeof(buf) - 1] = 0;
> + pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> + return -EINVAL;
> + }
> + if (strncmp(buf, "config:", 7)) {
> + pr_debug("expected 'config:' prefix, found '%s'\n", buf);
> + return -EINVAL;
> + }
> + return parse_uint(buf + 7);
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> + return parse_config_from_file(file);
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> + return parse_config_from_file(file);
> +}
> +
> +static int perf_event_open_probe(bool uprobe, bool retprobe, const char* name,
> + uint64_t offset, int pid)
> +{
> + struct perf_event_attr attr = {};
> + char errmsg[STRERR_BUFSIZE];
> + int type, pfd, err;
> +
> + type = uprobe ? determine_uprobe_perf_type()
> + : determine_kprobe_perf_type();
> + if (type < 0) {
> + pr_warning("failed to determine %s perf type: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> + return type;
> + }
> + if (retprobe) {
> + int bit = uprobe ? determine_uprobe_retprobe_bit()
> + : determine_kprobe_retprobe_bit();
> +
> + if (bit < 0) {
> + pr_warning("failed to determine %s retprobe bit: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(bit, errmsg,
> + sizeof(errmsg)));
> + return bit;
> + }
> + attr.config |= 1 << bit;
> + }
> + attr.size = sizeof(attr);
> + attr.type = type;
> + attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> + attr.config2 = offset; /* kprobe_addr or probe_offset */
> +
> + /* pid filter is meaningful only for uprobes */
> + pfd = syscall(__NR_perf_event_open, &attr,
> + pid < 0 ? -1 : pid /* pid */,
> + pid == -1 ? 0 : -1 /* cpu */,
> + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> + if (pfd < 0) {
> + err = -errno;
> + pr_warning("%s perf_event_open() failed: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
> +
> +int bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> + const char *func_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + int pfd, err;
> +
> + pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> + 0 /* offset */, -1 /* pid */);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return pfd;
> + }
> + err = bpf_program__attach_perf_event(prog, pfd);
> + if (err) {
> + libbpf_perf_event_disable_and_close(pfd);
> + pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
I do like that we facilitate usage by adding these APIs to libbpf, but my $0.02
would be that they should be designed slightly different. See it as a nit, but
given it's exposed in libbpf.map and therefore immutable in future it's worth
considering; right now with this set here you have:
int bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
const char *func_name)
int bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
pid_t pid, const char *binary_path,
size_t func_offset)
int bpf_program__attach_tracepoint(struct bpf_program *prog,
const char *tp_category,
const char *tp_name)
int bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
const char *tp_name)
int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
int libbpf_perf_event_disable_and_close(int pfd)
So the idea is that all the bpf_program__attach_*() APIs return an fd that you
can later on pass into libbpf_perf_event_disable_and_close(). I think there is
a bit of a disconnect in that the bpf_program__attach_*() APIs try to do too
many things at once. For example, the bpf_program__attach_raw_tracepoint() fd
has nothing to do with perf, so passing to libbpf_perf_event_disable_and_close()
kind of works, but is hacky since there's no PERF_EVENT_IOC_DISABLE for it so this
would always error if a user cares to check the return code. In the kernel, we
use anon inode for this kind of object. Also, if a user tries to add more than
one program to the same event, we need to recreate a new event fd every time.
What this boils down to is that this should get a proper abstraction, e.g. as
in struct libbpf_event which holds the event object. There should be helper
functions like libbpf_event_create_{kprobe,uprobe,tracepoint,raw_tracepoint} returning
such an struct libbpf_event object on success, and a single libbpf_event_destroy()
that does the event specific teardown. bpf_program__attach_event() can then take
care of only attaching the program to it. Having an object for this is also more
extensible than just a fd number. Nice thing is that this can also be completely
internal to libbpf.c as with struct bpf_program and other abstractions where we
don't expose the internals in the public header.
Thanks,
Daniel
^ permalink raw reply
* Re: samples/bpf compilation failures - 5.2.0
From: Joel Fernandes @ 2019-06-26 14:21 UTC (permalink / raw)
To: Srinivas Ramana
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, open list:BPF (Safe dynamic programs and tools),
bpf, Joel Fernandes (Google)
In-Reply-To: <0bcdd38c-5cdb-0510-573a-9a6098ab2105@codeaurora.org>
On Wed, Jun 26, 2019 at 1:41 AM Srinivas Ramana <sramana@codeaurora.org> wrote:
>
> + Joel if he has seen this issue.
>
I have not seen this issue and it has been some time since I built BPF
samples, sorry. I am mostly building BPF programs through bcc/bpftrace
and Android's build system.
We ought to make samples easier to build though, I remember when I
built it last year there were header related issues which should be
fixed. It could be that new BPF features broke it again.
J.
> On 5/28/2019 2:27 PM, Srinivas Ramana wrote:
> > Hello,
> >
> > I am trying to build samples/bpf in kernel(5.2.0-rc1) but unsuccessful
> > with below errors. Can you help to point what i am missing or if there
> > is some known issue?
> >
> > ==============================8<===================================
> > $ make samples/bpf/
> > LLC=/local/mnt/workspace/tools/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/llc
> > CLANG=/local/mnt/workspace/tools/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > V=1
> > make -C /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel -f
> > /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/Makefile
> > samples/bpf/
> > ................
> > ................
> > ................
> > make KBUILD_MODULES=1 -f ./scripts/Makefile.build obj=samples/bpf
> > (cat /dev/null; ) > samples/bpf/modules.order
> > make -C
> > /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../tools/lib/bpf/
> > RM='rm -rf' LDFLAGS=
> > srctree=/local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../
> > O=
> >
> > Auto-detecting system features:
> > ... libelf: [ on ]
> > ... bpf: [ on ]
> >
> > make -C
> > /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../..//tools/build
> > CFLAGS= LDFLAGS= fixdep
> > make -f
> > /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../..//tools/build/Makefile.build
> > dir=. obj=fixdep
> > ld -r -o fixdep-in.o fixdep.o
> > ld: fixdep.o: Relocations in generic ELF (EM: 183)
> > ld: fixdep.o: Relocations in generic ELF (EM: 183)
> > fixdep.o: error adding symbols: File in wrong format
> > make[5]: *** [fixdep-in.o] Error 1
> > make[4]: *** [fixdep-in.o] Error 2
> > make[3]: *** [fixdep] Error 2
> > make[2]: ***
> > [/local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../tools/lib/bpf/libbpf.a]
> > Error 2
> > make[1]: *** [samples/bpf/] Error 2
> > make: *** [sub-make] Error 2
> > ==============================>8=======================================
> >
> >
> > I am using the below commands to build:
> > ========================================================
> > export ARCH=arm64
> > export CROSS_COMPILE=<path>linaro-toolchain/5.1/bin/aarch64-linux-gnu-
> > export CLANG_TRIPLE=arm64-linux-gnu-
> >
> > make
> > CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > defconfig
> >
> > make
> > CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > -j8
> >
> > make
> > CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > headers_install INSTALL_HDR_PATH=./usr
> >
> > make samples/bpf/
> > LLC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/llc
> > CLANG=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > V=1
> > CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> >
> > ========================================================
> >
> > Thanks,
> > -- Srinivas R
> >
>
>
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation
> Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
> Collaborative Project
^ permalink raw reply
* Re: [PATCH net-next] can: dev: call netif_carrier_off() in register_candev()
From: Willem de Bruijn @ 2019-06-26 14:17 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Rasmus Villemoes, linux-can@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ff8160d4-3357-9b4f-1840-bbe46195da5a@prevas.dk>
On Wed, Jun 26, 2019 at 5:31 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 24/06/2019 19.26, Willem de Bruijn wrote:
> > On Mon, Jun 24, 2019 at 4:34 AM Rasmus Villemoes
> > <rasmus.villemoes@prevas.dk> wrote:
> >>
> >> CONFIG_CAN_LEDS is deprecated. When trying to use the generic netdev
> >> trigger as suggested, there's a small inconsistency with the link
> >> property: The LED is on initially, stays on when the device is brought
> >> up, and then turns off (as expected) when the device is brought down.
> >>
> >> Make sure the LED always reflects the state of the CAN device.
> >>
> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> >
> > Should this target net?
>
> No, I think this should go through the CAN tree. Perhaps I've
> misunderstood when to use the net-next prefix - is that only for things
> that should be applied directly to the net-next tree? If so, sorry.
I don't see consistent behavior on the list, so this is probably fine.
It would probably help to target can (for fixes) or can-next (for new
features).
Let me reframe the question: should this target can, instead of can-next?
> > Regardless of CONFIG_CAN_LEDS deprecation,
> > this is already not initialized properly if that CONFIG is disabled
> > and a can_led_event call at device probe is a noop.
>
> I'm not sure I understand this part. The CONFIG_CAN_LEDS support for
> showing the state of the interface is implemented via hooking into the
> ndo_open/ndo_stop callbacks, and does not look at or touch the
> __LINK_STATE_NOCARRIER bit at all.
>
> Other than via the netdev LED trigger I don't think one can even observe
> the slightly odd initial state of the __LINK_STATE_NOCARRIER bit for CAN
> devices,
it's still incorrect, though I guess that's moot in practice.
> which is why I framed this as a fix purely to allow the netdev
> trigger to be a closer drop-in replacement for CONFIG_CAN_LEDS.
So the entire CONFIG_CAN_LEDS code is to be removed? What exactly is
this netdev trigger replacement, if not can_led_event? Sorry, I
probably miss some context.
^ permalink raw reply
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Ivan Khoronzhuk @ 2019-06-26 14:01 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David Miller, grygorii.strashko, hawk, brouer, saeedm, leon,
Alexei Starovoitov, linux-kernel, linux-omap, xdp-newbies,
ilias.apalodimas, Network Development, Daniel Borkmann,
jakub.kicinski, John Fastabend
In-Reply-To: <CA+FuTSff=+zqxxmCv3+bNxraigNgx_1Wm5Kn2FM7TTSZV4dnOg@mail.gmail.com>
On Tue, Jun 25, 2019 at 09:36:15PM -0400, Willem de Bruijn wrote:
>On Tue, Jun 25, 2019 at 2:00 PM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> Add user counter allowing to delete pool only when no users.
>> It doesn't prevent pool from flush, only prevents freeing the
>> pool instance. Helps when no need to delete the pool and now
>> it's user responsibility to free it by calling page_pool_free()
>> while destroying procedure. It also makes to use page_pool_free()
>> explicitly, not fully hidden in xdp unreg, which looks more
>> correct after page pool "create" routine.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>
>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>> index f07c518ef8a5..1ec838e9927e 100644
>> --- a/include/net/page_pool.h
>> +++ b/include/net/page_pool.h
>> @@ -101,6 +101,7 @@ struct page_pool {
>> struct ptr_ring ring;
>>
>> atomic_t pages_state_release_cnt;
>> + atomic_t user_cnt;
>
>refcount_t?
yes, thanks.
>
>> };
>>
>> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
>> @@ -183,6 +184,12 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
>> return page->dma_addr;
>> }
>>
>> +/* used to prevent pool from deallocation */
>> +static inline void page_pool_get(struct page_pool *pool)
>> +{
>> + atomic_inc(&pool->user_cnt);
>> +}
>> +
>> static inline bool is_page_pool_compiled_in(void)
>> {
>> #ifdef CONFIG_PAGE_POOL
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index b366f59885c1..169b0e3c870e 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -48,6 +48,7 @@ static int page_pool_init(struct page_pool *pool,
>> return -ENOMEM;
>>
>> atomic_set(&pool->pages_state_release_cnt, 0);
>> + atomic_set(&pool->user_cnt, 0);
>>
>> if (pool->p.flags & PP_FLAG_DMA_MAP)
>> get_device(pool->p.dev);
>> @@ -70,6 +71,8 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
>> kfree(pool);
>> return ERR_PTR(err);
>> }
>> +
>> + page_pool_get(pool);
>> return pool;
>> }
>> EXPORT_SYMBOL(page_pool_create);
>> @@ -356,6 +359,10 @@ static void __warn_in_flight(struct page_pool *pool)
>>
>> void __page_pool_free(struct page_pool *pool)
>> {
>> + /* free only if no users */
>> + if (!atomic_dec_and_test(&pool->user_cnt))
>> + return;
>> +
>> WARN(pool->alloc.count, "API usage violation");
>> WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 829377cc83db..04bdcd784d2e 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -372,6 +372,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>>
>> mutex_unlock(&mem_id_lock);
>>
>> + if (type == MEM_TYPE_PAGE_POOL)
>> + page_pool_get(xdp_alloc->page_pool);
>> +
>
>need an analogous page_pool_put in xdp_rxq_info_unreg_mem_model? mlx5
>does not use that inverse function, but intel drivers do.
no need, it's put after call to page_pool_free() in unreg workqueue.
>
>> trace_mem_connect(xdp_alloc, xdp_rxq);
>> return 0;
>> err:
>> --
>> 2.17.1
>>
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Arnd Bergmann @ 2019-06-26 13:58 UTC (permalink / raw)
To: Alex Elder
Cc: Johannes Berg, Dan Williams, Subash Abhinov Kasiviswanathan,
abhishek.esse, Ben Chan, Bjorn Andersson, cpratapa, David Miller,
DTML, Eric Caruso, evgreen, Ilias Apalodimas, Linux ARM,
linux-arm-msm, Linux Kernel Mailing List, linux-soc, Networking,
syadagir
In-Reply-To: <edea19ef-f225-bdcd-f394-77e326d1d3ad@linaro.org>
On Wed, Jun 26, 2019 at 3:39 PM Alex Elder <elder@linaro.org> wrote:
> On 6/25/19 9:19 AM, Johannes Berg wrote:
> > On Mon, 2019-06-24 at 18:40 +0200, Arnd Bergmann wrote:
>
> > So, IOW, I'm not sure I see how you'd split that up. I guess you could
> > if you actually do something like the "rmnet" model, and I suppose
> > you're free to do that for IPA if you like, but I tend to think that's
> > actually a burden, not a win since you just get more complex code that
> > needs to interact with more pieces. A single driver for a single
> > hardware that knows about the few types of channels seems simpler to me.
> >
> >> - to answer Johannes question, my understanding is that the interface
> >> between kernel and firmware/hardware for IPA has a single 'struct
> >> device' that is used for both the data and the control channels,
> >> rather than having a data channel and an independent control device,
> >> so this falls into the same category as the Intel one (please correct
> >> me on that)
>
> I don't think that's quite right, but it might be partially
> right. There is a single device representing IPA, but the
> picture is a little more complicated.
>
> The IPA hardware is actually something that sits *between* the
> AP and the modem. It implements one form of communication
> pathway (IP data), but there are others (including QMI, which
> presents a network-like interface but it's actually implemented
> via clever use of shared memory and interrupts).
Can you clarify how QMI fits in here? Do you mean one has to
talk to both IPA and QMI to use the modem, or are these two
alternative implementations for the same basic purpose?
> > That sounds about the same then, right.
> >
> > Are the control channels to IPA are actually also tunnelled over the
> > rmnet protocol? And even if they are, perhaps they have a different
> > hardware queue or so? That'd be the case for Intel - different hardware
> > queue, same (or at least similar) protocol spoken for the DMA hardware
> > itself, but different contents of the messages obviously.
>
> I want to be careful talking about "control" but for IPA it comes
> from user space. For the purpose of getting initial code upstream,
> all of that control functionality (which was IOCTL based) has been
> removed, and a fixed configuration is assumed.
My previous understanding was that from the hardware perspective
there is only one control interface, which is for IPA. Part of this
is abstracted to user space with ioctl commands to the IPA driver,
and then one must set up rmnet to match these by configuring
an rmnet device over netlink messages from user space, but
rmnet does not have a control protocol with the hardware.
The exception here is the flow control, which is handled using
in-band XON/OFF messages from the modem to rmnet (and
corresponding Acks the other way) that IPA just passes through.
If we also need to talk to QMI, that would be something completely
different though.
Arnd
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Alex Elder @ 2019-06-26 13:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Johannes Berg, Dan Williams, Subash Abhinov Kasiviswanathan,
abhishek.esse, Ben Chan, Bjorn Andersson, cpratapa, David Miller,
DTML, Eric Caruso, evgreen, Ilias Apalodimas, Linux ARM,
linux-arm-msm, Linux Kernel Mailing List, linux-soc, Networking,
syadagir
In-Reply-To: <CAK8P3a1ixL9ZjYz=pWTxvMfeD89S6QxSeHt9ZCL9dkCNV5pMHQ@mail.gmail.com>
On 6/24/19 11:40 AM, Arnd Bergmann wrote:
> On Mon, Jun 24, 2019 at 6:21 PM Alex Elder <elder@linaro.org> wrote:
>> On 6/18/19 2:03 PM, Johannes Berg wrote:
>>
>>> Really there are two possible ways (and they intersect to some extent).
. . .
>>> The other is something like IPA or the Intel modem driver, where the
>>> device is actually a single (e.g. PCIe) device and just has a single
>>> driver, but that single driver offers different channels.
>>
>> What I don't like about this is that it's more monolithic. It
>> seems better to have the low-level IPA or Intel modem driver (or
>> any other driver that can support communication between the AP
>> and WWAN device) present communication paths that other function-
>> specific drivers can attach to and use.
>
> I did not understand Johannes description as two competing models
> for the same code, but rather two kinds of existing hardware that
> a new driver system would have to deal with.
Based on my understanding of what he said in a message I just
responded to, I think you are exactly right.
. . .
> What we should try to avoid though is a way to add driver private
> interfaces that risk having multiple drivers create similar functionality
> in incompatible ways.
Agreed.
-Alex
^ permalink raw reply
* [PATCH net-next 08/10] net: stmmac: Update RX Tail Pointer to last free entry
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
Update the RX Tail Pointer to the last available SKB entry.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 65ff2a2a15e4..0893af8b432d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3337,6 +3337,8 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
}
rx_q->dirty_rx = entry;
+ rx_q->rx_tail_addr = rx_q->dma_rx_phy +
+ (rx_q->dirty_rx * sizeof(struct dma_desc));
stmmac_set_rx_tail_ptr(priv, priv->ioaddr, rx_q->rx_tail_addr, queue);
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 10/10] net: stmmac: Try to get C45 PHY if everything else fails
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
On PCI based setups that are connected to C45 PHY we won't have DT
bindings specifying what's the correct PHY type.
Fallback to C45 if everything else fails when trying to acquire PHY.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bc949665c529..e790ab79e819 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1014,6 +1014,20 @@ static int stmmac_init_phy(struct net_device *dev)
phydev = mdiobus_get_phy(priv->mii, addr);
if (!phydev) {
+ /* Try C45 */
+ phydev = get_phy_device(priv->mii, addr, true);
+ if (phydev && !IS_ERR(phydev)) {
+ ret = phy_device_register(phydev);
+ if (ret) {
+ phy_device_free(phydev);
+ phydev = NULL;
+ }
+ } else {
+ phydev = NULL;
+ }
+ }
+
+ if (!phydev) {
netdev_err(priv->dev, "no phy at addr %d\n", addr);
return -ENODEV;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 03/10] net: stmmac: Decrease default RX Watchdog value
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
For performance reasons decrease the default RX Watchdog value for the
minimum allowed.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index ad9e9368535d..14d8f6d7cb9a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -246,7 +246,7 @@ struct stmmac_safety_stats {
/* Max/Min RI Watchdog Timer count value */
#define MAX_DMA_RIWT 0xff
-#define MIN_DMA_RIWT 0x20
+#define MIN_DMA_RIWT 0x10
/* Tx coalesce parameters */
#define STMMAC_COAL_TX_TIMER 1000
#define STMMAC_MAX_COAL_TX_TICK 100000
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fab65f129207..dc57a2c0a630 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2512,9 +2512,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
priv->tx_lpi_timer = STMMAC_DEFAULT_TWT_LS;
if (priv->use_riwt) {
- ret = stmmac_rx_watchdog(priv, priv->ioaddr, MAX_DMA_RIWT, rx_cnt);
+ ret = stmmac_rx_watchdog(priv, priv->ioaddr, MIN_DMA_RIWT, rx_cnt);
if (!ret)
- priv->rx_riwt = MAX_DMA_RIWT;
+ priv->rx_riwt = MIN_DMA_RIWT;
}
if (priv->hw->pcs)
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 00/10] net: stmmac: 10GbE using XGMAC
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
Support for 10Gb Link using XGMAC core plus some performance tweaks.
Tested in a PCI based setup.
iperf3 TCP results:
TSO ON, MTU=1500, TX Queues = 1, RX Queues = 1, Flow Control ON
Pinned CPU (-A), Zero-Copy (-Z)
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-600.00 sec 643 GBytes 9.21 Gbits/sec 1 sender
[ 5] 0.00-600.00 sec 643 GBytes 9.21 Gbits/sec receiver
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Jose Abreu (10):
net: stmmac: dwxgmac: Enable EDMA by default
net: stmmac: Do not try to enable PHY EEE if MAC does not support it
net: stmmac: Decrease default RX Watchdog value
net: stmmac: dwxgmac: Fix the undefined burst setting
net: stmmac: Add the missing speeds that XGMAC supports
net: stmmac: Do not disable interrupts when cleaning TX
net: stmmac: Enable support for > 32 Bits addressing in XGMAC
net: stmmac: Update RX Tail Pointer to last free entry
net: stmmac: Only disable interrupts if NAPI is scheduled
net: stmmac: Try to get C45 PHY if everything else fails
drivers/net/ethernet/stmicro/stmmac/common.h | 9 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 14 +-
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 14 +-
.../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 4 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 27 +++-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 165 ++++++++++++++++-----
6 files changed, 178 insertions(+), 55 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next 01/10] net: stmmac: dwxgmac: Enable EDMA by default
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
Enable the EDMA feature by default which gives higher performance.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 ++
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index b8296eb41011..50b41ecb0325 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -177,6 +177,8 @@
#define XGMAC_BLEN8 BIT(2)
#define XGMAC_BLEN4 BIT(1)
#define XGMAC_UNDEF BIT(0)
+#define XGMAC_TX_EDMA_CTRL 0x00003040
+#define XGMAC_RX_EDMA_CTRL 0x00003044
#define XGMAC_DMA_CH_CONTROL(x) (0x00003100 + (0x80 * (x)))
#define XGMAC_PBLx8 BIT(16)
#define XGMAC_DMA_CH_TX_CONTROL(x) (0x00003104 + (0x80 * (x)))
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 7861a938420a..a1ad49680c07 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -122,6 +122,8 @@ static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
}
writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+ writel(GENMASK(29, 0), ioaddr + XGMAC_TX_EDMA_CTRL);
+ writel(GENMASK(29, 0), ioaddr + XGMAC_RX_EDMA_CTRL);
}
static void dwxgmac2_dma_rx_mode(void __iomem *ioaddr, int mode,
--
2.7.4
^ 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