pf: fix pfi_ifnet leak on interface removal

[kp]

Summary

The detach of the interface and group were leaving pfi_ifnet memory
behind. Check if the kif still has references, and clean it up if it
doesn't

On interface detach, the group deletion was notified first and then a
change notification was sent. This would recreate the group in the kif
layer. Reorder the change to before the delete.

PR: 257218
MFC after: 2 weeks

[zlei]

Some design concerns:
For interface groups, typically there're four kinds of event:

1. new group added ( with a joining member )
2. a new member joins
3. a existing member leaves
4. a group deleted ( also the last member leaves )

The current implementation triggers a group_attach_event for 1, and a group_detach_event for 4, and also triggers a group_change_event for all kinds of event.

In principal both group_attach_event and group_detach_event imply that group member changed, then it seems there's no need to trigger a redundant group_change_event.

For the current implementation I think it may ease consumers of group_change_event. But for consumers that are also aware of group_attach_event and group_detach_event it may be POLA.

So can we refine group_attach_event and group_detach_event so that they no longger implies member joining or leaving ?

Currently in base system pf is the only consumer of those events.

[zlei]

sys/net/if.c

1548

This is definitely the correct order ( of if group events ) !

[kp]

In D37569#854380, @zlei wrote:

Some design concerns:
For interface groups, typically there're four kinds of event:

1. new group added ( with a joining member )
2. a new member joins
3. a existing member leaves
4. a group deleted ( also the last member leaves )

The current implementation triggers a group_attach_event for 1, and a group_detach_event for 4, and also triggers a group_change_event for all kinds of event.

In principal both group_attach_event and group_detach_event imply that group member changed, then it seems there's no need to trigger a redundant group_change_event.

For the current implementation I think it may ease consumers of group_change_event. But for consumers that are also aware of group_attach_event and group_detach_event it may be POLA.

So can we refine group_attach_event and group_detach_event so that they no longger implies member joining or leaving ?

Your analysis is, I believe largely correct, but I think you're missing one point. It's not possible to have a group without members. Essentially, interfaces keep a list of groups they're a member of. So every group_attach_event always implies that at least one interface is a member of that group. That means there should be a group_change_event for that group at the same time. (Arguably we could define group_attach_event as implying a group_change_event, but that seems like a bigger POLA than fixing the incorrect order of events on group_detach_event).