On Thu, Aug 27, 2015 at 06:29:16PM -0700, Jarno Rajahalme wrote:
> Define struct eth_addr and use it instead of a uint8_t array for all
> ethernet addresses in OVS userspace. The struct is always the right
> size, and it can be assigned without an explicit memcpy, which makes
> code more readable.
>
> "struct eth_addr" is a good type name for this as many utility
> functions are already named accordingly.
>
> struct eth_addr can be accessed as bytes as well as ovs_be16's, which
> makes the struct 16-bit aligned. All use seems to be 16-bit aligned,
> so some algorithms on the ethernet addresses can be made a bit more
> efficient making use of this fact.
>
> As the struct fits into a register (in 64-bit systems) we pass it by
> value when possible.
>
> This patch also changes the few uses of Linux specific ETH_ALEN to
> OVS's own ETH_ADDR_LEN, and removes the OFP_ETH_ALEN, as it is no
> longer needed.
>
> This work stemmed from a desire to make all struct flow members
> assignable for unrelated exploration purposes. However, I think this
> might be a nice code readability improvement by itself.
>
> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
I like this. I've thought about doing the same thing before and never
got around to it.
I checked your claim about passing by value by looking at the x86-64
ABI. It's true! Older ABIs were not so flexible--for example, I seem
to recall that Borland C++ __fastcall ABI for Win32 would pass 1 and 2
and 4 byte structs in a register, but not 3-byte ones.
However, GCC is almost criminally bad at optimizing it:
blp@sigabrt:~/nicira/ovs/_build(0)$ cat tmp.c
struct x {
union {
unsigned char b[6];
unsigned short w[3];
};
};
void g(struct x);
void f(void)
{
struct x y = { { { 1, 2, 3, 4, 5, 6 } } };
g(y);
}
blp@sigabrt:~/nicira/ovs/_build(0)$ gcc -O2 -g -m64 -c tmp.c
blp@sigabrt:~/nicira/ovs/_build(0)$ objdump -dr tmp.o
tmp.o: file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <f>:
0: 48 83 ec 18 sub $0x18,%rsp
4: c6 04 24 01 movb $0x1,(%rsp)
8: c6 44 24 01 02 movb $0x2,0x1(%rsp)
d: c6 44 24 02 03 movb $0x3,0x2(%rsp)
12: c6 44 24 03 04 movb $0x4,0x3(%rsp)
17: c6 44 24 04 05 movb $0x5,0x4(%rsp)
1c: c6 44 24 05 06 movb $0x6,0x5(%rsp)
21: 48 8b 3c 24 mov (%rsp),%rdi
25: e8 00 00 00 00 callq 2a <f+0x2a>
26: R_X86_64_PC32 g-0x4
2a: 48 83 c4 18 add $0x18,%rsp
2e: c3 retq
Clang does better:
blp@sigabrt:~/nicira/ovs/_build(0)$ clang -O2 -g -m64 -c tmp.c
blp@sigabrt:~/nicira/ovs/_build(0)$ objdump -dr tmp.o
tmp.o: file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <f>:
0: 48 bf 01 02 03 04 05 movabs $0x60504030201,%rdi
7: 06 00 00
a: e9 00 00 00 00 jmpq f <f+0xf>
b: R_X86_64_PC32 g-0x4
Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
