* RE: [PATCH iproute2-next v1 11/19] rdma: Simplify code to reuse existing functions
From: Steve Wise @ 2019-02-21 20:23 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-12-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 11/19] rdma: Simplify code to reuse
> existing functions
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Remove duplicated functions in favour general res_print_uint() call.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> rdma/res-cmid.c | 4 ++--
> rdma/res-cq.c | 14 +++-----------
> rdma/res-mr.c | 2 +-
> rdma/res-pd.c | 4 ++--
> rdma/res-qp.c | 14 +++-----------
> rdma/res.c | 34 ++--------------------------------
> rdma/res.h | 3 ---
> 7 files changed, 13 insertions(+), 62 deletions(-)
>
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 10/19] rdma: Properly mark RDMAtool license
From: Steve Wise @ 2019-02-21 20:22 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-11-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 10/19] rdma: Properly mark RDMAtool
> license
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> RDMA subsystem is dual-licensed with "GPL-2.0 OR Linux-OpenIB" proper
> license and Mellanox submission are supposed to have this type of license.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> rdma/Makefile | 2 +-
> rdma/dev.c | 7 +------
> rdma/link.c | 7 +------
> rdma/rdma.c | 7 +------
> rdma/rdma.h | 7 +------
> rdma/res.c | 7 +------
> rdma/utils.c | 7 +------
> 7 files changed, 7 insertions(+), 37 deletions(-)
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 09/19] rdma: Move resource PD logic to separate file
From: Steve Wise @ 2019-02-21 20:21 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-10-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 09/19] rdma: Move resource PD logic to
> separate file
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Logically separate resource PD logic to separate file,
> in order to make PD specific logic self-contained.
>
This should be "QP" not "PD"
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> rdma/Makefile | 2 +-
> rdma/res-qp.c | 234
> +++++++++++++++++++++++++++++++++++++++++++++++++
> rdma/res.c | 236 --------------------------------------------------
> rdma/res.h | 17 ++++
> 4 files changed, 252 insertions(+), 237 deletions(-)
> create mode 100644 rdma/res-qp.c
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 08/19] rdma: Move out resource CM-ID logic to separate file
From: Steve Wise @ 2019-02-21 20:17 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-9-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 08/19] rdma: Move out resource CM-ID
> logic to separate file
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Logically separate resource CM-ID logic to separate file,
> in order to make CM-ID specific logic self-contained.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 07/19] rdma: Move out resource CQ logic to separate file
From: Steve Wise @ 2019-02-21 20:16 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-8-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 07/19] rdma: Move out resource CQ logic
> to separate file
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Logically separate resource CQ logic to separate file,
> in order to make CQ specific logic self-contained.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 06/19] rdma: Refactor out resource MR logic to separate file
From: Steve Wise @ 2019-02-21 20:15 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-7-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 06/19] rdma: Refactor out resource MR
> logic to separate file
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Logically separate resource MR logic to separate file,
> in order to make MR specific logic self-contained.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
Ah I guess you're moving them all into their own files. Ok.
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 05/19] rdma: Move resource PD logic to separate file
From: Steve Wise @ 2019-02-21 20:15 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-6-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 05/19] rdma: Move resource PD logic to
> separate file
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Logically separate resource PD logic to separate file,
> in order to make PD specific logic self-contained.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
Why does it need to be self contained?
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 04/19] rdma: Provide parent context index for all objects except CM_ID
From: Steve Wise @ 2019-02-21 20:12 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-5-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 04/19] rdma: Provide parent context index
> for all objects except CM_ID
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Allow users to correlate allocated object with relevant parent
>
> [leonro@server ~]$ rdma res show pd
> dev mlx5_0 users 5 pid 0 comm [ib_core] pdn 1
> dev mlx5_0 users 7 pid 0 comm [ib_ipoib] pdn 2
> dev mlx5_0 users 0 pid 0 comm [mlx5_ib] pdn 3
> dev mlx5_0 users 2 pid 548 comm ibv_rc_pingpong ctxn 0 pdn 4
>
> [leonro@server ~]$ rdma res show cq cqn 0-100
> dev mlx5_0 cqe 2047 users 6 poll-ctx UNBOUND_WORKQUEUE pid 0 comm
> [ib_core] cqn 2
> dev mlx5_0 cqe 255 users 2 poll-ctx SOFTIRQ pid 0 comm [mlx5_ib] cqn 3
> dev mlx5_0 cqe 511 users 1 poll-ctx DIRECT pid 0 comm [ib_ipoib] cqn 4
> dev mlx5_0 cqe 255 users 1 poll-ctx DIRECT pid 0 comm [ib_ipoib] cqn 5
> dev mlx5_0 cqe 255 users 0 poll-ctx SOFTIRQ pid 0 comm [mlx5_ib] cqn 6
> dev mlx5_0 cqe 511 users 2 pid 548 comm ibv_rc_pingpong cqn 7 ctxn 0
>
> [leonro@server ~]$ rdma res show mr
> dev mlx5_0 mrlen 4096 pid 548 comm ibv_rc_pingpong mrn 4 pdn 0
>
> [leonro@nps-server-14-015 ~]$ /images/leonro/src/iproute2/rdma/rdma res
> show qp
> link mlx5_0/1 lqpn 0 type SMI state RTS sq-psn 0 pid 0 comm [ib_core]
> link mlx5_0/1 lqpn 1 type GSI state RTS sq-psn 0 pid 0 comm [ib_core]
> link mlx5_0/1 lqpn 7 type UD state RTS sq-psn 0 pid 0 comm [ib_core]
> link mlx5_0/1 lqpn 8 type UD state RTS sq-psn 0 pid 0 comm [ib_ipoib]
> link mlx5_0/1 lqpn 9 pdn 4 rqpn 0 type RC state INIT rq-psn 0 sq-psn 0
path-
> mig-state MIGRATED pid 548 comm ibv_rc_pingpong
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> rdma/res.c | 110 +++++++++++++++++++++++++++++++++++----------------
> --
> 1 file changed, 72 insertions(+), 38 deletions(-)
>
> diff --git a/rdma/res.c b/rdma/res.c
> index f941de5c..308f66c0 100644
> --- a/rdma/res.c
> +++ b/rdma/res.c
> @@ -320,6 +320,22 @@ static char *get_task_name(uint32_t pid)
> return comm;
> }
>
> +static void print_key(struct rd *rd, const char *name, uint64_t val)
> +{
> + if (rd->json_output)
> + jsonw_xint_field(rd->jw, name, val);
> + else
> + pr_out("%s 0x%" PRIx64 " ", name, val);
> +}
> +
> +static void res_print_uint(struct rd *rd, const char *name, uint64_t val)
> +{
> + if (rd->json_output)
> + jsonw_uint_field(rd->jw, name, val);
> + else
> + pr_out("%s %" PRIu64 " ", name, val);
> +}
> +
> static int res_qp_parse_cb(const struct nlmsghdr *nlh, void *data)
> {
> struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
> @@ -343,6 +359,7 @@ static int res_qp_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> uint32_t lqpn, rqpn = 0, rq_psn = 0, sq_psn;
> uint8_t type, state, path_mig_state = 0;
> uint32_t port = 0, pid = 0;
> + uint32_t pdn = 0;
> char *comm = NULL;
> int err;
>
> @@ -369,6 +386,11 @@ static int res_qp_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> if (rd_check_is_filtered(rd, "lqpn", lqpn))
> continue;
>
> + if (nla_line[RDMA_NLDEV_ATTR_RES_PDN])
> + pdn =
> mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PDN]);
> + if (rd_check_is_filtered(rd, "pdn", pdn))
> + continue;
> +
> if (nla_line[RDMA_NLDEV_ATTR_RES_RQPN]) {
> rqpn =
> mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_RQPN]);
> if (rd_check_is_filtered(rd, "rqpn", rqpn))
> @@ -428,6 +450,8 @@ static int res_qp_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> print_link(rd, idx, name, port, nla_line);
>
> print_lqpn(rd, lqpn);
> + if (nla_line[RDMA_NLDEV_ATTR_RES_PDN])
> + res_print_uint(rd, "pdn", pdn);
> print_rqpn(rd, rqpn, nla_line);
>
> print_type(rd, type);
> @@ -549,22 +573,6 @@ static int ss_ntop(struct nlattr *nla_line, char
> *addr_str, uint16_t *port)
> return 0;
> }
>
> -static void print_key(struct rd *rd, const char *name, uint64_t val)
> -{
> - if (rd->json_output)
> - jsonw_xint_field(rd->jw, name, val);
> - else
> - pr_out("%s 0x%" PRIx64 " ", name, val);
> -}
> -
> -static void res_print_uint(struct rd *rd, const char *name, uint64_t val)
> -{
> - if (rd->json_output)
> - jsonw_uint_field(rd->jw, name, val);
> - else
> - pr_out("%s %" PRIu64 " ", name, val);
> -}
> -
> static int res_cm_id_parse_cb(const struct nlmsghdr *nlh, void *data)
> {
> struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
> @@ -768,6 +776,7 @@ static int res_cq_parse_cb(const struct nlmsghdr *nlh,
> void *data)
> char *comm = NULL;
> uint32_t pid = 0;
> uint8_t poll_ctx = 0;
> + uint32_t ctxn = 0;
> uint32_t cqn = 0;
> uint64_t users;
> uint32_t cqe;
> @@ -815,6 +824,12 @@ static int res_cq_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> if (rd_check_is_filtered(rd, "cqn", cqn))
> continue;
>
> + if (nla_line[RDMA_NLDEV_ATTR_RES_CTXN])
> + ctxn = mnl_attr_get_u32(
> + nla_line[RDMA_NLDEV_ATTR_RES_CTXN]);
> + if (rd_check_is_filtered(rd, "ctxn", ctxn))
> + continue;
> +
> if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
> /* discard const from mnl_attr_get_str */
> comm = (char *)mnl_attr_get_str(
> @@ -833,6 +848,8 @@ static int res_cq_parse_cb(const struct nlmsghdr *nlh,
> void *data)
>
> if (nla_line[RDMA_NLDEV_ATTR_RES_CQN])
> res_print_uint(rd, "cqn", cqn);
> + if (nla_line[RDMA_NLDEV_ATTR_RES_CTXN])
> + res_print_uint(rd, "ctxn", ctxn);
>
> if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
> free(comm);
> @@ -866,6 +883,7 @@ static int res_mr_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> uint32_t rkey = 0, lkey = 0;
> uint64_t iova = 0, mrlen;
> char *comm = NULL;
> + uint32_t pdn = 0;
> uint32_t mrn = 0;
> uint32_t pid = 0;
> int err;
> @@ -911,6 +929,12 @@ static int res_mr_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> if (rd_check_is_filtered(rd, "mrn", mrn))
> continue;
>
> + if (nla_line[RDMA_NLDEV_ATTR_RES_PDN])
> + pdn = mnl_attr_get_u32(
> + nla_line[RDMA_NLDEV_ATTR_RES_PDN]);
> + if (rd_check_is_filtered(rd, "pdn", pdn))
> + continue;
> +
> if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
> /* discard const from mnl_attr_get_str */
> comm = (char *)mnl_attr_get_str(
> @@ -933,6 +957,9 @@ static int res_mr_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> if (nla_line[RDMA_NLDEV_ATTR_RES_MRN])
> res_print_uint(rd, "mrn", mrn);
>
> + if (nla_line[RDMA_NLDEV_ATTR_RES_PDN])
> + res_print_uint(rd, "pdn", pdn);
> +
> if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
> free(comm);
>
> @@ -964,6 +991,7 @@ static int res_pd_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> uint32_t local_dma_lkey = 0, unsafe_global_rkey = 0;
> struct nlattr *nla_line[RDMA_NLDEV_ATTR_MAX] = {};
> char *comm = NULL;
> + uint32_t ctxn = 0;
> uint32_t pid = 0;
> uint32_t pdn = 0;
> uint64_t users;
> @@ -997,7 +1025,13 @@ static int res_pd_parse_cb(const struct nlmsghdr
> *nlh, void *data)
> comm = get_task_name(pid);
> }
>
> - if (rd_check_is_filtered(rd, "pid", pid))
> + if (rd_check_is_filtered(rd, "pid", pid))
Did this add some whitespace problem?
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 03/19] rdma: Provide unique indexes for all visible objects
From: Steve Wise @ 2019-02-21 20:08 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-4-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 03/19] rdma: Provide unique indexes for
all
> visible objects
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 02/19] rdma: Remove duplicated print code
From: Steve Wise @ 2019-02-21 20:04 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-3-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 02/19] rdma: Remove duplicated print
> code
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> There is no need to keep same print functions for
> uint32_t and uint64_t, unify them into one function.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* RE: [PATCH iproute2-next v1 01/19] rdma: update uapi headers
From: Steve Wise @ 2019-02-21 20:02 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20190220072136.3840-2-leon@kernel.org>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> Sent: Wednesday, February 20, 2019 1:21 AM
> To: David Ahern <dsahern@gmail.com>
> Cc: Leon Romanovsky <leonro@mellanox.com>; netdev
> <netdev@vger.kernel.org>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH iproute2-next v1 01/19] rdma: update uapi headers
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Update rdma_netlink,h file upto kernel commit
Nit: should be "rdma_netlink.h file up to"
> f2a0e45f36b0 RDMA/nldev: Don't expose number of not-visible entries
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* [PATCH net] MAINTAINERS: mark CAIF as orphan
From: Jann Horn @ 2019-02-21 19:58 UTC (permalink / raw)
To: David S. Miller, jannh
Cc: netdev, Dmitry Tarnyagin, abi.dmitryt, linux-kernel
The listed address for the CAIF maintainer bounces with
"553 5.3.0 <dmitry.tarnyagin@lockless.no>... No such user here", and the
only existing email address of the maintainer in git history hasn't
responded in a week.
Therefore, remove the listed maintainer and mark CAIF as orphan.
Signed-off-by: Jann Horn <jannh@google.com>
---
MAINTAINERS | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 41ce5f4ad838..98457a87b238 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3390,9 +3390,8 @@ F: Documentation/media/v4l-drivers/cafe_ccic*
F: drivers/media/platform/marvell-ccic/
CAIF NETWORK LAYER
-M: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>
L: netdev@vger.kernel.org
-S: Supported
+S: Orphan
F: Documentation/networking/caif/
F: drivers/net/caif/
F: include/uapi/linux/caif/
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* Re: [PATCH bpf-next v2] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Kees Cook @ 2019-02-21 19:53 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jann Horn, Daniel Borkmann, Andy Lutomirski, Alexei Starovoitov,
Network Development
In-Reply-To: <20190221192916.2mcd4fmxbdj2j2u3@ast-mbp.dhcp.thefacebook.com>
On Thu, Feb 21, 2019 at 11:29 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Feb 21, 2019 at 01:56:53PM +0100, Jann Horn wrote:
> > On Thu, Feb 21, 2019 at 9:53 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> > > On 02/21/2019 06:31 AM, Kees Cook wrote:
> > > > On Wed, Feb 20, 2019 at 8:03 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > >>
> > > >> On Wed, Feb 20, 2019 at 3:59 PM Alexei Starovoitov
> > > >> <alexei.starovoitov@gmail.com> wrote:
> > > >>>
> > > >>> On Thu, Feb 21, 2019 at 12:01:35AM +0100, Daniel Borkmann wrote:
> > > >>>> In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> > > >>>> a check was added for BPF_PROG_RUN() that for every invocation preemption is
> > > >>>> disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this does
> > > >>>> not count for seccomp because only cBPF -> eBPF is loaded here and it does
> > > >>>> not make use of any functionality that would require this assertion. Fix this
> > > >>>> false positive by adding and using SECCOMP_RUN() variant that does not have
> > > >>>> the cant_sleep(); check.
> > > >>>>
> > > >>>> Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> > > >>>> Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
> > > >>>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> > > >>>> Acked-by: Kees Cook <keescook@chromium.org>
> > > >>>
> > > >>> Applied, Thanks
> > > >>
> > > >> Actually I think it's a wrong approach to go long term.
> > > >> I'm thinking to revert it.
> > > >> I think it's better to disable preemption for duration of
> > > >> seccomp cbpf prog.
> > > >> It's short and there is really no reason for it to be preemptible.
> > > >> When seccomp switches to ebpf we'll have this weird inconsistency.
> > > >> Let's just disable preemption for seccomp as well.
> > > >
> > > > A lot of changes will be needed for seccomp ebpf -- not the least of
> > > > which is convincing me there is a use-case. ;)
> > > >
> > > > But the main issue is that I'm not a huge fan of dropping two
> > > > barriers() across syscall entry. That seems pretty heavy-duty for
> > > > something that is literally not needed right now.
> > >
> > > Yeah, I think it's okay to add once actually technically needed. Last
> > > time I looked, if I recall correctly, at least Chrome installs some
> > > heavy duty seccomp programs that go close to prog limit.
> >
> > Half of that is probably because that seccomp BPF code is so
> > inefficient, though.
> >
> > This snippet shows that those programs constantly recheck the high
> > halves of arguments:
> >
> > Some of the generated code is pointless because all reachable code
> > from that point on has the same outcome (the last "ret ALLOW" in the
> > following sample is unreachable because they've already checked that
> > the high bit of the low half is set, so the low half can't be 3):
>
> and with ebpf these optimizations will be available for free
> because llvm will remove unnecessary loads and simplify branches.
> There is no technical reason not to use ebpf in seccomp.
>
> When we discussed preemption of classic vs extended in socket filters
> context we agreed to make it a requirement that preemption must be
> disabled though it's not strictly necessary. RX side of socket filters
> was already non-preempt while TX was preemptible.
> We must not make an exception of this rule for seccomp.
> Hence I've reverted this commit.
>
> Here is the actual fix for seccomp:
> From: Alexei Starovoitov <ast@kernel.org>
> Date: Thu, 21 Feb 2019 10:40:14 -0800
> Subject: [PATCH] seccomp, bpf: disable preemption before calling into bpf prog
>
> All BPF programs must be called with preemption disabled.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> kernel/seccomp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index e815781ed751..a43c601ac252 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -267,6 +267,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
> * All filters in the list are evaluated and the lowest BPF return
> * value always takes priority (ignoring the DATA).
> */
> + preempt_disable();
> for (; f; f = f->prev) {
> u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
>
> @@ -275,6 +276,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
> *match = f;
> }
> }
> + preempt_enable();
> return ret;
> }
> #endif /* CONFIG_SECCOMP_FILTER */
> --
>
> Doing per-cpu increment of cache hot data is practically free and it makes seccomp
> play by the rules.
Other accesses should dominate the run time, yes. I'm still not a big
fan of unconditionally adding this, but I won't NAK. :P
--
Kees Cook
^ permalink raw reply
* Re: [RFC v1 01/19] net/i40e: Add peer register/unregister to struct i40e_netdev_priv
From: Jason Gunthorpe @ 2019-02-21 19:35 UTC (permalink / raw)
To: Saleem, Shiraz
Cc: dledford@redhat.com, davem@davemloft.net,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
Ismail, Mustafa, Kirsher, Jeffrey T, Patil, Kiran,
Ertman, David M
In-Reply-To: <9DD61F30A802C4429A01CA4200E302A7A5A471B8@fmsmsx124.amr.corp.intel.com>
On Thu, Feb 21, 2019 at 02:19:33AM +0000, Saleem, Shiraz wrote:
> >Subject: Re: [RFC v1 01/19] net/i40e: Add peer register/unregister to struct
> >i40e_netdev_priv
> >
> >On Fri, Feb 15, 2019 at 11:10:48AM -0600, Shiraz Saleem wrote:
> >> Expose the register/unregister function pointers in the struct
> >> i40e_netdev_priv which is accesible via the netdev_priv() interface in
> >> the RDMA driver. On a netdev notification in the RDMA driver, the
> >> appropriate LAN driver register/unregister functions are invoked from
> >> the struct i40e_netdev_priv structure,
> >
> >Why? In later patches we get an entire device_add() based thing. Why do you
> >need two things?
> >
> >The RDMA driver should bind to the thing that device_add created and from there
> >reliably get the netdev. It should not listen to netdev notifiers for attachment.
>
> In the new IDC mechanism between ice<->irdma, the LAN driver setups up the
> device for us and attaches it to a software bus via device_add() based mechanism.
> However, RDMA driver binds to the device only when the LAN 'register' function is
> called in irdma.
That doesn't make sense. The PCI driver should always create the
required struct device attachment point when attachment is
becomes possible.
> There is no ordering guarantee in which irdma, i40e and ice modules load.
> The netdev notifier is for the case where the irdma loads before
> i40e or ice.
You are supposed to use the driver core to handle this ordering.
The pci driver creates the attachment points in the correct order,
when they are ready for use, and the driver core will automatically
attach registered device drivers to the attachement points, no matter
the module load loader.
You will have a netdev and a rdma attachment point, sounds like the
RDMA one is created once the netdev is happy.
Maybe what you are missing is a struct device_driver?
Jason
^ permalink raw reply
* Re: stmmac / meson8b-dwmac
From: Simon Huelck @ 2019-02-21 19:34 UTC (permalink / raw)
To: Jerome Brunet, Jose Abreu, Martin Blumenstingl
Cc: linux-amlogic, Gpeppe.cavallaro, alexandre.torgue,
Emiliano Ingrassia, netdev
In-Reply-To: <d7de65c614ee788152300f6d3799fd537b438496.camel@baylibre.com>
Am 21.02.2019 um 18:46 schrieb Jerome Brunet:
> On Thu, 2019-02-21 at 18:27 +0100, Simon Huelck wrote:
>> Hi,
>>
>>
>>
>> this was changed recently, with a patch for the EEE stuff , see here:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc7&id=e35e26b26e955c53e61c154ba26b9bb15da6b858
> Hu, I was not aware this finally went through. Good !
> As explained in the patch and by Jose, the GMAC should be using IRQ_LEVEL.
>
> The realtek PHY has EEE enabled by default. Having this enabled generates a
> lot of (Low Power) Interrupts.
>
> Previously, when the GMAC used IRQ_EDGE. Because it is wrong, we would
> eventually miss an IRQ and the interface would just die. Unfortunately, it was
> not that easy find out.
>
> 2 years ago, we just noticed that disabling EEE would make the failure go
> away. Forcing this EEE feature off through DT was merely a work around.
>
> Now that the real cause of the problem is known, there is no reason to keep
> this hack around.
>
> Whether EEE adds a performance penality and why, is another topic.
> As Jose pointed out, you can disable EEE at runtime, using ethtool.
>
> Jerome
>
Hi,
i disabled EEE via ethtool and via the .dtb , but the performance
penalty stays. Kernel 4.14 still gives me the former "good" performance.
regards,
Simon
^ permalink raw reply
* [net 2/5] i40e: fix potential RX buffer starvation for AF_XDP
From: Jeff Kirsher @ 2019-02-21 19:33 UTC (permalink / raw)
To: davem
Cc: Magnus Karlsson, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190221193319.22318-1-jeffrey.t.kirsher@intel.com>
From: Magnus Karlsson <magnus.karlsson@intel.com>
When the RX rings are created they are also populated with buffers
so that packets can be received. Usually these are kernel buffers,
but for AF_XDP in zero-copy mode, these are user-space buffers and
in this case the application might not have sent down any buffers
to the driver at this point. And if no buffers are allocated at ring
creation time, no packets can be received and no interrupts will be
generated so the NAPI poll function that allocates buffers to the
rings will never get executed.
To rectify this, we kick the NAPI context of any queue with an
attached AF_XDP zero-copy socket in two places in the code. Once
after an XDP program has loaded and once after the umem is registered.
This take care of both cases: XDP program gets loaded first then AF_XDP
socket is created, and the reverse, AF_XDP socket is created first,
then XDP program is loaded.
Fixes: 0a714186d3c0 ("i40e: add AF_XDP zero-copy Rx support")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 13 ++++++++++++-
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 5 +++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index f52e2c46e6a7..3a0990de81c1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3289,8 +3289,11 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
i40e_alloc_rx_buffers_zc(ring, I40E_DESC_UNUSED(ring)) :
!i40e_alloc_rx_buffers(ring, I40E_DESC_UNUSED(ring));
if (!ok) {
+ /* Log this in case the user has forgotten to give the kernel
+ * any buffers, even later in the application.
+ */
dev_info(&vsi->back->pdev->dev,
- "Failed allocate some buffers on %sRx ring %d (pf_q %d)\n",
+ "Failed to allocate some buffers on %sRx ring %d (pf_q %d)\n",
ring->xsk_umem ? "UMEM enabled " : "",
ring->queue_index, pf_q);
}
@@ -11895,6 +11898,14 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi,
if (old_prog)
bpf_prog_put(old_prog);
+ /* Kick start the NAPI context if there is an AF_XDP socket open
+ * on that queue id. This so that receiving will start.
+ */
+ if (need_reset && prog)
+ for (i = 0; i < vsi->num_queue_pairs; i++)
+ if (vsi->xdp_rings[i]->xsk_umem)
+ (void)i40e_xsk_async_xmit(vsi->netdev, i);
+
return 0;
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 870cf654e436..3827f16e6923 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -183,6 +183,11 @@ static int i40e_xsk_umem_enable(struct i40e_vsi *vsi, struct xdp_umem *umem,
err = i40e_queue_pair_enable(vsi, qid);
if (err)
return err;
+
+ /* Kick start the NAPI context so that receiving will start */
+ err = i40e_xsk_async_xmit(vsi->netdev, qid);
+ if (err)
+ return err;
}
return 0;
--
2.20.1
^ permalink raw reply related
* [net 0/5][pull request] Intel Wired LAN Driver Fixes 2019-02-21
From: Jeff Kirsher @ 2019-02-21 19:33 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann
This series contains fixes to ixgbe and i40e.
Majority of the fixes are to resolve XDP issues found in both drivers,
there is only one fix which is not XDP related. That one fix resolves
an issue seen on older 10GbE devices, where UDP traffic was either being
dropped or being transmitted out of order when the bit to enable L3/L4
filtering for transmit switched packets is enabled on older devices that
did not support this option.
Magnus fixes an XDP issue for both ixgbe and i40e, where receive rings
are created but no buffers are allocated for AF_XDP in zero-copy mode,
so no packets can be received and no interrupts will be generated so
that NAPI poll function that allocates buffers to the rings will never
get executed.
Björn fixes a race in XDP xmit ring cleanup for i40e, where
ndo_xdp_xmit() must be taken into consideration. Added a
synchronize_rcu() to wait for napi(s) before clearing the queue.
Jan fixes a ixgbe AF_XDP zero-copy transmit issue which can cause a
reset to be triggered, so add a check to ensure that netif carrier is
'ok' before trying to transmit packets.
The following are changes since commit ae3b564179bfd06f32d051b9e5d72ce4b2a07c37:
missing barriers in some of unix_sock ->addr and ->path accesses
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue 40GbE
Björn Töpel (1):
i40e: fix XDP_REDIRECT/XDP xmit ring cleanup race
Jan Sokolowski (1):
ixgbe: don't do any AF_XDP zero-copy transmit if netif is not OK
Jeff Kirsher (1):
ixgbe: fix older devices that do not support IXGBE_MRQC_L3L4TXSWEN
Magnus Karlsson (2):
i40e: fix potential RX buffer starvation for AF_XDP
ixgbe: fix potential RX buffer starvation for AF_XDP
drivers/net/ethernet/intel/i40e/i40e_main.c | 27 ++++++++++++++++---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 4 ++-
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 5 ++++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 19 ++++++++++---
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 15 ++++++++---
5 files changed, 60 insertions(+), 10 deletions(-)
--
2.20.1
^ permalink raw reply
* [net 4/5] i40e: fix XDP_REDIRECT/XDP xmit ring cleanup race
From: Jeff Kirsher @ 2019-02-21 19:33 UTC (permalink / raw)
To: davem
Cc: Björn Töpel, netdev, nhorman, sassmann,
Maciej Fijalkowski, Jeff Kirsher
In-Reply-To: <20190221193319.22318-1-jeffrey.t.kirsher@intel.com>
From: Björn Töpel <bjorn.topel@intel.com>
When the driver clears the XDP xmit ring due to re-configuration or
teardown, in-progress ndo_xdp_xmit must be taken into consideration.
The ndo_xdp_xmit function is typically called from a NAPI context that
the driver does not control. Therefore, we must be careful not to
clear the XDP ring, while the call is on-going. This patch adds a
synchronize_rcu() to wait for napi(s) (preempt-disable regions and
softirqs), prior clearing the queue. Further, the __I40E_CONFIG_BUSY
flag is checked in the ndo_xdp_xmit implementation to avoid touching
the XDP xmit queue during re-configuration.
Fixes: d9314c474d4f ("i40e: add support for XDP_REDIRECT")
Fixes: 123cecd427b6 ("i40e: added queue pair disable/enable functions")
Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 14 ++++++++++++--
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 4 +++-
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3a0990de81c1..e4ff531db14a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6728,8 +6728,13 @@ void i40e_down(struct i40e_vsi *vsi)
for (i = 0; i < vsi->num_queue_pairs; i++) {
i40e_clean_tx_ring(vsi->tx_rings[i]);
- if (i40e_enabled_xdp_vsi(vsi))
+ if (i40e_enabled_xdp_vsi(vsi)) {
+ /* Make sure that in-progress ndo_xdp_xmit
+ * calls are completed.
+ */
+ synchronize_rcu();
i40e_clean_tx_ring(vsi->xdp_rings[i]);
+ }
i40e_clean_rx_ring(vsi->rx_rings[i]);
}
@@ -11966,8 +11971,13 @@ static void i40e_queue_pair_reset_stats(struct i40e_vsi *vsi, int queue_pair)
static void i40e_queue_pair_clean_rings(struct i40e_vsi *vsi, int queue_pair)
{
i40e_clean_tx_ring(vsi->tx_rings[queue_pair]);
- if (i40e_enabled_xdp_vsi(vsi))
+ if (i40e_enabled_xdp_vsi(vsi)) {
+ /* Make sure that in-progress ndo_xdp_xmit calls are
+ * completed.
+ */
+ synchronize_rcu();
i40e_clean_tx_ring(vsi->xdp_rings[queue_pair]);
+ }
i40e_clean_rx_ring(vsi->rx_rings[queue_pair]);
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index a7e14e98889f..6c97667d20ef 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -3709,6 +3709,7 @@ int i40e_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
struct i40e_netdev_priv *np = netdev_priv(dev);
unsigned int queue_index = smp_processor_id();
struct i40e_vsi *vsi = np->vsi;
+ struct i40e_pf *pf = vsi->back;
struct i40e_ring *xdp_ring;
int drops = 0;
int i;
@@ -3716,7 +3717,8 @@ int i40e_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
if (test_bit(__I40E_VSI_DOWN, vsi->state))
return -ENETDOWN;
- if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
+ if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs ||
+ test_bit(__I40E_CONFIG_BUSY, pf->state))
return -ENXIO;
if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
--
2.20.1
^ permalink raw reply related
* [net 5/5] ixgbe: don't do any AF_XDP zero-copy transmit if netif is not OK
From: Jeff Kirsher @ 2019-02-21 19:33 UTC (permalink / raw)
To: davem; +Cc: Jan Sokolowski, netdev, nhorman, sassmann, Jeff Kirsher
In-Reply-To: <20190221193319.22318-1-jeffrey.t.kirsher@intel.com>
From: Jan Sokolowski <jan.sokolowski@intel.com>
An issue has been found while testing zero-copy XDP that
causes a reset to be triggered. As it takes some time to
turn the carrier on after setting zc, and we already
start trying to transmit some packets, watchdog considers
this as an erroneous state and triggers a reset.
Don't do any work if netif carrier is not OK.
Fixes: 8221c5eba8c13 (ixgbe: add AF_XDP zero-copy Tx support)
Signed-off-by: Jan Sokolowski <jan.sokolowski@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 654ae92342ea..36a8879536a4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -642,7 +642,8 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
dma_addr_t dma;
while (budget-- > 0) {
- if (unlikely(!ixgbe_desc_unused(xdp_ring))) {
+ if (unlikely(!ixgbe_desc_unused(xdp_ring)) ||
+ !netif_carrier_ok(xdp_ring->netdev)) {
work_done = false;
break;
}
--
2.20.1
^ permalink raw reply related
* [net 3/5] ixgbe: fix potential RX buffer starvation for AF_XDP
From: Jeff Kirsher @ 2019-02-21 19:33 UTC (permalink / raw)
To: davem
Cc: Magnus Karlsson, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190221193319.22318-1-jeffrey.t.kirsher@intel.com>
From: Magnus Karlsson <magnus.karlsson@intel.com>
When the RX rings are created they are also populated with buffers so
that packets can be received. Usually these are kernel buffers, but
for AF_XDP in zero-copy mode, these are user-space buffers and in this
case the application might not have sent down any buffers to the
driver at this point. And if no buffers are allocated at ring creation
time, no packets can be received and no interrupts will be generated so
the NAPI poll function that allocates buffers to the rings will never
get executed.
To rectify this, we kick the NAPI context of any queue with an
attached AF_XDP zero-copy socket in two places in the code. Once after
an XDP program has loaded and once after the umem is registered. This
take care of both cases: XDP program gets loaded first then AF_XDP
socket is created, and the reverse, AF_XDP socket is created first,
then XDP program is loaded.
Fixes: d0bcacd0a130 ("ixgbe: add AF_XDP zero-copy Rx support")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 12 +++++++++++-
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 12 ++++++++++--
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 3cbb7e0324fd..cb35d8202572 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10228,6 +10228,7 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
int i, frame_size = dev->mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
struct ixgbe_adapter *adapter = netdev_priv(dev);
struct bpf_prog *old_prog;
+ bool need_reset;
if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)
return -EINVAL;
@@ -10250,9 +10251,10 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
return -ENOMEM;
old_prog = xchg(&adapter->xdp_prog, prog);
+ need_reset = (!!prog != !!old_prog);
/* If transitioning XDP modes reconfigure rings */
- if (!!prog != !!old_prog) {
+ if (need_reset) {
int err = ixgbe_setup_tc(dev, adapter->hw_tcs);
if (err) {
@@ -10268,6 +10270,14 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
if (old_prog)
bpf_prog_put(old_prog);
+ /* Kick start the NAPI context if there is an AF_XDP socket open
+ * on that queue id. This so that receiving will start.
+ */
+ if (need_reset && prog)
+ for (i = 0; i < adapter->num_rx_queues; i++)
+ if (adapter->xdp_ring[i]->xsk_umem)
+ (void)ixgbe_xsk_async_xmit(adapter->netdev, i);
+
return 0;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 65c3e2c979d4..654ae92342ea 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -144,11 +144,19 @@ static int ixgbe_xsk_umem_enable(struct ixgbe_adapter *adapter,
ixgbe_txrx_ring_disable(adapter, qid);
err = ixgbe_add_xsk_umem(adapter, umem, qid);
+ if (err)
+ return err;
- if (if_running)
+ if (if_running) {
ixgbe_txrx_ring_enable(adapter, qid);
- return err;
+ /* Kick start the NAPI context so that receiving will start */
+ err = ixgbe_xsk_async_xmit(adapter->netdev, qid);
+ if (err)
+ return err;
+ }
+
+ return 0;
}
static int ixgbe_xsk_umem_disable(struct ixgbe_adapter *adapter, u16 qid)
--
2.20.1
^ permalink raw reply related
* [net 1/5] ixgbe: fix older devices that do not support IXGBE_MRQC_L3L4TXSWEN
From: Jeff Kirsher @ 2019-02-21 19:33 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, Andrew Bowers
In-Reply-To: <20190221193319.22318-1-jeffrey.t.kirsher@intel.com>
The enabling L3/L4 filtering for transmit switched packets for all
devices caused unforeseen issue on older devices when trying to send UDP
traffic in an ordered sequence. This bit was originally intended for X550
devices, which supported this feature, so limit the scope of this bit to
only X550 devices.
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index daff8183534b..3cbb7e0324fd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3953,8 +3953,11 @@ static void ixgbe_setup_mrqc(struct ixgbe_adapter *adapter)
else
mrqc = IXGBE_MRQC_VMDQRSS64EN;
- /* Enable L3/L4 for Tx Switched packets */
- mrqc |= IXGBE_MRQC_L3L4TXSWEN;
+ /* Enable L3/L4 for Tx Switched packets only for X550,
+ * older devices do not support this feature
+ */
+ if (hw->mac.type >= ixgbe_mac_X550)
+ mrqc |= IXGBE_MRQC_L3L4TXSWEN;
} else {
if (tcs > 4)
mrqc = IXGBE_MRQC_RTRSS8TCEN;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next v2] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Alexei Starovoitov @ 2019-02-21 19:29 UTC (permalink / raw)
To: Jann Horn
Cc: Daniel Borkmann, Kees Cook, Andy Lutomirski, Alexei Starovoitov,
Network Development
In-Reply-To: <CAG48ez1Edngv8LLb_svz8Kq3vQKf7-a_Q1whyHfXiTDP8FO+rw@mail.gmail.com>
On Thu, Feb 21, 2019 at 01:56:53PM +0100, Jann Horn wrote:
> On Thu, Feb 21, 2019 at 9:53 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> > On 02/21/2019 06:31 AM, Kees Cook wrote:
> > > On Wed, Feb 20, 2019 at 8:03 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > >>
> > >> On Wed, Feb 20, 2019 at 3:59 PM Alexei Starovoitov
> > >> <alexei.starovoitov@gmail.com> wrote:
> > >>>
> > >>> On Thu, Feb 21, 2019 at 12:01:35AM +0100, Daniel Borkmann wrote:
> > >>>> In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> > >>>> a check was added for BPF_PROG_RUN() that for every invocation preemption is
> > >>>> disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this does
> > >>>> not count for seccomp because only cBPF -> eBPF is loaded here and it does
> > >>>> not make use of any functionality that would require this assertion. Fix this
> > >>>> false positive by adding and using SECCOMP_RUN() variant that does not have
> > >>>> the cant_sleep(); check.
> > >>>>
> > >>>> Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> > >>>> Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
> > >>>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> > >>>> Acked-by: Kees Cook <keescook@chromium.org>
> > >>>
> > >>> Applied, Thanks
> > >>
> > >> Actually I think it's a wrong approach to go long term.
> > >> I'm thinking to revert it.
> > >> I think it's better to disable preemption for duration of
> > >> seccomp cbpf prog.
> > >> It's short and there is really no reason for it to be preemptible.
> > >> When seccomp switches to ebpf we'll have this weird inconsistency.
> > >> Let's just disable preemption for seccomp as well.
> > >
> > > A lot of changes will be needed for seccomp ebpf -- not the least of
> > > which is convincing me there is a use-case. ;)
> > >
> > > But the main issue is that I'm not a huge fan of dropping two
> > > barriers() across syscall entry. That seems pretty heavy-duty for
> > > something that is literally not needed right now.
> >
> > Yeah, I think it's okay to add once actually technically needed. Last
> > time I looked, if I recall correctly, at least Chrome installs some
> > heavy duty seccomp programs that go close to prog limit.
>
> Half of that is probably because that seccomp BPF code is so
> inefficient, though.
>
> This snippet shows that those programs constantly recheck the high
> halves of arguments:
>
> Some of the generated code is pointless because all reachable code
> from that point on has the same outcome (the last "ret ALLOW" in the
> following sample is unreachable because they've already checked that
> the high bit of the low half is set, so the low half can't be 3):
and with ebpf these optimizations will be available for free
because llvm will remove unnecessary loads and simplify branches.
There is no technical reason not to use ebpf in seccomp.
When we discussed preemption of classic vs extended in socket filters
context we agreed to make it a requirement that preemption must be
disabled though it's not strictly necessary. RX side of socket filters
was already non-preempt while TX was preemptible.
We must not make an exception of this rule for seccomp.
Hence I've reverted this commit.
Here is the actual fix for seccomp:
From: Alexei Starovoitov <ast@kernel.org>
Date: Thu, 21 Feb 2019 10:40:14 -0800
Subject: [PATCH] seccomp, bpf: disable preemption before calling into bpf prog
All BPF programs must be called with preemption disabled.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/seccomp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index e815781ed751..a43c601ac252 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -267,6 +267,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
* All filters in the list are evaluated and the lowest BPF return
* value always takes priority (ignoring the DATA).
*/
+ preempt_disable();
for (; f; f = f->prev) {
u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
@@ -275,6 +276,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
*match = f;
}
}
+ preempt_enable();
return ret;
}
#endif /* CONFIG_SECCOMP_FILTER */
--
Doing per-cpu increment of cache hot data is practically free and it makes seccomp
play by the rules.
I'm working on another set of patches that will introduce bpf program stats as:
#define BPF_RPOG_RUN(prog, ctx) ({ \
+ u32 ret; \
+ cant_sleep(); \
+ if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
+ u64 start = sched_clock(); \
+ ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
+ this_cpu_inc(prog->aux->stats->cnt); \
+ this_cpu_add(prog->aux->stats->nsecs, \
+ sched_clock() - start); \
+ } else { \
+ ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
+ } \
+ ret; })
and seccomp progs will get their stats just like socket filters and the rest of
classic and extended bpf.
^ permalink raw reply related
* [PATCH V2] test_sysctl: add proc_do_large_bitmap test function
From: Eric Sandeen @ 2019-02-21 19:16 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
In-Reply-To: <a8caf97b-9dbb-5846-7e93-242226a6c8c7@redhat.com>
Add test to build up bitmap range string and test the bitmap
proc handler.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: set rc=0 for test success
however this still fails indeterminately for me. Debugging, if I
save off the test write string and the read string, re-writing it to
the handler works fine. My standalone test also works fine for 1000
iterations. So I don't know what's going on here.
nb: test_modprobe & load_req_mod fail for me before we ever
get to this test, but commenting them out, my test runs as expected.
I'm new to this script, so careful review would be wise.
Thanks,
-Eric
diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 584eb8ea780a..e531074f94bd 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -37,6 +37,7 @@ ALL_TESTS="$ALL_TESTS 0002:1:1"
ALL_TESTS="$ALL_TESTS 0003:1:1"
ALL_TESTS="$ALL_TESTS 0004:1:1"
ALL_TESTS="$ALL_TESTS 0005:3:1"
+ALL_TESTS="$ALL_TESTS 0006:3:1"
test_modprobe()
{
@@ -149,6 +150,9 @@ reset_vals()
string_0001)
VAL="(none)"
;;
+ bitmap_0001)
+ VAL=""
+ ;;
*)
;;
esac
@@ -548,6 +552,48 @@ run_stringtests()
test_rc
}
+run_bitmaptest() {
+ # Total length of bitmaps string to use, a bit under
+ # the maximum input size of the test node
+ LENGTH=$((RANDOM % 65000))
+
+ # First bit to set
+ BIT=$((RANDOM % 1024))
+
+ # String containing our list of bits to set
+ TEST_STR=$BIT
+
+ # build up the string
+ while [ "${#TEST_STR}" -le "$LENGTH" ]; do
+ # Make sure next entry is discontiguous,
+ # skip ahead at least 2
+ BIT=$((BIT + $((2 + RANDOM % 10))))
+
+ # Add new bit to the list
+ TEST_STR="${TEST_STR},${BIT}"
+
+ # Randomly make it a range
+ if [ "$((RANDOM % 2))" -eq "1" ]; then
+ RANGE_END=$((BIT + $((1 + RANDOM % 10))))
+ TEST_STR="${TEST_STR}-${RANGE_END}"
+ BIT=$RANGE_END
+ fi
+ done
+
+ echo -n "Checking bitmap handler... "
+ echo $TEST_STR > /tmp/test_str
+ echo -n $TEST_STR > $TARGET 2> /dev/null
+
+ if verify "${TARGET}"; then
+ echo "FAIL" >&2
+ rc=1
+ else
+ echo "ok"
+ rc=0
+ fi
+ test_rc
+}
+
sysctl_test_0001()
{
TARGET="${SYSCTL}/int_0001"
@@ -605,6 +651,14 @@ sysctl_test_0005()
run_limit_digit_int_array
}
+sysctl_test_0006()
+{
+ TARGET="${SYSCTL}/bitmap_0001"
+ reset_vals
+ ORIG=""
+ run_bitmaptest
+}
+
list_tests()
{
echo "Test ID list:"
@@ -618,6 +672,7 @@ list_tests()
echo "0003 x $(get_test_count 0003) - tests proc_dointvec()"
echo "0004 x $(get_test_count 0004) - tests proc_douintvec()"
echo "0005 x $(get_test_count 0005) - tests proc_douintvec() array"
+ echo "0006 x $(get_test_count 0006) - tests proc_do_large_bitmap"
}
test_reqs
^ permalink raw reply related
* [PATCH bpf-next] selftests/bpf: make sure signal interrupts BPF_PROG_TEST_RUN
From: Stanislav Fomichev @ 2019-02-21 19:11 UTC (permalink / raw)
To: netdev; +Cc: davem, ast, daniel, Stanislav Fomichev
Simple test that I used to reproduce the issue in the previous commit:
Do BPF_PROG_TEST_RUN with max iterations, each program is 4096 simple
move instructions. File alarm in 0.1 second and check that
bpf_prog_test_run is interrupted (i.e. test doesn't hang).
Note: reposting this for bpf-next to avoid linux-next conflict. In this
version I test both BPF_PROG_TYPE_SOCKET_FILTER (which uses generic
bpf_test_run implementation) and BPF_PROG_TYPE_FLOW_DISSECTOR (which has
it own loop with preempt handling in bpf_prog_test_run_flow_dissector).
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_progs.c | 44 ++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index c52bd90fbb34..c59d2e015d16 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -12,6 +12,7 @@
#include <stdlib.h>
#include <stdarg.h>
#include <time.h>
+#include <signal.h>
#include <linux/types.h>
typedef __u16 __sum16;
@@ -28,6 +29,7 @@ typedef __u16 __sum16;
#include <sys/ioctl.h>
#include <sys/wait.h>
#include <sys/types.h>
+#include <sys/time.h>
#include <fcntl.h>
#include <pthread.h>
#include <linux/bpf.h>
@@ -2108,6 +2110,46 @@ static void test_map_lock(void)
bpf_object__close(obj);
}
+static void sigalrm_handler(int s) {}
+static struct sigaction sigalrm_action = {
+ .sa_handler = sigalrm_handler,
+};
+
+static void test_signal_pending(enum bpf_prog_type prog_type)
+{
+ struct bpf_insn prog[4096];
+ struct itimerval timeo = {
+ .it_value.tv_usec = 100000, /* 100ms */
+ };
+ __u32 duration, retval;
+ int prog_fd;
+ int err;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(prog); i++)
+ prog[i] = BPF_ALU64_IMM(BPF_MOV, BPF_REG_0, 0);
+ prog[ARRAY_SIZE(prog) - 1] = BPF_EXIT_INSN();
+
+ prog_fd = bpf_load_program(prog_type, prog, ARRAY_SIZE(prog),
+ "GPL", 0, NULL, 0);
+ CHECK(prog_fd < 0, "test-run", "errno %d\n", errno);
+
+ err = sigaction(SIGALRM, &sigalrm_action, NULL);
+ CHECK(err, "test-run-signal-sigaction", "errno %d\n", errno);
+
+ err = setitimer(ITIMER_REAL, &timeo, NULL);
+ CHECK(err, "test-run-signal-timer", "errno %d\n", errno);
+
+ err = bpf_prog_test_run(prog_fd, 0xffffffff, &pkt_v4, sizeof(pkt_v4),
+ NULL, NULL, &retval, &duration);
+ CHECK(duration > 500000000, /* 500ms */
+ "test-run-signal-duration",
+ "duration %dns > 500ms\n",
+ duration);
+
+ signal(SIGALRM, SIG_DFL);
+}
+
int main(void)
{
srand(time(NULL));
@@ -2138,6 +2180,8 @@ int main(void)
test_flow_dissector();
test_spinlock();
test_map_lock();
+ test_signal_pending(BPF_PROG_TYPE_SOCKET_FILTER);
+ test_signal_pending(BPF_PROG_TYPE_FLOW_DISSECTOR);
printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* Re: [PATCH] test_sysctl: add proc_do_large_bitmap test function
From: Eric Sandeen @ 2019-02-21 19:01 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
In-Reply-To: <a8caf97b-9dbb-5846-7e93-242226a6c8c7@redhat.com>
On 2/21/19 12:43 PM, Eric Sandeen wrote:
> Add test to build up bitmap range string and test the bitmap
> proc handler.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> nb: test_modprobe & load_req_mod fail for me before we ever
> get to this test, but commenting them out, my test runs as expected.
> I'm new to this script, so careful review would be wise. ;)
>
> Thanks,
> -Eric
Gah, running this in different ways, it's not doing the right thing.
Hang on, V2 pending...
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox