Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use 'getprotobyname' #4

Open
superbobry opened this issue Mar 7, 2012 · 4 comments
Open

Use 'getprotobyname' #4

superbobry opened this issue Mar 7, 2012 · 4 comments

Comments

@superbobry
Copy link

Hello, I think procket should call getprotobyname instead of using hardcoded protocol numbers, since the actual values are platform dependant and can be overriden in /etc/protocols/.

@msantos
Copy link
Owner

msantos commented Mar 7, 2012

On Wed, Mar 07, 2012 at 12:33:30AM -0800, Sergei Lebedev wrote:

Hello, I think procket should call getprotobyname instead of using hardcoded protocol numbers, since the actual values are platform dependant and can be overriden in /etc/protocols/.

Hey Sergei!

Thanks for the suggestion!

Protocol numbers are set by IANA, so they should be consistent between
platforms. I checked the protocols file on an Ubuntu system and it's
just a subset. There's a comment in the file to use the nmap protocols
file if you need a full list.

Offhand, I can't think of a reason for changing /etc/protocols but that
doesn't mean there isn't a good one.

As for getprotobyname(3), I guess the options are:

  1. Hard coding a subset like today (maybe the same defaults as on
    ubuntu). The user can still pass in an integer for unknown protocols or
    if they've changed the protocol number locally.
  2. Generating a hard coded map from the nmap protocols file
  3. Looking up the protocol at runtime from C (getprotobyname(3))
  4. Looking up the prococol at runtime from Erlang. Something like the following:

-module(pr).
-export([l/1]).

l(Proto) when is_atom(Proto) ->
l(atom_to_binary(Proto, latin1));
l(Proto) when is_binary(Proto) ->
{ok, Bin} = file:read_file("/etc/protocols"),
find(Proto, binary:split(Bin, <<"\n">>, [global, trim])).

find(_Proto, []) ->
undefined;
find(Proto, [H|T]) ->
Size = byte_size(Proto),
case H of
<<Proto:Size/bytes, "\t", Rest/binary>> ->
btoi(hd(binary:split(Rest, <<"\t">>)));
_ ->
find(Proto, T)
end.

btoi(Bin) ->
list_to_integer(binary_to_list(Bin)).

What do you think?

@superbobry
Copy link
Author

Michael,

I wasn't aware that protocol numbers are fixed -- in that case, maybe hardcoding them isn't a bad idea; but overall, I think delegating this part to 'libc' might still be a better solution. Especially since you already have a procket nif, so adding one more function to it is a noproblem :)

@msantos
Copy link
Owner

msantos commented Mar 7, 2012

On Wed, Mar 07, 2012 at 09:32:23AM -0800, Sergei Lebedev wrote:

Michael,

I wasn't aware that protocol numbers are fixed -- in that case, maybe hardcoding them isn't a bad idea; but overall, I think delegating this part to 'libc' might still be a better solution. Especially since you already have a procket nif, so adding one more function to it is a noproblem :)

Definitely, should be simple to add. The general idea behind
procket is that the C NIF should just be a thin wrapper around Unix
syscalls. Everything else, including preparing the args for the system
calls, is done in Erlang. The reason for doing this is for reliability:
keeps the C code simpler and moves the work from risky native code to
safe code running on the VM.

I haven't looked at the code for getprotobyame(3) yet but the
implementation looks simple (the file search that is, I guess it can
also look up in other directories like NIS and LDAP). So my preference at
this point, if getprotobyname is added, would be to do the work in Erlang.

Using getprotobyname(3) might actually be risky. If someone did have
protocols configured using a directory service, the call might block
which in turn would cause the Erlang VM to hang.

@superbobry
Copy link
Author

This sounds totally reasonable. I've looked through 'getprotobyname' code and it does nothing special -- so implementing it in Erlang would be just as easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants