Re: [lwip-devel] Packet processing stack hooks

[Bogad, Katharina]

On 2025-01-30 11:32 James G. Smith wrote:
> If you really "need" to see the word INPUT in the hook name then 
> maybe LWIP_HOOK_IP4_PREROUTING_INPUT :-)

Maybe there's a misunderstanding here: my point was that I need another hook 
*purely* for the input path, that does not receive packets that will end up 
being routed and are not destined for the local system. The problem arises of 
the need for a name for this hook: whatever name I end up with needs to convey 
(a) what it is for (packets destined to be processed by the local host), (b) 
needs to be sufficiently different from LWIP_HOOK_IP4_INPUT, so that those 
can't be confused and (c) needs to be somewhat semantically clear.

I am having a very hard time coming up with a name that fits all three criteria 
-- and my current solution is to rename the current LWIP_HOOK_IP4_INPUT, so I 
can use the name for a hook in a (in my opinion) more appropriate place.

If you have a suggestion on how to name the hook that I described in my 
original email just before the packet gets passed to the next layer, feel free 
to drop it! The best I came up with is LWIP_HOOK_IP4_INPUT_POSTROUTING, but I 
don't like that for the same reason I, in hindsight, don't like 
LWIP_HOOK_IP4_PREROUTING: the names only make sense if IP_FORWARD is on. 
Another option I dismissed was LWIP_HOOK_IP4_HOST_INPUT, because I feel that 
the difference between LWIP_HOOK_IP4_INPUT and LWIP_HOOK_IP4_HOST_INPUT isn't 
really obvious from the name.

I obviously can relax the semantically clear name requirement and just document 
the packet flow and at what point the packet may be put through a hook. 
However, at the very least, I want to avoid creating a gotcha. I could see 
that, with sufficient documentation, LWIP_HOOK_IP4_HOST_INPUT may be an option: 
It still describes roughly what it does and (hopefully) triggers a 
documentation read for the difference in semantics. There's still the issue 
that LWIP_HOOK_IP4_INPUT gets packets I would not have expected from the name, 
but since this is currently already the case, I can live with that and write 
more documentation instead.

> Your new code (as well as being configurable by a lwipopts.h manifest) 
> could easily use a new HOOK name, and if you need it can be documented
> for the new hooks that LWIP_HOOK_IP4_INPUT is a misnomer re.
>  firewall functionality in that it is a unprocessed packet hook and is not 
> needed/enabled for your firewall support.

I'd go a step further and argue that the semantics of the hooks need to be 
documented more clearly in lwipopts.h anyways; this is not only an issue with 
LWIP_HOOK_IP4_INPUT, but at least the corresponding IPv6 hook as well. I'll 
include this into my patchset.

Regards,
Katharina

---
Katharina Bogad
Secure Operating Systems

Fraunhofer Institute AISEC
Lichtenbergstr. 11, 85748 Garching near Munich, Germany
tel:+49-89-3229986-1020
mailto:katharina.bogad@aisec.fraunhofer.de
https://www.aisec.fraunhofer.de

-----Ursprüngliche Nachricht-----
Von: lwip-devel-bounces+katharina.bogad=aisec.fraunhofer.de@nongnu.org 
<lwip-devel-bounces+katharina.bogad=aisec.fraunhofer.de@nongnu.org> Im Auftrag 
von James G. Smith via lwip-devel
Gesendet: Donnerstag, 30. Januar 2025 11:32
An: lwip-devel@nongnu.org
Cc: James G. Smith <jsmith@ecoscentric.com>
Betreff: Re: [lwip-devel] Packet processing stack hooks

On 30/01/2025 09:28, Bogad, Katharina wrote:
> ...
>> Question: why not leave LWIP_HOOK_IP4_INPUT as it is and just add 
>> LWIP_HOOK_IP4_PREROUTING and your other hooks?
>> Just issue a warning saying not to use it if you use firewall code.
>> I think it's a good idea to leave existing code functioning as before.
> 
> My most glaring problem with the current placement of the hook is that 
> is simply in the wrong place in the processing stack. Compared to 
> other IP stacks, input always means "packets destined for local 
> processing only". The current placement leads to a situation where the 
> input hook receives more packets than I would expect. Maybe I am alone 
> in this feeling; and I don't know the development history here, but I 
> if I had to guess I'd suspect that the hook was there before 
> forwarding was a thing.

This is purely a naming issue, and in my opinion (like Eric) for backwards 
compatibility it should remain as-is.

You may believe, With hindsight, that the name choice may not be the best; but 
why create diffs and merge heartache when we are not talking about a bug-fix or 
functionality change. It exists just now at the start of the ip4_input() path 
and can continue to live there.

Your new code (as well as being configurable by a lwipopts.h manifest) could 
easily use a new HOOK name, and if you need it can be documented for the new 
hooks that LWIP_HOOK_IP4_INPUT is a misnomer re. firewall functionality in that 
it is a unprocessed packet hook and is not needed/enabled for your firewall 
support.

If you really "need" to see the word INPUT in the hook name then maybe 
LWIP_HOOK_IP4_PREROUTING_INPUT :-)

There is no **need** to move/rename LWIP_HOOK_IP4_INPUT.

In your firewall configured world the existing hook would just not be 
enabled/used.

Just my tuppence worth.
Cheers,
-- Jamie