Page MenuHomeFreeBSD

netinet: Remove unneeded mb_unmapped_to_ext() calls
ClosedPublic

Authored by markj on Tue, Nov 23, 2:30 PM.

Details

Summary

in_cksum_skip() now handles unmapped mbufs on platforms where they're
permitted.

MFC after: 1 week
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.Tue, Nov 23, 2:30 PM
sys/netinet6/ip6_output.c
237

Oops, this is dead code since we have !frag here. I'm not sure why there's a special case for fragments here.

sys/netinet/ip_output.c
746

This is now broken for an interface that doesn't support IFCAP_MEXTPG and has checksum offload disabled as we will not fall through to the else. I think the IFCAP_MEXTPG check now needs to just be its own check. If it makes the checksum calculations more optimal we should perhaps do it before doing the checksums.

sys/netinet6/ip6_output.c
237

Hmmm, the boolean is a bit odd here and should probably be called "is_frag" as it is true when transmitting a fragment and false when sending an unfragmented packet. I think the intention was that we would always end up doing this check and converting the entire packet chain to unmapped if the interface didn't support the flag in the first call when frag is false and that in the frag == true case it should be redundant. Put another way, it should be possible to assert in the frag == true case that the mbuf is already mapped. This does mean that in the case of a fragmented packet I think we calculate the full checksums if needed and then throw that work away and recalc checksums on each fragment. You could probably just remove the frag check and only check for IFCAP_MEXTPG and it would be ok. I'm not sure if it is worth trying to preserve this optimization for fragments.

Alternatively, you could hoist this check back out of this function and into the ip6_output itself perhaps just after the 'passout' label in ip6_output. I think I prefer that as it makes the code more obvious. It was here only to try to put all the logic in one place for calling mb_unmapped_to_ext and IFCAP_MEXTPG doesn't really have anything to do with checksums otherwise.

markj added inline comments.
sys/netinet/ip_output.c
746

Oops, thanks. I realized this when writing the patch but forgot to actually do something about it. :(

I guess m_apply() is a bit cheaper for non-extpg mbufs, so it makes some sense to do that first.

sys/netinet6/ip6_output.c
237

You last suggestion results in some nice simplification, so I'll go with that.

markj marked 2 inline comments as done.
  • Fix ip_output()
  • Move the mb_unmapped_to_ext() call out of ip6_output_delayed_csum()
This revision is now accepted and ready to land.Tue, Nov 23, 8:15 PM