* [patch iproute2-next v3 1/6] ip/ipnetns: move internals of get_netnsid_from_name() into namespace.c
2023-10-24 10:03 [patch iproute2-next v3 0/6] expose devlink instances relationships Jiri Pirko
@ 2023-10-24 10:03 ` Jiri Pirko
2023-10-26 16:56 ` David Ahern
2023-10-24 10:03 ` [patch iproute2-next v3 2/6] devlink: do conditional new line print in pr_out_port_handle_end() Jiri Pirko
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-24 10:03 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, daniel.machon
From: Jiri Pirko <jiri@nvidia.com>
In order to be able to reuse get_netnsid_from_name() function outside of
ip code, move the internals to lib/namespace.c to a new function called
netns_id_from_name().
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- s/netns_netnsid_from_name/netns_id_from_name/
v1->v2:
- new patch
---
include/namespace.h | 4 ++++
ip/ipnetns.c | 45 +----------------------------------------
lib/namespace.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 54 insertions(+), 44 deletions(-)
diff --git a/include/namespace.h b/include/namespace.h
index e47f9b5d49d1..e860a4b8ee5b 100644
--- a/include/namespace.h
+++ b/include/namespace.h
@@ -8,6 +8,8 @@
#include <sys/syscall.h>
#include <errno.h>
+#include "namespace.h"
+
#ifndef NETNS_RUN_DIR
#define NETNS_RUN_DIR "/var/run/netns"
#endif
@@ -58,4 +60,6 @@ struct netns_func {
void *arg;
};
+int netns_id_from_name(struct rtnl_handle *rtnl, const char *name);
+
#endif /* __NAMESPACE_H__ */
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 9d996832aef8..0ae46a874a0c 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -105,52 +105,9 @@ static int ipnetns_have_nsid(void)
int get_netnsid_from_name(const char *name)
{
- struct {
- struct nlmsghdr n;
- struct rtgenmsg g;
- char buf[1024];
- } req = {
- .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg)),
- .n.nlmsg_flags = NLM_F_REQUEST,
- .n.nlmsg_type = RTM_GETNSID,
- .g.rtgen_family = AF_UNSPEC,
- };
- struct nlmsghdr *answer;
- struct rtattr *tb[NETNSA_MAX + 1];
- struct rtgenmsg *rthdr;
- int len, fd, ret = -1;
-
netns_nsid_socket_init();
- fd = netns_get_fd(name);
- if (fd < 0)
- return fd;
-
- addattr32(&req.n, 1024, NETNSA_FD, fd);
- if (rtnl_talk(&rtnsh, &req.n, &answer) < 0) {
- close(fd);
- return -2;
- }
- close(fd);
-
- /* Validate message and parse attributes */
- if (answer->nlmsg_type == NLMSG_ERROR)
- goto out;
-
- rthdr = NLMSG_DATA(answer);
- len = answer->nlmsg_len - NLMSG_SPACE(sizeof(*rthdr));
- if (len < 0)
- goto out;
-
- parse_rtattr(tb, NETNSA_MAX, NETNS_RTA(rthdr), len);
-
- if (tb[NETNSA_NSID]) {
- ret = rta_getattr_s32(tb[NETNSA_NSID]);
- }
-
-out:
- free(answer);
- return ret;
+ return netns_id_from_name(&rtnsh, name);
}
struct nsid_cache {
diff --git a/lib/namespace.c b/lib/namespace.c
index 1202fa85f97d..f03f4bbabceb 100644
--- a/lib/namespace.c
+++ b/lib/namespace.c
@@ -7,9 +7,11 @@
#include <fcntl.h>
#include <dirent.h>
#include <limits.h>
+#include <linux/net_namespace.h>
#include "utils.h"
#include "namespace.h"
+#include "libnetlink.h"
static void bind_etc(const char *name)
{
@@ -139,3 +141,50 @@ int netns_foreach(int (*func)(char *nsname, void *arg), void *arg)
closedir(dir);
return 0;
}
+
+int netns_id_from_name(struct rtnl_handle *rtnl, const char *name)
+{
+ struct {
+ struct nlmsghdr n;
+ struct rtgenmsg g;
+ char buf[1024];
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETNSID,
+ .g.rtgen_family = AF_UNSPEC,
+ };
+ struct nlmsghdr *answer;
+ struct rtattr *tb[NETNSA_MAX + 1];
+ struct rtgenmsg *rthdr;
+ int len, fd, ret = -1;
+
+ fd = netns_get_fd(name);
+ if (fd < 0)
+ return fd;
+
+ addattr32(&req.n, 1024, NETNSA_FD, fd);
+ if (rtnl_talk(rtnl, &req.n, &answer) < 0) {
+ close(fd);
+ return -2;
+ }
+ close(fd);
+
+ /* Validate message and parse attributes */
+ if (answer->nlmsg_type == NLMSG_ERROR)
+ goto out;
+
+ rthdr = NLMSG_DATA(answer);
+ len = answer->nlmsg_len - NLMSG_SPACE(sizeof(*rthdr));
+ if (len < 0)
+ goto out;
+
+ parse_rtattr(tb, NETNSA_MAX, NETNS_RTA(rthdr), len);
+
+ if (tb[NETNSA_NSID])
+ ret = rta_getattr_s32(tb[NETNSA_NSID]);
+
+out:
+ free(answer);
+ return ret;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 1/6] ip/ipnetns: move internals of get_netnsid_from_name() into namespace.c
2023-10-24 10:03 ` [patch iproute2-next v3 1/6] ip/ipnetns: move internals of get_netnsid_from_name() into namespace.c Jiri Pirko
@ 2023-10-26 16:56 ` David Ahern
2023-10-27 8:25 ` Jiri Pirko
0 siblings, 1 reply; 16+ messages in thread
From: David Ahern @ 2023-10-26 16:56 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: stephen, daniel.machon
On 10/24/23 4:03 AM, Jiri Pirko wrote:
> diff --git a/include/namespace.h b/include/namespace.h
> index e47f9b5d49d1..e860a4b8ee5b 100644
> --- a/include/namespace.h
> +++ b/include/namespace.h
> @@ -8,6 +8,8 @@
> #include <sys/syscall.h>
> #include <errno.h>
>
> +#include "namespace.h"
??? including this file into itself?
> +
> #ifndef NETNS_RUN_DIR
> #define NETNS_RUN_DIR "/var/run/netns"
> #endif
> @@ -58,4 +60,6 @@ struct netns_func {
> void *arg;
> };
>
> +int netns_id_from_name(struct rtnl_handle *rtnl, const char *name);
> +
> #endif /* __NAMESPACE_H__ */
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 1/6] ip/ipnetns: move internals of get_netnsid_from_name() into namespace.c
2023-10-26 16:56 ` David Ahern
@ 2023-10-27 8:25 ` Jiri Pirko
0 siblings, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2023-10-27 8:25 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, stephen, daniel.machon
Thu, Oct 26, 2023 at 06:56:20PM CEST, dsahern@gmail.com wrote:
>On 10/24/23 4:03 AM, Jiri Pirko wrote:
>> diff --git a/include/namespace.h b/include/namespace.h
>> index e47f9b5d49d1..e860a4b8ee5b 100644
>> --- a/include/namespace.h
>> +++ b/include/namespace.h
>> @@ -8,6 +8,8 @@
>> #include <sys/syscall.h>
>> #include <errno.h>
>>
>> +#include "namespace.h"
>
>??? including this file into itself?
Interesting. Must have been done during code move. Will fix.
>
>> +
>> #ifndef NETNS_RUN_DIR
>> #define NETNS_RUN_DIR "/var/run/netns"
>> #endif
>> @@ -58,4 +60,6 @@ struct netns_func {
>> void *arg;
>> };
>>
>> +int netns_id_from_name(struct rtnl_handle *rtnl, const char *name);
>> +
>> #endif /* __NAMESPACE_H__ */
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch iproute2-next v3 2/6] devlink: do conditional new line print in pr_out_port_handle_end()
2023-10-24 10:03 [patch iproute2-next v3 0/6] expose devlink instances relationships Jiri Pirko
2023-10-24 10:03 ` [patch iproute2-next v3 1/6] ip/ipnetns: move internals of get_netnsid_from_name() into namespace.c Jiri Pirko
@ 2023-10-24 10:03 ` Jiri Pirko
2023-10-24 10:04 ` [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object Jiri Pirko
` (3 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2023-10-24 10:03 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, daniel.machon
From: Jiri Pirko <jiri@nvidia.com>
Instead of printing out new line unconditionally, use __pr_out_newline()
to print it only when needed avoiding double prints.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- new patch
---
devlink/devlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 3baad355759e..c18a4a4fbc5a 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2974,7 +2974,7 @@ static void pr_out_port_handle_end(struct dl *dl)
if (dl->json_output)
close_json_object();
else
- pr_out("\n");
+ __pr_out_newline();
}
static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object
2023-10-24 10:03 [patch iproute2-next v3 0/6] expose devlink instances relationships Jiri Pirko
2023-10-24 10:03 ` [patch iproute2-next v3 1/6] ip/ipnetns: move internals of get_netnsid_from_name() into namespace.c Jiri Pirko
2023-10-24 10:03 ` [patch iproute2-next v3 2/6] devlink: do conditional new line print in pr_out_port_handle_end() Jiri Pirko
@ 2023-10-24 10:04 ` Jiri Pirko
2023-10-26 17:03 ` David Ahern
2023-10-24 10:04 ` [patch iproute2-next v3 4/6] devlink: introduce support for netns id for nested handle Jiri Pirko
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-24 10:04 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, daniel.machon
From: Jiri Pirko <jiri@nvidia.com>
For existing pr_out_nested_handle() user (line card), the output stays
the same. For the new users, introduce __pr_out_nested_handle()
to allow to print devlink instance as object allowing to carry
attributes in it (like netns).
Note that as __pr_out_handle_start() and pr_out_handle_end() are newly
used, the function is moved below the definitions.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- new patch
---
devlink/devlink.c | 51 +++++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index c18a4a4fbc5a..f7325477f271 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2747,25 +2747,6 @@ static bool should_arr_last_handle_end(struct dl *dl, const char *bus_name,
!cmp_arr_last_handle(dl, bus_name, dev_name);
}
-static void pr_out_nested_handle(struct nlattr *nla_nested_dl)
-{
- struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
- char buf[64];
- int err;
-
- err = mnl_attr_parse_nested(nla_nested_dl, attr_cb, tb);
- if (err != MNL_CB_OK)
- return;
-
- if (!tb[DEVLINK_ATTR_BUS_NAME] ||
- !tb[DEVLINK_ATTR_DEV_NAME])
- return;
-
- sprintf(buf, "%s/%s", mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]),
- mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]));
- print_string(PRINT_ANY, "nested_devlink", " nested_devlink %s", buf);
-}
-
static void __pr_out_handle_start(struct dl *dl, struct nlattr **tb,
bool content, bool array)
{
@@ -2861,6 +2842,38 @@ static void pr_out_selftests_handle_end(struct dl *dl)
__pr_out_newline();
}
+static void __pr_out_nested_handle(struct dl *dl, struct nlattr *nla_nested_dl,
+ bool is_object)
+{
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+ int err;
+
+ err = mnl_attr_parse_nested(nla_nested_dl, attr_cb, tb);
+ if (err != MNL_CB_OK)
+ return;
+
+ if (!tb[DEVLINK_ATTR_BUS_NAME] ||
+ !tb[DEVLINK_ATTR_DEV_NAME])
+ return;
+
+ if (!is_object) {
+ char buf[64];
+
+ sprintf(buf, "%s/%s", mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]),
+ mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]));
+ print_string(PRINT_ANY, "nested_devlink", " nested_devlink %s", buf);
+ return;
+ }
+
+ __pr_out_handle_start(dl, tb, false, false);
+ pr_out_handle_end(dl);
+}
+
+static void pr_out_nested_handle(struct nlattr *nla_nested_dl)
+{
+ __pr_out_nested_handle(NULL, nla_nested_dl, false);
+}
+
static bool cmp_arr_last_port_handle(struct dl *dl, const char *bus_name,
const char *dev_name, uint32_t port_index)
{
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object
2023-10-24 10:04 ` [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object Jiri Pirko
@ 2023-10-26 17:03 ` David Ahern
2023-10-27 8:26 ` Jiri Pirko
2023-10-27 13:12 ` Petr Machata
0 siblings, 2 replies; 16+ messages in thread
From: David Ahern @ 2023-10-26 17:03 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: stephen, daniel.machon
On 10/24/23 4:04 AM, Jiri Pirko wrote:
> @@ -2861,6 +2842,38 @@ static void pr_out_selftests_handle_end(struct dl *dl)
> __pr_out_newline();
> }
>
> +static void __pr_out_nested_handle(struct dl *dl, struct nlattr *nla_nested_dl,
> + bool is_object)
> +{
> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
> + int err;
> +
> + err = mnl_attr_parse_nested(nla_nested_dl, attr_cb, tb);
> + if (err != MNL_CB_OK)
> + return;
> +
> + if (!tb[DEVLINK_ATTR_BUS_NAME] ||
> + !tb[DEVLINK_ATTR_DEV_NAME])
> + return;
> +
> + if (!is_object) {
> + char buf[64];
> +
> + sprintf(buf, "%s/%s", mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]),
> + mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]));
buf[64] - 1 for null terminator - 16 for IFNAMSIZ leaves 47. I do not
see limits on bus name length, so how can you guarantee it is always <
47 characters?
Make this snprintf, check the return and make sure buf is null terminated.
> + print_string(PRINT_ANY, "nested_devlink", " nested_devlink %s", buf);
> + return;
> + }
> +
> + __pr_out_handle_start(dl, tb, false, false);
> + pr_out_handle_end(dl);
> +}
> +
> +static void pr_out_nested_handle(struct nlattr *nla_nested_dl)
> +{
> + __pr_out_nested_handle(NULL, nla_nested_dl, false);
> +}
> +
> static bool cmp_arr_last_port_handle(struct dl *dl, const char *bus_name,
> const char *dev_name, uint32_t port_index)
> {
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object
2023-10-26 17:03 ` David Ahern
@ 2023-10-27 8:26 ` Jiri Pirko
2023-10-27 13:12 ` Petr Machata
1 sibling, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2023-10-27 8:26 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, stephen, daniel.machon
Thu, Oct 26, 2023 at 07:03:30PM CEST, dsahern@gmail.com wrote:
>On 10/24/23 4:04 AM, Jiri Pirko wrote:
>> @@ -2861,6 +2842,38 @@ static void pr_out_selftests_handle_end(struct dl *dl)
>> __pr_out_newline();
>> }
>>
>> +static void __pr_out_nested_handle(struct dl *dl, struct nlattr *nla_nested_dl,
>> + bool is_object)
>> +{
>> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
>> + int err;
>> +
>> + err = mnl_attr_parse_nested(nla_nested_dl, attr_cb, tb);
>> + if (err != MNL_CB_OK)
>> + return;
>> +
>> + if (!tb[DEVLINK_ATTR_BUS_NAME] ||
>> + !tb[DEVLINK_ATTR_DEV_NAME])
>> + return;
>> +
>> + if (!is_object) {
>> + char buf[64];
>> +
>> + sprintf(buf, "%s/%s", mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]),
>> + mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]));
>
>buf[64] - 1 for null terminator - 16 for IFNAMSIZ leaves 47. I do not
IFNAMSIZ is irrelevant here.
>see limits on bus name length, so how can you guarantee it is always <
>47 characters?
>
>Make this snprintf, check the return and make sure buf is null terminated.
I will fix it in separate patch, as this is just copied here.
>
>> + print_string(PRINT_ANY, "nested_devlink", " nested_devlink %s", buf);
>> + return;
>> + }
>> +
>> + __pr_out_handle_start(dl, tb, false, false);
>> + pr_out_handle_end(dl);
>> +}
>> +
>> +static void pr_out_nested_handle(struct nlattr *nla_nested_dl)
>> +{
>> + __pr_out_nested_handle(NULL, nla_nested_dl, false);
>> +}
>> +
>> static bool cmp_arr_last_port_handle(struct dl *dl, const char *bus_name,
>> const char *dev_name, uint32_t port_index)
>> {
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object
2023-10-26 17:03 ` David Ahern
2023-10-27 8:26 ` Jiri Pirko
@ 2023-10-27 13:12 ` Petr Machata
2023-10-27 17:16 ` David Ahern
1 sibling, 1 reply; 16+ messages in thread
From: Petr Machata @ 2023-10-27 13:12 UTC (permalink / raw)
To: David Ahern; +Cc: Jiri Pirko, netdev, stephen, daniel.machon
David Ahern <dsahern@gmail.com> writes:
> On 10/24/23 4:04 AM, Jiri Pirko wrote:
>> @@ -2861,6 +2842,38 @@ static void pr_out_selftests_handle_end(struct dl *dl)
>> __pr_out_newline();
>> }
>>
>> +static void __pr_out_nested_handle(struct dl *dl, struct nlattr *nla_nested_dl,
>> + bool is_object)
>> +{
>> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
>> + int err;
>> +
>> + err = mnl_attr_parse_nested(nla_nested_dl, attr_cb, tb);
>> + if (err != MNL_CB_OK)
>> + return;
>> +
>> + if (!tb[DEVLINK_ATTR_BUS_NAME] ||
>> + !tb[DEVLINK_ATTR_DEV_NAME])
>> + return;
>> +
>> + if (!is_object) {
>> + char buf[64];
>> +
>> + sprintf(buf, "%s/%s", mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]),
>> + mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]));
>
> buf[64] - 1 for null terminator - 16 for IFNAMSIZ leaves 47. I do not
> see limits on bus name length, so how can you guarantee it is always <
> 47 characters?
>
> Make this snprintf, check the return and make sure buf is null terminated.
I was wondering whether somehing like this might make sense in the
iproute2 library:
#define alloca_sprintf(FMT, ...) ({ \
int xasprintf_n = snprintf(NULL, 0, (FMT), __VA_ARGS__); \
char *xasprintf_buf = alloca(xasprintf_n); \
sprintf(xasprintf_buf, (FMT), __VA_ARGS__); \
xasprintf_buf; \
})
void foo() {
const char *buf = alloca_sprintf("%x %y %z", etc.);
printf(... buf ...);
}
I'm not really happy with it -- because of alloca vs. array, and because
of the double evaluation. But all those SPRINT_BUF's peppered everywhere
make me uneasy every time I read or write them.
Or maybe roll something custom asprintf-like that can reuse and/or
realloc a passed-in buffer?
The sprintf story is pretty bad in iproute2 right now, IMHO.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object
2023-10-27 13:12 ` Petr Machata
@ 2023-10-27 17:16 ` David Ahern
2023-10-30 11:03 ` Petr Machata
0 siblings, 1 reply; 16+ messages in thread
From: David Ahern @ 2023-10-27 17:16 UTC (permalink / raw)
To: Petr Machata; +Cc: Jiri Pirko, netdev, stephen, daniel.machon
On 10/27/23 7:12 AM, Petr Machata wrote:
> I was wondering whether somehing like this might make sense in the
> iproute2 library:
>
> #define alloca_sprintf(FMT, ...) ({ \
> int xasprintf_n = snprintf(NULL, 0, (FMT), __VA_ARGS__); \
> char *xasprintf_buf = alloca(xasprintf_n); \
> sprintf(xasprintf_buf, (FMT), __VA_ARGS__); \
> xasprintf_buf; \
> })
>
> void foo() {
> const char *buf = alloca_sprintf("%x %y %z", etc.);
> printf(... buf ...);
> }
>
> I'm not really happy with it -- because of alloca vs. array, and because
> of the double evaluation. But all those SPRINT_BUF's peppered everywhere
> make me uneasy every time I read or write them.
agreed.
>
> Or maybe roll something custom asprintf-like that can reuse and/or
> realloc a passed-in buffer?
>
> The sprintf story is pretty bad in iproute2 right now, IMHO.
It is a bit of a mess. If you have a few cycles, want to send an RFC?
Just pick 1 or 2 to convert to show intent with a new design.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object
2023-10-27 17:16 ` David Ahern
@ 2023-10-30 11:03 ` Petr Machata
0 siblings, 0 replies; 16+ messages in thread
From: Petr Machata @ 2023-10-30 11:03 UTC (permalink / raw)
To: David Ahern; +Cc: Petr Machata, Jiri Pirko, netdev, stephen, daniel.machon
David Ahern <dsahern@gmail.com> writes:
> On 10/27/23 7:12 AM, Petr Machata wrote:
>> I was wondering whether somehing like this might make sense in the
>> iproute2 library:
>>
>> #define alloca_sprintf(FMT, ...) ({ \
>> int xasprintf_n = snprintf(NULL, 0, (FMT), __VA_ARGS__); \
>> char *xasprintf_buf = alloca(xasprintf_n); \
>> sprintf(xasprintf_buf, (FMT), __VA_ARGS__); \
>> xasprintf_buf; \
>> })
>>
>> void foo() {
>> const char *buf = alloca_sprintf("%x %y %z", etc.);
>> printf(... buf ...);
>> }
>>
>> I'm not really happy with it -- because of alloca vs. array, and because
>> of the double evaluation. But all those SPRINT_BUF's peppered everywhere
>> make me uneasy every time I read or write them.
>
> agreed.
>
>>
>> Or maybe roll something custom asprintf-like that can reuse and/or
>> realloc a passed-in buffer?
>>
>> The sprintf story is pretty bad in iproute2 right now, IMHO.
>
> It is a bit of a mess. If you have a few cycles, want to send an RFC?
> Just pick 1 or 2 to convert to show intent with a new design.
I picked at it a bit over the weekend, but came up with nothing that I
find comfortable proposing. The static buffer approach has some major
advantages: nothing ever fails and nothing ever needs cleanups. This
keeps the client code tidy and compact. Anything dynamic adds points of
failure and cleanups, which in C means more client-side boilerplate.
Anyway, I'll pick at it some more and see I find anything.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch iproute2-next v3 4/6] devlink: introduce support for netns id for nested handle
2023-10-24 10:03 [patch iproute2-next v3 0/6] expose devlink instances relationships Jiri Pirko
` (2 preceding siblings ...)
2023-10-24 10:04 ` [patch iproute2-next v3 3/6] devlink: extend pr_out_nested_handle() to print object Jiri Pirko
@ 2023-10-24 10:04 ` Jiri Pirko
2023-10-26 17:08 ` David Ahern
2023-10-24 10:04 ` [patch iproute2-next v3 5/6] devlink: print nested handle for port function Jiri Pirko
2023-10-24 10:04 ` [patch iproute2-next v3 6/6] devlink: print nested devlink handle for devlink dev Jiri Pirko
5 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-24 10:04 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, daniel.machon
From: Jiri Pirko <jiri@nvidia.com>
Nested handle may contain DEVLINK_ATTR_NETNS_ID attribute that indicates
the network namespace where the nested devlink instance resides. Process
this converting to netns name if possible and print to user.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- moved netns_name_by_id() into lib/namespace.c
- s/netns_name_by_id/netns_name_from_id/
- rebased on top of new patch "devlink: extend pr_out_nested_handle() to
print object"
v1->v2:
- use previously introduced netns_netnsid_from_name() instead of code
duplication for the same function.
- s/nesns_name_by_id_func/netns_name_by_id_func/
---
devlink/devlink.c | 23 ++++++++++++++++++++++-
include/namespace.h | 1 +
lib/namespace.c | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index f7325477f271..7ba2d0dcac72 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -24,6 +24,7 @@
#include <linux/genetlink.h>
#include <linux/devlink.h>
#include <linux/netlink.h>
+#include <linux/net_namespace.h>
#include <libmnl/libmnl.h>
#include <netinet/ether.h>
#include <sys/select.h>
@@ -722,6 +723,7 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_LINECARD_SUPPORTED_TYPES] = MNL_TYPE_NESTED,
[DEVLINK_ATTR_NESTED_DEVLINK] = MNL_TYPE_NESTED,
[DEVLINK_ATTR_SELFTESTS] = MNL_TYPE_NESTED,
+ [DEVLINK_ATTR_NETNS_ID] = MNL_TYPE_U32,
};
static const enum mnl_attr_data_type
@@ -2865,7 +2867,26 @@ static void __pr_out_nested_handle(struct dl *dl, struct nlattr *nla_nested_dl,
return;
}
- __pr_out_handle_start(dl, tb, false, false);
+ __pr_out_handle_start(dl, tb, tb[DEVLINK_ATTR_NETNS_ID], false);
+ if (tb[DEVLINK_ATTR_NETNS_ID]) {
+ int32_t id = mnl_attr_get_u32(tb[DEVLINK_ATTR_NETNS_ID]);
+
+ if (id >= 0) {
+ char *name = netns_name_from_id(id);
+
+ if (name) {
+ print_string(PRINT_ANY, "netns",
+ " netns %s", name);
+ free(name);
+ } else {
+ print_int(PRINT_ANY, "netnsid",
+ " netnsid %d", id);
+ }
+ } else {
+ print_string(PRINT_FP, NULL, " netnsid %s", "unknown");
+ print_int(PRINT_JSON, "netnsid", NULL, id);
+ }
+ }
pr_out_handle_end(dl);
}
diff --git a/include/namespace.h b/include/namespace.h
index e860a4b8ee5b..fbdfbc9a1c3b 100644
--- a/include/namespace.h
+++ b/include/namespace.h
@@ -61,5 +61,6 @@ struct netns_func {
};
int netns_id_from_name(struct rtnl_handle *rtnl, const char *name);
+char *netns_name_from_id(int32_t id);
#endif /* __NAMESPACE_H__ */
diff --git a/lib/namespace.c b/lib/namespace.c
index f03f4bbabceb..d3aeb9658e73 100644
--- a/lib/namespace.c
+++ b/lib/namespace.c
@@ -188,3 +188,37 @@ out:
free(answer);
return ret;
}
+
+struct netns_name_from_id_ctx {
+ int32_t id;
+ char *name;
+ struct rtnl_handle *rth;
+};
+
+static int netns_name_from_id_func(char *nsname, void *arg)
+{
+ struct netns_name_from_id_ctx *ctx = arg;
+ int32_t ret;
+
+ ret = netns_id_from_name(ctx->rth, nsname);
+ if (ret < 0 || ret != ctx->id)
+ return 0;
+ ctx->name = strdup(nsname);
+ return 1;
+}
+
+char *netns_name_from_id(int32_t id)
+{
+ struct rtnl_handle rth;
+ struct netns_name_from_id_ctx ctx = {
+ .id = id,
+ .rth = &rth,
+ };
+
+ if (rtnl_open(&rth, 0) < 0)
+ return NULL;
+ netns_foreach(netns_name_from_id_func, &ctx);
+ rtnl_close(&rth);
+
+ return ctx.name;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 4/6] devlink: introduce support for netns id for nested handle
2023-10-24 10:04 ` [patch iproute2-next v3 4/6] devlink: introduce support for netns id for nested handle Jiri Pirko
@ 2023-10-26 17:08 ` David Ahern
2023-10-27 8:29 ` Jiri Pirko
0 siblings, 1 reply; 16+ messages in thread
From: David Ahern @ 2023-10-26 17:08 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: stephen, daniel.machon
On 10/24/23 4:04 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Nested handle may contain DEVLINK_ATTR_NETNS_ID attribute that indicates
> the network namespace where the nested devlink instance resides. Process
> this converting to netns name if possible and print to user.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> v2->v3:
> - moved netns_name_by_id() into lib/namespace.c
> - s/netns_name_by_id/netns_name_from_id/
> - rebased on top of new patch "devlink: extend pr_out_nested_handle() to
> print object"
> v1->v2:
> - use previously introduced netns_netnsid_from_name() instead of code
> duplication for the same function.
> - s/nesns_name_by_id_func/netns_name_by_id_func/
> ---
> devlink/devlink.c | 23 ++++++++++++++++++++++-
> include/namespace.h | 1 +
> lib/namespace.c | 34 ++++++++++++++++++++++++++++++++++
> 3 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index f7325477f271..7ba2d0dcac72 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -24,6 +24,7 @@
> #include <linux/genetlink.h>
> #include <linux/devlink.h>
> #include <linux/netlink.h>
> +#include <linux/net_namespace.h>
> #include <libmnl/libmnl.h>
> #include <netinet/ether.h>
> #include <sys/select.h>
> @@ -722,6 +723,7 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
> [DEVLINK_ATTR_LINECARD_SUPPORTED_TYPES] = MNL_TYPE_NESTED,
> [DEVLINK_ATTR_NESTED_DEVLINK] = MNL_TYPE_NESTED,
> [DEVLINK_ATTR_SELFTESTS] = MNL_TYPE_NESTED,
> + [DEVLINK_ATTR_NETNS_ID] = MNL_TYPE_U32,
> };
>
> static const enum mnl_attr_data_type
> @@ -2865,7 +2867,26 @@ static void __pr_out_nested_handle(struct dl *dl, struct nlattr *nla_nested_dl,
> return;
> }
>
> - __pr_out_handle_start(dl, tb, false, false);
> + __pr_out_handle_start(dl, tb, tb[DEVLINK_ATTR_NETNS_ID], false);
> + if (tb[DEVLINK_ATTR_NETNS_ID]) {
> + int32_t id = mnl_attr_get_u32(tb[DEVLINK_ATTR_NETNS_ID]);
> +
> + if (id >= 0) {
why does the kernel side even add DEVLINK_ATTR_NETNS_ID if
peernet2id_alloc fails?
> + char *name = netns_name_from_id(id);
> +
> + if (name) {
> + print_string(PRINT_ANY, "netns",
> + " netns %s", name);
> + free(name);
> + } else {
> + print_int(PRINT_ANY, "netnsid",
> + " netnsid %d", id);
> + }
> + } else {
> + print_string(PRINT_FP, NULL, " netnsid %s", "unknown");
> + print_int(PRINT_JSON, "netnsid", NULL, id);
ie., how is -1 useful to userspace?
> + }
> + }
> pr_out_handle_end(dl);
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [patch iproute2-next v3 4/6] devlink: introduce support for netns id for nested handle
2023-10-26 17:08 ` David Ahern
@ 2023-10-27 8:29 ` Jiri Pirko
0 siblings, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2023-10-27 8:29 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, stephen, daniel.machon
Thu, Oct 26, 2023 at 07:08:04PM CEST, dsahern@gmail.com wrote:
>On 10/24/23 4:04 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Nested handle may contain DEVLINK_ATTR_NETNS_ID attribute that indicates
>> the network namespace where the nested devlink instance resides. Process
>> this converting to netns name if possible and print to user.
>>
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>> v2->v3:
>> - moved netns_name_by_id() into lib/namespace.c
>> - s/netns_name_by_id/netns_name_from_id/
>> - rebased on top of new patch "devlink: extend pr_out_nested_handle() to
>> print object"
>> v1->v2:
>> - use previously introduced netns_netnsid_from_name() instead of code
>> duplication for the same function.
>> - s/nesns_name_by_id_func/netns_name_by_id_func/
>> ---
>> devlink/devlink.c | 23 ++++++++++++++++++++++-
>> include/namespace.h | 1 +
>> lib/namespace.c | 34 ++++++++++++++++++++++++++++++++++
>> 3 files changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/devlink/devlink.c b/devlink/devlink.c
>> index f7325477f271..7ba2d0dcac72 100644
>> --- a/devlink/devlink.c
>> +++ b/devlink/devlink.c
>> @@ -24,6 +24,7 @@
>> #include <linux/genetlink.h>
>> #include <linux/devlink.h>
>> #include <linux/netlink.h>
>> +#include <linux/net_namespace.h>
>> #include <libmnl/libmnl.h>
>> #include <netinet/ether.h>
>> #include <sys/select.h>
>> @@ -722,6 +723,7 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
>> [DEVLINK_ATTR_LINECARD_SUPPORTED_TYPES] = MNL_TYPE_NESTED,
>> [DEVLINK_ATTR_NESTED_DEVLINK] = MNL_TYPE_NESTED,
>> [DEVLINK_ATTR_SELFTESTS] = MNL_TYPE_NESTED,
>> + [DEVLINK_ATTR_NETNS_ID] = MNL_TYPE_U32,
>> };
>>
>> static const enum mnl_attr_data_type
>> @@ -2865,7 +2867,26 @@ static void __pr_out_nested_handle(struct dl *dl, struct nlattr *nla_nested_dl,
>> return;
>> }
>>
>> - __pr_out_handle_start(dl, tb, false, false);
>> + __pr_out_handle_start(dl, tb, tb[DEVLINK_ATTR_NETNS_ID], false);
>> + if (tb[DEVLINK_ATTR_NETNS_ID]) {
>> + int32_t id = mnl_attr_get_u32(tb[DEVLINK_ATTR_NETNS_ID]);
>> +
>> + if (id >= 0) {
>
>why does the kernel side even add DEVLINK_ATTR_NETNS_ID if
>peernet2id_alloc fails?
Yep. So the user knows that this is in a different namespace. This
logic is copied from iplink
>
>
>> + char *name = netns_name_from_id(id);
>> +
>> + if (name) {
>> + print_string(PRINT_ANY, "netns",
>> + " netns %s", name);
>> + free(name);
>> + } else {
>> + print_int(PRINT_ANY, "netnsid",
>> + " netnsid %d", id);
>> + }
>> + } else {
>> + print_string(PRINT_FP, NULL, " netnsid %s", "unknown");
>> + print_int(PRINT_JSON, "netnsid", NULL, id);
>
>ie., how is -1 useful to userspace?
The userspace knows that this device is in a different netnamespace, but
it is unknown as peernet2id_alloc failed. Should be really a rare
corner case when system is under memory pressure.
>
>
>> + }
>> + }
>> pr_out_handle_end(dl);
>> }
>>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch iproute2-next v3 5/6] devlink: print nested handle for port function
2023-10-24 10:03 [patch iproute2-next v3 0/6] expose devlink instances relationships Jiri Pirko
` (3 preceding siblings ...)
2023-10-24 10:04 ` [patch iproute2-next v3 4/6] devlink: introduce support for netns id for nested handle Jiri Pirko
@ 2023-10-24 10:04 ` Jiri Pirko
2023-10-24 10:04 ` [patch iproute2-next v3 6/6] devlink: print nested devlink handle for devlink dev Jiri Pirko
5 siblings, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2023-10-24 10:04 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, daniel.machon
From: Jiri Pirko <jiri@nvidia.com>
If port function contains nested handle attribute, print it.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- rebased on top of new patch "devlink: extend pr_out_nested_handle() to
print object"
---
devlink/devlink.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 7ba2d0dcac72..90e8872b07ef 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -772,6 +772,7 @@ static const enum mnl_attr_data_type
devlink_function_policy[DEVLINK_PORT_FUNCTION_ATTR_MAX + 1] = {
[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR ] = MNL_TYPE_BINARY,
[DEVLINK_PORT_FN_ATTR_STATE] = MNL_TYPE_U8,
+ [DEVLINK_PORT_FN_ATTR_DEVLINK] = MNL_TYPE_NESTED,
};
static int function_attr_cb(const struct nlattr *attr, void *data)
@@ -2895,6 +2896,22 @@ static void pr_out_nested_handle(struct nlattr *nla_nested_dl)
__pr_out_nested_handle(NULL, nla_nested_dl, false);
}
+static void pr_out_nested_handle_obj(struct dl *dl,
+ struct nlattr *nla_nested_dl,
+ bool obj_start, bool obj_end)
+{
+ if (obj_start) {
+ pr_out_object_start(dl, "nested_devlink");
+ check_indent_newline(dl);
+ }
+ __pr_out_nested_handle(dl, nla_nested_dl, true);
+ if (obj_end) {
+ if (!dl->json_output)
+ __pr_out_indent_dec();
+ pr_out_object_end(dl);
+ }
+}
+
static bool cmp_arr_last_port_handle(struct dl *dl, const char *bus_name,
const char *dev_name, uint32_t port_index)
{
@@ -4837,6 +4854,9 @@ static void pr_out_port_function(struct dl *dl, struct nlattr **tb_port)
port_fn_caps->value & DEVLINK_PORT_FN_CAP_IPSEC_PACKET ?
"enable" : "disable");
}
+ if (tb[DEVLINK_PORT_FN_ATTR_DEVLINK])
+ pr_out_nested_handle_obj(dl, tb[DEVLINK_PORT_FN_ATTR_DEVLINK],
+ true, true);
if (!dl->json_output)
__pr_out_indent_dec();
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [patch iproute2-next v3 6/6] devlink: print nested devlink handle for devlink dev
2023-10-24 10:03 [patch iproute2-next v3 0/6] expose devlink instances relationships Jiri Pirko
` (4 preceding siblings ...)
2023-10-24 10:04 ` [patch iproute2-next v3 5/6] devlink: print nested handle for port function Jiri Pirko
@ 2023-10-24 10:04 ` Jiri Pirko
5 siblings, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2023-10-24 10:04 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, daniel.machon
From: Jiri Pirko <jiri@nvidia.com>
Devlink dev may contain one or more nested devlink instances.
Print them using previously introduced pr_out_nested_handle_obj()
helper.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- rebased on top of new patch "devlink: extend pr_out_nested_handle() to
print object" and previous patch to use pr_out_nested_handle_obj()
---
devlink/devlink.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 90e8872b07ef..09ac347051f4 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -3858,13 +3858,35 @@ static void pr_out_reload_data(struct dl *dl, struct nlattr **tb)
pr_out_object_end(dl);
}
+static void pr_out_dev_nested(struct dl *dl, const struct nlmsghdr *nlh)
+{
+ int i = 0, count = 0;
+ struct nlattr *attr;
+
+ mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+ if (mnl_attr_get_type(attr) == DEVLINK_ATTR_NESTED_DEVLINK)
+ count++;
+ }
+ if (!count)
+ return;
+
+ mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+ if (mnl_attr_get_type(attr) != DEVLINK_ATTR_NESTED_DEVLINK)
+ continue;
+ pr_out_nested_handle_obj(dl, attr, i == 0, i == count - 1);
+ i++;
+ }
+}
-static void pr_out_dev(struct dl *dl, struct nlattr **tb)
+static void pr_out_dev(struct dl *dl, const struct nlmsghdr *nlh,
+ struct nlattr **tb)
{
if ((tb[DEVLINK_ATTR_RELOAD_FAILED] && mnl_attr_get_u8(tb[DEVLINK_ATTR_RELOAD_FAILED])) ||
- (tb[DEVLINK_ATTR_DEV_STATS] && dl->stats)) {
+ (tb[DEVLINK_ATTR_DEV_STATS] && dl->stats) ||
+ tb[DEVLINK_ATTR_NESTED_DEVLINK]) {
__pr_out_handle_start(dl, tb, true, false);
pr_out_reload_data(dl, tb);
+ pr_out_dev_nested(dl, nlh);
pr_out_handle_end(dl);
} else {
pr_out_handle(dl, tb);
@@ -3881,7 +3903,7 @@ static int cmd_dev_show_cb(const struct nlmsghdr *nlh, void *data)
if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
return MNL_CB_ERROR;
- pr_out_dev(dl, tb);
+ pr_out_dev(dl, nlh, tb);
return MNL_CB_OK;
}
@@ -6808,7 +6830,7 @@ static int cmd_mon_show_cb(const struct nlmsghdr *nlh, void *data)
return MNL_CB_ERROR;
pr_out_mon_header(genl->cmd);
dl->stats = true;
- pr_out_dev(dl, tb);
+ pr_out_dev(dl, nlh, tb);
pr_out_mon_footer();
break;
case DEVLINK_CMD_PORT_GET: /* fall through */
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread