Skip to contentSkip to conversation

RFC: XFRM changing the view of xfrm_aevent_id

DISCUSSION jamal 3 participants 5 messages Dec 1, 2006 – Dec 3, 2006

Subsystems (4)
lore
Share & tools
lore

jamal

author
#0 lore raw

while trying to patch iproute2 to display the xfrm aevents i came to the conclusion that the xfrm_aevent_id struct didnt provide enough info for me to uniquely identify an SA.
I need two other items: the reqid and saddr.

The structure currently looks like

struct xfrm_aevent_id {
        struct xfrm_usersa_id           sa_id;
        __u32                           flags;
};

I could add the two items mentioned above in it and break the ABI. This may sound dangerous, but the usage of this ABI is not widespread. AFAIK, the only other person who might have used this is Kristzian (on CC).

If this is considered evil, then i will transfer the two items in a new TLV but it wont be as pretty.

Thoughts?

cheers,
jamal

#1 lore raw

Hi,

Quoted 9 lines · jamal
struct xfrm_aevent_id {
        struct xfrm_usersa_id           sa_id;
        __u32                           flags;
};

I could add the two items mentioned above in it and break the ABI. This may sound dangerous, but the usage of this ABI is not widespread. AFAIK, the only other person who might have used this is Kristzian (on CC).

I do not use the XFRM netlink interface at the moment, so breaking the
ABI is absolutely not a problem for me.

--
Regards,
Krisztian Kovacs

maintainer
#2 lore raw
In reply to jamal

From: jamal <[email protected]>
Date: Fri, 01 Dec 2006 09:37:52 -0500

Quoted 12 lines

The structure currently looks like

struct xfrm_aevent_id {
        struct xfrm_usersa_id           sa_id;
        __u32                           flags;
};

I could add the two items mentioned above in it and break the ABI. This may sound dangerous, but the usage of this ABI is not widespread. AFAIK, the only other person who might have used this is Kristzian (on CC).

Quoted 4 lines

If this is considered evil, then i will transfer the two items in a new TLV but it wont be as pretty.

Thoughts?

Let's change it now while we still can.

A few months from now, I will probably not give the same answer :)

author
#3 lore raw
Quoted 5 lines · David Miller

Let's change it now while we still can.

A few months from now, I will probably not give the same answer :)

:-> Ok, here it is ...

cheers,
jamal

[XFRM] fix aevent structuring to be more complete

aevents can not uniquely identify an SA. We break the ABI with this patch, but consensus is that since it is not yet utilized by any (known) application then it is fine (better do it now than later).

---
commit 4bacaeb2cea3c6684f9b3acc54f71958bb2c25c5
tree c5f416a504abdb7583b799fec5245495c462bc92
parent 5465ae68b5ec11b2820db3f9b4c6fd94f113da44
author Jamal Hadi Salim <[email protected]> Sat, 02 Dec 2006 06:05:42 -0500 committer Jamal Hadi Salim <[email protected]> Sat, 02 Dec 2006 06:05:42 -0500

 Documentation/networking/xfrm_sync.txt |    5 ++++-
 include/linux/xfrm.h                   |    2 ++
 net/xfrm/xfrm_user.c                   |    4 +++-
 3 files changed, 9 insertions(+), 2 deletions(-)
Documentation/networking/xfrm_sync.txt +4 -1
index 8be626f..fb88f1d 100644
@@ -47,10 +47,13 @@ aevent_id structure looks like:
 
    struct xfrm_aevent_id {
              struct xfrm_usersa_id           sa_id;
+             xfrm_address_t                  saddr;
              __u32                           flags;
+             __u32                           reqid;
    };
 
-xfrm_usersa_id in this message layout identifies the SA.
+The unique SA is identified by the combination of xfrm_usersa_id, 
+reqid and saddr.
 
 flags are used to indicate different things. The possible
 flags are:
include/linux/xfrm.h +2 -0
index 7907c42..088ba81 100644
@@ -289,7 +289,9 @@ struct xfrm_usersa_id {
 
 struct xfrm_aevent_id {
 	struct xfrm_usersa_id		sa_id;
+	xfrm_address_t			saddr;
 	__u32				flags;
+	__u32				reqid;
 };
 
 struct xfrm_userspi_info {
net/xfrm/xfrm_user.c +3 -1
index 055b72f..6f97665 100644
@@ -1281,10 +1281,12 @@ static int build_aevent(struct sk_buff *skb, struct xfrm_state *x, struct km_eve
 	id = NLMSG_DATA(nlh);
 	nlh->nlmsg_flags = 0;
 
-	id->sa_id.daddr = x->id.daddr;
+	memcpy(&id->sa_id.daddr, &x->id.daddr,sizeof(x->id.daddr));
 	id->sa_id.spi = x->id.spi;
 	id->sa_id.family = x->props.family;
 	id->sa_id.proto = x->id.proto;
+	memcpy(&id->saddr, &x->props.saddr,sizeof(x->props.saddr));
+	id->reqid = x->props.reqid;
 	id->flags = c->data.aevent;
 
 	RTA_PUT(skb, XFRMA_REPLAY_VAL, sizeof(x->replay), &x->replay);
maintainer
#4 lore raw

From: jamal <[email protected]>
Date: Sat, 02 Dec 2006 06:09:08 -0500

Quoted 9 lines · David Miller
Quoted 6 lines

Let's change it now while we still can.

A few months from now, I will probably not give the same answer :)

:-> Ok, here it is ...

Applied thanks.