Re: [iovisor-dev] Contributing example code

2018-05-08 Thread Y Song via iovisor-dev
On Tue, May 8, 2018 at 4:17 AM, Muhammad Usman  wrote:
> Dear Song,
>
> While I am making my code ready for the contribution, can you share details
> about standard workflow steps to upload code on IO-Visor GitHub repository?

Here is the github information about how to create a pull request.
https://help.github.com/articles/creating-a-pull-request/
Basically you create a pull request against iovisor/bcc repository
for review and merge.

Thanks.

>
> Thanks
>
> --
>
> Regards
>
> Muhammad Usman
> Application Engineer
> LMK Resources (pvt) Limited
> www.lmkr.com
> +92 (323) 5599 068
>
> On Sat, May 5, 2018 at 12:36 AM, Y Song  wrote:
>>
>> On Fri, May 4, 2018 at 3:19 AM, Muhammad Usman  wrote:
>> > Dear Song,
>> >
>> > Thanks for your reply.
>> >
>> > The code I am willing to contribute is self contained and there are no
>> > additional requirements.
>>
>> Then it should be okay. Thanks for your contribution!
>>
>> Yonghong
>>
>> >
>> > I checked in /bcc/examples/networking there are different example codes.
>> > My
>> > code is more like extension of HTTP filter example but rather than
>> > looking
>> > into application header it's about extracting VXLAN headers and
>> > incorporated
>> > VLAN and respective header fields. Then by using Kafka, delivering
>> > extracted
>> > fields to the central server for additional actions like traffic
>> > visualization.
>> >
>> > I hope this code would be helpful to a lot of people who want to monitor
>> > VM
>> > traffic on data planes across data centers.
>> >
>> > Let me know if you have any further comments.
>> >
>> > --
>> >
>> > Regards
>> >
>> > Muhammad Usman
>> > Application Engineer
>> > LMK Resources (pvt) Limited
>> > www.lmkr.com
>> > +92 (323) 5599 068
>> >
>> > On Wed, May 2, 2018 at 3:55 PM, Y Song  wrote:
>> >>
>> >> On Tue, May 1, 2018 at 4:48 AM, Muhammad Usman via iovisor-dev
>> >>  wrote:
>> >> > Hello,
>> >> >
>> >> > My name is Usman.
>> >> >
>> >> > I am interested to contribute an example code for OpenStack VLAN
>> >> > traffic
>> >> > monitoring.
>> >>
>> >> Any details of exactly what you want to minotor? Just sniff the packet
>> >> and
>> >> if it is vlan do some accounting in map?
>> >>
>> >> The example code should be self contained so people can just type a
>> >> simple
>> >> command and run. It is not a good idea to have an open stack
>> >> dependence.
>> >> You may have to work through it.
>> >>
>> >> >
>> >> > Regarding this can you please guide me how I can submit my code on
>> >> > GitHub
>> >> > (Since I could not find details on iovisor GitHub page).
>> >>
>> >> We already have some networking examples at bcc/examples/networking.
>> >> In general, an example is used to show how to solve a particular
>> >> problem
>> >> using bcc APIs. Please first check whether bcc/examples/networking has
>> >> anything similar or not.
>> >>
>> >> >
>> >> > I would appreciate your time and consideration.
>> >> >
>> >> > Thanks
>> >> >
>> >> > --
>> >> >
>> >> > Regards
>> >> >
>> >> > Muhammad Usman
>> >> > Application Engineer
>> >> > LMK Resources (pvt) Limited
>> >> > www.lmkr.com
>> >> > +92 (323) 5599 068
>> >> >
>> >> > ___
>> >> > iovisor-dev mailing list
>> >> > iovisor-dev@lists.iovisor.org
>> >> > https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>> >> >
>> >
>> >
>
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Contributing example code

2018-05-04 Thread Y Song via iovisor-dev
On Fri, May 4, 2018 at 3:19 AM, Muhammad Usman  wrote:
> Dear Song,
>
> Thanks for your reply.
>
> The code I am willing to contribute is self contained and there are no
> additional requirements.

Then it should be okay. Thanks for your contribution!

Yonghong

>
> I checked in /bcc/examples/networking there are different example codes. My
> code is more like extension of HTTP filter example but rather than looking
> into application header it's about extracting VXLAN headers and incorporated
> VLAN and respective header fields. Then by using Kafka, delivering extracted
> fields to the central server for additional actions like traffic
> visualization.
>
> I hope this code would be helpful to a lot of people who want to monitor VM
> traffic on data planes across data centers.
>
> Let me know if you have any further comments.
>
> --
>
> Regards
>
> Muhammad Usman
> Application Engineer
> LMK Resources (pvt) Limited
> www.lmkr.com
> +92 (323) 5599 068
>
> On Wed, May 2, 2018 at 3:55 PM, Y Song  wrote:
>>
>> On Tue, May 1, 2018 at 4:48 AM, Muhammad Usman via iovisor-dev
>>  wrote:
>> > Hello,
>> >
>> > My name is Usman.
>> >
>> > I am interested to contribute an example code for OpenStack VLAN traffic
>> > monitoring.
>>
>> Any details of exactly what you want to minotor? Just sniff the packet and
>> if it is vlan do some accounting in map?
>>
>> The example code should be self contained so people can just type a simple
>> command and run. It is not a good idea to have an open stack dependence.
>> You may have to work through it.
>>
>> >
>> > Regarding this can you please guide me how I can submit my code on
>> > GitHub
>> > (Since I could not find details on iovisor GitHub page).
>>
>> We already have some networking examples at bcc/examples/networking.
>> In general, an example is used to show how to solve a particular problem
>> using bcc APIs. Please first check whether bcc/examples/networking has
>> anything similar or not.
>>
>> >
>> > I would appreciate your time and consideration.
>> >
>> > Thanks
>> >
>> > --
>> >
>> > Regards
>> >
>> > Muhammad Usman
>> > Application Engineer
>> > LMK Resources (pvt) Limited
>> > www.lmkr.com
>> > +92 (323) 5599 068
>> >
>> > ___
>> > iovisor-dev mailing list
>> > iovisor-dev@lists.iovisor.org
>> > https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>> >
>
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] minutes: IO Visor TSC / Dev Meeting

2018-05-02 Thread Y Song via iovisor-dev
On Wed, May 2, 2018 at 4:39 PM, Brenden Blanco via iovisor-dev
 wrote:
> Hi All,
>
> Thanks for joining the discussion today. As usual, here are my notes.
>
> -Brenden
>
> === Discussion ===
>
> John:
> - sockmap fixes pushed
>   - fixed broken tests
> - sockmap hash impl next
> - bounded loop rfc making progress
> - cilium code using sockhash almost finished as well
>
> Yonghong:
> - python3 test cleanup
>   - except memleak tool

This is the test_tools_memleak issue I mentioned.
https://github.com/iovisor/bcc/issues/1685

I would be great if somebody who knows python (esp. python3) better
can help take a look.
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Contributing example code

2018-05-02 Thread Y Song via iovisor-dev
On Tue, May 1, 2018 at 4:48 AM, Muhammad Usman via iovisor-dev
 wrote:
> Hello,
>
> My name is Usman.
>
> I am interested to contribute an example code for OpenStack VLAN traffic
> monitoring.

Any details of exactly what you want to minotor? Just sniff the packet and
if it is vlan do some accounting in map?

The example code should be self contained so people can just type a simple
command and run. It is not a good idea to have an open stack dependence.
You may have to work through it.

>
> Regarding this can you please guide me how I can submit my code on GitHub
> (Since I could not find details on iovisor GitHub page).

We already have some networking examples at bcc/examples/networking.
In general, an example is used to show how to solve a particular problem
using bcc APIs. Please first check whether bcc/examples/networking has
anything similar or not.

>
> I would appreciate your time and consideration.
>
> Thanks
>
> --
>
> Regards
>
> Muhammad Usman
> Application Engineer
> LMK Resources (pvt) Limited
> www.lmkr.com
> +92 (323) 5599 068
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] BPF verifier: ctx+const+const is not allowed

2018-04-29 Thread Y Song via iovisor-dev
On Sat, Apr 28, 2018 at 12:00 PM, Y Song  wrote:
> Hi, William,
>
> Do you have a test (just compile and load) to demonstrate the problem?
> I would like to understand why the compiler generates r1+0 and whether
> we could possibly avoid it.
>
> Thanks,
> Yonghong
>
>
> On Sat, Apr 28, 2018 at 7:57 AM, William Tu via iovisor-dev
>  wrote:
>> Hi,
>>
>> We're hitting a BPF verifier error saying
>> "dereference of modified ctx ptr R1 off=36+0, ctx+const is allowed,
>> ctx+const+const is not
>> "
>> but actually the 2nd const is 0. I don't know why compiler generate (r1+0)
>> in this case:
>> 602: (61) r4 = *(u32 *)(r1 +0)
>>
>> verifier log
>> 
>> 594: (15) if r6 == 0x0 goto pc+8
>>  R0=inv(id=0) R1=inv7161128523638600565
>> R3=inv(id=0,umax_value=65535,var_off=(0x0; 0x)) R4=inv0
>> R6=ctx(id=0,off=0,imm=0) R7=inv2
>> R8=map_value(id=0,off=0,ks=232,vs=4352,imm=0) R9=inv(id=0)
>> R10=fp0,call_-1 fp-352=map_value fp-360=ctx
>> 595: (bf) r1 = r6
>> 596: (07) r1 += 40
>> 597: (61) r2 = *(u32 *)(r6 +52)
>> 598: (b7) r4 = 0
>> 599: (1d) if r2 == r4 goto pc+2
>>  R0=inv(id=0) R1=ctx(id=0,off=40,imm=0)
>> R2=inv(id=0,umax_value=4294967295,var_off=(0x0; 0x))
>> R3=inv(id=0,umax_value=65535,var_off=(0x0; 0x)) R4=inv0
>> R6=ctx(id=0,off=0,imm=0) R7=inv2
>> R8=map_value(id=0,off=0,ks=232,vs=4352,imm=0) R9=inv(id=0)
>> R10=fp0,call_-1 fp-352=map_value fp-360=ctx
>> 600: (bf) r1 = r6
>> 601: (07) r1 += 36   >
>> r1 has offset 36
>> 602: (61) r4 = *(u32 *)(r1 +0)   ->
>> then r1 + 0
>> dereference of modified ctx ptr R1 off=36+0, ctx+const is allowed,
>> ctx+const+const is not
>>
>>
>> llvm-objdump
>> ==
>> ; if (!skb)
>>  594:   if r6 == 0 goto 8
>> ; return skb->ifindex;
>>  595:   r1 = r6
>>  596:   r1 += 40
>> ; if (skb->cb[OVS_CB_INGRESS]) {
>>  597:   r2 = *(u32 *)(r6 + 52)
>>  598:   r4 = 0
>>  599:   if r2 == r4 goto 2
>> ; return skb->ingress_ifindex;
>>  600:   r1 = r6
>>  601:   r1 += 36
>>
>> LBB1_28:
>>  602:   r4 = *(u32 *)(r1 + 0)


Now I remembered that we had this issue before in bcc.
it is a compiler optimization likes this:
if (...)
*(ctx + 60)
else
*(ctx + 56)

The compiler translates it to
if (...)
   ptr = ctx + 60
else
   ptr = ctx + 56
*(ptr + 0)

Yes, this *(ptr + 0) will cause problem in verifier
as verifier only allows direct "ctx + offset" offset access.

Since in bcc this happens in auto generated code, we put the barrier
after the original
ctx memory assess

if (...) {
*(ctx + 60)
__asm__ __volatile__(\"\": : :\"memory\");
} else {
*(ctx + 56)
__asm__ __volatile__(\"\": : :\"memory\");
}

Should we fix this in verifier? Probably as adding these compiler
barriers indeed very annoying.


>>
>> With the below patch fix the issue
>> ==
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -1619,7 +1619,7 @@ static int check_mem_access(struct
>> bpf_verifier_env *env, int insn_idx, u32 regn
>> /* ctx accesses must be at a fixed offset, so that we can
>>  * determine what type of data were returned.
>>  */
>> -   if (reg->off) {
>> +   if (reg->off && reg->off != off) {
>> verbose(env,
>> "dereference of modified ctx ptr R%d
>> off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
>> regno, reg->off, off - reg->off);
>>

You change looks okay. When you send upstream patch,
you may want to add some context about
when this case can happen, what workaround you could have
and why this change is preferable.

>> Is this the right fix?
>> Thanks
>> William
>> ___
>> iovisor-dev mailing list
>> iovisor-dev@lists.iovisor.org
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] BPF verifier: ctx+const+const is not allowed

2018-04-28 Thread Y Song via iovisor-dev
Hi, William,

Do you have a test (just compile and load) to demonstrate the problem?
I would like to understand why the compiler generates r1+0 and whether
we could possibly avoid it.

Thanks,
Yonghong


On Sat, Apr 28, 2018 at 7:57 AM, William Tu via iovisor-dev
 wrote:
> Hi,
>
> We're hitting a BPF verifier error saying
> "dereference of modified ctx ptr R1 off=36+0, ctx+const is allowed,
> ctx+const+const is not
> "
> but actually the 2nd const is 0. I don't know why compiler generate (r1+0)
> in this case:
> 602: (61) r4 = *(u32 *)(r1 +0)
>
> verifier log
> 
> 594: (15) if r6 == 0x0 goto pc+8
>  R0=inv(id=0) R1=inv7161128523638600565
> R3=inv(id=0,umax_value=65535,var_off=(0x0; 0x)) R4=inv0
> R6=ctx(id=0,off=0,imm=0) R7=inv2
> R8=map_value(id=0,off=0,ks=232,vs=4352,imm=0) R9=inv(id=0)
> R10=fp0,call_-1 fp-352=map_value fp-360=ctx
> 595: (bf) r1 = r6
> 596: (07) r1 += 40
> 597: (61) r2 = *(u32 *)(r6 +52)
> 598: (b7) r4 = 0
> 599: (1d) if r2 == r4 goto pc+2
>  R0=inv(id=0) R1=ctx(id=0,off=40,imm=0)
> R2=inv(id=0,umax_value=4294967295,var_off=(0x0; 0x))
> R3=inv(id=0,umax_value=65535,var_off=(0x0; 0x)) R4=inv0
> R6=ctx(id=0,off=0,imm=0) R7=inv2
> R8=map_value(id=0,off=0,ks=232,vs=4352,imm=0) R9=inv(id=0)
> R10=fp0,call_-1 fp-352=map_value fp-360=ctx
> 600: (bf) r1 = r6
> 601: (07) r1 += 36   >
> r1 has offset 36
> 602: (61) r4 = *(u32 *)(r1 +0)   ->
> then r1 + 0
> dereference of modified ctx ptr R1 off=36+0, ctx+const is allowed,
> ctx+const+const is not
>
>
> llvm-objdump
> ==
> ; if (!skb)
>  594:   if r6 == 0 goto 8
> ; return skb->ifindex;
>  595:   r1 = r6
>  596:   r1 += 40
> ; if (skb->cb[OVS_CB_INGRESS]) {
>  597:   r2 = *(u32 *)(r6 + 52)
>  598:   r4 = 0
>  599:   if r2 == r4 goto 2
> ; return skb->ingress_ifindex;
>  600:   r1 = r6
>  601:   r1 += 36
>
> LBB1_28:
>  602:   r4 = *(u32 *)(r1 + 0)
>
>
> With the below patch fix the issue
> ==
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1619,7 +1619,7 @@ static int check_mem_access(struct
> bpf_verifier_env *env, int insn_idx, u32 regn
> /* ctx accesses must be at a fixed offset, so that we can
>  * determine what type of data were returned.
>  */
> -   if (reg->off) {
> +   if (reg->off && reg->off != off) {
> verbose(env,
> "dereference of modified ctx ptr R%d
> off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
> regno, reg->off, off - reg->off);
>
>
> Is this the right fix?
> Thanks
> William
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] bps helper functions

2018-04-07 Thread Y Song via iovisor-dev
Ashish,

Some comments below.

On Sat, Apr 7, 2018 at 1:44 PM, riya khanna  wrote:
> 
>
> On Mon, Apr 2, 2018 at 1:30 PM, riya khanna 
> wrote:
>>
>> Oh its simply to inspect/filter/drop a compressed application packet in
>> the kernel without incurring network stack processing.

Could you have more details here? Maybe have a skeleton of bpf program
to at least see how this helper will be used? A lot of compression
happens in the application
and kernel chops them into TCP stream. Or you are talking IPcomp protocol?
I am not sure how bpf program could do decompression based on a single packet.
Further, do we need todecompress the whole packet or on ly part of the packet?

Yonghong

>>
>> On Mon, Apr 2, 2018 at 11:36 AM, Y Song  wrote:
>>>
>>> Hi, Ashish,
>>>
>>> It seems that this has not been actively discussed before. Could you
>>> describe your use case?
>>>
>>> Thanks!
>>>
>>> Yonghong
>>>
>>> On Mon, Apr 2, 2018 at 8:11 AM, riya khanna via iovisor-dev
>>>  wrote:
>>> > Dear developers,
>>> >
>>> > Is it a good idea to export kernel compression/decompression
>>> > (lib/zlib_*,
>>> > lib/lz*) as helper functions to enable corresponding use cases? Has
>>> > this
>>> > been tried before?
>>> >
>>> > Thanks,
>>> > Ashish
>>> >
>>> > ___
>>> > iovisor-dev mailing list
>>> > iovisor-dev@lists.iovisor.org
>>> > https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>>> >
>>
>>
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] bps helper functions

2018-04-02 Thread Y Song via iovisor-dev
Hi, Ashish,

It seems that this has not been actively discussed before. Could you
describe your use case?

Thanks!

Yonghong

On Mon, Apr 2, 2018 at 8:11 AM, riya khanna via iovisor-dev
 wrote:
> Dear developers,
>
> Is it a good idea to export kernel compression/decompression (lib/zlib_*,
> lib/lz*) as helper functions to enable corresponding use cases? Has this
> been tried before?
>
> Thanks,
> Ashish
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] trace_printk

2018-03-28 Thread Y Song via iovisor-dev
No, this can still be used. The warning just tells you this should
mostly be used in debug mode.

On Wed, Mar 28, 2018 at 11:59 AM, Saran Kumar Krishnan via iovisor-dev
 wrote:
> Hi -
>
>
> When I use bpf_trace_printk - I am getting this NOTICE. Does it mean, that
> eBPF shouldn't be used in the production kernel.
>
>
>
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467518]
> **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467518] **   NOTICE NOTICE NOTICE
> NOTICE NOTICE NOTICE NOTICE   **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467518] **
> **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467519] ** trace_printk() being
> used. Allocating extra memory.  **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467519] **
> **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467520] ** This means that this
> is a DEBUG kernel and it is **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467520] ** unsafe for production
> use.   **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467520] **
> **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467521] ** If you see this
> message and you are not debugging**
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467522] ** the kernel, report
> this immediately to your vendor!  **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467522] **
> **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467522] **   NOTICE NOTICE NOTICE
> NOTICE NOTICE NOTICE NOTICE   **
> Mar 20 12:26:44 lenovo-e565 kernel: [  267.467523]
> **
>
>
>
> Regards
> Saran
>
>
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] ask for help to build bcc on EL7 with kernel oracle uek5

