Support optional socket options

[meox]

Add the possibility to create a server passing extra options like netns and vrf.

Example:

%% vim: ft=erlang
[{eradius, [
  {tables, [dictionary]},
  {session_nodes, local},
  {radius_callback, fake_radius},
  {root, [
      {{"root", []}, [{"10.11.11.1", "testing123"}]}
  ]},
  {servers, [
      {root, {"10.11.11.2", [1812, 1813], [{netns, "/var/run/netns/myns"}]}}
  ]}
]}].

[vkatsuba]

Thanks for your contribution! Please add test cases and provide example with description in README.

[vkatsuba]

Please also run GitHub Actions manually in your fork and provide here the link to success CI.
How to:

• Open https://github.com/meox/eradius
• Select tab Actions - https://github.com/meox/eradius/actions
• Select tab CI
• Find button Run workflow
• Select in Run workflow branch master and click Run workflow

[vkatsuba]

Please squash all commits into single commit.

[meox]

If I put my test: config_extra_options after config_1 and config_2 doesn't works:

{timeout,{gen_server,call,[eradius_server_mon,reconfigure]}}

but if I put before works.

[meox]

https://github.com/meox/eradius/actions/runs/1300832775

[RoadRunnr]

Please squash all commits into single commit.

please don't do that. Structure you commits in a way that there is a single commit for each logical change. That means, one commit for the spelling (and only those), one for the code formatting changes and one commit for adding the socket options.

[RoadRunnr]

this could use a check to restrict the options to the once we want to allow. For example there is no point to allow the binary (or list), the active, ip and recbuf options.

Don't do a negative filtering, explicitly check only for allowed options.

A full validation of the content of the options is not needed.

[meox]

Done: I have introduced validate_socket_options that checks if a specific options is in a banned list

[0xAX]

Hello @meox and thanks for PR.

For now eradius UDP server uses options in a kind of mixed way. Some of them are predefined and could not be changed. Besides predefined there is recbuf that eradius takes from eradius application. In this case of intoducing per-server options - eradius server will collect options from different places.

I would suggest to introduce a generic place (and format maybe property list or map) for all socket related options in addition to predefined options.

Something like:

[{eradius, [
  {tables, [dictionary]},
  {session_nodes, local},
  {radius_callback, fake_radius},
  {root, [
      {{"root", []}, [{"10.11.11.1", "testing123"}]}
  ]},
  {servers, [
    {root, {"10.11.11.2", [1812, 1813], [{socket_opts, [{recbuf, 8192000},
                                                        {netns, "/var/run/netns/myns"}]}]}}
  ]}
]}].

So eradius server will start:

-define(DEFAULT_RADIUS_SERVER_OPTS(IP), [{{active, once}, {ip, IP}, binary]).


init({ServerName, IP, Port, Opts}) ->
  ...
  ...
  ...
  ExtraServerOptions = proplists:get_value(socket_opts, Opts, []),
  case gen_udp:open(Port, ?DEFAULT_RADIU_SERVER_OPTS(IP) ++ ExtraServerOptions) of
  ...
  ...

or something like this. This will allow to have recbuf per-server and also should simplify validation of options like @RoadRunnr suggested

[meox]

@vkatsuba : https://github.com/meox/eradius/actions/runs/1303840191

[vkatsuba]

LGTM.

[vkatsuba]

@RoadRunnr, @0xAX, please take a look again.

[0xAX]

@meox Thanks for the update. I've put some comments, but in general the changes look pretty good and I think it could be merged after clarification the situation with recbuf in eradius_server

[meox]

https://github.com/meox/eradius/actions/runs/1304288823

[0xAX]

LGTM after fixing the things related to the last comments.

[meox]

LGTM after fixing the things related to the last comments.

perfect: ping me if I have to change other stuff! and thanks for the support/suggestions.

[meox]

Added a new test to check some cases for the eradius_config:validate_server function

[meox]

any chance to merge this branch soon?

[vkatsuba]

any chance to merge this branch soon?

Yep. Merged 😄. Thanks again for your contribution!