All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Palethorpe <rpalethorpe@suse.de>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v2] BPF: Regression test for 64bit arithmetic
Date: Tue, 10 Sep 2019 16:06:48 +0200	[thread overview]
Message-ID: <87d0g8kzyf.fsf@rpws.prws.suse.cz> (raw)
In-Reply-To: <20190910132338.GB29865@rei>

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
>> > +static int load_prog(int fd)
>> > +{
>> > +	struct bpf_insn *prog;
>> > +	struct bpf_insn insn[] = {
>> > +		BPF_MOV64_IMM(BPF_REG_6, 1),            /* r6 = 1 */
>> > +
>> > +		BPF_LD_MAP_FD(BPF_REG_1, fd),	        /* r1 = &fd */
>> > +		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),   /* r2 = fp */
>> > +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),  /* r2 = r2 - 8 */
>> > +		BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),    /* *r2 = 0 */
>> > +		BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
>> > +		BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 17), /* if(!r0) goto exit */
>>
>> Patch looks good to me.
>>
>> But I keep thinking if there's way to make it more obvious where
>> offset (e.g. 17) came from.
>>
>> Idea 1: use multiple lines per instruction to denote length
>>   BPF_LD_IMM64(BPF_REG_4,
>>                A64INT),
>>
>> Idea 2: prefix commented instructions with offset
>>         /* 1: r3 = r0 */
>>         /* 2: r4 = 2^61 */
>
> I guess I like the Idea 2 better.
>
> Another option would be having eBPF assembler included in the LTP build
> system. I guess that it may be useful later on and there seems to be one
> written in python:
>
> https://github.com/solarflarecom/ebpf_asm
>
> But for the short term I would go with adding the offset to the
> comments.

Another idea I had was to use place holder values in the instruction
array which can be substituted.

Infact we could use an invalid instruction code to indicate a goto
instruction and another code for a tag. Then replace the tag with a NOP
and replace the goto with a valid jump statement.

I decided not to try any of that because it seemed like overkill at the
time. However for a more complex program I can see this getting very
confusing.

I wonder if an assembler will make things better in some tests and worse
in others. Sometimes the assembler has pseudo instructions where it is
not clear what the resulting machine instructions will be.

--
Thank you,
Richard.

      reply	other threads:[~2019-09-10 14:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-05 12:45 [LTP] [PATCH v2] BPF: Regression test for 64bit arithmetic Richard Palethorpe
2019-09-10 12:55 ` Jan Stancek
2019-09-10 13:23   ` Cyril Hrubis
2019-09-10 14:06     ` Richard Palethorpe [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d0g8kzyf.fsf@rpws.prws.suse.cz \
    --to=rpalethorpe@suse.de \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.