2018-03-13 Thread Y Song via iovisor-dev
On Tue, Mar 13, 2018 at 8:56 AM, Suse Shi via iovisor-dev
 wrote:
> env. EL7
> kernel  4.14.23-1.el7uek.x86_64
> http://yum.oracle.com/repo/OracleLinux/OL7/developer_UEKR5/x86_64/index.html
>
> with llvm
> llvm-toolset-7-libomp-4.0.1-1.el7.x86_64
> llvm-toolset-7-llvm-libs-4.0.1-3.el7.x86_64
> llvm-toolset-7-clang-libs-4.0.1-1.el7.x86_64
> llvm-toolset-7-llvm-4.0.1-3.el7.x86_64
> llvm-toolset-7-llvm-devel-4.0.1-3.el7.x86_64
> llvm-toolset-7-clang-4.0.1-1.el7.x86_64
> llvm-toolset-7-compiler-rt-4.0.1-1.el7.x86_64
> llvm-toolset-7-llvm-static-4.0.1-3.el7.x86_64
> llvm-toolset-7-clang-devel-4.0.1-1.el7.x86_64
> llvm-toolset-7-runtime-4.0.1-2.el7.x86_64
>
>
> err msg:
> $make
> [  3%] Built target bpf-shared
> [  9%] Built target clang_frontend
> [ 11%] Built target bpf-static
> [ 14%] Built target api-static
> [ 17%] Built target usdt-static
> [ 27%] Built target b_frontend
> [ 41%] Built target bcc-shared
> [ 48%] Built target bcc-loader-static
> [ 61%] Built target bcc-lua-static
> [ 70%] Built target bcc-static
> [ 71%] Built target bcc_py
> [ 74%] Built target bcc-lua
> [ 76%] Built target bps
> [ 78%] Built target FollyRequestContextSwitch
> [ 79%] Built target CPUDistribution
> [ 81%] Built target LLCStat
> [ 83%] Built target HelloWorld
> [ 85%] Built target RecordMySQLQuery
> [ 86%] Built target TCPSendStack
> [ 88%] Built target RandomRead
> [ 89%] Linking CXX executable test_libbcc
> CMakeFiles/test_libbcc.dir/test_usdt_probes.cc.o: In function
> `C_A_T_C_HT_E_S_T37()':
> test_usdt_probes.cc:(.text+0x7ffc): undefined reference to
> `__dtrace_libbcc_test___sample_probe_1(unsigned long, unsigned long)'
> collect2: error: ld returned 1 exit status
> make[2]: *** [tests/cc/CMakeFiles/test_libbcc.dir/build.make:394:
> tests/cc/test_libbcc] Error 1
> make[1]: *** [CMakeFiles/Makefile2:1398:
> tests/cc/CMakeFiles/test_libbcc.dir/all] Error 2
> make: *** [Makefile:139: all] Error 2

This should be related to your /usr/include/sys/sdt.h
That file probably introduced this additional symbol
__dtrace_libbcc_test___sample_probe_1.
Maybe you can check whether additional macro definition needed to avoid this.


>
> 
> any help is welcome, thanks.
>
> --
> Regards,
> -suse
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Failing bcc tests on xenial with 4.13 kernel

2018-03-07 Thread Y Song via iovisor-dev
Your workaround looks good. Basically, you can put the workaround in
for 4.13 for any tools which references
task_struct construct.

The bcc issue https://github.com/iovisor/bcc/issues/1492 has more details.
It is an issue for 4.13.
it is an issue for 4.14 original release, but the patch is ported to
stable release, so later 4.14 update should contain the fix.
4.15 and later contains the fix.

