Merge branch 'nsid-cleanup' into next

Guillaume Nault  says:

====================

It's currently hard to review ipnetns. The netns ids are inconsistently
treated as signed or unsigned and most helper functions aren't prepared
to use negative ids.

Netns id attributes can be negative: NETNSA_NSID_NOT_ASSIGNED =3D=3D -1.
So let's consistently treat nsids as signed and also reject negative
values in functions that are supposed to only handle assigned netns
ids.

While there, let's drop the extra blank line generated by some command
line parsing errors (patch 5/5).

====================

Signed-off-by: David Ahern <dsahern@gmail.com>
This commit is contained in:
David Ahern 2019-11-09 01:34:24 +00:00
commit 611d3123be
1 changed files with 29 additions and 20 deletions

View File

@ -137,7 +137,7 @@ int get_netnsid_from_name(const char *name)
parse_rtattr(tb, NETNSA_MAX, NETNS_RTA(rthdr), len); parse_rtattr(tb, NETNSA_MAX, NETNS_RTA(rthdr), len);
if (tb[NETNSA_NSID]) { if (tb[NETNSA_NSID]) {
ret = rta_getattr_u32(tb[NETNSA_NSID]); ret = rta_getattr_s32(tb[NETNSA_NSID]);
} }
out: out:
@ -161,9 +161,13 @@ static struct hlist_head name_head[NSIDMAP_SIZE];
static struct nsid_cache *netns_map_get_by_nsid(int nsid) static struct nsid_cache *netns_map_get_by_nsid(int nsid)
{ {
uint32_t h = NSID_HASH_NSID(nsid);
struct hlist_node *n; struct hlist_node *n;
uint32_t h;
if (nsid < 0)
return NULL;
h = NSID_HASH_NSID(nsid);
hlist_for_each(n, &nsid_head[h]) { hlist_for_each(n, &nsid_head[h]) {
struct nsid_cache *c = container_of(n, struct nsid_cache, struct nsid_cache *c = container_of(n, struct nsid_cache,
nsid_hash); nsid_hash);
@ -178,6 +182,9 @@ char *get_name_from_nsid(int nsid)
{ {
struct nsid_cache *c; struct nsid_cache *c;
if (nsid < 0)
return NULL;
netns_nsid_socket_init(); netns_nsid_socket_init();
netns_map_init(); netns_map_init();
@ -266,6 +273,9 @@ static int netns_get_name(int nsid, char *name)
DIR *dir; DIR *dir;
int id; int id;
if (nsid < 0)
return -EINVAL;
dir = opendir(NETNS_RUN_DIR); dir = opendir(NETNS_RUN_DIR);
if (!dir) if (!dir)
return -ENOENT; return -ENOENT;
@ -277,7 +287,7 @@ static int netns_get_name(int nsid, char *name)
continue; continue;
id = get_netnsid_from_name(entry->d_name); id = get_netnsid_from_name(entry->d_name);
if (nsid == id) { if (id >= 0 && nsid == id) {
strcpy(name, entry->d_name); strcpy(name, entry->d_name);
closedir(dir); closedir(dir);
return 0; return 0;
@ -317,20 +327,20 @@ int print_nsid(struct nlmsghdr *n, void *arg)
if (n->nlmsg_type == RTM_DELNSID) if (n->nlmsg_type == RTM_DELNSID)
print_bool(PRINT_ANY, "deleted", "Deleted ", true); print_bool(PRINT_ANY, "deleted", "Deleted ", true);
nsid = rta_getattr_u32(tb[NETNSA_NSID]); nsid = rta_getattr_s32(tb[NETNSA_NSID]);
if (nsid < 0) if (nsid < 0)
print_string(PRINT_ANY, "nsid", "nsid %s ", "not-assigned"); print_string(PRINT_FP, NULL, "nsid unassigned ", NULL);
else else
print_uint(PRINT_ANY, "nsid", "nsid %u ", nsid); print_int(PRINT_ANY, "nsid", "nsid %d ", nsid);
if (tb[NETNSA_CURRENT_NSID]) { if (tb[NETNSA_CURRENT_NSID]) {
current = rta_getattr_u32(tb[NETNSA_CURRENT_NSID]); current = rta_getattr_s32(tb[NETNSA_CURRENT_NSID]);
if (current < 0) if (current < 0)
print_string(PRINT_ANY, "current-nsid", print_string(PRINT_FP, NULL,
"current-nsid %s ", "not-assigned"); "current-nsid unassigned ", NULL);
else else
print_uint(PRINT_ANY, "current-nsid", print_int(PRINT_ANY, "current-nsid",
"current-nsid %u ", current); "current-nsid %d ", current);
} }
c = netns_map_get_by_nsid(tb[NETNSA_CURRENT_NSID] ? current : nsid); c = netns_map_get_by_nsid(tb[NETNSA_CURRENT_NSID] ? current : nsid);
@ -340,7 +350,7 @@ int print_nsid(struct nlmsghdr *n, void *arg)
netns_map_del(c); netns_map_del(c);
} }
/* During 'ip monitor nsid', no chance to have new nsid in cache. */ /* nsid might not be in cache */
if (c == NULL && n->nlmsg_type == RTM_NEWNSID) if (c == NULL && n->nlmsg_type == RTM_NEWNSID)
if (netns_get_name(nsid, name) == 0) { if (netns_get_name(nsid, name) == 0) {
print_string(PRINT_ANY, "name", print_string(PRINT_ANY, "name",
@ -429,10 +439,10 @@ static int netns_list_id(int argc, char **argv)
NEXT_ARG(); NEXT_ARG();
if (get_integer(&filter.target_nsid, *argv, 0)) if (get_integer(&filter.target_nsid, *argv, 0))
invarg("\"target-nsid\" value is invalid\n", invarg("\"target-nsid\" value is invalid",
*argv); *argv);
else if (filter.target_nsid < 0) else if (filter.target_nsid < 0)
invarg("\"target-nsid\" value should be >= 0\n", invarg("\"target-nsid\" value should be >= 0",
argv[1]); argv[1]);
} else if (strcmp(*argv, "nsid") == 0) { } else if (strcmp(*argv, "nsid") == 0) {
if (nsid >= 0) if (nsid >= 0)
@ -440,9 +450,9 @@ static int netns_list_id(int argc, char **argv)
NEXT_ARG(); NEXT_ARG();
if (get_integer(&nsid, *argv, 0)) if (get_integer(&nsid, *argv, 0))
invarg("\"nsid\" value is invalid\n", *argv); invarg("\"nsid\" value is invalid", *argv);
else if (nsid < 0) else if (nsid < 0)
invarg("\"nsid\" value should be >= 0\n", invarg("\"nsid\" value should be >= 0",
argv[1]); argv[1]);
} else } else
usage(); usage();
@ -491,8 +501,7 @@ static int netns_list(int argc, char **argv)
if (ipnetns_have_nsid()) { if (ipnetns_have_nsid()) {
id = get_netnsid_from_name(entry->d_name); id = get_netnsid_from_name(entry->d_name);
if (id >= 0) if (id >= 0)
print_uint(PRINT_ANY, "id", print_int(PRINT_ANY, "id", " (id: %d)", id);
" (id: %d)", id);
} }
print_string(PRINT_FP, NULL, "\n", NULL); print_string(PRINT_FP, NULL, "\n", NULL);
close_json_object(); close_json_object();
@ -923,9 +932,9 @@ static int netns_set(int argc, char **argv)
if (strcmp(argv[1], "auto") == 0) if (strcmp(argv[1], "auto") == 0)
nsid = -1; nsid = -1;
else if (get_integer(&nsid, argv[1], 0)) else if (get_integer(&nsid, argv[1], 0))
invarg("Invalid \"netnsid\" value\n", argv[1]); invarg("Invalid \"netnsid\" value", argv[1]);
else if (nsid < 0) else if (nsid < 0)
invarg("\"netnsid\" value should be >= 0\n", argv[1]); invarg("\"netnsid\" value should be >= 0", argv[1]);
snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name); snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name);
netns = open(netns_path, O_RDONLY | O_CLOEXEC); netns = open(netns_path, O_RDONLY | O_CLOEXEC);