msantos / pkt

Erlang network protocol library
http://blog.listincomprehension.com/search/label/epcap
BSD 3-Clause "New" or "Revised" License
150 stars 44 forks source link

add ?IPV6_PROTO_TCP and ?IPV4_PROTO_TCP macros instead of IPPROTO_TCP #43

Closed 0xAX closed 7 years ago

0xAX commented 7 years ago

Because without them we will have problems during using of pkt from elixir because of fail of elixir's Record.extract

msantos commented 7 years ago

@0xAX thank you for the patch!

I haven't tried doing a Record.extract yet but will the other records (for example, ipv6_hopts uses ?IPPROTO_NONE) also fail?

Minor issue: the naming is a bit weird because TCP is supposed to be independent of the IP layer. Possible fix:

-define(IPV6_PROTO_TCP, ?IPPROTO_TCP).

Thanks again!

0xAX commented 7 years ago

Hello @msantos,

I haven't tried doing a Record.extract yet but will the other records (for example, ipv6_hopts uses ?IPPROTO_NONE) also fail?

Didn't try to, but I think yes. I will revise all hrl(s) for such case and update the PR if need after the weekends.

?Minor issue: the naming is a bit weird because TCP is supposed to be independent of the IP layer. Possible fix: -define(IPV6_PROTO_TCP, ?IPPROTO_TCP).

👍

msantos commented 7 years ago

I tried running Record.extract and think I understand the problem: the other header files aren't being pulled in.

iex(4)> Record.extract(:ipv6, from: "include/pkt_ipv6.hrl")
** (ArgumentError) no record ipv6 found at include/pkt_ipv6.hrl
    (elixir) lib/record/extractor.ex:55: Record.Extractor.extract_record/2

Using the top level pkt.hrl which pulls in the other headers seems to work:

iex(4)> Record.extract(:ipv6, from: "include/pkt.hrl")     
[v: 6, class: 0, flow: 0, len: 40, next: 6, hop: 0, saddr: :undefined,
 daddr: :undefined]

iex(5)> Record.extract(:ipv4, from: "include/pkt.hrl")
[v: 4, hl: 5, tos: 0, len: 20, id: 0, df: 0, mf: 0, off: 0, ttl: 64, p: 6,
 sum: 0, saddr: {127, 0, 0, 1}, daddr: {127, 0, 0, 1}, opt: ""]

iex(6)> Record.extract(:ipv6_hopopts, from: "include/pkt.hrl")
[next: 59, len: 0, opt: ""]
0xAX commented 7 years ago

@msantos that's great! Didn't think to try with pkt.hrl. In this way I think the PR can be closed