Fix bridge_fragment()

[olivier]

Summary

Fragmented UDP and ICMP packets were corrupted if a firewall with reassembling feature (like pf'scrub) is enabled on the bridge.
This patch fixes corrupted packet problem and the panic (triggered easly with low RAM) as explain in PR 185633.

bridge_pfil and bridge_fragment relationship:

1. bridge_pfil() receive (IN direction) packets and sent it to the firewall
2. The firewall can be configured for reassembling fragmented packet (like pf'scrubing) in one mbuf chain
3. when bridge_pfil() need to send this reassembled packet to the outgoing interface, it needs to re-fragment it by using bridge_fragment()
4. bridge_fragment() had to split this mbuf (using ip_fragment) first then had to M_PREPEND each packet in the mbuf chain for adding Ethernet header.
5. But M_PREPEND can sometime create a new mbuf on the begining of the mbuf chain, then the "main" pointer of this mbuf chain should be updated and this case is tottaly forgotten.

The original bridge_fragment code (Revision 158140, 2006 April 29) came from OpenBSD, and the call to bridge_enqueue was embedded.
But on FreeBSD, bridge_enqueue() is done after bridge_fragment(), then the original OpenBSD code can't work as-it of FreeBSD.

Test Plan

Enable pf (scrub;pass) on a bridge and generate a fragmented ping (-s 2000) across the bridge.

[kp]

Just so we don't lose track of this: I think you've indeed identified the source of the problem with the bridge.

There's still a couple of problems with the error handling though.
m_freem() will not walk the m_nextpkt chain, so the error path leaks memory. Perhaps even worse is that it will repeatedly free m, which is also incorrect.
It also messes up (doesn't free anything) if the error occurs in the last packet of the m_nextpkt chain.

[olivier]

Rewrote the error handling part

[kp]

sys/net/if_bridge.c

3517 ↗

(On Diff #20067)

Doesn't this need to go before the if (snap) block? If we end up prepending an mbuf for the snap header but not for the ethernet header we still need to do the m_next/m_nextpkt pointer reshuffle.

3530 ↗

(On Diff #20067)

Won't this break in a (worst-case) scenario where both the snap and ethernet M_PREPEND()s end up adding a new mbuf?

3545 ↗

(On Diff #20067)

You don't need this check, the for loop has the same effect.

[olivier]

Take care of the snap M_PREPEND that end up adding a new mbuf and remove the useless test into the dropit:

[olivier]

Simplify the code for being more readable

[olivier]

Added style(9) fixes from @kristof.

[kp]

Gnn suggested that Robert had ideas about this. Include him in the review.