iflib: prevent possible infinite loop in iflib_encap

[jacob.e.keller_intel.com]

Summary

iflib_encap calls bus_dmamap_load_mbuf_sg. Upon it returning EFBIG, an
m_collapse and an m_defrag are attempted to shrink the mbuf cluster to
fit within the DMA segment limitations.

However, if we call m_defrag, and then bus_dmamap_load_mbuf_sg returns
EFBIG on the now defragmented mbuf, we will continuously re-call
bus_dmamap_load_mbuf_sg over and over.

This happens because m_head isn't NULL, and remap is >1, so we don't try
to m_collapse or m_defrag again. The only way we exit the loop is if
m_head is NULL. However, m_head can't be modified by the call to
bus_dmamap_load_mbuf_sg, because we don't pass it as a double pointer.

I believe this will be an incredibly rare occurrence, because it is
unlikely that bus_dmamap_load_mbuf_sg will actually fail on the second
defragment with an EFBIG error. However, it still seems like
a possibility that we should account for.

Fix the exit check to ensure that if remap is >1, we will also exit,
even if m_head is not NULL.

Signed-off-by: Jacob Keller <[e-mail]>

[jacob.e.keller_intel.com]

I found this during code inspection while looking at a similar construction for a legacy device driver. It's possible this can never trigger, depending on how bus_dmamap_load_mbuf_sg returns EFBIG, but it definitely seemed plausible that it might happen, and potentially locking up a CPU is not a happy thing to do.

[jacob.e.keller_intel.com]

Move the remap++ to after the error check

[jacob.e.keller_intel.com]

sys/net/iflib.c

3289 ↗

(On Diff #54738)

Actually, I'm pretty sure this is incorrect. because remap will be >1 after the first call to m_defrag. Woops.

[jacob.e.keller_intel.com]

I'm definitely open to better/alternative solutions, and no, I don't actually have a live replication of the issue.

[gallatin]

I think this is a mostly theoretical issue, as you'll have had a chain which was impossible to send (eg, far too large, etc) even after defrag.

[jacob.e.keller_intel.com]

add an MPASS so we trigger a panic when in debug

[jacob.e.keller_intel.com]

In D19468#418973, @gallatin wrote:

I think this is a mostly theoretical issue, as you'll have had a chain which was impossible to send (eg, far too large, etc) even after defrag.

Yea, I agree it's theoretical. It's more of a "the consequences of this is a locked up CPU, so I'd rather avoid that if it ever does happen"

[shurd]

sys/net/iflib.c

3283 ↗

(On Diff #55033)

Shouldn't this be moved to the top of the block? No need to actually call m_defrag() before the MPASS() I think.

[gallatin]

sys/net/iflib.c

3283 ↗

(On Diff #55033)

if remap is >1, then neither defrag nor collapse will be called. so i don't think the placement matters, as long is it is prior to the increment (which it is)

[jacob.e.keller_intel.com]

sys/net/iflib.c

3283 ↗

(On Diff #55033)

Yea, we don't call either one if the MPASS is ever triggered.

[shurd]