Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/route: add initial support for br_vlan global_opts module #407

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

lib/route: add initial support for br_vlan global_opts module #407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9383d7
misc: add more list methods
ronand-atl Oct 25, 2024
1ee0376
core: support receiving multipart objects
ronand-atl Oct 25, 2024
66659a1
lib/route: add initial support for br_vlan module
ronand-atl Oct 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ libnlinclude_netlink_route_act_HEADERS = \
include/netlink/route/act/skbedit.h \
include/netlink/route/act/vlan.h \
$(NULL)
libnlinclude_netlink_route_br_vlandir = $(libnlincludedir)/netlink/route/br_vlan
libnlinclude_netlink_route_br_vlan_HEADERS = \
include/netlink/route/br_vlan/global_opts.h \
$(NULL)
libnlinclude_netlink_route_clsdir = $(libnlincludedir)/netlink/route/cls
libnlinclude_netlink_route_cls_HEADERS = \
include/netlink/route/cls/basic.h \
Expand Down Expand Up @@ -239,6 +243,7 @@ public_headers = \
$(libnlinclude_netlink_netfilter_HEADERS) \
$(libnlinclude_netlink_route_HEADERS) \
$(libnlinclude_netlink_route_act_HEADERS) \
$(libnlinclude_netlink_route_br_vlan_HEADERS) \
$(libnlinclude_netlink_route_cls_HEADERS) \
$(libnlinclude_netlink_route_cls_ematch_HEADERS) \
$(libnlinclude_netlink_route_link_HEADERS) \
Expand Down Expand Up @@ -442,6 +447,7 @@ lib_libnl_route_3_la_SOURCES = \
lib/route/act/skbedit.c \
lib/route/act/vlan.c \
lib/route/addr.c \
lib/route/br_vlan/global_opts.c \
lib/route/class.c \
lib/route/classid.c \
lib/route/cls.c \
Expand Down
3 changes: 2 additions & 1 deletion include/netlink/errno.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ extern "C" {
#define NLE_IMMUTABLE 32
#define NLE_DUMP_INTR 33
#define NLE_ATTRSIZE 34
#define NLE_MERGE_FAILURE 35

#define NLE_MAX NLE_ATTRSIZE
#define NLE_MAX NLE_MERGE_FAILURE

extern const char * nl_geterror(int);
extern void nl_perror(int, const char *);
Expand Down
43 changes: 43 additions & 0 deletions include/netlink/list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,35 @@ static inline void nl_list_add_head(struct nl_list_head *obj,
__nl_list_add(obj, head, head->next);
}

static inline void nl_list_insert_before(struct nl_list_head *obj,
struct nl_list_head *ref)
{
__nl_list_add(obj, ref->prev, ref);
}

static inline void nl_list_insert_after(struct nl_list_head *obj,
struct nl_list_head *ref)
{
__nl_list_add(obj, ref, ref->next);
}

static inline void nl_list_insert_list_after(struct nl_list_head *head,
struct nl_list_head *ref)
{
ref->next->prev = head->prev;
head->prev->next = ref->next;
ref->next = head->next;
head->next->prev = ref;
head->next = head;
head->prev = head;
}

static inline void nl_list_join(struct nl_list_head *head_1,
struct nl_list_head *head_2)
{
nl_list_insert_list_after(head_2, head_1->prev);
Copy link
Owner

@thom311 thom311 Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this code come from linux kernel headers (and do they call the functionality this way)? If kernel does it, that would be a strong reason to name them the same (and basically implement them the same). I didn't check. The commit message should explain how this new API relates to the list API in kernel (and optimally, they are obviously related and similar).

With these linked lists, often the head is a special object and not a regular entry. That is, often we could not meaningfully call nl_list_entry(head, Node, lst_node). It should be clear what happens if head_1 or head_2 point to such a special head element (or not).

for example, we could define that head_1 can be either a special element or not (it doesn't matter). But that head_2 must point to a special head element which afterwards will be empty (and the content of list 2 moved over to head_1. Alternatively, we could define that head_2 points to a regular element, and the entire list 2 will be joined. That is, list 2 is never empty, it at least contains head_2 element. In the latter case, `head_2 would not be empty afterwards.

This should be defined (by having a code comment that explains how to use this). Optimally, we would also have unit tests, since it's not at all obvious whether this implementation is correct (which it probably is).

The same applies to nl_list_insert_list_after(). Also, don't do nl_list_join and nl_list_insert_list_after something similar? Couldn't nl_list_insert_list_after() be named something like nl_list_join_front() (am not not sure exactly, since I don't fully understand what they do(*))? However, their name is rather different. There should be code comment that explains the difference (and also makes it clear why they are named differently).

(*) yes, I could spend some time trying to understand what they do, and then reason backwards whether the name makes sense. However, the name should make it clear upfront, and optimally a code comment explains the non-obvious details beyond a good name. Then I'd be more inclined to reason whether the implementation actually implements what the name+doc indicates.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the code is copied from linux kernel, then we must take care to not use invalid licenses (libnl is LGPL2.1-only).

Granted, those functions seem so minimal, I don't think you can implement them any other way.

If you want to get inspired, then compare to https://github.com/c-util/c-list/blob/main/src/c-list.h which has a compatible license. The downside is, we really want that the API looks similar to what is in kernel, so that somebody familiar with kernel API understand it.

Use your best judgment :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would merge this patch early, even if no users are merged yet. I think it's generally useful API beyond this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with all your points. I myself found it hard to understand how the list module worked since it had no documentation. It also didn't help that I'd never seen that type of implementation of a linked list before. I'll make a separate PR adds the extra methods and documents the module. I can add some unit tests as well if you could give me some pointers on how to do that.

Does this code come from linux kernel headers

No it doesn't. I hadn't looked at the code in the Linux kernel for their list implementation until now. I wrote the code based on a method I worked out on paper. The closest thing the kernel has seems to be list_splice_tail_init in include/linux/list.h. You seem to be able to use that function to do the same thing. It just has the parameters swapped around I think.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I myself found it hard to understand how the list module worked since it had no documentation.

With API like nl_list_add_tail() it's mostly clear what the function is going to do. On the other hand, with nl_list_join() there are more details that are not obvious.

I can add some unit tests as well if you could give me some pointers on how to do that.

In this case, unit tests would go to tests/check-direct.c. This test statically links the libraries and runs without a separate namespace, so it's suited for testing "pure" internal code.

The closest thing the kernel has [...]. It just has the parameters swapped around I think.

That might be an argument for following kernel names and API. On the other hand, there is the potential licensing concern (??) and maybe c-list.h (below) would still be nicer.

I wrote the code based on a method I worked out on paper.

That's great :)

I am quite a fan of https://github.com/c-util/c-list/blob/main/src/c-list.h

It's 2024 and I wonder, whether we shouldn't re-use an existing, solid implementation. Granted, a circular, intrusive linked list is relatively easy to reimplement. But should we? In NetworkManager, c-list is used via git-subtree. Since it's header-only, it's easy to use.

Import with

git subtree add --prefix shared/c-list/ [email protected]:c-util/c-list.git main --squash

upgrade (if ever necessary) with

git subtree pull --prefix shared/c-list/ [email protected]:c-util/c-list.git main --squash

and then add shared/c-list/src/ to Makefile.am as

default_includes = \
»·······-I$(srcdir)/include/linux-private \

I would do that. WDYT?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked and the library provides a means to do everything we can currently do with our implementation, except it it doesn't have reverse iteration macros. Those would be easy enough to add, but that might mean the code would have to copied over instead of using a subtree (or another file includes that file to add to it).

I think the point would be that we submit such features to c-list "upstream", and just do a git subtree pull. That requires that upstream is susceptible to such patches. I think upstream is, otherwise there would be a problem with pulling it.

Thereby others could potentially benefit to that contribution, not just some obscure part inside libnl3.

Of course it will still take some work to convert the existing code to use the new interface.

I don't think we need to rework existing code (unless somebody wants to send patches). It could be used only for new code.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about #412 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I'll also make a PR that adds reverse iterators to that library. I've added the code; I've just got to adapt some of their tests. I'll probably submit that tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the PR I made, for reference: c-util/c-list#14

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

In the meantime, for your WIP branch here, I think you can do:

git subtree pull --prefix third_party/c-list/ [email protected]:c-util/c-list.git refs/pull/14/head --squash

}

static inline void nl_list_del(struct nl_list_head *obj)
{
obj->next->prev = obj->prev;
Expand Down Expand Up @@ -76,17 +105,31 @@ static inline int nl_list_empty(struct nl_list_head *head)
#define nl_list_first_entry(head, type, member) \
nl_list_entry((head)->next, type, member)

#define nl_list_last_entry(head, type, member) \
nl_list_entry((head)->prev, type, member)

#define nl_list_for_each_entry(pos, head, member) \
for (pos = nl_list_entry((head)->next, __typeof__(*pos), member); \
&(pos)->member != (head); \
(pos) = nl_list_entry((pos)->member.next, __typeof__(*(pos)), member))

#define nl_list_for_each_entry_reverse(pos, head, member) \
for (pos = nl_list_entry((head)->prev, __typeof__(*pos), member); \
&(pos)->member != (head); \
(pos) = nl_list_entry((pos)->member.prev, __typeof__(*(pos)), member))

#define nl_list_for_each_entry_safe(pos, n, head, member) \
for (pos = nl_list_entry((head)->next, __typeof__(*pos), member), \
n = nl_list_entry(pos->member.next, __typeof__(*pos), member); \
&(pos)->member != (head); \
pos = n, n = nl_list_entry(n->member.next, __typeof__(*n), member))

#define nl_list_for_each_entry_safe_reverse(pos, n, head, member) \
for (pos = nl_list_entry((head)->prev, __typeof__(*pos), member), \
n = nl_list_entry(pos->member.prev, __typeof__(*pos), member); \
&(pos)->member != (head); \
pos = n, n = nl_list_entry(n->member.prev, __typeof__(*n), member))

#define nl_init_list_head(head) \
do { (head)->next = (head); (head)->prev = (head); } while (0)

Expand Down
159 changes: 159 additions & 0 deletions include/netlink/route/br_vlan/global_opts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/* SPDX-License-Identifier: LGPL-2.1-only */

#ifndef NETLINK_BR_VLAN_GLOBAL_OPTS_H_
#define NETLINK_BR_VLAN_GLOBAL_OPTS_H_

#include <netlink/netlink.h>

#ifdef __cplusplus
extern "C" {
#endif

struct rtnl_br_vlan_gopts;
struct rtnl_br_vlan_gopts_entry;

struct rtnl_br_vlan_gopts *rtnl_br_vlan_gopts_alloc(void);
void rtnl_br_vlan_gopts_put(struct rtnl_br_vlan_gopts *gopts);

struct rtnl_br_vlan_gopts_entry *rtnl_br_vlan_gopts_entry_alloc(void);
void rtnl_br_vlan_gopts_entry_free(struct rtnl_br_vlan_gopts_entry *entry);
struct rtnl_br_vlan_gopts_entry *
rtnl_br_vlan_gopts_entry_clone(const struct rtnl_br_vlan_gopts_entry *entry);

int rtnl_br_vlan_gopts_build_modify_request(
const struct rtnl_br_vlan_gopts *gopts, struct nl_msg **result);

int rtnl_br_vlan_gopts_modify(struct nl_sock *sk,
const struct rtnl_br_vlan_gopts *gopts);

int rtnl_br_vlan_gopts_build_get_request(uint32_t ifindex,
struct nl_msg **result);

int rtnl_br_vlan_gopts_get_kernel(struct nl_sock *sk, uint32_t ifindex,
struct rtnl_br_vlan_gopts **result);

int rtnl_br_vlan_gopts_set_ifindex(struct rtnl_br_vlan_gopts *gopts,
uint32_t value);
int rtnl_br_vlan_gopts_get_ifindex(const struct rtnl_br_vlan_gopts *gopts,
uint32_t *out);

int rtnl_br_vlan_gopts_set_entry(struct rtnl_br_vlan_gopts *gopts,
const struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_set_entry_range(
struct rtnl_br_vlan_gopts *gopts,
const struct rtnl_br_vlan_gopts_entry *entry, uint16_t vid_end);

int rtnl_br_vlan_gopts_unset_entry(struct rtnl_br_vlan_gopts *gopts,
uint16_t vid);
int rtnl_br_vlan_gopts_unset_entry_range(struct rtnl_br_vlan_gopts *gopts,
uint16_t vid_start, uint16_t vid_end);

int rtnl_br_vlan_gopts_get_entry(const struct rtnl_br_vlan_gopts *gopts,
uint16_t vid,
struct rtnl_br_vlan_gopts_entry **out);

int rtnl_br_vlan_gopts_foreach_gopts_entry(
const struct rtnl_br_vlan_gopts *gopts,
void (*cb)(struct rtnl_br_vlan_gopts_entry *entry, void *arg),
void *arg);

int rtnl_br_vlan_gopts_entry_set_vid(struct rtnl_br_vlan_gopts_entry *entry,
uint16_t value);
int rtnl_br_vlan_gopts_entry_get_vid(
const struct rtnl_br_vlan_gopts_entry *entry, uint16_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_snooping(
struct rtnl_br_vlan_gopts_entry *entry, uint8_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_snooping(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_snooping(
const struct rtnl_br_vlan_gopts_entry *entry, uint8_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_igmp_version(
struct rtnl_br_vlan_gopts_entry *entry, uint8_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_igmp_version(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_igmp_version(
const struct rtnl_br_vlan_gopts_entry *entry, uint8_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_mld_version(
struct rtnl_br_vlan_gopts_entry *entry, uint8_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_mld_version(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_mld_version(
const struct rtnl_br_vlan_gopts_entry *entry, uint8_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_last_member_cnt(
struct rtnl_br_vlan_gopts_entry *entry, uint32_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_last_member_cnt(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_last_member_cnt(
const struct rtnl_br_vlan_gopts_entry *entry, uint32_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_startup_query_cnt(
struct rtnl_br_vlan_gopts_entry *entry, uint32_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_startup_query_cnt(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_startup_query_cnt(
const struct rtnl_br_vlan_gopts_entry *entry, uint32_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_last_member_intvl(
struct rtnl_br_vlan_gopts_entry *entry, uint64_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_last_member_intvl(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_last_member_intvl(
const struct rtnl_br_vlan_gopts_entry *entry, uint64_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_membership_intvl(
struct rtnl_br_vlan_gopts_entry *entry, uint64_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_membership_intvl(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_membership_intvl(
const struct rtnl_br_vlan_gopts_entry *entry, uint64_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_querier_intvl(
struct rtnl_br_vlan_gopts_entry *entry, uint64_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_querier_intvl(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_querier_intvl(
const struct rtnl_br_vlan_gopts_entry *entry, uint64_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_query_intvl(
struct rtnl_br_vlan_gopts_entry *entry, uint64_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_query_intvl(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_query_intvl(
const struct rtnl_br_vlan_gopts_entry *entry, uint64_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_query_response_intvl(
struct rtnl_br_vlan_gopts_entry *entry, uint64_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_query_response_intvl(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_query_response_intvl(
const struct rtnl_br_vlan_gopts_entry *entry, uint64_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_startup_query_intvl(
struct rtnl_br_vlan_gopts_entry *entry, uint64_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_startup_query_intvl(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_startup_query_intvl(
const struct rtnl_br_vlan_gopts_entry *entry, uint64_t *out);

int rtnl_br_vlan_gopts_entry_set_mcast_querier(
struct rtnl_br_vlan_gopts_entry *entry, uint8_t value);
int rtnl_br_vlan_gopts_entry_unset_mcast_querier(
struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_mcast_querier(
const struct rtnl_br_vlan_gopts_entry *entry, uint8_t *out);

int rtnl_br_vlan_gopts_entry_set_msti(struct rtnl_br_vlan_gopts_entry *entry,
uint16_t value);
int rtnl_br_vlan_gopts_entry_unset_msti(struct rtnl_br_vlan_gopts_entry *entry);
int rtnl_br_vlan_gopts_entry_get_msti(
const struct rtnl_br_vlan_gopts_entry *entry, uint16_t *out);

#ifdef __cplusplus
}
#endif

#endif /* NETLINK_BR_VLAN_GLOBAL_OPTS_H_ */
15 changes: 15 additions & 0 deletions include/nl-aux-route/nl-route.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,21 @@ void rtnl_cls_put(struct rtnl_cls *);
_NL_AUTO_DEFINE_FCN_TYPED0(struct rtnl_cls *, _nl_auto_rtnl_cls_fcn,
rtnl_cls_put);

struct rtnl_br_vlan_gopts;
void rtnl_br_vlan_gopts_put(struct rtnl_br_vlan_gopts *);
#define _nl_auto_rtnl_br_vlan_gopts _nl_auto(_nl_auto_rtnl_br_vlan_gopts_fcn)
_NL_AUTO_DEFINE_FCN_TYPED0(struct rtnl_br_vlan_gopts *,
_nl_auto_rtnl_br_vlan_gopts_fcn,
rtnl_br_vlan_gopts_put);

struct rtnl_br_vlan_gopts_entry;
void rtnl_br_vlan_gopts_entry_free(struct rtnl_br_vlan_gopts_entry *);
#define _nl_auto_rtnl_br_vlan_gopts_entry \
_nl_auto(_nl_auto_rtnl_br_vlan_gopts_entry_fcn)
_NL_AUTO_DEFINE_FCN_TYPED0(struct rtnl_br_vlan_gopts_entry *,
_nl_auto_rtnl_br_vlan_gopts_entry_fcn,
rtnl_br_vlan_gopts_entry_free);

/*****************************************************************************/

static inline int _rtnl_act_append_get(struct rtnl_act **head,
Expand Down
1 change: 1 addition & 0 deletions lib/error.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ static const char *errmsg[NLE_MAX+1] = {
[NLE_IMMUTABLE] = "Immutable attribute",
[NLE_DUMP_INTR] = "Dump inconsistency detected, interrupted",
[NLE_ATTRSIZE] = "Attribute max length exceeded",
[NLE_MERGE_FAILURE] = "Unable to merge objects",
};

/**
Expand Down
41 changes: 31 additions & 10 deletions lib/nl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1137,12 +1137,21 @@ struct pickup_param
static int __store_answer(struct nl_object *obj, struct nl_parser_param *p)
{
struct pickup_param *pp = p->pp_arg;
/*
* the parser will put() the object at the end, expecting the cache
* to take the reference.
*/
nl_object_get(obj);
pp->result = obj;

if (pp->result == NULL) {
/* The parser will put() the object at the end, expecting the
* cache to take the reference. This line ensures the object
* won't get deleted when that happens.
*/
nl_object_get(obj);

pp->result = obj;

return 0;
}

if (nl_object_update(pp->result, obj) < 0)
return -NLE_MERGE_FAILURE;

return 0;
}
Expand All @@ -1168,11 +1177,15 @@ static int __pickup_answer_syserr(struct sockaddr_nl *nla, struct nlmsgerr *nler
/** @endcond */

/**
* Pickup netlink answer, parse is and return object
* Pickup netlink answer, parse it and return an object
* @arg sk Netlink socket
* @arg parser Parser function to parse answer
* @arg result Result pointer to return parsed object
*
* @note If this function returns success, result may be NULL, so the caller
* should check that before accessing the result. This can happen if the
* kernel doesn't return any object data and reports success.
*
* @return 0 on success or a negative error code.
*/
int nl_pickup(struct nl_sock *sk,
Expand All @@ -1184,11 +1197,17 @@ int nl_pickup(struct nl_sock *sk,
}

/**
* Pickup netlink answer, parse is and return object with preserving system error
* Pickup netlink answer, parse it and return the object while preserving the
* system error
* @arg sk Netlink socket
* @arg parser Parser function to parse answer
* @arg result Result pointer to return parsed object
* @arg syserr Result pointer for the system error in case of failure
* @arg syserror Result pointer for the system error in case of failure
* (optional)
*
* @note If this function returns success, result may be NULL, so the caller
* should check that before accessing the result. This can happen if the
* kernel doesn't return any object data and reports success.
*
* @return 0 on success or a negative error code.
*/
Expand Down Expand Up @@ -1216,8 +1235,10 @@ int nl_pickup_keep_syserr(struct nl_sock *sk,
}

err = nl_recvmsgs(sk, cb);
if (err < 0)
if (err < 0) {
nl_object_put(pp.result);
goto errout;
}

*result = pp.result;
errout:
Expand Down
Loading
Loading