I'm not sure if we should return ENETUNREACH (that's likely be returned to the user) for the blackhole routes.
Also I'm trying to find the code you refer to in IPv6 and I can't seem to find any place in ip6_output() that increments ip6s_cantforward counter.

For NHF_REJECT I think it is OK to have EHOSTUNREACH or ENETUNREACH since this is explicit by admin. Well I'd prefer the former ( EHOSTUNREACH ) for unicast destination. For a multicast / broadcast destination I think ENETUNREACH is proper.

For NHF_BLACKHOLE, the system should behave as if the packet was transmitted / forwarded, so should return 0 IIUC.

This is the IPv6 equivalent: https://cgit.freebsd.org/src/tree/sys/netinet6/ip6_forward.c#n197

It's in ip_output() here, because ip_forward() doesn't do a routing lookup, but calls into ip_forward(). ip6_forward() does do its own work.

I have no particularly strong feelings on what error code we return in this case, although I think I would prefer not making a distinction between NHF_REJECT and NHF_BLACKHOLE, if only to keep the code simpler (also, IPv6 doesn't).

I'm afraid I don't understand :-(

ip_output() main purpose is to output local packets, not forward.
The default forwarding path, fast forwarding, does not touch ip_output() at all. It's only ipsec and multicast that go through "slow" forwarding process and end up in ip_output().

I don't disagree with proper accounting for errors, but let's actually account *only* for the packets that are being forwarded.

Hmm, now I'm confused too. I missed the ip_fastfwd() call, where it looks like we ought to increment ips_cantforward (via ip_findroute()), but the fwd_ip_blackhole test failed without this patch, and succeeds with. Presumably that means we don't take ip_fastfwd(), but I don't immediately see why.

I'm also tempted to make this code use ip_findroute(), because that handles the counter bits for ip_fastfwd(), but that would still fail to make the output/forward distinction. I don't really want to add even more flags to make ip_findroute() cope with that.

This mirrors the check done in the fast forward case: https://cgit.freebsd.org/src/tree/sys/netinet/ip_fastfwd.c#n207

And given that the commonality between REJECT and BLACKHOLE|BROADCAST is two lines (IPSTAT_INC(ips_cantforward) and NH_FREE()) I'm not sure that's worth making things more complicated. (Much like we do in the fastforward case)

We'd end up with this:

if (ro.ro_nh->nh_flags & (NHF_BLACKHOLE | NHF_BROADCAST
    | NHF_REJECT)) {
   	IPSTAT_INC(ips_cantforward);
	NH_FREE(ro.ro_nh);
   	if (ro.ro_nh->nh_flags & NHF_REJECT) {
   	    	 icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, 0, 0);
   	} else {
   	    	m_freem(m);
   	}
   	return;
}

(i.e. more complex, and we're now repeating the NHF_REJECT check)