This function is not used anywhere. So why do you define it? Later down, .pr_ctloutput is set to udp_ctloutput. I guess you want to tweak udp_ctloutput to do the right thing for handling UDP_ENCAP for IPv6. Or am I missing something?

Hi !
You are totally right, this code is not used mostly because I have worked on old code.
I am currently working to fix that.
Testing the code if it is correctly working and will fix this soon.

The break statement does nothing here.

There is a mix of styles here: some error paths return an error number, others set error equal to the error number, then break;. We should be consistent. Most of the code in this function does the latter, so the new code should do that as well.

Same comments as above.

Rather than checking IPSEC_ENABLED(ipv6) twice, it'd be cleaner to structure the code like this:

if (IPSEC_ENABLED(ipv6)) {
    if (IPSEC_CHECK_POLICY(...) != 0) {
        ...
    }
    if ((up->u_flags & UF_ESPINUDP) != 0 &&
        UDPENCAP_INPUT(...) != 0) {
            return (0);
    }
}

This line is too long.

The style here is wrong, the opening brace should be on the previous line.

The indentation here is wrong. checkstyle9.pl should complain about this.

What if there is an IPv6 extension header following the base header? We will end up inserting the UDP header in between, which seems wrong. Is there some guarantee that this won't happen?

There should be an empty line after variable declarations.

Why do you use literal '16' there?

There should be a blank line after the locals declarations.

A blank line should be added before the block comment

There too

extra ()

extra ()

Good Idea.

Will add it, but this comment was not from me.

Will do.

@tuexen can you say whether I'm missing something here? Is there some reason we can simply not worry about extension headers here?

@markj Good point. I think the UDP header should be placed right before the ESP header, not after the IPv6 header. There can be extension headers between the IPv6 header and the ESP header.

From RFC 4303:

In the IPv6 context, ESP is viewed as an end-to-end payload, and thus
should appear after hop-by-hop, routing, and fragmentation extension
headers. Destination options extension header(s) could appear
before, after, or both before and after the ESP header depending on
the semantics desired. However, because ESP protects only fields
after the ESP header, it generally will be desirable to place the
destination options header(s) after the ESP header. The following
diagram illustrates ESP transport mode positioning for a typical IPv6
packet.

This looks like it should instead be

#if defined(INET) || defined(INET6)

The style here is still wrong. Please make sure to address the issues that checkstyle9.pl reports.

Looking closer, I think the current implementation always puts the ESP header immediately after the base header: ipsec(4|6)_perform_request() pass the offset of the "next header" field to esp_output(), which inserts its own header before the original next header. Subsequently the IP layer may insert extension headers, but that happens only after UDP encapsulation is done.

So, I think the code is ok, it should just assert that ip6->ip6_nxt == IPPROTO_ESP, i.e., I'd like to see a KASSERT() to that effect.

There should be an empty line after variable declarations. The same issue exists in a few places in the diff.

It complains in lots of part of this file...

Same as previous comment.

@markj Do I have to add something to this code?

Will fix

The opening brace should be on the previous line.

Please add a KASSERT which verifies that ip6->ip6_nxt is equal to IPPROTO_ESP.

Continuing lines should be indented by four spaces, not one.

Okay but checkstyle9pl is complaining about 80 col. So... is this an exception or not?

Sorry

Forgotten this one... sorry;