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

Jump to bottom

Allow to allocate cache manager with custom refill socket #378

Closed
wants to merge 1 commit into from
Closed

Allow to allocate cache manager with custom refill socket #378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 7 additions & 2 deletions include/netlink/cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,17 @@ extern struct nl_cache * __nl_cache_mngt_require(const char *);

struct nl_cache_mngr;

#define NL_AUTO_PROVIDE 1
#define NL_ALLOCATED_SOCK 2 /* For internal use only, do not use */
#define NL_AUTO_PROVIDE 1
#define NL_ALLOCATED_SOCK 2 /* For internal use only, do not use */
#define NL_ALLOCATED_SYNC_SOCK 4 /* For internal use only, do not use */

extern int nl_cache_mngr_alloc(struct nl_sock *,
int, int,
struct nl_cache_mngr **);
extern int nl_cache_mngr_alloc_ex(struct nl_sock *,
struct nl_sock *,
int, int,
struct nl_cache_mngr **);
extern int nl_cache_mngr_add(struct nl_cache_mngr *,
const char *,
change_func_t,
Expand Down
51 changes: 38 additions & 13 deletions lib/cache_mngr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,17 +148,46 @@ static int event_input(struct nl_msg *msg, void *arg)
*/
int nl_cache_mngr_alloc(struct nl_sock *sk, int protocol, int flags,
struct nl_cache_mngr **result)
{
return nl_cache_mngr_alloc_ex(sk, NULL, protocol, flags, result);
}

/**
* Allocate new cache manager, with custom callback on refill socket
* @arg sk Netlink socket or NULL to auto allocate
* @arg sync_sk Blocking Netlink socket for cache refills
* @arg protocol Netlink protocol this manager is used for
* @arg flags Flags (\c NL_AUTO_PROVIDE)
* @arg result Result pointer
*
* Same as \f nl_cache_mngr_alloc, but sets custom refill socket
* Note: ownership of the sync_sk passes to the cache manager
*/
int nl_cache_mngr_alloc_ex(struct nl_sock *sk, struct nl_sock *sync_sk, int protocol, int flags,
struct nl_cache_mngr **result)
{
struct nl_cache_mngr *mngr;
int err = -NLE_NOMEM;

/* Catch abuse of flags */
if (flags & NL_ALLOCATED_SOCK)
if (flags & (NL_ALLOCATED_SOCK|NL_ALLOCATED_SYNC_SOCK))
BUG();

mngr = calloc(1, sizeof(*mngr));
if (!mngr)
if (!mngr) {
return -NLE_NOMEM;
}

if(!sync_sk) {
if (!(sync_sk = nl_socket_alloc()))
goto errout;

flags |= NL_ALLOCATED_SOCK;
}
/* have to assign before calling nl_connect, so that it gets freed in case
* of an nl_socket_allock error for sk
*/
mngr->cm_sync_sock = sync_sk;

if (!sk) {
if (!(sk = nl_socket_alloc()))
Expand All @@ -176,6 +205,10 @@ int nl_cache_mngr_alloc(struct nl_sock *sk, int protocol, int flags,
if (!mngr->cm_assocs)
goto errout;

if ((err = nl_connect(sync_sk, protocol)) < 0) {
goto errout;
}

/* Required to receive async event notifications */
nl_socket_disable_seq_check(mngr->cm_sock);

Expand All @@ -185,23 +218,13 @@ int nl_cache_mngr_alloc(struct nl_sock *sk, int protocol, int flags,
if ((err = nl_socket_set_nonblocking(mngr->cm_sock)) < 0)
goto errout;

/* Create and allocate socket for sync cache fills */
mngr->cm_sync_sock = nl_socket_alloc();
if (!mngr->cm_sync_sock) {
err = -NLE_NOMEM;
goto errout;
}
if ((err = nl_connect(mngr->cm_sync_sock, protocol)) < 0)
goto errout_free_sync_sock;
Copy link
Owner

Choose a reason for hiding this comment

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

not how for mngr->cm_sock, we call connect and non-blocking above. That means, for the sk argument, the caller is not expected to do that. That may be a questionable choice, but it is probably better to handle his consistently. Meaning: leave the nl_connect(cm_sync_sock) call here, also for the externally provided sk_blocking.


NL_DBG(1, "Allocated cache manager %p, protocol %d, %d caches\n",
mngr, protocol, mngr->cm_nassocs);

*result = mngr;
return 0;

errout_free_sync_sock:
nl_socket_free(mngr->cm_sync_sock);
Copy link
Owner

Choose a reason for hiding this comment

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

like sk, the new parameter sync_sk/sk_blocking should be optional/nullable. If the user did not specify a blocking socket, in case of error the internally allocated one will still be needed to released.

You need to right logic, so that all internally allocated data is correctly freed on every error path. Use _nl_auto_nl_socket instead of explicit free for that.

errout:
nl_cache_mngr_free(mngr);
return err;
Expand Down Expand Up @@ -624,9 +647,11 @@ void nl_cache_mngr_free(struct nl_cache_mngr *mngr)

if (mngr->cm_sync_sock) {
nl_close(mngr->cm_sync_sock);
nl_socket_free(mngr->cm_sync_sock);
}

if (mngr->cm_flags & NL_ALLOCATED_SYNC_SOCK)
nl_socket_free(mngr->cm_sync_sock);

if (mngr->cm_flags & NL_ALLOCATED_SOCK)
nl_socket_free(mngr->cm_sock);

Expand Down
5 changes: 5 additions & 0 deletions libnl-3.sym
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,8 @@ global:

libnl_3_6 {
} libnl_3_5;

libnl_3_10 {
global:
nl_cache_mngr_alloc_ex;
} libnl_3_6;
Loading