given the relationship to parent, I would also go for 'child' as the keyword....and add the protocol too. (the only issue would be confusion in e.g.l logs between parent/child logs related to this and parent/child logs related to threads...) alan On 7 September 2017 at 09:23, Winfield, Alister <Alister.Winfield@sky.uk> wrote:

My thoughts,

I’d also include the ‘protocol’ eg (RADIUS, DHCP, Diameter (Sometime in a future world)) so it could be:

<keyword> [virtual-server].[protocol].<packet-type> { <sub-request-attr0> := <parent attr0> <sub-request-attr1> += <parent attr1> <sub-request-attrN> += <parent attrN> }

Now for what that keyword should be “child” sounds okay but that’s not perfect by any means.

Note: I think async should be a generic parallel concept to allow other types of use. For example log to db1, db2, db3 and answer if one returns ok. Or send json trigger without waiting for it to answer or…

Alister

On 07/09/2017, 07:28, "Freeradius-Users on behalf of Arran Cudbard-Bell" <freeradius-users-bounces+alister.winfield=sky.uk@lists.freeradius.org on behalf of a.cudbardb@freeradius.org> wrote:

New functionality is being added in v4.0.x to allow new sub-requests to be generated by requests received by the server.

The basic format is:

<keyword> [virtual-server.]<packet-type> { <sub-request-attr0> := <parent attr0> <sub-request-attr1> += <parent attr1> <sub-request-attrN> += <parent attrN> }

There's debate about what <keyword> should be, and whether there should be multiple <keywords> for async - where we split the lifetime of the sub-request from its parent, and synchronous behaviour - where we wait for the sub-request to return before we continue processing.

The original keyword was "fork", and has since been changed to "create". Do people have any opinions on what the easiest to infer and most consistent keyword to use here would be?

Some other ideas were "spawn", and "child".

There's no guarantee that the most popular keyword/idea will be used, but it'll at least inform us of general public opinion :)

-Arran

Information in this email including any attachments may be privileged, confidential and is intended exclusively for the addressee. The views expressed may not be official policy, but the personal views of the originator. If you have received it in error, please notify the sender by return e-mail and delete it from your system. You should not reproduce, distribute, store, retransmit, use or disclose its contents to anyone. Please note we reserve the right to monitor all e-mail communication through our internal and external networks. SKY and the SKY marks are trademarks of Sky plc and Sky International AG and are used under licence.

Sky UK Limited (Registration No. 2906991), Sky-In-Home Service Limited (Registration No. 2067075) and Sky Subscribers Services Limited (Registration No. 2340150) are direct or indirect subsidiaries of Sky plc (Registration No. 2247735). All of the companies mentioned in this paragraph are incorporated in England and Wales and share the same registered office at Grant Way, Isleworth, Middlesex TW7 5QD.

- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html

On 7 Sep 2017, at 17:25, Matthew Newton <mcn@freeradius.org> wrote:

On Thu, 2017-09-07 at 13:28 +0700, Arran Cudbard-Bell wrote:

...

New functionality is being added in v4.0.x to allow new sub-requests to be generated by requests received by the server.

The basic format is:

<keyword> [virtual-server.]<packet-type> { <sub-request-attr0> := <parent attr0> <sub-request-attr1> += <parent attr1> <sub-request-attrN> += <parent attrN> }

Best I could come up with yesterday (tweaked slightly again here) was something like:

subrequest { update { request:User-Name = &parent.request:User-Name request:User-Password = &parent:User-Password }

call [virtual-server].Access-Request

update parent { reply:Reply-Message := &Reply-Message } }

so splits off defining what lists contain what before actually forking. So you could potentially update control or reply lists before calling the virtual server as well.

The issue here is that when you're creating a new request for a different protocol, you need to dip in to a foreign dictionary for the attributes. You really need to know the protocol type before creating the subrequest, or attempting to populate it with attributes. So whilst I really do like that layout, it's fairly difficult to implement, especially given you could be calling multiple virtual servers of different types, and so you can't really infer the dictionary/protocol from a nested call statement. subrequest <protocol> { } Might work? That matches the code more closely. Mismatched request/virtual server protocols could be caught on startup. It would also need to change the protocol namespace for all its children, but that shouldn't be too too hard.

The protocol is defined by the namespace of the called virtual-server, so I don't think that needs setting here.

Agreed, specifying protocol is redundant. A virtual server can only service one protocol at a time. There's no reason you can't share policies or modules between virtual servers, but a virtual server has one namespace only.

This does give the ability for the child to update the parent afterwards, but I'm not sure how or where that would happen in practise (it likely couldn't without a wait)

Yes, that's the "sync" variant only. The parent may be long dead and cleaned up by the time you try and update it from an "async" request. I was talking about it with Alan D, and tried to come up with a use case for async requests updating the parent, but couldn't think of anything... The only way it'd work would be to have some sort of wait {} section in the virtual server which only ran when all async requests had completed.

so I guess without that then your syntax is more concise. Not sure if it's as clear - I quite like the separation between creating the sub-request and setting up its attribute lists, and then calling the virtual server, as two distinct things.

Me too.

...

There's debate about what <keyword> should be, and whether there should be multiple <keywords> for async - where we split the lifetime of the sub-request from its parent, and synchronous behaviour - where we wait for the sub-request to return before we continue processing.

Thought on waiting was that you don't, unless the whole lot is in another block, say "parallel" or "concurrent" which enforces a wait for the children at the end, e.g.

parallel { subrequest { update { ... } call default.Access-Request } subrequest { update { ... } call default.Access-Request

} group { some normal module calls here

} } # implicit wait here

other modules

would run two subrequests and a load of module calls in the original request simultaneously, and wait for them all to complete before calling "other" and "modules".

