diff mbox series

[iproute2,2/7] devlink: mnlg: Add support for extended ack

Message ID 1518598522-27713-3-git-send-email-arkadis@mellanox.com
State Accepted, archived
Delegated to: stephen hemminger
Headers show
Series Add support for devlink resource abstraction | expand

Commit Message

Arkadi Sharshevsky Feb. 14, 2018, 8:55 a.m. UTC 
Add support for extended ack.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/mnlg.c       | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 include/libnetlink.h |  1 +
 lib/libnetlink.c     |  4 ++--
 3 files changed, 54 insertions(+), 4 deletions(-)

Comments

Stephen Hemminger Feb. 14, 2018, 3:12 p.m. UTC | #1 
On Wed, 14 Feb 2018 10:55:17 +0200
Arkadi Sharshevsky <arkadis@mellanox.com> wrote:

> +static mnl_cb_t mnlg_cb_array[NLMSG_MIN_TYPE] = {
> +	[NLMSG_NOOP]	= mnlg_cb_noop,
> +	[NLMSG_ERROR]	= mnlg_cb_error,
> +	[NLMSG_DONE]	= mnlg_cb_stop,
> +	[NLMSG_OVERRUN]	= mnlg_cb_noop,
> +};
> +

Could be const?
Arkadi Sharshevsky Feb. 15, 2018, 11:57 a.m. UTC | #2 
On 02/14/2018 05:12 PM, Stephen Hemminger wrote:
> On Wed, 14 Feb 2018 10:55:17 +0200
> Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
> 
>> +static mnl_cb_t mnlg_cb_array[NLMSG_MIN_TYPE] = {
>> +	[NLMSG_NOOP]	= mnlg_cb_noop,
>> +	[NLMSG_ERROR]	= mnlg_cb_error,
>> +	[NLMSG_DONE]	= mnlg_cb_stop,
>> +	[NLMSG_OVERRUN]	= mnlg_cb_noop,
>> +};
>> +
> 
> Could be const?
> 

I pass the array to mnl_cb_run2() which will discard the 'const'
qualifier. So I dont think this is very beneficial.
Stephen Hemminger Feb. 15, 2018, 6:15 p.m. UTC | #3 
On Thu, 15 Feb 2018 13:57:18 +0200
Arkadi Sharshevsky <arkadis@mellanox.com> wrote:

> On 02/14/2018 05:12 PM, Stephen Hemminger wrote:
> > On Wed, 14 Feb 2018 10:55:17 +0200
> > Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
> >   
> >> +static mnl_cb_t mnlg_cb_array[NLMSG_MIN_TYPE] = {
> >> +	[NLMSG_NOOP]	= mnlg_cb_noop,
> >> +	[NLMSG_ERROR]	= mnlg_cb_error,
> >> +	[NLMSG_DONE]	= mnlg_cb_stop,
> >> +	[NLMSG_OVERRUN]	= mnlg_cb_noop,
> >> +};
> >> +  
> > 
> > Could be const?
> >   
> 
> I pass the array to mnl_cb_run2() which will discard the 'const'
> qualifier. So I dont think this is very beneficial.

Thanks, makes sense,
diff mbox series

Patch

diff --git a/devlink/mnlg.c b/devlink/mnlg.c
index 9e27de2..37c5687 100644
--- a/devlink/mnlg.c
+++ b/devlink/mnlg.c
@@ -18,6 +18,8 @@ 
 #include <libmnl/libmnl.h>
 #include <linux/genetlink.h>
 
+#include "libnetlink.h"
+#include "utils.h"
 #include "mnlg.h"
 
 struct mnlg_socket {
@@ -60,6 +62,39 @@  int mnlg_socket_send(struct mnlg_socket *nlg, const struct nlmsghdr *nlh)
 	return mnl_socket_sendto(nlg->nl, nlh, nlh->nlmsg_len);
 }
 
