RFC: XFRM changing the view of xfrm_aevent_id

RFC: XFRM changing the view of xfrm_aevent_id

[jamal]

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

Re: RFC: XFRM changing the view of xfrm_aevent_id

[KOVACS Krisztian]

  Hi,

On Friday 01 December 2006 15:37, jamal wrote:
> 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

Re: RFC: XFRM changing the view of xfrm_aevent_id

[David Miller]

From: jamal <hadi@cyberus.ca>
Date: Fri, 01 Dec 2006 09:37:52 -0500

> 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?

Let's change it now while we still can.

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

Re: RFC: XFRM changing the view of xfrm_aevent_id

[jamal]

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

On Fri, 2006-01-12 at 20:08 -0800, David Miller wrote:

> 
> 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


[-- Attachment #2: aevent-break --]
[-- Type: text/plain, Size: 2560 bytes --]

[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).

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

---
commit 4bacaeb2cea3c6684f9b3acc54f71958bb2c25c5
tree c5f416a504abdb7583b799fec5245495c462bc92
parent 5465ae68b5ec11b2820db3f9b4c6fd94f113da44
author Jamal Hadi Salim <hadi@cyberus.ca> Sat, 02 Dec 2006 06:05:42 -0500
committer Jamal Hadi Salim <hadi@cyberus.ca> 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(-)

diff --git a/Documentation/networking/xfrm_sync.txt b/Documentation/networking/xfrm_sync.txt
index 8be626f..fb88f1d 100644
--- a/Documentation/networking/xfrm_sync.txt
+++ b/Documentation/networking/xfrm_sync.txt
@@ -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:
diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index 7907c42..088ba81 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -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 {
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 055b72f..6f97665 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -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);

Re: RFC: XFRM changing the view of xfrm_aevent_id

[David Miller]

From: jamal <hadi@cyberus.ca>
Date: Sat, 02 Dec 2006 06:09:08 -0500

> On Fri, 2006-01-12 at 20:08 -0800, David Miller wrote:
> 
> > 
> > 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.