I think you'd end up running 5 sub-requests with the current code, as the parallel section creates sub-requests implicitly for each of its direct children. You could check to see if the child was a subrequest, and just use that syntax to control the protocol.

"group" is probably redundant, as you couldn't have two groups (one would have to be a subrequest, or they would get called sequentially anyway)

Implicitly creating a subrequest of the parent type would fix that.

...

The original keyword was "fork", and has since been changed to "create". Do people have any opinions on what the easiest to infer and most consistent keyword to use here would be?

Only really that fork only makes sense to programmers, and that's not helpful in making the server easier for most people.

Yeah a fork is what you stab people in the eye with, not what you use to create child requests in a RADIUS server.

I think a lot of the syntax comes down to what happens when the child request *finishes*. Where does its reply go? If something needs to happen then the syntax needs to allow for doing that (even if it might in future).

You can update the parent from within the child, otherwise the reply list is discarded. I think that's the only sane way to do it. One of the fundamentals for the v4.0.x design was no mixing protocols in requests. So when you have a reply coming back from a different subrequest protocol to its parent, you can't implicitly merge the two. I do really like the idea of the subrequest keyword though. It makes policies more useful too, as for really complex ones, you can run the whole thing in a subrequest, and just pull/pull input/output attributes to/from the parent. Any attributes you used just to track state within the policy get automatically deleted when the server returns from the subrequest section. It also reduces administrator cognitive load as all the logic for a service can be contained within one virtual server. call is essentially inter-virtual-server proxying, but it's not required to send packets of a foreign protocol if you have subrequests. e.g. recv Access-Request { subrequest dhcp { update request { Request-Packet-Type := Discover } dhcp switch &Response-Packet-Type { case Offer { update parent { Framed-IP-Address := &DHCP-Your-IP-Address } } } } } -Arran

On Sep 7, 2017, at 9:36 AM, Winfield, Alister <Alister.Winfield@sky.uk> wrote:

recv Access-Request { subrequest dhcp {

Hmm... putting the protocol there is a reasonable choice. But... the issue is we also need to run the DHCP state machine: recv foo { } send foo { } etc. The idea is that *all* DHCP packets undergo the same processing. Which avoids the weird issues we have in v3 with inner-tunnel sometimes not being the same as the default server.

update request { Request-Packet-Type := Discover } dhcp

That's better done via Matthew's suggestion: call server.Packet

Nicely represents the intent of the author of the configuration. I assume that the update there could add values from the parent request into related DHCP attributes.

Yes.

It is also obvious that straight RADIUS proxy is trivial to write in that form without any special magic. Something like this would be really obvious to even a first time reader of a freeRADIUS configuration.

Yes. Alan DeKok.

On Sep 7, 2017, at 6:25 AM, Matthew Newton <mcn@freeradius.org> wrote:

Best I could come up with yesterday (tweaked slightly again here) was something like:

I think splitting the functionality into pieces is best. That way it's more under user control. It requires care and attention, but it's *clear* at least.

subrequest { update { request:User-Name = &parent.request:User-Name request:User-Password = &parent:User-Password }

And if you want to run it asynchronously: detach which would explicitly detach it from the parent.

call [virtual-server].Access-Request

Yeah... tho minor tweaks to the syntax may help, in order to fit it into the horrible parser. :(

update parent { reply:Reply-Message := &Reply-Message } }

so splits off defining what lists contain what before actually forking. So you could potentially update control or reply lists before calling the virtual server as well.

That makes sense.

The protocol is defined by the namespace of the called virtual-server, so I don't think that needs setting here.

Yes.

This does give the ability for the child to update the parent afterwards, but I'm not sure how or where that would happen in practise (it likely couldn't without a wait)

Yes. The unlang compiler could probably be updated to notice that, and complain about it. So then the server doesn't start, but the admin at least gets a good error message as to *why* it doesn't start.

so I guess without that then your syntax is more concise. Not sure if it's as clear - I quite like the separation between creating the sub-request and setting up its attribute lists, and then calling the virtual server, as two distinct things.

I agree.

...

There's debate about what <keyword> should be, and whether there should be multiple <keywords> for async - where we split the lifetime of the sub-request from its parent, and synchronous behaviour - where we wait for the sub-request to return before we continue processing.

Thought on waiting was that you don't, unless the whole lot is in another block, say "parallel" or "concurrent" which enforces a wait for the children at the end, e.g.

For various reasons, the "parallel" section already creates sub-requests...

I think a lot of the syntax comes down to what happens when the child request *finishes*. Where does its reply go? If something needs to happen then the syntax needs to allow for doing that (even if it might in future).

update { parent.foo = bar } That's the only answer which makes any kind of sense, unfortunately. Alan DeKok.

On Sep 7, 2017, at 10:36 AM, Matthew Newton <mcn@freeradius.org> wrote:

So doing a subrequest {} block (or whatever it ends up being called) is essentially just a block which has its own attribute lists, so a space for local variables that won't mess up the main request. It runs in sequence with everything else, and at the end processing continues at the next instruction in the calling block.

Yes.

But as soon as you call 'detach', you've forked. You can never return from this request to the parent, and the parent immediately starts to continue at the next instruction after the sub request.

Yes.

The only issue I can think of, are there ever any cases where you want to detach (maybe once, maybe more times), but then later on update something in the parent?

Nope. Once it's detached, the parent can *go away*. So there's no way to update the parent, as it doesn't exist.

If so, maybe you also need "wait" as well as detach. That sits in the parent, and waits for all subrequests to finish. At which point they could have updated the parent.

That's what "parallel" is for.

update parent { request:foo = bar }

Would also be nice, or "update parent.request".

The "update parent.request" syntax is already supported. Alan DeKok.