Please note that I've decided to not change the protocol version. Making the code work with messages from older peer would not be as easy as it is with subheaders.

Please don't treat this version of structure as final yet, I am not 100% sure how will it play with D38025.

State deletions are sent out using pfsync_out_del which sends out struct pfsync_del_c containing only creator id and state id. The actions are queued in PFSYNC_Q_DEL. The receiving functions for those is pfsync_in_del_c.

It seems that pfsync_in_del is never used. Should we remove it? And clean up queue and function names to _c?

A new version would be introduced only when compatibility is broken. In theory the 1301 version could be renamed to something lower. As far as my own experience with FreeBSD goes it could be probably 10.0. But I have tested it only with a 13.1 peer, so let's keep this as the initial value.

I'm fine with breaking the protocol compatibility within 14 until we release 14.0 anyway.

I mean, we shouldn't go out of our way to break it, but we're not going to install compatibility hacks for changes within main prior to a release either.

It seems so. I'll try to do a bit of a dive in the history later, but it seems like this might be for compatibility with some older version, in which case it's probably old enough we can just delete it.

Do you want me to split the pf_sync_in_del change into a separate review?

Yeah, that seems like w good idea.

Sorry I haven’t gotten around to this one yet. It’s on my list, but you may be familiar with the ever growing nature of todo lists.

I'd separate this as its own commit as well.

Extending the state information passed to userspace probably belongs in a separate commit as well.

I did a little bit of digging, and I think that sending of PFSYNC_ACT_DEL was retired in OpenBSD in this commit:

commit c62efabc0a1b17d5fbc801b77749f7b4b8b7049b
Author: dlg <dlg@openbsd.org>
Date:   Mon Feb 16 00:31:25 2009 +0000

    pfsync v5, mostly written at n2k9, but based on work done at n2k8.

    WARNING: THIS BREAKS COMPATIBILITY WITH THE PREVIOUS VERSION OF PFSYNC

    this is a new variant of the protocol and a large reworking of the
    pfsync code to address some performance issues. the single largest
    benefit comes from having multiple pfsync messages of different
    types handled in a single packet. pfsyncs handling of pf states is
    highly optimised now, along with packet parsing and construction.

    huggz for beck@ for testing.
    huge thanks to mcbride@ for his help during development and for
    finding all the bugs during the initial tests.
    thanks to peter sutton for letting me get credit for this work.

    ok beck@ mcbride@ "good." deraadt@

So I'm inclined to say we can indeed just get rid of support for it entirely.

I'd default: this, because if we add a new version that doesn't affect PFSYNC_S_INS we'd have to add another case here (or change it to default).

Moved into D40013

Moved into D40004.