Opened 9 years ago

Closed 9 years ago

#454 closed task (fixed)

Zone cut handling in MemoryZone

Reported by: jinmei Owned by: jinmei
Priority: medium Milestone: y2 12 month milestone
Component: data source Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Copied from the sprint page:

This is not trivial because we need to separate the delegation case and the glue case for names at or under a zone cut (see the review discussion for trac #447). What BIND 9 does is as follows:

  • the RBT is created with a special callback function
  • when loading a zone, mark a zone cut node (detecting it by seeing a DNAME or non-origin NS RR) to indicate the need for calling the callback
  • when finding an RRset (or rdataset in the BIND 9 terminology), if the rbt search reaches a marked node, it calls the callback function. The callback searches the node for NS or DNAME, and if found, records the fact in a search state.
  • if the RBT search is performed in the normal (= not "GLUE OK") mode, and if the callback found a delegation RR, the search stops at that node
  • the "zone find" function (that performed the RBT search) recognizes the callback is called, and simply returns the callback result with the code of "DELEGATION"
  • on the other hand, if we need to find a glue A/AAAA RRs, the "zone find" function performs the RBT search in the "GLUE OK" mode. In that mode, even if the search encounters a callback node, the search is continued for a best match. If an exact match node is found and it has the requested type of RRs, these RRs will be returned; otherwise the found delegation will be returned.

If we can come up with a better idea quickly we can do that. Otherwise, we'll port this logic for our initial implementation. I'd also separate the "GLUE OK" case and pure delegation case into separate sub tasks to keep the amount of development/review minimal.

Subtickets

Attachments (1)

zonecut.diff (28.9 KB) - added by jinmei 9 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 9 years ago by jinmei

I've been working on it based on branches/trac447. It's mostly ready for review, simply waiting for the completion of #447.

For the record I'm attaching the current diff.

Changed 9 years ago by jinmei

comment:2 follow-up: Changed 9 years ago by vorner

  • Status changed from new to assigned

Few minor points:

The compiler warned about possibly uninitialized variable „node“ in find. As I wanted to run the test, I took the liberty of fixing it, so I put it to the repository as well.

+        return (false);         // note: right now this case is impossible.

If it is impossible, shouldn't it be assert, just to ensure that the impossible doesn't happen?

+    /// \param node On success (either \c EXAMPLE or \c PARTIALMATCH) it will

Shouldn't it be EXACTMATCH?

+    // shouldn't confuse the apex node (having NS) with delegation
+    // test to be added
+    findTest(origin_, RRType::NS(), Zone::SUCCESS, rr_ns_)

I don't really understand the comment. What test should be added? Like something more than just the findTest(…) right there?

comment:3 in reply to: ↑ 2 ; follow-up: Changed 9 years ago by jinmei

  • Owner changed from jinmei to vorner

Replying to vorner:

Few minor points:

The compiler warned about possibly uninitialized variable „node“ in find. As I wanted to run the test, I took the liberty of fixing it, so I put it to the repository as well.

That's fine, thanks.

+        return (false);         // note: right now this case is impossible.

If it is impossible, shouldn't it be assert, just to ensure that the impossible doesn't happen?

In this case I think we can leave it as is. This will soon be updated in the support for the 'glue OK' mode, which I'll work on next.

+    /// \param node On success (either \c EXAMPLE or \c PARTIALMATCH) it will

Shouldn't it be EXACTMATCH?

Ah, that's right. Good catch. Fixed (r4084).

+    // shouldn't confuse the apex node (having NS) with delegation
+    // test to be added
+    findTest(origin_, RRType::NS(), Zone::SUCCESS, rr_ns_)

I don't really understand the comment. What test should be added? Like something more than just the findTest(…) right there?

Hmm, I don't understand the comment either:-) What about this?

    // finding NS for the apex (origin) node.  This must not be confused
    // with delegation due to the existence of an NS RR.
    findTest(origin_, RRType::NS(), Zone::SUCCESS, rr_ns_);

comment:4 in reply to: ↑ 3 Changed 9 years ago by jinmei

Replying to jinmei:

+        return (false);         // note: right now this case is impossible.

If it is impossible, shouldn't it be assert, just to ensure that the impossible doesn't happen?

In this case I think we can leave it as is. This will soon be updated in the support for the 'glue OK' mode, which I'll work on next.

On a closer look, I changed my mind. This shouldn't happen regardless of the existence of the 'glue OK' mode. I modified the code using assert() in r4086.

I don't really understand the comment. What test should be added? Like something more than just the findTest(…) right there?

Hmm, I don't understand the comment either:-) What about this?

    // finding NS for the apex (origin) node.  This must not be confused
    // with delegation due to the existence of an NS RR.
    findTest(origin_, RRType::NS(), Zone::SUCCESS, rr_ns_);

The proposed revised comment was committed in r4085.

comment:5 follow-up: Changed 9 years ago by vorner

  • Owner changed from vorner to jinmei
  • Status changed from assigned to reviewing

Seems OK. Please feel free to merge.

comment:6 in reply to: ↑ 5 Changed 9 years ago by jinmei

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

Replying to vorner:

Seems OK. Please feel free to merge.

Okay, thanks. Committed. Closing ticket.

Note: See TracTickets for help on using tickets.