On Wed, Mar 7, 2018 at 10:34 PM, Mike Percy  wrote:
> Y Song, thank you very much for your detailed feedback.
>
> Let me focus on the specific wakeuptime.py failure since that seems to be my
> biggest blocker at the moment:
>
>> >   File "../../tools/wakeuptime.py", line 216, in 
>> > print("%-16s %s" % ("target:", k.target.decode()))
>> > UnicodeDecodeError: 'ascii' codec can't decode byte 0xe0 in position 0:
>> > ordinal not in range(128)
>>
>> Maybe this is a real issue related to python 2/3 compatibility, could
>> you help take a look?
>
>
> It looks like this is a known issue. According to
> https://github.com/iovisor/bcc/blob/master/docs/reference_guide.md#7-bpf_get_current_task
> :
>
>> With Linux 4.13, due to issues with field randomization, you may need two
>> #define directives before the includes:
>> #define randomized_struct_fields_start  struct {
>> #define randomized_struct_fields_end};
>> #include 
>> int do_trace(void *ctx) {
>> struct task_struct *t = (struct task_struct *)bpf_get_current_task();
>> [...]
>
>
> And indeed that fixed the wakeuptime tool. For now, this patch seems to do
> the trick on this kernel (the undefs just get rid of some warnings):
>
> diff --git a/tools/wakeuptime.py b/tools/wakeuptime.py
> index cf0ca7d..3f6fd67 100755
> --- a/tools/wakeuptime.py
> +++ b/tools/wakeuptime.py
> @@ -86,6 +86,10 @@ def signal_ignore(signal, frame):
>
>  # define BPF program
>  bpf_text = """
> +#undef randomized_struct_fields_start
> +#undef randomized_struct_fields_end
> +#define randomized_struct_fields_start  struct {
> +#define randomized_struct_fields_end};
>  #include 
>  #include 
>
> It appears that same workaround is also needed for many other tools in the
> repo to work correctly on this kernel.
>
> I was looking around at the kernel 4.13 release notes and I found the
> RANDSTRUCT patch
> 
> but I'm not sure that is what I'm hitting here since I didn't find a
> reference to it in my kernel config. Any ideas about the root cause for this
> workaround and if this is a permanent issue with kernels after 4.13? If so,
> then I suppose we should find a more maintainable way of injecting that
> workaround automatically in the BPF class, or find a more permanent
> workaround for bcc.
>
> Thoughts?
>
> Thanks again,
> Mike
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] handle pool of elements in eBPF maps

2018-02-28 Thread Y Song via iovisor-dev
On Wed, Feb 28, 2018 at 2:24 PM, Mauricio Vasquez via iovisor-dev
 wrote:
> Hello Teng,
>
> (Adding the list as dropped in last message)
>
> On 02/28/2018 02:37 AM, Teng Qin wrote:
>
>
>
> On Tue, Feb 27, 2018 at 12:27 PM, Mauricio Vasquez via iovisor-dev
>  wrote:
>>
>> Dear All,
>>
>> We know from our experience implementing network functions in eBPF that
>> some services require to keep pool of elements, for example addresses and
>> ports in a NAT.  So far we haven't found a way to do it entirely in eBPF, we
>> have implemented some workarounds as described in [1] (use an array map and
>> a counter for example), we also have moved this logic into user space for
>> some applications, however none of these solutions fulfill our requirements.
>>
>> We want to bring the discussion of a possible extension to eBPF maps, we
>> think the right way to go is to have a map that supports the push and pop
>> methods.
>
>
> I think we could (kind of) simulate a stack-like data structure now, by
> using
> a normal BPF array as storage, along with another global variable
> (i.e., array of size 1) to keep track of the stack top index, and inc / dec
> it
> on push / pop. There could be concurrency issues, so maybe using per-CPU
> version
> of those?
>
>
> Unfortunately percpu would not work, the set of elements has to be shared
> across all cpus.
> We could think about using a synchronization primitive to avoid potential
> problems, however that synchronization should also be available from user
> space, because in our use case the eBPF programs are consumers while an
> application in userspace is the producer.
>
>
> I agree it feels complicated and error-prone, a native stack / queue map
> type
> would definitely make such use case nicer.
>
>
> If there is consensus about this map, we could propose an implementation.

If a particular map (e.g., LRU map) serves your general need, and you only need
two more map operations (e.g., pop and push), you can just add two new
operations
for that map. if this is only available from BPF programs, you do not
even need to
introduce a sub command.

I think if you can prototype this, it will be great for people to see
your needs and
provide suggestions.

>
> /Mauricio.
>
>
>>
>>
>> Any thoughts on this?
>>
>> Thanks,
>>
>> Mauricio
>>
>> [1]
>> https://lists.iovisor.org/pipermail/iovisor-dev/2017-January/000614.html
>>
>> ___
>> iovisor-dev mailing list
>> iovisor-dev@lists.iovisor.org
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
>
>
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Notification when an eBPF map is modified

2018-02-16 Thread Y Song via iovisor-dev
On Fri, Feb 16, 2018 at 11:19 PM, Fulvio Risso <fulvio.ri...@polito.it> wrote:
>
>
> On 17/02/2018 08:02, Y Song via iovisor-dev wrote:
>>
>> On Fri, Feb 16, 2018 at 7:59 AM, Francesco Picciariello via
>> iovisor-dev <iovisor-dev@lists.iovisor.org> wrote:
>>>
>>> Hello all,
>>> Is there a way to receive asynchronous notification each time an eBPF map
>>> is
>>> modified?
>>
>>
>> When map is modified in kernel, you can send something into a ring buffer
>> and userspace can poll on this ring buffer to get notification.
>
>
> When you say "you" means "the dataplane program"?

Yes.

> In other words, the dataplane must be collaborative and send a notification
> to userspace when the map has been modified?

Yes. Not just notification, may be actual data or part of actual data, or
aggregated data if modification is too frequent.

>
> Thanks,
>
> fulvio
>
>>
>>>
>>> I used bpf_obj_pin() in order to save a specific map on filesystem, and I
>>> called linux inotify() on the pinned object.
>>>
>>> The inotify API provides a mechanism for monitoring filesystem events,
>>> and
>>> the goal is to notice when a pinned eBPF map is modified, but it seems
>>> inotify() does not work properly with bpf filesystem. In fact it's able
>>> to
>>> detect the creation and the deletion, but not the modification of the
>>> pinned
>>> object.
>>
>>
>> The inotify takes vfs_read/write for read/write operations.
>> Here bpf map read/write happens inside
>> bpf program, or through bpf syscall, and hence inotify mechanism won't
>> work.
>>
>>>
>>> Regards.
>>>
>>> ___
>>> iovisor-dev mailing list
>>> iovisor-dev@lists.iovisor.org
>>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>>>
>> ___
>> iovisor-dev mailing list
>> iovisor-dev@lists.iovisor.org
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>>
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Notification when an eBPF map is modified

2018-02-16 Thread Y Song via iovisor-dev
On Fri, Feb 16, 2018 at 7:59 AM, Francesco Picciariello via
iovisor-dev  wrote:
> Hello all,
> Is there a way to receive asynchronous notification each time an eBPF map is
> modified?

When map is modified in kernel, you can send something into a ring buffer
and userspace can poll on this ring buffer to get notification.

>
> I used bpf_obj_pin() in order to save a specific map on filesystem, and I
> called linux inotify() on the pinned object.
>
> The inotify API provides a mechanism for monitoring filesystem events, and
> the goal is to notice when a pinned eBPF map is modified, but it seems
> inotify() does not work properly with bpf filesystem. In fact it's able to
> detect the creation and the deletion, but not the modification of the pinned
> object.

The inotify takes vfs_read/write for read/write operations.
Here bpf map read/write happens inside
bpf program, or through bpf syscall, and hence inotify mechanism won't work.

>
> Regards.
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Is this possible to attach eBPF programs to virtual interfaces?

2018-02-06 Thread Y Song via iovisor-dev
On Tue, Feb 6, 2018 at 8:33 AM, Mauricio Vasquez via iovisor-dev
 wrote:
> Hello All,
>
> I tried to load a PROG_TYPE_SCHED_CLS eBPF program and attach it to a linux
> interface, netlink fails because it is not able to find the interface.
>
> Those are the steps  I did:
>
> sudo ifconfig veth1:1 192.168.2.210/32 up
>

How veth1:1 is created? Looks like a vlan interface?
I am using a simple veth interface like below
```
$ sudo ip link add veth1 type veth peer name veth2
$ sudo ifconfig veth1 192.168.2.210/32 up
```
And the run the script containing the following lines:
```
ipr = IPRoute()
idx = ipr.link_lookup(ifname="veth1")[0]
print idx
```

It works fine.

> `
>
> #!/usr/bin/python
> # Copyright (c) PLUMgrid, Inc.
> # Licensed under the Apache License, Version 2.0 (the "License")
>
> from bcc import BPF
> from pyroute2 import IPRoute
>
> ipr = IPRoute()
>
> b = BPF(src_file="helloworld.c", debug=0)
> fn = b.load_func("hello", BPF.SCHED_CLS)
>
> idx = ipr.link_lookup(ifname="veth1:1")[0]
>
> try:
> ipr.tc("add", "ingress", idx, ":")
> ipr.tc("add-filter", "bpf", idx, ":1", fd=fn.fd,
> name=fn.name, parent=":", action="ok", classid=1)
>
> raw_input("promt: ")
> finally:
> ipr.tc("del", "ingress", idx)
>
> print("BPF tc functionality - SCHED_CLS: OK")
> `
>
> `
>
> Traceback (most recent call last):
>   File "./helloworld.py", line 13, in 
> idx = ipr.link_lookup(ifname="veth1:1")[0]
> IndexError: list index out of range
>
> `
>
> It is possible to attach eBPF programs to virtual interfaces?
>
> Thanks,
>
> Mauricio
>
>
>
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] How to use USDT probes without having to pass the pid of the traced program?

2018-01-17 Thread Y Song via iovisor-dev
to enable this systemwide or is it a feature which hasn't
>>
>> been implemented?
>>
>> Maybe we get pids of the process as they hit another probe and then
>>
>> get the semaphore addresses and enable them?
>>
>> Or is there a way to start the processes with the semaphore enabled --
>>
>> a elf indicator or something?  Just thinking out loud.
>>
>>
>> Thanks,
>>
>> Kiran
>>
>>
>> On Mon, Jan 15, 2018 at 10:30 PM, Sasha Goldshtein <golds...@gmail.com>
>> wrote:
>>
>> Please note that some USDT probes have an associated "semaphore",
>>
>> which is really just a memory location that the probed code checks
>>
>> before actually invoking the probe. You cannot enable USDT probes that
>>
>> have a semaphore system-wide, without a process ID, because the
>>
>> semaphore location has to be poked in each target process.
>>
>>
>> To see if your probes have an associated semaphore, just run tplist with
>>
>> -v -v and it will print out the semaphore address for each probe. If it's
>>
>> 0, you're good to do system-wide tracing.
>>
>>
>> Sasha
>>
>>
>>
>> On Tue, Jan 16, 2018 at 8:08 AM Y Song via iovisor-dev
>>
>> <iovisor-dev@lists.iovisor.org> wrote:
>>
>>
>> Kiran,
>>
>>
>> Yes, tracing through USDT without PID should work.
>>
>> You can just remove "-p" parameter and give a try.
>>
>>
>> Please try latest bcc as it fixed a few bugs.
>>
>> Let us know if you hit any issues.
>>
>>
>> Yonghong
>>
>>
>>
>> On Mon, Jan 15, 2018 at 5:11 PM, Kiran T via iovisor-dev
>>
>> <iovisor-dev@lists.iovisor.org> wrote:
>>
>> I meant to say
>>
>>
>> Can one request to monitor binaries -- like with
>>
>> uprobes/uretprobes but with USDT?
>>
>>
>> On Mon, Jan 15, 2018 at 5:09 PM, Kiran T <kiran.l...@gmail.com> wrote:
>>
>> Hi
>>
>> All the examples on tracing processes with USDT require the pid of the
>>
>> traced program:
>>
>>
>> https://github.com/iovisor/bcc/tree/master/examples/tracing
>>
>>
>> Can one not request to monitor binaries -- like with
>>
>> uprobes/uretprobes but with USDT?
>>
>>
>> I am trying to trace php scripts running on a webserver, and USDT
>>
>> would be ideal.  But, I won't have the pid of the process that will be
>>
>> spawned by the webserver in advance.
>>
>>
>> Thanks,
>>
>> Kiran
>>
>> ___
>>
>> iovisor-dev mailing list
>>
>> iovisor-dev@lists.iovisor.org
>>
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>>
>> ___
>>
>> iovisor-dev mailing list
>>
>> iovisor-dev@lists.iovisor.org
>>
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>>
>> ___
>>
>> iovisor-dev mailing list
>>
>> iovisor-dev@lists.iovisor.org
>>
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] How to use USDT probes without having to pass the pid of the traced program?

2018-01-15 Thread Y Song via iovisor-dev
Kiran,

Yes, tracing through USDT without PID should work.
You can just remove "-p" parameter and give a try.

Please try latest bcc as it fixed a few bugs.
Let us know if you hit any issues.

Yonghong


On Mon, Jan 15, 2018 at 5:11 PM, Kiran T via iovisor-dev
 wrote:
> I meant to say
>
> Can one request to monitor binaries -- like with
> uprobes/uretprobes but with USDT?
>
> On Mon, Jan 15, 2018 at 5:09 PM, Kiran T  wrote:
>> Hi
>> All the examples on tracing processes with USDT require the pid of the
>> traced program:
>>
>> https://github.com/iovisor/bcc/tree/master/examples/tracing
>>
>> Can one not request to monitor binaries -- like with
>> uprobes/uretprobes but with USDT?
>>
>> I am trying to trace php scripts running on a webserver, and USDT
>> would be ideal.  But, I won't have the pid of the process that will be
>> spawned by the webserver in advance.
>>
>> Thanks,
>> Kiran
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Kernel crash when running bcc's test_xlate

2018-01-13 Thread Y Song via iovisor-dev
It seems okay for bcc with latest 4.15-rc7 on x64 and with multiple
runs I cannot reproduce the issue:

[yhs@localhost python]$ ../../build/tests/wrapper.sh py_xlate1_c
namespace ./test_xlate1.py test_xlate1.c
Actual changes:
tx-checksumming: off
tx-checksum-ip-generic: off
tx-checksum-sctp: off
tcp-segmentation-offload: off
tx-tcp-segmentation: off [requested on]
tx-tcp-ecn-segmentation: off [requested on]
tx-tcp-mangleid-segmentation: off [requested on]
tx-tcp6-segmentation: off [requested on]
PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data.
64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=0.041 ms

--- 192.168.1.1 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.041/0.041/0.041/0.000 ms
.
--
Ran 1 test in 0.104s

OK
[yhs@localhost python]$ uname -a
Linux localhost.localdomain 4.15.0-rc7+ #2 SMP Fri Jan 12 22:29:57 PST
2018 x86_64 x86_64 x86_64 GNU/Linux
[yhs@localhost python]$

Regarding to how to translate various ip.addr, ip.tc proute2 commands to
plain "ip ..." or "tc ..." commands, the python implementation is in
/lib/python2.7/site-packages/pyroute2 through netlink interface.

I guess most cases you probably can figure it out easily, e.g.,
ip.addr("del", index=ifindex, address="172.16.1.2", mask=24)
=>
   ip addr del 172.16.1.2/24 dev 

ip.tc("add", "ingress", ifindex, ":")
=>
tc qdisc add dev  handle : ingress


Maybe you can figure out the rest by looking at pyroute2 implementation as above
if you cannot simply map it to ip/tc commands.

On Wed, Jan 10, 2018 at 10:05 PM, Sandipan Das via iovisor-dev
 wrote:
> Hi,
>
> I was trying to run the bcc tests on a ppc64le VM with Fedora 26 and
> v4.15-rc7 kernel and 'test_xlate' was causing a kernel panic. The test
> crashes on all of the v4.15-rcX kernels that I built but run fine on
> v4.14.11 though. To build the kernels, I used the same config as F26's
> v4.14.11 distro kernel with default choices in case any new options
> were added.
>
> From my initial analysis, the crash occurs after the following statement
> (line 35 of tests/python/test_xlate1.py) is executed.
>
> ip.tc("add-filter", "u32", ifindex, ":1", parent=":", action=[action],
> protocol=protocols.ETH_P_ALL, classid=1, target=0x10002, 
> keys=['0x0/0x0+0'])
>
> Any ideas about why this is happening? Also, it would be really helpful
> if someone can translate the Pyroute2 calls in the test script to the
> corresponding tc commands.
>
> Here is the kernel crash log:
>
> [  710.746123] IPv6: ADDRCONF(NETDEV_UP): py_xlate1_c.out: link is not ready
> [  710.746457] IPv6: ADDRCONF(NETDEV_CHANGE): py_xlate1_c.out: link becomes 
> ready
> [  710.746662] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> [  711.141263] Unable to handle kernel paging request for data at address 
> 0x3fecb
> [  711.145240] Faulting instruction address: 0xc09f6f14
> [  711.145613] Oops: Kernel access of bad area, sig: 11 [#1]
> [  711.145898] LE SMP NR_CPUS=1024 NUMA pSeries
> [  711.146191] Modules linked in: act_bpf cls_u32 sch_sfq sch_ingress veth 
> kvm_pr kvm ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set 
> nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat 
> nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw 
> ip6table_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 
> nf_nat nf_conntrack iptable_mangle iptable_raw iptable_security 
> ebtable_filter ebtables ip6table_filter ip6_tables sunrpc pseries_rng 
> vmx_crypto crct10dif_vpmsum 9pnet_virtio 9pnet virtio_balloon xfs libcrc32c 
> virtio_net virtio_blk virtio_pci ibmvscsi virtio_ring scsi_transport_srp 
> crc32c_vpmsum virtio
> [  711.149415] CPU: 27 PID: 1857 Comm: ping Not tainted 4.15.0-rc7 #4
> [  711.149768] NIP:  c09f6f14 LR: c09fbaf8 CTR: 
> c0027020
> [  711.150195] REGS: c003fff07a00 TRAP: 0300   Not tainted  (4.15.0-rc7)
> [  711.150550] MSR:  80009033   CR: 28002882  
> XER: 2000
> [  711.150982] CFAR: 7fff9f240520 DAR: 0003fecb DSISR: 4000 
> SOFTE: 1
> [  711.150982] GPR00: c09fbaf8 c003fff07c80 c14a2d00 
> 001c
> [  711.150982] GPR04: 0001  0636 
> 0003fecb
> [  711.150982] GPR08: 0003fecb  c003f2558a80 
> d7c80fd0
> [  711.150982] GPR12: 2200 cfd71b80  
> 000108d32594
> [  711.150982] GPR16: 8906  0001 
> 0001
> [  711.150982] GPR20: c003e4f42ba0  0005 
> 
> [  711.150982] GPR24: 01080020 c1036fa8 dd86 
> a888
> [  711.150982] GPR28: c305a000 0001 

Re: [iovisor-dev] eBPF program to forward pkt from if-a to if-b

2017-12-19 Thread Y Song via iovisor-dev
There are a few examples under examples/networking with
bpf_clone_redirect helper to redirect a packet from one
interface to another.

Right socket_filter cannot modify packets, you may need to use a
different one, e.g., cls_act type.

On Mon, Dec 18, 2017 at 6:23 AM, Avi Cohen (A) via iovisor-dev
 wrote:
> Hello,
> 1. I don't find under bcc/examples  a reference for forwarding function . 
> e,g, capture pkt on if-a  look for any match and then forward to if-b.
> Can you send a reference to a similar function. ?
>
> 2. in the module examples/networking/http-filter-simple.c - when I try to 
> write to the packet I receive an exception - do I have to set the socket tp 
> other than SOCKET_FILTER ?
>
> Best Regards
> Avi
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Simple example to start XDP/eBPF programming

2017-12-19 Thread Y Song via iovisor-dev
I looked and played your example a little bit. Your observation is correct.
The user space is not able to see the dropped the packet and the
ping still works. From the kernel, the packet going to the user sock
is a cloned socket, which gets the dropped and the original icmp
packet still walks through the kernel ip stack, hence ping still working.

Maybe some networking people can give more explanation here.

On Mon, Dec 18, 2017 at 3:41 AM, Avi Cohen (A) via iovisor-dev
 wrote:
> Thank you Paul
>
> Pls see inline marked [Avi Cohen (A)]
>
>
>
> From: Paul Chaignon [mailto:paul.chaig...@gmail.com]
> Sent: Monday, 18 December, 2017 12:15 PM
> To: Avi Cohen (A)
> Cc: iovisor-dev@lists.iovisor.org
> Subject: Re: [iovisor-dev] Simple example to start XDP/eBPF programming
>
>
>
>
>
> On Mon, Dec 18, 2017 at 10:23 AM, Avi Cohen (A) 
> wrote:
>
> [Avi Cohen (A)]
>
>  Thank you Paul and Y. Song
>  I have 2 questions:
>  1. I’m already running some ‘packet filtering’ , e.g ICMP pkts,  I try to
> ’DROP’
>  all ICMP pkts (return 0) Indeed it is not sent to userspace (python
> program) ,
>  but pass the whole ip-stack and [Avi Cohen (A)] ping is working (I expect
> that ping will fail)  – I thought that the
>  packet is dropped also from the ip-stack
>
>
>
> We really can't do much to help without the source code.
>
> [Avi Cohen (A)]  - attached c and py files – I’ve  override existing files
> (http-parse_simple.*)  with icmp filter [btw – what are the steps to create
> new c , py file?]
>
>
>
> 2. I understand that eBPF function is invoked before sk-buff is allocated
> for the pkt in the ip-stack  (i.e. before the
>  DMA) – so where is the packet being read from by the eBPF?  from the
> physical device buffer ?
>
>
>
> I'm guessing you're referring to the XDP hook. The eBPF program is
> executed on the packets before the skbuff is allocated but after
> the DMA. The DMA writes the packet in memory, so without it the eBPF
> program won't run.
>
> [Avi Cohen (A)]  - Yes XDP, so the DMA writes to a pre-allocated  memory ,
> and then we further allocate an sk-buff and copy the packet from DMA area to
> the sk-buff ? this doesn’t make sense unless I miss something here
>
> Best Regards
>
> Avi
>
>
>
>
>  Best Regards
>  Avi
>
>
>> From: Paul Chaignon [mailto:paul.chaig...@gmail.com]
>> Sent: Monday, 18 December, 2017 10:52 AM
>> To: Avi Cohen (A)
>> Cc: iovisor-dev@lists.iovisor.org
>> Subject: Re: [iovisor-dev] Simple example to start XDP/eBPF programming
>>
>>
>> On Sun, Dec 17, 2017 at 12:55 PM, Avi Cohen (A) via iovisor-dev > d...@lists.iovisor.org> wrote:
>> I've installed (packaged) bcc as per
>> https://github.com/iovisor/bcc/blob/master/INSTALL.md
>> Now I want to start playing.  a simple one like 'hello-world' . I couldn't
>> find a
>> guide for this simple example in this tutorial
>> [https://github.com/iovisor/bcc/blob/master/docs/reference_guide.md]
>> Can you refer to a step by step tutorial ?
>>
>> If bcc is correctly installed, to run the hello_world example, you should
>> only need to run ./examples/hello_world.py as root.
>> This tutorial is probably a better source of information in your case:
>>
>> https://github.com/iovisor/bcc/blob/master/docs/tutorial_bcc_python_develop
>> er.md
>>
>> Best Regards
>> Avi
>> ___
>> iovisor-dev mailing list
>> iovisor-dev@lists.iovisor.org
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
>
>
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Simple example to start XDP/eBPF programming

2017-12-17 Thread Y Song via iovisor-dev
There is an example at bcc/examples/networking/xdp which you can try.

On Sun, Dec 17, 2017 at 3:55 AM, Avi Cohen (A) via iovisor-dev
 wrote:
> I've installed (packaged) bcc as per 
> https://github.com/iovisor/bcc/blob/master/INSTALL.md
> Now I want to start playing.  a simple one like 'hello-world' . I couldn't  
> find a guide for this simple example in this tutorial 
> [https://github.com/iovisor/bcc/blob/master/docs/reference_guide.md]
> Can you refer to a step by step tutorial ?
> Best Regards
> Avi
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Getting stack trace from a different CPU core (Y Song)

2017-12-13 Thread Y Song via iovisor-dev
On Wed, Dec 13, 2017 at 4:07 AM, Nair, Reena via iovisor-dev <
iovisor-dev@lists.iovisor.org> wrote:

> Hi,
>
> I have a tracepoint probe for sched_switch event. While tracing the
> context-switches of a multi-threaded program, I would like to get stack
> trace of other active threads under certain conditions.
>

You cannot get stack trace on other threads (tasks) in bpf programs.
However, depending on what your "certain conditions" are, you may be able
to attach multiple bpf programs to your application, and record stacks with
timestamp/taskid etc and coordinate through shared maps, you may post
process to find how different tasks progress in the time line.



>
> Reena
>
>
>
> --
> *From:* iovisor-dev-boun...@lists.iovisor.org  iovisor.org> on behalf of iovisor-dev-requ...@lists.iovisor.org <
> iovisor-dev-requ...@lists.iovisor.org>
> *Sent:* 13 December 2017 12:00
> *To:* iovisor-dev@lists.iovisor.org
> *Subject:* iovisor-dev Digest, Vol 28, Issue 1
>
> Send iovisor-dev mailing list submissions to
> iovisor-dev@lists.iovisor.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
> or, via email, send a message with subject or body 'help' to
> iovisor-dev-requ...@lists.iovisor.org
>
> You can reach the person managing the list at
> iovisor-dev-ow...@lists.iovisor.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of iovisor-dev digest..."
>
>
> Today's Topics:
>
>1. Getting stack trace from a different CPU core (Nair, Reena)
>2. Re: Getting stack trace from a different CPU core (Y Song)
>3. reminder: IO Visor TSC/Dev Meeting (Brenden Blanco)
>
>
> --
>
> Message: 1
> Date: Tue, 12 Dec 2017 14:26:51 +
> From: "Nair, Reena" 
> To: "iovisor-dev@lists.iovisor.org" 
> Subject: [iovisor-dev] Getting stack trace from a different CPU core
> Message-ID:
>  eurprd06.prod.outlook.com>
>
> Content-Type: text/plain; charset="iso-8859-1"
>
> Hello,
>
>
> Is it possible to get stack trace from a different CPU core with eBPF?
>
>
> Thank you,
>
> Reena
> -- next part --
> An HTML attachment was scrubbed...
> URL:  attachments/20171212/01519d06/attachment-0001.html>
>
> --
>
> Message: 2
> Date: Tue, 12 Dec 2017 09:12:09 -0800
> From: Y Song 
> To: "Nair, Reena" 
> Cc: "iovisor-dev@lists.iovisor.org" 
> Subject: Re: [iovisor-dev] Getting stack trace from a different CPU
> core
> Message-ID:
>  gmail.com>
> Content-Type: text/plain; charset="UTF-8"
>
> Could you describe your use case? Getting stack trace from a different
> CPU involves an
> IPI (inter-processor interrupt) which is beyond bpf. But typically you
> could run your programs
> on both cpus and coordinate through maps to dump stack trace only
> under certain conditions.
>
> On Tue, Dec 12, 2017 at 6:26 AM, Nair, Reena via iovisor-dev
>  wrote:
> > Hello,
> >
> >
> > Is it possible to get stack trace from a different CPU core with eBPF?
> >
> >
> > Thank you,
> >
> > Reena
> >
> >
> > ___
> > iovisor-dev mailing list
> > iovisor-dev@lists.iovisor.org
> > https://lists.iovisor.org/mailman/listinfo/iovisor-dev
> >
>
>
> --
>
> Message: 3
> Date: Tue, 12 Dec 2017 16:46:32 -0800
> From: Brenden Blanco 
> To: iovisor-dev 
> Subject: [iovisor-dev] reminder: IO Visor TSC/Dev Meeting
> Message-ID:
>  mail.gmail.com>
> Content-Type: text/plain; charset="UTF-8"
>
> Please join us tomorrow for our bi-weekly call. As usual, this meeting is
> open to everybody and completely optional.
> You might be interested to join if:
> You want to know what is going on in BPF land
> You are doing something interesting yourself with BPF and would like to
> share
> You want to know what the heck BPF is
>
> === IO Visor Dev/TSC Meeting ===
>
> Every 2 weeks on Wednesday, from Wednesday, January 25, 2017, to no end
> date
> 11:00 am  |  Pacific Daylight Time (San Francisco, GMT-08:00)  |  30 min
>
> https://bluejeans.com/568677804/
> BlueJeans Network | Video Collaboration in the Cloud
> 
> bluejeans.com
> BlueJeans Network - Interoperable, Cloud-based, Affordable Video
> Conferencing Service
>
>
> https://www.timeanddate.com/worldclock/meetingdetails.
> html?year=2017=12=13=19=0=0=900
>
> 

Re: [iovisor-dev] Getting stack trace from a different CPU core

2017-12-12 Thread Y Song via iovisor-dev
Could you describe your use case? Getting stack trace from a different
CPU involves an
IPI (inter-processor interrupt) which is beyond bpf. But typically you
could run your programs
on both cpus and coordinate through maps to dump stack trace only
under certain conditions.

On Tue, Dec 12, 2017 at 6:26 AM, Nair, Reena via iovisor-dev
 wrote:
> Hello,
>
>
> Is it possible to get stack trace from a different CPU core with eBPF?
>
>
> Thank you,
>
> Reena
>
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Forcing compiler to "respect" verifier needs

2017-11-03 Thread Y Song via iovisor-dev
On Fri, Nov 3, 2017 at 11:36 AM, Gianluca Borello via iovisor-dev
 wrote:
> On Thu, Nov 2, 2017 at 10:07 PM, Y Song  wrote:
>>
>>if (len > 0 && len < ...)
>>   bpf_perf_event_output(..., len)
>>
>> Today it may not work in some cases. You can try.
>>
>
> Also, one more thing I forgot to add, and perhaps it wasn't clear in
> my original message: this suggestion above will not work in this case.

Right. I had a test as well to show this does not work. But it is my *goal*
to make it work like it or similar. Still experimenting with kernel patches.

> In fact, from a compiler point of view there is no difference between
>
> if (len > 0 && len < SCRATCH_SIZE)
> bpf_perf_event_output(..., len)
>
> and my original
>
> len &= SCRATCH_SIZE;
> ++len;
> bpf_perf_event_output(..., len);
>
> The compiler knows for a fact that len is always less than
> SCRATCH_SIZE (because there is a previous more restrictive check done
> before the spilling), so it will generate the exact same BPF code I
> posted below, without doing any range check at all:
>
> // checks, whether &= or if(), are removed by the compiler:
> 149:   r6 = *(u64 *)(r10 - 80)
> ...
> 157:   r5 = r6
> 158:   call 25
>
> And I couldn't find a way to force the compiler into emitting those
> instructions.
>
> Thanks
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Forcing compiler to "respect" verifier needs

2017-11-02 Thread Y Song via iovisor-dev
I am working on a linux patch to address a similar issue to this.
Basically, variable "len" to
certain helper functions cause verifier failure.

In this case, my proposal is add additional checks around the
bpf_perf_event_output
to make it work. The reason is as you said, the state got lost due to
register spill.
   if (len > 0 && len < ...)
  bpf_perf_event_output(..., len)

Today it may not work in some cases. You can try.

On Thu, Nov 2, 2017 at 2:53 PM, Gianluca Borello via iovisor-dev
 wrote:
> Hello,
>
> As the complexity of my BPF programs increases, I am finding myself
> spending a lot of time fighting with the verifier due to the way the
> compiler generates optimized BPF code. I apologize in advance if the
> question is naive, it might very well be caused by my poor knowledge
> of llvm.
>
> For example, this is a very common scenario where I get in trouble.
> I'm trying to push a variable-sized frame through perf:
>
>
> #define SCRATCH_SIZE 0x3fff
> #define MAX_STR_LEN 0x3ff
>
> void write_perf_pdu(void)
> {
> /* scratch_map is a per-cpu array used as an extended
>  * stack space
>  */
> int id = 0;
> char *scratchp = bpf_map_lookup_elem(_map, );
> if (!scratchp)
> return;
>
> int len = 0;
> char *p = scratchp;
>
> /* Write first frame element (variable-size string) */
> int res = bpf_probe_read_str(p, MAX_STR_LEN, NULL);
> if (res < 0)
> return;
> /* Check boundaries (needed to write the second
>  * frame element at p[res])
>  */
> res &= MAX_STR_LEN;
> len += res;
> p += res;
> /* Write the second frame element at scratchp[res] */
> *(int *) p = res;
> len += sizeof(int);
>
> /* Do some other stuff that will cause the compiler
>  * to spill "len" onto the stack. It really doesn't
>  * take much, just a "switch" with a few cases is often
>  * enough.
>  */
> do_some_work();
>
> /* This is needed because "len" was spilled so we need to
>  * re-tell the verifier that the range is safe. However,
>  * this instruction will never be emitted because the
>  * compiler already knows that len is < SCRATCH_SIZE
>  * (because of the previous &= MAX_STR_LEN), so the verifier
>  * will refuse to load the program.
>  */
> len &= SCRATCH_SIZE;
> ++len;
> bpf_perf_event_output(ctx, _map,
>   bpf_get_smp_processor_id(), scratchp,
>   len);
> }
>
>
> In case my commentary was not clear enough, here is an alternative
> commented version of the generated BPF code:
>
>
> 13: (85) call bpf_probe_read_str#45
>
> // res &= MAX_STR_LEN;
> // verifier knows r0 is now in the proper range
> 17: (57) r0 &= 1023
>
> // res/len is spilled into the stack, and the range information
> // about r0 is forgotten
> 18: (7b) *(u64 *)(r10 -80) = r0
>
> // *(int *) p = res;
> // before recycling r0, the range-checked r0 is used for a variable
> // access inside the map
> 19: (bf) r1 = r8
> 20: (0f) r1 += r0
> 21: (63) *(u32 *)(r1 +0) = r0
>
> ...
> // do_some_work() is called
> ...
>
> // len is restored from the stack
> 149: (79) r6 = *(u64 *)(r10 -80)
>
> // len += sizeof(int);
> // ++len;
> // However, len &= SCRATCH_SIZE is ignored because it's useless from
> // a compiler point of view
> 150: (07) r6 += 5
> ...
>
> // as a result, helper call naturally fails since length is not
> // range checked
> 157: (bf) r5 = r6
> 158: (85) call bpf_perf_event_output#25
> R5 min value is negative, either use unsigned or 'var &= const'
>
>
> Notice that, if I manually edit the BPF code and add an explicit check
> on r5 right before instruction 158, it will just work fine. But the
> compiler decides, for optimization reasons, to not do that.
>
> This happens in several other circumstances to the point where it's my
> biggest struggle with BPF at the moment, hence this email.
>
> The solutions I see would be:
>
> - Manually reorganize the code to make sure do_some_work() is called
> before or after: this was initially my strategy, but as my needs are
> evolving I'm more frequently generating automatic BPF code based on
> some metadata description of my tracing needs, so I can't really tweak
> the code too much.
>
> - Allow the verifier to track spilled SCALAR_VALUE registers: I think
> this was discussed a while ago and the consensus was it would be a
> large patch and would also make the verifier state tracking explode?
> Maybe things changed in the last few months?
>
> - Tell llvm to always emit an instruction via some asm("") trick or
> similar? This is where my poor knowledge of llvm penalizes me, but by
> quickly looking in the bcc code base I couldn't find any example of
> such usage.
>
> - Take the generated BPF program and patch it by 

Re: [iovisor-dev] BCC: Dentry_path_raw() support

2017-10-30 Thread Y Song via iovisor-dev
The function dentry_path_raw calls __dentry_path which has locks
called inside the function,
so itself or a wrapper cannot be used as the helper.

You could traverse the dentry itself in the bpf program. It may not
give all valid results since
it could a parallel update to the same data structure. But it should
be close enough.

On Sun, Oct 29, 2017 at 11:16 AM, Raffaele Sommese via iovisor-dev
 wrote:
> Hello Everybody,
> I'm going to implement a storage solution based on eBPF.
> I'm looking for a function that is included in linux kernel for recover the
> pathname from a dentry structure.
> The function is the  dentry_path_raw() and it is in /fs/dcache.c
> I try to import the function with #include but i notice that only the file
> of "/include" folder can be imported.
> So I'm looking for a wrapper for call external kernel function in eBPF, can
> you help me please?
> Thank You
> Best Regards
> Raffaele Sommese
>
> --
> 
> Raffaele Sommese
> Mail:raffyso...@gmail.com
> About me:https://about.me/r4ffy
> Gpg Key:http://www.r4ffy.info/Openpgp.asc
> GPG key ID: 0x830b1428cf91db2a on http://pgp.mit.edu:11371/
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Bcc on 32-bit ARM

2017-10-13 Thread Y Song via iovisor-dev
On Fri, Oct 13, 2017 at 5:53 AM, Adrian Szyndela via iovisor-dev
 wrote:
>
> W dniu 04.10.2017 o 21:14, Y Song pisze:
>>
>> On Wed, Oct 4, 2017 at 3:24 AM, Adrian Szyndela via iovisor-dev
>>  wrote:
>>>
>>> - return type of bpf_pseudo_fd is 'u64', and it is passed to functions
>>> that
>>> take pointers; the compiler generates 'trunc' in IR for this, which
>>> becomes
>>> shift-left + shift-right in final bpf code; and bpf verifier does not
>>> like
>>> it when pointers are shifted. We worked it around by changing return type
>>> of
>>> bpf_pseudo_fd/llvm.bpf.pseudo from 'u64' to 'void *' - both in bcc
>>>
>>> (https://github.com/aszyndela/bcc/commit/8eb8fb2e265b93d38c1ad53efcb7da8883a3d643)
>>> and llvm
>>> (include/llvm/IR/IntrinsicsBPF.td):
>>>
>>>   def int_bpf_pseudo : GCCBuiltin<"__builtin_bpf_pseudo">,
>>>-  Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty]>;
>>>+  Intrinsic<[llvm_ptr_ty], [llvm_i64_ty, llvm_i64_ty]>;
>>>
>>> Obviously we don't know what we're doing, but at least it works for us.
>>
>>
>> This should work as well. essentially bpf_pseudo tries to convert a
>> load map_id to load_imm64 address. So its return value is indeed a ptr.
>
>
> I asked about it on llvm-dev and the only suggestion I got so far is to set
> bpf target for clang:
> https://lists.llvm.org/pipermail/llvm-dev/2017-October/118149.html
>
> And it makes sense...
>
> We can easily get into trouble when we mix two targets in the process. For
> example, it looks like sizeofs are computed at the "clang" stage, but actual
> sizes of some types are "known" at the "llc" stage.
>
> However, switching to bpf target for the whole process takes away
> convenience and maintainability: we can't rely on including kernel headers
> anymore. So we have to craft everything we need by hand, including struct
> pt_regs...
>
> Any thoughts how to handle this?

All these are valid. For certain program types e.g., networking where pt_regs
are not needed, you can already use -target bpf in clang. Check
linux:tools/testing/selftests/bpf/Makefile.

Since currently we do not have good solutions for pt_regs when "clang
-target bpf" is
used, let us stick to the approach "clang -target arm"/"llc -target bpf".

> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [PATCH, bpf-llvm] Fix bug on silently truncating 64-bit immediate

2017-10-12 Thread Y Song via iovisor-dev
Hi, Jiong,

The patch looks good. Thanks for fixing the issue. Will merge soon.

Yonghong

On Thu, Oct 12, 2017 at 5:08 AM, Jiong Wang via iovisor-dev
 wrote:
> On 12/10/2017 12:04, Jesper Dangaard Brouer wrote:
>>
>> On Thu, 12 Oct 2017 06:21:30 -0400
>> Jiong Wang  wrote:
>>
>>> We came across an llvm bug when compiling some testcases that 64-bit
>>> immediates are silently truncated into 32-bit and then packed into
>>> BPF_JMP | BPF_K encoding.  This caused comparison with wrong value.
>>
>> I think you send this to the wrong mailing list... this looks like a
>> patch against the LLVM source code.
>>
>> Shouldn't you send to: llvm-...@lists.llvm.org ?
>
>
> Hi Jesper,
>
>   I am thinking the users & developers of eBPF llvm are quite focused in
> xdp-newbies and iovisor-dev, so if the code is related with user visible
> effect, for example bug fix, and if the code change is straightforward
> and small, I tend to just send it to these two so related user could be
> aware of it in case they are encountering the same problem, and then this
> patch could be posted to llvm review site (https://reviews.llvm.org).
>
>>
>>>
>>> This bug looks to be introduced by r308080.
>>
>> This looks like a very recent change:
>>https://llvm.org/viewvc/llvm-project?view=revision=308080
>>Sat Jul 15 05:41:42 2017 UTC (2 months, 4 weeks ago)
>>
>> (As you are sending this to a user mailing list:
>> xdp-newb...@vger.kernel.org)
>>
>> I want to know if this have made it into a LLVM release? and which
>> release?
>
>
> This has not been made into any LLVM official release.
> The bug may show up if the user is trying to use distributor snapshop
> release.
>
>>
>> As the git-repo[1] replica of LLVM SVN-repo does not git-tag the
>> releases, I could not answer this question myself via the command:
>>
>> $ git describe --contains 7c423e0690
>>
>> [1] http://llvm.org/git/llvm.git
>>
>>> The Select_Ri pattern is
>>> supposed to be lowered into J*_Ri while the latter only support 32-bit
>>> immediate encoding, therefore Select_Ri should have similar immediate
>>> predicate check as what J*_Ri are doing.
>>>
>>> Reported-by: Jakub Kicinski 
>>> Signed-off-by: Jiong Wang 
>>> ---
>>>   lib/Target/BPF/BPFISelLowering.cpp |  8 ++--
>>>   lib/Target/BPF/BPFInstrInfo.td |  2 +-
>>>   test/CodeGen/BPF/select_ri.ll  | 35
>>> +++
>>>   3 files changed, 42 insertions(+), 3 deletions(-)
>
>
> --
>
> Regards,
> Jiong
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [PATCH RFC 0/4] Initial 32-bit eBPF encoding support

2017-09-23 Thread Y Song via iovisor-dev
On Sat, Sep 23, 2017 at 1:41 AM, Jakub Kicinski via iovisor-dev
 wrote:
> On Fri, 22 Sep 2017 22:03:47 -0700, Yonghong Song wrote:
>> On 9/22/17 9:24 AM, Jakub Kicinski wrote:
>> > On Thu, 21 Sep 2017 11:56:55 -0700, Alexei Starovoitov wrote:
>> >> On Wed, Sep 20, 2017 at 12:20:40AM +0100, Jiong Wang via iovisor-dev 
>> >> wrote:
>> >>> On 18/09/2017 22:29, Daniel Borkmann wrote:
>>  On 09/18/2017 10:47 PM, Jiong Wang wrote:
>> > Hi,
>> >
>> > Currently, LLVM eBPF backend always generate code in 64-bit mode,
>> > this may
>> > cause troubles when JITing to 32-bit targets.
>> >
>> > For example, it is quite common for XDP eBPF program to access
>> > some packet
>> > fields through base + offset that the default eBPF will generate
>> > BPF_ALU64 for
>> > the address formation, later when JITing to 32-bit hardware,
>> > BPF_ALU64 needs
>> > to be expanded into 32 bit ALU sequences even though the address
>> > space is
>> > 32-bit that the high bits is not significant.
>> >
>> > While a complete 32-bit mode implemention may need an new ABI
>> > (something like
>> > -target-abi=ilp32), this patch set first add some initial code so we
>> > could
>> > construct 32-bit eBPF tests through hand-written assembly.
>> >
>> > A new 32-bit register set is introduced, its name is with "w"
>> > prefix and LLVM
>> > assembler will encode statements like "w1 += w2" into the following
>> > 8-bit code
>> > field:
>> >
>> >   BPF_ADD | BPF_X | BPF_ALU
>> >
>> > BPF_ALU will be used instead of BPF_ALU64.
>> >
>> > NOTE, currently you can only use "w" register with ALU
>> > statements, not with
>> > others like branches etc as they don't have different encoding for
>> > 32-bit
>> > target.
>> 
>>  Great to see work in this direction! Can we also enable to use / emit
>>  all the 32bit BPF_ALU instructions whenever possible for the currently
>>  available bpf targets while at it (which only use BPF_ALU64 right now)?
>> >>>
>> >>> Hi Daniel,
>> >>>
>> >>> Thanks for the feedback.
>> >>>
>> >>> I think we could also enable the use of all the 32bit BPF_ALU under
>> >>> currently
>> >>> available bpf targets.  As we now have 32bit register set support, we 
>> >>> could
>> >>> make
>> >>> i32 type as legal type to prevent it be promoted into i64, then hook it 
>> >>> up
>> >>> with i32
>> >>> ALU patterns, will look into this.
>> >>
>> >> I don't think we need to gate 32bit alu generation with a flag.
>> >> Though interpreter and JITs support 32-bit since day one, the verifier
>> >> never seen such programs before, so some valid programs may get
>> >> rejected. After some time passes and we're sure that all progs
>> >> still work fine when they're optimized with 32-bit alu, we can flip
>> >> the switch in llvm and make it default.
>> >
>> > Thinking about next steps - do we expect the 32b operations to clear the
>> > upper halves of the registers?  The interpreter does it, and so does
>> > x86.  I don't think we can load 32bit-only programs on 64bit hosts, so
>> > we would need some form of data flow analysis in the kernel to prune
>> > the zeroing for 32bit offload targets.  Is that correct?
>>
>> Could you contrive an example to show the problem? If I understand
>> correctly, you most worried that some natural sign extension is gone
>> with "clearing the upper 32-bit register" and such clearing may make
>> some operation, esp. memory operation not correct in 64-bit machine?
>
> Hm.  Perhaps it's a blunder on my side, but let's take:
>
>   r1 = ~0ULL
>   w1 = 0
>   # use r1
>
> on x86 and the interpreter, the w1 = 0 will clear upper 32bits, so r1
> ends up as 0.  32b arches may translate this to something like:
>
>   # r1 = ~0ULL
>   r1.lo = ~0
>   r1.hi = ~0
>   # w1 = 0
>   r1.lo = 0
>   # r1.hi not touched
>
> which will obviously result in r1 == 0x.  LLVM should
> not assume r1.hi is cleared, but I'm not sure this is a strong enough
> argument.

Not sure what LLVM will do in this case  for later "r1" access
unless going through the real implementation. My hunch is LLVM
should do a conversion from 32bit to 64bit, "r1 <<= 32" and
"r1 >>= 32" after "w1 = 0" before using r1. Let us wait and check
once implementation in place.

> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [PATCH RFC 3/4] New 32-bit register set

2017-09-21 Thread Y Song via iovisor-dev
Hi, Jiong,

The new patch looks good. I did some basic testing on
net-next:samples/bpf and net-next:tools/testing/selftests/bpf and it
works fine. All existing llvm unit tests are not impacted as well as
expected.

I have applied the patch to the trunk. Besides your other work to
support 32bit abi, it would be
interesting to see how new 32bit register can be used in 64bit
architecture which may help improve performance and/or reduce
instruction count.

Thanks,
Yonghong

On Tue, Sep 19, 2017 at 4:10 PM, Jiong Wang  wrote:
> On 19/09/2017 07:44, Y Song wrote:
>>
>> Hi, Jiong,
>>
>> Thanks for the patch! It is a great start to support 32bit register in
>> BPF.
>> In the past, I have studied a little bit to see whether 32bit register
>> support may reduce
>> the number of unnecessary shifts on x86_64 and improve the
>> performance. Looking through
>> a few bpf programs and it looks like the opportunity is not great, but
>> still nice to have if we
>> have this capability. As you mentioned, this definitely helped 32bit
>> architecture!
>>
>> I am not an expert in LLVM tablegen. I briefly looked through the code
>> change and it looks like
>> correct. Hopefully some llvm-dev tablegen experts can comment on the
>> implementation.
>>
>> Below I only have a couple of minor comments.
>
>
> Yong Hong,
>
>   Thanks for the review.
>
>   I have addressed your comments and attached the updated patch.
>
>   Do you want me to put this patch set on to llvm review website? I guess it
> is the
>   formal review channel?
>
> Regards,
> Jiong
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Accessing maps on already installed BPF program

2017-09-15 Thread Y Song via iovisor-dev
On Thu, Sep 14, 2017 at 1:22 PM, Douglas Caetano dos Santos via
iovisor-dev  wrote:
> Hi there,
>
> I'm writing some long running BPF programs with BCC for packet filtering
> purposes, but don't want to keep the Python part running all the time, only 
> when
> I want to read some statistics or when I want to update some maps. I couldn't
> find, though, documentation on how to make a Python script get access to maps
> being used by an already installed program, instead of creating new ones and
> recompiling the program. I know that maps' file descriptors can be exported 
> and
> imported with tc alone, but is it possible through BCC?

The better way than export/import is prog/map pinning.
Unfortunately, this is not available in bcc API yet.

>
> Thanks,
> Douglas.
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [v2][PATCH RFC] Add BPF AsmParser support in LLVM

2017-09-12 Thread Y Song via iovisor-dev
Sorry. Resend with "plain text format" to satisfy xdp-newbies mailing
list requirement.

On Tue, Sep 12, 2017 at 11:10 AM, Y Song  wrote:
> Thanks, Jiong,
>
> The patch looks good. I have applied to llvm trunk.
> https://reviews.llvm.org/rL313055
> A little bit more comments below.
>
> On Tue, Sep 12, 2017 at 4:10 AM, Jiong Wang 
> wrote:
>>>
>>>
 - Adjusted constant load tests.
 - Interleaved the expected results with input insns.

   * Changed the disassembly output "ll" to "LL" in BPFInstPrinter.cpp.
 This is because I noticed MC parser only accept "LL" as long long
 suffix.
>>>
>>>
>>> Sorry for the confusion caused by "ll". I tested your patch and found
>>> that
>>> both r = 5 and r = 5LL generated ld_imm64 insn. This is not what compiler
>>> will do. Further debugging I found that this is due to my change to
>>> remove
>>> "ll" from asmstring in the insn definition (.td file). Since the
>>> assembler is
>>> patten matching based on asmstring. "r = 5" matched to ld_imm64 as well.
>>>
>>> I just fixed the issue and restored " ll" suffix for ld_imm64 asmstring.
>>> You can then restore your previous change to add "ll" as a valid
>>> identifier
>>> in the middle of asm statement.
>>
>>
>> Hmm, I actually feel there is no need to offer an separate syntax for
>> 64bit imm
>> encoding, it might be better to offer a unified single syntax "r =
>> signed_imm"
>> to the user and the encoder (BPFMCCodeEmitter::encodeInstruction)
>> guarantee to
>> choose optimal encoding.
>>
>> Encoder could choose BPF_ALU64 | BPF_MOV | BPF_K for both "r1 = -1" and
>> "r1 = -1ll" while only resorting to BPF_LD | BPF_DW | BPF_IMM when the imm
>> really
>> doesn't fit into 32bit (!isInt<32>(imm_opnd)), for example, "r1 =
>> 0x1ll".
>>
> Right now, this optimization actually
> has been done in compiler side. So depending on the value, the compiler will
> use
> "move r1, <32_bit_value>" or "ld_imm64 r1, ".
> In this case, it make sense to have different insn formats for two different
> kinds of
> insns.
>
> What you described is to move the optimization down to MC Emit Code stage,
> based on
> the value, it can choose to use "move" or "ld_imm64" insn. So optimization
> will be available
> for .c => .o and for .s => .o. (The current scheme, optimization not
> available from .s => .o).
>
> Yes, it is possible. But typically, there is one-to-one mapping between asm
> insn to insn encoding.
> If later on, we found the complain about this, we can revisit this issue.
>
> Thanks.
> Yonghong
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [v2][PATCH RFC] Add BPF AsmParser support in LLVM

2017-09-12 Thread Y Song via iovisor-dev
Thanks, Jiong,

The patch looks good. I have applied to llvm trunk.
https://reviews.llvm.org/rL313055
A little bit more comments below.

On Tue, Sep 12, 2017 at 4:10 AM, Jiong Wang 
wrote:

>
>> - Adjusted constant load tests.
>>> - Interleaved the expected results with input insns.
>>>
>>>   * Changed the disassembly output "ll" to "LL" in BPFInstPrinter.cpp.
>>> This is because I noticed MC parser only accept "LL" as long long
>>> suffix.
>>>
>>
>> Sorry for the confusion caused by "ll". I tested your patch and found that
>> both r = 5 and r = 5LL generated ld_imm64 insn. This is not what compiler
>> will do. Further debugging I found that this is due to my change to remove
>> "ll" from asmstring in the insn definition (.td file). Since the
>> assembler is
>> patten matching based on asmstring. "r = 5" matched to ld_imm64 as well.
>>
>> I just fixed the issue and restored " ll" suffix for ld_imm64 asmstring.
>> You can then restore your previous change to add "ll" as a valid
>> identifier
>> in the middle of asm statement.
>>
>
> Hmm, I actually feel there is no need to offer an separate syntax for
> 64bit imm
> encoding, it might be better to offer a unified single syntax "r =
> signed_imm"
> to the user and the encoder (BPFMCCodeEmitter::encodeInstruction)
> guarantee to
> choose optimal encoding.
>
> Encoder could choose BPF_ALU64 | BPF_MOV | BPF_K for both "r1 = -1" and
> "r1 = -1ll" while only resorting to BPF_LD | BPF_DW | BPF_IMM when the imm
> really
> doesn't fit into 32bit (!isInt<32>(imm_opnd)), for example, "r1 =
> 0x1ll".
>
> Right now, this optimization actually
has been done in compiler side. So depending on the value, the compiler
will use
"move r1, <32_bit_value>" or "ld_imm64 r1, ".
In this case, it make sense to have different insn formats for two
different kinds of
insns.

What you described is to move the optimization down to MC Emit Code stage,
based on
the value, it can choose to use "move" or "ld_imm64" insn. So optimization
will be available
for .c => .o and for .s => .o. (The current scheme, optimization not
available from .s => .o).

Yes, it is possible. But typically, there is one-to-one mapping between asm
insn to insn encoding.
If later on, we found the complain about this, we can revisit this issue.

Thanks.
Yonghong
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [v2][PATCH RFC] Add BPF AsmParser support in LLVM

2017-09-11 Thread Y Song via iovisor-dev
Hi, Jiong,

Thanks for the patch. Comments inlined.


> Changes since the initial version:
>
>   * Addressed all comments on instruction unit tests.
> - Renamed test file to "insn-unit.s".
> - Removed unnecessary comments.
> - Added BPF_B tests for load and store.
> - Added BPF_K test for jmp and alu.
>   (Noticed a seperate issue with unsigned compare then jump. Looks like
> BPF
>backend is alway printing the immediate as signed, I guess we need to
>change the operand types in instruction patterns.)

Right. We may need to separate out signed comparison vs. others. Will do
that later.

> - Adjusted constant load tests.
> - Interleaved the expected results with input insns.
>
>   * Changed the disassembly output "ll" to "LL" in BPFInstPrinter.cpp.
> This is because I noticed MC parser only accept "LL" as long long
> suffix.

Sorry for the confusion caused by "ll". I tested your patch and found that
both r = 5 and r = 5LL generated ld_imm64 insn. This is not what compiler
will do. Further debugging I found that this is due to my change to remove
"ll" from asmstring in the insn definition (.td file). Since the assembler is
patten matching based on asmstring. "r = 5" matched to ld_imm64 as well.

I just fixed the issue and restored " ll" suffix for ld_imm64 asmstring.
You can then restore your previous change to add "ll" as a valid identifier
in the middle of asm statement.

Thanks.
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [PATCH RFC] Add BPF AsmParser support in LLVM

2017-09-08 Thread Y Song via iovisor-dev
On Fri, Sep 8, 2017 at 1:00 PM, Jiong Wang  wrote:
>
> Hi Y Song,
>
>   Thanks for the review and test.
>
> [snip]
>>
>> I guess, we may need to add something like LD_VAR_ADDR so that
>> the proper assembly code can be issued. Also we could drop "ll" with
>> "r1 = tx_port"?
>
>
> Personally, I prefer drop "ll". The "ll" suffix was there to tell people it 
> is 64bit
> constant while the register "r1" is with 64-bit width so I feel it is enough. 
> For
> 32-bit situation, we need new register set, some thing like "w1 = tx_port".

I just push a patch (similar to your suggestion below but without
assertion) which still has
"ll" suffix for the constant, but no "ll" suffix for symbols. The
reason is that  we use "ll"
in the asm printout to differentiate "r = 5"=>"mov r, 5" (32bit imm) and
"r = 5ll" => "ld_imm64 r, 5ll" (64bit imm).

Also, for long long constant, C standard does not allow space between
value and "ll" and
hence "567 ll" is not allowed to represent a number "567ll". Could you
try to disallow
64bit immediate like "567 ll" as well in your patch?

Thanks,

Yonghong

>
> And to generate proper assembly, it looks to me the customized print method 
> for
> "uimm64" needs the following code (copied from printOperand):
>
> @@ -90,5 +90,8 @@ void BPFInstPrinter::printImm64Operand(const MCInst *MI, 
> unsigned OpNo,
>if (Op.isImm())
>  O << (uint64_t)Op.getImm();
>else
> -O << Op;
> +{
> +  assert(Op.isExpr() && "Expected an expression");
> +  printExpr(Op.getExpr(), O);
> +}
>  }
>
>>
>> Back to your test case bpf-insn-unit.s, you do not need add bpf in the file 
>> name
>> since it is already under BPF directory.
>
>
> Will change the name. and will fix all reviews below in the next update.
>
>>>  r0 = *(u16 *)skb[2] // BPF_LD | BPF_ABS | BPF_H
>>> r0 = * (u32*)skb[4] // BPF_LD | BPF_ABS | BPF_W
>>
>> could you add BPF_B test as well?
>>
>>>  r0 = *  (u16 *)skb[r1] // BPF_LD | BPF_IND | BPF_H
>>>  r0 = *(u32 *)skb[r2]   // BPF_LD | BPF_IND | BPF_W
>>
>> BPF_B test?
>>
>>>  r1 = 8589934591 ll // BPF_LD | BPF_DW | BPF_IMM
>>>  r1 = dummy_map ll  // Symbolic version
>>
>>
>> //  BPF_LDX Class 
>>   r5 = *(u8 *)(r0 + 0)   // BPF_LDX | BPF_B
>>   r6 = *(u16 *)(r1 + 8)  // BPF_LDX | BPF_H
>>   r7 = *(u32 *)(r2 + 16) // BPF_LDX | BPF_W
>>   r8 = *(u64 *)(r3 - 30) // BPF_LDX | BPF_DW
>>
>> //  TODO: BPF_ST Class ===
>>>
>>> There is no insn here, you can remove this line for now.
>>
>>
>> //  BPF_STX Class 
>>   *(u8 *)(r0 + 0) = r7// BPF_STX | BPF_B
>>   *(u16 *)(r1 + 8) = r8   // BPF_STX | BPF_H
>>   *(u32 *)(r2 + 16) = r9  // BPF_STX | BPF_W
>>   *(u64 *)(r3 - 30) = r10 // BPF_STX | BPF_DW
>>
>>   lock *(u32 *)(r2 + 16) += r9  // BPF_STX | BPF_W | BPF_XADD
>>   lock *(u64 *)(r3 - 30) += r10 // BPF_STX | BPF_DW | BPF_XADD
>>
>> //  BPF_JMP Class 
>>   goto Llabel0  // BPF_JA
>>   if r0 == r1 goto Llabel0   // BPF_JEQ
>>   if r1 > r2 goto Llabel0// BPF_JGT
>>   if r2 >= r3 goto Llabel0   // BPF_JGE
>>   // TODO: BPF_JSET
>>>
>>> there is no BPF_JSET, you can remove this.
>>
>>   if r3 != r4 goto Llabel0   // BPF_JNE
>>   if r4 s> r5 goto Llabel0   // BPF_JSGT
>>   if r5 s>= r6 goto Llabel0  // BPF_JSGE
>>   call 1 // BPF_CALL
>>   exit   // BPF_EXIT
>>   if r6 < r7 goto Llabel0// BPF_JLT
>>   if r7 <= r8 goto Llabel0   // BPF_JLE
>>   if r8 s< r9 goto Llabel0   // BPF_JSLT
>>   if r9 s<= r10 goto Llabel0 // BPF_JSLE
>>
>>> Could you add test compare register vs. imm?
>>
>>
>> //  BPF_ALU64 Class 
>>   r0 += r1// BPF_ADD
>>   r1 -= r2// BPF_SUB
>>   r2 *= r3// BPF_MUL
>>   r3 /= r4// BPF_DIV
>> Llabel0:
>>   r4 |= r5// BPF_OR
>>   r5 &= r6// BPF_AND
>>   r6 <<= r7   // BPF_LSH
>>   r7 >>= r8   // BPF_RSH
>>   // TODO: BPF_NEG
>>   // TODO: BPF_MOD
>>>
>>> There is no BPF_NEG/MOD yet, you can remove these two lines.
>>
>>
>>   r8 ^= r9// BPF_XOR
>>   r9 = r10// BPF_MOV
>>   r10 s>>= r0 // BPF_ARSH
>>>
>>> Could you add arith operations with register and imm. operands?
>>
>>
>>   bswap16 r1  // BPF_END | BPF_TO_BE
>>   bswap32 r2  // BPF_END | BPF_TO_BE
>>   bswap64 r3  // BPF_END | BPF_TO_BE
>>   // TODO: BPF_END | BPF_TO_LE
>>>
>>> There is no BPF_TO_LE yet, you can remove this line.
>>
>>
>>> Could you interleave the CHECK results with insn? This will be
>>
>> clearer to do manual comparison.
>
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] New bcc helpers

2017-09-08 Thread Y Song via iovisor-dev
On Fri, Sep 8, 2017 at 12:21 PM, carlos antonio neira bustos via
iovisor-dev  wrote:

> Hi,
>
> I'm trying to add new helpers to obtain a pid namespace, I'm working on
> kernel 4.13
>
> --- linux/linux-4.13/kernel/bpf/helpers.c 2017-09-03 13:56:17.0
> -0700
> +++ /home/cnb/ebpf-backports/new-bcc-helpers/linux-4.13/kernel/bpf/helpers.c 
> 2017-09-07
> 18:52:40.839525862 -0700
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  /* If kernel subsystem is allowing eBPF programs to call this function,
>   * inside its own verifier_ops->get_func_proto() callback it should return
> @@ -179,3 +180,64 @@
>   .arg1_type = ARG_PTR_TO_UNINIT_MEM,
>   .arg2_type = ARG_CONST_SIZE,
>  };
> +
> +BPF_CALL_0(bpf_get_current_pid_ns)
> +{
> +#ifdef CONFIG_PID_NS
> + struct pid_namespace *current_ns =
> +  task_active_pid_ns(current);
> +
> + if (unlikely(!current_ns))
> +  return -EINVAL;
> +
> + return (long) current_ns;
> +#else
> +
> + return 0;
> +#endif
> +
> +}
> +
> +const struct bpf_func_proto bpf_get_current_pid_ns_proto = {
> + .func  = bpf_get_current_pid_ns,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> +};
> +
> +BPF_CALL_0(bpf_get_current_ns_id)
> +{
> + struct task_struct *ts = current;
> +
> + if (unlikely(!ts))
> +  return -EINVAL;
> +
> + return (unsigned int)
> +  ts->nsproxy->pid_ns_for_children->ns.inum;
> +
> +}
> +
> +const struct bpf_func_proto bpf_get_current_ns_id_proto = {
> + .func  = bpf_get_current_ns_id,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> +};
> +
> +BPF_CALL_0(bpf_get_current_pid)
> +{
> + struct task_struct *ts = current;
> +
> + if (unlikely(!ts))
> +  return -EINVAL;
> +
> + pid_t pid = task_pid_vnr(ts);
> +
> + return (u64) ts->tgid << 32 | pid;
> +}
> +
> +const struct bpf_func_proto bpf_get_current_pid_proto = {
> + .func  = bpf_get_current_pid,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> +};
> +
> +
> I wanted to integrate this on bcc tools, so I added these helpers on
> bcc/src/cc/compat/linux/virtual_bpf.h
> bcc/src/cc/compat/linux/bpf.h
> bcc/src/cc/export/helpers.h
> bcc/src/cc/export/helpers.h
>
> then just  to test one of them I modified bcc/tools/funccount.py
>
> --- funccount.py 2017-09-08 12:14:57.601604654 -0700
> +++ /home/cnb/bcc-new-helpers/bcc/tools/funccount.py 2017-09-07
> 20:27:32.982815146 -0700
> @@ -185,7 +185,7 @@
>  # the top 32 bits of bpf_get_current_pid_tgid().
>  if self.pid:
>  trace_count_text = trace_count_text.replace('FILTER',
> -"""u32 pid = bpf_get_current_pid_tgid() >> 32;
> +"""u32 pid = bpf_get_current_pid() >> 32;
> if (pid != %d) { return 0; }""" % self.pid)
>  else:
>  trace_count_text = trace_count_text.replace('FILTER', '')
>
>
> but I'm getting this error
>
> cnb@Debian9:~/bcc/tools$ sudo /usr/share/bcc/tools/funccount -p 385
> c:malloc
> bpf: Invalid argument
> 0: (85) call unknown#51
> invalid func unknown#51
> Failed to load BPF program trace_count_0: Invalid argument
>
>
> Is something that I'm missing on the bcc side or on bpf side ?
>

