pf: Force logging if pf_create_state() fails

[markj]

Summary

Currently packets are logged before pf_create_state() is called, so we
might log a packet as passed that is subsequently dropped due to state
creation failure. In particular, the drop is not logged, which is
wrong.

Improve the situation a bit: force logging if state creation fails.
This isn't totally right as we'll end up logging the packet twice in
this case, but it's better than not logging the drop at all.

Add a regression test.

Co-authored by: Franco Fichtner <[e-mail]>

[kp]

Does this need an MFC tag too?

tests/sys/netpfil/pf/pflog.sh

139

That description doesn't seem to really describe the point of this test.

151

Why the alias?

[vegeta_tuxpowered.net]

Improve the situation a bit: force logging if state creation fails.
This isn't totally right as we'll end up logging the packet twice in
this case, but it's better than not logging the drop at all.

This seems more like a workaround than a fix of underlying design issues. Since I'm responsible for the mess of pf_rule_actions and match rules, I might be able to propose an alternative solution. Can you wait a few days?

[markj]

In D47953#1094057, @vegeta_tuxpowered.net wrote:

Improve the situation a bit: force logging if state creation fails.
This isn't totally right as we'll end up logging the packet twice in
this case, but it's better than not logging the drop at all.

This seems more like a workaround than a fix of underlying design issues.

This is certainly true.

Since I'm responsible for the mess of pf_rule_actions and match rules, I might be able to propose an alternative solution. Can you wait a few days?

Sure, thank you, I'm not in any particular rush. I will note that the problem appears to exist in OpenBSD as well, by code inspection.

[markj]

• Add a useful test description.
• Remove an unneeded interface alias.

[markj]

In D47953#1094057, @vegeta_tuxpowered.net wrote:

Improve the situation a bit: force logging if state creation fails.
This isn't totally right as we'll end up logging the packet twice in
this case, but it's better than not logging the drop at all.

This seems more like a workaround than a fix of underlying design issues. Since I'm responsible for the mess of pf_rule_actions and match rules, I might be able to propose an alternative solution. Can you wait a few days?

Just a gentle poke: do you have a specific change in mind?

[markj]

In D47953#1098215, @markj wrote:

In D47953#1094057, @vegeta_tuxpowered.net wrote:

Improve the situation a bit: force logging if state creation fails.
This isn't totally right as we'll end up logging the packet twice in
this case, but it's better than not logging the drop at all.

This seems more like a workaround than a fix of underlying design issues. Since I'm responsible for the mess of pf_rule_actions and match rules, I might be able to propose an alternative solution. Can you wait a few days?

Just a gentle poke: do you have a specific change in mind?

@vegeta_tuxpowered.net I'm sorry to nag, but if there isn't an alternative solution available in the short term, can we please go ahead with this one for now? This change is small and can easily be reverted later if need be.