Bug 1646666
| Summary: | ipset-service fails to load ipsets with set dependencies | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Rik Theys <rik.theys> | ||||
| Component: | ipset | Assignee: | Stefano Brivio <sbrivio> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Tomas Dolezal <todoleza> | ||||
| Severity: | urgent | Docs Contact: | Marc Muehlfeld <mmuehlfe> | ||||
| Priority: | urgent | ||||||
| Version: | 7.6 | CC: | atragler, lmiksik, pasik, qe-baseos-daemons, sbrivio, todoleza, toneata, vdanek | ||||
| Target Milestone: | rc | Keywords: | Regression, ZStream | ||||
| Target Release: | 7.7 | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | ipset-7.1-1.el7 | Doc Type: | Bug Fix | ||||
| Doc Text: |
.The `ipset` service can now load sets which depends on other sets
Τhe `ipset` service saves IP sets (lists of IP addresses) in separate files. In Red Hat Enterprise Linux (RHEL) 7.6, when starting the service, each set was loaded sequentially ignoring dependencies between them. As a consequence, the service failed to load IP sets with dependencies on other sets. With this update, the `ipset` service creates first all the sets included in the saved configuration, and then adds their entries. As a result, IP sets with dependencies on other sets can now be loaded.
|
Story Points: | --- | ||||
| Clone Of: | |||||||
| : | 1647096 1647836 (view as bug list) | Environment: | |||||
| Last Closed: | 2019-08-06 13:00:07 UTC | Type: | Bug | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Bug Depends On: | |||||||
| Bug Blocks: | 1647096, 1647836, 1709731 | ||||||
| Attachments: |
|
||||||
|
Description
Rik Theys
2018-11-05 20:18:41 UTC
Created attachment 1502217 [details]
minimal patch
Hi,
I've created the attached minimal patch that seems to solve the problem.
By reverse sorting the merged file before loading, all create statements are sorted before the add statements. This way all sets are already created before they are added by sets of the list:set type.
Regards,
Rik
Thanks for reporting this, Rik. Hi, I'm confused. This bug is marked as regression and urgent, but is only targeted for 7.7? Isn't that a contradiction? This bug can (silently) break existing firewall setups upon upgrade to 7.6. It would be awesome if it could be fixed in 7.6 as well. Regards, Rik Hi Rik, (In reply to Rik Theys from comment #7) > I'm confused. This bug is marked as regression and urgent, but is only > targeted for 7.7? Isn't that a contradiction? > > This bug can (silently) break existing firewall setups upon upgrade to 7.6. > It would be awesome if it could be fixed in 7.6 as well. Engineering has internally requested a z-stream fix for 7.6. The target release does not prevent this bug from moving forward. By the way, (In reply to Rik Theys from comment #2) > I've created the attached minimal patch that seems to solve the problem. > > By reverse sorting the merged file before loading, all create statements are > sorted before the add statements. This way all sets are already created > before they are added by sets of the list:set type. Thanks for your patch. While it definitely fixes the issue, it doesn't scale too well for sets containing a huge number of entries, because of the added computational complexity given by sort(1) applied on all the entries. I benchmarked a few different solutions, and ended up with something similar, that doesn't however require sorting all the entries. Rather, we'll ensure that all the 'create' commands go before the 'add' commands, directly while merging single set files, before 'ipset restore' is performed. (In reply to Stefano Brivio from comment #11) > Thanks for your patch. While it definitely fixes the issue, it doesn't scale > too well for sets containing a huge number of entries, because of the added > computational complexity given by sort(1) applied on all the entries. I've noticed that ipset restore also takes a lot longer than before. Prior to the upgrade to 7.6 it took about 4s, now it takes 40s. > I benchmarked a few different solutions, and ended up with something > similar, that doesn't however require sorting all the entries. Rather, we'll > ensure that all the 'create' commands go before the 'add' commands, directly > while merging single set files, before 'ipset restore' is performed. Would it be possible to attach the patch that fixes it that way to this bug report? Mvg, Rik (In reply to Rik Theys from comment #12) > Would it be possible to attach the patch that fixes it that way to this bug > report? The proposed solution would be something on the lines of: diff --git a/ipset.start-stop b/ipset.start-stop index 6c84bd3..61178c1 100644 --- a/ipset.start-stop +++ b/ipset.start-stop @@ -104,9 +104,16 @@ load() { chmod 600 "${merged}" set +f if [ -d ${IPSET_DATA_DIR} ]; then + # Copy the first lines of each saved set first, as they create + # the sets, then the rest: list:set entries depend on other + # sets, so make sure they all get created first for f in ${IPSET_DATA_DIR}/*; do [ "${f}" = "${IPSET_DATA_DIR}/*" ] && break - cat "${f}" >> ${merged} + head -n1 "${f}" >> ${merged} + done + for f in ${IPSET_DATA_DIR}/*; do + [ "${f}" = "${IPSET_DATA_DIR}/*" ] && break + tail -n +2 "${f}" >> ${merged} done fi set -f Please mind that this is not tested and will not necessarily be the final fix. MfG, Stefano Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2019:2158 |