In kernel, you need to add your function proto to kprobe_prog_func_proto
in kernel/trace/bpf_trace.c


>
> Bests
>
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [PATCH RFC] Add BPF AsmParser support in LLVM

2017-09-07 Thread Y Song via iovisor-dev
Hi, Jiong,

The patch looks great! I tested with a few bpf programs from
kernel:samples/bpf/ directory.
Your test case with verifier like input looks good. I will have a few
suggestions later.

I tested with compiler generated asm as well with both big endian and
little endian.
   . clang generate .ll
   . llc generate .asm
   . llvm-mc generate .o
   . llvm-objdump disassembles.
It works fine except in one case:

For ld_imm64, we have:
class LD_IMM64 Pseudo, string OpcodeStr>
: InstBPF<(outs GPR:$dst), (ins u64imm:$imm),
  "$dst "#OpcodeStr#" ${imm}ll",
  [(set GPR:$dst, (i64 imm:$imm))]>

In reality, the map it tries to access will not be constant, so
the current assembler prints like:
  r1 = ll

The assembler does not like this and this insn is ignored.
If I change the above to
  r1 = tx_port ll
as expressed in the test case, the code will be generated properly with
relocation record.

I guess, we may need to add something like LD_VAR_ADDR so that
the proper assembly code can be issued. Also we could drop "ll" with
"r1 = tx_port"?

Back to your test case bpf-insn-unit.s, you do not need add bpf in the file name
since it is already under BPF directory.

>  r0 = *(u16 *)skb[2] // BPF_LD | BPF_ABS | BPF_H
> r0 = * (u32*)skb[4] // BPF_LD | BPF_ABS | BPF_W
could you add BPF_B test as well?

>  r0 = *  (u16 *)skb[r1] // BPF_LD | BPF_IND | BPF_H
>  r0 = *(u32 *)skb[r2]   // BPF_LD | BPF_IND | BPF_W
BPF_B test?

>  r1 = 8589934591 ll // BPF_LD | BPF_DW | BPF_IMM
>  r1 = dummy_map ll  // Symbolic version

//  BPF_LDX Class 
  r5 = *(u8 *)(r0 + 0)   // BPF_LDX | BPF_B
  r6 = *(u16 *)(r1 + 8)  // BPF_LDX | BPF_H
  r7 = *(u32 *)(r2 + 16) // BPF_LDX | BPF_W
  r8 = *(u64 *)(r3 - 30) // BPF_LDX | BPF_DW

//  TODO: BPF_ST Class ===
> There is no insn here, you can remove this line for now.

//  BPF_STX Class 
  *(u8 *)(r0 + 0) = r7// BPF_STX | BPF_B
  *(u16 *)(r1 + 8) = r8   // BPF_STX | BPF_H
  *(u32 *)(r2 + 16) = r9  // BPF_STX | BPF_W
  *(u64 *)(r3 - 30) = r10 // BPF_STX | BPF_DW

  lock *(u32 *)(r2 + 16) += r9  // BPF_STX | BPF_W | BPF_XADD
  lock *(u64 *)(r3 - 30) += r10 // BPF_STX | BPF_DW | BPF_XADD

//  BPF_JMP Class 
  goto Llabel0  // BPF_JA
  if r0 == r1 goto Llabel0   // BPF_JEQ
  if r1 > r2 goto Llabel0// BPF_JGT
  if r2 >= r3 goto Llabel0   // BPF_JGE
  // TODO: BPF_JSET
> there is no BPF_JSET, you can remove this.
  if r3 != r4 goto Llabel0   // BPF_JNE
  if r4 s> r5 goto Llabel0   // BPF_JSGT
  if r5 s>= r6 goto Llabel0  // BPF_JSGE
  call 1 // BPF_CALL
  exit   // BPF_EXIT
  if r6 < r7 goto Llabel0// BPF_JLT
  if r7 <= r8 goto Llabel0   // BPF_JLE
  if r8 s< r9 goto Llabel0   // BPF_JSLT
  if r9 s<= r10 goto Llabel0 // BPF_JSLE

> Could you add test compare register vs. imm?

//  BPF_ALU64 Class 
  r0 += r1// BPF_ADD
  r1 -= r2// BPF_SUB
  r2 *= r3// BPF_MUL
  r3 /= r4// BPF_DIV
Llabel0:
  r4 |= r5// BPF_OR
  r5 &= r6// BPF_AND
  r6 <<= r7   // BPF_LSH
  r7 >>= r8   // BPF_RSH
  // TODO: BPF_NEG
  // TODO: BPF_MOD
> There is no BPF_NEG/MOD yet, you can remove these two lines.

  r8 ^= r9// BPF_XOR
  r9 = r10// BPF_MOV
  r10 s>>= r0 // BPF_ARSH
> Could you add arith operations with register and imm. operands?

  bswap16 r1  // BPF_END | BPF_TO_BE
  bswap32 r2  // BPF_END | BPF_TO_BE
  bswap64 r3  // BPF_END | BPF_TO_BE
  // TODO: BPF_END | BPF_TO_LE
> There is no BPF_TO_LE yet, you can remove this line.

> Could you interleave the CHECK results with insn? This will be
clearer to do manual comparison.

On Thu, Sep 7, 2017 at 1:43 AM, Jiong Wang via iovisor-dev
 wrote:
> Hi,
>
>   As discussed at threads:
>
> https://www.spinics.net/lists/xdp-newbies/msg00320.html
> https://www.spinics.net/lists/xdp-newbies/msg00323.html
>
>   This patch adds the initial BPF AsmParser support in LLVM.
>
>   This support is following the "verifier instruction format" which is
> C-like.
>
>   Things need to mention are:
>
> 1. LLVM AsmParser doesn't expect the character "*" to be used as the
> start
>of a statement while BPF "verifier instruction format" is.  So an new
>generic method "starIsStartOfStatement" is introduced.
>
> 2. As the MC assembler is sharing code infrastructure with disassembler,
> the
>current supported instruction format is *strictly* those registered
> in
>instruction information .td files.  For example, the parser doesn't
>acccept "*(u8 *)(r0) = r7", instead, you need to write
>"*(u8 *)(r0 + 0) = r7". The offset is mandatory, even when it is
> zero.
>The instruction information .td files may need to register customized
>ParserMethods to allow more flexible syntaxes.
>
>   The testcase bpf-insn-unit.s contains unit tests for supported syntaxes.
>
>   

Re: [iovisor-dev] Getting stack traces from tracepoint probes

2017-09-05 Thread Y Song via iovisor-dev
On Tue, Sep 5, 2017 at 10:01 AM, Sasha Goldshtein <golds...@gmail.com> wrote:
> On Tue, Sep 5, 2017 at 7:40 PM Y Song via iovisor-dev
> <iovisor-dev@lists.iovisor.org> wrote:
>>
>> On Tue, Sep 5, 2017 at 7:43 AM, Nair, Reena via iovisor-dev
>> <iovisor-dev@lists.iovisor.org> wrote:
>> > Hi,
>> >
>> >
>> > I was trying to retrieve stack traces from tracepoint probe by modifying
>> > offcputime.py
>> >
>> >
>> > The original code is in blue and the changed code is in yellow.
>> >
>> >
>> > //int oncpu(struct pt_regs *ctx, struct task_struct *prev) {
>> > TRACEPOINT_PROBE(sched, sched_switch){
>> >
>> > //u32 pid = prev->pid;
>> > //u32 tgid = prev->tgid;
>> >
>> > u32 pid = args->prev_pid, tgid;
>> > u64 ts, *tsp;
>> > // record previous thread sleep time
>> > if ((THREAD_FILTER) && (STATE_FILTER)) {
>> > ts = bpf_ktime_get_ns();
>> > start.update(, );
>> > }
>> > // get the current thread's start time
>> > //pid = bpf_get_current_pid_tgid();
>> >
>> > pid = args->next_pid;
>> > tgid = bpf_get_current_pid_tgid() >> 32;
>> >
>> > ...
>> > ...
>> > ...
>> >
>> > I also replaced 'ctx' with 'args'
>> >
>> > kernel_stack_get = "stack_traces.get_stackid(args, BPF_F_REUSE_STACKID)"
>> > user_stack_get = \
>> > "stack_traces.get_stackid(args, BPF_F_REUSE_STACKID |
>> > BPF_F_USER_STACK)"
>>
>> This won't work. get_stackid needs pt_regs so that it can walk through
>> the frame to
>> get stacks. Here for tracepoint, "args" is the tracepoint structure.
>
>
> It works for me in the 'trace' tool, e.g.:

Now I remember. The tracepoint get_stackid should still work.
In kernel, we store the pt_regs in the first 8-byte unused args space and use it
to walk through the stacks.

>
> # /usr/share/bcc/tools/trace -K t:sched:sched_process_fork
> PIDTIDCOMM FUNC
> 1632   1632   sshd sched_process_fork
> _do_fork+0x1e9 [kernel]
> SyS_clone+0x19 [kernel]
> do_syscall_64+0x5e [kernel]
> return_from_SYSCALL_64+0x0 [kernel]
>
> The tool uses stacks.get_stack_id(args, ...) exactly like the code above.
> Indeed it looks like the patch linked below is responsible for enabling it
> -- unless I'm missing something.
>>
>>
>> >
>> > However, no stack traces, kernel/ user, were returned. Is it not
>> > possible to
>> > get stack traces from tracepoint probes?
>> >
>> > I came across this link:
>> > https://patchwork.kernel.org/patch/8747201/
>> >
>> > Do we have any functions to access the pt_regs from tracepoint probes? I
>> > am
>> > running the probes in kernel version 4.8.
>> >
>> > Thanks,
>> > Reena.
>> >
>> >
>> >
>> >
>> > ___
>> > iovisor-dev mailing list
>> > iovisor-dev@lists.iovisor.org
>> > https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>> >
>> ___
>> iovisor-dev mailing list
>> iovisor-dev@lists.iovisor.org
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Getting stack traces from tracepoint probes

2017-09-05 Thread Y Song via iovisor-dev
On Tue, Sep 5, 2017 at 7:43 AM, Nair, Reena via iovisor-dev
 wrote:
> Hi,
>
>
> I was trying to retrieve stack traces from tracepoint probe by modifying
> offcputime.py
>
>
> The original code is in blue and the changed code is in yellow.
>
>
> //int oncpu(struct pt_regs *ctx, struct task_struct *prev) {
> TRACEPOINT_PROBE(sched, sched_switch){
>
> //u32 pid = prev->pid;
> //u32 tgid = prev->tgid;
>
> u32 pid = args->prev_pid, tgid;
> u64 ts, *tsp;
> // record previous thread sleep time
> if ((THREAD_FILTER) && (STATE_FILTER)) {
> ts = bpf_ktime_get_ns();
> start.update(, );
> }
> // get the current thread's start time
> //pid = bpf_get_current_pid_tgid();
>
> pid = args->next_pid;
> tgid = bpf_get_current_pid_tgid() >> 32;
>
> ...
> ...
> ...
>
> I also replaced 'ctx' with 'args'
>
> kernel_stack_get = "stack_traces.get_stackid(args, BPF_F_REUSE_STACKID)"
> user_stack_get = \
> "stack_traces.get_stackid(args, BPF_F_REUSE_STACKID | BPF_F_USER_STACK)"

This won't work. get_stackid needs pt_regs so that it can walk through
the frame to
get stacks. Here for tracepoint, "args" is the tracepoint structure.

>
> However, no stack traces, kernel/ user, were returned. Is it not possible to
> get stack traces from tracepoint probes?
>
> I came across this link:
> https://patchwork.kernel.org/patch/8747201/
>
> Do we have any functions to access the pt_regs from tracepoint probes? I am
> running the probes in kernel version 4.8.
>
> Thanks,
> Reena.
>
>
>
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] modifying packets in XDP

2017-08-24 Thread Y Song via iovisor-dev
This is the problem:
static inline uint16_t parse_tcp_pld(void *data, u64 nh_off, void *data_end) {
  uint16_t *pld = data + nh_off;
  if ((void*)[1] > data_end)
return 0;
  return pld;
}

return value should be "void *" or "uint16_t *".

In the future, we will try to add more diagnostic information to debug
such issues.


On Thu, Aug 24, 2017 at 12:52 PM, Ilya Baldin  wrote:
> If I just copy your swapu16 function into my code (and it looks very similar 
> to mine) the result continues to be the same (error).
>
> Here is a gist with python and c-components of my program
>
> https://gist.github.com/ibaldin/716d70d490b44e05d15db59ee983c0c0
>
> Looking at the BPF byte code print out from BCC (below) something strange is 
> indeed going on that I think Yonghong alluded to.
>
> The last instruction is r2 = *(u16*)(r1 + 8), and r1 is invalid. R1 is 
> initially set, but then for some reason becomes invalid:
>
> 50: (2d) if r4 > r2 goto pc+35
>  R1=pkt(id=2,off=0,r=2),aux_off_align=2 R2=pkt_end 
> R3=pkt(id=1,off=0,r=20),aux_off_align=2 
> R4=pkt(id=2,off=2,r=2),aux_off_align=2 
> R5=inv56,min_value=22,max_value=142,min_align=2,aux_off_align=2 R10=fp
> 51: (57) r1 &= 65535
> 52: (15) if r1 == 0x0 goto pc+33
>  R1=inv,min_value=0,max_value=65535,min_align=1,aux_off_align=2 R2=pkt_end 
> R3=pkt(id=1,off=0,r=20),aux_off_align=2 
> R4=pkt(id=2,off=2,r=2),aux_off_align=2 R5=inv56,min_value=22,max_value=14
>
>
> —— FULL TEXT OF BPF PROGRAM BYTECODE ———
>
>
> 0: (61) r2 = *(u32 *)(r1 +4)
> 1: (61) r1 = *(u32 *)(r1 +0)
> 2: (bf) r3 = r1
> 3: (07) r3 += 14
> 4: (2d) if r3 > r2 goto pc+81
>  R1=pkt(id=0,off=0,r=14) R2=pkt_end R3=pkt(id=0,off=14,r=14) R10=fp
> 5: (71) r3 = *(u8 *)(r1 +12)
> 6: (71) r4 = *(u8 *)(r1 +13)
> 7: (67) r4 <<= 8
> 8: (4f) r4 |= r3
> 9: (15) if r4 == 0xa888 goto pc+2
>  R1=pkt(id=0,off=0,r=14) R2=pkt_end R3=inv56 R4=inv R10=fp
> 10: (b7) r3 = 14
> 11: (55) if r4 != 0x81 goto pc+4
>  R1=pkt(id=0,off=0,r=14) R2=pkt_end 
> R3=imm14,min_value=14,max_value=14,min_align=2 
> R4=inv,min_value=129,max_value=129 R10=fp
> 12: (b7) r3 = 18
> 13: (bf) r5 = r1
> 14: (07) r5 += 18
> 15: (2d) if r5 > r2 goto pc+70
>  R1=pkt(id=0,off=0,r=18) R2=pkt_end 
> R3=imm18,min_value=18,max_value=18,min_align=2 
> R4=inv,min_value=129,max_value=129 R5=pkt(id=0,off=18,r=18) R10=fp
> 16: (15) if r4 == 0xa888 goto pc+1
>  R1=pkt(id=0,off=0,r=18) R2=pkt_end 
> R3=imm18,min_value=18,max_value=18,min_align=2 
> R4=inv,min_value=129,max_value=129 R5=pkt(id=0,off=18,r=18) R10=fp
> 17: (55) if r4 != 0x81 goto pc+4
>  R1=pkt(id=0,off=0,r=18) R2=pkt_end 
> R3=imm18,min_value=18,max_value=18,min_align=2 
> R4=inv,min_value=129,max_value=129 R5=pkt(id=0,off=18,r=18) R10=fp
> 18: (07) r3 += 4
> 19: (bf) r5 = r1
> 20: (0f) r5 += r3
> 21: (2d) if r5 > r2 goto pc+64
>  R1=pkt(id=0,off=0,r=22) R2=pkt_end 
> R3=imm22,min_value=22,max_value=22,min_align=2 
> R4=inv,min_value=129,max_value=129 R5=pkt(id=0,off=22,r=22) R10=fp
> 22: (55) if r4 != 0x8 goto pc+63
>  R1=pkt(id=0,off=0,r=22) R2=pkt_end 
> R3=imm22,min_value=22,max_value=22,min_align=2 R4=inv,min_value=8,max_value=8 
> R5=pkt(id=0,off=22,r=22) R10=fp
> 23: (bf) r4 = r1
> 24: (0f) r4 += r3
> 25: (15) if r4 == 0x0 goto pc+60
>  R1=pkt(id=0,off=0,r=22) R2=pkt_end 
> R3=imm22,min_value=22,max_value=22,min_align=2 R4=pkt(id=0,off=22,r=22) 
> R5=pkt(id=0,off=22,r=22) R10=fp
> 26: (bf) r5 = r4
> 27: (07) r5 += 20
> 28: (2d) if r5 > r2 goto pc+57
>  R1=pkt(id=0,off=0,r=42) R2=pkt_end 
> R3=imm22,min_value=22,max_value=22,min_align=2 R4=pkt(id=0,off=22,r=42) 
> R5=pkt(id=0,off=42,r=42) R10=fp
> 29: (71) r4 = *(u8 *)(r4 +9)
> 30: (55) if r4 != 0x6 goto pc+55
>  R1=pkt(id=0,off=0,r=42) R2=pkt_end 
> R3=imm22,min_value=22,max_value=22,min_align=2 
> R4=inv56,min_value=6,max_value=6 R5=pkt(id=0,off=42,r=42) R10=fp
> 31: (bf) r4 = r1
> 32: (0f) r4 += r3
> 33: (71) r4 = *(u8 *)(r4 +0)
> 34: (57) r4 &= 15
> 35: (67) r4 <<= 2
> 36: (0f) r4 += r3
> 37: (bf) r3 = r1
> 38: (0f) r3 += r4
> 39: (15) if r3 == 0x0 goto pc+46
>  R1=pkt(id=0,off=0,r=42) R2=pkt_end R3=pkt(id=1,off=0,r=0),aux_off_align=2 
> R4=inv57,min_value=22,max_value=82,min_align=2 R5=pkt(id=0,off=42,r=42) R10=fp
> 40: (bf) r5 = r3
> 41: (07) r5 += 20
> 42: (2d) if r5 > r2 goto pc+43
>  R1=pkt(id=0,off=0,r=42) R2=pkt_end R3=pkt(id=1,off=0,r=20),aux_off_align=2 
> R4=inv57,min_value=22,max_value=82,min_align=2 
> R5=pkt(id=1,off=20,r=20),aux_off_align=2 R10=fp
> 43: (69) r5 = *(u16 *)(r3 +12)
> 44: (77) r5 >>= 2
> 45: (57) r5 &= 60
> 46: (0f) r5 += r4
> 47: (0f) r1 += r5
> 48: (bf) r4 = r1
> 49: (07) r4 += 2
> 50: (2d) if r4 > r2 goto pc+35
>  R1=pkt(id=2,off=0,r=2),aux_off_align=2 R2=pkt_end 
> R3=pkt(id=1,off=0,r=20),aux_off_align=2 
> R4=pkt(id=2,off=2,r=2),aux_off_align=2 
> R5=inv56,min_value=22,max_value=142,min_align=2,aux_off_align=2 R10=fp
> 51: (57) r1 &= 65535
> 52: (15) if r1 == 0x0 goto pc+33
>  R1=inv,min_value=0,max_value=65535,min_align=1,aux_off_align=2 R2=pkt_end 
> 

