Technically, this is both ccval and cscov. It probably needs something similar to this excerpt from struct tcphdr:

#if BYTE_ORDER == LITTLE_ENDIAN
        u_char  th_x2:4,                /* (unused) */
                th_off:4;               /* data offset */
#endif
#if BYTE_ORDER == BIG_ENDIAN
        u_char  th_off:4,               /* data offset */
                th_x2:4;                /* (unused) */
#endif

These last few fields don't exactly match the RFC. Because this header could be used by others in the future, I think its important that what you commit match the RFC. (OTOH, I think it is fine to just define the common part [through the "X" field] and add a placeholder for what follows.)

This isn't the right statistic for this error. We might need to teach frag6_freef to deal with different error codes.

In a failure, this results in a double-free.

Even though this resets m, it doesn't update the caller's concept of m. To do that you need to change the prototype to something like:
int ip6_validhdrchain(struct mbuf **mp, ...)

Then, you can do this early in the code:

struct mbuf *m;

m = *mp;

Then, after this failure, you would do:
*mp = NULL;

Alternatively, you can change the semantics of the return value such that a certain return value always means the mbuf is freed.

Again, doublefree. See above.

Doublefree. See above.

I can see a possibility that this will cause strange problems in two cases:

1. Down the road, when someone adds a new protocol, they may forget to add it to this list. For that reason, I think we need to do similar checking on unfragmented packets -- at least, for INVARIANTS builds. In fact, there is no reason you couldn't use exactly this same function to check unfragmented packets.
2. In cases where someone uses raw sockets to listen for a new protocol. There are two options for dealing with this that immediately come to mind. The easy option is to have the default behavior controlled by a sysctl. The harder option is to have the default behavior vary based on the protocols for which raw sockets are listening. For version 1, I would suggest the "easy" way (sysctl, and with a default of "allow").

This is unnecessary. I think you should either have a default clause in the switch statement or this, but not both.

This still isn't fixed: "doesn't have an upper layer..." or "doesn't contain an upper layer..."

This comment still needs to be a full sentence.

As implemented, why is this not just a static function?

Due to alignment, it seems likely that an extra byte will be added before d_cksum. The easiest way to get around this is to turn this into a 2-byte array (e.g. uint8_t d_cksum[2]).

The indentation looks funny here; however, that could be a Phabricator display issue. Please verify the continuation line is tabbed to line up with the if + 4 spaces more. See style(9) for more complete details.

Let me illustrate my concern with this change by way of example: Assume I have a packet with this set of headers: IP6, frag, dstopts, tcp. As far as I can tell, this change will not verify that the tcp header is in the first fragment.

To counteract this, I think you need to add some sort of loop to recursively check headers until it reaches a ULP header, exceeds some limit (e.g. the same loop limit we have in the main header processing code), or reaches the end of the packet.

Comments need to be a full sentence.

If this is to be done, it's a separate commit which could just happen right away.

I would even reference RFC 8200 (pg 18/19):

The first fragment packet is composed of:
..
    (3)  Extension headers, if any, and the Upper-Layer header.  These
         headers must be in the first fragment.  Note: This restricts
         the size of the headers through the Upper-Layer header to the
         MTU of the path to the packet's destinations(s).

I've just worked my way through frag6.c and I start to understand what I didn't like on the current approach:

(1) one call to a function would be enough the "ip6_upperlayerhdr(ip6f->ip6f_nxt)" could be done there.
(2) the V_ip6_allow_unknown_upperlayerhdr is something with this generic name I'd love to have inside that one function (or a helper function called from there) but not in the code itself. It is not a property of frag6.c to decide if this policy is acceptable or not but of the checking code.

So I guess I am asking to collapse the initial check and the additional full search into one KPI (making the other two functions possible helper functions, though only if that makes sense code/loop wise). I haven't looked yet.

Then lastly I'll probably go back and revise where/when the check is done but let's leave that for the moment as frag6.c is likely to change in the meantime.

This code really should not live in frag6.c. There's a few more places in the source tree which can re-use this. Having done one of the first loops in the tree in ipfw2 after v6 support was merged and people having added more protocols to it since, please make sure these two are in sync, compatible, if not even able to share code.