Re: Replace bcopy() to update ether_addr

2012-08-23 Thread Adrian Chadd
You can't assume the ethernet addresses will be aligned. For wifi headers you're definitely going to have at least one address be non-dword aligned, even if the header itself is dword aligned. The whole point of using a macro is so it becomes really easy to change the copy type at compile time.

Re: Replace bcopy() to update ether_addr

2012-08-23 Thread Bruce Evans
@FreeBSD.org, Adrian Chadd adr...@freebsd.org, Mitya mi...@cabletv.dp.ua, Wojciech Puchar woj...@wojtek.tensor.gdynia.pl, freebsd-...@freebsd.org Subject: Re: Replace bcopy() to update ether_addr X-BeenThere: freebsd-...@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List

Re: Replace bcopy() to update ether_addr

2012-08-23 Thread Bruce Evans
luigi wrote: On Wed, Aug 22, 2012 at 03:21:06PM -0400, John Baldwin wrote: On Wednesday, August 22, 2012 2:54:07 pm Adrian Chadd wrote: On 22 August 2012 05:02, John Baldwin j...@freebsd.org wrote: On Tuesday, August 21, 2012 12:34:42 pm Adrian Chadd wrote: Hi, What about

Re: Replace bcopy() to update ether_addr

2012-08-22 Thread Bruce Evans
mitya wrote: 22.08.2012 05:07, Bruce Evans íàïèñàë: On Mon, Aug 20, 2012 at 05:46:12PM +0300, Mitya wrote: Hi. I found some overhead code in /src/sys/net/if_ethersubr.c and /src/sys/netgraph/ng_ether.c It contains strings, like bcopy(src, dst, ETHER_ADDR_LEN); When src and dst are

Re: Replace bcopy() to update ether_addr

2012-08-22 Thread John Baldwin
On Tuesday, August 21, 2012 12:34:42 pm Adrian Chadd wrote: Hi, What about just creating an ETHER_ADDR_COPY(dst, src) and putting that in a relevant include file, then hide the ugliness there? The same benefits will likely appear when copying wifi MAC addresses to/from headers. Thanks,

Re: Replace bcopy() to update ether_addr

2012-08-22 Thread Warner Losh
On Aug 22, 2012, at 6:02 AM, John Baldwin wrote: On Tuesday, August 21, 2012 12:34:42 pm Adrian Chadd wrote: Hi, What about just creating an ETHER_ADDR_COPY(dst, src) and putting that in a relevant include file, then hide the ugliness there? The same benefits will likely appear when

speed tests (Re: Replace bcopy() to update ether_addr)

2012-08-22 Thread Luigi Rizzo
On Wed, Aug 22, 2012 at 02:32:21AM +, Bruce Evans wrote: luigi wrote: even more orthogonal: I found that copying 8n + (5, 6 or 7) bytes was much much slower than copying a multiple of 8 bytes. For n=0, 1,2,4,8 bytes are efficient, other cases are slow (turned into 2 or 3

Re: speed tests (Re: Replace bcopy() to update ether_addr)

2012-08-22 Thread Luigi Rizzo
On Wed, Aug 22, 2012 at 05:26:47PM +0300, Mitya wrote: 22.08.2012 17:36, Luigi Rizzo ??: On Wed, Aug 22, 2012 at 02:32:21AM +, Bruce Evans wrote: luigi wrote: even more orthogonal: I found that copying 8n + (5, 6 or 7) bytes was much much slower than copying a multiple of

Re: speed tests (Re: Replace bcopy() to update ether_addr)

2012-08-22 Thread Mitya
22.08.2012 17:36, Luigi Rizzo написал: On Wed, Aug 22, 2012 at 02:32:21AM +, Bruce Evans wrote: luigi wrote: even more orthogonal: I found that copying 8n + (5, 6 or 7) bytes was much much slower than copying a multiple of 8 bytes. For n=0, 1,2,4,8 bytes are efficient, other cases are

Re: Replace bcopy() to update ether_addr

2012-08-22 Thread Adrian Chadd
On 22 August 2012 05:02, John Baldwin j...@freebsd.org wrote: On Tuesday, August 21, 2012 12:34:42 pm Adrian Chadd wrote: Hi, What about just creating an ETHER_ADDR_COPY(dst, src) and putting that in a relevant include file, then hide the ugliness there? The same benefits will likely appear

Re: Replace bcopy() to update ether_addr

2012-08-22 Thread Warner Losh
On Aug 22, 2012, at 12:54 PM, Adrian Chadd wrote: On 22 August 2012 05:02, John Baldwin j...@freebsd.org wrote: On Tuesday, August 21, 2012 12:34:42 pm Adrian Chadd wrote: Hi, What about just creating an ETHER_ADDR_COPY(dst, src) and putting that in a relevant include file, then hide the

Re: Replace bcopy() to update ether_addr

2012-08-22 Thread John Baldwin
On Wednesday, August 22, 2012 2:54:07 pm Adrian Chadd wrote: On 22 August 2012 05:02, John Baldwin j...@freebsd.org wrote: On Tuesday, August 21, 2012 12:34:42 pm Adrian Chadd wrote: Hi, What about just creating an ETHER_ADDR_COPY(dst, src) and putting that in a relevant include file,

Re: Replace bcopy() to update ether_addr

2012-08-22 Thread Wojciech Puchar
but wonder whether the same does hold for MIPS/ARM. Getting it wrong there will lead to some very very poor performing code. 1) do - as already pointed out - standard copy of structure in C. 2) if compiler is found to generate bad code on some archs put assembly. 1 even if compiler is not

Re: Replace bcopy() to update ether_addr

2012-08-22 Thread Luigi Rizzo
On Wed, Aug 22, 2012 at 03:21:06PM -0400, John Baldwin wrote: On Wednesday, August 22, 2012 2:54:07 pm Adrian Chadd wrote: On 22 August 2012 05:02, John Baldwin j...@freebsd.org wrote: On Tuesday, August 21, 2012 12:34:42 pm Adrian Chadd wrote: Hi, What about just creating an

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Wojciech Puchar
Even without this tagging, the code to do a structure level copy of 6 bytes is going to be tiny... true. just to make sure it will be absolutely portable how about bcopymacaddress(dst,src) and then define it whatever you find it fastest on any architecture?

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Marius Strobl
On Mon, Aug 20, 2012 at 05:46:12PM +0300, Mitya wrote: Hi. I found some overhead code in /src/sys/net/if_ethersubr.c and /src/sys/netgraph/ng_ether.c It contains strings, like bcopy(src, dst, ETHER_ADDR_LEN); When src and dst are struct ether_addr*, and ETHER_ADDR_LEN equal 6. This code

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Luigi Rizzo
On Tue, Aug 21, 2012 at 12:26:30PM +0200, Marius Strobl wrote: ... Why we are use bcopy(), to copy only 6 bytes? Answer - in some architectures we are can not directly copy unaligned data. I propose this solution. In file /usr/src/include/net/ethernet.h add this lines: static

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Marius Strobl
On Mon, Aug 20, 2012 at 01:20:29PM -0600, Warner Losh wrote: On Aug 20, 2012, at 1:17 PM, Wojciech Puchar wrote: or use ++. i think it is always aligned to 2 bytes and this should produce usable code on any CPU? should be 6 instructions on MIPS and PPC IMHO. We should tag it as

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Mitya
20.08.2012 22:20, Warner Losh написал: On Aug 20, 2012, at 1:17 PM, Wojciech Puchar wrote: or use ++. i think it is always aligned to 2 bytes and this should produce usable code on any CPU? should be 6 instructions on MIPS and PPC IMHO. We should tag it as __aligned(2) then, no? If so,

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Mitya
21.08.2012 14:26, Marius Strobl написал: On Mon, Aug 20, 2012 at 01:20:29PM -0600, Warner Losh wrote: On Aug 20, 2012, at 1:17 PM, Wojciech Puchar wrote: or use ++. i think it is always aligned to 2 bytes and this should produce usable code on any CPU? should be 6 instructions on MIPS and

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Warner Losh
On Aug 21, 2012, at 5:26 AM, Marius Strobl wrote: On Mon, Aug 20, 2012 at 01:20:29PM -0600, Warner Losh wrote: On Aug 20, 2012, at 1:17 PM, Wojciech Puchar wrote: or use ++. i think it is always aligned to 2 bytes and this should produce usable code on any CPU? should be 6

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread John Baldwin
On Monday, August 20, 2012 10:46:12 am Mitya wrote: Hi. I found some overhead code in /src/sys/net/if_ethersubr.c and /src/sys/netgraph/ng_ether.c It contains strings, like bcopy(src, dst, ETHER_ADDR_LEN); When src and dst are struct ether_addr*, and ETHER_ADDR_LEN equal 6. This code call

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Warner Losh
On Aug 21, 2012, at 1:42 AM, Wojciech Puchar wrote: Even without this tagging, the code to do a structure level copy of 6 bytes is going to be tiny... true. just to make sure it will be absolutely portable how about bcopymacaddress(dst,src) and then define it whatever you find

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Adrian Chadd
Hi, What about just creating an ETHER_ADDR_COPY(dst, src) and putting that in a relevant include file, then hide the ugliness there? The same benefits will likely appear when copying wifi MAC addresses to/from headers. Thanks, I'm glad someone noticed this. Adrian

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Bruce Evans
On Mon, Aug 20, 2012 at 05:46:12PM +0300, Mitya wrote: Hi. I found some overhead code in /src/sys/net/if_ethersubr.c and /src/sys/netgraph/ng_ether.c It contains strings, like bcopy(src, dst, ETHER_ADDR_LEN); When src and dst are struct ether_addr*, and ETHER_ADDR_LEN equal 6. Only

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Bruce Evans
luigi wrote: even more orthogonal: I found that copying 8n + (5, 6 or 7) bytes was much much slower than copying a multiple of 8 bytes. For n=0, 1,2,4,8 bytes are efficient, other cases are slow (turned into 2 or 3 different writes). The netmap code uses a pkt_copy routine that does

