unix/dgram: dispose mbufs on uxdg_mb queue

[khng]

Summary

Since the new implementation of unix/dgram is introduced, the old
one-size-fits-all solution is not longer in use. Do our own cleanup by
walking mbufs through m_nextpkt/m_stailqpkt (alias of m_nextpkt) so that
nothing is leaked if unp_disconnect/unp_dispose found buffers to be
freed.

Sponsored by: Juniper Networks, Inc.
MFC after: 3 days

[khng]

Oops.

[glebius]

m_stailqpkt is an alias to m_nextpkt, so the code:

m0 = is_dgram ? STAILQ_NEXT(m0, m_stailqpkt) : m0->m_nextpkt;

doesn't change anything with is_dgram switching between true/false.

Do you really see a leak fixed with this patch? Is this the leak reported in bug 279467?

[glebius]

Just seen reply there from Dmitry. It is the same leak.

[khng]

In D45443#1037070, @glebius wrote:

m_stailqpkt is an alias to m_nextpkt, so the code:

m0 = is_dgram ? STAILQ_NEXT(m0, m_stailqpkt) : m0->m_nextpkt;

doesn't change anything with is_dgram switching between true/false.

Do you really see a leak fixed with this patch? Is this the leak reported in bug 279467?

ok, then the change to unp_gc can be reverted, We just need the uxdg queue m_freem.

[khng]

m_nextpkt and m_stailqnxt are the same.

[glebius]

Maybe I'm wrong and some of my assumptions don't work. Please give me a bit more time to look into that. Thanks a lot for working on that!

[glebius]

Ah, I see the bug. It is just because m_freem() follows only m_next linkage and doesn't follow m_nextpkt linkage. I always assumed it does both.

[markj]

sys/kern/uipc_usrreq.c

3220

You don't need to remove it. The usual idioms are:

while ((m = STAILQ_FIRST(...)) != NULL) {
    STAILQ_REMOVE(...);
    m_freem(m);
}

or

STAILQ_FOREACH_SAFE(...) {
    m_freem(m);
}

3307–3310

I think the first parameter needs to be removed?

[khng]

In D45443#1037231, @glebius wrote:

Ah, I see the bug. It is just because m_freem() follows only m_next linkage and doesn't follow m_nextpkt linkage. I always assumed it does both.

Yep, the old cold did it with sbflush instead. So in this case I make the fix into implementation specific part than to let sbflush do the job.

[khng]

markj@: code should be idiomatic

[glebius]

https://reviews.freebsd.org/D45477
https://reviews.freebsd.org/D45478

[khng]

In favour of D45477 and D45478.