Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and improve D scripts for new message formats #171

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bnaecker
Copy link
Collaborator

@bnaecker bnaecker commented Nov 3, 2023

No description provided.

@bnaecker
Copy link
Collaborator Author

bnaecker commented Nov 3, 2023

Fixes #170

@bnaecker
Copy link
Collaborator Author

bnaecker commented Nov 3, 2023

So during this, @ahl and I discovered what appears to be DTrace bug. I initially tried to write a few if-else statements to stringify the SpResponse variant ID into a name. Something like this:

xcvr-ctl$target:::packet-received
{
    this->peer = json(copyinstr(arg0), "ok");
    this->n_bytes = arg1;

    /* Pointer to the packet payload itself. */
    this->buf = (char*) copyin(arg2, this->n_bytes);
    this->vers = this->buf[0];
    this->message_id = *(uint64_t*) (this->buf + HEADER_MESSAGE_ID_OFFSET);
    this->message_kind = this->buf[HEADER_MESSAGE_KIND_OFFSET];
    printf("Recv payload from: %s\n", this->peer);
    printf("  Raw packet:\n");
    tracemem(this->buf, 128, this->n_bytes);
    printf("  n_bytes: %d\n", this->n_bytes);
    printf("  version: %d\n", this->vers);
    printf("  msg id: %d\n", this->message_id);

    this->message_kind_str = "Unknown";
    if (this->message_kind == MESSAGE_KIND_ERROR) {
        this->message_kind_str = "Error";
    } else if (this->message_kind == MESSAGE_KIND_HOST_REQUEST) {
        this->message_kind_str = "HostRequest";
    } else if (this->message_kind == MESSAGE_KIND_SP_RESPONSE) {
        this->message_kind_str = "SpResponse";
    }
    printf("MessageBody: %s (%d)\n", this->message_kind_str, this->message_kind);
    tracemem(this->buf, 128);
}

However, I noticed that the tracemem output at the beginning of the action and end were different, despite nothing touching the printed variable this->buf. Here is the output of running on one of the tests in this repo:

bnaecker@shale : ~/transceiver-control $ pfexec ./dtrace/trace-packets.d -c './target/debug/deps/transceiver_controller-1fd5c472f0e52c63 --test-threads 1 -- --nocapture test_enable_power'

running 1 test
test controller::tests::test_enable_power ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 30 filtered out; finished in 0.00s

Sent payload to ::1
  n_bytes: 21
  version: 1
  msg id: 0
  msg kind: HostRequest (1)
Recv payload from: ::1
  Raw packet:

             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f  0123456789abcdef
         0: 01 00 00 00 00 00 00 00 00 02 03 02 03 03 00 00  ................
        10: 00 00 00 00 00 00 00 00 00 00 00 00 00           .............
  n_bytes: 29
  version: 1
  msg id: 0
MessageBody: SpResponse (2)

             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f  0123456789abcdef
         0: 02 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00  ................
        10: 02 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00  ................
        20: 02 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00  ................
        30: 02 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00  ................
        40: 02 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00  ................
        50: 02 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00  ................
        60: 02 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00  ................
        70: 02 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00  ................
Sent payload to ::1
  n_bytes: 21
  version: 1
  msg id: 1
  msg kind: HostRequest (1)
Recv payload from: ::1
  Raw packet:

             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f  0123456789abcdef
         0: 01 01 00 00 00 00 00 00 00 02 03 02 03 02 00 00  ................
        10: 00 00 00 00 00 01 00 00 00 00 00 00 00 02        ..............
  n_bytes: 30
  version: 1
  msg id: 1
MessageBody: SpResponse (2)

             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f  0123456789abcdef
         0: 02 01 00 00 00 00 00 00 02 01 00 00 00 00 00 00  ................
        10: 02 01 00 00 00 00 00 00 02 01 00 00 00 00 00 00  ................
        20: 02 01 00 00 00 00 00 00 02 01 00 00 00 00 00 00  ................
        30: 02 01 00 00 00 00 00 00 02 01 00 00 00 00 00 00  ................
        40: 02 01 00 00 00 00 00 00 02 01 00 00 00 00 00 00  ................
        50: 02 01 00 00 00 00 00 00 02 01 00 00 00 00 00 00  ................
        60: 02 01 00 00 00 00 00 00 02 01 00 00 00 00 00 00  ................
        70: 02 01 00 00 00 00 00 00 02 01 00 00 00 00 00 00  ................

At the end there, you can see the first row of octets are different in those two outputs. (The latter prints more than it should, sorry.)

After I manually desugared the if-else stuff into a bunch of separate actions (the script in this PR), everything worked fine, and this->buf was the same in both printouts. Here is Adam's attempt to show the desugaring DTrace does to the broken action:

xcvr-ctl26649:::packet-received
{
        ((self->%error) = 0x0);
}

xcvr-ctl26649:::packet-received
{
        ((this->peer) = json(copyinstr(arg0), "ok"));
        ((this->n_bytes) = arg1);
        ((this->buf) = (/* bad node 160c220, kind 6 */
)copyin(arg2, (this->n_bytes)));
        ((this->vers) = ((this->buf)[0x0]));
        ((this->message_id) = *((/* bad node 142ebd0, kind 6 */
)((this->buf) + 0x1)));
        ((this->message_kind) = ((this->buf)[0x9]));
        printf("Recv payload from: %s\n", (this->peer));
        printf("  Raw packet:\n");
        tracemem((this->buf), 0x80, (this->n_bytes));
        printf("  n_bytes: %d\n", (this->n_bytes));
        printf("  version: %d\n", (this->vers));
        printf("  msg id: %d\n", (this->message_id));
        ((this->message_kind_str) = "Unknown");
}

xcvr-ctl26649:::packet-received
/!((self->%error))/
{
        ((this->%condition_1) = (0x1 && ((this->message_kind) == 0x4d2)));
}

xcvr-ctl26649:::packet-received
/(!((self->%error)) && (this->%condition_1))/
{
        ((this->message_kind_str) = "Error");
}

xcvr-ctl26649:::packet-received
/!((self->%error))/
{
        ((this->%condition_2) = (0x1 && !((this->%condition_1))));
}

xcvr-ctl26649:::packet-received
/!((self->%error))/
{
        ((this->%condition_3) = ((this->%condition_2) && ((this->message_kind) == 0x162e)));
}

xcvr-ctl26649:::packet-received
/(!((self->%error)) && (this->%condition_3))/
{
        ((this->message_kind_str) = "HostRequest");
}

xcvr-ctl26649:::packet-received
/!((self->%error))/
{
        ((this->%condition_4) = ((this->%condition_2) && !((this->%condition_3))));
}

xcvr-ctl26649:::packet-received
/!((self->%error))/
{
        ((this->%condition_5) = ((this->%condition_4) && ((this->message_kind) == 0x2)));
}

xcvr-ctl26649:::packet-received
/(!((self->%error)) && (this->%condition_5))/
{
        ((this->message_kind_str) = "SpResponse");
}

xcvr-ctl26649:::packet-received
/!((self->%error))/
{
        printf("MessageBody: %s (%d)\n", (this->message_kind_str), (this->message_kind));
        tracemem((this->buf), 0x80);
}

That looks pretty close to what I wrote by hand, so nothing is obviously broken. We noted that every line of the wrong tracemem output is identical, and Adam is curious if something is stomping on scratch space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant