2021-07-24 01:02:11

by Corey Minyard

[permalink] [raw]
Subject: IPSec questions and comments

<[email protected]>, "David S. Miller" <[email protected]>
Cc: [email protected], [email protected]
Bcc:
Subject: IPSec questions
Reply-To: [email protected]

I've been going through the XFRM code trying to understand it. I've
been documenting things in the code as I go.

I have a specific usage question, then a general question:

1) In struct xfrm_dst, what is the difference between the route and path
fields? From what I can tell, in the first element of a bundle they
will both point the route the packet will take after it has been
transformed. In the other elements of a bundle, route is the same as in
the first element and path will be NULL. Is this really the intent?
Can path just be eliminated?

2) This code is really hard to understand. Nobody should have to go
through what I'm going through. If I can convince my employer to allow
me to submit the comments I'm adding, would that be something acceptable?
It would obviously take a lot of time to review. If nobody's going to
have the time to review it, I don't need to put forth the extra effort
to make it submittable.

Thanks,

-corey


2021-07-24 02:43:15

by Eyal Birger

[permalink] [raw]
Subject: Re: IPSec questions and comments

Ho Corey,

On Sat, Jul 24, 2021 at 4:02 AM Corey Minyard <[email protected]> wrote:
> 1) In struct xfrm_dst, what is the difference between the route and path
> fields? From what I can tell, in the first element of a bundle they
> will both point the route the packet will take after it has been
> transformed. In the other elements of a bundle, route is the same as in
> the first element and path will be NULL. Is this really the intent?
> Can path just be eliminated?

For non-transport modes - such as tunnel - 'route' and 'path' won't be the
same in the first element (xdst0): 'route' will be the original dst and
'path' will be the route the transformed packet will take. the dst is
overridden in the xfrm_dst_lookup() call within xfrm_bundle_create(), after
xdst->route had been set.

AFAICT, the intent for the 'path' member is described in commit
0f6c480f23f4 ("xfrm: Move dst->path into struct xfrm_dst") - essentially
'path' contains the reference to the underlay route from the topmost bundle
member avoiding a walk through the child chain when needed.

Hope this helps.
Eyal.

2021-07-26 08:16:01

by Steffen Klassert

[permalink] [raw]
Subject: Re: IPSec questions and comments

On Fri, Jul 23, 2021 at 08:01:17PM -0500, Corey Minyard wrote:
> <[email protected]>, "David S. Miller" <[email protected]>
> Cc: [email protected], [email protected]
> Bcc:
> Subject: IPSec questions
> Reply-To: [email protected]
>
> I've been going through the XFRM code trying to understand it. I've
> been documenting things in the code as I go.
>
> I have a specific usage question, then a general question:
>
> 1) In struct xfrm_dst, what is the difference between the route and path
> fields? From what I can tell, in the first element of a bundle they
> will both point the route the packet will take after it has been
> transformed. In the other elements of a bundle, route is the same as in
> the first element and path will be NULL. Is this really the intent?
> Can path just be eliminated?

Eyal gave a good explanation of this.

>
> 2) This code is really hard to understand. Nobody should have to go
> through what I'm going through. If I can convince my employer to allow
> me to submit the comments I'm adding, would that be something acceptable?
> It would obviously take a lot of time to review. If nobody's going to
> have the time to review it, I don't need to put forth the extra effort
> to make it submittable.

Documentation is always welcome. If you submit your documentation
in small reviewable patches, then it should be accepted and merged
over time.