Re: Changing pf to use kmalloc instead of zalloc

[Ilya Dryomov <idryomov@xxxxxxxxx>]

On Sun, Jan 23, 2011 at 10:25:33AM +0100, Jan Lentfer wrote:
> I updated and rebased my branch at
> http://gitweb.dragonflybsd.org/~lentferj/dragonfly.git/shortlog/refs/heads/pf44
> so everything is in one patch/diff and no intermediate stuff anymore.
>
> pf.ko loads and I can load the ruleset, etc. It looks all good, until...
>
> 2) I kldunload pf.ko
> malloc_uninit: 208 bytes of 'pfstatepl' still allocated on cpu 4
> malloc_uninit: 896 bytes of 'pfrulepl' still allocated on cpu 4

I coded a quick and dirty kmalloc leak detector (as per dillon's
suggestion) and it was able to point to specific kmalloc() calls:

dumping kmalloc leak list
  kmalloc addr 0xddf94ce8 on line 1205 in /usr/src-nfs/sys/net/pf/pf_ioctl.c
  kmalloc addr 0xd8658280 on line 3613 in /usr/src-nfs/sys/net/pf/pf.c
malloc_uninit: 224 bytes of 'pfstatepl' still allocated on cpu 2
malloc_uninit: 896 bytes of 'pfrulepl' still allocated on cpu 2

It turns out you are missing kfree() calls for a particular [rule,
state] pair.  Here's some kgdb info that can help:

(kgdb) p $rule
$1 = {src = {addr = {v = {a = {addr = {pfa = {v4 = {s_addr = 0}, v6 = {__u6_addr = {__u6_addr8 = '\000' <repeats 15 times>, 
                  __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}, addr8 = '\000' <repeats 15 times>, 
              addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, addr32 = {0, 0, 0, 0}}}, mask = {pfa = {v4 = {s_addr = 0}, v6 = {
                __u6_addr = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {
                    0, 0, 0, 0}}}, addr8 = '\000' <repeats 15 times>, addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, addr32 = {0, 0, 0, 
                0}}}}, ifname = '\000' <repeats 15 times>, tblname = '\000' <repeats 31 times>, 
        rtlabelname = '\000' <repeats 31 times>, rtlabel = 0}, p = {dyn = 0x0, tbl = 0x0, dyncnt = 0, tblcnt = 0}, 
      type = 0 '\000', iflags = 0 '\000'}, port = {0, 0}, neg = 0 '\000', port_op = 0 '\000'}, dst = {addr = {v = {a = {
          addr = {pfa = {v4 = {s_addr = 0}, v6 = {__u6_addr = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 
                    0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}, addr8 = '\000' <repeats 15 times>, addr16 = {0, 0, 0, 0, 
                0, 0, 0, 0}, addr32 = {0, 0, 0, 0}}}, mask = {pfa = {v4 = {s_addr = 0}, v6 = {__u6_addr = {
                  __u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 
                    0}}}, addr8 = '\000' <repeats 15 times>, addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, addr32 = {0, 0, 0, 0}}}}, 
        ifname = '\000' <repeats 15 times>, tblname = '\000' <repeats 31 times>, rtlabelname = '\000' <repeats 31 times>, 
        rtlabel = 0}, p = {dyn = 0x0, tbl = 0x0, dyncnt = 0, tblcnt = 0}, type = 0 '\000', iflags = 0 '\000'}, port = {0, 0}, 
    neg = 0 '\000', port_op = 0 '\000'}, skip = {{ptr = 0x0, nr = 0}, {ptr = 0x0, nr = 0}, {ptr = 0x0, nr = 0}, {ptr = 0x0, 
      nr = 0}, {ptr = 0x0, nr = 0}, {ptr = 0x0, nr = 0}, {ptr = 0x0, nr = 0}, {ptr = 0x0, nr = 0}}, 
  label = '\000' <repeats 63 times>, ifname = '\000' <repeats 15 times>, qname = "inet_ul_bulk", '\000' <repeats 51 times>, 
  pqname = "inet_ul_im", '\000' <repeats 53 times>, tagname = '\000' <repeats 63 times>, 
  match_tagname = '\000' <repeats 63 times>, overload_tblname = '\000' <repeats 31 times>, entries = {tqe_next = 0x0, 
    tqe_prev = 0x0}, rpool = {list = {tqh_first = 0x0, tqh_last = 0xddf94ee0}, cur = 0x0, key = {pfk = {
        key8 = '\000' <repeats 15 times>, key16 = {0, 0, 0, 0, 0, 0, 0, 0}, key32 = {0, 0, 0, 0}}}, counter = {pfa = {v4 = {
          s_addr = 0}, v6 = {__u6_addr = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, 
            __u6_addr32 = {0, 0, 0, 0}}}, addr8 = '\000' <repeats 15 times>, addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, addr32 = {0, 
          0, 0, 0}}}, tblidx = 0, proxy_port = {0, 0}, port_op = 0 '\000', opts = 0 '\000'}, evaluations = 1, packets = {28, 
    15}, bytes = {2080, 1500}, kif = 0x0, anchor = 0x0, overload_tbl = 0x0, os_fingerprint = 0, rtableid = -1, timeout = {
    0 <repeats 20 times>}, states_cur = 1, states_tot = 1, max_states = 0, src_nodes = 0, max_src_nodes = 0, 
  max_src_states = 0, max_src_conn = 0, max_src_conn_rate = {limit = 0, seconds = 0}, qid = 1, pqid = 3, rt_listid = 0, 
  nr = 4294967295, prob = 0, cuid = 0, cpid = 0, return_icmp = 0, return_icmp6 = 0, max_mss = 0, tag = 0, match_tag = 0, 
  uid = {uid = {0, 0}, op = 0 '\000'}, gid = {gid = {0, 0}, op = 0 '\000'}, rule_flag = 0, action = 0 '\000', 
  direction = 2 '\002', log = 0 '\000', logif = 0 '\000', quick = 0 '\000', ifnot = 0 '\000', match_tag_not = 0 '\000', 
  natpass = 0 '\000', keep_state = 1 '\001', af = 0 '\000', proto = 0 '\000', type = 0 '\000', code = 0 '\000', 
  flags = 0 '\000', flagset = 0 '\000', min_ttl = 0 '\000', allow_opts = 0 '\000', rt = 0 '\000', return_ttl = 0 '\000', 
  tos = 0 '\000', set_tos = 0 '\000', anchor_relative = 0 '\000', anchor_wildcard = 0 '\000', flush = 0 '\000', divert = {
    addr = {pfa = {v4 = {s_addr = 0}, v6 = {__u6_addr = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 
              0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}, addr8 = '\000' <repeats 15 times>, addr16 = {0, 0, 0, 0, 0, 0, 0, 
          0}, addr32 = {0, 0, 0, 0}}}, port = 0}, pickup_mode = 3 '\003', unused01 = 0 '\000'}

(kgdb) p $state
$2 = {id = 968179021, creatorid = 2068286261, direction = 2 '\002', pad = "\000\000", entry_list = {tqe_next = 0x0, 
    tqe_prev = 0xdde31858}, entry_id = {rbe_left = 0x0, rbe_right = 0x0, rbe_parent = 0x0, rbe_color = 0}, src = {
    scrub = 0x0, seqlo = 3015172378, seqhi = 3015172855, seqdiff = 0, max_win = 1810, mss = 0, state = 5 '\005', 
    wscale = 0 '\000', tcp_est = 0 '\000', pad = ""}, dst = {scrub = 0x0, seqlo = 2525813544, seqhi = 1810, seqdiff = 0, 
    max_win = 477, mss = 0, state = 5 '\005', wscale = 0 '\000', tcp_est = 0 '\000', pad = ""}, rule = {ptr = 0xddf94ce8, 
    nr = 3724102888}, anchor = {ptr = 0x0, nr = 0}, nat_rule = {ptr = 0x0, nr = 0}, rt_addr = {pfa = {v4 = {s_addr = 0}, 
      v6 = {__u6_addr = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 
            0, 0}}}, addr8 = '\000' <repeats 15 times>, addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, addr32 = {0, 0, 0, 0}}}, key = {
    0x0, 0x0}, kif = 0xdd9ebd70, rt_kif = 0x0, src_node = 0x0, nat_src_node = 0x0, packets = {15, 28}, bytes = {1500, 2080}, 
  hash = 0, creation = 1295889721, expire = 1295889724, pfsync_time = 0, tag = 0, log = 0 '\000', state_flags = 0 '\000', 
  timeout = 22 '\026', sync_flags = 1 '\001', pickup_mode = 0 '\000'}

These are the rule and state that keep sitting there after kldunload.
Note that state.rule points to the rule that misses kfree() call.  I
don't have time to go further, but it should be easy to fix this now
that you know what is actually going on.

Thanks,

		Ilya