[PATCH v3] cache_mngr: add include callback v2

Tobias Jungel tobi at bisdn.de
Fri Oct 7 12:51:24 PDT 2016


Hi,

just a minor clarification. I currently skipped the NL_ACT_UPDATE, that
we spoke about previously. Former delete notifications notify using an
object, that was in the cache. However its removed from the cache
previous to the notification, hence I don't see a difference to an
object that never was in any cache. In case there is a notable
difference I will move to the NL_ACT_CHANGE otherwise all "old" objects
in the v2 don't have a cache relation.

more inline

best
Tobi

On Fr, 2016-10-07 at 21:32 +0200, Tobias Jungel wrote:
> This patch adds change_func_v2_t to add a more detailed callback in
> case of a cache change. The change function is registered using the
> new
> nl_cache_mngr_add_cache_v2. In case the new change function is set,
> nl_cache_include_v2 and thus cache_include_v2 will be used to perform
> the cache
> inclusion.
> 
> The parameter of change_func_v2_t are the following:
> * struct nl_cache * => cache
> * struct nl_object * => the old/deleted nl_object
> * struct nl_object * => the new nl_object
> * uint64_t => the result of nl_object_diff64 in case of a change
> * int => NL_ACT_*
> * void * => data
> 
> closes #71
> ---
>  include/netlink-private/cache-api.h |  3 +-
>  include/netlink-private/types.h     |  1 +
>  include/netlink/cache.h             |  9 ++++
>  lib/cache.c                         | 68 +++++++++++++++++++++++++
> ----
>  lib/cache_mngr.c                    | 87
> ++++++++++++++++++++++++++++++++++++-
>  lib/xfrm/sa.c                       | 31 ++++++++++---
>  libnl-3.sym                         |  2 +
>  7 files changed, 182 insertions(+), 19 deletions(-)
> 
> diff --git a/include/netlink-private/cache-api.h b/include/netlink-
> private/cache-api.h
> index f3d9f01..c684e79 100644
> --- a/include/netlink-private/cache-api.h
> +++ b/include/netlink-private/cache-api.h
> @@ -237,7 +237,8 @@ struct nl_cache_ops
>  	 * @see nl_cache_include()
>  	 */
>  	int   (*co_include_event)(struct nl_cache *cache, struct
> nl_object *obj,
> -				  change_func_t change_cb, void
> *data);
> +				  change_func_t change_cb,
> change_func_v2_t change_cb_v2,
> +				  void *data);
>  
>  	void (*reserved_1)(void);
>  	void (*reserved_2)(void);
> diff --git a/include/netlink-private/types.h b/include/netlink-
> private/types.h
> index e80c8a1..4702162 100644
> --- a/include/netlink-private/types.h
> +++ b/include/netlink-private/types.h
> @@ -96,6 +96,7 @@ struct nl_cache_assoc
>  {
>  	struct nl_cache *	ca_cache;
>  	change_func_t		ca_change;
> +	change_func_v2_t	ca_change_v2;
>  	void *			ca_change_data;
>  };
>  
> diff --git a/include/netlink/cache.h b/include/netlink/cache.h
> index 71eaceb..c0797d0 100644
> --- a/include/netlink/cache.h
> +++ b/include/netlink/cache.h
> @@ -35,6 +35,8 @@ enum {
>  
>  struct nl_cache;
>  typedef void (*change_func_t)(struct nl_cache *, struct nl_object *,
> int, void *);
> +typedef void (*change_func_v2_t)(struct nl_cache *, struct nl_object
> *old_obj,
> +	      struct nl_object *new_obj, uint64_t, int, void *);
>  
>  /**
>   * @ingroup cache
> @@ -88,6 +90,10 @@ extern int			nl_cache_include
> (struct nl_cache *,
>  						 struct nl_object *,
>  						 change_func_t,
>  						 void *);
> +extern int			nl_cache_include_v2(struct
> nl_cache *,
> +						    struct nl_object
> *,
> +						    change_func_v2_t
> ,
> +						    void *);
>  extern void			nl_cache_set_arg1(struct nl_cache
> *, int);
>  extern void			nl_cache_set_arg2(struct nl_cache
> *, int);
>  extern void			nl_cache_set_flags(struct
> nl_cache *, unsigned int);
> @@ -154,6 +160,9 @@ extern int			nl_cache_mngr_a
> dd(struct nl_cache_mngr *,
>  extern int			nl_cache_mngr_add_cache(struct
> nl_cache_mngr *mngr,
>  							struct
> nl_cache *cache,
>  							change_func_
> t cb, void *data);
> +extern int			nl_cache_mngr_add_cache_v2(struct
> nl_cache_mngr *mngr,
> +							   struct
> nl_cache *cache,
> +							   change_fu
> nc_v2_t cb, void *data);
>  extern int			nl_cache_mngr_get_fd(struct
> nl_cache_mngr *);
>  extern int			nl_cache_mngr_poll(struct
> nl_cache_mngr *,
>  						   int);
> diff --git a/lib/cache.c b/lib/cache.c
> index d8592b6..d9742b6 100644
> --- a/lib/cache.c
> +++ b/lib/cache.c
> @@ -784,30 +784,45 @@ int nl_cache_pickup(struct nl_sock *sk, struct
> nl_cache *cache)
>  }
>  
>  static int cache_include(struct nl_cache *cache, struct nl_object
> *obj,
> -			 struct nl_msgtype *type, change_func_t cb,
> void *data)
> +			 struct nl_msgtype *type, change_func_t cb,
> +			 change_func_v2_t cb_v2, void *data)
>  {
>  	struct nl_object *old;
> +	struct nl_object *clone = NULL;
> +	uint64_t diff;
>  
>  	switch (type->mt_act) {
>  	case NL_ACT_NEW:
>  	case NL_ACT_DEL:
>  		old = nl_cache_search(cache, obj);
>  		if (old) {
> +			if (cb_v2 && old->ce_ops->oo_update) {

I guess this should be fine, since previous calls already checked for
ce_ops, right?

> +				clone = nl_object_clone(old);
> +				diff = nl_object_diff64(old, obj);
> +			}
>  			/*
>  			 * Some objects types might support merging
> the new
>  			 * object with the old existing cache
> object.
>  			 * Handle them first.
>  			 */
>  			if (nl_object_update(old, obj) == 0) {
> -				if (cb)
> +				if (cb_v2) {
> +					cb_v2(cache, clone, obj,
> diff,
> +					      NL_ACT_CHANGE, data);
> +					nl_object_put(clone);
> +				} else if (cb)
>  					cb(cache, old,
> NL_ACT_CHANGE, data);
>  				nl_object_put(old);
>  				return 0;
>  			}
> +			nl_object_put(clone);
>  
>  			nl_cache_remove(old);
>  			if (type->mt_act == NL_ACT_DEL) {
> -				if (cb)
> +				if (cb_v2)
> +					cb_v2(cache, old, NULL, 0,
> NL_ACT_DEL,
> +					      data);
> +				else if (cb)
>  					cb(cache, old, NL_ACT_DEL,
> data);
>  				nl_object_put(old);
>  			}
> @@ -815,10 +830,20 @@ static int cache_include(struct nl_cache
> *cache, struct nl_object *obj,
>  
>  		if (type->mt_act == NL_ACT_NEW) {
>  			nl_cache_move(cache, obj);
> -			if (old == NULL && cb)
> -				cb(cache, obj, NL_ACT_NEW, data);
> -			else if (old) {
> -				if (nl_object_diff(old, obj) && cb)
> +			if (old == NULL) {
> +				if (cb_v2) {
> +					cb_v2(cache, NULL, obj, 0,
> NL_ACT_NEW,
> +					      data);
> +				} else if (cb)
> +					cb(cache, obj, NL_ACT_NEW,
> data);
> +			} else if (old) {
> +				uint64_t diff = 0;
> +				if (cb || cb_v2)
> +					diff = nl_object_diff64(old,
> obj);
> +				if (diff && cb_v2) {
> +					cb_v2(cache, old, obj, diff,
> NL_ACT_CHANGE,
> +					      data);
> +				} else if (diff && cb)
>  					cb(cache, obj,
> NL_ACT_CHANGE, data);
>  
>  				nl_object_put(old);
> @@ -845,7 +870,27 @@ int nl_cache_include(struct nl_cache *cache,
> struct nl_object *obj,
>  	for (i = 0; ops->co_msgtypes[i].mt_id >= 0; i++)
>  		if (ops->co_msgtypes[i].mt_id == obj->ce_msgtype)
>  			return cache_include(cache, obj, &ops-
> >co_msgtypes[i],
> -					     change_cb, data);
> +					     change_cb, NULL, data);
> +
> +	NL_DBG(3, "Object %p does not seem to belong to cache %p
> <%s>\n",
> +	       obj, cache, nl_cache_name(cache));
> +
> +	return -NLE_MSGTYPE_NOSUPPORT;
> +}
> +
> +int nl_cache_include_v2(struct nl_cache *cache, struct nl_object
> *obj,
> +			change_func_v2_t change_cb, void *data)
> +{
> +	struct nl_cache_ops *ops = cache->c_ops;
> +	int i;
> +
> +	if (ops->co_obj_ops != obj->ce_ops)
> +		return -NLE_OBJ_MISMATCH;
> +
> +	for (i = 0; ops->co_msgtypes[i].mt_id >= 0; i++)
> +		if (ops->co_msgtypes[i].mt_id == obj->ce_msgtype)
> +			return cache_include(cache, obj, &ops-
> >co_msgtypes[i],
> +						NULL, change_cb,
> data);
>  
>  	NL_DBG(3, "Object %p does not seem to belong to cache %p
> <%s>\n",
>  	       obj, cache, nl_cache_name(cache));
> @@ -857,7 +902,12 @@ static int resync_cb(struct nl_object *c, struct
> nl_parser_param *p)
>  {
>  	struct nl_cache_assoc *ca = p->pp_arg;
>  
> -	return nl_cache_include(ca->ca_cache, c, ca->ca_change, ca-
> >ca_change_data);
> +	if (ca->ca_change_v2)
> +		return nl_cache_include_v2(ca->ca_cache, c, ca-
> >ca_change_v2,
> +					   ca->ca_change_data);
> +	else
> +		return nl_cache_include(ca->ca_cache, c, ca-
> >ca_change,
> +					ca->ca_change_data);
>  }
>  
>  int nl_cache_resync(struct nl_sock *sk, struct nl_cache *cache,
> diff --git a/lib/cache_mngr.c b/lib/cache_mngr.c
> index 1f23eb1..8c0f595 100644
> --- a/lib/cache_mngr.c
> +++ b/lib/cache_mngr.c
> @@ -60,9 +60,15 @@ static int include_cb(struct nl_object *obj,
> struct nl_parser_param *p)
>  
>  	if (ops->co_include_event)
>  		return ops->co_include_event(ca->ca_cache, obj, ca-
> >ca_change,
> +					     ca->ca_change_v2,
>  					     ca->ca_change_data);
> -	else
> -		return nl_cache_include(ca->ca_cache, obj, ca-
> >ca_change, ca->ca_change_data);
> +	else {
> +		if (ca->ca_change_v2)
> +			return nl_cache_include_v2(ca->ca_cache,
> obj, ca->ca_change_v2, ca->ca_change_data);
> +		else
> +			return nl_cache_include(ca->ca_cache, obj,
> ca->ca_change, ca->ca_change_data);
> +	}
> +
>  }
>  
>  static int event_input(struct nl_msg *msg, void *arg)
> @@ -195,6 +201,70 @@ errout:
>  }
>  
>  /**
> + * Set change_func_v2 for cache manager
> + * @arg mngr		Cache manager.
> + * @arg cache		Cache associated with the callback
> + * @arg cb		Function to be called upon changes.
> + * @arg data		Argument passed on to change callback
> + *
> + * Adds callback change_func_v2 to a registered cache. This callback
> provides
> + * in like the standard change_func the added or remove netlink
> object. In case
> + * of a change the old and the new object is provided as well as the
> according
> + * diff. If this callback is registered this has a higher priority
> then the
> + * change_func registered during cache registration. Hence only one
> callback is
> + * executed.
> + *
> + * The first netlink object in the callback is refering to the old
> object and
> + * the second to the new. This means on NL_ACT_CHANGE the first is
> the previous
> + * object in the cache and the second the updated version. On
> NL_ACT_DEL the
> + * first is the deleted object the second is NULL. On NL_ACT_NEW the
> first is
> + * NULL and the second the new netlink object.
> + *
> + * The user is responsible for calling nl_cache_mngr_poll() or
> monitor
> + * the socket and call nl_cache_mngr_data_ready() to allow the
> library
> + * to process netlink notification events.
> + *
> + * @see nl_cache_mngr_poll()
> + * @see nl_cache_mngr_data_ready()
> + *
> + * @return 0 on success or a negative error code.
> + * @return -NLE_PROTO_MISMATCH Protocol mismatch between cache
> manager and
> + * 			       cache type
> + * @return -NLE_OPNOTSUPP Cache type does not support updates
> + * @return -NLE_RANGE Cache of this type is not registered
> + */
> +static int nl_cache_mngr_set_change_func_v2(struct nl_cache_mngr
> *mngr,
> +					    struct nl_cache *cache,
> +					    change_func_v2_t cb,
> void *data)
> +{
> +	struct nl_cache_ops *ops;
> +	int i;
> +
> +	ops = cache->c_ops;
> +	if (!ops)
> +		return -NLE_INVAL;
> +
> +	if (ops->co_protocol != mngr->cm_protocol)
> +		return -NLE_PROTO_MISMATCH;
> +
> +	if (ops->co_groups == NULL)
> +		return -NLE_OPNOTSUPP;
> +
> +	for (i = 0; i < mngr->cm_nassocs; i++)
> +		if (mngr->cm_assocs[i].ca_cache == cache)
> +			break;
> +
> +	if (i >= mngr->cm_nassocs) {
> +		return -NLE_RANGE;
> +	}
> +
> +	mngr->cm_assocs[i].ca_change_v2 = cb;
> +	mngr->cm_assocs[i].ca_change_data = data;
> +
> +	return 0;
> +}
> +
> +/**
>   * Add cache to cache manager
>   * @arg mngr		Cache manager.
>   * @arg cache		Cache to be added to cache manager
> @@ -292,6 +362,19 @@ errout_drop_membership:
>  }
>  
>  /**
> + * XXX

just seen this one... will add a v4.

> + */
> +int nl_cache_mngr_add_cache_v2(struct nl_cache_mngr *mngr, struct
> nl_cache *cache,
> +		      change_func_v2_t cb, void *data) {
> +	int err;
> +	err = nl_cache_mngr_add_cache(mngr, cache, NULL, NULL);
> +	if (err < 0)
> +		return err;
> +
> +	return nl_cache_mngr_set_change_func_v2(mngr, cache, cb,
> data);
> +}
> +
> +/**
>   * Add cache to cache manager
>   * @arg mngr		Cache manager.
>   * @arg name		Name of cache to keep track of
> diff --git a/lib/xfrm/sa.c b/lib/xfrm/sa.c
> index 9e2c353..38b7f5e 100644
> --- a/lib/xfrm/sa.c
> +++ b/lib/xfrm/sa.c
> @@ -919,7 +919,8 @@ errout:
>  }
>  
>  static int xfrm_sa_update_cache (struct nl_cache *cache, struct
> nl_object *obj,
> -                                 change_func_t change_cb, void
> *data)
> +                                 change_func_t change_cb,
> change_func_v2_t change_cb_v2,
> +				 void *data)
>  {
>  	struct nl_object*       old_sa;
>  	struct xfrmnl_sa*       sa = (struct xfrmnl_sa*)obj;
> @@ -948,18 +949,29 @@ static int xfrm_sa_update_cache (struct
> nl_cache *cache, struct nl_object *obj,
>  			 * cache and notify application of the
> expiry event. */
>  			nl_cache_move (cache, obj);
>  
> -			if (old_sa == NULL && change_cb)
> +			if (old_sa == NULL)
>  			{
>  				/* Application CB present, no
> previous instance of SA object present.
>  				 * Notify application CB as a NEW
> event */
> -				change_cb (cache, obj, NL_ACT_NEW,
> data);
> +				if (change_cb_v2)
> +					change_cb_v2(cache, NULL,
> obj, 0, NL_ACT_NEW, data);
> +				else if (change_cb)
> +					change_cb(cache, obj,
> NL_ACT_NEW, data);
>  			}
>  			else if (old_sa)
>  			{
> +				uint64_t diff = 0;
> +				if (change_cb || change_cb_v2)
> +					diff =
> nl_object_diff64(old_sa, obj);
> +
>  				/* Application CB present, a
> previous instance of SA object present.
>  				 * Notify application CB as a
> CHANGE1 event */
> -				if (nl_object_diff (old_sa, obj) &&
> change_cb)
> -					change_cb (cache, obj,
> NL_ACT_CHANGE, data);
> +				if (diff) {
> +					if (change_cb_v2) {
> +						change_cb_v2(cache,
> old_sa, obj, diff, NL_ACT_CHANGE, data);
> +					} else if (change_cb)
> +						change_cb(cache,
> obj, NL_ACT_CHANGE, data);
> +				}
>  				nl_object_put (old_sa);
>  			}
>  		}
> @@ -967,7 +979,9 @@ static int xfrm_sa_update_cache (struct nl_cache
> *cache, struct nl_object *obj,
>  		{
>  			/* Hard expiry event: Delete the object from
> the
>  			 * cache and notify application of the
> expiry event. */
> -			if (change_cb)
> +			if (change_cb_v2)
> +				change_cb_v2(cache, obj, NULL, 0,
> NL_ACT_DEL, data);
> +			else if (change_cb)
>  				change_cb (cache, obj, NL_ACT_DEL,
> data);
>  			nl_object_put (old_sa);
>  		}
> @@ -979,7 +993,10 @@ static int xfrm_sa_update_cache (struct nl_cache
> *cache, struct nl_object *obj,
>  	{
>  		/* All other messages other than Expire, let the
> standard Libnl cache
>  		 * module handle it. */
> -		return nl_cache_include (cache, obj, change_cb,
> data);
> +		if (change_cb_v2)
> +			return nl_cache_include_v2(cache, obj,
> change_cb_v2, data);
> +		else
> +			return nl_cache_include (cache, obj,
> change_cb, data);
>  	}
>  }
>  
> diff --git a/libnl-3.sym b/libnl-3.sym
> index 9119e66..4546a40 100644
> --- a/libnl-3.sym
> +++ b/libnl-3.sym
> @@ -354,5 +354,7 @@ global:
>  
>  libnl_3_2_29 {
>  global:
> +	nl_cache_include_v2;
> +	nl_cache_mngr_add_cache_v2;
>  	nl_strerror_l;
>  } libnl_3_2_28;



More information about the libnl mailing list