On 8/24/2021 3:42 AM, Jason Gunthorpe wrote:
On Wed, Aug 18, 2021 at 02:24:24PM +0300, Mark Zhang wrote:
From: Aharon Landau <aharonl@xxxxxxxxxx>
This patch adds the ability to add/remove optional counter to a link
through RDMA netlink. Limit it to users with ADMIN capability only.
Examples:
$ sudo rdma statistic add link rocep8s0f0/1 optional-set cc_rx_ce_pkts
$ sudo rdma statistic remove link rocep8s0f0/1 optional-set cc_rx_ce_pkts
Signed-off-by: Aharon Landau <aharonl@xxxxxxxxxx>
Signed-off-by: Neta Ostrovsky <netao@xxxxxxxxxx>
Signed-off-by: Mark Zhang <markzhang@xxxxxxxxxx>
drivers/infiniband/core/counters.c | 50 ++++++++++++++++
drivers/infiniband/core/device.c | 2 +
drivers/infiniband/core/nldev.c | 93 ++++++++++++++++++++++++++++++
include/rdma/ib_verbs.h | 7 +++
include/rdma/rdma_counter.h | 4 ++
include/rdma/rdma_netlink.h | 1 +
include/uapi/rdma/rdma_netlink.h | 9 +++
7 files changed, 166 insertions(+)
diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
...
+static int nldev_stat_set_op_stat(struct sk_buff *skb,
+ struct nlmsghdr *nlh,
+ struct netlink_ext_ack *extack,
+ bool cmd_add)
+{
...
+
+ nlh = nlmsg_put(msg, NETLINK_CB(skb).portid, nlh->nlmsg_seq,
+ RDMA_NL_GET_TYPE(RDMA_NL_NLDEV,
+ (cmd_add ?
+ RDMA_NLDEV_CMD_STAT_ADD_OPCOUNTER :
+ RDMA_NLDEV_CMD_STAT_REMOVE_OPCOUNTER)),
+ 0, 0);
+
+ if (cmd_add)
+ ret = rdma_opcounter_add(device, port, opcounter);
+ else
+ ret = rdma_opcounter_remove(device, port, opcounter);
+ if (ret)
+ goto err_msg;
+
+ nlmsg_end(msg, nlh);
Shouldn't the netlink message for a 'set' always return the current
value of the thing being set on return? Eg the same output that GET
would generate?
May I ask why can't just return an error code?
+static int nldev_stat_add_op_stat_doit(struct sk_buff *skb,
+ struct nlmsghdr *nlh,
+ struct netlink_ext_ack *extack)
+{
+ return nldev_stat_set_op_stat(skb, nlh, extack, true);
+}
+
+static int nldev_stat_remove_op_stat_doit(struct sk_buff *skb,
+ struct nlmsghdr *nlh,
+ struct netlink_ext_ack *extack)
+{
+ return nldev_stat_set_op_stat(skb, nlh, extack, false);
+}
+
static int nldev_stat_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
struct netlink_ext_ack *extack)
{
@@ -2342,6 +2427,14 @@ static const struct rdma_nl_cbs nldev_cb_table[RDMA_NLDEV_NUM_OPS] = {
.dump = nldev_res_get_mr_raw_dumpit,
.flags = RDMA_NL_ADMIN_PERM,
},
+ [RDMA_NLDEV_CMD_STAT_ADD_OPCOUNTER] = {
+ .doit = nldev_stat_add_op_stat_doit,
+ .flags = RDMA_NL_ADMIN_PERM,
+ },
+ [RDMA_NLDEV_CMD_STAT_REMOVE_OPCOUNTER] = {
+ .doit = nldev_stat_remove_op_stat_doit,
+ .flags = RDMA_NL_ADMIN_PERM,
+ },
};
And here I wonder if this is the cannonical way to manipulate lists of
strings in netlink? I'm trying to think of another case like this, did
you reference something?
For add/remove, we only support one op-counter at one time (for
simplicity), so it's just a string, not a list of string.
This is supported:
# rdma stat add link mlx5_0/1 optional-set cc_rx_ce_pkts
This is not supported:
# rdma stat add link mlx5_0/1 optional-set cc_rx_ce_pkts cc_tx_cnp_pkts
Are you sure this shouldn't be done via some set on some counter
object?
Currently we don't support do on a counter object, just per-port.
Jason
