Bug 212630 - ipfw swap does not swap tables between sets
Summary: ipfw swap does not swap tables between sets
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 11.0-RC1
Hardware: Any Any
: --- Affects Only Me
Assignee: Andrey V. Elsukov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-12 20:25 UTC by John Zielinski
Modified: 2016-09-20 16:22 UTC (History)
1 user (show)

See Also:


Attachments
Proposed patch (2.10 KB, patch)
2016-09-13 14:47 UTC, Andrey V. Elsukov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Zielinski 2016-09-12 20:25:46 UTC
I have net.inet.ip.fw.tables_sets=1 to place tables into sets.  The ipfw set move command works and moves the tables into the new set.  The ipfw set swap command does not swap the tables between sets.

#sysctl net.inet.ip.fw.tables_sets=1
#ipfw set 2 table test create
#ipfw set 3 table test create
#ipfw set 2 table test add 10.0.0.0/8
#ipfw add set 2 deny all from any to "table(test)" via tun
#ipfw -S table all list
--- table(test), set(3) ---
--- table(test), set(2) ---
10.0.0.0/8 0
#ipfw set swap 2 3
#ipfw -S table all list
--- table(test), set(3) ---
--- table(test), set(2) ---
10.0.0.0/8 0

It looks like the swap_sets_cb function in is getting called multiple times for the same tables:

swap_sets_cb: no=0xfffff80013313d00, no->kidx=2, no->name=test, no->set=3, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff800b21e0b00, no->kidx=1, no->name=test, no->set=2, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff80013313d00, no->kidx=2, no->name=test, no->set=2, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff800b21e0b00, no->kidx=1, no->name=test, no->set=3, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff80013313d00, no->kidx=2, no->name=test, no->set=3, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff800b21e0b00, no->kidx=1, no->name=test, no->set=2, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff80013313d00, no->kidx=2, no->name=test, no->set=2, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff800b21e0b00, no->kidx=1, no->name=test, no->set=3, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff80013313d00, no->kidx=2, no->name=test, no->set=3, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff800b21e0b00, no->kidx=1, no->name=test, no->set=2, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff80013313d00, no->kidx=2, no->name=test, no->set=2, args->set=2, args->new_set=3
swap_sets_cb: no=0xfffff800b21e0b00, no->kidx=1, no->name=test, no->set=3, args->set=2, args->new_set=3

That is all the calls for one "ipfw set swap 2 3" command.  Since it's an even number of calls for each object they all end up in their original sets instead of swapped.

Ah, just figured out why.  Each table rewriter is asked to swap tables and their are six rewriters.  Each rewriter's table_manage_sets calls ipfw_obj_manage_sets to swap the two tables.  That's why I see 6 calls to swap_sets_cb, 6 rewriters x 2 tables = 12 calls.  That's why it doesn't swap.

I also checked how many times move_sets_cb is called and it's also 12 times (6 * 2).  But for move it's only one way so it ends up working.
Comment 1 Andrey V. Elsukov freebsd_committer freebsd_triage 2016-09-13 14:47:07 UTC
Created attachment 174729 [details]
Proposed patch

Hi, can you test this patch?
Comment 2 John Zielinski 2016-09-13 18:00:00 UTC
Tested and it works.  Thank you.
Comment 3 commit-hook freebsd_committer freebsd_triage 2016-09-13 18:17:04 UTC
A commit references this bug:

Author: ae
Date: Tue Sep 13 18:16:16 UTC 2016
New revision: 305778
URL: https://svnweb.freebsd.org/changeset/base/305778

Log:
  Fix swap tables between sets when this functional is enabled.

  We have 6 opcode rewriters for table opcodes. When `set swap' command
  invoked, it is called for each rewriter, so at the end we get the same
  result, because opcode rewriter uses ETLV type to match opcode. And all
  tables opcodes have the same ETLV type. To solve this problem, use
  separate sets handler for one opcode rewriter. Use it to handle TEST_ALL,
  SWAP_ALL and MOVE_ALL commands.

  PR:		212630
  MFC after:	1 week

Changes:
  head/sys/netpfil/ipfw/ip_fw_table.c
Comment 4 commit-hook freebsd_committer freebsd_triage 2016-09-20 13:23:49 UTC
A commit references this bug:

Author: ae
Date: Tue Sep 20 13:23:09 UTC 2016
New revision: 306025
URL: https://svnweb.freebsd.org/changeset/base/306025

Log:
  MFC r305778:
    Fix swap tables between sets when this functional is enabled.

    We have 6 opcode rewriters for table opcodes. When `set swap' command
    invoked, it is called for each rewriter, so at the end we get the same
    result, because opcode rewriter uses ETLV type to match opcode. And all
    tables opcodes have the same ETLV type. To solve this problem, use
    separate sets handler for one opcode rewriter. Use it to handle TEST_ALL,
    SWAP_ALL and MOVE_ALL commands.

    PR:		212630

Changes:
_U  stable/11/
  stable/11/sys/netpfil/ipfw/ip_fw_table.c
Comment 5 Andrey V. Elsukov freebsd_committer freebsd_triage 2016-09-20 16:22:33 UTC
Fixed in head/ and stable/11, but since 11.0-RC3 is the last planned release candidate, it will not be merged in releng/11.0.
Thanks!