Re: [iovisor-dev] modifying packets in XDP

2017-08-24 Thread Y Song via iovisor-dev
CC to bcc mailing list as well.

bpf_probe_read is not allowed in XDP programs.

Your comparison may need to comparison not just starting offset, but
also including the memory size so the
whole write won't fall beyond the "data_end".

Regarding to bcc translates "start[off2]" to bpf_probe_read, it is
possible that bcc rewriter tries to infer bpf_probe candidate and
finds this one ...

On Thu, Aug 24, 2017 at 8:22 AM, Ilya Baldin  wrote:
> Hello everyone,
>
> A couple of questions. I’m using XDP with BCC on a Fedora 25 with kernel 
> 4.13.0-0.rc5.git0.2.fc27.x86_64 with e1000 driver. Basic XDP examples appear 
> to work, I was able to create my own simple example counting TCP SYN packets, 
> so the setup is at least partially correct.
>
> I’m playing around now with modifying TCP payload on the fly and failing 
> miserably. The function that is supposed to swap two u16s in TCP payload is 
> below
>
> static inline int swapu16(uint16_t *start, void *data_end, int off1, int 
> off2) {
>   int len = (uint16_t*)data_end - start;
>   uint16_t poff1, poff2;
>
>   if (((void*)[off1] > data_end) || ((void*)[off2] > data_end))
> return 0;
>
>   poff1 = start[off1];
>   poff2 = start[off2];
>
>   //start[off2] = poff1;
>   //start[off1] = poff2;
>
>   return 1;
> }
>
> and it gets called with start pointing to the beginning of the payload (I’m 
> reasonably sure that is correct).
>
> If I *uncomment* either of the two lines in the function I get verifier 
> errors suggesting I use bpf_probe_read. First question
>
> 1. Is what I’m attempting even possible - am I allowed to modify the packet 
> in XDP hook? If no this may be a short conversation.
>
> 2. If it is possible, is there a canonical way of doing it compared to what 
> I’m trying to do?
>
> 3. I actually attempted to use bpf_probe_read() (even though with the code 
> structured as above, it appears they are inserted automatically, because the 
> BPF program is installed properly with those two lines commented out), like 
> in the code shown below:
>
> static inline int swapu16(uint16_t *start, void *data_end, int off1, int 
> off2) {
>   int len = (uint16_t*)data_end - start;
>   uint16_t poff1, poff2;
>
>   if (((void*)[off1] > data_end) || ((void*)[off2] > data_end))
> return 0;
>
>   bpf_probe_read(, sizeof(uint16_t), (void*)[off1]);
>   //poff1 = start[off1];
>   //poff2 = start[off2];
>
>   //start[off2] = poff1;
>   //start[off1] = poff2;
>
>   return 1;
> }
>
> I get a verifier error
> ….
> 60: (2d) if r1 > r2 goto pc+5
>  R1=inv,min_value=4,max_value=65539,min_align=1,aux_off_align=2 R2=pkt_end 
> R3=inv,min_value=2,max_value=65537,min_align=1,aux_off_align=2 
> R4=imm0,min_value=0,max_value=0,min_align=2147483648 R5=pkt(id=0,off=42,r=42) 
> R6=pkt(id=1,off=0,r=20),aux_off_align=2 
> R7=imm0,min_value=0,max_value=0,min_align=2147483648 R10=fp
> 61: (bf) r1 = r10
> 62: (07) r1 += -32
> 63: (b7) r2 = 2
> 64: (85) call bpf_probe_read#4
> unknown func bpf_probe_read#4
>
> which is really strange. In fact it appears I’m unable to invoke any of the 
> helper bpf functions explicitly.
>
> Many thanks for your suggestions.
>
> -ilya
>
> Ilya Baldin
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] bcc tools profile.py: open procmap fails

2017-08-16 Thread Y Song via iovisor-dev
>
> // always fails, with errno = 2
> procmap = fopen(procmap_filename, "r");
>

errno 2 (ENOENT) means file does not exist. You can add additional
print to check whether the file truly exists or not.

On Sat, Aug 12, 2017 at 5:14 AM, Fenggang Wu via iovisor-dev
 wrote:
> Hi there,
>
> I am trying out bcc tools from http://github.com/iovisor/bcc, more
> specificaly profile.py and offcputime.py. However in the tracing result,
> user functions are shown as "[unknown]", whereas kernel functions such as
> "sys_*" are interpreted well. Any ideas?
>
> Thank you very much!
> Fenggang
>
> ==
>
> Here are more details (I use profile.py as the example). If there are more
> details that are necessary please feel free to let me know.
>
> 1) how to reproduce the issue:
> #~> cat Makefile
> all:
> g++ -o spin spin.cc
> objcopy --only-keep-debug spin spin.debug
> objcopy --add-gnu-debuglink=spin.debug spin
>
>
> clean:
> rm spin spin.debug
> #~> cat spin.cc
> #include
> using namespace std;
> void myfunc() {
>   int i, j, cnt;
>   cout << "my func" << endl;
>   for (i = 0; i < 4; i++) {
> for ( j = 0; j < 5; j++) {
>   cnt ^= i + j;
> }
> cout << i << " " << cnt;
>   }
> }
>
> int main() {
>   cout << "hello" << endl;
>   myfunc();
> }
> #~> cat test_profile.sh
> #!/bin/bash
> sudo make
>
> sudo -- bash -c  "./spin & PID=\$!; echo PID=\$PID; cat /proc/\$PID/maps;
> (trap - SIGINT; profile -U -p \$PID > spin.profile)& wait \$PID;"; sudo kill
> -SIGINT `pgrep -x profile`; sleep 1; cat spin.profile;
> sleep 1
> sudo make clean
>
> #~>./test_profile.sh
>
> Here, in my server,  spin.profile cannot resolve user-level symbols (i.e.
> symbols in spin.cc). It only shows userland symbol names as "[unknown]".
>
> I am using Ubuntu 16.04.3 LTS with Linux Kernel ver. 4.10.0-28-generic. I've
> tried both installing by apt-get or building from source (latest pull from
> github). the results are the same: user-land functions names cannot be
> resolved.
>
> 2) What I've tried/found:
>
> - perf_event works well under my environment in resolving both the kernel
> and userspace symbols.
> - bcc/test/python/test_debuginfo.py pass all tests. Showing that build-id
> and debug-link both methods work in bcc.
> - [MAJOR PROBLEM] I found the failed statement:
> src/cc/bcc_proc.c:78:
> int bcc_procutils_each_module(int pid, bcc_procutils_modulecb callback, void
> *payload) {
>
> ...
> // always fails, with errno = 2
> procmap = fopen(procmap_filename, "r");
> ...
>
> }
>
> - Afterwards, I've written a similar little c++ program to read
> "/proc/pid/maps", and it can open/read the proc map file successfully.
> Besides, I can also cat the proc map file in the shell, the file can be read
> successfully too.
>
> the little testing c++ program to open and read the procmap file is as
> follows (very similar to bcc_procutils_each_module):
>
> #~> cat read-proc-map.cc
> #include 
> #include 
> #include 
> #include 
>
> int main(int argc, char*argv[]) {
>   char procmap_filename[128];
>   FILE *procmap;
>   int ret;
>
>   printf("%d\n", argc);
>
>   if (argc != 2) return -1;
>
>   printf("mytest\n");
>
>   snprintf(procmap_filename, sizeof(procmap_filename),
>   "/proc/%s/maps", argv[1]);
>   procmap = fopen(procmap_filename, "r");
>
>   printf("mytest: fopen <%s>\n", procmap_filename);
>
>   if (!procmap) {
> printf("mytest:   ... failed errno=%d\n", errno);
> return -1;
>   }
>
>   printf("mytest:   ... success <%s>\n", procmap_filename);
>
>
>   do {
> char endline[4096];
> char perm[8], dev[8];
> long long begin, end, size, inode;
>
> ret = fscanf(procmap, "%llx-%llx %s %llx %s %lld", , , perm,
> , dev, );
>
> if (!fgets(endline, sizeof(endline), procmap))
>   break;
>
> if (ret == 6) {
>   char *mapname = endline;
>   char *newline = strchr(endline, '\n');
>
>   if (newline)
> newline[0] = '\0';
>
>   while (isspace(mapname[0])) mapname++;
>
>   printf("%s, %llx, %llx\n", mapname, begin, end);
> }
>   } while (ret && ret != EOF);
>
>   fclose(procmap);
>   printf("mytest: fclosed %s\n", procmap_filename);
>   return 0;
> }
>
> 
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [PATCH v3 net-next 00/12] bpf: rewrite value tracking in verifier

2017-07-14 Thread Y Song via iovisor-dev
I did an experiment with one of our internal bpf programs.
The program has 1563 insns.

Without Edward's patch:
processed 13634 insns, stack depth 160

With Edward's patch:
processed 15807 insns, stack depth 160

So the number of processed insns regressed by roughly 16%.
Did anybody do any similar experiments to quantify the patch's
impact in verification performance (e.g., in terms of processed insns)?

On Tue, Jun 27, 2017 at 5:53 AM, Edward Cree via iovisor-dev
 wrote:
> This series simplifies alignment tracking, generalises bounds tracking and
>  fixes some bounds-tracking bugs in the BPF verifier.  Pointer arithmetic on
>  packet pointers, stack pointers, map value pointers and context pointers has
>  been unified, and bounds on these pointers are only checked when the pointer
>  is dereferenced.
> Operations on pointers which destroy all relation to the original pointer
>  (such as multiplies and shifts) are disallowed if !env->allow_ptr_leaks,
>  otherwise they convert the pointer to an unknown scalar and feed it to the
>  normal scalar arithmetic handling.
> Pointer types have been unified with the corresponding adjusted-pointer types
>  where those existed (e.g. PTR_TO_MAP_VALUE[_ADJ] or FRAME_PTR vs
>  PTR_TO_STACK); similarly, CONST_IMM and UNKNOWN_VALUE have been unified into
>  SCALAR_VALUE.
> Pointer types (except CONST_PTR_TO_MAP, PTR_TO_MAP_VALUE_OR_NULL and
>  PTR_TO_PACKET_END, which do not allow arithmetic) have a 'fixed offset' and
>  a 'variable offset'; the former is used when e.g. adding an immediate or a
>  known-constant register, as long as it does not overflow.  Otherwise the
>  latter is used, and any operation creating a new variable offset creates a
>  new 'id' (and, for PTR_TO_PACKET, clears the 'range').
> SCALAR_VALUEs use the 'variable offset' fields to track the range of possible
>  values; the 'fixed offset' should never be set on a scalar.
>
> As of patch 12/12, all tests of tools/testing/selftests/bpf/test_verifier
>  and tools/testing/selftests/bpf/test_align pass.
>
> v3: added a few more tests; removed RFC tags.
>
> v2: fixed nfp build, made test_align pass again and extended it with a few
>  new tests (though still need to add more).
>
> Edward Cree (12):
>   selftests/bpf: add test for mixed signed and unsigned bounds checks
>   bpf/verifier: rework value tracking
>   nfp: change bpf verifier hooks to match new verifier data structures
>   bpf/verifier: track signed and unsigned min/max values
>   bpf/verifier: more concise register state logs for constant var_off
>   selftests/bpf: change test_verifier expectations
>   selftests/bpf: rewrite test_align
>   selftests/bpf: add a test to test_align
>   selftests/bpf: add test for bogus operations on pointers
>   selftests/bpf: don't try to access past MAX_PACKET_OFF in
> test_verifier
>   selftests/bpf: add tests for subtraction & negative numbers
>   selftests/bpf: variable offset negative tests
>
>  drivers/net/ethernet/netronome/nfp/bpf/verifier.c |   24 +-
>  include/linux/bpf.h   |   34 +-
>  include/linux/bpf_verifier.h  |   56 +-
>  include/linux/tnum.h  |   81 +
>  kernel/bpf/Makefile   |2 +-
>  kernel/bpf/tnum.c |  180 ++
>  kernel/bpf/verifier.c | 1943 
> -
>  tools/testing/selftests/bpf/test_align.c  |  462 -
>  tools/testing/selftests/bpf/test_verifier.c   |  293 ++--
>  9 files changed, 2034 insertions(+), 1041 deletions(-)
>  create mode 100644 include/linux/tnum.h
>  create mode 100644 kernel/bpf/tnum.c
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] LLVM backend handling of packed structures

2017-07-04 Thread Y Song via iovisor-dev
On Mon, Jul 3, 2017 at 8:23 PM, Nadav Amit via iovisor-dev
 wrote:
> I get a strange phenomenon with packed structs in which each byte is
> loaded in a different instruction.
>
> Consider for example xdp_prog1 for Linux samples. After building it, the
> disassembly starts with:
>
> xdp_prog1:
>0:   61 12 04 00 00 00 00 00 r2 = *(u32 *)(r1 + 4)
>1:   61 11 00 00 00 00 00 00 r1 = *(u32 *)(r1 + 0)
>2:   bf 13 00 00 00 00 00 00 r3 = r1
>3:   07 03 00 00 0e 00 00 00 r3 += 14
>4:   2d 23 37 00 00 00 00 00 if r3 > r2 goto 55
>5:   71 13 0c 00 00 00 00 00 r3 = *(u8 *)(r1 + 12)
>6:   71 14 0d 00 00 00 00 00 r4 = *(u8 *)(r1 + 13)
>7:   67 04 00 00 08 00 00 00 r4 <<= 8
>8:   4f 34 00 00 00 00 00 00 r4 |= r3
>9:   15 04 02 00 88 a8 00 00 if r4 == 43144 goto 2
>   10:   b7 03 00 00 0e 00 00 00 r3 = 14
>   11:   55 04 05 00 81 00 00 00 if r4 != 129 goto 5
>
>
> As you can see, the games in instructions 5-8 that access h_proto (u16) are
> very unnecessary. Redefining 'struct ethhdr’ as “unpacked” (i.e., without
> the packed attribute), eliminates this behavior.
>
> xdp_prog1:
>0:   61 12 04 00 00 00 00 00 r2 = *(u32 *)(r1 + 4)
>1:   61 11 00 00 00 00 00 00 r1 = *(u32 *)(r1 + 0)
>2:   bf 13 00 00 00 00 00 00 r3 = r1
>3:   07 03 00 00 0e 00 00 00 r3 += 14
>4:   2d 23 34 00 00 00 00 00 if r3 > r2 goto 52
>5:   69 14 0c 00 00 00 00 00 r4 = *(u16 *)(r1 + 12)
>6:   15 04 02 00 88 a8 00 00 if r4 == 43144 goto 2
>7:   b7 03 00 00 0e 00 00 00 r3 = 14
>8:   55 04 05 00 81 00 00 00 if r4 != 129 goto 5
>
> As you can see, now the the variable is accessed in one chunk, as expected.
>
> Any clues to what went wrong?

