Opened 11 months ago

Closed 8 months ago

#5564 closed defect (fixed)

Kea does unload libraries late

Reported by: tomek Owned by: tmark
Priority: medium Milestone: Kea1.4
Component: hooks Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

As reported here: https://lists.isc.org/pipermail/kea-dev/2017-December/000841.html

Kea unload the hook library when handing the first incoming
message after reload by using command "keactrl reload -c configfile".
This cause hook library function cannot be called and message handling
stop.

Reason: Kea hold the hook library reference by using shared_ptr in
several place. Hook library is not correctly released when reload the
kea, But released when handing the first incoming message.

Subtickets

Attachments (1)

fix5564-unload-libs.patch (1.1 KB) - added by tomek 11 months ago.

Download all attachments as: .zip

Change History (11)

Changed 11 months ago by tomek

comment:1 Changed 11 months ago by fdupont

BTW there is an (unfortunately incorrect) PR about this.

comment:2 Changed 10 months ago by tomek

  • Milestone changed from Kea-proposed to Kea1.4

Yes, this is a real problem we need to fix.

comment:3 Changed 9 months ago by fdupont

Reinvestigate this when #5565 will be merged (IMHO they are related so the when handling the first incoming message).

comment:4 Changed 8 months ago by tmark

  • Owner set to tmark
  • Status changed from new to assigned

comment:5 Changed 8 months ago by tmark

I retested with master now that 5565 has been merged and neither it nor the requestor's patch appear to correct this issue.

comment:6 Changed 8 months ago by fdupont

IMHO you need to add in callout_handle_store.h a way to clear the whole store and call it in reconfigure and exit.

comment:7 Changed 8 months ago by tmark

  • Owner changed from tmark to UnAssigned
  • Status changed from assigned to reviewing

The reporter's patch actually does work if you test as v6 server ;). In other words,the patch clears v6 stores, not v4 stores.

I modified both kea-dhcp4 and kea-dhcp6 to call a new dumpPackets() function that clears both the handle store and parking lots, during reconfig and exit. This makes the servers explicitly clean up when they should.

The call to clear the parking lots in HookManager::unloadLibrariesInternal(), in theory, could be
removed but I left in.

ChangeLog?:

13xx.   [bug]       tmark
    Corrected an issue which caused kea-dhcp4 and kea-dhcp6 servers
    to unload their hooks libraries upon receipt of the first client
    message following a dynamic reconfigure.
    (Trac #5564, git TBD)

Ready for review.

comment:8 Changed 8 months ago by tomek

  • Owner changed from UnAssigned to tomek

comment:9 Changed 8 months ago by tomek

  • Owner changed from tomek to tmark

hooks_manager.h
clearPakingLots() and clearParkingLotsInternal should have a
comment. Added one. Please pull and review.

dhcp6_srv.cc
The dumpPackets code didn't use clearParkingLots, but instead did the
work itself. Updated.

dhcp{4,6}_srv.cc
I don't like the name dumpPackets. "Dump" can be misiterpreted
as write to somewhere. Renamed to discardPackets.

All above are fixed. Pushed my changes. Please pull and review. If you're ok with them,
please go ahead and merge.

This code doesn't have unit-tests, but given the complixity of
testing this properly I agree with your assessment to not try
implementing them.

Your changeslog looks good.

Code builds and unit-tests pass on Ubuntu 17.10.

comment:10 Changed 8 months ago by tmark

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged with git# 5111f569bd251c2a98a2e6d958e8f6b640a1802d
Added ChangeLog? entry 1401.

ticket is complete.

Note: See TracTickets for help on using tickets.