+static int mnlg_cb_noop(const struct nlmsghdr *nlh, void *data)
+{
+	return MNL_CB_OK;
+}
+
+static int mnlg_cb_error(const struct nlmsghdr *nlh, void *data)
+{
+	const struct nlmsgerr *err = mnl_nlmsg_get_payload(nlh);
+
+	if (nl_dump_ext_ack(nlh, NULL))
+		return MNL_CB_STOP;
+
+	/* Netlink subsystems returns the errno value with different signess */
+	if (err->error < 0)
+		errno = -err->error;
+	else
+		errno = err->error;
+
+	return err->error == 0 ? MNL_CB_STOP : MNL_CB_ERROR;
+}
+
+static int mnlg_cb_stop(const struct nlmsghdr *nlh, void *data)
+{
+	return MNL_CB_STOP;
+}
+
+static mnl_cb_t mnlg_cb_array[NLMSG_MIN_TYPE] = {
+	[NLMSG_NOOP]	= mnlg_cb_noop,
+	[NLMSG_ERROR]	= mnlg_cb_error,
+	[NLMSG_DONE]	= mnlg_cb_stop,
+	[NLMSG_OVERRUN]	= mnlg_cb_noop,
+};
+
 int mnlg_socket_recv_run(struct mnlg_socket *nlg, mnl_cb_t data_cb, void *data)
 {
 	int err;
@@ -69,8 +104,9 @@  int mnlg_socket_recv_run(struct mnlg_socket *nlg, mnl_cb_t data_cb, void *data)
 					  MNL_SOCKET_BUFFER_SIZE);
 		if (err <= 0)
 			break;
-		err = mnl_cb_run(nlg->buf, err, nlg->seq, nlg->portid,
-				 data_cb, data);
+		err = mnl_cb_run2(nlg->buf, err, nlg->seq, nlg->portid,
+				  data_cb, data, mnlg_cb_array,
+				  ARRAY_SIZE(mnlg_cb_array));
 	} while (err > 0);
 
 	return err;
@@ -220,6 +256,7 @@  struct mnlg_socket *mnlg_socket_open(const char *family_name, uint8_t version)
 {
 	struct mnlg_socket *nlg;
 	struct nlmsghdr *nlh;
+	int one = 1;
 	int err;
 
 	nlg = malloc(sizeof(*nlg));
@@ -234,6 +271,16 @@  struct mnlg_socket *mnlg_socket_open(const char *family_name, uint8_t version)
 	if (!nlg->nl)
 		goto err_mnl_socket_open;
 
+	err = mnl_socket_setsockopt(nlg->nl, NETLINK_CAP_ACK, &one,
+				    sizeof(one));
+	if (err)
+		goto err_mnl_set_ack;
+
+	err = mnl_socket_setsockopt(nlg->nl, NETLINK_EXT_ACK, &one,
+				    sizeof(one));
+	if (err)
+		goto err_mnl_set_ext_ack;
+
 	err = mnl_socket_bind(nlg->nl, 0, MNL_SOCKET_AUTOPID);
 	if (err < 0)
 		goto err_mnl_socket_bind;
@@ -258,6 +305,8 @@  struct mnlg_socket *mnlg_socket_open(const char *family_name, uint8_t version)
 err_mnlg_socket_recv_run:
 err_mnlg_socket_send:
 err_mnl_socket_bind:
+err_mnl_set_ext_ack:
+err_mnl_set_ack:
 	mnl_socket_close(nlg->nl);
 err_mnl_socket_open:
 	free(nlg->buf);
diff --git a/include/libnetlink.h b/include/libnetlink.h
index d632219..9d9249e 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -109,6 +109,7 @@  int rtnl_send(struct rtnl_handle *rth, const void *buf, int)
 	__attribute__((warn_unused_result));
 int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int)
 	__attribute__((warn_unused_result));
+int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn);
 
 int addattr(struct nlmsghdr *n, int maxlen, int type);
 int addattr8(struct nlmsghdr *n, int maxlen, int type, __u8 data);
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 7ca47b2..8bb1c8d 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -65,7 +65,7 @@  static int err_attr_cb(const struct nlattr *attr, void *data)
 }
 
 /* dump netlink extended ack error message */
-static int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
+int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 {
 	struct nlattr *tb[NLMSGERR_ATTR_MAX + 1] = {};
 	const struct nlmsgerr *err = mnl_nlmsg_get_payload(nlh);
@@ -120,7 +120,7 @@  static int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 #warning "libmnl required for error support"
 
 /* No extended error ack without libmnl */
-static int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
+int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 {
 	return 0;
 }