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

Support IP comparison #2248

Closed
xh4n3 opened this issue May 30, 2022 · 5 comments
Closed

Support IP comparison #2248

xh4n3 opened this issue May 30, 2022 · 5 comments
Labels
enhancement New feature or request, changes on existing features

Comments

@xh4n3
Copy link
Contributor

xh4n3 commented May 30, 2022

Is your feature request related to a problem? Please describe.

One example:

kfunc:dev_queue_xmit {
        $skb = args->skb;
        $iph = (struct iphdr *)($skb->head + $skb->network_header);
        $sip = ntop($iph->saddr);
        // I want to filter skbs by sip here.
        printf("%-8d %14s\n", pid, $sip);
}

This script may output a lot skbs, IP filtering should be added here.

Workaround 1

        if (strncmp($sip, "127.0.0.1", 9)) {
                return;
        }

This prints error message: ERROR: strncmp() only supports string arguments (inet provided)

Workaround 2

        if (strncmp(str($sip), "127.0.0.1", 9)) {
                return;
        }

This prints error message: ERROR: str() expects an integer or a pointer type as first argument (inet provided)

Workaround 3

        if ($iph->saddr == 0x100007f ) {
                return;
        }

This actually worked, but if the script isn't very human-readable.

Describe the solution you'd like

Solution 1: Add a new built-in func like pton, so we can compare numbers

        if ($iph->saddr == pton(AF_INET, "127.0.0.1)) {
                return;
        }

Then this script can be hand over to anyone unfamiliar with hex to dig into network issues, IMHO, this would work great with the command line arguments #1945

Describe alternative solutions or features you've considered

Somehow fixing the case in workaround 1/2, but strncmp takes a length argument, which would be hard to determine.

@xh4n3 xh4n3 added the enhancement New feature or request, changes on existing features label May 30, 2022
@viktormalik
Copy link
Contributor

This would be a nice feature, thanks for the suggestion!

I think that the solution using pton is the only one possible here as the ntop string is resolved asynchronously, making it impossible to be compared inside the BPF program (using e.g. strncmp).

@xh4n3
Copy link
Contributor Author

xh4n3 commented May 30, 2022

This would be a nice feature, thanks for the suggestion!

May I draft a PR regarding this? 😃

@viktormalik
Copy link
Contributor

May I draft a PR regarding this? smiley

Yes, please do, that would be great :-)

@xh4n3
Copy link
Contributor Author

xh4n3 commented Jun 2, 2022

I'm able to convert string-format IPv6 address to char array now, but I think we may need the ability to compare char array in bpftrace language, otherwise we can only compare them char by char, or compare them stringified.

What I'm having now:

tracepoint:tcp:tcp_retransmit_skb {
  printf("skb daddr %s\n", ntop(AF_INET6, args->daddr_v6));

  // convert back and forth
  printf("filter daddr %s\n", ntop(AF_INET6, pton(AF_INET6, "2001::1f0d:4435")));

  // store the string as char addr[16]
  $addr = pton(AF_INET6, "2001::1f0d:4435");

  // access the char in the char array
  printf("char 0 %x\n", $addr[0]);

  // compare chars one by one between skb's daddr and filter daddr
  if (args->daddr_v6[0] == $addr[0]) {
    printf("partial matched\n");
  }

  // compare string
  if (strncmp(str(args->daddr_v6), str($addr), 16)==0) {
    printf("string matched\n");
  }

  // * cannot compare char array directly
  // if (args->daddr_v6 == $addr) {
  //   printf("all matched\n");
  // }
  return;
}

There is a working branch on https://github.com/xh4n3/bpftrace/commits/features/add-pton-builtin

I have some ideas, which one do you recommend? @viktormalik

  • compare them stringified is good enough?
  • implementing comparing on char arrays, (btw, how should we determine its length?)
  • add a new built-in arrayncmp like strncmp

@xh4n3
Copy link
Contributor Author

xh4n3 commented Jan 4, 2023

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request, changes on existing features
Projects
None yet
Development

No branches or pull requests

2 participants