msantos / procket

Erlang interface to low level socket operations
http://blog.listincomprehension.com/search/label/procket
BSD 3-Clause "New" or "Revised" License
283 stars 80 forks source link

Use 'getprotobyname' #4

Open superbobry opened 12 years ago

superbobry commented 12 years ago

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 commented 12 years ago

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 = bytesize(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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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.