search

Igalia / pflua

Packet filtering in Lua
Other
313 stars 39 forks source link

More thoughts on avoiding offset littering #230

Closed kbara closed 9 years ago

kbara commented 9 years ago

If we're going to avoid littering code with offsets, can you think of a good way to abstract the offsets for the following?:

Also, does pflang do the right thing in the presence of IPv6 extension headers? I know there has been discussion on the tcpdump-workers mailing list about adding a limited loop primitive or other similar features to make handling IPv6 headers better; is the current state of things good enough for what we're trying to do?

(Discussion forked from https://github.com/Igalia/pflua/issues/229 ).

wingo commented 9 years ago

Hard to answer this as it's not really phrased in a way that orients itself towards resolution -- it's more of an invitation to think ;)

However I would propose that "offsets" are not an issue -- values are. If there is no way to name an IPv6 hop limit, then we can't compute the offset of it -- but the problem is rooted in the value level and not the address level. So a good bug would be e.g. "Pflang extension: ip6:hop-limit" where ip6:hop-limit or whatever the proposal is is an arithmetic expression. &ip6:hop-limit falls out naturally. Similarly for TTL. Looking forward to concrete proposals :) Only caution would be, let's stay on target and only propose what we need.

kbara commented 9 years ago

Well, the way pflang does this so far seems to just be symbolic names (without a : or qualification), like in the example tcp[tcpflags] & (tcp-syn|tcp-fin). I'd be tempted to bikeshed ip6:hop-limit to ip6_hoplimit and just add symbolic names to this bug as-needed, rather than having a bug for each. Thoughts?

I'm restricting it to things we definitely need to look at; the fields I mentioned are important in the RFCs/drafts we've been reading.

wingo commented 9 years ago

Symbolic offsets are fine to me; I was thinking that we'd have to do conditionals or something if the offset isn't always in the same place, but if that can be avoided, cool. I guess even tcp[tcpflags] isn't a constant expression given differing IPv4 header sizes; cool, whatevs. Underscores are not pflangic, NAK on that :)

Regarding issue count and scope, I think it's most helpful to have focused issues that we can act on and know when to close. This one isn't one of those, for me at least. Smaller more numerous task-focused bugs are fine and even good i think -- we can make single commits that fix them.

kbara commented 9 years ago

I've looked into extension header behaviour; details at https://github.com/Igalia/pflua/issues/229#issuecomment-117442366 . (Short version: yes, it's a problem).