ipset does NSS lookups even if ports are numeric

[Junien F]

[Impact]
A change included ipset 6.37 as a performance regression as all ip set rule incur a getprotocolbyname lookup, irrespective of whether the name of the protocol or the actual port number is specified in the set configuration. For large sets this can double the speed of applying changes to ipset tables.

[Test Plan]
# Create a suitable large set of data to restore to the ipset
for x in `seq 1 7`; do for y in `seq 1 254`; do for z in `seq 1 254`; do echo "add test 10.1.1.0/21,80,150.$x.$y.$z/32" >> whitelist-ipv4 ;done; done; done

# Destroy,create, restore
sudo ipset destroy test
sudo ipset create test hash:net,port,net hashsize 4096 maxelem 786432
time sudo ipset restore < ./whitelist-ipv4

Large reduction in time taken to restore the ipset (32s-> 5s on an 8 core machine).

[Where problems could occur]
The original patch to resolve this issue did introduce another bug which as subsequently been fixed as well (and is included in the updated packages).

If the fix introduces issues its likely that iptable rules making use of ipset groups will start to fail in some way - probably rejecting traffic or suchlike.

[Other Info]

[Original Bug Report]
Hi,

Do you think we could get https://git.netfilter.org/ipset/commit/?id=dbeb20a667e82e4efb8b26b24a0ec641dab5c857 SRUed to 20.04 ?

This divides our ipset loading time by ~2 (from ~60s to ~25s).

Thanks

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Haw Loeung (hloeung)]

Current size of ipset used for testing:

| ubuntu@juju-87625f-hloeung-93:~/ipset$ wc -l ~/whitelist-ipv4
| 515698 /home/ubuntu/whitelist-ipv4

With the patch:

| ubuntu@juju-87625f-hloeung-93:~/ipset$ sudo ipset destroy test
| ubuntu@juju-87625f-hloeung-93:~/ipset$ sudo ipset create test hash:net,port,net hashsize 4096 maxelem 786432
| ubuntu@juju-87625f-hloeung-93:~/ipset$ time sudo ~/ipset/src/ipset restore < ~/whitelist-ipv4
|
| real 0m7.204s
| user 0m3.104s
| sys 0m3.877s

vs without.

| ubuntu@juju-87625f-hloeung-93:~/ipset$ sudo ipset destroy test
| ubuntu@juju-87625f-hloeung-93:~/ipset$ sudo ipset create test hash:net,port,net hashsize 4096 maxelem 786432
| ubuntu@juju-87625f-hloeung-93:~/ipset$ time sudo ~/ipset/src/ipset restore < ~/whitelist-ipv4
|
| real 0m33.232s
| user 0m25.291s
| sys 0m7.682s

Output of what I used to revert to compare - https://paste.ubuntu.com/p/x9wcsQdxMn/

[Haw Loeung (hloeung)]

Example entries in ipset used for testing:

| add test 10.1.1.0/21,80,150.222.129.122/31
| add test 10.1.1.0/21,80,150.222.129.124/31
| add test 10.1.1.0/21,80,150.222.129.126/31

We're not specifying the protocol to avoid the just as expensive getprotobyname() lookup.

[Christian Ehrhardt  (paelzer)]

$ git tag --contains dbeb20a667e82e4efb8b26b24a0ec641dab5c857
v7.11

 ipset | 6.34-1 | bionic | source, amd64, arm64, armhf, i386, ppc64el, s390x
 ipset | 7.5-1~exp1 | focal | source, amd64, arm64, armhf, ppc64el, riscv64, s390x
 ipset | 7.6-2 | groovy | source, amd64, arm64, armhf, ppc64el, riscv64, s390x
 ipset | 7.10-1 | hirsute | source, amd64, arm64, armhf, ppc64el, riscv64, s390x
 ipset | 7.10-1 | impish | source, amd64, arm64, armhf, ppc64el, riscv64, s390x

You'll need to fix it in -dev first and then SRU that to all the releases you think will benefit.

[Haw Loeung (hloeung)]

Per discussion on Mattermost, the original behavior was to try string_to_u16() before parse_portname(). This was switched in upstream commit https://git.netfilter.org/ipset/tree/lib/parse.c?id=516600858cb54906fb728d04e5edf1131ee7b3b2 and released with ipset v6.37.

[Christian Ehrhardt  (paelzer)]

I have wrapped you all of this up in MPs that should do it.
I have scripts to do most of that, so it was fast and easy to do and applies cleanly.
But it is the openstacks team package and I don't want to interfere too much.
OTOH from here - if they like it - they can more or less checkout&upload.
(or have them say yes via an Ack on the MPs and I can do the upload).

https://code.launchpad.net/~paelzer/ubuntu/+source/ipset/+git/ipset/+merge/404743
https://code.launchpad.net/~paelzer/ubuntu/+source/ipset/+git/ipset/+merge/404744
https://code.launchpad.net/~paelzer/ubuntu/+source/ipset/+git/ipset/+merge/404745
https://code.launchpad.net/~paelzer/ubuntu/+source/ipset/+git/ipset/+merge/404746

