Re: IFQ_ENQUEUE argument refactor (was Re: RFC: ALTQ caller refactor)

[Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>]

Hi,

Thank you for review and comment.

On 2016/03/29 23:22, Taylor R Campbell wrote:
> Hmm...  It looks like there are some paths out of IFQ_CLASSIFY that do
> not lead to IFQ_ENQUEUE, which would cause a memory leak.  For
> example, in atm_output, IFQ_CLASSIFY is unconditional, but every
> `senderr' branch ends in `goto bad', which skips ifq_enqueue.  When
> the pktattrs were stack-allocated that was OK, but now we need to
> explicitly release them with altq_delete_pattr.
> 
> Except maybe it won't be a memory leak -- maybe it will attempt to
> free(9) a tag that was allocated with pool_cache(9), via m_freem ->
> MFREE -> m_tag_delete_chain -> m_tag_delete -> m_tag_free -> free.
> When all tags were created with m_tag_get that was correct, but now it
> is necessary to clean them up explicitly.

You are right. Hmm..., I'm sorry, I change my objective. I think it could
be better to use pool cache for m_tag itself.

To fix a memory leak you point out, it must free altq_pktattr_tag in
the case that M_PREPEND failed. Considering also other cases, m_tag_delete()
might be modified like the following.
====================
m_tag_delete(struct mbuf *m, struct m_tag *t)
{

#ifdef ALTQ
       if (t->m_tag_id == PACKET_TAG_ALTQ_PATTR) {
               struct altq_pktattr_tag *apt;
               apt = container_of(t, struct altq_pktattr_tag,
                   apt_tag);
               altq_m_tag_delete(m, apt);
               return;
       }
#endif
       m_tag_unlink(m, t);
       m_tag_free(t);
}
====================
Hmm, It could be overdoing, however I think it would rather use
pool cache for m_tag itself than add above ALTQ specific code to
m_tag_delete().

So, I implement m_tag pool cache and refactor IFQ_ENQUEUE. Here is
the patch series.
    http://www.netbsd.org/~knakahara/mtag-pool-cache-ifq-enqueue/
        0001-introduce-pool-cache-for-m_tag.patch
        0002-add-dtrace-probe.patch
        0003-eliminate-pktattr-argument-from-IFQ_ENQUEUE.patch

Could you comment above patches?


I'm sorry to change later...

Thanks,

-- 
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>