headers
Joe Perches | 1 Nov 2007 01:34

Re: [PATCH] - e1000_ethtool.c - convert macros to functions

On Wed, 2007-10-31 at 14:30 -0700, Kok, Auke wrote:
> Joe Perches wrote:
> > Convert REG_PATTERN_TEST and REG_SET_AND_CHECK macros to functions
> > Reduces x86 defconfig image by about 3k

Convert REG_PATTERN_TEST and REG_SET_AND_CHECK macros to functions
Reduces x86 defconfig image by about 3k

compiled, untested (no hardware)

Signed-off-by: Joe Perches <joe <at> perches.com>

New:

$ size vmlinux
   text    data     bss     dec     hex filename
4792735  490626  606208 5889569  59de21 vmlinux

Current:

$ size vmlinux
   text    data     bss     dec     hex filename
4795759  490626  606208 5892593  59e9f1 vmlinux

Changed since last submission:

Fixed existing controler->controller typo
Added E1000_REG_ADDR macro
Added E1000_FLASH_ADDR macro
The #define E1000_PHY_CTRL conflicted with the PHY_CTRL
(Continue reading)

Permalink | Reply | 0 comments |
headers
Kok, Auke | 1 Nov 2007 01:39
Picon
Favicon

Re: [PATCH] - e1000_ethtool.c - convert macros to functions

Joe Perches wrote:
> On Wed, 2007-10-31 at 14:30 -0700, Kok, Auke wrote:
>> Joe Perches wrote:
>> that's not a bad idea, however see below:
>> can't we keep the macro here (and just make it call the function instead of
>> expanding). the resulting code is much more lenghty and contains all these logic
>> traps that the previous code didn't have.
>> just have the macro expand to `if (reg_pattern_test(...)) return 1)` and you don't
>> need to change any of the calling lines.
> 
> You could define something like:
> 
> #define REG_PATTERN_TEST(reg, mask, write) \
> 	if (reg_pattern_test(adapter, data, \
> 			     E1000_REG(&adapter->hw, reg), \
> 			     mask, write)) \
> 		return 1;
> 
> But isn't the macro with an embedded return a bit too obfuscating?

in this case it saves a bunch of time reading and makes the code actually more
readable and robust IMO. Now we're explicitly mobming out on an error. with your
patch we need to assure that for every call to reg_pattern_test we check the
return value.

>>> +#define E1000_READ_REG_ARRAY(a, reg, offset)		\
>>> +	(readl((a)->hw_addr +				\
>>> +	       (((a)->mac_type >= e1000_82543)		\
>>> +		? E1000_##reg : E1000_82542_##reg) +	\
>>> +	       ((offset) << 2)))
(Continue reading)

Permalink | Reply | 0 comments |
headers
Eric W. Biederman | 1 Nov 2007 01:51
Favicon

Re: [PATCH 0/5] Make nicer CONFIG_NET_NS=n case code

Eric Dumazet <dada1 <at> cosmosbay.com> writes:

> Eric W. Biederman a écrit :
>> Eric Dumazet <dada1 <at> cosmosbay.com> writes:
>>
>>
>>> Definitly wanted here. Thank you.
>>> One more refcounting on each socket creation/deletion was expensive.
>>
>> Really?  Have you actually measured that?  If the overhead is
>> measurable and expensive we may want to look at per cpu counters or
>> something like that.  So far I don't have any numbers that say any
>> of the network namespace work inherently has any overhead.
>
> It seems that on some old opterons (two 246 for example),
> "if (atomic_dec_and_test(&net->count))" is rather expensive yes :(

I won't argue that atomic_dec_and_test is costly.  My gut feel is that
socket creation/destruction is sufficiently rare that such a test
would be lost in the noise.  Doing anything more sophisticated is
likely to be less readable, and unless we can measure some overhead
my preference right now is to keep the code stupid and simple.  Which
usually has a good icache footprint.

> I am not sure per cpu counters help : I tried this and got no speedup. (This was
> on net_device refcnt at that time)
>
> (on this machines, the access through fs/gs selector seems expensive too)
>
> Maybe a lazy mode could be done, ie only do a atomic_dec(), as done in dev_put()
(Continue reading)

Permalink | Reply | 2 comments |
headers
Eric W. Biederman | 1 Nov 2007 01:58
Favicon

Re: [PATCH 0/5] Make nicer CONFIG_NET_NS=n case code

David Miller <davem <at> davemloft.net> writes:

> From: Eric Dumazet <dada1 <at> cosmosbay.com>
> Date: Wed, 31 Oct 2007 23:40:59 +0100
>
>> Eric W. Biederman a écrit :
>> > Eric Dumazet <dada1 <at> cosmosbay.com> writes:
>> > 
>> > 
>> >> Definitly wanted here. Thank you.
>> >> One more refcounting on each socket creation/deletion was expensive.
>> > 
>> > Really?  Have you actually measured that?  If the overhead is
>> > measurable and expensive we may want to look at per cpu counters or
>> > something like that.  So far I don't have any numbers that say any
>> > of the network namespace work inherently has any overhead.
>> 
>> It seems that on some old opterons (two 246 for example),
>> "if (atomic_dec_and_test(&net->count))" is rather expensive yes :(
>
> P4 chips are generally very poor at mispredicted branches and
> atomics.  So every atomic you remove from the socket paths
> gives a noticable improvement on them.

Interesting.

> Network device reference counting is such a stupid problem.  There has
> to be a way to get rid of it on the packet side.
>
> I think we could get rid of all of the device refcounting from packets
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ben Greear | 1 Nov 2007 02:06
Favicon

Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?

Stephen Hemminger wrote:
> On Wed, 31 Oct 2007 14:43:51 -0400
> Dave Johnson <djohnson+linux-kernel <at> sw.starentnetworks.com> wrote:
>
>   
>> Depending on the network driver, I'm seeing different behavior if
>> a .1q packet is received to an PF_PACKET, SOCK_RAW, ETH_P_ALL socket.
>>
>>
>> On devices what do not use NETIF_F_HW_VLAN_RX, the packet socket gets
>> the complete packet with vlan tag included as the driver simply calls
>> netif_receive_skb() or equivilant.  packet_rcv() then gets the whole
>> thing vlan tag included and sends this through the socket.
>>
>> vlan_skb_recv() also gets these all and will drop them because there
>> are no vlans configured.
>>
>>     
>
> The VLAN acceleration grabs and hides the tag. It is a design flaw
> that should be fixed, feel free to post a patch.
>   
There may be several ways to 'fix' this.  Perhaps it would be worth 
discussing what
we want the end result to be at least?

Should we always pass the vlan header up to raw sockets as part of the
data payload?

Or, maybe pass it in an auxiliary message such as how timestamps may be
(Continue reading)

Permalink | Reply | 0 comments |
headers
David Miller | 1 Nov 2007 02:10
Favicon

Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?

From: Ben Greear <greearb <at> candelatech.com>
Date: Wed, 31 Oct 2007 18:06:34 -0700

> DaveM did the HW Accel for VLANs if I remember correctly...perhaps he 
> has some input?

Not really, I'm busy and also not motivated to work on this, someone
else will need to.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |
headers
Stephen Hemminger | 1 Nov 2007 02:23

Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?

Ben Greear wrote:
> Stephen Hemminger wrote:
>> On Wed, 31 Oct 2007 14:43:51 -0400
>> Dave Johnson <djohnson+linux-kernel <at> sw.starentnetworks.com> wrote:
>>
>>  
>>> Depending on the network driver, I'm seeing different behavior if
>>> a .1q packet is received to an PF_PACKET, SOCK_RAW, ETH_P_ALL socket.
>>>
>>>
>>> On devices what do not use NETIF_F_HW_VLAN_RX, the packet socket gets
>>> the complete packet with vlan tag included as the driver simply calls
>>> netif_receive_skb() or equivilant.  packet_rcv() then gets the whole
>>> thing vlan tag included and sends this through the socket.
>>>
>>> vlan_skb_recv() also gets these all and will drop them because there
>>> are no vlans configured.
>>>
>>>     
>>
>> The VLAN acceleration grabs and hides the tag. It is a design flaw
>> that should be fixed, feel free to post a patch.
>>   
> There may be several ways to 'fix' this.  Perhaps it would be worth 
> discussing what
> we want the end result to be at least?
>
> Should we always pass the vlan header up to raw sockets as part of the
> data payload?
>
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ben Greear | 1 Nov 2007 02:31
Favicon

Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?

Stephen Hemminger wrote:
>
> The code in AF_PACKET should fix the skb before passing to user space 
> so that there is
> no difference between accel and non-accel hardware.  Internal choices 
> shouldn't
> leak to user space.  Ditto, the receive checksum offload should be 
> fixed up as well.
Ok, I guess that will fix the sniffing issues and any user-space 
bridging type applications.

Currently, VLAN devices offer the ability to 'reorder' the header and 
explicitly remove the VLAN
header.  I assume we keep this feature and have the AF_PACKET logic 
check the device
flags to see if it should insert the VLAN header for hw-accel vlans?

Either way, if we sniff the underlying device, we should always get the 
VLAN header.

What about drivers and filtering VLANs?  It seems there is still a 
difference between software
vlans and hw-accel in this case.

Thanks,
Ben

--

-- 
Ben Greear <greearb <at> candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com
(Continue reading)

Permalink | Reply | 0 comments |
headers
Li Yang-r58472 | 1 Nov 2007 03:33
Favicon

RE: [PATCH] ucc_geth: add support for netpoll

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbou <at> mail.ru] 
> Sent: Thursday, November 01, 2007 5:59 AM
> To: Li Yang-r58472
> Cc: netdev <at> vger.kernel.org; linux-kernel <at> vger.kernel.org; 
> linuxppc-dev <at> ozlabs.org
> Subject: Re: [PATCH] ucc_geth: add support for netpoll
> 
> On Mon, Oct 29, 2007 at 03:17:44PM +0300, Anton Vorontsov wrote:
> [...]
> > > Oops.  The original patch happened to hit the Junk mail box. :(
> > 
> > That one as well? http://lkml.org/lkml/2007/10/11/128
> > 
> > > I think
> > > the patch is good to merge after the cosmetic change.  I 
> can do it 
> > > in next pull request to Jeff.
> > 
> > Ok, great. Thanks.
> 
> I'm wondering if you missed that email again. Maybe your mail 
> client/server doing weird things with emails from  <at> ru.mvista.com?

No.  I have explicitly add you to the whitelist. :) Please be patient,
isn't this patch a new feature which can only be integrated in the merge
window?  Thanks.

- Leo
-
(Continue reading)

Permalink | Reply | 1 comment |
headers
Joe Perches | 1 Nov 2007 04:29

[PATCH] - e1000e/ethtool.c - convert macros to functions

Add functions for reg_pattern_test and reg_set_and check
Changed macros to use these functions

Compiled x86, untested

Size decreased ~2K

old:

$ size drivers/net/e1000e/ethtool.o
   text    data     bss     dec     hex filename
  14461       0       0   14461    387d drivers/net/e1000e/ethtool.o

new:

$ size drivers/net/e1000e/ethtool.o
   text    data     bss     dec     hex filename
  12498       0       0   12498    30d2 drivers/net/e1000e/ethtool.o

Signed-off-by: Joe Perches <joe <at> perches.com>

---

 drivers/net/e1000e/ethtool.c |   78 +++++++++++++++++++++++++----------------
 1 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 6a39784..225db17 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
(Continue reading)

Permalink | Reply | 0 comments |

Gmane