Also someone has to add an SRU Template before this can go on.
@Haw - you might consider making it even easier for the openstack team by adding an SRU Template to the bug description (https://wiki.ubuntu.com/StableReleaseUpdates#SRU_Bug_Template)

[Haw Loeung (hloeung)]

Apologies, can we update it to using the attached patch?

It includes the fix by Jozsef from upstream:

"""
Fix patch "Parse port before trying by service name"
The patch broke parsing service names: number parsing failures
are hard errors which erase data, thus making impossible to
parse input as a string. Fix it by enabling soft (warning)
failures in the case of port number parsing.
"""

https://git.netfilter.org/ipset/commit/?id=fd7d97c57e9dbe215c71be5a2fe049a1f905fddb

Thanks

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "ipset.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[James Page (james-page)]

All patches discussed are included in 7.15 release which is is Jammy development; marking this task as fix released.

[James Page (james-page)]

I've uploaded updates for focal, hirsute and impish for SRU team review.

The updated packages include the original fix and the subsequent followup fix both of which have landed upstream and are included in the release of ipset in Jammy.

[Brian Murray (brian-murray)]

Hello Junien, or anyone else affected,

Accepted ipset into impish-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ipset/7.10-1ubuntu0.21.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-impish to verification-done-impish. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-impish. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Brian Murray (brian-murray)]

Hello Junien, or anyone else affected,

Accepted ipset into hirsute-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ipset/7.10-1ubuntu0.21.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-hirsute to verification-done-hirsute. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-hirsute. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Brian Murray (brian-murray)]

Hello Junien, or anyone else affected,

Accepted ipset into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ipset/7.5-1ubuntu0.20.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Haw Loeung (hloeung)]

Tested on a Focal VM in Canonistack.

Using current ipset:

| ubuntu@juju-87625f-hloeung-110:~$ sudo apt-get install ipset
| ...
| Get:1 http://us.archive.ubuntu.com/ubuntu focal/main amd64 libipset13 amd64 7.5-1~exp1 [53.4 kB]
| Get:2 http://us.archive.ubuntu.com/ubuntu focal/main amd64 ipset amd64 7.5-1~exp1 [29.8 kB]

| ubuntu@juju-87625f-hloeung-110:~$ time sudo ipset restore < whitelist-ipv4-2021-06-29-with-proto-nums
|
| real 1m55.870s
| user 1m3.704s
| sys 0m26.604s

Using the one in -proposed:

| Get:1 http://us.archive.ubuntu.com/ubuntu focal-proposed/main amd64 ipset amd64 7.5-1ubuntu0.20.04.1 [29.8 kB]
| Get:1 http://us.archive.ubuntu.com/ubuntu focal-proposed/main amd64 libipset13 amd64 7.5-1ubuntu0.20.04.1 [53.6 kB]

| ubuntu@juju-87625f-hloeung-110:~$ time sudo ipset restore < whitelist-ipv4-2021-06-29-with-proto-nums
|
| real 0m41.372s
| user 0m18.724s
| sys 0m16.023s

Full output - https://paste.ubuntu.com/p/w74Kjvnn84/

[Haw Loeung (hloeung)]

Hirsute verified - https://paste.ubuntu.com/p/Zs6vG7M6Jf/

| real 1m46.509s

vs.

| real 0m43.891s

Impish verified - https://paste.ubuntu.com/p/yWW4k8Jy79/

| real 1m14.348s

vs.

| real 0m27.687s

[Launchpad Janitor (janitor)]

This bug was fixed in the package ipset - 7.10-1ubuntu0.21.10.1

---------------
ipset (7.10-1ubuntu0.21.10.1) impish; urgency=medium

  * d/p/lp-1918936-{fix-p,P}arse-port-before-trying-by-service-name.patch:
    speed up numeric port adds (LP: #1918936)

 -- Christian Ehrhardt <email address hidden> Thu, 25 Nov 2021 09:47:36 +0000

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for ipset has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package ipset - 7.10-1ubuntu0.21.04.1

---------------
ipset (7.10-1ubuntu0.21.04.1) hirsute; urgency=medium

  * d/p/lp-1918936-{fix-p,P}arse-port-before-trying-by-service-name.patch:
    speed up numeric port adds (LP: #1918936)

 -- Christian Ehrhardt <email address hidden> Thu, 25 Nov 2021 09:50:47 +0000

[Launchpad Janitor (janitor)]

This bug was fixed in the package ipset - 7.5-1ubuntu0.20.04.1

---------------
ipset (7.5-1ubuntu0.20.04.1) focal; urgency=medium

  * d/p/lp-1918936-{fix-p,P}arse-port-before-trying-by-service-name.patch:
    speed up numeric port adds (LP: #1918936).

 -- Christian Ehrhardt <email address hidden> Thu, 25 Nov 2021 09:55:19 +0000