Re: Replace bcopy() to update ether_addr

2012-08-21 Thread Bruce Evans
jhb wrote: On Monday, August 20, 2012 10:46:12 am Mitya wrote: ... I propose this solution. In file /usr/src/include/net/ethernet.h add this lines: static inline void ether_addr_copy(ether_addr* src, ether_addr* dst) { #if defined(__i386__) || defined(__amd64__) *dst =

Replace bcopy() to update ether_addr

2012-08-20 Thread Mitya
Hi. I found some overhead code in /src/sys/net/if_ethersubr.c and /src/sys/netgraph/ng_ether.c It contains strings, like bcopy(src, dst, ETHER_ADDR_LEN); When src and dst are struct ether_addr*, and ETHER_ADDR_LEN equal 6. This code call every time, when we send Ethernet packet. On example, on

Re: Replace bcopy() to update ether_addr

2012-08-20 Thread Warner Losh
On Aug 20, 2012, at 8:46 AM, Mitya wrote: Hi. I found some overhead code in /src/sys/net/if_ethersubr.c and /src/sys/netgraph/ng_ether.c It contains strings, like bcopy(src, dst, ETHER_ADDR_LEN); When src and dst are struct ether_addr*, and ETHER_ADDR_LEN equal 6. This code call every

Re: Replace bcopy() to update ether_addr

2012-08-20 Thread Wojciech Puchar
#if defined(__i386__) || defined(__amd64__) *dst = *src; #else bcopy(src, dst, ETHER_ADDR_LEN); #else short *tmp1=((*short)src),*tmp2=((*short)dst); *tmp2=*tmp1; *(tmp2+1)=*(tmp1+1); *(tmp2+2)=*(tmp1+2); or use ++. i think it is always aligned to 2 bytes and this should produce usable

Re: Replace bcopy() to update ether_addr

2012-08-20 Thread Warner Losh
On Aug 20, 2012, at 10:48 AM, Wojciech Puchar wrote: #if defined(__i386__) || defined(__amd64__) *dst = *src; #else bcopy(src, dst, ETHER_ADDR_LEN); #else short *tmp1=((*short)src),*tmp2=((*short)dst); *tmp2=*tmp1; *(tmp2+1)=*(tmp1+1); *(tmp2+2)=*(tmp1+2); or use ++. i think it

Re: Replace bcopy() to update ether_addr

2012-08-20 Thread Wojciech Puchar
or use ++. i think it is always aligned to 2 bytes and this should produce usable code on any CPU? should be 6 instructions on MIPS and PPC IMHO. We should tag it as __aligned(2) then, no? If so, then the compiler should generate the code you posted. should is the most important word in

Re: Replace bcopy() to update ether_addr

2012-08-20 Thread Warner Losh
On Aug 20, 2012, at 1:17 PM, Wojciech Puchar wrote: or use ++. i think it is always aligned to 2 bytes and this should produce usable code on any CPU? should be 6 instructions on MIPS and PPC IMHO. We should tag it as __aligned(2) then, no? If so, then the compiler should generate

Re: Replace bcopy() to update ether_addr

2012-08-20 Thread Bakul Shah
On Mon, 20 Aug 2012 13:05:51 MDT Warner Losh i...@bsdimp.com wrote: On Aug 20, 2012, at 10:48 AM, Wojciech Puchar wrote: #if defined(__i386__) || defined(__amd64__) *dst =3D *src; #else bcopy(src, dst, ETHER_ADDR_LEN); #else short *tmp1=3D((*short)src),*tmp2=3D((*short)dst);