This is expected from current llvm implementation. For unpacked, the
compiler is able to use field natural alignment to decide which load
variant to use.
For packed, the field alignment is assumed to be 1, so it use load-byte.

>
> Thanks,
> Nadav
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] Trace point events missing with bpf_trace_printk

2017-06-30 Thread Y Song via iovisor-dev
On Fri, Jun 30, 2017 at 6:12 AM, Nair, Reena via iovisor-dev
 wrote:
> Hi,
>
>
> I am trying to track the scheduling events of a particular process using the
> static tracepoint:  sched_switch.
>
> A map entry is updated for each context switch, which is working fine,
> however some of the events are missed while printing the value using
> bpf_trace_printk() (Each context switch updates the value by '1', however
> sometimes the output printed is not continuous).
>
> What could be the reason?

If you try to print too much in a very short period of time, kernel
may skip some of prints.

>
> Why is it given that PERF_OUTPUT is better than bpf_trace_printk?

perf_output is per process and bpf_trace_printk is global.

>
> Is there any way to use PERF_OUTPUT with static tracepoints?

Yes. You can take a look at bcc/tests/python/test_usdt.py as an example.

>
>
> Many Thanks,
>
> Reena
>
>
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [iovisor/bcc] USDT broken by b0f891d129a9b372

2017-06-06 Thread Y Song via iovisor-dev
100% agree. Will work on a test case soon (maybe next week).

On Tue, Jun 6, 2017 at 9:01 PM, Brenden Blanco <bbla...@gmail.com> wrote:
> Also, I would suggest that after that fix, we also update the test to
> include actually testing for proper values. Probably you can send the values
> over the perf ring buffer and assert at the end that all of the values are
> collected properly.
>
> On Tue, Jun 6, 2017 at 8:59 PM, Brenden Blanco <bbla...@gmail.com> wrote:
>>
>> Yonghong, can you send a PR for your branch?
>>
>> On Tue, Jun 6, 2017 at 12:20 AM, Y Song via iovisor-dev
>> <iovisor-dev@lists.iovisor.org> wrote:
>>>
>>> Hi, Tetsuo,
>>>
>>> You are right. The bug is actually introduced by my last patch. I just
>>> focused one aspect of issue and inadvertently introduced another
>>> *blocker* bug.
>>>
>>> I just pushed a patch. Thanks a lot for reporting the issue and
>>> testing out the fix.
>>>
>>> Yonghong
>>>
>>> On Mon, Jun 5, 2017 at 5:25 PM, Tetsuo Handa
>>> <penguin-ker...@i-love.sakura.ne.jp> wrote:
>>> > Hello.
>>> >
>>> > I changed the hook as below and confirmed that cp == NULL at
>>> > bpf_probe_read().
>>> > That is, it is bpf_usdt_readarg() which got broken.
>>> >
>>> > --
>>> > int do_start(struct pt_regs *ctx) {
>>> > char *cp = NULL;
>>> > bpf_usdt_readarg(1, ctx, );
>>> > struct { char query[QUERY_MAX]; } data = { };
>>> > if (cp != NULL)
>>> > bpf_probe_read(, sizeof(data.query), (void *) cp);
>>> > else
>>> > data.query[0] = '!';
>>> > events.perf_submit(ctx, , sizeof(data));
>>> > return 0;
>>> > };
>>> > --
>>> >
>>> > If I do
>>> >
>>> > -std::string cptr = tfm::format("*((volatile %s *)dest)", ctype);
>>> > +std::string cptr = tfm::format("*((%s *)dest)", ctype);
>>> >
>>> > (i.e. drop only "volatile " part) in that commit like below
>>> >
>>> > --
>>> > diff --git a/src/cc/usdt.cc b/src/cc/usdt.cc
>>> > --- a/src/cc/usdt.cc
>>> > +++ b/src/cc/usdt.cc
>>> > @@ -152,7 +152,7 @@ bool Probe::usdt_getarg(std::ostream ) {
>>> >
>>> >for (size_t arg_n = 0; arg_n < arg_count; ++arg_n) {
>>> >  std::string ctype = largest_arg_type(arg_n);
>>> > -std::string cptr("dest");
>>> > +std::string cptr = tfm::format("*((%s *)dest)", ctype);
>>> >
>>> >  tfm::format(stream,
>>> >  "static __always_inline int _bpf_readarg_%s_%d("
>>> > --
>>> >
>>> > USDT probe works again as well as build warning shown below goes away.
>>> >
>>> > --
>>> > /virtual/main.c:4:8: warning: incompatible integer to pointer
>>> > conversion assigning to 'void *' from 'volatile uint64_t' (aka 'volatile
>>> > unsigned long long') [-Wint-conversion]
>>> >   dest = *(volatile uint64_t *)>bx;
>>> >^ ~~
>>> > --
>>> ___
>>> iovisor-dev mailing list
>>> iovisor-dev@lists.iovisor.org
>>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>>
>>
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [iovisor/bcc] USDT broken by b0f891d129a9b372

2017-06-06 Thread Y Song via iovisor-dev
Done. Sorry, I actually pushed the change to my yhs_dev branch last
night, but forgot to generate a pull request ...

On Tue, Jun 6, 2017 at 8:59 PM, Brenden Blanco <bbla...@gmail.com> wrote:
> Yonghong, can you send a PR for your branch?
>
> On Tue, Jun 6, 2017 at 12:20 AM, Y Song via iovisor-dev
> <iovisor-dev@lists.iovisor.org> wrote:
>>
>> Hi, Tetsuo,
>>
>> You are right. The bug is actually introduced by my last patch. I just
>> focused one aspect of issue and inadvertently introduced another
>> *blocker* bug.
>>
>> I just pushed a patch. Thanks a lot for reporting the issue and
>> testing out the fix.
>>
>> Yonghong
>>
>> On Mon, Jun 5, 2017 at 5:25 PM, Tetsuo Handa
>> <penguin-ker...@i-love.sakura.ne.jp> wrote:
>> > Hello.
>> >
>> > I changed the hook as below and confirmed that cp == NULL at
>> > bpf_probe_read().
>> > That is, it is bpf_usdt_readarg() which got broken.
>> >
>> > --
>> > int do_start(struct pt_regs *ctx) {
>> > char *cp = NULL;
>> > bpf_usdt_readarg(1, ctx, );
>> > struct { char query[QUERY_MAX]; } data = { };
>> > if (cp != NULL)
>> > bpf_probe_read(, sizeof(data.query), (void *) cp);
>> > else
>> > data.query[0] = '!';
>> > events.perf_submit(ctx, , sizeof(data));
>> > return 0;
>> > };
>> > --
>> >
>> > If I do
>> >
>> > -std::string cptr = tfm::format("*((volatile %s *)dest)", ctype);
>> > +std::string cptr = tfm::format("*((%s *)dest)", ctype);
>> >
>> > (i.e. drop only "volatile " part) in that commit like below
>> >
>> > --
>> > diff --git a/src/cc/usdt.cc b/src/cc/usdt.cc
>> > --- a/src/cc/usdt.cc
>> > +++ b/src/cc/usdt.cc
>> > @@ -152,7 +152,7 @@ bool Probe::usdt_getarg(std::ostream ) {
>> >
>> >for (size_t arg_n = 0; arg_n < arg_count; ++arg_n) {
>> >  std::string ctype = largest_arg_type(arg_n);
>> > -std::string cptr("dest");
>> > +std::string cptr = tfm::format("*((%s *)dest)", ctype);
>> >
>> >  tfm::format(stream,
>> >  "static __always_inline int _bpf_readarg_%s_%d("
>> > --
>> >
>> > USDT probe works again as well as build warning shown below goes away.
>> >
>> > --
>> > /virtual/main.c:4:8: warning: incompatible integer to pointer conversion
>> > assigning to 'void *' from 'volatile uint64_t' (aka 'volatile unsigned long
>> > long') [-Wint-conversion]
>> >   dest = *(volatile uint64_t *)>bx;
>> >^ ~~
>> > --
>> ___
>> iovisor-dev mailing list
>> iovisor-dev@lists.iovisor.org
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [iovisor/bcc] USDT broken by b0f891d129a9b372

2017-06-06 Thread Y Song via iovisor-dev
Hi, Tetsuo,

You are right. The bug is actually introduced by my last patch. I just
focused one aspect of issue and inadvertently introduced another
*blocker* bug.

I just pushed a patch. Thanks a lot for reporting the issue and
testing out the fix.

Yonghong

On Mon, Jun 5, 2017 at 5:25 PM, Tetsuo Handa
 wrote:
> Hello.
>
> I changed the hook as below and confirmed that cp == NULL at bpf_probe_read().
> That is, it is bpf_usdt_readarg() which got broken.
>
> --
> int do_start(struct pt_regs *ctx) {
> char *cp = NULL;
> bpf_usdt_readarg(1, ctx, );
> struct { char query[QUERY_MAX]; } data = { };
> if (cp != NULL)
> bpf_probe_read(, sizeof(data.query), (void *) cp);
> else
> data.query[0] = '!';
> events.perf_submit(ctx, , sizeof(data));
> return 0;
> };
> --
>
> If I do
>
> -std::string cptr = tfm::format("*((volatile %s *)dest)", ctype);
> +std::string cptr = tfm::format("*((%s *)dest)", ctype);
>
> (i.e. drop only "volatile " part) in that commit like below
>
> --
> diff --git a/src/cc/usdt.cc b/src/cc/usdt.cc
> --- a/src/cc/usdt.cc
> +++ b/src/cc/usdt.cc
> @@ -152,7 +152,7 @@ bool Probe::usdt_getarg(std::ostream ) {
>
>for (size_t arg_n = 0; arg_n < arg_count; ++arg_n) {
>  std::string ctype = largest_arg_type(arg_n);
> -std::string cptr("dest");
> +std::string cptr = tfm::format("*((%s *)dest)", ctype);
>
>  tfm::format(stream,
>  "static __always_inline int _bpf_readarg_%s_%d("
> --
>
> USDT probe works again as well as build warning shown below goes away.
>
> --
> /virtual/main.c:4:8: warning: incompatible integer to pointer conversion 
> assigning to 'void *' from 'volatile uint64_t' (aka 'volatile unsigned long 
> long') [-Wint-conversion]
>   dest = *(volatile uint64_t *)>bx;
>^ ~~
> --
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] [iovisor/bcc] USDT broken by b0f891d129a9b372

2017-06-05 Thread Y Song via iovisor-dev
Tetsuo,

I cannot reproduce the issue.

The transformation from:
  dest = ctx->bx;
to:
  dest = *(volatile uint64_t *)>bx;

should be safe.

I launched a psql console and did a query:

...
yhs=> SELECT order_id FROM order_details;
 order_id
--
6
(1 row)
...

On the usdt side, I did not see anything printout.

I hacked the code to forcefully emit a meaningful string, but it does
not print either. So it looks like the probe function is not called.

int do_start(struct pt_regs *ctx) {
char *cp = NULL;
bpf_usdt_readarg(1, ctx, );
struct { char query[QUERY_MAX]; } data = { };
bpf_probe_read(, sizeof(data.query), (void *) cp);
data.query[0] = 'a'; data.query[1] = 0;
events.perf_submit(ctx, , sizeof(data));
return 0;
};

Could you double check on your side?

Yonghong

On Mon, Jun 5, 2017 at 3:27 AM, Tetsuo Handa via iovisor-dev
 wrote:
> Hello.
>
> I noticed that b0f891d129a9b372 ("Force udst ctx->#reg load to be volatile")
> broke an USDT probe example shown below.
>
> --
> #!/usr/bin/python
>>from bcc import BPF, USDT
> import sys
> import ctypes as ct
>
> def usage():
> print("USAGE: usdt PID")
> exit()
> if len(sys.argv) < 1:
> usage()
> if sys.argv[1][0:1] == "-":
> usage()
> pid = int(sys.argv[1])
> QUERY_MAX = 504
>
> bpf_text = """
> #include 
> #define QUERY_MAX   """ + str(QUERY_MAX) + """
> BPF_PERF_OUTPUT(events);
> int do_start(struct pt_regs *ctx) {
> char *cp = NULL;
> bpf_usdt_readarg(1, ctx, );
> struct { char query[QUERY_MAX]; } data = { };
> bpf_probe_read(, sizeof(data.query), (void *) cp);
> events.perf_submit(ctx, , sizeof(data));
> return 0;
> };
> """
>
> u = USDT(pid=pid)
> u.enable_probe(probe="query__start", fn_name="do_start")
> b = BPF(text=bpf_text, usdt_contexts=[u])
>
> class Data(ct.Structure):
> _fields_ = [ ("query", ct.c_char * QUERY_MAX) ]
>
> def print_event(cpu, data, size):
> event = ct.cast(data, ct.POINTER(Data)).contents
> print("%s" % (event.query))
>
> b["events"].open_perf_buffer(print_event, page_cnt=64)
> while 1:
> b.kprobe_poll()
> --
>
> Since bpf_probe_read() no longer reads PostgreSQL query string
> passed to query__start event, nothing is printed out.
> (Oh, I forgot to check whether cp != NULL.
> Maybe it is bpf_usdt_readarg() which got broken?)
>
> # git bisect log
> # bad: [505c110a5f4f48cff1a501f8b83a548ade6a7739] Add a python test for usdt
> # good: [aa4543ff2d42a2ca38a52963af9d6d078e48809a] Mention that linux-headers 
> deb must be installed.
> git bisect start '505c110a5f4f48cff1a501f8b83a548ade6a7739' 
> 'aa4543ff2d42a2ca38a52963af9d6d078e48809a'
> # good: [a934c903583c415ebed70d03a6c15e9b20c52efa] Add linux/sched.h to list 
> of libbpf.c includes
> git bisect good a934c903583c415ebed70d03a6c15e9b20c52efa
> # bad: [cb8e704ca64de58aab6137faa0b8d6b96f24a257] Merge pull request #1212 
> from iovisor/yhs_dev
> git bisect bad cb8e704ca64de58aab6137faa0b8d6b96f24a257
> # bad: [b0f891d129a9b372e7e8af5b951a1b63985afe26] Force udst ctx->#reg load 
> to be volatile
> git bisect bad b0f891d129a9b372e7e8af5b951a1b63985afe26
> # good: [e7b0ab2dc44b244ef6600630c4e7b16c1f3b123a] Merge pull request #1211 
> from iovisor/libbpf_c_includes
> git bisect good e7b0ab2dc44b244ef6600630c4e7b16c1f3b123a
> # first bad commit: [b0f891d129a9b372e7e8af5b951a1b63985afe26] Force udst 
> ctx->#reg load to be volatile
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


Re: [iovisor-dev] BPF unrolled loop doesn't work and its workaround

2017-04-18 Thread Y Song via iovisor-dev
Hi, William,

Your analysis is correct. In this case, the error message could be improved.
In general, a message like this "back-edge from insn 240 to 219" indicates
a loop (back-edge ...).

Compiler probably should give a warning if it is BPF backend to indicate
there
are loops in the byte code. I will look into this.

Yonghong


On Tue, Apr 18, 2017 at 9:00 PM, William Tu via iovisor-dev <
iovisor-dev@lists.iovisor.org> wrote:

> Hi Mauricio,
>
> Thanks! Yes, I think like you said the compiler couldn't unroll in
> this case. Because we are passing memory address of i () to a
> function, the function might change the value of 'i', which might
> change the number of loops. So compiler should disable unrolling.
>
> Regards,
> William
>
> On Tue, Apr 18, 2017 at 6:22 PM, Mauricio Vasquez
>  wrote:
> > Hi William,
> >
> > On 04/18/2017 06:54 PM, William Tu via iovisor-dev wrote:
> >>
> >> Hi,
> >>
> >> I found that if the loop variable "int i" is used in the map lookup as
> >> below:
> >> #pragma clang loop unroll(full)
> >> for (i = 0; i < 8; ++i) {
> >> struct bpf_flow_keys *mask;
> >> mask = bpf_map_lookup_elem(_mask, );
> >> if (!mask)
> >> break;
> >> }
> >> Then the compiled BPF code does not unrolled the loop, causing errors:
> >> "back-edge from insn 240 to 219"
> >>
> >> A workaround for that is to declare another variable "int j", then I
> >> can pass the verifier.
> >>
> >> #pragma clang loop unroll(full)
> >> for (i = 0; i < 8; ++i) {
> >> int j = i;
> >> struct bpf_flow_keys *mask;
> >> mask = bpf_map_lookup_elem(_mask, ); // ---> use "j"
> >> if (!mask)
> >> break;
> >> }
> >>
> >> Does anyone hit similar issue?
> >
> >
> > Yes, we hit exactly the same problem [1], at that time we thought it was
> > because of the missing "const" qualifier in the bpf_map_* helpers [2],
> > however we added it and the result was the same.
> >
> > Until now we don't know what is the cause of the problem, we continue to
> use
> > that workaround.
> >
> > Mauricio V,
> >
> > [1]
> > https://lists.iovisor.org/pipermail/iovisor-dev/2016-
> December/000560.html
> > [2]
> > https://lists.iovisor.org/pipermail/iovisor-dev/2016-
> December/000559.html
> >>
> >> Regards,
> >> William
> >> ___
> >> iovisor-dev mailing list
> >> iovisor-dev@lists.iovisor.org
> >> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
> >>
> >
> ___
> iovisor-dev mailing list
> iovisor-dev@lists.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>
___
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev