PATCH: IPSEC xfrm events

PATCH: IPSEC xfrm events

[jamal]

[-- Attachment #1: Type: text/plain, Size: 233 bytes --]


Dave,

Heres the final patch.
What this patch provides

- netlink xfrm events
- ability to have events generated by netlink propagated to pfkey
and vice versa.
- fixes the acquire lets-be-happy-with-one-success issue

cheers,
jamal

[-- Attachment #2: ipsec-event-take2-6 --]
[-- Type: text/plain, Size: 28779 bytes --]

--- a/include/net/xfrm.h	2005-04-05 07:19:11.000000000 -0400
+++ b/include/net/xfrm.h	2005-04-05 07:29:00.000000000 -0400
@@ -157,6 +157,28 @@
 	XFRM_STATE_DEAD
 };
 
+/* events that could be sent by kernel */
+enum {
+	XFRM_SAP_INVALID,
+	XFRM_SAP_EXPIRED,
+	XFRM_SAP_ADDED,
+	XFRM_SAP_UPDATED,
+	XFRM_SAP_DELETED,
+	XFRM_SAP_FLUSHED,
+	__XFRM_SAP_MAX
+};
+#define XFRM_SAP_MAX (__XFRM_SAP_MAX - 1)
+
+/* callback structure passed from either netlink or pfkey */
+struct km_event
+{
+	u32	data; 
+	u32	seq; 
+	u32	pid; 
+	u32	event; 
+};
+
+
 struct xfrm_type;
 struct xfrm_dst;
 struct xfrm_policy_afinfo {
@@ -178,6 +200,9 @@
 
 extern int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo);
 extern int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo);
+extern void km_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c);
+extern void km_state_notify(struct xfrm_state *x, struct km_event *c);
+
 
 #define XFRM_ACQ_EXPIRES	30
 
@@ -283,17 +308,17 @@
 	struct xfrm_tmpl       	xfrm_vec[XFRM_MAX_DEPTH];
 };
 
-#define XFRM_KM_TIMEOUT		30
 
+#define XFRM_KM_TIMEOUT		30
 struct xfrm_mgr
 {
 	struct list_head	list;
 	char			*id;
-	int			(*notify)(struct xfrm_state *x, int event);
+	int			(*notify)(struct xfrm_state *x, struct km_event *c);
 	int			(*acquire)(struct xfrm_state *x, struct xfrm_tmpl *, struct xfrm_policy *xp, int dir);
 	struct xfrm_policy	*(*compile_policy)(u16 family, int opt, u8 *data, int len, int *dir);
 	int			(*new_mapping)(struct xfrm_state *x, xfrm_address_t *ipaddr, u16 sport);
-	int			(*notify_policy)(struct xfrm_policy *x, int dir, int event);
+	int			(*notify_policy)(struct xfrm_policy *x, int dir, struct km_event *c);
 };
 
 extern int xfrm_register_km(struct xfrm_mgr *km);
@@ -805,7 +830,7 @@
 extern int xfrm_state_update(struct xfrm_state *x);
 extern struct xfrm_state *xfrm_state_lookup(xfrm_address_t *daddr, u32 spi, u8 proto, unsigned short family);
 extern struct xfrm_state *xfrm_find_acq_byseq(u32 seq);
-extern void xfrm_state_delete(struct xfrm_state *x);
+extern int xfrm_state_delete(struct xfrm_state *x);
 extern void xfrm_state_flush(u8 proto);
 extern int xfrm_replay_check(struct xfrm_state *x, u32 seq);
 extern void xfrm_replay_advance(struct xfrm_state *x, u32 seq);
--- a/include/linux/xfrm.h	2005-03-02 02:38:37.000000000 -0500
+++ b/include/linux/xfrm.h	2005-04-05 07:29:00.000000000 -0400
@@ -254,5 +254,7 @@
 
 #define XFRMGRP_ACQUIRE		1
 #define XFRMGRP_EXPIRE		2
+#define XFRMGRP_SA		4
+#define XFRMGRP_POLICY		8
 
 #endif /* _LINUX_XFRM_H */
--- a/net/xfrm/xfrm_state.c	2005-04-05 07:19:30.000000000 -0400
+++ b/net/xfrm/xfrm_state.c	2005-04-05 07:29:00.000000000 -0400
@@ -50,7 +50,7 @@
 
 static int xfrm_state_gc_flush_bundles;
 
-static void __xfrm_state_delete(struct xfrm_state *x);
+static int __xfrm_state_delete(struct xfrm_state *x);
 
 static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned short family);
 static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo);
@@ -215,8 +215,10 @@
 }
 EXPORT_SYMBOL(__xfrm_state_destroy);
 
-static void __xfrm_state_delete(struct xfrm_state *x)
+static int __xfrm_state_delete(struct xfrm_state *x)
 {
+	int err = -ESRCH;
+
 	if (x->km.state != XFRM_STATE_DEAD) {
 		x->km.state = XFRM_STATE_DEAD;
 		spin_lock(&xfrm_state_lock);
@@ -245,14 +247,21 @@
 		 * is what we are dropping here.
 		 */
 		atomic_dec(&x->refcnt);
+		err = 0;
 	}
+
+	return err;
 }
 
-void xfrm_state_delete(struct xfrm_state *x)
+int xfrm_state_delete(struct xfrm_state *x)
 {
+	int err;
+
 	spin_lock_bh(&x->lock);
-	__xfrm_state_delete(x);
+	err = __xfrm_state_delete(x);
 	spin_unlock_bh(&x->lock);
+
+	return err;
 }
 EXPORT_SYMBOL(xfrm_state_delete);
 
@@ -430,6 +439,7 @@
 
 static struct xfrm_state *__xfrm_find_acq_byseq(u32 seq);
 
+
 int xfrm_state_add(struct xfrm_state *x)
 {
 	struct xfrm_state_afinfo *afinfo;
@@ -795,34 +805,60 @@
 static struct list_head xfrm_km_list = LIST_HEAD_INIT(xfrm_km_list);
 static DEFINE_RWLOCK(xfrm_km_lock);
 
-static void km_state_expired(struct xfrm_state *x, int hard)
+void km_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct xfrm_mgr *km;
 
-	if (hard)
-		x->km.state = XFRM_STATE_EXPIRED;
-	else
-		x->km.dying = 1;
+	read_lock(&xfrm_km_lock);
+	list_for_each_entry(km, &xfrm_km_list, list)
+		if (km->notify_policy)
+			km->notify_policy(xp, dir, c);
+	read_unlock(&xfrm_km_lock);
+}
 
+void km_state_notify(struct xfrm_state *x, struct km_event *c)
+{
+	struct xfrm_mgr *km;
 	read_lock(&xfrm_km_lock);
 	list_for_each_entry(km, &xfrm_km_list, list)
-		km->notify(x, hard);
+		if (km->notify)
+			km->notify(x, c);
 	read_unlock(&xfrm_km_lock);
+}
+
+EXPORT_SYMBOL(km_policy_notify);
+EXPORT_SYMBOL(km_state_notify);
+
+static void km_state_expired(struct xfrm_state *x, int hard)
+{
+	struct km_event c;
+
+	if (hard)
+		x->km.state = XFRM_STATE_EXPIRED;
+	else
+		x->km.dying = 1;
+	c.data = hard;
+	c.event = XFRM_SAP_EXPIRED;
+	km_state_notify(x, &c);
 
 	if (hard)
 		wake_up(&km_waitq);
 }
 
+/*
+ * We send to all registered managers regardless of failure
+ * We are happy with one success
+*/
 static int km_query(struct xfrm_state *x, struct xfrm_tmpl *t, struct xfrm_policy *pol)
 {
-	int err = -EINVAL;
+	int err = -EINVAL, acqret;
 	struct xfrm_mgr *km;
 
 	read_lock(&xfrm_km_lock);
 	list_for_each_entry(km, &xfrm_km_list, list) {
-		err = km->acquire(x, t, pol, XFRM_POLICY_OUT);
-		if (!err)
-			break;
+		acqret = km->acquire(x, t, pol, XFRM_POLICY_OUT);
+		if (!acqret)
+			err = acqret;
 	}
 	read_unlock(&xfrm_km_lock);
 	return err;
@@ -847,13 +883,12 @@
 
 void km_policy_expired(struct xfrm_policy *pol, int dir, int hard)
 {
-	struct xfrm_mgr *km;
+	struct km_event c;
 
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list)
-		if (km->notify_policy)
-			km->notify_policy(pol, dir, hard);
-	read_unlock(&xfrm_km_lock);
+	c.data = hard;
+	c.data = hard;
+	c.event = XFRM_SAP_EXPIRED;
+	km_policy_notify(pol, dir, &c);
 
 	if (hard)
 		wake_up(&km_waitq);
--- a/net/xfrm/xfrm_user.c	2005-03-02 02:38:10.000000000 -0500
+++ b/net/xfrm/xfrm_user.c	2005-04-05 07:47:45.000000000 -0400
@@ -268,6 +268,7 @@
 	struct xfrm_usersa_info *p = NLMSG_DATA(nlh);
 	struct xfrm_state *x;
 	int err;
+	struct km_event c;
 
 	err = verify_newsa_info(p, (struct rtattr **) xfrma);
 	if (err)
@@ -277,6 +278,7 @@
 	if (!x)
 		return err;
 
+	xfrm_state_hold(x);
 	if (nlh->nlmsg_type == XFRM_MSG_NEWSA)
 		err = xfrm_state_add(x);
 	else
@@ -285,14 +287,27 @@
 	if (err < 0) {
 		x->km.state = XFRM_STATE_DEAD;
 		xfrm_state_put(x);
+		return err;
 	}
 
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	if (nlh->nlmsg_type == XFRM_MSG_NEWSA)
+		c.event = XFRM_SAP_ADDED;
+	else
+		c.event = XFRM_SAP_UPDATED;
+
+	km_state_notify(x, &c);
+	xfrm_state_put(x);
+
 	return err;
 }
 
 static int xfrm_del_sa(struct sk_buff *skb, struct nlmsghdr *nlh, void **xfrma)
 {
 	struct xfrm_state *x;
+	int err;
+	struct km_event c;
 	struct xfrm_usersa_id *p = NLMSG_DATA(nlh);
 
 	x = xfrm_state_lookup(&p->daddr, p->spi, p->proto, p->family);
@@ -304,10 +319,19 @@
 		return -EPERM;
 	}
 
-	xfrm_state_delete(x);
+	err = xfrm_state_delete(x);
+	if (err < 0) {
+		xfrm_state_put(x);
+		return err;
+	}
+
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	c.event = XFRM_SAP_DELETED;
+	km_state_notify(x, &c);
 	xfrm_state_put(x);
 
-	return 0;
+	return err;
 }
 
 static void copy_to_user_state(struct xfrm_state *x, struct xfrm_usersa_info *p)
@@ -335,6 +359,7 @@
 	int this_idx;
 };
 
+
 static int dump_one_state(struct xfrm_state *x, int count, void *ptr)
 {
 	struct xfrm_dump_info *sp = ptr;
@@ -672,6 +697,7 @@
 {
 	struct xfrm_userpolicy_info *p = NLMSG_DATA(nlh);
 	struct xfrm_policy *xp;
+	struct km_event c;
 	int err;
 	int excl;
 
@@ -683,6 +709,10 @@
 	if (!xp)
 		return err;
 
+	/* shouldnt excl be based on nlh flags?? 
+	 * Aha! this is anti-netlink really i.e  more pfkey derived
+	 * in netlink excl is a flag and you wouldnt need
+	 * a type XFRM_MSG_UPDPOLICY - JHS */
 	excl = nlh->nlmsg_type == XFRM_MSG_NEWPOLICY;
 	err = xfrm_policy_insert(p->dir, xp, excl);
 	if (err) {
@@ -690,6 +720,16 @@
 		return err;
 	}
 
+
+	if (!excl)
+		c.event = XFRM_SAP_UPDATED;
+	else
+		c.event = XFRM_SAP_ADDED;
+
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	km_policy_notify(xp, p->dir, &c);
+
 	xfrm_pol_put(xp);
 
 	return 0;
@@ -807,8 +847,10 @@
 	struct xfrm_policy *xp;
 	struct xfrm_userpolicy_id *p;
 	int err;
+	struct km_event c;
 	int delete;
 
+
 	p = NLMSG_DATA(nlh);
 	delete = nlh->nlmsg_type == XFRM_MSG_DELPOLICY;
 
@@ -834,6 +876,11 @@
 					      NETLINK_CB(skb).pid,
 					      MSG_DONTWAIT);
 		}
+	} else {
+		c.event = XFRM_SAP_DELETED;
+		c.seq = nlh->nlmsg_seq;
+		c.pid = nlh->nlmsg_pid;
+		km_policy_notify(xp, p->dir, &c);
 	}
 
 	xfrm_pol_put(xp);
@@ -843,15 +890,28 @@
 
 static int xfrm_flush_sa(struct sk_buff *skb, struct nlmsghdr *nlh, void **xfrma)
 {
+	struct km_event c;
 	struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
 
 	xfrm_state_flush(p->proto);
+	c.data = p->proto;
+	c.event = XFRM_SAP_FLUSHED;
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	km_state_notify(NULL, &c);
+
 	return 0;
 }
 
 static int xfrm_flush_policy(struct sk_buff *skb, struct nlmsghdr *nlh, void **xfrma)
 {
+	struct km_event c;
+
 	xfrm_policy_flush();
+	c.event = XFRM_SAP_FLUSHED;
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	km_policy_notify(NULL, 0, &c);
 	return 0;
 }
 
@@ -1053,10 +1113,11 @@
 	return -1;
 }
 
-static int xfrm_send_state_notify(struct xfrm_state *x, int hard)
+static int xfrm_exp_state_notify(struct xfrm_state *x, struct km_event *c)
 {
 	struct sk_buff *skb;
-
+	int hard = c ->data;
+	/* fix to do alloc using NLM macros */
 	skb = alloc_skb(sizeof(struct xfrm_user_expire) + 16, GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
@@ -1069,6 +1130,122 @@
 	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_EXPIRE, GFP_ATOMIC);
 }
 
+static int xfrm_notify_sa_flush(struct km_event *c)
+{
+	struct xfrm_usersa_flush *p;
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	unsigned char *b;
+	int len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_flush));
+
+	skb = alloc_skb(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	b = skb->tail;
+
+	nlh = NLMSG_PUT(skb, c->pid,  c->seq,
+			XFRM_MSG_FLUSHSA, sizeof(*p));
+	nlh->nlmsg_flags = 0;
+
+	p = NLMSG_DATA(nlh);
+	p->proto = c->data;
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_SA, GFP_ATOMIC);
+
+nlmsg_failure:
+	kfree_skb(skb);
+	return -1;
+}
+
+static int inline xfrm_sa_len(struct xfrm_state *x)
+{
+	int l = NLMSG_LENGTH(sizeof(struct xfrm_usersa_info));
+	if (x->aalg)
+		l+= RTA_SPACE(sizeof(*(x->aalg))+(x->aalg->alg_key_len+7)/8);
+	if (x->ealg)
+		l+= RTA_SPACE(sizeof(*(x->ealg))+(x->ealg->alg_key_len+7)/8);
+	if (x->calg)
+		l+= RTA_SPACE(sizeof(*(x->calg)));
+	if (x->encap)
+		l+= RTA_SPACE(sizeof(*x->encap));
+
+	return l;
+}
+
+static int xfrm_notify_sa(struct xfrm_state *x, struct km_event *c)
+{
+	struct xfrm_usersa_info *p;
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	u32 nlt;
+	unsigned char *b;
+	int len = xfrm_sa_len(x);
+
+	skb = alloc_skb(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	b = skb->tail;
+
+	if (c->event == XFRM_SAP_ADDED)
+		nlt = XFRM_MSG_NEWSA;
+	else if (c->event == XFRM_SAP_UPDATED)
+		nlt = XFRM_MSG_UPDSA;
+	else if (c->event == XFRM_SAP_DELETED)
+		nlt = XFRM_MSG_DELSA;
+	else
+		goto nlmsg_failure;
+
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, nlt, sizeof(*p));
+	nlh->nlmsg_flags = 0;
+
+	p = NLMSG_DATA(nlh);
+	copy_to_user_state(x, p);
+
+	if (x->aalg)
+		RTA_PUT(skb, XFRMA_ALG_AUTH,
+			sizeof(*(x->aalg))+(x->aalg->alg_key_len+7)/8, x->aalg);
+	if (x->ealg)
+		RTA_PUT(skb, XFRMA_ALG_CRYPT,
+			sizeof(*(x->ealg))+(x->ealg->alg_key_len+7)/8, x->ealg);        
+	if (x->calg)
+		RTA_PUT(skb, XFRMA_ALG_COMP, sizeof(*(x->calg)), x->calg);
+
+	if (x->encap)
+		RTA_PUT(skb, XFRMA_ENCAP, sizeof(*x->encap), x->encap);
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_SA, GFP_ATOMIC);
+
+nlmsg_failure:
+rtattr_failure:
+	kfree_skb(skb);
+	return -1;
+}
+
+static int xfrm_send_state_notify(struct xfrm_state *x, struct km_event *c)
+{
+
+	switch (c->event) {
+	case XFRM_SAP_EXPIRED:
+		return xfrm_exp_state_notify(x, c);
+	case XFRM_SAP_DELETED:
+	case XFRM_SAP_UPDATED:
+	case XFRM_SAP_ADDED:
+		return xfrm_notify_sa(x, c);
+	case XFRM_SAP_FLUSHED:
+		return xfrm_notify_sa_flush(c);
+	default:
+		 printk("xfrm_user: Unknown SA event %d\n",c->event);
+		 break;
+	}
+
+	return 0;
+
+}
+
 static int build_acquire(struct sk_buff *skb, struct xfrm_state *x,
 			 struct xfrm_tmpl *xt, struct xfrm_policy *xp,
 			 int dir)
@@ -1202,7 +1379,8 @@
 	return -1;
 }
 
-static int xfrm_send_policy_notify(struct xfrm_policy *xp, int dir, int hard)
+
+static int xfrm_exp_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct sk_buff *skb;
 	size_t len;
@@ -1213,7 +1391,7 @@
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_polexpire(skb, xp, dir, hard) < 0)
+	if (build_polexpire(skb, xp, dir, c->data) < 0)
 		BUG();
 
 	NETLINK_CB(skb).dst_groups = XFRMGRP_EXPIRE;
@@ -1221,6 +1399,93 @@
 	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_EXPIRE, GFP_ATOMIC);
 }
 
+static int xfrm_notify_policy( struct xfrm_policy *xp, int dir, struct km_event *c)
+{
+	struct xfrm_userpolicy_info *p;
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	u32 nlt = 0 ;
+	unsigned char *b;
+	int len = RTA_SPACE(sizeof(struct xfrm_user_tmpl) * xp->xfrm_nr);
+	len += NLMSG_SPACE(sizeof(struct xfrm_userpolicy_info));
+
+	skb = alloc_skb(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	b = skb->tail;
+
+	if (c->event == XFRM_SAP_ADDED)
+		nlt = XFRM_MSG_NEWPOLICY;
+	else if (c->event == XFRM_SAP_UPDATED)
+		nlt = XFRM_MSG_UPDPOLICY;
+	else if (c->event == XFRM_SAP_DELETED)
+		nlt = XFRM_MSG_DELPOLICY;
+	else
+		goto nlmsg_failure;
+
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, nlt, sizeof(*p));
+
+	p = NLMSG_DATA(nlh);
+
+	nlh->nlmsg_flags = 0;
+
+	copy_to_user_policy(xp, p, dir);
+	if (copy_to_user_tmpl(xp, skb) < 0)
+		goto nlmsg_failure;
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_POLICY, GFP_ATOMIC);
+
+nlmsg_failure:
+	kfree_skb(skb);
+	return -1;
+}
+
+static int xfrm_notify_policy_flush(struct km_event *c)
+{
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	unsigned char *b;
+	int len = NLMSG_LENGTH(0);
+
+	skb = alloc_skb(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	b = skb->tail;
+
+
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, XFRM_MSG_FLUSHPOLICY, 0);
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_POLICY, GFP_ATOMIC);
+
+nlmsg_failure:
+	kfree_skb(skb);
+	return -1;
+}
+
+static int xfrm_send_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
+{
+
+	switch (c->event) {
+	case XFRM_SAP_ADDED:
+	case XFRM_SAP_UPDATED:
+	case XFRM_SAP_DELETED:
+		return xfrm_notify_policy(xp, dir, c);
+	case XFRM_SAP_FLUSHED:
+		return xfrm_notify_policy_flush(c);
+	case XFRM_SAP_EXPIRED:
+		return xfrm_exp_policy_notify(xp, dir, c);
+	default:
+		printk("xfrm_user: Unknown Policy event %d\n",c->event);
+	}
+
+	return 0;
+
+}
+
 static struct xfrm_mgr netlink_mgr = {
 	.id		= "netlink",
 	.notify		= xfrm_send_state_notify,
--- a/net/key/af_key.c	2005-04-05 07:19:26.000000000 -0400
+++ b/net/key/af_key.c	2005-04-05 07:48:31.000000000 -0400
@@ -1240,13 +1240,85 @@
 	return 0;
 }
 
+static inline int event2poltype (int event)
+{
+	switch (event) {
+	case XFRM_SAP_DELETED:
+		return SADB_X_SPDDELETE;
+	case XFRM_SAP_ADDED:
+		return SADB_X_SPDADD;
+	case XFRM_SAP_UPDATED:
+		return SADB_X_SPDUPDATE;
+	case XFRM_SAP_EXPIRED:
+	//	return SADB_X_SPDEXPIRE;
+	default:
+		printk("pfkey: Unknown policy event %d\n",event);
+		break;
+	}
+
+	return 0;
+}
+
+static inline int event2keytype (int event)
+{
+	switch (event) {
+	case XFRM_SAP_DELETED:
+		return SADB_DELETE;
+	case XFRM_SAP_ADDED:
+		return SADB_ADD;
+	case XFRM_SAP_UPDATED:
+		return SADB_UPDATE;
+	case XFRM_SAP_EXPIRED:
+		return SADB_EXPIRE;
+	default:
+		printk("pfkey: Unknown SA event %d\n",event);
+		break;
+	}
+
+	return 0;
+}
+
+/* ADD/UPD/DEL */
+static int key_notify_sa(struct xfrm_state *x, struct km_event *c)
+{
+	struct sk_buff *skb;
+	struct sadb_msg *hdr;
+	int hsc = 3;
+
+	if (c->event == XFRM_SAP_DELETED)
+		hsc = 0;
+
+	if (c->event == XFRM_SAP_EXPIRED) {
+		if (c->data)
+			hsc = 2;
+		else
+			hsc = 1;
+	}
+
+	skb = pfkey_xfrm_state2msg(x, 0, hsc);
+
+	if (IS_ERR(skb))
+		return  PTR_ERR(skb);
+
+	hdr = (struct sadb_msg *) skb->data;
+	hdr->sadb_msg_version = PF_KEY_V2;
+	hdr->sadb_msg_type = event2keytype(c->event);
+	hdr->sadb_msg_satype = pfkey_proto2satype(x->id.proto);
+	hdr->sadb_msg_errno = 0;
+	hdr->sadb_msg_reserved = 0;
+	hdr->sadb_msg_seq = c->seq;
+	hdr->sadb_msg_pid = c->pid;
+
+	pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL);
+
+	return 0;
+}
 
 static int pfkey_add(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
-	struct sk_buff *out_skb;
-	struct sadb_msg *out_hdr;
 	struct xfrm_state *x;
 	int err;
+	struct km_event c;
 
 	xfrm_probe_algs();
 	
@@ -1254,6 +1326,7 @@
 	if (IS_ERR(x))
 		return PTR_ERR(x);
 
+	xfrm_state_hold(x);
 	if (hdr->sadb_msg_type == SADB_ADD)
 		err = xfrm_state_add(x);
 	else
@@ -1265,27 +1338,23 @@
 		return err;
 	}
 
-	out_skb = pfkey_xfrm_state2msg(x, 0, 3);
-	if (IS_ERR(out_skb))
-		return  PTR_ERR(out_skb); /* XXX Should we return 0 here ? */
-
-	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version = hdr->sadb_msg_version;
-	out_hdr->sadb_msg_type = hdr->sadb_msg_type;
-	out_hdr->sadb_msg_satype = pfkey_proto2satype(x->id.proto);
-	out_hdr->sadb_msg_errno = 0;
-	out_hdr->sadb_msg_reserved = 0;
-	out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
-	out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
-
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, sk);
+	if (hdr->sadb_msg_type == SADB_ADD)
+		c.event = XFRM_SAP_ADDED;
+	else
+		c.event = XFRM_SAP_UPDATED;
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	km_state_notify(x, &c);
+	xfrm_state_put(x);
 
-	return 0;
+	return err;
 }
 
 static int pfkey_delete(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
 	struct xfrm_state *x;
+	struct km_event c;
+	int err;
 
 	if (!ext_hdrs[SADB_EXT_SA-1] ||
 	    !present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1],
@@ -1301,13 +1370,19 @@
 		return -EPERM;
 	}
 	
-	xfrm_state_delete(x);
-	xfrm_state_put(x);
+	err = xfrm_state_delete(x);
+	if (err < 0) {
+		xfrm_state_put(x);
+		return err;
+	}
 
-	pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL, 
-			BROADCAST_ALL, sk);
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	c.event = XFRM_SAP_DELETED;
+	km_state_notify(x, &c);
+	xfrm_state_put(x);
 
-	return 0;
+	return err;
 }
 
 static int pfkey_get(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
@@ -1445,28 +1520,42 @@
 	return 0;
 }
 
+static int key_notify_sa_flush(struct km_event *c)
+{
+	struct sk_buff *skb;
+	struct sadb_msg *hdr;
+
+	skb = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_ATOMIC);
+	if (!skb)
+		return -ENOBUFS;
+	hdr = (struct sadb_msg *) skb_put(skb, sizeof(struct sadb_msg));
+	hdr->sadb_msg_satype = pfkey_proto2satype(c->data);
+	hdr->sadb_msg_seq = c->seq;
+	hdr->sadb_msg_pid = c->pid;
+	hdr->sadb_msg_version = PF_KEY_V2;
+	hdr->sadb_msg_errno = (uint8_t) 0;
+	hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
+
+	pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL);
+
+	return 0;
+}
+
 static int pfkey_flush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
 	unsigned proto;
-	struct sk_buff *skb_out;
-	struct sadb_msg *hdr_out;
+	struct km_event c;
 
 	proto = pfkey_satype2proto(hdr->sadb_msg_satype);
 	if (proto == 0)
 		return -EINVAL;
 
-	skb_out = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_KERNEL);
-	if (!skb_out)
-		return -ENOBUFS;
-
 	xfrm_state_flush(proto);
-
-	hdr_out = (struct sadb_msg *) skb_put(skb_out, sizeof(struct sadb_msg));
-	pfkey_hdr_dup(hdr_out, hdr);
-	hdr_out->sadb_msg_errno = (uint8_t) 0;
-	hdr_out->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
-
-	pfkey_broadcast(skb_out, GFP_KERNEL, BROADCAST_ALL, NULL);
+	c.data = proto;
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	c.event = XFRM_SAP_FLUSHED;
+	km_state_notify(NULL, &c);
 
 	return 0;
 }
@@ -1859,6 +1948,35 @@
 	hdr->sadb_msg_reserved = atomic_read(&xp->refcnt);
 }
 
+static int key_notify_policy( struct xfrm_policy *xp, int dir, struct km_event *c)
+{
+	struct sk_buff *out_skb;
+	struct sadb_msg *out_hdr;
+	int err;
+
+	out_skb = pfkey_xfrm_policy2msg_prep(xp);
+	if (IS_ERR(out_skb)) {
+		err =  PTR_ERR(out_skb);
+		goto out;
+	}
+	pfkey_xfrm_policy2msg(out_skb, xp, dir);
+
+	out_hdr = (struct sadb_msg *) out_skb->data;
+	out_hdr->sadb_msg_version =  PF_KEY_V2;
+
+	if (c->data && c->event == XFRM_SAP_DELETED)
+		out_hdr->sadb_msg_type = SADB_X_SPDDELETE2;
+	else
+		out_hdr->sadb_msg_type = event2poltype(c->event);
+	out_hdr->sadb_msg_errno = 0;
+	out_hdr->sadb_msg_seq = c->seq;
+	out_hdr->sadb_msg_pid = c->pid;
+	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, NULL);
+out:
+	return 0;
+
+}
+
 static int pfkey_spdadd(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
 	int err;
@@ -1866,8 +1984,7 @@
 	struct sadb_address *sa;
 	struct sadb_x_policy *pol;
 	struct xfrm_policy *xp;
-	struct sk_buff *out_skb;
-	struct sadb_msg *out_hdr;
+	struct km_event c;
 
 	if (!present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1],
 				     ext_hdrs[SADB_EXT_ADDRESS_DST-1]) ||
@@ -1935,31 +2052,25 @@
 	    (err = parse_ipsecrequests(xp, pol)) < 0)
 		goto out;
 
-	out_skb = pfkey_xfrm_policy2msg_prep(xp);
-	if (IS_ERR(out_skb)) {
-		err =  PTR_ERR(out_skb);
-		goto out;
-	}
 
 	err = xfrm_policy_insert(pol->sadb_x_policy_dir-1, xp,
 				 hdr->sadb_msg_type != SADB_X_SPDUPDATE);
+
 	if (err) {
-		kfree_skb(out_skb);
-		goto out;
+		kfree(xp);
+		return err;
 	}
 
-	pfkey_xfrm_policy2msg(out_skb, xp, pol->sadb_x_policy_dir-1);
+	if (hdr->sadb_msg_type == SADB_X_SPDUPDATE) 
+		c.event = XFRM_SAP_UPDATED;
+	else 
+		c.event = XFRM_SAP_ADDED;
 
-	xfrm_pol_put(xp);
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
 
-	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version = hdr->sadb_msg_version;
-	out_hdr->sadb_msg_type = hdr->sadb_msg_type;
-	out_hdr->sadb_msg_satype = 0;
-	out_hdr->sadb_msg_errno = 0;
-	out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
-	out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, sk);
+	km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
+	xfrm_pol_put(xp);
 	return 0;
 
 out:
@@ -1973,9 +2084,8 @@
 	struct sadb_address *sa;
 	struct sadb_x_policy *pol;
 	struct xfrm_policy *xp;
-	struct sk_buff *out_skb;
-	struct sadb_msg *out_hdr;
 	struct xfrm_selector sel;
+	struct km_event c;
 
 	if (!present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1],
 				     ext_hdrs[SADB_EXT_ADDRESS_DST-1]) ||
@@ -2010,25 +2120,41 @@
 
 	err = 0;
 
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	c.event = XFRM_SAP_DELETED;
+	km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
+
+	xfrm_pol_put(xp);
+	return err;
+}
+
+
+static int key_pol_get_resp(struct sock *sk, struct xfrm_policy *xp, struct sadb_msg *hdr, int dir)
+{
+	int err;
+	struct sk_buff *out_skb;
+	struct sadb_msg *out_hdr;
+	err = 0;
+
 	out_skb = pfkey_xfrm_policy2msg_prep(xp);
 	if (IS_ERR(out_skb)) {
 		err =  PTR_ERR(out_skb);
 		goto out;
 	}
-	pfkey_xfrm_policy2msg(out_skb, xp, pol->sadb_x_policy_dir-1);
+	pfkey_xfrm_policy2msg(out_skb, xp, dir);
 
 	out_hdr = (struct sadb_msg *) out_skb->data;
 	out_hdr->sadb_msg_version = hdr->sadb_msg_version;
-	out_hdr->sadb_msg_type = SADB_X_SPDDELETE;
+	out_hdr->sadb_msg_type = hdr->sadb_msg_type;
 	out_hdr->sadb_msg_satype = 0;
 	out_hdr->sadb_msg_errno = 0;
 	out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
 	out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, sk);
+	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, sk);
 	err = 0;
 
 out:
-	xfrm_pol_put(xp);
 	return err;
 }
 
@@ -2037,8 +2163,7 @@
 	int err;
 	struct sadb_x_policy *pol;
 	struct xfrm_policy *xp;
-	struct sk_buff *out_skb;
-	struct sadb_msg *out_hdr;
+	struct km_event c;
 
 	if ((pol = ext_hdrs[SADB_X_EXT_POLICY-1]) == NULL)
 		return -EINVAL;
@@ -2050,24 +2175,16 @@
 
 	err = 0;
 
-	out_skb = pfkey_xfrm_policy2msg_prep(xp);
-	if (IS_ERR(out_skb)) {
-		err =  PTR_ERR(out_skb);
-		goto out;
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	if (hdr->sadb_msg_type == SADB_X_SPDDELETE2) {
+		c.data = 1; // to signal pfkey of SADB_X_SPDDELETE2
+		c.event = XFRM_SAP_DELETED;
+		km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
+	} else {
+		err = key_pol_get_resp(sk, xp, hdr, pol->sadb_x_policy_dir-1);
 	}
-	pfkey_xfrm_policy2msg(out_skb, xp, pol->sadb_x_policy_dir-1);
 
-	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version = hdr->sadb_msg_version;
-	out_hdr->sadb_msg_type = hdr->sadb_msg_type;
-	out_hdr->sadb_msg_satype = 0;
-	out_hdr->sadb_msg_errno = 0;
-	out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
-	out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, sk);
-	err = 0;
-
-out:
 	xfrm_pol_put(xp);
 	return err;
 }
@@ -2102,22 +2219,33 @@
 	return xfrm_policy_walk(dump_sp, &data);
 }
 
-static int pfkey_spdflush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
+static int key_notify_policy_flush(struct km_event *c)
 {
 	struct sk_buff *skb_out;
-	struct sadb_msg *hdr_out;
-
-	skb_out = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_KERNEL);
+	struct sadb_msg *hdr;
+	skb_out = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_ATOMIC);
 	if (!skb_out)
 		return -ENOBUFS;
+	hdr = (struct sadb_msg *) skb_put(skb_out, sizeof(struct sadb_msg));
+	hdr->sadb_msg_seq = c->seq;
+	hdr->sadb_msg_pid = c->pid;
+	hdr->sadb_msg_version = PF_KEY_V2;
+	hdr->sadb_msg_errno = (uint8_t) 0;
+	hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
+	pfkey_broadcast(skb_out, GFP_ATOMIC, BROADCAST_ALL, NULL);
+	return 0;
 
-	xfrm_policy_flush();
+}
 
-	hdr_out = (struct sadb_msg *) skb_put(skb_out, sizeof(struct sadb_msg));
-	pfkey_hdr_dup(hdr_out, hdr);
-	hdr_out->sadb_msg_errno = (uint8_t) 0;
-	hdr_out->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
-	pfkey_broadcast(skb_out, GFP_KERNEL, BROADCAST_ALL, NULL);
+static int pfkey_spdflush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
+{
+	struct km_event c;
+
+	xfrm_policy_flush();
+	c.event = XFRM_SAP_FLUSHED;
+	c.pid = hdr->sadb_msg_pid;
+	c.seq = hdr->sadb_msg_seq;
+	km_policy_notify(NULL, 0, &c);
 
 	return 0;
 }
@@ -2317,11 +2445,24 @@
 	}
 }
 
-static int pfkey_send_notify(struct xfrm_state *x, int hard)
+/* XXX: Noisy for now */
+static int key_notify_policy_expire(struct xfrm_policy *xp, struct km_event *c)
+{
+	return 0;
+}
+
+static int key_notify_sa_expire(struct xfrm_state *x, struct km_event *c)
 {
 	struct sk_buff *out_skb;
 	struct sadb_msg *out_hdr;
-	int hsc = (hard ? 2 : 1);
+	int hard;
+	int hsc;
+
+	hard = c->data;
+	if (hard)
+		hsc = 2;
+	else
+		hsc = 1;
 
 	out_skb = pfkey_xfrm_state2msg(x, 0, hsc);
 	if (IS_ERR(out_skb))
@@ -2340,6 +2481,43 @@
 	return 0;
 }
 
+static int pfkey_send_notify(struct xfrm_state *x, struct km_event *c)
+{
+	switch (c->event) {
+	case XFRM_SAP_EXPIRED:
+		return key_notify_sa_expire(x, c);
+	case XFRM_SAP_DELETED:
+	case XFRM_SAP_ADDED:
+	case XFRM_SAP_UPDATED:
+		return key_notify_sa(x, c);
+	case XFRM_SAP_FLUSHED:
+		return key_notify_sa_flush(c);
+	default:
+		printk("pfkey: Unknown SA event %d\n",c->event);
+		break;
+	}
+
+	return 0;
+}
+
+static int pfkey_send_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
+{
+	switch (c->event) {
+	case XFRM_SAP_EXPIRED:
+		return key_notify_policy_expire(xp, c);
+	case XFRM_SAP_DELETED:
+	case XFRM_SAP_ADDED:
+	case XFRM_SAP_UPDATED:
+		return key_notify_policy(xp, dir, c);
+	case XFRM_SAP_FLUSHED:
+		return key_notify_policy_flush(c);
+	default:
+		printk("pfkey: Unknown policy event %d\n",c->event);
+		break;
+	}
+
+	return 0;
+}
 static u32 get_acqseq(void)
 {
 	u32 res;
@@ -2856,6 +3034,7 @@
 	.acquire	= pfkey_send_acquire,
 	.compile_policy	= pfkey_compile_policy,
 	.new_mapping	= pfkey_send_new_mapping,
+	.notify_policy	= pfkey_send_policy_notify,
 };
 
 static void __exit ipsec_pfkey_exit(void)

Re: PATCH: IPSEC xfrm events

[Herbert Xu]

On Tue, Apr 05, 2005 at 08:03:24AM -0400, jamal wrote:
> 
> Heres the final patch.
> What this patch provides
> 
> - netlink xfrm events
> - ability to have events generated by netlink propagated to pfkey
> and vice versa.
> - fixes the acquire lets-be-happy-with-one-success issue

Jamal you forgot to sign off your own patch :)

Anyway this looks good to me.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: PATCH: IPSEC xfrm events

[jamal]

Gah, Ok - I guess i too can be famous

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

cheers,
jamal

On Tue, 2005-04-05 at 08:07, Herbert Xu wrote:
> 
> Jamal you forgot to sign off your own patch :)
> 
> Anyway this looks good to me.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Re: PATCH: IPSEC xfrm events

[Arnaldo Carvalho de Melo]

On 05 Apr 2005 08:19:06 -0400, jamal <hadi@cyberus.ca> wrote:
> 
> Gah, Ok - I guess i too can be famous

Or funny! /me runs

- Arnaldo

[1/4] [IPSEC] Improve xfrm to pfkey SA state conversion

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

Hi:

This series of patches will fix the spurious state/policy expire
notification problem identified in the discussion regarding Jamal's
IPsec events patch.  It will also address other minor issues arising
from the events patch.

The first patch adjusts the SA state conversion in af_key such that
XFRM_STATE_ERROR/XFRM_STATE_DEAD will be converted to SADB_STATE_DEAD
instead of SADB_STATE_DYING.

According to RFC 2367, SADB_STATE_DYING SAs can be turned into
mature ones through updating their lifetime settings.  Since SAs
which are in the states XFRM_STATE_ERROR/XFRM_STATE_DEAD cannot
be resurrected, this value is unsuitable.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-1 --]
[-- Type: text/plain, Size: 920 bytes --]

===== net/key/af_key.c 1.73 vs edited =====
--- 1.73/net/key/af_key.c	2005-04-08 23:08:26 +10:00
+++ edited/net/key/af_key.c	2005-04-09 14:15:45 +10:00
@@ -656,13 +656,18 @@
 	sa->sadb_sa_exttype = SADB_EXT_SA;
 	sa->sadb_sa_spi = x->id.spi;
 	sa->sadb_sa_replay = x->props.replay_window;
-	sa->sadb_sa_state = SADB_SASTATE_DYING;
-	if (x->km.state == XFRM_STATE_VALID && !x->km.dying)
-		sa->sadb_sa_state = SADB_SASTATE_MATURE;
-	else if (x->km.state == XFRM_STATE_ACQ)
+	switch (x->km.state) {
+	case XFRM_STATE_VALID:
+		sa->sadb_sa_state = x->km.dying ?
+			SADB_SASTATE_DYING : SADB_SASTATE_MATURE;
+		break;
+	case XFRM_STATE_ACQ:
 		sa->sadb_sa_state = SADB_SASTATE_LARVAL;
-	else if (x->km.state == XFRM_STATE_EXPIRED)
+		break;
+	default:
 		sa->sadb_sa_state = SADB_SASTATE_DEAD;
+		break;
+	}
 	sa->sadb_sa_auth = 0;
 	if (x->aalg) {
 		struct xfrm_algo_desc *a = xfrm_aalg_get_byname(x->aalg->alg_name, 0);

[2/4] [IPSEC] Kill spurious hard expire messages

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

Hi:

This patch ensures that the hard state/policy expire notifications are
only sent when the state/policy is successfully removed from their
respective tables.

As it is, it's possible for a state/policy to both expire through
reaching a hard limit, as well as being deleted by the user.

Note that this behaviour isn't actually forbidden by RFC 2367.
However, it is a quality of implementation issue.

As an added bonus, the restructuring in this patch will help
eventually in moving the expire notifications from softirq
context into process context, thus improving their reliability.

One important side-effect from this change is that SAs reaching
their hard byte/packet limits are now deleted immediately, just
like SAs that have reached their hard time limits.

Previously they were announced immediately but only deleted after
30 seconds.

This is bad because it prevents the system from issuing an ACQUIRE
command until the existing state was deleted by the user or expires
after the time is up.

In the scenario where the expire notification was lost this introduces
a 30 second delay into the system for no good reason.
 
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

 include/net/xfrm.h     |    2 +-
 net/xfrm/xfrm_policy.c |    8 +++++---
 net/xfrm/xfrm_state.c  |   16 +++++++---------
 3 files changed, 13 insertions(+), 13 deletions(-)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-2 --]
[-- Type: text/plain, Size: 2560 bytes --]

===== include/net/xfrm.h 1.80 vs edited =====
--- 1.80/include/net/xfrm.h	2005-04-08 23:08:26 +10:00
+++ edited/include/net/xfrm.h	2005-04-09 17:55:24 +10:00
@@ -669,7 +669,7 @@
 	return 0;
 }
 
-extern void xfrm_policy_delete(struct xfrm_policy *pol, int dir);
+extern int xfrm_policy_delete(struct xfrm_policy *pol, int dir);
 
 static inline void xfrm_sk_free_policy(struct sock *sk)
 {
===== net/xfrm/xfrm_policy.c 1.75 vs edited =====
--- 1.75/net/xfrm/xfrm_policy.c	2005-04-01 16:24:20 +10:00
+++ edited/net/xfrm/xfrm_policy.c	2005-04-09 18:02:53 +10:00
@@ -216,8 +216,8 @@
 
 expired:
 	read_unlock(&xp->lock);
-	km_policy_expired(xp, dir, 1);
-	xfrm_policy_delete(xp, dir);
+	if (!xfrm_policy_delete(xp, dir))
+		km_policy_expired(xp, dir, 1);
 	xfrm_pol_put(xp);
 }
 
@@ -555,7 +555,7 @@
 	return NULL;
 }
 
-void xfrm_policy_delete(struct xfrm_policy *pol, int dir)
+int xfrm_policy_delete(struct xfrm_policy *pol, int dir)
 {
 	write_lock_bh(&xfrm_policy_lock);
 	pol = __xfrm_policy_unlink(pol, dir);
@@ -564,7 +564,9 @@
 		if (dir < XFRM_POLICY_MAX)
 			atomic_inc(&flow_cache_genid);
 		xfrm_policy_kill(pol);
+		return 0;
 	}
+	return -ENOENT;
 }
 
 int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol)
===== net/xfrm/xfrm_state.c 1.61 vs edited =====
--- 1.61/net/xfrm/xfrm_state.c	2005-04-08 23:08:26 +10:00
+++ edited/net/xfrm/xfrm_state.c	2005-04-09 19:55:48 +10:00
@@ -154,6 +154,7 @@
 			next = tmo;
 	}
 
+	x->km.dying = warn;
 	if (warn)
 		km_state_expired(x, 0);
 resched:
@@ -169,9 +170,8 @@
 		next = 2;
 		goto resched;
 	}
-	if (x->id.spi != 0)
+	if (!__xfrm_state_delete(x) && x->id.spi)
 		km_state_expired(x, 1);
-	__xfrm_state_delete(x);
 
 out:
 	spin_unlock(&x->lock);
@@ -566,16 +566,18 @@
 
 	if (x->curlft.bytes >= x->lft.hard_byte_limit ||
 	    x->curlft.packets >= x->lft.hard_packet_limit) {
-		km_state_expired(x, 1);
-		if (!mod_timer(&x->timer, jiffies + XFRM_ACQ_EXPIRES*HZ))
+		x->km.state = XFRM_STATE_EXPIRED;
+		if (!mod_timer(&x->timer, jiffies))
 			xfrm_state_hold(x);
 		return -EINVAL;
 	}
 
 	if (!x->km.dying &&
 	    (x->curlft.bytes >= x->lft.soft_byte_limit ||
-	     x->curlft.packets >= x->lft.soft_packet_limit))
+	     x->curlft.packets >= x->lft.soft_packet_limit)) {
+		x->km.dying = 1;
 		km_state_expired(x, 0);
+	}
 	return 0;
 }
 EXPORT_SYMBOL(xfrm_state_check_expire);
@@ -833,10 +835,6 @@
 {
 	struct km_event c;
 
-	if (hard)
-		x->km.state = XFRM_STATE_EXPIRED;
-	else
-		x->km.dying = 1;
 	c.data = hard;
 	c.event = XFRM_SAP_EXPIRED;
 	km_state_notify(x, &c);

[3/4] [IPSEC] Turn km_event.data into a union

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 584 bytes --]

Hi:

This patch turns km_event.data into a union.  This makes code that
uses it clearer.
  
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

 include/net/xfrm.h    |    7 ++++++-
 net/key/af_key.c      |   17 +++++------------
 net/xfrm/xfrm_state.c |    5 ++---
 net/xfrm/xfrm_user.c  |   10 +++++-----
 4 files changed, 18 insertions(+), 21 deletions(-)
 
Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-3 --]
[-- Type: text/plain, Size: 3729 bytes --]

===== include/net/xfrm.h 1.81 vs edited =====
--- 1.81/include/net/xfrm.h	2005-04-09 20:21:15 +10:00
+++ edited/include/net/xfrm.h	2005-04-09 20:37:05 +10:00
@@ -172,7 +172,12 @@
 /* callback structure passed from either netlink or pfkey */
 struct km_event
 {
-	u32	data; 
+	union {
+		u32 hard;
+		u32 proto; 
+		u32 byid;
+	} data;
+
 	u32	seq; 
 	u32	pid; 
 	u32	event; 
===== net/key/af_key.c 1.74 vs edited =====
--- 1.74/net/key/af_key.c	2005-04-09 14:39:58 +10:00
+++ edited/net/key/af_key.c	2005-04-09 20:32:54 +10:00
@@ -1293,13 +1293,6 @@
 	if (c->event == XFRM_SAP_DELETED)
 		hsc = 0;
 
-	if (c->event == XFRM_SAP_EXPIRED) {
-		if (c->data)
-			hsc = 2;
-		else
-			hsc = 1;
-	}
-
 	skb = pfkey_xfrm_state2msg(x, 0, hsc);
 
 	if (IS_ERR(skb))
@@ -1534,7 +1527,7 @@
 	if (!skb)
 		return -ENOBUFS;
 	hdr = (struct sadb_msg *) skb_put(skb, sizeof(struct sadb_msg));
-	hdr->sadb_msg_satype = pfkey_proto2satype(c->data);
+	hdr->sadb_msg_satype = pfkey_proto2satype(c->data.proto);
 	hdr->sadb_msg_seq = c->seq;
 	hdr->sadb_msg_pid = c->pid;
 	hdr->sadb_msg_version = PF_KEY_V2;
@@ -1556,7 +1549,7 @@
 		return -EINVAL;
 
 	xfrm_state_flush(proto);
-	c.data = proto;
+	c.data.proto = proto;
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
 	c.event = XFRM_SAP_FLUSHED;
@@ -1969,7 +1962,7 @@
 	out_hdr = (struct sadb_msg *) out_skb->data;
 	out_hdr->sadb_msg_version =  PF_KEY_V2;
 
-	if (c->data && c->event == XFRM_SAP_DELETED)
+	if (c->data.byid && c->event == XFRM_SAP_DELETED)
 		out_hdr->sadb_msg_type = SADB_X_SPDDELETE2;
 	else
 		out_hdr->sadb_msg_type = event2poltype(c->event);
@@ -2183,7 +2176,7 @@
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
 	if (hdr->sadb_msg_type == SADB_X_SPDDELETE2) {
-		c.data = 1; // to signal pfkey of SADB_X_SPDDELETE2
+		c.data.byid = 1;
 		c.event = XFRM_SAP_DELETED;
 		km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
 	} else {
@@ -2463,7 +2456,7 @@
 	int hard;
 	int hsc;
 
-	hard = c->data;
+	hard = c->data.hard;
 	if (hard)
 		hsc = 2;
 	else
===== net/xfrm/xfrm_state.c 1.62 vs edited =====
--- 1.62/net/xfrm/xfrm_state.c	2005-04-09 20:21:15 +10:00
+++ edited/net/xfrm/xfrm_state.c	2005-04-09 20:38:54 +10:00
@@ -835,7 +835,7 @@
 {
 	struct km_event c;
 
-	c.data = hard;
+	c.data.hard = hard;
 	c.event = XFRM_SAP_EXPIRED;
 	km_state_notify(x, &c);
 
@@ -883,8 +883,7 @@
 {
 	struct km_event c;
 
-	c.data = hard;
-	c.data = hard;
+	c.data.hard = hard;
 	c.event = XFRM_SAP_EXPIRED;
 	km_policy_notify(pol, dir, &c);
 
===== net/xfrm/xfrm_user.c 1.53 vs edited =====
--- 1.53/net/xfrm/xfrm_user.c	2005-04-08 23:08:26 +10:00
+++ edited/net/xfrm/xfrm_user.c	2005-04-09 20:35:06 +10:00
@@ -894,7 +894,7 @@
 	struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
 
 	xfrm_state_flush(p->proto);
-	c.data = p->proto;
+	c.data.proto = p->proto;
 	c.event = XFRM_SAP_FLUSHED;
 	c.seq = nlh->nlmsg_seq;
 	c.pid = nlh->nlmsg_pid;
@@ -1116,13 +1116,13 @@
 static int xfrm_exp_state_notify(struct xfrm_state *x, struct km_event *c)
 {
 	struct sk_buff *skb;
-	int hard = c ->data;
+
 	/* fix to do alloc using NLM macros */
 	skb = alloc_skb(sizeof(struct xfrm_user_expire) + 16, GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_expire(skb, x, hard) < 0)
+	if (build_expire(skb, x, c->data.hard) < 0)
 		BUG();
 
 	NETLINK_CB(skb).dst_groups = XFRMGRP_EXPIRE;
@@ -1148,7 +1148,7 @@
 	nlh->nlmsg_flags = 0;
 
 	p = NLMSG_DATA(nlh);
-	p->proto = c->data;
+	p->proto = c->data.proto;
 
 	nlh->nlmsg_len = skb->tail - b;
 
@@ -1391,7 +1391,7 @@
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_polexpire(skb, xp, dir, c->data) < 0)
+	if (build_polexpire(skb, xp, dir, c->data.hard) < 0)
 		BUG();
 
 	NETLINK_CB(skb).dst_groups = XFRMGRP_EXPIRE;

[4/4] [IPSEC] Set byid for km_event in xfrm_get_policy

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

Hi:

This patch fixes policy deletion in xfrm_user so that it sets
km_event.data.byid.  This puts xfrm_user on par with what af_key
does in this case.
   
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

 xfrm_user.c |    1 +
 1 files changed, 1 insertion(+)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-4 --]
[-- Type: text/plain, Size: 339 bytes --]

===== net/xfrm/xfrm_user.c 1.54 vs edited =====
--- 1.54/net/xfrm/xfrm_user.c	2005-04-09 20:40:43 +10:00
+++ edited/net/xfrm/xfrm_user.c	2005-04-09 20:41:29 +10:00
@@ -877,6 +877,7 @@
 					      MSG_DONTWAIT);
 		}
 	} else {
+		c.data.byid = p->index;
 		c.event = XFRM_SAP_DELETED;
 		c.seq = nlh->nlmsg_seq;
 		c.pid = nlh->nlmsg_pid;

[5/*] [IPSEC] Use XFRM_MSG_* instead of XFRM_SAP_*

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

Hi:

I spotted some more issues while preparing the last patch of the
series.  So I'll extend it a little further.

This patch removes XFRM_SAP_* and converts them over to XFRM_MSG_*.
The netlink interface is meant to map directly onto the underlying
xfrm subsystem.  Therefore rather than using a new independent
representation for the events we can simply use the existing ones
from xfrm_user.

I also killed a couple of inline's in af_key since

1) They are not performance critical.
2) None of the other x2y functions here are inlined.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

 include/net/xfrm.h    |   12 --------
 net/key/af_key.c      |   70 +++++++++++++++++++++++++-------------------------
 net/xfrm/xfrm_state.c |    4 +-
 net/xfrm/xfrm_user.c  |   63 ++++++++++++---------------------------------
 4 files changed, 55 insertions(+), 94 deletions(-)
 
Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-5 --]
[-- Type: text/plain, Size: 9546 bytes --]

===== include/net/xfrm.h 1.82 vs edited =====
--- 1.82/include/net/xfrm.h	2005-04-09 20:40:43 +10:00
+++ edited/include/net/xfrm.h	2005-04-10 18:01:39 +10:00
@@ -157,18 +157,6 @@
 	XFRM_STATE_DEAD
 };
 
-/* events that could be sent by kernel */
-enum {
-	XFRM_SAP_INVALID,
-	XFRM_SAP_EXPIRED,
-	XFRM_SAP_ADDED,
-	XFRM_SAP_UPDATED,
-	XFRM_SAP_DELETED,
-	XFRM_SAP_FLUSHED,
-	__XFRM_SAP_MAX
-};
-#define XFRM_SAP_MAX (__XFRM_SAP_MAX - 1)
-
 /* callback structure passed from either netlink or pfkey */
 struct km_event
 {
===== net/key/af_key.c 1.75 vs edited =====
--- 1.75/net/key/af_key.c	2005-04-09 20:40:43 +10:00
+++ edited/net/key/af_key.c	2005-04-10 18:51:51 +10:00
@@ -1245,38 +1245,38 @@
 	return 0;
 }
 
-static inline int event2poltype (int event)
+static int event2poltype(int event)
 {
 	switch (event) {
-	case XFRM_SAP_DELETED:
+	case XFRM_MSG_DELPOLICY:
 		return SADB_X_SPDDELETE;
-	case XFRM_SAP_ADDED:
+	case XFRM_MSG_NEWPOLICY:
 		return SADB_X_SPDADD;
-	case XFRM_SAP_UPDATED:
+	case XFRM_MSG_UPDPOLICY:
 		return SADB_X_SPDUPDATE;
-	case XFRM_SAP_EXPIRED:
+	case XFRM_MSG_POLEXPIRE:
 	//	return SADB_X_SPDEXPIRE;
 	default:
-		printk("pfkey: Unknown policy event %d\n",event);
+		printk("pfkey: Unknown policy event %d\n", event);
 		break;
 	}
 
 	return 0;
 }
 
-static inline int event2keytype (int event)
+static int event2keytype(int event)
 {
 	switch (event) {
-	case XFRM_SAP_DELETED:
+	case XFRM_MSG_DELSA:
 		return SADB_DELETE;
-	case XFRM_SAP_ADDED:
+	case XFRM_MSG_NEWSA:
 		return SADB_ADD;
-	case XFRM_SAP_UPDATED:
+	case XFRM_MSG_UPDSA:
 		return SADB_UPDATE;
-	case XFRM_SAP_EXPIRED:
+	case XFRM_MSG_EXPIRE:
 		return SADB_EXPIRE;
 	default:
-		printk("pfkey: Unknown SA event %d\n",event);
+		printk("pfkey: Unknown SA event %d\n", event);
 		break;
 	}
 
@@ -1290,7 +1290,7 @@
 	struct sadb_msg *hdr;
 	int hsc = 3;
 
-	if (c->event == XFRM_SAP_DELETED)
+	if (c->event == XFRM_MSG_DELSA)
 		hsc = 0;
 
 	skb = pfkey_xfrm_state2msg(x, 0, hsc);
@@ -1337,9 +1337,9 @@
 	}
 
 	if (hdr->sadb_msg_type == SADB_ADD)
-		c.event = XFRM_SAP_ADDED;
+		c.event = XFRM_MSG_NEWSA;
 	else
-		c.event = XFRM_SAP_UPDATED;
+		c.event = XFRM_MSG_UPDSA;
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
 	km_state_notify(x, &c);
@@ -1376,7 +1376,7 @@
 
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
-	c.event = XFRM_SAP_DELETED;
+	c.event = XFRM_MSG_DELSA;
 	km_state_notify(x, &c);
 	xfrm_state_put(x);
 
@@ -1552,7 +1552,7 @@
 	c.data.proto = proto;
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
-	c.event = XFRM_SAP_FLUSHED;
+	c.event = XFRM_MSG_FLUSHSA;
 	km_state_notify(NULL, &c);
 
 	return 0;
@@ -1962,7 +1962,7 @@
 	out_hdr = (struct sadb_msg *) out_skb->data;
 	out_hdr->sadb_msg_version =  PF_KEY_V2;
 
-	if (c->data.byid && c->event == XFRM_SAP_DELETED)
+	if (c->data.byid && c->event == XFRM_MSG_DELPOLICY)
 		out_hdr->sadb_msg_type = SADB_X_SPDDELETE2;
 	else
 		out_hdr->sadb_msg_type = event2poltype(c->event);
@@ -2060,9 +2060,9 @@
 	}
 
 	if (hdr->sadb_msg_type == SADB_X_SPDUPDATE) 
-		c.event = XFRM_SAP_UPDATED;
+		c.event = XFRM_MSG_UPDPOLICY;
 	else 
-		c.event = XFRM_SAP_ADDED;
+		c.event = XFRM_MSG_NEWPOLICY;
 
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
@@ -2120,7 +2120,7 @@
 
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
-	c.event = XFRM_SAP_DELETED;
+	c.event = XFRM_MSG_DELPOLICY;
 	km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
 
 	xfrm_pol_put(xp);
@@ -2177,7 +2177,7 @@
 	c.pid = hdr->sadb_msg_pid;
 	if (hdr->sadb_msg_type == SADB_X_SPDDELETE2) {
 		c.data.byid = 1;
-		c.event = XFRM_SAP_DELETED;
+		c.event = XFRM_MSG_DELPOLICY;
 		km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
 	} else {
 		err = key_pol_get_resp(sk, xp, hdr, pol->sadb_x_policy_dir-1);
@@ -2240,7 +2240,7 @@
 	struct km_event c;
 
 	xfrm_policy_flush();
-	c.event = XFRM_SAP_FLUSHED;
+	c.event = XFRM_MSG_FLUSHPOLICY;
 	c.pid = hdr->sadb_msg_pid;
 	c.seq = hdr->sadb_msg_seq;
 	km_policy_notify(NULL, 0, &c);
@@ -2482,16 +2482,16 @@
 static int pfkey_send_notify(struct xfrm_state *x, struct km_event *c)
 {
 	switch (c->event) {
-	case XFRM_SAP_EXPIRED:
+	case XFRM_MSG_EXPIRE:
 		return key_notify_sa_expire(x, c);
-	case XFRM_SAP_DELETED:
-	case XFRM_SAP_ADDED:
-	case XFRM_SAP_UPDATED:
+	case XFRM_MSG_DELSA:
+	case XFRM_MSG_NEWSA:
+	case XFRM_MSG_UPDSA:
 		return key_notify_sa(x, c);
-	case XFRM_SAP_FLUSHED:
+	case XFRM_MSG_FLUSHSA:
 		return key_notify_sa_flush(c);
 	default:
-		printk("pfkey: Unknown SA event %d\n",c->event);
+		printk("pfkey: Unknown SA event %d\n", c->event);
 		break;
 	}
 
@@ -2501,16 +2501,16 @@
 static int pfkey_send_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	switch (c->event) {
-	case XFRM_SAP_EXPIRED:
+	case XFRM_MSG_POLEXPIRE:
 		return key_notify_policy_expire(xp, c);
-	case XFRM_SAP_DELETED:
-	case XFRM_SAP_ADDED:
-	case XFRM_SAP_UPDATED:
+	case XFRM_MSG_DELPOLICY:
+	case XFRM_MSG_NEWPOLICY:
+	case XFRM_MSG_UPDPOLICY:
 		return key_notify_policy(xp, dir, c);
-	case XFRM_SAP_FLUSHED:
+	case XFRM_MSG_FLUSHPOLICY:
 		return key_notify_policy_flush(c);
 	default:
-		printk("pfkey: Unknown policy event %d\n",c->event);
+		printk("pfkey: Unknown policy event %d\n", c->event);
 		break;
 	}
 
===== net/xfrm/xfrm_state.c 1.63 vs edited =====
--- 1.63/net/xfrm/xfrm_state.c	2005-04-09 20:40:43 +10:00
+++ edited/net/xfrm/xfrm_state.c	2005-04-10 18:55:01 +10:00
@@ -836,7 +836,7 @@
 	struct km_event c;
 
 	c.data.hard = hard;
-	c.event = XFRM_SAP_EXPIRED;
+	c.event = XFRM_MSG_EXPIRE;
 	km_state_notify(x, &c);
 
 	if (hard)
@@ -884,7 +884,7 @@
 	struct km_event c;
 
 	c.data.hard = hard;
-	c.event = XFRM_SAP_EXPIRED;
+	c.event = XFRM_MSG_POLEXPIRE;
 	km_policy_notify(pol, dir, &c);
 
 	if (hard)
===== net/xfrm/xfrm_user.c 1.55 vs edited =====
--- 1.55/net/xfrm/xfrm_user.c	2005-04-10 17:45:12 +10:00
+++ edited/net/xfrm/xfrm_user.c	2005-04-10 18:05:53 +10:00
@@ -292,10 +292,7 @@
 
 	c.seq = nlh->nlmsg_seq;
 	c.pid = nlh->nlmsg_pid;
-	if (nlh->nlmsg_type == XFRM_MSG_NEWSA)
-		c.event = XFRM_SAP_ADDED;
-	else
-		c.event = XFRM_SAP_UPDATED;
+	c.event = nlh->nlmsg_type;
 
 	km_state_notify(x, &c);
 	xfrm_state_put(x);
@@ -327,7 +324,7 @@
 
 	c.seq = nlh->nlmsg_seq;
 	c.pid = nlh->nlmsg_pid;
-	c.event = XFRM_SAP_DELETED;
+	c.event = nlh->nlmsg_type;
 	km_state_notify(x, &c);
 	xfrm_state_put(x);
 
@@ -721,11 +718,7 @@
 	}
 
 
-	if (!excl)
-		c.event = XFRM_SAP_UPDATED;
-	else
-		c.event = XFRM_SAP_ADDED;
-
+	c.event = nlh->nlmsg_type;
 	c.seq = nlh->nlmsg_seq;
 	c.pid = nlh->nlmsg_pid;
 	km_policy_notify(xp, p->dir, &c);
@@ -878,7 +871,7 @@
 		}
 	} else {
 		c.data.byid = p->index;
-		c.event = XFRM_SAP_DELETED;
+		c.event = nlh->nlmsg_type;
 		c.seq = nlh->nlmsg_seq;
 		c.pid = nlh->nlmsg_pid;
 		km_policy_notify(xp, p->dir, &c);
@@ -896,7 +889,7 @@
 
 	xfrm_state_flush(p->proto);
 	c.data.proto = p->proto;
-	c.event = XFRM_SAP_FLUSHED;
+	c.event = nlh->nlmsg_type;
 	c.seq = nlh->nlmsg_seq;
 	c.pid = nlh->nlmsg_pid;
 	km_state_notify(NULL, &c);
@@ -909,7 +902,7 @@
 	struct km_event c;
 
 	xfrm_policy_flush();
-	c.event = XFRM_SAP_FLUSHED;
+	c.event = nlh->nlmsg_type;
 	c.seq = nlh->nlmsg_seq;
 	c.pid = nlh->nlmsg_pid;
 	km_policy_notify(NULL, 0, &c);
@@ -1180,7 +1173,6 @@
 	struct xfrm_usersa_info *p;
 	struct nlmsghdr *nlh;
 	struct sk_buff *skb;
-	u32 nlt;
 	unsigned char *b;
 	int len = xfrm_sa_len(x);
 
@@ -1189,16 +1181,7 @@
 		return -ENOMEM;
 	b = skb->tail;
 
-	if (c->event == XFRM_SAP_ADDED)
-		nlt = XFRM_MSG_NEWSA;
-	else if (c->event == XFRM_SAP_UPDATED)
-		nlt = XFRM_MSG_UPDSA;
-	else if (c->event == XFRM_SAP_DELETED)
-		nlt = XFRM_MSG_DELSA;
-	else
-		goto nlmsg_failure;
-
-	nlh = NLMSG_PUT(skb, c->pid, c->seq, nlt, sizeof(*p));
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, c->event, sizeof(*p));
 	nlh->nlmsg_flags = 0;
 
 	p = NLMSG_DATA(nlh);
@@ -1230,13 +1213,13 @@
 {
 
 	switch (c->event) {
-	case XFRM_SAP_EXPIRED:
+	case XFRM_MSG_EXPIRE:
 		return xfrm_exp_state_notify(x, c);
-	case XFRM_SAP_DELETED:
-	case XFRM_SAP_UPDATED:
-	case XFRM_SAP_ADDED:
+	case XFRM_MSG_DELSA:
+	case XFRM_MSG_UPDSA:
+	case XFRM_MSG_NEWSA:
 		return xfrm_notify_sa(x, c);
-	case XFRM_SAP_FLUSHED:
+	case XFRM_MSG_FLUSHSA:
 		return xfrm_notify_sa_flush(c);
 	default:
 		 printk("xfrm_user: Unknown SA event %d\n",c->event);
@@ -1405,7 +1388,6 @@
 	struct xfrm_userpolicy_info *p;
 	struct nlmsghdr *nlh;
 	struct sk_buff *skb;
-	u32 nlt = 0 ;
 	unsigned char *b;
 	int len = RTA_SPACE(sizeof(struct xfrm_user_tmpl) * xp->xfrm_nr);
 	len += NLMSG_SPACE(sizeof(struct xfrm_userpolicy_info));
@@ -1415,16 +1397,7 @@
 		return -ENOMEM;
 	b = skb->tail;
 
-	if (c->event == XFRM_SAP_ADDED)
-		nlt = XFRM_MSG_NEWPOLICY;
-	else if (c->event == XFRM_SAP_UPDATED)
-		nlt = XFRM_MSG_UPDPOLICY;
-	else if (c->event == XFRM_SAP_DELETED)
-		nlt = XFRM_MSG_DELPOLICY;
-	else
-		goto nlmsg_failure;
-
-	nlh = NLMSG_PUT(skb, c->pid, c->seq, nlt, sizeof(*p));
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, c->event, sizeof(*p));
 
 	p = NLMSG_DATA(nlh);
 
@@ -1471,13 +1444,13 @@
 {
 
 	switch (c->event) {
-	case XFRM_SAP_ADDED:
-	case XFRM_SAP_UPDATED:
-	case XFRM_SAP_DELETED:
+	case XFRM_MSG_NEWPOLICY:
+	case XFRM_MSG_UPDPOLICY:
+	case XFRM_MSG_DELPOLICY:
 		return xfrm_notify_policy(xp, dir, c);
-	case XFRM_SAP_FLUSHED:
+	case XFRM_MSG_FLUSHPOLICY:
 		return xfrm_notify_policy_flush(c);
-	case XFRM_SAP_EXPIRED:
+	case XFRM_MSG_POLEXPIRE:
 		return xfrm_exp_policy_notify(xp, dir, c);
 	default:
 		printk("xfrm_user: Unknown Policy event %d\n",c->event);

[6/*] [IPSEC] Add xfrm_userpolicy_delete for xfrm_user notification

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

Hi:

This patch changes the format of the XFRM_MSG_DELPOLICY notification
so that we don't lose the byid information carried in km_event.

Since this user interface is introduced by Jamal's patch we can still
afford to change it.

I know that this is a bit trivial but as it is the information carried
in xfrm_user's notifications is a strict superset of that of af_key
and I'd like to keep it that way :)
 
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-6 --]
[-- Type: text/plain, Size: 1435 bytes --]

===== include/linux/xfrm.h 1.24 vs edited =====
--- 1.24/include/linux/xfrm.h	2005-04-08 23:08:26 +10:00
+++ edited/include/linux/xfrm.h	2005-04-10 19:25:22 +10:00
@@ -227,6 +227,11 @@
 	__u8				dir;
 };
 
+struct xfrm_userpolicy_delete {
+	struct xfrm_userpolicy_info	pol;
+	__u8				byid;
+};
+
 struct xfrm_user_acquire {
 	struct xfrm_id			id;
 	xfrm_address_t			saddr;
===== net/xfrm/xfrm_user.c 1.56 vs edited =====
--- 1.56/net/xfrm/xfrm_user.c	2005-04-10 19:03:30 +10:00
+++ edited/net/xfrm/xfrm_user.c	2005-04-10 19:30:30 +10:00
@@ -1386,20 +1386,30 @@
 static int xfrm_notify_policy( struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct xfrm_userpolicy_info *p;
+	struct xfrm_userpolicy_delete *pdel;
 	struct nlmsghdr *nlh;
 	struct sk_buff *skb;
 	unsigned char *b;
 	int len = RTA_SPACE(sizeof(struct xfrm_user_tmpl) * xp->xfrm_nr);
-	len += NLMSG_SPACE(sizeof(struct xfrm_userpolicy_info));
+	int payload;
+
+	payload = (c->event == XFRM_MSG_DELPOLICY) ?
+		  sizeof(*pdel) : sizeof(*p);
+	len += NLMSG_SPACE(payload);
 
 	skb = alloc_skb(len, GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
 	b = skb->tail;
 
-	nlh = NLMSG_PUT(skb, c->pid, c->seq, c->event, sizeof(*p));
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, c->event, payload);
 
 	p = NLMSG_DATA(nlh);
+	if (c->event == XFRM_MSG_DELPOLICY) {
+		pdel = NLMSG_DATA(nlh);
+		pdel->byid = !!c->data.byid;
+		p = &pdel->pol;
+	}
 
 	nlh->nlmsg_flags = 0;
 

Re: [5/*] [IPSEC] Use XFRM_MSG_* instead of XFRM_SAP_*

[jamal]

On Sun, 2005-04-10 at 05:02, Herbert Xu wrote:
> Hi:
> 
> I spotted some more issues while preparing the last patch of the
> series.  So I'll extend it a little further.
> 
> This patch removes XFRM_SAP_* and converts them over to XFRM_MSG_*.
> The netlink interface is meant to map directly onto the underlying
> xfrm subsystem.  Therefore rather than using a new independent
> representation for the events we can simply use the existing ones
> from xfrm_user.
> 

The main reason i did the XFRM_SAP_* is to be able to cover a case where
a message was relevant to one KM but not the other. That may not
exist now (actually it does with policy expiration that pfkey cant
handle - but thats easy to take care of).
Hopefully XFRM_MSG_xxx is the superset and will be sufficient.

Do you have anymore patches? If not i can give these a quick test;
Masahide has a better test setup and if he has time he should as well.

cheers,
jamal

Re: [5/*] [IPSEC] Use XFRM_MSG_* instead of XFRM_SAP_*

[Herbert Xu]

On Sun, Apr 10, 2005 at 10:15:11AM -0400, jamal wrote:
> 
> The main reason i did the XFRM_SAP_* is to be able to cover a case where
> a message was relevant to one KM but not the other. That may not
> exist now (actually it does with policy expiration that pfkey cant
> handle - but thats easy to take care of).
> Hopefully XFRM_MSG_xxx is the superset and will be sufficient.

Since xfrm_user is meant to be the native interface, this should never
happen.

> Do you have anymore patches? If not i can give these a quick test;
> Masahide has a better test setup and if he has time he should as well.

That's it for now.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [5/*] [IPSEC] Use XFRM_MSG_* instead of XFRM_SAP_*

[Masahide NAKAMURA]

> Do you have anymore patches? If not i can give these a quick test;
> Masahide has a better test setup and if he has time he should as well.

I can, but about 24 hours later. I'll test it there is no update and
if it isn't too late then.

Thanks,

-- 
Masahide NAKAMURA


jamal wrote:
> On Sun, 2005-04-10 at 05:02, Herbert Xu wrote:
> 
>>Hi:
>>
>>I spotted some more issues while preparing the last patch of the
>>series.  So I'll extend it a little further.
>>
>>This patch removes XFRM_SAP_* and converts them over to XFRM_MSG_*.
>>The netlink interface is meant to map directly onto the underlying
>>xfrm subsystem.  Therefore rather than using a new independent
>>representation for the events we can simply use the existing ones
>>from xfrm_user.
>>
> 
> 
> The main reason i did the XFRM_SAP_* is to be able to cover a case where
> a message was relevant to one KM but not the other. That may not
> exist now (actually it does with policy expiration that pfkey cant
> handle - but thats easy to take care of).
> Hopefully XFRM_MSG_xxx is the superset and will be sufficient.
> 
> Do you have anymore patches? If not i can give these a quick test;
> Masahide has a better test setup and if he has time he should as well.
> 
> cheers,
> jamal
> 
> 

Re: [5/*] [IPSEC] Use XFRM_MSG_* instead of XFRM_SAP_*

[jamal]

On Mon, 2005-04-11 at 01:45, Masahide NAKAMURA wrote:
> > Do you have anymore patches? If not i can give these a quick test;
> > Masahide has a better test setup and if he has time he should as well.
> 
> I can, but about 24 hours later. I'll test it there is no update and
> if it isn't too late then.
> 

I will on my part test in about 12 hours.

cheers,
jamal

Re: [5/*] [IPSEC] Use XFRM_MSG_* instead of XFRM_SAP_*

[Masahide NAKAMURA]

Hello Herbert and Jamal,

jamal wrote:
> On Mon, 2005-04-11 at 01:45, Masahide NAKAMURA wrote:
> 
>>>Do you have anymore patches? If not i can give these a quick test;
>>>Masahide has a better test setup and if he has time he should as well.
>>
>>I can, but about 24 hours later. I'll test it there is no update and
>>if it isn't too late then.
>>

short report:
My testing is not completed but, I've tested below and it is fine:
 add/del/flush SP and their notifications through netlink (using modified iproute2/ip).

 new "xfrm_userpolicy_delete" works fine on this case;
 used byid=1 when deleting SP with specifying SP index.

I'll test the rest case (17 hours later):
- using pfkey
- using both sockets

Thanks,


-- 
Masahide NAKAMURA

Re: [5/*] [IPSEC] Use XFRM_MSG_* instead of XFRM_SAP_*

[jamal]

On Tue, 2005-04-12 at 04:17, Masahide NAKAMURA wrote:

> short report:
> My testing is not completed but, I've tested below and it is fine:
>  add/del/flush SP and their notifications through netlink (using modified iproute2/ip).
> 
>  new "xfrm_userpolicy_delete" works fine on this case;
>  used byid=1 when deleting SP with specifying SP index.
> 
> I'll test the rest case (17 hours later):
> - using pfkey
> - using both sockets
> 

I did basic testing with pfkey generated events (policy/state) showing
up in both. The vice-versa also looks good.
Herbert, try to push forward to Dave.

cheers,
jamal

Re: [5/*] [IPSEC] Use XFRM_MSG_* instead of XFRM_SAP_*

[Masahide NAKAMURA]

Hello Jamal and Herbert,

jamal wrote:
> On Tue, 2005-04-12 at 04:17, Masahide NAKAMURA wrote:
> 
> 
>>short report:
>>My testing is not completed but, I've tested below and it is fine:
>> add/del/flush SP and their notifications through netlink (using modified iproute2/ip).
>>
>> new "xfrm_userpolicy_delete" works fine on this case;
>> used byid=1 when deleting SP with specifying SP index.
>>
>>I'll test the rest case (17 hours later):
>>- using pfkey
>>- using both sockets
>>
> 
> 
> I did basic testing with pfkey generated events (policy/state) showing
> up in both. The vice-versa also looks good.

Thanks, Jamal.
I'm sorry to be late, I've also tested pfkey, both sockets, and using racoon
(on 2.6.12-rc2 + Jamal's patch + Herbert's 6 patches).

# If somebody is interested in, modified iproute2 for the kernel can be found at:
# http://www.linux-ipv6.org/~nakam/iproute2-xfrm-event/iproute2-xfrm-event-20050413.tar.gz
# a patch against the latest iproute2 bk tree
# http://www.linux-ipv6.org/~nakam/iproute2-xfrm-event/iproute2-xfrm-event-20050413.diff
# This iproute2 change will be sent to Stephen after the kernel released.


> Herbert, try to push forward to Dave.

Agreed.

Regards,

-- 
Masahide NAKAMURA

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[jamal]

Herbert,
This is nice since it seems to go on top of the patch i posted.
I didnt get an ACK from Dave - so i assumed that patch is not on his 
tree yet. Which tree are you using?

Small comment below:

On Sat, 2005-04-09 at 07:12, Herbert Xu wrote:

> ===== net/xfrm/xfrm_policy.c 1.75 vs edited =====
> --- 1.75/net/xfrm/xfrm_policy.c 2005-04-01 16:24:20 +10:00
> +++ edited/net/xfrm/xfrm_policy.c       2005-04-09 18:02:53 +10:00
> @@ -216,8 +216,8 @@
> 
> expired:
>        read_unlock(&xp->lock);
> -       km_policy_expired(xp, dir, 1);
> -       xfrm_policy_delete(xp, dir);
> +       if (!xfrm_policy_delete(xp, dir))
> +               km_policy_expired(xp, dir, 1);
>        xfrm_pol_put(xp);>
> }
>

If the policy was already dead you will still return a 0 from
xfrm_policy_delete(). Fixable by policy_kill returning an int.

cheers,
jamal

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[Herbert Xu]

Hi Jamal:

On Sat, Apr 09, 2005 at 08:30:44AM -0400, jamal wrote:
> 
> This is nice since it seems to go on top of the patch i posted.
> I didnt get an ACK from Dave - so i assumed that patch is not on his 
> tree yet. Which tree are you using?

The Linus tree plus your patch.

> On Sat, 2005-04-09 at 07:12, Herbert Xu wrote:
> 
> > ===== net/xfrm/xfrm_policy.c 1.75 vs edited =====
> > --- 1.75/net/xfrm/xfrm_policy.c 2005-04-01 16:24:20 +10:00
> > +++ edited/net/xfrm/xfrm_policy.c       2005-04-09 18:02:53 +10:00
> > @@ -216,8 +216,8 @@
> > 
> > expired:
> >        read_unlock(&xp->lock);
> > -       km_policy_expired(xp, dir, 1);
> > -       xfrm_policy_delete(xp, dir);
> > +       if (!xfrm_policy_delete(xp, dir))
> > +               km_policy_expired(xp, dir, 1);
> >        xfrm_pol_put(xp);>
> > }
> >
> 
> If the policy was already dead you will still return a 0 from
> xfrm_policy_delete(). Fixable by policy_kill returning an int.

You're right.  The other callers of xfrm_policy_kill needs to
check it too.

I'll fix this up.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[Herbert Xu]

On Sun, Apr 10, 2005 at 05:29:26AM +1000, herbert wrote:
>
> > On Sat, 2005-04-09 at 07:12, Herbert Xu wrote:
> > 
> > > ===== net/xfrm/xfrm_policy.c 1.75 vs edited =====
> > > --- 1.75/net/xfrm/xfrm_policy.c 2005-04-01 16:24:20 +10:00
> > > +++ edited/net/xfrm/xfrm_policy.c       2005-04-09 18:02:53 +10:00
> > > @@ -216,8 +216,8 @@
> > > 
> > > expired:
> > >        read_unlock(&xp->lock);
> > > -       km_policy_expired(xp, dir, 1);
> > > -       xfrm_policy_delete(xp, dir);
> > > +       if (!xfrm_policy_delete(xp, dir))
> > > +               km_policy_expired(xp, dir, 1);
> > >        xfrm_pol_put(xp);>
> > > }
> > >
> > 
> > If the policy was already dead you will still return a 0 from
> > xfrm_policy_delete(). Fixable by policy_kill returning an int.
> 
> You're right.  The other callers of xfrm_policy_kill needs to
> check it too.

Actually, my code was right :)

xfrm_policy_kill can only be called by the one who removed the
policy from the linked list.  Therefore it can never fail.

If this logic is wrong we will be getting fat warnings from
xfrm_policy_kill.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[jamal]

On Sat, 2005-04-09 at 16:03, Herbert Xu wrote:

> xfrm_policy_kill can only be called by the one who removed the
> policy from the linked list.  Therefore it can never fail.
> 
> If this logic is wrong we will be getting fat warnings from
> xfrm_policy_kill.
> 

The warning will kick in but it may be as rare as the issue of a delete
and expire happening on the same policy that your patch was supposed to
stop ;->

cheers,
jamal

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[Herbert Xu]

On Sun, Apr 10, 2005 at 10:10:44AM -0400, jamal wrote:
> On Sat, 2005-04-09 at 16:03, Herbert Xu wrote:
> 
> > xfrm_policy_kill can only be called by the one who removed the
> > policy from the linked list.  Therefore it can never fail.
> > 
> > If this logic is wrong we will be getting fat warnings from
> > xfrm_policy_kill.
> 
> The warning will kick in but it may be as rare as the issue of a delete
> and expire happening on the same policy that your patch was supposed to
> stop ;->

What I am saying is that this is impossible.  If it really bothers you
we can turn the WARN_ON into a BUG.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[jamal]

Herbert,

On Sun, 2005-04-10 at 17:27, Herbert Xu wrote:

> 
> What I am saying is that this is impossible.  If it really bothers you
> we can turn the WARN_ON into a BUG.
> 

We started this discussion by asserting that an expire may be issued on
a policy (or state) despite a delete event notification already having
happened. Thats what we were/are trying to stop, no?
What i am saying is you are not stopping that with this:
---
  if (!xfrm_policy_delete(xp, dir))
        km_policy_expired(xp, dir, 1);
---

xfrm_policy_delete will return 0 whether the policy is dead or not.
OTOH, if it returns <0 for the case where it is dead, then an expire
event will not be issued. 

cheers,
jamal

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[Herbert Xu]

Hi Jamal:

On Mon, Apr 11, 2005 at 07:20:20AM -0400, jamal wrote:
>
> We started this discussion by asserting that an expire may be issued on
> a policy (or state) despite a delete event notification already having
> happened. Thats what we were/are trying to stop, no?

Yes.

> What i am saying is you are not stopping that with this:
> ---
>   if (!xfrm_policy_delete(xp, dir))
>         km_policy_expired(xp, dir, 1);
> ---
> 
> xfrm_policy_delete will return 0 whether the policy is dead or not.

xfrm_policy_delete returns 0 if and only if the policy was removed
from xfrm_policy_list.

A user-initiated delete policy action can only succeed if the policy
was removed from xfrm_policy_list.

The locking in xfrm_policy ensures that a given policy can only be
removed from the xfrm_policy_list once.

Therefore knowing that xfrm_policy_delete returned 0 implies that
no user-initiated delete action can succeed either before or after
the expire event.

Whether a policy is dead or not is equivalent to whether it's on
xfrm_policy_list.  That is, while it's on xfrm_policy_list it
must be alive.  As soon as it is removed from xfrm_policy_list it
is killed by the entity that performed the removal.

As a corollary, xfrm_policy_delete returns 0 if and only if the
policy was on xfrm_policy_list prior to the call and was successfully
removed by the call and subsequently killed.

> OTOH, if it returns <0 for the case where it is dead, then an expire
> event will not be issued. 

I'm not sure what you're trying to say here.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[jamal]

Herbert,

On Mon, 2005-04-11 at 07:30, Herbert Xu wrote:
> Hi Jamal:

> Therefore knowing that xfrm_policy_delete returned 0 implies that
> no user-initiated delete action can succeed either before or after
> the expire event.
> 

Ok, what you are saying is sensible, but: does this mean there was never
an issue then there was never a possibility of delete and expire
intefering with each other then?

cheers,
jamal

Re: [2/4] [IPSEC] Kill spurious hard expire messages

[Herbert Xu]

On Mon, Apr 11, 2005 at 07:57:29AM -0400, jamal wrote:
> Herbert,
> 
> Ok, what you are saying is sensible, but: does this mean there was never
> an issue then there was never a possibility of delete and expire
> intefering with each other then?

The problem before was that the timer didn't check the return value
of xfrm_policy_delete (in fact the latter didn't return anything).
xfrm_policy_delete tself always checked whether it succeeded in
deleting the entry from xfrm_policy_list.  This is used to determine
whether it should call xfrm_policy_kill.

Not checking the return value in the timer allows the following
situation to occur:

CPU 0				CPU 1
xfrm_policy_timer
	goto expired
				xfrm_get_policy(delete == 1)
					xfrm_policy_byid() succeeds
					notification is sent
	xfrm_policy_delete
	km_policy_expired <--- BUG

So as long as xfrm_policy_delete returns the correct value and it's
checked by the xfrm_policy_timer then we're OK.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[0/7] [IPSEC] IPsec event notification

[Herbert Xu]

Hi:

I'm reposting the series of IPsec events patches that I will be asking
Andrew to include in mm.  It's mostly the same as what's been posted
before with some new white space clean-up's and a change to the xfrm
delete sa/policy notification.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[1/7] [IPSEC] Add complete xfrm event notification

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

From: Jamal Hadi Salim <hadi@cyberus.ca>

Heres the final patch.
What this patch provides

- netlink xfrm events
- ability to have events generated by netlink propagated to pfkey
  and vice versa.
- fixes the acquire lets-be-happy-with-one-success issue

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

I've made some white space clean-up's.  I'll include an incremental
patch showing the changes in a reply to this message.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-1 --]
[-- Type: text/plain, Size: 29383 bytes --]

--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -257,5 +257,7 @@ struct xfrm_usersa_flush {
 
 #define XFRMGRP_ACQUIRE		1
 #define XFRMGRP_EXPIRE		2
+#define XFRMGRP_SA		4
+#define XFRMGRP_POLICY		8
 
 #endif /* _LINUX_XFRM_H */
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -158,6 +158,27 @@ enum {
 	XFRM_STATE_DEAD
 };
 
+/* events that could be sent by kernel */
+enum {
+	XFRM_SAP_INVALID,
+	XFRM_SAP_EXPIRED,
+	XFRM_SAP_ADDED,
+	XFRM_SAP_UPDATED,
+	XFRM_SAP_DELETED,
+	XFRM_SAP_FLUSHED,
+	__XFRM_SAP_MAX
+};
+#define XFRM_SAP_MAX (__XFRM_SAP_MAX - 1)
+
+/* callback structure passed from either netlink or pfkey */
+struct km_event
+{
+	u32	data;
+	u32	seq;
+	u32	pid;
+	u32	event;
+};
+
 struct xfrm_type;
 struct xfrm_dst;
 struct xfrm_policy_afinfo {
@@ -179,6 +200,8 @@ struct xfrm_policy_afinfo {
 
 extern int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo);
 extern int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo);
+extern void km_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c);
+extern void km_state_notify(struct xfrm_state *x, struct km_event *c);
 
 #define XFRM_ACQ_EXPIRES	30
 
@@ -290,11 +313,11 @@ struct xfrm_mgr
 {
 	struct list_head	list;
 	char			*id;
-	int			(*notify)(struct xfrm_state *x, int event);
+	int			(*notify)(struct xfrm_state *x, struct km_event *c);
 	int			(*acquire)(struct xfrm_state *x, struct xfrm_tmpl *, struct xfrm_policy *xp, int dir);
 	struct xfrm_policy	*(*compile_policy)(u16 family, int opt, u8 *data, int len, int *dir);
 	int			(*new_mapping)(struct xfrm_state *x, xfrm_address_t *ipaddr, u16 sport);
-	int			(*notify_policy)(struct xfrm_policy *x, int dir, int event);
+	int			(*notify_policy)(struct xfrm_policy *x, int dir, struct km_event *c);
 };
 
 extern int xfrm_register_km(struct xfrm_mgr *km);
@@ -815,7 +838,7 @@ extern int xfrm_state_add(struct xfrm_st
 extern int xfrm_state_update(struct xfrm_state *x);
 extern struct xfrm_state *xfrm_state_lookup(xfrm_address_t *daddr, u32 spi, u8 proto, unsigned short family);
 extern struct xfrm_state *xfrm_find_acq_byseq(u32 seq);
-extern void xfrm_state_delete(struct xfrm_state *x);
+extern int xfrm_state_delete(struct xfrm_state *x);
 extern void xfrm_state_flush(u8 proto);
 extern int xfrm_replay_check(struct xfrm_state *x, u32 seq);
 extern void xfrm_replay_advance(struct xfrm_state *x, u32 seq);
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1240,13 +1240,85 @@ static int pfkey_acquire(struct sock *sk
 	return 0;
 }
 
+static inline int event2poltype(int event)
+{
+	switch (event) {
+	case XFRM_SAP_DELETED:
+		return SADB_X_SPDDELETE;
+	case XFRM_SAP_ADDED:
+		return SADB_X_SPDADD;
+	case XFRM_SAP_UPDATED:
+		return SADB_X_SPDUPDATE;
+	case XFRM_SAP_EXPIRED:
+	//	return SADB_X_SPDEXPIRE;
+	default:
+		printk("pfkey: Unknown policy event %d\n", event);
+		break;
+	}
+
+	return 0;
+}
+
+static inline int event2keytype(int event)
+{
+	switch (event) {
+	case XFRM_SAP_DELETED:
+		return SADB_DELETE;
+	case XFRM_SAP_ADDED:
+		return SADB_ADD;
+	case XFRM_SAP_UPDATED:
+		return SADB_UPDATE;
+	case XFRM_SAP_EXPIRED:
+		return SADB_EXPIRE;
+	default:
+		printk("pfkey: Unknown SA event %d\n", event);
+		break;
+	}
+
+	return 0;
+}
+
+/* ADD/UPD/DEL */
+static int key_notify_sa(struct xfrm_state *x, struct km_event *c)
+{
+	struct sk_buff *skb;
+	struct sadb_msg *hdr;
+	int hsc = 3;
+
+	if (c->event == XFRM_SAP_DELETED)
+		hsc = 0;
+
+	if (c->event == XFRM_SAP_EXPIRED) {
+		if (c->data)
+			hsc = 2;
+		else
+			hsc = 1;
+	}
+
+	skb = pfkey_xfrm_state2msg(x, 0, hsc);
+
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	hdr = (struct sadb_msg *) skb->data;
+	hdr->sadb_msg_version = PF_KEY_V2;
+	hdr->sadb_msg_type = event2keytype(c->event);
+	hdr->sadb_msg_satype = pfkey_proto2satype(x->id.proto);
+	hdr->sadb_msg_errno = 0;
+	hdr->sadb_msg_reserved = 0;
+	hdr->sadb_msg_seq = c->seq;
+	hdr->sadb_msg_pid = c->pid;
+
+	pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL);
+
+	return 0;
+}
 
 static int pfkey_add(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
-	struct sk_buff *out_skb;
-	struct sadb_msg *out_hdr;
 	struct xfrm_state *x;
 	int err;
+	struct km_event c;
 
 	xfrm_probe_algs();
 	
@@ -1254,6 +1326,7 @@ static int pfkey_add(struct sock *sk, st
 	if (IS_ERR(x))
 		return PTR_ERR(x);
 
+	xfrm_state_hold(x);
 	if (hdr->sadb_msg_type == SADB_ADD)
 		err = xfrm_state_add(x);
 	else
@@ -1265,27 +1338,23 @@ static int pfkey_add(struct sock *sk, st
 		return err;
 	}
 
-	out_skb = pfkey_xfrm_state2msg(x, 0, 3);
-	if (IS_ERR(out_skb))
-		return  PTR_ERR(out_skb); /* XXX Should we return 0 here ? */
-
-	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version = hdr->sadb_msg_version;
-	out_hdr->sadb_msg_type = hdr->sadb_msg_type;
-	out_hdr->sadb_msg_satype = pfkey_proto2satype(x->id.proto);
-	out_hdr->sadb_msg_errno = 0;
-	out_hdr->sadb_msg_reserved = 0;
-	out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
-	out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
-
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, sk);
+	if (hdr->sadb_msg_type == SADB_ADD)
+		c.event = XFRM_SAP_ADDED;
+	else
+		c.event = XFRM_SAP_UPDATED;
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	km_state_notify(x, &c);
+	xfrm_state_put(x);
 
-	return 0;
+	return err;
 }
 
 static int pfkey_delete(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
 	struct xfrm_state *x;
+	struct km_event c;
+	int err;
 
 	if (!ext_hdrs[SADB_EXT_SA-1] ||
 	    !present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1],
@@ -1301,13 +1370,19 @@ static int pfkey_delete(struct sock *sk,
 		return -EPERM;
 	}
 	
-	xfrm_state_delete(x);
-	xfrm_state_put(x);
+	err = xfrm_state_delete(x);
+	if (err < 0) {
+		xfrm_state_put(x);
+		return err;
+	}
 
-	pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL, 
-			BROADCAST_ALL, sk);
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	c.event = XFRM_SAP_DELETED;
+	km_state_notify(x, &c);
+	xfrm_state_put(x);
 
-	return 0;
+	return err;
 }
 
 static int pfkey_get(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
@@ -1445,28 +1520,42 @@ static int pfkey_register(struct sock *s
 	return 0;
 }
 
+static int key_notify_sa_flush(struct km_event *c)
+{
+	struct sk_buff *skb;
+	struct sadb_msg *hdr;
+
+	skb = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_ATOMIC);
+	if (!skb)
+		return -ENOBUFS;
+	hdr = (struct sadb_msg *) skb_put(skb, sizeof(struct sadb_msg));
+	hdr->sadb_msg_satype = pfkey_proto2satype(c->data);
+	hdr->sadb_msg_seq = c->seq;
+	hdr->sadb_msg_pid = c->pid;
+	hdr->sadb_msg_version = PF_KEY_V2;
+	hdr->sadb_msg_errno = (uint8_t) 0;
+	hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
+
+	pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL);
+
+	return 0;
+}
+
 static int pfkey_flush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
 	unsigned proto;
-	struct sk_buff *skb_out;
-	struct sadb_msg *hdr_out;
+	struct km_event c;
 
 	proto = pfkey_satype2proto(hdr->sadb_msg_satype);
 	if (proto == 0)
 		return -EINVAL;
 
-	skb_out = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_KERNEL);
-	if (!skb_out)
-		return -ENOBUFS;
-
 	xfrm_state_flush(proto);
-
-	hdr_out = (struct sadb_msg *) skb_put(skb_out, sizeof(struct sadb_msg));
-	pfkey_hdr_dup(hdr_out, hdr);
-	hdr_out->sadb_msg_errno = (uint8_t) 0;
-	hdr_out->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
-
-	pfkey_broadcast(skb_out, GFP_KERNEL, BROADCAST_ALL, NULL);
+	c.data = proto;
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	c.event = XFRM_SAP_FLUSHED;
+	km_state_notify(NULL, &c);
 
 	return 0;
 }
@@ -1859,6 +1948,35 @@ static void pfkey_xfrm_policy2msg(struct
 	hdr->sadb_msg_reserved = atomic_read(&xp->refcnt);
 }
 
+static int key_notify_policy(struct xfrm_policy *xp, int dir, struct km_event *c)
+{
+	struct sk_buff *out_skb;
+	struct sadb_msg *out_hdr;
+	int err;
+
+	out_skb = pfkey_xfrm_policy2msg_prep(xp);
+	if (IS_ERR(out_skb)) {
+		err = PTR_ERR(out_skb);
+		goto out;
+	}
+	pfkey_xfrm_policy2msg(out_skb, xp, dir);
+
+	out_hdr = (struct sadb_msg *) out_skb->data;
+	out_hdr->sadb_msg_version = PF_KEY_V2;
+
+	if (c->data && c->event == XFRM_SAP_DELETED)
+		out_hdr->sadb_msg_type = SADB_X_SPDDELETE2;
+	else
+		out_hdr->sadb_msg_type = event2poltype(c->event);
+	out_hdr->sadb_msg_errno = 0;
+	out_hdr->sadb_msg_seq = c->seq;
+	out_hdr->sadb_msg_pid = c->pid;
+	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, NULL);
+out:
+	return 0;
+
+}
+
 static int pfkey_spdadd(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
 	int err;
@@ -1866,8 +1984,7 @@ static int pfkey_spdadd(struct sock *sk,
 	struct sadb_address *sa;
 	struct sadb_x_policy *pol;
 	struct xfrm_policy *xp;
-	struct sk_buff *out_skb;
-	struct sadb_msg *out_hdr;
+	struct km_event c;
 
 	if (!present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1],
 				     ext_hdrs[SADB_EXT_ADDRESS_DST-1]) ||
@@ -1935,31 +2052,23 @@ static int pfkey_spdadd(struct sock *sk,
 	    (err = parse_ipsecrequests(xp, pol)) < 0)
 		goto out;
 
-	out_skb = pfkey_xfrm_policy2msg_prep(xp);
-	if (IS_ERR(out_skb)) {
-		err =  PTR_ERR(out_skb);
-		goto out;
-	}
-
 	err = xfrm_policy_insert(pol->sadb_x_policy_dir-1, xp,
 				 hdr->sadb_msg_type != SADB_X_SPDUPDATE);
 	if (err) {
-		kfree_skb(out_skb);
-		goto out;
+		kfree(xp);
+		return err;
 	}
 
-	pfkey_xfrm_policy2msg(out_skb, xp, pol->sadb_x_policy_dir-1);
+	if (hdr->sadb_msg_type == SADB_X_SPDUPDATE)
+		c.event = XFRM_SAP_UPDATED;
+	else
+		c.event = XFRM_SAP_ADDED;
 
-	xfrm_pol_put(xp);
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
 
-	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version = hdr->sadb_msg_version;
-	out_hdr->sadb_msg_type = hdr->sadb_msg_type;
-	out_hdr->sadb_msg_satype = 0;
-	out_hdr->sadb_msg_errno = 0;
-	out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
-	out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, sk);
+	km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
+	xfrm_pol_put(xp);
 	return 0;
 
 out:
@@ -1973,9 +2082,8 @@ static int pfkey_spddelete(struct sock *
 	struct sadb_address *sa;
 	struct sadb_x_policy *pol;
 	struct xfrm_policy *xp;
-	struct sk_buff *out_skb;
-	struct sadb_msg *out_hdr;
 	struct xfrm_selector sel;
+	struct km_event c;
 
 	if (!present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC-1],
 				     ext_hdrs[SADB_EXT_ADDRESS_DST-1]) ||
@@ -2010,25 +2118,40 @@ static int pfkey_spddelete(struct sock *
 
 	err = 0;
 
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	c.event = XFRM_SAP_DELETED;
+	km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
+
+	xfrm_pol_put(xp);
+	return err;
+}
+
+static int key_pol_get_resp(struct sock *sk, struct xfrm_policy *xp, struct sadb_msg *hdr, int dir)
+{
+	int err;
+	struct sk_buff *out_skb;
+	struct sadb_msg *out_hdr;
+	err = 0;
+
 	out_skb = pfkey_xfrm_policy2msg_prep(xp);
 	if (IS_ERR(out_skb)) {
 		err =  PTR_ERR(out_skb);
 		goto out;
 	}
-	pfkey_xfrm_policy2msg(out_skb, xp, pol->sadb_x_policy_dir-1);
+	pfkey_xfrm_policy2msg(out_skb, xp, dir);
 
 	out_hdr = (struct sadb_msg *) out_skb->data;
 	out_hdr->sadb_msg_version = hdr->sadb_msg_version;
-	out_hdr->sadb_msg_type = SADB_X_SPDDELETE;
+	out_hdr->sadb_msg_type = hdr->sadb_msg_type;
 	out_hdr->sadb_msg_satype = 0;
 	out_hdr->sadb_msg_errno = 0;
 	out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
 	out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, sk);
+	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, sk);
 	err = 0;
 
 out:
-	xfrm_pol_put(xp);
 	return err;
 }
 
@@ -2037,8 +2160,7 @@ static int pfkey_spdget(struct sock *sk,
 	int err;
 	struct sadb_x_policy *pol;
 	struct xfrm_policy *xp;
-	struct sk_buff *out_skb;
-	struct sadb_msg *out_hdr;
+	struct km_event c;
 
 	if ((pol = ext_hdrs[SADB_X_EXT_POLICY-1]) == NULL)
 		return -EINVAL;
@@ -2050,24 +2172,16 @@ static int pfkey_spdget(struct sock *sk,
 
 	err = 0;
 
-	out_skb = pfkey_xfrm_policy2msg_prep(xp);
-	if (IS_ERR(out_skb)) {
-		err =  PTR_ERR(out_skb);
-		goto out;
+	c.seq = hdr->sadb_msg_seq;
+	c.pid = hdr->sadb_msg_pid;
+	if (hdr->sadb_msg_type == SADB_X_SPDDELETE2) {
+		c.data = 1; // to signal pfkey of SADB_X_SPDDELETE2
+		c.event = XFRM_SAP_DELETED;
+		km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
+	} else {
+		err = key_pol_get_resp(sk, xp, hdr, pol->sadb_x_policy_dir-1);
 	}
-	pfkey_xfrm_policy2msg(out_skb, xp, pol->sadb_x_policy_dir-1);
 
-	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version = hdr->sadb_msg_version;
-	out_hdr->sadb_msg_type = hdr->sadb_msg_type;
-	out_hdr->sadb_msg_satype = 0;
-	out_hdr->sadb_msg_errno = 0;
-	out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
-	out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, sk);
-	err = 0;
-
-out:
 	xfrm_pol_put(xp);
 	return err;
 }
@@ -2102,22 +2216,34 @@ static int pfkey_spddump(struct sock *sk
 	return xfrm_policy_walk(dump_sp, &data);
 }
 
-static int pfkey_spdflush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
+static int key_notify_policy_flush(struct km_event *c)
 {
 	struct sk_buff *skb_out;
-	struct sadb_msg *hdr_out;
+	struct sadb_msg *hdr;
 
-	skb_out = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_KERNEL);
+	skb_out = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_ATOMIC);
 	if (!skb_out)
 		return -ENOBUFS;
+	hdr = (struct sadb_msg *) skb_put(skb_out, sizeof(struct sadb_msg));
+	hdr->sadb_msg_seq = c->seq;
+	hdr->sadb_msg_pid = c->pid;
+	hdr->sadb_msg_version = PF_KEY_V2;
+	hdr->sadb_msg_errno = (uint8_t) 0;
+	hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
+	pfkey_broadcast(skb_out, GFP_ATOMIC, BROADCAST_ALL, NULL);
+	return 0;
 
-	xfrm_policy_flush();
+}
+
+static int pfkey_spdflush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
+{
+	struct km_event c;
 
-	hdr_out = (struct sadb_msg *) skb_put(skb_out, sizeof(struct sadb_msg));
-	pfkey_hdr_dup(hdr_out, hdr);
-	hdr_out->sadb_msg_errno = (uint8_t) 0;
-	hdr_out->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
-	pfkey_broadcast(skb_out, GFP_KERNEL, BROADCAST_ALL, NULL);
+	xfrm_policy_flush();
+	c.event = XFRM_SAP_FLUSHED;
+	c.pid = hdr->sadb_msg_pid;
+	c.seq = hdr->sadb_msg_seq;
+	km_policy_notify(NULL, 0, &c);
 
 	return 0;
 }
@@ -2317,11 +2443,23 @@ static void dump_esp_combs(struct sk_buf
 	}
 }
 
-static int pfkey_send_notify(struct xfrm_state *x, int hard)
+static int key_notify_policy_expire(struct xfrm_policy *xp, struct km_event *c)
+{
+	return 0;
+}
+
+static int key_notify_sa_expire(struct xfrm_state *x, struct km_event *c)
 {
 	struct sk_buff *out_skb;
 	struct sadb_msg *out_hdr;
-	int hsc = (hard ? 2 : 1);
+	int hard;
+	int hsc;
+
+	hard = c->data;
+	if (hard)
+		hsc = 2;
+	else
+		hsc = 1;
 
 	out_skb = pfkey_xfrm_state2msg(x, 0, hsc);
 	if (IS_ERR(out_skb))
@@ -2340,6 +2478,44 @@ static int pfkey_send_notify(struct xfrm
 	return 0;
 }
 
+static int pfkey_send_notify(struct xfrm_state *x, struct km_event *c)
+{
+	switch (c->event) {
+	case XFRM_SAP_EXPIRED:
+		return key_notify_sa_expire(x, c);
+	case XFRM_SAP_DELETED:
+	case XFRM_SAP_ADDED:
+	case XFRM_SAP_UPDATED:
+		return key_notify_sa(x, c);
+	case XFRM_SAP_FLUSHED:
+		return key_notify_sa_flush(c);
+	default:
+		printk("pfkey: Unknown SA event %d\n", c->event);
+		break;
+	}
+
+	return 0;
+}
+
+static int pfkey_send_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
+{
+	switch (c->event) {
+	case XFRM_SAP_EXPIRED:
+		return key_notify_policy_expire(xp, c);
+	case XFRM_SAP_DELETED:
+	case XFRM_SAP_ADDED:
+	case XFRM_SAP_UPDATED:
+		return key_notify_policy(xp, dir, c);
+	case XFRM_SAP_FLUSHED:
+		return key_notify_policy_flush(c);
+	default:
+		printk("pfkey: Unknown policy event %d\n", c->event);
+		break;
+	}
+
+	return 0;
+}
+
 static u32 get_acqseq(void)
 {
 	u32 res;
@@ -2856,6 +3032,7 @@ static struct xfrm_mgr pfkeyv2_mgr =
 	.acquire	= pfkey_send_acquire,
 	.compile_policy	= pfkey_compile_policy,
 	.new_mapping	= pfkey_send_new_mapping,
+	.notify_policy	= pfkey_send_policy_notify,
 };
 
 static void __exit ipsec_pfkey_exit(void)
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -50,7 +50,7 @@ static DEFINE_SPINLOCK(xfrm_state_gc_loc
 
 static int xfrm_state_gc_flush_bundles;
 
-static void __xfrm_state_delete(struct xfrm_state *x);
+static int __xfrm_state_delete(struct xfrm_state *x);
 
 static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned short family);
 static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo);
@@ -215,8 +215,10 @@ void __xfrm_state_destroy(struct xfrm_st
 }
 EXPORT_SYMBOL(__xfrm_state_destroy);
 
-static void __xfrm_state_delete(struct xfrm_state *x)
+static int __xfrm_state_delete(struct xfrm_state *x)
 {
+	int err = -ESRCH;
+
 	if (x->km.state != XFRM_STATE_DEAD) {
 		x->km.state = XFRM_STATE_DEAD;
 		spin_lock(&xfrm_state_lock);
@@ -245,14 +247,21 @@ static void __xfrm_state_delete(struct x
 		 * is what we are dropping here.
 		 */
 		atomic_dec(&x->refcnt);
+		err = 0;
 	}
+
+	return err;
 }
 
-void xfrm_state_delete(struct xfrm_state *x)
+int xfrm_state_delete(struct xfrm_state *x)
 {
+	int err;
+
 	spin_lock_bh(&x->lock);
-	__xfrm_state_delete(x);
+	err = __xfrm_state_delete(x);
 	spin_unlock_bh(&x->lock);
+
+	return err;
 }
 EXPORT_SYMBOL(xfrm_state_delete);
 
@@ -796,34 +805,60 @@ EXPORT_SYMBOL(xfrm_replay_advance);
 static struct list_head xfrm_km_list = LIST_HEAD_INIT(xfrm_km_list);
 static DEFINE_RWLOCK(xfrm_km_lock);
 
-static void km_state_expired(struct xfrm_state *x, int hard)
+void km_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct xfrm_mgr *km;
 
-	if (hard)
-		x->km.state = XFRM_STATE_EXPIRED;
-	else
-		x->km.dying = 1;
+	read_lock(&xfrm_km_lock);
+	list_for_each_entry(km, &xfrm_km_list, list)
+		if (km->notify_policy)
+			km->notify_policy(xp, dir, c);
+	read_unlock(&xfrm_km_lock);
+}
 
+void km_state_notify(struct xfrm_state *x, struct km_event *c)
+{
+	struct xfrm_mgr *km;
 	read_lock(&xfrm_km_lock);
 	list_for_each_entry(km, &xfrm_km_list, list)
-		km->notify(x, hard);
+		if (km->notify)
+			km->notify(x, c);
 	read_unlock(&xfrm_km_lock);
+}
+
+EXPORT_SYMBOL(km_policy_notify);
+EXPORT_SYMBOL(km_state_notify);
+
+static void km_state_expired(struct xfrm_state *x, int hard)
+{
+	struct km_event c;
+
+	if (hard)
+		x->km.state = XFRM_STATE_EXPIRED;
+	else
+		x->km.dying = 1;
+	c.data = hard;
+	c.event = XFRM_SAP_EXPIRED;
+	km_state_notify(x, &c);
 
 	if (hard)
 		wake_up(&km_waitq);
 }
 
+/*
+ * We send to all registered managers regardless of failure
+ * We are happy with one success
+*/
 static int km_query(struct xfrm_state *x, struct xfrm_tmpl *t, struct xfrm_policy *pol)
 {
-	int err = -EINVAL;
+	int err = -EINVAL, acqret;
 	struct xfrm_mgr *km;
 
 	read_lock(&xfrm_km_lock);
 	list_for_each_entry(km, &xfrm_km_list, list) {
-		err = km->acquire(x, t, pol, XFRM_POLICY_OUT);
-		if (!err)
-			break;
+		acqret = km->acquire(x, t, pol, XFRM_POLICY_OUT);
+		if (!acqret)
+			err = acqret;
 	}
 	read_unlock(&xfrm_km_lock);
 	return err;
@@ -848,13 +883,12 @@ EXPORT_SYMBOL(km_new_mapping);
 
 void km_policy_expired(struct xfrm_policy *pol, int dir, int hard)
 {
-	struct xfrm_mgr *km;
+	struct km_event c;
 
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list)
-		if (km->notify_policy)
-			km->notify_policy(pol, dir, hard);
-	read_unlock(&xfrm_km_lock);
+	c.data = hard;
+	c.data = hard;
+	c.event = XFRM_SAP_EXPIRED;
+	km_policy_notify(pol, dir, &c);
 
 	if (hard)
 		wake_up(&km_waitq);
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -268,6 +268,7 @@ static int xfrm_add_sa(struct sk_buff *s
 	struct xfrm_usersa_info *p = NLMSG_DATA(nlh);
 	struct xfrm_state *x;
 	int err;
+	struct km_event c;
 
 	err = verify_newsa_info(p, (struct rtattr **) xfrma);
 	if (err)
@@ -277,6 +278,7 @@ static int xfrm_add_sa(struct sk_buff *s
 	if (!x)
 		return err;
 
+	xfrm_state_hold(x);
 	if (nlh->nlmsg_type == XFRM_MSG_NEWSA)
 		err = xfrm_state_add(x);
 	else
@@ -285,14 +287,27 @@ static int xfrm_add_sa(struct sk_buff *s
 	if (err < 0) {
 		x->km.state = XFRM_STATE_DEAD;
 		xfrm_state_put(x);
+		return err;
 	}
 
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	if (nlh->nlmsg_type == XFRM_MSG_NEWSA)
+		c.event = XFRM_SAP_ADDED;
+	else
+		c.event = XFRM_SAP_UPDATED;
+
+	km_state_notify(x, &c);
+	xfrm_state_put(x);
+
 	return err;
 }
 
 static int xfrm_del_sa(struct sk_buff *skb, struct nlmsghdr *nlh, void **xfrma)
 {
 	struct xfrm_state *x;
+	int err;
+	struct km_event c;
 	struct xfrm_usersa_id *p = NLMSG_DATA(nlh);
 
 	x = xfrm_state_lookup(&p->daddr, p->spi, p->proto, p->family);
@@ -304,10 +319,19 @@ static int xfrm_del_sa(struct sk_buff *s
 		return -EPERM;
 	}
 
-	xfrm_state_delete(x);
+	err = xfrm_state_delete(x);
+	if (err < 0) {
+		xfrm_state_put(x);
+		return err;
+	}
+
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	c.event = XFRM_SAP_DELETED;
+	km_state_notify(x, &c);
 	xfrm_state_put(x);
 
-	return 0;
+	return err;
 }
 
 static void copy_to_user_state(struct xfrm_state *x, struct xfrm_usersa_info *p)
@@ -672,6 +696,7 @@ static int xfrm_add_policy(struct sk_buf
 {
 	struct xfrm_userpolicy_info *p = NLMSG_DATA(nlh);
 	struct xfrm_policy *xp;
+	struct km_event c;
 	int err;
 	int excl;
 
@@ -683,6 +708,10 @@ static int xfrm_add_policy(struct sk_buf
 	if (!xp)
 		return err;
 
+	/* shouldnt excl be based on nlh flags??
+	 * Aha! this is anti-netlink really i.e  more pfkey derived
+	 * in netlink excl is a flag and you wouldnt need
+	 * a type XFRM_MSG_UPDPOLICY - JHS */
 	excl = nlh->nlmsg_type == XFRM_MSG_NEWPOLICY;
 	err = xfrm_policy_insert(p->dir, xp, excl);
 	if (err) {
@@ -690,6 +719,15 @@ static int xfrm_add_policy(struct sk_buf
 		return err;
 	}
 
+	if (!excl)
+		c.event = XFRM_SAP_UPDATED;
+	else
+		c.event = XFRM_SAP_ADDED;
+
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	km_policy_notify(xp, p->dir, &c);
+
 	xfrm_pol_put(xp);
 
 	return 0;
@@ -807,6 +845,7 @@ static int xfrm_get_policy(struct sk_buf
 	struct xfrm_policy *xp;
 	struct xfrm_userpolicy_id *p;
 	int err;
+	struct km_event c;
 	int delete;
 
 	p = NLMSG_DATA(nlh);
@@ -834,6 +873,11 @@ static int xfrm_get_policy(struct sk_buf
 					      NETLINK_CB(skb).pid,
 					      MSG_DONTWAIT);
 		}
+	} else {
+		c.event = XFRM_SAP_DELETED;
+		c.seq = nlh->nlmsg_seq;
+		c.pid = nlh->nlmsg_pid;
+		km_policy_notify(xp, p->dir, &c);
 	}
 
 	xfrm_pol_put(xp);
@@ -843,15 +887,28 @@ static int xfrm_get_policy(struct sk_buf
 
 static int xfrm_flush_sa(struct sk_buff *skb, struct nlmsghdr *nlh, void **xfrma)
 {
+	struct km_event c;
 	struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
 
 	xfrm_state_flush(p->proto);
+	c.data = p->proto;
+	c.event = XFRM_SAP_FLUSHED;
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	km_state_notify(NULL, &c);
+
 	return 0;
 }
 
 static int xfrm_flush_policy(struct sk_buff *skb, struct nlmsghdr *nlh, void **xfrma)
 {
+	struct km_event c;
+
 	xfrm_policy_flush();
+	c.event = XFRM_SAP_FLUSHED;
+	c.seq = nlh->nlmsg_seq;
+	c.pid = nlh->nlmsg_pid;
+	km_policy_notify(NULL, 0, &c);
 	return 0;
 }
 
@@ -1060,10 +1117,12 @@ nlmsg_failure:
 	return -1;
 }
 
-static int xfrm_send_state_notify(struct xfrm_state *x, int hard)
+static int xfrm_exp_state_notify(struct xfrm_state *x, struct km_event *c)
 {
 	struct sk_buff *skb;
+	int hard = c ->data;
 
+	/* fix to do alloc using NLM macros */
 	skb = alloc_skb(sizeof(struct xfrm_user_expire) + 16, GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
@@ -1076,6 +1135,122 @@ static int xfrm_send_state_notify(struct
 	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_EXPIRE, GFP_ATOMIC);
 }
 
+static int xfrm_notify_sa_flush(struct km_event *c)
+{
+	struct xfrm_usersa_flush *p;
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	unsigned char *b;
+	int len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_flush));
+
+	skb = alloc_skb(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	b = skb->tail;
+
+	nlh = NLMSG_PUT(skb, c->pid, c->seq,
+			XFRM_MSG_FLUSHSA, sizeof(*p));
+	nlh->nlmsg_flags = 0;
+
+	p = NLMSG_DATA(nlh);
+	p->proto = c->data;
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_SA, GFP_ATOMIC);
+
+nlmsg_failure:
+	kfree_skb(skb);
+	return -1;
+}
+
+static int inline xfrm_sa_len(struct xfrm_state *x)
+{
+	int l = NLMSG_LENGTH(sizeof(struct xfrm_usersa_info));
+	if (x->aalg)
+		l += RTA_SPACE(sizeof(*x->aalg) + (x->aalg->alg_key_len+7)/8);
+	if (x->ealg)
+		l += RTA_SPACE(sizeof(*x->ealg) + (x->ealg->alg_key_len+7)/8);
+	if (x->calg)
+		l += RTA_SPACE(sizeof(*x->calg));
+	if (x->encap)
+		l += RTA_SPACE(sizeof(*x->encap));
+
+	return l;
+}
+
+static int xfrm_notify_sa(struct xfrm_state *x, struct km_event *c)
+{
+	struct xfrm_usersa_info *p;
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	u32 nlt;
+	unsigned char *b;
+	int len = xfrm_sa_len(x);
+
+	skb = alloc_skb(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	b = skb->tail;
+
+	if (c->event == XFRM_SAP_ADDED)
+		nlt = XFRM_MSG_NEWSA;
+	else if (c->event == XFRM_SAP_UPDATED)
+		nlt = XFRM_MSG_UPDSA;
+	else if (c->event == XFRM_SAP_DELETED)
+		nlt = XFRM_MSG_DELSA;
+	else
+		goto nlmsg_failure;
+
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, nlt, sizeof(*p));
+	nlh->nlmsg_flags = 0;
+
+	p = NLMSG_DATA(nlh);
+	copy_to_user_state(x, p);
+
+	if (x->aalg)
+		RTA_PUT(skb, XFRMA_ALG_AUTH,
+			sizeof(*(x->aalg))+(x->aalg->alg_key_len+7)/8, x->aalg);
+	if (x->ealg)
+		RTA_PUT(skb, XFRMA_ALG_CRYPT,
+			sizeof(*(x->ealg))+(x->ealg->alg_key_len+7)/8, x->ealg);
+	if (x->calg)
+		RTA_PUT(skb, XFRMA_ALG_COMP, sizeof(*(x->calg)), x->calg);
+
+	if (x->encap)
+		RTA_PUT(skb, XFRMA_ENCAP, sizeof(*x->encap), x->encap);
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_SA, GFP_ATOMIC);
+
+nlmsg_failure:
+rtattr_failure:
+	kfree_skb(skb);
+	return -1;
+}
+
+static int xfrm_send_state_notify(struct xfrm_state *x, struct km_event *c)
+{
+
+	switch (c->event) {
+	case XFRM_SAP_EXPIRED:
+		return xfrm_exp_state_notify(x, c);
+	case XFRM_SAP_DELETED:
+	case XFRM_SAP_UPDATED:
+	case XFRM_SAP_ADDED:
+		return xfrm_notify_sa(x, c);
+	case XFRM_SAP_FLUSHED:
+		return xfrm_notify_sa_flush(c);
+	default:
+		 printk("xfrm_user: Unknown SA event %d\n", c->event);
+		 break;
+	}
+
+	return 0;
+
+}
+
 static int build_acquire(struct sk_buff *skb, struct xfrm_state *x,
 			 struct xfrm_tmpl *xt, struct xfrm_policy *xp,
 			 int dir)
@@ -1209,7 +1384,7 @@ nlmsg_failure:
 	return -1;
 }
 
-static int xfrm_send_policy_notify(struct xfrm_policy *xp, int dir, int hard)
+static int xfrm_exp_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct sk_buff *skb;
 	size_t len;
@@ -1220,7 +1395,7 @@ static int xfrm_send_policy_notify(struc
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_polexpire(skb, xp, dir, hard) < 0)
+	if (build_polexpire(skb, xp, dir, c->data) < 0)
 		BUG();
 
 	NETLINK_CB(skb).dst_groups = XFRMGRP_EXPIRE;
@@ -1228,6 +1403,93 @@ static int xfrm_send_policy_notify(struc
 	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_EXPIRE, GFP_ATOMIC);
 }
 
+static int xfrm_notify_policy(struct xfrm_policy *xp, int dir, struct km_event *c)
+{
+	struct xfrm_userpolicy_info *p;
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	u32 nlt = 0 ;
+	unsigned char *b;
+	int len = RTA_SPACE(sizeof(struct xfrm_user_tmpl) * xp->xfrm_nr);
+	len += NLMSG_SPACE(sizeof(struct xfrm_userpolicy_info));
+
+	skb = alloc_skb(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	b = skb->tail;
+
+	if (c->event == XFRM_SAP_ADDED)
+		nlt = XFRM_MSG_NEWPOLICY;
+	else if (c->event == XFRM_SAP_UPDATED)
+		nlt = XFRM_MSG_UPDPOLICY;
+	else if (c->event == XFRM_SAP_DELETED)
+		nlt = XFRM_MSG_DELPOLICY;
+	else
+		goto nlmsg_failure;
+
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, nlt, sizeof(*p));
+
+	p = NLMSG_DATA(nlh);
+
+	nlh->nlmsg_flags = 0;
+
+	copy_to_user_policy(xp, p, dir);
+	if (copy_to_user_tmpl(xp, skb) < 0)
+		goto nlmsg_failure;
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_POLICY, GFP_ATOMIC);
+
+nlmsg_failure:
+	kfree_skb(skb);
+	return -1;
+}
+
+static int xfrm_notify_policy_flush(struct km_event *c)
+{
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	unsigned char *b;
+	int len = NLMSG_LENGTH(0);
+
+	skb = alloc_skb(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	b = skb->tail;
+
+
+	nlh = NLMSG_PUT(skb, c->pid, c->seq, XFRM_MSG_FLUSHPOLICY, 0);
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_POLICY, GFP_ATOMIC);
+
+nlmsg_failure:
+	kfree_skb(skb);
+	return -1;
+}
+
+static int xfrm_send_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
+{
+
+	switch (c->event) {
+	case XFRM_SAP_ADDED:
+	case XFRM_SAP_UPDATED:
+	case XFRM_SAP_DELETED:
+		return xfrm_notify_policy(xp, dir, c);
+	case XFRM_SAP_FLUSHED:
+		return xfrm_notify_policy_flush(c);
+	case XFRM_SAP_EXPIRED:
+		return xfrm_exp_policy_notify(xp, dir, c);
+	default:
+		printk("xfrm_user: Unknown Policy event %d\n", c->event);
+	}
+
+	return 0;
+
+}
+
 static struct xfrm_mgr netlink_mgr = {
 	.id		= "netlink",
 	.notify		= xfrm_send_state_notify,

Re: [1/7] [IPSEC] Add complete xfrm event notification

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

On Sat, May 07, 2005 at 05:18:24PM +1000, herbert wrote:
> 
> I've made some white space clean-up's.  I'll include an incremental
> patch showing the changes in a reply to this message.

Here goes.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-white-space --]
[-- Type: text/plain, Size: 7631 bytes --]

diff -urN old/include/net/xfrm.h new/include/net/xfrm.h
--- old/include/net/xfrm.h	2005-05-07 15:54:50.000000000 +1000
+++ new/include/net/xfrm.h	2005-05-07 15:38:09.000000000 +1000
@@ -173,13 +173,12 @@
 /* callback structure passed from either netlink or pfkey */
 struct km_event
 {
-	u32	data; 
-	u32	seq; 
-	u32	pid; 
-	u32	event; 
+	u32	data;
+	u32	seq;
+	u32	pid;
+	u32	event;
 };
 
-
 struct xfrm_type;
 struct xfrm_dst;
 struct xfrm_policy_afinfo {
@@ -204,7 +203,6 @@
 extern void km_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c);
 extern void km_state_notify(struct xfrm_state *x, struct km_event *c);
 
-
 #define XFRM_ACQ_EXPIRES	30
 
 struct xfrm_tmpl;
@@ -309,8 +307,8 @@
 	struct xfrm_tmpl       	xfrm_vec[XFRM_MAX_DEPTH];
 };
 
-
 #define XFRM_KM_TIMEOUT		30
+
 struct xfrm_mgr
 {
 	struct list_head	list;
diff -urN old/net/key/af_key.c new/net/key/af_key.c
--- old/net/key/af_key.c	2005-05-07 15:54:50.000000000 +1000
+++ new/net/key/af_key.c	2005-05-07 15:51:42.000000000 +1000
@@ -1240,7 +1240,7 @@
 	return 0;
 }
 
-static inline int event2poltype (int event)
+static inline int event2poltype(int event)
 {
 	switch (event) {
 	case XFRM_SAP_DELETED:
@@ -1252,14 +1252,14 @@
 	case XFRM_SAP_EXPIRED:
 	//	return SADB_X_SPDEXPIRE;
 	default:
-		printk("pfkey: Unknown policy event %d\n",event);
+		printk("pfkey: Unknown policy event %d\n", event);
 		break;
 	}
 
 	return 0;
 }
 
-static inline int event2keytype (int event)
+static inline int event2keytype(int event)
 {
 	switch (event) {
 	case XFRM_SAP_DELETED:
@@ -1271,7 +1271,7 @@
 	case XFRM_SAP_EXPIRED:
 		return SADB_EXPIRE;
 	default:
-		printk("pfkey: Unknown SA event %d\n",event);
+		printk("pfkey: Unknown SA event %d\n", event);
 		break;
 	}
 
@@ -1298,7 +1298,7 @@
 	skb = pfkey_xfrm_state2msg(x, 0, hsc);
 
 	if (IS_ERR(skb))
-		return  PTR_ERR(skb);
+		return PTR_ERR(skb);
 
 	hdr = (struct sadb_msg *) skb->data;
 	hdr->sadb_msg_version = PF_KEY_V2;
@@ -1948,7 +1948,7 @@
 	hdr->sadb_msg_reserved = atomic_read(&xp->refcnt);
 }
 
-static int key_notify_policy( struct xfrm_policy *xp, int dir, struct km_event *c)
+static int key_notify_policy(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct sk_buff *out_skb;
 	struct sadb_msg *out_hdr;
@@ -1956,13 +1956,13 @@
 
 	out_skb = pfkey_xfrm_policy2msg_prep(xp);
 	if (IS_ERR(out_skb)) {
-		err =  PTR_ERR(out_skb);
+		err = PTR_ERR(out_skb);
 		goto out;
 	}
 	pfkey_xfrm_policy2msg(out_skb, xp, dir);
 
 	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version =  PF_KEY_V2;
+	out_hdr->sadb_msg_version = PF_KEY_V2;
 
 	if (c->data && c->event == XFRM_SAP_DELETED)
 		out_hdr->sadb_msg_type = SADB_X_SPDDELETE2;
@@ -2052,18 +2052,16 @@
 	    (err = parse_ipsecrequests(xp, pol)) < 0)
 		goto out;
 
-
 	err = xfrm_policy_insert(pol->sadb_x_policy_dir-1, xp,
 				 hdr->sadb_msg_type != SADB_X_SPDUPDATE);
-
 	if (err) {
 		kfree(xp);
 		return err;
 	}
 
-	if (hdr->sadb_msg_type == SADB_X_SPDUPDATE) 
+	if (hdr->sadb_msg_type == SADB_X_SPDUPDATE)
 		c.event = XFRM_SAP_UPDATED;
-	else 
+	else
 		c.event = XFRM_SAP_ADDED;
 
 	c.seq = hdr->sadb_msg_seq;
@@ -2129,7 +2127,6 @@
 	return err;
 }
 
-
 static int key_pol_get_resp(struct sock *sk, struct xfrm_policy *xp, struct sadb_msg *hdr, int dir)
 {
 	int err;
@@ -2223,6 +2220,7 @@
 {
 	struct sk_buff *skb_out;
 	struct sadb_msg *hdr;
+
 	skb_out = alloc_skb(sizeof(struct sadb_msg) + 16, GFP_ATOMIC);
 	if (!skb_out)
 		return -ENOBUFS;
@@ -2445,7 +2443,6 @@
 	}
 }
 
-/* XXX: Noisy for now */
 static int key_notify_policy_expire(struct xfrm_policy *xp, struct km_event *c)
 {
 	return 0;
@@ -2493,7 +2490,7 @@
 	case XFRM_SAP_FLUSHED:
 		return key_notify_sa_flush(c);
 	default:
-		printk("pfkey: Unknown SA event %d\n",c->event);
+		printk("pfkey: Unknown SA event %d\n", c->event);
 		break;
 	}
 
@@ -2512,12 +2509,13 @@
 	case XFRM_SAP_FLUSHED:
 		return key_notify_policy_flush(c);
 	default:
-		printk("pfkey: Unknown policy event %d\n",c->event);
+		printk("pfkey: Unknown policy event %d\n", c->event);
 		break;
 	}
 
 	return 0;
 }
+
 static u32 get_acqseq(void)
 {
 	u32 res;
diff -urN old/net/xfrm/xfrm_state.c new/net/xfrm/xfrm_state.c
--- old/net/xfrm/xfrm_state.c	2005-05-07 15:54:50.000000000 +1000
+++ new/net/xfrm/xfrm_state.c	2005-05-07 15:44:40.000000000 +1000
@@ -440,7 +440,6 @@
 
 static struct xfrm_state *__xfrm_find_acq_byseq(u32 seq);
 
-
 int xfrm_state_add(struct xfrm_state *x)
 {
 	struct xfrm_state_afinfo *afinfo;
diff -urN old/net/xfrm/xfrm_user.c new/net/xfrm/xfrm_user.c
--- old/net/xfrm/xfrm_user.c	2005-05-07 15:54:50.000000000 +1000
+++ new/net/xfrm/xfrm_user.c	2005-05-07 15:53:09.000000000 +1000
@@ -359,7 +359,6 @@
 	int this_idx;
 };
 
-
 static int dump_one_state(struct xfrm_state *x, int count, void *ptr)
 {
 	struct xfrm_dump_info *sp = ptr;
@@ -709,7 +708,7 @@
 	if (!xp)
 		return err;
 
-	/* shouldnt excl be based on nlh flags?? 
+	/* shouldnt excl be based on nlh flags??
 	 * Aha! this is anti-netlink really i.e  more pfkey derived
 	 * in netlink excl is a flag and you wouldnt need
 	 * a type XFRM_MSG_UPDPOLICY - JHS */
@@ -720,7 +719,6 @@
 		return err;
 	}
 
-
 	if (!excl)
 		c.event = XFRM_SAP_UPDATED;
 	else
@@ -850,7 +848,6 @@
 	struct km_event c;
 	int delete;
 
-
 	p = NLMSG_DATA(nlh);
 	delete = nlh->nlmsg_type == XFRM_MSG_DELPOLICY;
 
@@ -1124,6 +1121,7 @@
 {
 	struct sk_buff *skb;
 	int hard = c ->data;
+
 	/* fix to do alloc using NLM macros */
 	skb = alloc_skb(sizeof(struct xfrm_user_expire) + 16, GFP_ATOMIC);
 	if (skb == NULL)
@@ -1150,7 +1148,7 @@
 		return -ENOMEM;
 	b = skb->tail;
 
-	nlh = NLMSG_PUT(skb, c->pid,  c->seq,
+	nlh = NLMSG_PUT(skb, c->pid, c->seq,
 			XFRM_MSG_FLUSHSA, sizeof(*p));
 	nlh->nlmsg_flags = 0;
 
@@ -1170,13 +1168,13 @@
 {
 	int l = NLMSG_LENGTH(sizeof(struct xfrm_usersa_info));
 	if (x->aalg)
-		l+= RTA_SPACE(sizeof(*(x->aalg))+(x->aalg->alg_key_len+7)/8);
+		l += RTA_SPACE(sizeof(*x->aalg) + (x->aalg->alg_key_len+7)/8);
 	if (x->ealg)
-		l+= RTA_SPACE(sizeof(*(x->ealg))+(x->ealg->alg_key_len+7)/8);
+		l += RTA_SPACE(sizeof(*x->ealg) + (x->ealg->alg_key_len+7)/8);
 	if (x->calg)
-		l+= RTA_SPACE(sizeof(*(x->calg)));
+		l += RTA_SPACE(sizeof(*x->calg));
 	if (x->encap)
-		l+= RTA_SPACE(sizeof(*x->encap));
+		l += RTA_SPACE(sizeof(*x->encap));
 
 	return l;
 }
@@ -1215,7 +1213,7 @@
 			sizeof(*(x->aalg))+(x->aalg->alg_key_len+7)/8, x->aalg);
 	if (x->ealg)
 		RTA_PUT(skb, XFRMA_ALG_CRYPT,
-			sizeof(*(x->ealg))+(x->ealg->alg_key_len+7)/8, x->ealg);        
+			sizeof(*(x->ealg))+(x->ealg->alg_key_len+7)/8, x->ealg);
 	if (x->calg)
 		RTA_PUT(skb, XFRMA_ALG_COMP, sizeof(*(x->calg)), x->calg);
 
@@ -1245,7 +1243,7 @@
 	case XFRM_SAP_FLUSHED:
 		return xfrm_notify_sa_flush(c);
 	default:
-		 printk("xfrm_user: Unknown SA event %d\n",c->event);
+		 printk("xfrm_user: Unknown SA event %d\n", c->event);
 		 break;
 	}
 
@@ -1386,7 +1384,6 @@
 	return -1;
 }
 
-
 static int xfrm_exp_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct sk_buff *skb;
@@ -1406,7 +1403,7 @@
 	return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_EXPIRE, GFP_ATOMIC);
 }
 
-static int xfrm_notify_policy( struct xfrm_policy *xp, int dir, struct km_event *c)
+static int xfrm_notify_policy(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct xfrm_userpolicy_info *p;
 	struct nlmsghdr *nlh;
@@ -1486,7 +1483,7 @@
 	case XFRM_SAP_EXPIRED:
 		return xfrm_exp_policy_notify(xp, dir, c);
 	default:
-		printk("xfrm_user: Unknown Policy event %d\n",c->event);
+		printk("xfrm_user: Unknown Policy event %d\n", c->event);
 	}
 
 	return 0;

[2/7] [IPSEC] Fix xfrm to pfkey SA state conversion

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

Hi:

This patch adjusts the SA state conversion in af_key such that
XFRM_STATE_ERROR/XFRM_STATE_DEAD will be converted to SADB_STATE_DEAD
instead of SADB_STATE_DYING.

According to RFC 2367, SADB_STATE_DYING SAs can be turned into
mature ones through updating their lifetime settings.  Since SAs
which are in the states XFRM_STATE_ERROR/XFRM_STATE_DEAD cannot
be resurrected, this value is unsuitable.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-2 --]
[-- Type: text/plain, Size: 855 bytes --]

--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -656,13 +656,18 @@ static struct sk_buff * pfkey_xfrm_state
 	sa->sadb_sa_exttype = SADB_EXT_SA;
 	sa->sadb_sa_spi = x->id.spi;
 	sa->sadb_sa_replay = x->props.replay_window;
-	sa->sadb_sa_state = SADB_SASTATE_DYING;
-	if (x->km.state == XFRM_STATE_VALID && !x->km.dying)
-		sa->sadb_sa_state = SADB_SASTATE_MATURE;
-	else if (x->km.state == XFRM_STATE_ACQ)
+	switch (x->km.state) {
+	case XFRM_STATE_VALID:
+		sa->sadb_sa_state = x->km.dying ?
+			SADB_SASTATE_DYING : SADB_SASTATE_MATURE;
+		break;
+	case XFRM_STATE_ACQ:
 		sa->sadb_sa_state = SADB_SASTATE_LARVAL;
-	else if (x->km.state == XFRM_STATE_EXPIRED)
+		break;
+	default:
 		sa->sadb_sa_state = SADB_SASTATE_DEAD;
+		break;
+	}
 	sa->sadb_sa_auth = 0;
 	if (x->aalg) {
 		struct xfrm_algo_desc *a = xfrm_aalg_get_byname(x->aalg->alg_name, 0);

[3/7] [IPSEC] Kill spurious hard expire messages

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]

Hi:

This patch ensures that the hard state/policy expire notifications are
only sent when the state/policy is successfully removed from their
respective tables.

As it is, it's possible for a state/policy to both expire through
reaching a hard limit, as well as being deleted by the user.

Note that this behaviour isn't actually forbidden by RFC 2367.
However, it is a quality of implementation issue.

As an added bonus, the restructuring in this patch will help
eventually in moving the expire notifications from softirq
context into process context, thus improving their reliability.

One important side-effect from this change is that SAs reaching
their hard byte/packet limits are now deleted immediately, just
like SAs that have reached their hard time limits.

Previously they were announced immediately but only deleted after
30 seconds.

This is bad because it prevents the system from issuing an ACQUIRE
command until the existing state was deleted by the user or expires
after the time is up.

In the scenario where the expire notification was lost this introduces
a 30 second delay into the system for no good reason.
 
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
 
Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-3 --]
[-- Type: text/plain, Size: 2489 bytes --]

--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -677,7 +677,7 @@ static inline int xfrm_sk_clone_policy(s
 	return 0;
 }
 
-extern void xfrm_policy_delete(struct xfrm_policy *pol, int dir);
+extern int xfrm_policy_delete(struct xfrm_policy *pol, int dir);
 
 static inline void xfrm_sk_free_policy(struct sock *sk)
 {
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -216,8 +216,8 @@ out:
 
 expired:
 	read_unlock(&xp->lock);
-	km_policy_expired(xp, dir, 1);
-	xfrm_policy_delete(xp, dir);
+	if (!xfrm_policy_delete(xp, dir))
+		km_policy_expired(xp, dir, 1);
 	xfrm_pol_put(xp);
 }
 
@@ -555,7 +555,7 @@ static struct xfrm_policy *__xfrm_policy
 	return NULL;
 }
 
-void xfrm_policy_delete(struct xfrm_policy *pol, int dir)
+int xfrm_policy_delete(struct xfrm_policy *pol, int dir)
 {
 	write_lock_bh(&xfrm_policy_lock);
 	pol = __xfrm_policy_unlink(pol, dir);
@@ -564,7 +564,9 @@ void xfrm_policy_delete(struct xfrm_poli
 		if (dir < XFRM_POLICY_MAX)
 			atomic_inc(&flow_cache_genid);
 		xfrm_policy_kill(pol);
+		return 0;
 	}
+	return -ENOENT;
 }
 
 int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol)
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -154,6 +154,7 @@ static void xfrm_timer_handler(unsigned 
 			next = tmo;
 	}
 
+	x->km.dying = warn;
 	if (warn)
 		km_state_expired(x, 0);
 resched:
@@ -169,9 +170,8 @@ expired:
 		next = 2;
 		goto resched;
 	}
-	if (x->id.spi != 0)
+	if (!__xfrm_state_delete(x) && x->id.spi)
 		km_state_expired(x, 1);
-	__xfrm_state_delete(x);
 
 out:
 	spin_unlock(&x->lock);
