Poll of opinions for new keyword

[Arran Cudbard-Bell]

> On 7 Sep 2017, at 17:25, Matthew Newton <mcn at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.freeradius.org/pipermail/freeradius-users/attachments/20170907/29acfecd/attachment-0001.sig>