@@ -566,16 +566,18 @@ int xfrm_state_check_expire(struct xfrm_
 
 	if (x->curlft.bytes >= x->lft.hard_byte_limit ||
 	    x->curlft.packets >= x->lft.hard_packet_limit) {
-		km_state_expired(x, 1);
-		if (!mod_timer(&x->timer, jiffies + XFRM_ACQ_EXPIRES*HZ))
+		x->km.state = XFRM_STATE_EXPIRED;
+		if (!mod_timer(&x->timer, jiffies))
 			xfrm_state_hold(x);
 		return -EINVAL;
 	}
 
 	if (!x->km.dying &&
 	    (x->curlft.bytes >= x->lft.soft_byte_limit ||
-	     x->curlft.packets >= x->lft.soft_packet_limit))
+	     x->curlft.packets >= x->lft.soft_packet_limit)) {
+		x->km.dying = 1;
 		km_state_expired(x, 0);
+	}
 	return 0;
 }
 EXPORT_SYMBOL(xfrm_state_check_expire);
@@ -833,10 +835,6 @@ static void km_state_expired(struct xfrm
 {
 	struct km_event c;
 
-	if (hard)
-		x->km.state = XFRM_STATE_EXPIRED;
-	else
-		x->km.dying = 1;
 	c.data = hard;
 	c.event = XFRM_SAP_EXPIRED;
 	km_state_notify(x, &c);

[4/7] [IPSEC] Turn km_event.data into a union

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

Hi:

This patch turns km_event.data into a union.  This makes code that
uses it clearer.
  
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: xfrm-event-4 --]
[-- Type: text/plain, Size: 3761 bytes --]

--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -173,7 +173,12 @@ enum {
 /* callback structure passed from either netlink or pfkey */
 struct km_event
 {
-	u32	data;
+	union {
+		u32 hard;
+		u32 proto;
+		u32 byid;
+	} data;
+
 	u32	seq;
 	u32	pid;
 	u32	event;
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1293,13 +1293,6 @@ static int key_notify_sa(struct xfrm_sta
 	if (c->event == XFRM_SAP_DELETED)
 		hsc = 0;
 
-	if (c->event == XFRM_SAP_EXPIRED) {
-		if (c->data)
-			hsc = 2;
-		else
-			hsc = 1;
-	}
-
 	skb = pfkey_xfrm_state2msg(x, 0, hsc);
 
 	if (IS_ERR(skb))
@@ -1534,7 +1527,7 @@ static int key_notify_sa_flush(struct km
 	if (!skb)
 		return -ENOBUFS;
 	hdr = (struct sadb_msg *) skb_put(skb, sizeof(struct sadb_msg));
-	hdr->sadb_msg_satype = pfkey_proto2satype(c->data);
+	hdr->sadb_msg_satype = pfkey_proto2satype(c->data.proto);
 	hdr->sadb_msg_seq = c->seq;
 	hdr->sadb_msg_pid = c->pid;
 	hdr->sadb_msg_version = PF_KEY_V2;
@@ -1556,7 +1549,7 @@ static int pfkey_flush(struct sock *sk, 
 		return -EINVAL;
 
 	xfrm_state_flush(proto);
-	c.data = proto;
+	c.data.proto = proto;
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
 	c.event = XFRM_SAP_FLUSHED;
@@ -1969,7 +1962,7 @@ static int key_notify_policy(struct xfrm
 	out_hdr = (struct sadb_msg *) out_skb->data;
 	out_hdr->sadb_msg_version = PF_KEY_V2;
 
-	if (c->data && c->event == XFRM_SAP_DELETED)
+	if (c->data.byid && c->event == XFRM_SAP_DELETED)
 		out_hdr->sadb_msg_type = SADB_X_SPDDELETE2;
 	else
 		out_hdr->sadb_msg_type = event2poltype(c->event);
@@ -2180,7 +2173,7 @@ static int pfkey_spdget(struct sock *sk,
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
 	if (hdr->sadb_msg_type == SADB_X_SPDDELETE2) {
-		c.data = 1; // to signal pfkey of SADB_X_SPDDELETE2
+		c.data.byid = 1;
 		c.event = XFRM_SAP_DELETED;
 		km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
 	} else {
@@ -2460,7 +2453,7 @@ static int key_notify_sa_expire(struct x
 	int hard;
 	int hsc;
 
-	hard = c->data;
+	hard = c->data.hard;
 	if (hard)
 		hsc = 2;
 	else
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -835,7 +835,7 @@ static void km_state_expired(struct xfrm
 {
 	struct km_event c;
 
-	c.data = hard;
+	c.data.hard = hard;
 	c.event = XFRM_SAP_EXPIRED;
 	km_state_notify(x, &c);
 
@@ -883,8 +883,7 @@ void km_policy_expired(struct xfrm_polic
 {
 	struct km_event c;
 
-	c.data = hard;
-	c.data = hard;
+	c.data.hard = hard;
 	c.event = XFRM_SAP_EXPIRED;
 	km_policy_notify(pol, dir, &c);
 
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -891,7 +891,7 @@ static int xfrm_flush_sa(struct sk_buff 
 	struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
 
 	xfrm_state_flush(p->proto);
-	c.data = p->proto;
+	c.data.proto = p->proto;
 	c.event = XFRM_SAP_FLUSHED;
 	c.seq = nlh->nlmsg_seq;
 	c.pid = nlh->nlmsg_pid;
@@ -1120,14 +1120,13 @@ nlmsg_failure:
 static int xfrm_exp_state_notify(struct xfrm_state *x, struct km_event *c)
 {
 	struct sk_buff *skb;
-	int hard = c ->data;
 
 	/* fix to do alloc using NLM macros */
 	skb = alloc_skb(sizeof(struct xfrm_user_expire) + 16, GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_expire(skb, x, hard) < 0)
+	if (build_expire(skb, x, c->data.hard) < 0)
 		BUG();
 
 	NETLINK_CB(skb).dst_groups = XFRMGRP_EXPIRE;
@@ -1153,7 +1152,7 @@ static int xfrm_notify_sa_flush(struct k
 	nlh->nlmsg_flags = 0;
 
 	p = NLMSG_DATA(nlh);
-	p->proto = c->data;
+	p->proto = c->data.proto;
 
 	nlh->nlmsg_len = skb->tail - b;
 
@@ -1395,7 +1394,7 @@ static int xfrm_exp_policy_notify(struct
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_polexpire(skb, xp, dir, c->data) < 0)
+	if (build_polexpire(skb, xp, dir, c->data.hard) < 0)
 		BUG();
 
 	NETLINK_CB(skb).dst_groups = XFRMGRP_EXPIRE;

Re: [7/7] [IPSEC] Add XFRMA_SA/XFRMA_POLICY for delete notification

[jamal]

On Sat, 2005-07-05 at 17:23 +1000, Herbert Xu wrote:
> Hi:
> 
> This patch changes the format of the XFRM_MSG_DELSA and
> XFRM_MSG_DELPOLICY notification so that the main message
> sent is of the same format as that received by the kernel
> if the original message was via netlink.  This also means
> that we won't lose the byid information carried in km_event.
> 

This is incosistent in two ways:
1) Typical netlink behavior is to return the object being deleted.
Every other netlink user behaves that way - the only exception is sone
filters in tc and this is because they cant retrieve that detail
(something that needs resolution at some point). There is nothing that
xfrm_usersa_id provides that can be found in xfrm_usersa_info.
Same for the policy.

2) You cant have one behavior when something is deleted by pfkey and a
different one when it is deleted by netlink.

So i would recommend pulling this one out.

cheers,
jamal

Re: [7/7] [IPSEC] Add XFRMA_SA/XFRMA_POLICY for delete notification

[Herbert Xu]

On Sat, May 07, 2005 at 08:04:16AM -0400, jamal wrote:
> 
> This is incosistent in two ways:
> 1) Typical netlink behavior is to return the object being deleted.
> Every other netlink user behaves that way - the only exception is sone
> filters in tc and this is because they cant retrieve that detail
> (something that needs resolution at some point). There is nothing that
> xfrm_usersa_id provides that can be found in xfrm_usersa_info.
> Same for the policy.

This analogy is flawed since unlike other rtnetlink delete operations
the xfrm delete operations do not carry the same payload type as their
add/update cousins.

> 2) You cant have one behavior when something is deleted by pfkey and a
> different one when it is deleted by netlink.

As far as I can see the behaviour is identical.

> So i would recommend pulling this one out.

Well I don't really have time to get into another debate so I'll
chuck this one out.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [7/7] [IPSEC] Add XFRMA_SA/XFRMA_POLICY for delete notification

[jamal]

On Sat, 2005-07-05 at 22:25 +1000, Herbert Xu wrote:
> On Sat, May 07, 2005 at 08:04:16AM -0400, jamal wrote:
> > 
> > This is incosistent in two ways:
> > 1) Typical netlink behavior is to return the object being deleted.
> > Every other netlink user behaves that way - the only exception is sone
> > filters in tc and this is because they cant retrieve that detail
> > (something that needs resolution at some point). There is nothing that
> > xfrm_usersa_id provides that can be found in xfrm_usersa_info.
> > Same for the policy.
> 
> This analogy is flawed since unlike other rtnetlink delete operations
> the xfrm delete operations do not carry the same payload type as their
> add/update cousins.
> 

No, this is not true. Study the tc code.
It is nice to be able to return exactly the same detail - user space can
then infer what exactly happened. It is nicer to be able to return more
detail because user space doesnt have to infer anything.

> > 2) You cant have one behavior when something is deleted by pfkey and a
> > different one when it is deleted by netlink.
> 
> As far as I can see the behaviour is identical.
> 

If this is true, then #1 is forgivable.  This was my main concern.

You describe the patch this way

---
This patch changes the format of the XFRM_MSG_DELSA and
XFRM_MSG_DELPOLICY notification so that the main message
sent is of the same format as that received by the kernel
if the original message was via netlink.
----

That it only happens when you delete via netlink. Is this not so?

cheers,
jamal

Re: [7/7] [IPSEC] Add XFRMA_SA/XFRMA_POLICY for delete notification

[Herbert Xu]

On Sat, May 07, 2005 at 08:46:44AM -0400, jamal wrote:
> 
> No, this is not true. Study the tc code.
> It is nice to be able to return exactly the same detail - user space can
> then infer what exactly happened. It is nicer to be able to return more
> detail because user space doesnt have to infer anything.

This patch is making it return more detail, not less! The full
description of the deleted policy/state is still being returned,
albeit as RTA payloads.

Prior to the change, netlink users do not know whether the original
policy delete request was by selector or by id.  Now that information
is also returned.

> You describe the patch this way
> 
> ---
> This patch changes the format of the XFRM_MSG_DELSA and
> XFRM_MSG_DELPOLICY notification so that the main message
> sent is of the same format as that received by the kernel
> if the original message was via netlink.
> ----
> 
> That it only happens when you delete via netlink. Is this not so?

The same change applies even if you sent the delete via pfkey.  What
the change does is to make netlink always send a delete message that
is valid in the sense that if you sent it back to netlink then it
would delete that policy/state.

As it is the netlink delete messages sent by notification are invalid
by its own standard.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [7/7] [IPSEC] Add XFRMA_SA/XFRMA_POLICY for delete notification

[jamal]

On Sun, 2005-08-05 at 05:35 +1000, Herbert Xu wrote:
> On Sat, May 07, 2005 at 08:46:44AM -0400, jamal wrote:
> > 
> > No, this is not true. Study the tc code.
> > It is nice to be able to return exactly the same detail - user space can
> > then infer what exactly happened. It is nicer to be able to return more
> > detail because user space doesnt have to infer anything.
> 
> This patch is making it return more detail, not less! The full
> description of the deleted policy/state is still being returned,
> albeit as RTA payloads.

It's the other extreme of what i thought.
The only thing user space needs to know is what object was deleted.
If you just passed the id, it could be deduced if user space maintained
state; if you pass the object, no such deduction is needed.

> Prior to the change, netlink users do not know whether the original
> policy delete request was by selector or by id.  Now that information
> is also returned.
> 

Why would someone need to deduce whether it has been deleted by index or
selector?
If you gave me the object - i can reproduce the action of deleting it.
Thats the _only_ important_ thing.

> > You describe the patch this way
> > 
> > ---
> > This patch changes the format of the XFRM_MSG_DELSA and
> > XFRM_MSG_DELPOLICY notification so that the main message
> > sent is of the same format as that received by the kernel
> > if the original message was via netlink.
> > ----
> > 
> > That it only happens when you delete via netlink. Is this not so?
> 
> The same change applies even if you sent the delete via pfkey.  What
> the change does is to make netlink always send a delete message that
> is valid in the sense that if you sent it back to netlink then it
> would delete that policy/state.
> 

Does pfkey have ability to delete by index and selector?
If yes, how do you distinguish the two cases when you are sending the
netlink event?

> As it is the netlink delete messages sent by notification are invalid
> by its own standard.
> 

It doesnt seem to me what you provided in the patch produces exactly the
same thing that was sent by user space back in the event.
You introduce some other new TLVs to reflect it back.
But even that is besides the point:
None of those xfrm objects obey any of the standard rules 
netlink uses to begin with - you have more than one way of deleting an
object. What you need to do is the detail of what object was deleted.

Heres what i will say so we can put this to rest:
The patch is unneeded (i hate to use strong words like bogus - but it is
getting close to that), but if you feel strongly about it just lets have
it well documented and provide the iproute2 patch as well.

cheers,
jamal

Re: [7/7] [IPSEC] Add XFRMA_SA/XFRMA_POLICY for delete notification

[Herbert Xu]

On Sun, May 08, 2005 at 09:56:33AM -0400, jamal wrote:
> 
> Why would someone need to deduce whether it has been deleted by index or
> selector?

It isn't just about deducing the message.  It's about sending a delete
message in the same format as we would receive them.  As it is the
delete message sent would be not be accepted if you sent it back to the

> Does pfkey have ability to delete by index and selector?

Yes.

> If yes, how do you distinguish the two cases when you are sending the
> netlink event?

Using the byid attribute that *you* introduced :)

> > As it is the netlink delete messages sent by notification are invalid
> > by its own standard.
> 
> It doesnt seem to me what you provided in the patch produces exactly the
> same thing that was sent by user space back in the event.

That's not the point.  The point is if you send exactly the same
message to the kernel, even with the RTAs attached, the kernel
would accept it and perform the deletion if there is a matching
policy.
 
> Heres what i will say so we can put this to rest:
> The patch is unneeded (i hate to use strong words like bogus - but it is
> getting close to that), but if you feel strongly about it just lets have
> it well documented and provide the iproute2 patch as well.

I'll leave the decision up to Dave.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [7/7] [IPSEC] Add XFRMA_SA/XFRMA_POLICY for delete notification

[jamal]

On Mon, 2005-09-05 at 07:40 +1000, Herbert Xu wrote:
> On Sun, May 08, 2005 at 09:56:33AM -0400, jamal wrote:
> > 
> > Why would someone need to deduce whether it has been deleted by index or
> > selector?
> 
> It isn't just about deducing the message.  It's about sending a delete
> message in the same format as we would receive them.  As it is the
> delete message sent would be not be accepted if you sent it back to the
> 

If you enumerate all netlink messages, you will see this is not always
the case. It is a nice but not a necessary condition; infact, not even
what you generate with that patch is _the same_ message that was sent
(you add new TLVs in the response that didnt exist in user->kernel).

What is necessary is that if i look at the event i know exactly what was
deleted. If i have such detail, i can build the message that was sent
from user->kernel to delete the object (because i know exactly what was
deleted). 
As an example:
I can derive the xfrm_usersa_id that was sent to the kernel if the event
sent me xfrm_usersa_info and therefore build the same a message that
will delete _exactly_ the same object. 

It does get worse on occasion (I can point at tc filters) - where you
really cant derive the deleted object.


> > If yes, how do you distinguish the two cases when you are sending the
> > netlink event?
> 
> Using the byid attribute that *you* introduced :)
> 

Ok, theres no inconsistency then.

> > It doesnt seem to me what you provided in the patch produces exactly the
> > same thing that was sent by user space back in the event.
> 
> That's not the point.  The point is if you send exactly the same
> message to the kernel, even with the RTAs attached, the kernel
> would accept it and perform the deletion if there is a matching
> policy.

So you are depending on the kernel not checking for the extra TLVs you
send?;->
Refer to what i said above.

>  
> > Heres what i will say so we can put this to rest:
> > The patch is unneeded (i hate to use strong words like bogus - but it is
> > getting close to that), but if you feel strongly about it just lets have
> > it well documented and provide the iproute2 patch as well.
> 
> I'll leave the decision up to Dave.

Like i said: I think its extraneous stuff that is unneeded(what is in
there at the moment is sufficient detail) - but because theres no
inconsistency, i will not squirm in pain if it is included. I am
agreeing to disagree essentially ;->

cheers,
jamal

Re: [1/7] [IPSEC] Add complete xfrm event notification

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

Herbert Xu wrote:
> @@ -1254,6 +1326,7 @@ static int pfkey_add(struct sock *sk, st
>  	if (IS_ERR(x))
>  		return PTR_ERR(x);
>  
> +	xfrm_state_hold(x);

This introduces a leak when xfrm_state_add()/xfrm_state_update()
fail. We hold two references (one from xfrm_state_alloc(), one
from xfrm_state_hold()), but only drop one. We need to take the
reference because the reference from xfrm_state_alloc() can
be dropped by __xfrm_state_delete(), so the fix is to drop both
references on error. Same problem in xfrm_user.c.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1647 bytes --]

[XFRM]: Fix xfrm_state leaks in error path

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit a4222e4b4f4fe6a28204e7960972ef833ac0c4ce
tree c24f26cfe03081d10a3a3f66d5d3e503395090b4
parent 16efae13731912e8cd028a85257fb33726318770
author Patrick McHardy <kaber@trash.net> 1115477180 +0200
committer Patrick McHardy <kaber@trash.net> 1115477180 +0200

Index: net/key/af_key.c
===================================================================
--- 6c0df7e8f613031668cf54aec5735e8b9f76aaa9/net/key/af_key.c  (mode:100644 sha1:577f0bb5bb31816bb1ecf94848ae2758d9c2cbcf)
+++ c24f26cfe03081d10a3a3f66d5d3e503395090b4/net/key/af_key.c  (mode:100644 sha1:98b72f2024ffd84564530e5973861b908fd8f541)
@@ -1333,7 +1333,7 @@
 	if (err < 0) {
 		x->km.state = XFRM_STATE_DEAD;
 		xfrm_state_put(x);
-		return err;
+		goto out;
 	}
 
 	if (hdr->sadb_msg_type == SADB_ADD)
@@ -1343,8 +1343,8 @@
 	c.seq = hdr->sadb_msg_seq;
 	c.pid = hdr->sadb_msg_pid;
 	km_state_notify(x, &c);
+out:
 	xfrm_state_put(x);
-
 	return err;
 }
 
Index: net/xfrm/xfrm_user.c
===================================================================
--- 6c0df7e8f613031668cf54aec5735e8b9f76aaa9/net/xfrm/xfrm_user.c  (mode:100644 sha1:6c8c6d6924939fe30264caab9f6fca943cf70e3b)
+++ c24f26cfe03081d10a3a3f66d5d3e503395090b4/net/xfrm/xfrm_user.c  (mode:100644 sha1:4f37b4f2ea8a238b8ae5f97496b727df7489d5fb)
@@ -287,7 +287,7 @@
 	if (err < 0) {
 		x->km.state = XFRM_STATE_DEAD;
 		xfrm_state_put(x);
-		return err;
+		goto out;
 	}
 
 	c.seq = nlh->nlmsg_seq;
@@ -295,8 +295,8 @@
 	c.event = nlh->nlmsg_type;
 
 	km_state_notify(x, &c);
+out:
 	xfrm_state_put(x);
-
 	return err;
 }
 

Re: [1/7] [IPSEC] Add complete xfrm event notification

[Herbert Xu]

On Sat, May 07, 2005 at 04:51:33PM +0200, Patrick McHardy wrote:
> 
> This introduces a leak when xfrm_state_add()/xfrm_state_update()
> fail. We hold two references (one from xfrm_state_alloc(), one
> from xfrm_state_hold()), but only drop one. We need to take the
> reference because the reference from xfrm_state_alloc() can
> be dropped by __xfrm_state_delete(), so the fix is to drop both
> references on error. Same problem in xfrm_user.c.

Patch applied.  Thanks for plugging the leaks :)
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt