* Re: [PATCH net-next-2.6] be2net: free tx buffers when completions never arrive
From: Sathya Perla @ 2010-02-18 10:37 UTC (permalink / raw)
To: David Miller; +Cc: sathyap, netdev
In-Reply-To: <20100218.021616.127015823.davem@davemloft.net>
On 18/02/10 02:16 -0800, David Miller wrote:
<snip>
> > + bool dummy_wrb;
>
> Some of your lines start with tabs, some with spaces. You
> have to use tabs consistently.
>
<snip>
Sorry, mistake fixed; patch below. Thanks a lot...
-Sathya
>From 8f9134221833d4d00da1482f3051751b625649ca Mon Sep 17 00:00:00 2001
From: Sathya Perla <sathyap@serverengines.com>
Date: Thu, 18 Feb 2010 14:57:35 +0530
Subject: [PATCH net-next-2.6] be2net: free tx buffers when completions never arrive
In cases like when a pci device is disconnected on an error,
pending tx completions will never arrive. Unmap and free such
buffers in the tx cleanup path.
Signed-off-by: Sathya Perla <sathyap@serverengines.com>
---
drivers/net/benet/be_main.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 68e7848..545c841 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -1128,6 +1128,9 @@ static void be_tx_compl_clean(struct be_adapter *adapter)
struct be_queue_info *txq = &adapter->tx_obj.q;
struct be_eth_tx_compl *txcp;
u16 end_idx, cmpl = 0, timeo = 0;
+ struct sk_buff **sent_skbs = adapter->tx_obj.sent_skb_list;
+ struct sk_buff *sent_skb;
+ bool dummy_wrb;
/* Wait for a max of 200ms for all the tx-completions to arrive. */
do {
@@ -1151,6 +1154,15 @@ static void be_tx_compl_clean(struct be_adapter *adapter)
if (atomic_read(&txq->used))
dev_err(&adapter->pdev->dev, "%d pending tx-completions\n",
atomic_read(&txq->used));
+
+ /* free posted tx for which compls will never arrive */
+ while (atomic_read(&txq->used)) {
+ sent_skb = sent_skbs[txq->tail];
+ end_idx = txq->tail;
+ index_adv(&end_idx,
+ wrb_cnt_for_skb(sent_skb, &dummy_wrb) - 1, txq->len);
+ be_tx_compl_process(adapter, end_idx);
+ }
}
static void be_mcc_queues_destroy(struct be_adapter *adapter)
--
1.6.3.3
^ permalink raw reply related
* Re: kernel stack trace using conntrack
From: Patrick McHardy @ 2010-02-18 10:34 UTC (permalink / raw)
To: Ramblewski David
Cc: Eric Dumazet, Pablo Neira Ayuso, netfilter-devel@vger.kernel.org,
netdev
In-Reply-To: <7EF5DBE4C76A7B4DA655334E9F2BFD26CED7BC905D@FRSPX100.fr01.awl.atosorigin.net>
Ramblewski David wrote:
> Hi Eric,
>
> The conntrack patch works successfully.
>
>>> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
>>> index 0ffe689..d2657aa 100644
>>> --- a/net/netfilter/nf_conntrack_netlink.c
>>> +++ b/net/netfilter/nf_conntrack_netlink.c
>>> @@ -923,7 +923,7 @@ ctnetlink_change_status(struct nf_conn *ct, const struct nlattr * const cda[])
>>> unsigned int status = ntohl(nla_get_be32(cda[CTA_STATUS]));
>>> d = ct->status ^ status;
>>>
>>> - if (d & (IPS_EXPECTED|IPS_CONFIRMED|IPS_DYING))
>>> + if (d & (IPS_EXPECTED|IPS_DYING))
>>> /* unchangeable */
>>> return -EBUSY;
>> I think that we should explicitly report if the user unsets
>> IPS_CONFIRMED. Please, don't change this.
>>
>> Apart from that, the patch seems fine to me. Thanks!
>
> Problem is we now (I mean after my patch) enter
> ctnetlink_change_status() with ct->status being null (or at least,
> IPS_CONFIRMED not set)
Pablo, please let me know whether you want me to apply this.
^ permalink raw reply
* Re: Panic at tcp_xmit_retransmit_queue
From: Bruno Prémont @ 2010-02-18 10:28 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: sbs, Netdev, LKML
In-Reply-To: <alpine.DEB.2.00.1002151520250.7063@wel-95.cs.helsinki.fi>
On Mon, 15 Feb 2010 15:21:58 "Ilpo Järvinen" wrote:
> On Wed, 3 Feb 2010, Ilpo Järvinen wrote:
>
> > On Mon, 1 Feb 2010, sbs wrote:
> >
> > > actually removing netconsole from kernel didnt help.
> > > i found many guys with the same problem but with different
> > > hardware configurations here:
> > >
> > > freez in TCP stack :
> > > http://bugzilla.kernel.org/show_bug.cgi?id=14470
> > >
> > > is there someone who can investigate it?
> > >
> > >
> > > On Tue, Jan 19, 2010 at 7:13 PM, sbs <gexlie@gmail.com> wrote:
> > > > We are hiting kernel panics on servers with nVidia MCP55 NICs
> > > > once a day; it appears usualy under a high network trafic
> > > > ( around 10000Mbit/s) but it is not a rule, it has happened
> > > > even on low trafic.
> > > >
> > > > Servers are used as nginx+static content
> > > > On 2 equal servers this panic happens aprox 2 times a day
> > > > depending on network load. Machine completly freezes till the
> > > > netconsole reboots.
> > > >
> > > > Kernel: 2.6.32.3
> > > >
> > > > what can it be? whats wrong with tcp_xmit_retransmit_queue()
> > > > function ? can anyone explain or fix?
> >
> > You might want to try with to debug patch below. It might even make
> > the box to survive the event (if I got it coded right).
>
> Here should be a better version of the debug patch, hopefully the
> infinite looping is now gone.
I can reproduce the freeze pretty easily, even on an idle server,
all I need is netconsole enabled, an ssh connection to server and
permission to write to /proc/sysrq-trigger.
The following command, executed via SSH triggers the frozen system:
echo t > /proc/sysrq-trigger
when netconsole is enabled. Doing the same from local console has no
negative effect (idle system).
Unfortunately I can't get any useful information out of the system as
nothing reaches VGA console and interaction with the system is not
possible anymore (cursor is still blinking on VGA console).
Unfortunately I currently have no setup here to analyze dead system via
kexec crash kernel that would be run on watchdog.
System I'm using is HP Proliant DL360 G5 (4 logical CPUs, two sockets),
bnx2 NIC.
Eventually I will try with some other system to reproduce there as
well (to rule out NIC driver).
Any hints on how to get pertinent data out of that system would be
really nice!
Regards,
Bruno
^ permalink raw reply
* Re: [PATCH net-next-2.6] be2net: free tx buffers when completions never arrive
From: David Miller @ 2010-02-18 10:16 UTC (permalink / raw)
To: sathyap; +Cc: netdev
In-Reply-To: <20100218093603.GA28912@serverengines.com>
From: Sathya Perla <sathyap@serverengines.com>
Date: Thu, 18 Feb 2010 15:06:03 +0530
> In cases like when a pci device is disconnected after a pci error,
> pending tx completions will never arrive. Unmap and free such
> buffers in the tx cleanup path.
>
> Signed-off-by: Sathya Perla <sathyap@serverengines.com>
> ---
> drivers/net/benet/be_main.c | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
> index 68e7848..810d2a4 100644
> --- a/drivers/net/benet/be_main.c
> +++ b/drivers/net/benet/be_main.c
> @@ -1128,6 +1128,9 @@ static void be_tx_compl_clean(struct be_adapter *adapter)
> struct be_queue_info *txq = &adapter->tx_obj.q;
> struct be_eth_tx_compl *txcp;
> u16 end_idx, cmpl = 0, timeo = 0;
> + struct sk_buff **sent_skbs = adapter->tx_obj.sent_skb_list;
> + struct sk_buff *sent_skb;
> + bool dummy_wrb;
Some of your lines start with tabs, some with spaces. You
have to use tabs consistently.
> +
> + /* free posted tx for which compls will never arrive */
> + while (atomic_read(&txq->used)) {
> + sent_skb = sent_skbs[txq->tail];
> + end_idx = txq->tail;
> + index_adv(&end_idx,
> + wrb_cnt_for_skb(sent_skb, &dummy_wrb) - 1, txq->len);
> + be_tx_compl_process(adapter, end_idx);
> + }
> }
More tabbing and indentation problems here too.
^ permalink raw reply
* [patch] iwlwifi: testing the wrong variable
From: Dan Carpenter @ 2010-02-18 9:45 UTC (permalink / raw)
To: Zhu Yi
Cc: Reinette Chatre, Intel Linux Wireless, John W. Linville,
Wey-Yi Guy, Abhijeet Kolekar, Johannes Berg, linux-wireless,
netdev, linux-kernel, kernel-janitors
The first one fixes a smatch false positive and the second one fixes
a potential bug.
drivers/net/wireless/iwlwifi/iwl-5000.c +786 iwl5000_txq_update_byte_cnt_tbl(37) error: buffer overflow
'(scd_bc_tbl+txq_id)->tfd_offset' 320 <= 512
drivers/net/wireless/iwlwifi/iwl-5000.c +808 iwl5000_txq_inval_byte_cnt_tbl(19) error: buffer overflow
'(scd_bc_tbl+txq_id)->tfd_offset' 320 <= 512
Signed-off-by: Dan Carpenter <error27@gmail.com>
Cc: stable@kernel.org
---
I don't have the hardware to test this change.
diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index de45f30..b45150d 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -781,7 +781,7 @@ void iwl5000_txq_update_byte_cnt_tbl(struct iwl_priv *priv,
scd_bc_tbl[txq_id].tfd_offset[write_ptr] = bc_ent;
- if (txq->q.write_ptr < TFD_QUEUE_SIZE_BC_DUP)
+ if (write_ptr < TFD_QUEUE_SIZE_BC_DUP)
scd_bc_tbl[txq_id].
tfd_offset[TFD_QUEUE_SIZE_MAX + write_ptr] = bc_ent;
}
@@ -803,7 +803,7 @@ void iwl5000_txq_inval_byte_cnt_tbl(struct iwl_priv *priv,
bc_ent = cpu_to_le16(1 | (sta_id << 12));
scd_bc_tbl[txq_id].tfd_offset[read_ptr] = bc_ent;
- if (txq->q.write_ptr < TFD_QUEUE_SIZE_BC_DUP)
+ if (read_ptr < TFD_QUEUE_SIZE_BC_DUP)
scd_bc_tbl[txq_id].
tfd_offset[TFD_QUEUE_SIZE_MAX + read_ptr] = bc_ent;
}
^ permalink raw reply related
* RE: kernel stack trace using conntrack
From: Ramblewski David @ 2010-02-18 9:37 UTC (permalink / raw)
To: Eric Dumazet, Pablo Neira Ayuso; +Cc: netfilter-devel@vger.kernel.org, netdev
In-Reply-To: <1266327917.3045.55.camel@edumazet-laptop>
Hi Eric,
The conntrack patch works successfully.
Thanks for your help!
David
-----Message d'origine-----
De : Eric Dumazet [mailto:eric.dumazet@gmail.com]
Envoyé : mardi 16 février 2010 14:45
À : Pablo Neira Ayuso
Cc : Ramblewski David; netfilter-devel@vger.kernel.org; netdev
Objet : Re: kernel stack trace using conntrack
Le mardi 16 février 2010 à 14:33 +0100, Pablo Neira Ayuso a écrit :
> Eric Dumazet wrote:
> > OK thanks David, I reproduced the problem on latest net-next-2.6 tree
> > too. I wonder why nobody hit this before.
>
> Hmm, my config had not NETFILTER_DEBUG enabled, that's why I didn't hit
> that assertion.
>
> > [352468.556484] ------------[ cut here ]------------
> > [352468.556511] WARNING: at net/netfilter/nf_conntrack_extend.c:82
> > __nf_ct_ext_add+0x1c2/0x1e0 [nf_conntrack]()
> > [352468.556559] Hardware name: ProLiant BL460c G1
> > [352468.556582] Modules linked in: nf_defrag_ipv4 nf_conntrack_netlink
> > nf_conntrack sch_hfsc sch_sfq ipmi_devintf ipmi_si ipmi_msghandler hpilo
> > bonding [last unloaded: nf_conntrack_ipv4]
> > [352468.556675] Pid: 18852, comm: conntrack Tainted: G W
> > 2.6.33-rc5-02754-g0ea034c-dirty #545
> > [352468.556721] Call Trace:
> > [352468.556742] [<c054d45f>] ? printk+0x1d/0x26
> > [352468.556767] [<c023bbc2>] warn_slowpath_common+0x72/0xa0
> > [352468.556795] [<fee75e42>] ? __nf_ct_ext_add+0x1c2/0x1e0
> > [nf_conntrack]
> > [352468.556825] [<fee75e42>] ? __nf_ct_ext_add+0x1c2/0x1e0
> > [nf_conntrack]
> > [352468.556854] [<c023bc0a>] warn_slowpath_null+0x1a/0x20
> > [352468.556882] [<fee75e42>] __nf_ct_ext_add+0x1c2/0x1e0 [nf_conntrack]
> > [352468.556911] [<fee70dcc>] ? nf_conntrack_alloc+0x10c/0x1a0
> > [nf_conntrack]
> > [352468.556940] [<feecaf59>] ctnetlink_create_conntrack+0x339/0x360
> > [nf_conntrack_netlink]
> > [352468.556987] [<feeca26b>] ? ctnetlink_parse_tuple+0x14b/0x1c0
> > [nf_conntrack_netlink]
> > [352468.557039] [<fee6fd60>] ? __nf_conntrack_find+0x70/0x100
> > [nf_conntrack]
> > [352468.557068] [<feecb090>] ctnetlink_new_conntrack+0x110/0x680
> > [nf_conntrack_netlink]
> > [352468.557113] [<c04d93b5>] nfnetlink_rcv_msg+0x125/0x180
> > [352468.557140] [<c054ec57>] ? __mutex_lock_slowpath+0x197/0x230
> > [352468.557167] [<c04d9290>] ? nfnetlink_rcv_msg+0x0/0x180
> > [352468.557194] [<c04d5896>] netlink_rcv_skb+0x96/0xc0
> > [352468.557219] [<c04d927c>] nfnetlink_rcv+0x1c/0x30
> > [352468.557245] [<c04d5545>] netlink_unicast+0x255/0x2a0
> > [352468.557274] [<c04d5d3f>] netlink_sendmsg+0x1af/0x2b0
> > [352468.557300] [<c04a86ec>] sock_sendmsg+0xac/0xe0
> > [352468.559358] [<c029d042>] ? find_get_page+0x22/0xd0
> > [352468.559385] [<c029d9dc>] ? filemap_fault+0x8c/0x3c0
> > [352468.559410] [<c04a905a>] sys_sendto+0xaa/0xd0
> > [352468.559436] [<c02b3780>] ? __do_fault+0x370/0x470
> > [352468.559462] [<c02b54d9>] ? handle_mm_fault+0x1d9/0x7d0
> > [352468.559488] [<c04aa245>] sys_socketcall+0x195/0x280
> > [352468.559514] [<c0202c50>] sysenter_do_call+0x12/0x26
> > [352468.559539] ---[ end trace 6ecb842e4e35a653 ]---
> >
> > Could you try following patch ?
> >
> > Thanks
> >
> > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> > index 0ffe689..d2657aa 100644
> > --- a/net/netfilter/nf_conntrack_netlink.c
> > +++ b/net/netfilter/nf_conntrack_netlink.c
> > @@ -923,7 +923,7 @@ ctnetlink_change_status(struct nf_conn *ct, const struct nlattr * const cda[])
> > unsigned int status = ntohl(nla_get_be32(cda[CTA_STATUS]));
> > d = ct->status ^ status;
> >
> > - if (d & (IPS_EXPECTED|IPS_CONFIRMED|IPS_DYING))
> > + if (d & (IPS_EXPECTED|IPS_DYING))
> > /* unchangeable */
> > return -EBUSY;
>
> I think that we should explicitly report if the user unsets
> IPS_CONFIRMED. Please, don't change this.
>
> Apart from that, the patch seems fine to me. Thanks!
Problem is we now (I mean after my patch) enter
ctnetlink_change_status() with ct->status being null (or at least,
IPS_CONFIRMED not set)
Ce message et les pièces jointes sont confidentiels et réservés à l'usage exclusif de ses destinataires. Il peut également être protégé par le secret professionnel. Si vous recevez ce message par erreur, merci d'en avertir immédiatement l'expéditeur et de le détruire. L'intégrité du message ne pouvant être assurée sur Internet, la responsabilité du groupe Atos Origin ne pourra être recherchée quant au contenu de ce message. Bien que les meilleurs efforts soient faits pour maintenir cette transmission exempte de tout virus, l'expéditeur ne donne aucune garantie à cet égard et sa responsabilité ne saurait être recherchée pour tout dommage résultant d'un virus transmis.
This e-mail and the documents attached are confidential and intended solely for the addressee; it may also be privileged. If you receive this e-mail in error, please notify the sender immediately and destroy it. As its integrity cannot be secured on the Internet, the Atos Origin group liability cannot be triggered for the message content. Although the sender endeavours to maintain a computer virus-free network, the sender does not warrant that this transmission is virus-free and will not be liable for any damages resulting from any virus transmitted.
^ permalink raw reply
* [PATCH net-next-2.6] be2net: free tx buffers when completions never arrive
From: Sathya Perla @ 2010-02-18 9:36 UTC (permalink / raw)
To: netdev
In cases like when a pci device is disconnected after a pci error,
pending tx completions will never arrive. Unmap and free such
buffers in the tx cleanup path.
Signed-off-by: Sathya Perla <sathyap@serverengines.com>
---
drivers/net/benet/be_main.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 68e7848..810d2a4 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -1128,6 +1128,9 @@ static void be_tx_compl_clean(struct be_adapter *adapter)
struct be_queue_info *txq = &adapter->tx_obj.q;
struct be_eth_tx_compl *txcp;
u16 end_idx, cmpl = 0, timeo = 0;
+ struct sk_buff **sent_skbs = adapter->tx_obj.sent_skb_list;
+ struct sk_buff *sent_skb;
+ bool dummy_wrb;
/* Wait for a max of 200ms for all the tx-completions to arrive. */
do {
@@ -1151,6 +1154,15 @@ static void be_tx_compl_clean(struct be_adapter *adapter)
if (atomic_read(&txq->used))
dev_err(&adapter->pdev->dev, "%d pending tx-completions\n",
atomic_read(&txq->used));
+
+ /* free posted tx for which compls will never arrive */
+ while (atomic_read(&txq->used)) {
+ sent_skb = sent_skbs[txq->tail];
+ end_idx = txq->tail;
+ index_adv(&end_idx,
+ wrb_cnt_for_skb(sent_skb, &dummy_wrb) - 1, txq->len);
+ be_tx_compl_process(adapter, end_idx);
+ }
}
static void be_mcc_queues_destroy(struct be_adapter *adapter)
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH 4/5] xfrm: CONFIG_COMPAT support for x86 architecture
From: Florian Westphal @ 2010-02-18 9:33 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev
In-Reply-To: <1266479858.5564.8.camel@jlt3.sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> wrote:
> > +static bool xfrm_msg_compat(const struct sk_buff *skb)
> > +{
> > + return unlikely(!!NETLINK_CB(skb).msg_compat);
> > +}
>
> The !! seems pointless but that's not why I'm quoting this here.
>
> I think this function should only be used in the input path.
Fair enough. I'll see about introducing "bool compat" arguments
to the functions that currently use this helper in the output path.
> > +static bool xfrm_add_compatskb(struct sk_buff *skb,
> > + unsigned int len, gfp_t gfp)
> > +{
>
> I think this should return the new skb, and I don't think it should set
> the msg_compat flag in the NETLINK_CB.
Right, that would no longer be necessary if xfrm_msg_compat() is
restricted to the input path.
> > +static struct sk_buff *xfrm_get_compatskb(struct sk_buff *skb)
> > +{
>
> making this no longer needed.
>
[..]
> Shouldn't that be some sizeof() trickery instead of hardcoding 4?
I'll do that. For x86/x86_64 the delta is always 4 (or 0), though.
> > @@ -700,6 +856,9 @@ static int copy_one_state(struct sk_buff *skb,
> > struct xfrm_state *x, struct xfrm
> > size_t len = sizeof(*p);
> > int err;
> >
> > + if (xfrm_msg_compat(skb))
> > + len = sizeof(struct compat_xfrm_usersa_info);
> > +
>
> I really dislike this use though.
Alright, I will add a "bool is_compat" argument instead.
> > @@ -724,6 +883,13 @@ static int dump_one_state(struct xfrm_state *x,
> > int count, void *ptr)
> > struct xfrm_dump_info *sp = ptr;
> > struct sk_buff *skb = sp->out_skb;
> > int ret = copy_one_state(skb, x, sp);
> > +#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
> > + if (ret == 0) {
> > + skb = xfrm_get_compatskb(skb);
> > + if (skb)
> > + copy_one_state(skb, x, sp);
> > + }
> > +#endif
> > return ret;
> > }
>
> And that's really convoluted.
dump_one_state() is called from xfrm_state_walk(), so it has to prepare
both native and compat skb.
The "ifdef" is not necessary; I added it because gcc 4.3.4 did
no longer inline copy_one_state() without them in the
COMPAT_FOR_U64_ALIGNMENT=n case.
> > @@ -753,6 +919,8 @@ static int xfrm_dump_sa(struct sk_buff *skb,
> > struct netlink_callback *cb)
> > xfrm_state_walk_init(walk, 0);
> > }
> >
> > + xfrm_add_compatskb(skb, NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > +
> > (void) xfrm_state_walk(net, walk, dump_one_state, &info);
> >
> > return skb->len;
>
> And this just seems wrong -- doesn't that fill _only_ the compat skb
> here then??
No, dump_one_state() calls copy_one_state() twice (once for
skb, once for ->frag_list.
> > @@ -2200,6 +2455,9 @@ static int xfrm_exp_state_notify(struct
> > xfrm_state *x, struct km_event *c)
> > if (build_expire(skb, x, c) < 0)
> > BUG();
> >
> > + if (xfrm_add_compatskb(skb, compat_xfrm_expire_msgsize(), GFP_ATOMIC))
> > + compat_build_expire(skb_shinfo(skb)->frag_list, x, c);
>
> This is a good model, although I'd change it to be like this:
>
> if (cskb = xfrm_add_compatskb(skb, ...))
> compat_build_expire(cskb, x, c);
good point, I'll change this accordingly.
> And I think the code would be a lot clearer if the other user were like
>
> build_foo(skb, ..., false /* not compat */);
> if ((cskb = xfrm_add_compatskb(skb, ...)))
> build_foo(cskb, ..., true /* compat */);
>
> instead of all the trickery with setting msg_compat on the output path
> and then checking it again in the fill functions.
Thanks a lot for spending time on reviewing this; I'll make the changes
you suggested.
Florian
^ permalink raw reply
* Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts
From: Franco Fichtner @ 2010-02-18 9:25 UTC (permalink / raw)
To: Andreas Petlund
Cc: Ilpo Järvinen, Franco Fichtner, Netdev, eric.dumazet,
hannemann, LKML, shemminger, william.allen.simpson, damian,
ebiederm, David Miller
In-Reply-To: <4B7D05ED.4060900@simula.no>
Andreas Petlund wrote:
> On 02/18/2010 10:09 AM, Ilpo Järvinen wrote:
>
>> On Thu, 18 Feb 2010, Franco Fichtner wrote:
>>
>>
>>> Andreas Petlund wrote:
>>>
>>>> On 02/18/2010 09:41 AM, Ilpo Järvinen wrote:
>>>>
>>>>
>>>>> On Wed, 17 Feb 2010, David Miller wrote:
>>>>>
>>>>>
>>>>>
>>>>>> From: Andreas Petlund <apetlund@simula.no>
>>>>>> Date: Tue, 16 Feb 2010 15:40:41 +0100
>>>>>>
>>>>>>
>>>>>>
>>>>>>> @@ -341,6 +342,8 @@ struct tcp_sock {
>>>>>>> u16 advmss; /* Advertised MSS
>>>>>>> */
>>>>>>> u8 frto_counter; /* Number of new acks after RTO */
>>>>>>> u8 nonagle; /* Disable Nagle algorithm?
>>>>>>> */
>>>>>>> + u8 thin_lto : 1,/* Use linear timeouts for thin
>>>>>>> streams */
>>>>>>> + thin_undef : 7;
>>>>>>>
>>>>>>>
>>>>>> There is now a gap of 3 unused bytes here in this critical
>>>>>> core TCP socket data structure.
>>>>>>
>>>>>> Please either find a way to avoid this hole, or document
>>>>>> it with a comment.
>>>>>>
>>>>>>
>>>>> There would be multiple bits free for use in both frto_counter and nonagle
>>>>> byte.
>>>>>
>>>>>
>>>>>
>>>> I was playing aroud with this setup:
>>>>
>>>> =========
>>>> u8 nonagle : 4,/* Disable Nagle algorithm? */
>>>> thin_lto : 1,/* Use linear timeouts for thin streams */
>>>> thin_dupack : 1,/* Fast retransmit on first dupack */
>>>> thin_undef : 2;
>>>> =========
>>>>
>>>> Do you think that would do the trick?
>>>>
>>>>
>>> According to Ilpo, it would be ok to reduce both ftro_counter and
>>> nonagle, so why not join all these into u16 and leave the remaining
>>> free bits documented for other people. Like this:
>>>
>>> u16 frto_counter:x; /* Number of new acks after RTO */
>>> u16 nonagle:y; /* Disable Nagle algorithm? */
>>> u16 thin_lto:1; /* Use linear timeouts for thin streams */
>>> u16 unused:15-x-y;
>>>
>>> Not sure about the y and x. Ilpo, can you comment on those values?
>>>
>> I don't remember top of the hat how much of nonagle used, but for
>> frto_counter max value was 3 iirc.
>>
>
> I think nonagle uses 4 bits:
> ======
> #define TCP_NAGLE_OFF 1 /* Nagle's algo is disabled */
> #define TCP_NAGLE_CORK 2 /* Socket is corked */
> #define TCP_NAGLE_PUSH 4 /* Cork is overridden for already queued data */
> ======
>
>
That would be 3 bits. :)
>> However, I'm unsure if compiler is
>> nowadays wise enough to handle bitfields in some not all so stupid way.
>>
>
> Would you then recommend to use a byte for each value, thus avoiding the bitfields?
>
No, he meant he's not sure if the compiler can merge the bitfields in a
clever
way if written like this.
I use this style all the time at work and according to pahole it's okay.
But then,
I'm not sure if pahole can be trusted or if there is some compiler twist on
non-intel architectures.
Franco
^ permalink raw reply
* Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts
From: Ilpo Järvinen @ 2010-02-18 9:24 UTC (permalink / raw)
To: Andreas Petlund
Cc: Franco Fichtner, Netdev, eric.dumazet, hannemann, LKML,
shemminger, william.allen.simpson, damian, ebiederm, David Miller
In-Reply-To: <4B7D05ED.4060900@simula.no>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2838 bytes --]
On Thu, 18 Feb 2010, Andreas Petlund wrote:
> On 02/18/2010 10:09 AM, Ilpo Järvinen wrote:
> > On Thu, 18 Feb 2010, Franco Fichtner wrote:
> >
> >> Andreas Petlund wrote:
> >>> On 02/18/2010 09:41 AM, Ilpo Järvinen wrote:
> >>>
> >>>> On Wed, 17 Feb 2010, David Miller wrote:
> >>>>
> >>>>
> >>>>> From: Andreas Petlund <apetlund@simula.no>
> >>>>> Date: Tue, 16 Feb 2010 15:40:41 +0100
> >>>>>
> >>>>>
> >>>>>> @@ -341,6 +342,8 @@ struct tcp_sock {
> >>>>>> u16 advmss; /* Advertised MSS
> >>>>>> */
> >>>>>> u8 frto_counter; /* Number of new acks after RTO */
> >>>>>> u8 nonagle; /* Disable Nagle algorithm?
> >>>>>> */
> >>>>>> + u8 thin_lto : 1,/* Use linear timeouts for thin
> >>>>>> streams */
> >>>>>> + thin_undef : 7;
> >>>>>>
> >>>>> There is now a gap of 3 unused bytes here in this critical
> >>>>> core TCP socket data structure.
> >>>>>
> >>>>> Please either find a way to avoid this hole, or document
> >>>>> it with a comment.
> >>>>>
> >>>> There would be multiple bits free for use in both frto_counter and nonagle
> >>>> byte.
> >>>>
> >>>>
> >>>
> >>> I was playing aroud with this setup:
> >>>
> >>> =========
> >>> u8 nonagle : 4,/* Disable Nagle algorithm? */
> >>> thin_lto : 1,/* Use linear timeouts for thin streams */
> >>> thin_dupack : 1,/* Fast retransmit on first dupack */
> >>> thin_undef : 2;
> >>> =========
> >>>
> >>> Do you think that would do the trick?
> >>>
> >>
> >> According to Ilpo, it would be ok to reduce both ftro_counter and
> >> nonagle, so why not join all these into u16 and leave the remaining
> >> free bits documented for other people. Like this:
> >>
> >> u16 frto_counter:x; /* Number of new acks after RTO */
> >> u16 nonagle:y; /* Disable Nagle algorithm? */
> >> u16 thin_lto:1; /* Use linear timeouts for thin streams */
> >> u16 unused:15-x-y;
> >>
> >> Not sure about the y and x. Ilpo, can you comment on those values?
> >
> > I don't remember top of the hat how much of nonagle used, but for
> > frto_counter max value was 3 iirc.
>
> I think nonagle uses 4 bits:
> ======
> #define TCP_NAGLE_OFF 1 /* Nagle's algo is disabled */
> #define TCP_NAGLE_CORK 2 /* Socket is corked */
> #define TCP_NAGLE_PUSH 4 /* Cork is overridden for already queued data */
> ======
>
> > However, I'm unsure if compiler is
> > nowadays wise enough to handle bitfields in some not all so stupid way.
>
> Would you then recommend to use a byte for each value, thus avoiding the
> bitfields?
I don't know about the current compilers but at least in past there has
been a bias against bitfields. Alternative would be to combine and code
the accessors manually (thus bypassing any "too clever" compiler
intelligence).
--
i.
^ permalink raw reply
* [net-next-2.6 PATCH] smsc911x: convert to use netdev_for_each_mc_addr
From: Jiri Pirko @ 2010-02-18 9:22 UTC (permalink / raw)
To: netdev; +Cc: davem, steve.glendinning
also removed unnecessary checks
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/smsc911x.c | 27 +++++++++------------------
1 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index 13c0b76..4fd1d8b 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -1383,29 +1383,20 @@ static void smsc911x_set_multicast_list(struct net_device *dev)
/* Enabling specific multicast addresses */
unsigned int hash_high = 0;
unsigned int hash_low = 0;
- unsigned int count = 0;
- struct dev_mc_list *mc_list = dev->mc_list;
+ struct dev_mc_list *mc_list;
pdata->set_bits_mask = MAC_CR_HPFILT_;
pdata->clear_bits_mask = (MAC_CR_PRMS_ | MAC_CR_MCPAS_);
- while (mc_list) {
- count++;
- if ((mc_list->dmi_addrlen) == ETH_ALEN) {
- unsigned int bitnum =
- smsc911x_hash(mc_list->dmi_addr);
- unsigned int mask = 0x01 << (bitnum & 0x1F);
- if (bitnum & 0x20)
- hash_high |= mask;
- else
- hash_low |= mask;
- } else {
- SMSC_WARNING(DRV, "dmi_addrlen != 6");
- }
- mc_list = mc_list->next;
+ netdev_for_each_mc_addr(mc_list, dev) {
+ unsigned int bitnum = smsc911x_hash(mc_list->dmi_addr);
+ unsigned int mask = 0x01 << (bitnum & 0x1F);
+
+ if (bitnum & 0x20)
+ hash_high |= mask;
+ else
+ hash_low |= mask;
}
- if (count != (unsigned int)netdev_mc_count(dev))
- SMSC_WARNING(DRV, "mc_count != dev->mc_count");
pdata->hashhi = hash_high;
pdata->hashlo = hash_low;
--
1.6.6
^ permalink raw reply related
* Re: [net-next PATCH v4 3/3] net: TCP thin dupack
From: Andreas Petlund @ 2010-02-18 9:21 UTC (permalink / raw)
To: David Miller
Cc: netdev, ilpo.jarvinen, eric.dumazet, hannemann, linux-kernel,
shemminger, william.allen.simpson, damian, ebiederm
In-Reply-To: <20100217.163338.54972301.davem@davemloft.net>
On 02/18/2010 01:33 AM, David Miller wrote:
> From: Andreas Petlund <apetlund@simula.no>
> Date: Tue, 16 Feb 2010 15:40:46 +0100
>
>> Major changes:
>> -Renamed variables, ioctl and sysctl.
>> -Added sysctl documentation.
>> -Removed redundant allocation
>>
>> Signed-off-by: Andreas Petlund <apetlund@simula.no>
>
> Patches #2 and #3 look the same, what's going on here?
>
> Thanks.
The changes from v3 to v4 was basically in the naming. I
changed names for the given variables both for patch #2 and
patch #3. The removal of the unneeded allocation was also
done for both patches. It seems also I have forgotten to
include the general patch description for each patch. I will
fix this for the next round of changes.
Cheers,
Andreas
^ permalink raw reply
* Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts
From: Andreas Petlund @ 2010-02-18 9:18 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Franco Fichtner, Netdev, eric.dumazet, hannemann, LKML,
shemminger, william.allen.simpson, damian, ebiederm, David Miller
In-Reply-To: <alpine.DEB.2.00.1002181102260.9961@melkinpaasi.cs.helsinki.fi>
On 02/18/2010 10:09 AM, Ilpo Järvinen wrote:
> On Thu, 18 Feb 2010, Franco Fichtner wrote:
>
>> Andreas Petlund wrote:
>>> On 02/18/2010 09:41 AM, Ilpo Järvinen wrote:
>>>
>>>> On Wed, 17 Feb 2010, David Miller wrote:
>>>>
>>>>
>>>>> From: Andreas Petlund <apetlund@simula.no>
>>>>> Date: Tue, 16 Feb 2010 15:40:41 +0100
>>>>>
>>>>>
>>>>>> @@ -341,6 +342,8 @@ struct tcp_sock {
>>>>>> u16 advmss; /* Advertised MSS
>>>>>> */
>>>>>> u8 frto_counter; /* Number of new acks after RTO */
>>>>>> u8 nonagle; /* Disable Nagle algorithm?
>>>>>> */
>>>>>> + u8 thin_lto : 1,/* Use linear timeouts for thin
>>>>>> streams */
>>>>>> + thin_undef : 7;
>>>>>>
>>>>> There is now a gap of 3 unused bytes here in this critical
>>>>> core TCP socket data structure.
>>>>>
>>>>> Please either find a way to avoid this hole, or document
>>>>> it with a comment.
>>>>>
>>>> There would be multiple bits free for use in both frto_counter and nonagle
>>>> byte.
>>>>
>>>>
>>>
>>> I was playing aroud with this setup:
>>>
>>> =========
>>> u8 nonagle : 4,/* Disable Nagle algorithm? */
>>> thin_lto : 1,/* Use linear timeouts for thin streams */
>>> thin_dupack : 1,/* Fast retransmit on first dupack */
>>> thin_undef : 2;
>>> =========
>>>
>>> Do you think that would do the trick?
>>>
>>
>> According to Ilpo, it would be ok to reduce both ftro_counter and
>> nonagle, so why not join all these into u16 and leave the remaining
>> free bits documented for other people. Like this:
>>
>> u16 frto_counter:x; /* Number of new acks after RTO */
>> u16 nonagle:y; /* Disable Nagle algorithm? */
>> u16 thin_lto:1; /* Use linear timeouts for thin streams */
>> u16 unused:15-x-y;
>>
>> Not sure about the y and x. Ilpo, can you comment on those values?
>
> I don't remember top of the hat how much of nonagle used, but for
> frto_counter max value was 3 iirc.
I think nonagle uses 4 bits:
======
#define TCP_NAGLE_OFF 1 /* Nagle's algo is disabled */
#define TCP_NAGLE_CORK 2 /* Socket is corked */
#define TCP_NAGLE_PUSH 4 /* Cork is overridden for already queued data */
======
> However, I'm unsure if compiler is
> nowadays wise enough to handle bitfields in some not all so stupid way.
Would you then recommend to use a byte for each value, thus avoiding the bitfields?
Cheers,
Andreas
^ permalink raw reply
* [net-next-2.6 PATCH] 3c5xx: use netdev_mc_* helpers
From: Jiri Pirko @ 2010-02-18 9:12 UTC (permalink / raw)
To: netdev; +Cc: davem
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/3c501.c | 2 +-
drivers/net/3c505.c | 9 ++++-----
drivers/net/3c515.c | 2 +-
drivers/net/3c523.c | 9 ++++-----
drivers/net/3c527.c | 8 ++------
drivers/net/3c59x.c | 2 +-
6 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/net/3c501.c b/drivers/net/3c501.c
index 4d4cad3..b6de7b1 100644
--- a/drivers/net/3c501.c
+++ b/drivers/net/3c501.c
@@ -812,7 +812,7 @@ static void set_multicast_list(struct net_device *dev)
if (dev->flags & IFF_PROMISC) {
outb(RX_PROM, RX_CMD);
inb(RX_STATUS);
- } else if (dev->mc_list || dev->flags & IFF_ALLMULTI) {
+ } else if (!netdev_mc_empty(dev) || dev->flags & IFF_ALLMULTI) {
/* Multicast or all multicast is the same */
outb(RX_MULT, RX_CMD);
inb(RX_STATUS); /* Clear status. */
diff --git a/drivers/net/3c505.c b/drivers/net/3c505.c
index dadb46a..04b5bba 100644
--- a/drivers/net/3c505.c
+++ b/drivers/net/3c505.c
@@ -1216,7 +1216,7 @@ static int elp_close(struct net_device *dev)
static void elp_set_mc_list(struct net_device *dev)
{
elp_device *adapter = netdev_priv(dev);
- struct dev_mc_list *dmi = dev->mc_list;
+ struct dev_mc_list *dmi;
int i;
unsigned long flags;
@@ -1230,10 +1230,9 @@ static void elp_set_mc_list(struct net_device *dev)
/* if num_addrs==0 the list will be cleared */
adapter->tx_pcb.command = CMD_LOAD_MULTICAST_LIST;
adapter->tx_pcb.length = 6 * netdev_mc_count(dev);
- for (i = 0; i < netdev_mc_count(dev); i++) {
- memcpy(adapter->tx_pcb.data.multicast[i], dmi->dmi_addr, 6);
- dmi = dmi->next;
- }
+ i = 0;
+ netdev_for_each_mc_addr(dmi, dev)
+ memcpy(adapter->tx_pcb.data.multicast[i++], dmi->dmi_addr, 6);
adapter->got[CMD_LOAD_MULTICAST_LIST] = 0;
if (!send_pcb(dev, &adapter->tx_pcb))
pr_err("%s: couldn't send set_multicast command\n", dev->name);
diff --git a/drivers/net/3c515.c b/drivers/net/3c515.c
index 063b049..1e898b1 100644
--- a/drivers/net/3c515.c
+++ b/drivers/net/3c515.c
@@ -1536,7 +1536,7 @@ static void set_rx_mode(struct net_device *dev)
pr_debug("%s: Setting promiscuous mode.\n",
dev->name);
new_mode = SetRxFilter | RxStation | RxMulticast | RxBroadcast | RxProm;
- } else if ((dev->mc_list) || (dev->flags & IFF_ALLMULTI)) {
+ } else if (!netdev_mc_empty(dev) || dev->flags & IFF_ALLMULTI) {
new_mode = SetRxFilter | RxStation | RxMulticast | RxBroadcast;
} else
new_mode = SetRxFilter | RxStation | RxBroadcast;
diff --git a/drivers/net/3c523.c b/drivers/net/3c523.c
index 6948d66..beed4fa 100644
--- a/drivers/net/3c523.c
+++ b/drivers/net/3c523.c
@@ -625,7 +625,7 @@ static int init586(struct net_device *dev)
volatile struct iasetup_cmd_struct *ias_cmd;
volatile struct tdr_cmd_struct *tdr_cmd;
volatile struct mcsetup_cmd_struct *mc_cmd;
- struct dev_mc_list *dmi = dev->mc_list;
+ struct dev_mc_list *dmi;
int num_addrs = netdev_mc_count(dev);
ptr = (void *) ((char *) p->scb + sizeof(struct scb_struct));
@@ -787,10 +787,9 @@ static int init586(struct net_device *dev)
mc_cmd->cmd_cmd = CMD_MCSETUP | CMD_LAST;
mc_cmd->cmd_link = 0xffff;
mc_cmd->mc_cnt = num_addrs * 6;
- for (i = 0; i < num_addrs; i++) {
- memcpy((char *) mc_cmd->mc_list[i], dmi->dmi_addr, 6);
- dmi = dmi->next;
- }
+ i = 0;
+ netdev_for_each_mc_addr(dmi, dev)
+ memcpy((char *) mc_cmd->mc_list[i++], dmi->dmi_addr, 6);
p->scb->cbl_offset = make16(mc_cmd);
p->scb->cmd = CUC_START;
elmc_id_attn586();
diff --git a/drivers/net/3c527.c b/drivers/net/3c527.c
index ce98269..5c07b14 100644
--- a/drivers/net/3c527.c
+++ b/drivers/net/3c527.c
@@ -1533,9 +1533,7 @@ static void do_mc32_set_multicast_list(struct net_device *dev, int retry)
{
unsigned char block[62];
unsigned char *bp;
- struct dev_mc_list *dmc=dev->mc_list;
-
- int i;
+ struct dev_mc_list *dmc;
if(retry==0)
lp->mc_list_valid = 0;
@@ -1545,11 +1543,9 @@ static void do_mc32_set_multicast_list(struct net_device *dev, int retry)
block[0]=netdev_mc_count(dev);
bp=block+2;
- for(i=0;i<netdev_mc_count(dev);i++)
- {
+ netdev_for_each_mc_addr(dmc, dev) {
memcpy(bp, dmc->dmi_addr, 6);
bp+=6;
- dmc=dmc->next;
}
if(mc32_command_nowait(dev, 2, block,
2+6*netdev_mc_count(dev))==-1)
diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
index 5df46c2..f965431 100644
--- a/drivers/net/3c59x.c
+++ b/drivers/net/3c59x.c
@@ -2970,7 +2970,7 @@ static void set_rx_mode(struct net_device *dev)
if (vortex_debug > 3)
pr_notice("%s: Setting promiscuous mode.\n", dev->name);
new_mode = SetRxFilter|RxStation|RxMulticast|RxBroadcast|RxProm;
- } else if ((dev->mc_list) || (dev->flags & IFF_ALLMULTI)) {
+ } else if (!netdev_mc_empty(dev) || dev->flags & IFF_ALLMULTI) {
new_mode = SetRxFilter|RxStation|RxMulticast|RxBroadcast;
} else
new_mode = SetRxFilter | RxStation | RxBroadcast;
--
1.6.6
^ permalink raw reply related
* Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts
From: Ilpo Järvinen @ 2010-02-18 9:09 UTC (permalink / raw)
To: Franco Fichtner
Cc: Andreas Petlund, Netdev, eric.dumazet, hannemann, LKML,
shemminger, william.allen.simpson, damian, ebiederm, David Miller
In-Reply-To: <4B7D01CE.1030101@lastsummer.de>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2201 bytes --]
On Thu, 18 Feb 2010, Franco Fichtner wrote:
> Andreas Petlund wrote:
> > On 02/18/2010 09:41 AM, Ilpo Järvinen wrote:
> >
> > > On Wed, 17 Feb 2010, David Miller wrote:
> > >
> > >
> > > > From: Andreas Petlund <apetlund@simula.no>
> > > > Date: Tue, 16 Feb 2010 15:40:41 +0100
> > > >
> > > >
> > > > > @@ -341,6 +342,8 @@ struct tcp_sock {
> > > > > u16 advmss; /* Advertised MSS
> > > > > */
> > > > > u8 frto_counter; /* Number of new acks after RTO */
> > > > > u8 nonagle; /* Disable Nagle algorithm?
> > > > > */
> > > > > + u8 thin_lto : 1,/* Use linear timeouts for thin
> > > > > streams */
> > > > > + thin_undef : 7;
> > > > >
> > > > There is now a gap of 3 unused bytes here in this critical
> > > > core TCP socket data structure.
> > > >
> > > > Please either find a way to avoid this hole, or document
> > > > it with a comment.
> > > >
> > > There would be multiple bits free for use in both frto_counter and nonagle
> > > byte.
> > >
> > >
> >
> > I was playing aroud with this setup:
> >
> > =========
> > u8 nonagle : 4,/* Disable Nagle algorithm? */
> > thin_lto : 1,/* Use linear timeouts for thin streams */
> > thin_dupack : 1,/* Fast retransmit on first dupack */
> > thin_undef : 2;
> > =========
> >
> > Do you think that would do the trick?
> >
>
> According to Ilpo, it would be ok to reduce both ftro_counter and
> nonagle, so why not join all these into u16 and leave the remaining
> free bits documented for other people. Like this:
>
> u16 frto_counter:x; /* Number of new acks after RTO */
> u16 nonagle:y; /* Disable Nagle algorithm? */
> u16 thin_lto:1; /* Use linear timeouts for thin streams */
> u16 unused:15-x-y;
>
> Not sure about the y and x. Ilpo, can you comment on those values?
I don't remember top of the hat how much of nonagle used, but for
frto_counter max value was 3 iirc. However, I'm unsure if compiler is
nowadays wise enough to handle bitfields in some not all so stupid way.
Btw, also in ecn_flags we have some empty space. But all in all that
didn't vacate enough to get 4 bytes so I haven't taken effort of
combining.
--
i.
^ permalink raw reply
* Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts
From: Franco Fichtner @ 2010-02-18 9:01 UTC (permalink / raw)
To: Andreas Petlund
Cc: Ilpo Järvinen, Netdev, eric.dumazet, hannemann, LKML,
shemminger, william.allen.simpson, damian, ebiederm, David Miller
In-Reply-To: <4B7CFDE4.4010003@simula.no>
Hi,
Andreas Petlund wrote:
> On 02/18/2010 09:41 AM, Ilpo Järvinen wrote:
>
>> On Wed, 17 Feb 2010, David Miller wrote:
>>
>>
>>> From: Andreas Petlund <apetlund@simula.no>
>>> Date: Tue, 16 Feb 2010 15:40:41 +0100
>>>
>>>
>>>> @@ -341,6 +342,8 @@ struct tcp_sock {
>>>> u16 advmss; /* Advertised MSS */
>>>> u8 frto_counter; /* Number of new acks after RTO */
>>>> u8 nonagle; /* Disable Nagle algorithm? */
>>>> + u8 thin_lto : 1,/* Use linear timeouts for thin streams */
>>>> + thin_undef : 7;
>>>>
>>>>
>>> There is now a gap of 3 unused bytes here in this critical
>>> core TCP socket data structure.
>>>
>>> Please either find a way to avoid this hole, or document
>>> it with a comment.
>>>
>> There would be multiple bits free for use in both frto_counter and nonagle
>> byte.
>>
>>
>
> I was playing aroud with this setup:
>
> =========
> u8 nonagle : 4,/* Disable Nagle algorithm? */
> thin_lto : 1,/* Use linear timeouts for thin streams */
> thin_dupack : 1,/* Fast retransmit on first dupack */
> thin_undef : 2;
> =========
>
> Do you think that would do the trick?
>
According to Ilpo, it would be ok to reduce both ftro_counter and
nonagle, so why not join all these into u16 and leave the remaining
free bits documented for other people. Like this:
u16 frto_counter:x; /* Number of new acks after RTO */
u16 nonagle:y; /* Disable Nagle algorithm? */
u16 thin_lto:1; /* Use linear timeouts for thin streams */
u16 unused:15-x-y;
Not sure about the y and x. Ilpo, can you comment on those values?
Thanks,
Franco
^ permalink raw reply
* [net-next-2.6 PATCH] depca: remove forgotten needless inicialization
From: Jiri Pirko @ 2010-02-18 8:55 UTC (permalink / raw)
To: netdev; +Cc: davem
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/depca.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/depca.c b/drivers/net/depca.c
index 64a569b..744c192 100644
--- a/drivers/net/depca.c
+++ b/drivers/net/depca.c
@@ -1272,7 +1272,7 @@ static void set_multicast_list(struct net_device *dev)
static void SetMulticastFilter(struct net_device *dev)
{
struct depca_private *lp = netdev_priv(dev);
- struct dev_mc_list *dmi = dev->mc_list;
+ struct dev_mc_list *dmi;
char *addrs;
int i, j, bit, byte;
u16 hashcode;
--
1.6.6
^ permalink raw reply related
* Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts
From: Andreas Petlund @ 2010-02-18 8:44 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Netdev, eric.dumazet, hannemann, LKML, shemminger,
william.allen.simpson, damian, ebiederm, David Miller
In-Reply-To: <alpine.DEB.2.00.1002181040520.9961@melkinpaasi.cs.helsinki.fi>
On 02/18/2010 09:41 AM, Ilpo Järvinen wrote:
> On Wed, 17 Feb 2010, David Miller wrote:
>
>> From: Andreas Petlund <apetlund@simula.no>
>> Date: Tue, 16 Feb 2010 15:40:41 +0100
>>
>>> @@ -341,6 +342,8 @@ struct tcp_sock {
>>> u16 advmss; /* Advertised MSS */
>>> u8 frto_counter; /* Number of new acks after RTO */
>>> u8 nonagle; /* Disable Nagle algorithm? */
>>> + u8 thin_lto : 1,/* Use linear timeouts for thin streams */
>>> + thin_undef : 7;
>>>
>>
>> There is now a gap of 3 unused bytes here in this critical
>> core TCP socket data structure.
>>
>> Please either find a way to avoid this hole, or document
>> it with a comment.
>
> There would be multiple bits free for use in both frto_counter and nonagle
> byte.
>
I was playing aroud with this setup:
=========
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
thin_dupack : 1,/* Fast retransmit on first dupack */
thin_undef : 2;
=========
Do you think that would do the trick?
Regards,
Andreas
^ permalink raw reply
* Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts
From: Ilpo Järvinen @ 2010-02-18 8:41 UTC (permalink / raw)
To: apetlund
Cc: Netdev, eric.dumazet, hannemann, LKML, shemminger,
william.allen.simpson, damian, ebiederm, David Miller
In-Reply-To: <20100217.163255.133898079.davem@davemloft.net>
On Wed, 17 Feb 2010, David Miller wrote:
> From: Andreas Petlund <apetlund@simula.no>
> Date: Tue, 16 Feb 2010 15:40:41 +0100
>
> > @@ -341,6 +342,8 @@ struct tcp_sock {
> > u16 advmss; /* Advertised MSS */
> > u8 frto_counter; /* Number of new acks after RTO */
> > u8 nonagle; /* Disable Nagle algorithm? */
> > + u8 thin_lto : 1,/* Use linear timeouts for thin streams */
> > + thin_undef : 7;
> >
>
> There is now a gap of 3 unused bytes here in this critical
> core TCP socket data structure.
>
> Please either find a way to avoid this hole, or document
> it with a comment.
There would be multiple bits free for use in both frto_counter and nonagle
byte.
--
i.
^ permalink raw reply
* Re: [PATCH 4/5] xfrm: CONFIG_COMPAT support for x86 architecture
From: Johannes Berg @ 2010-02-18 7:57 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
In-Reply-To: <1266252393-20911-5-git-send-email-fw@strlen.de>
On Mon, 2010-02-15 at 17:46 +0100, Florian Westphal wrote:
> +static bool xfrm_msg_compat(const struct sk_buff *skb)
> +{
> + return unlikely(!!NETLINK_CB(skb).msg_compat);
> +}
The !! seems pointless but that's not why I'm quoting this here.
I think this function should only be used in the input path.
> +static bool xfrm_add_compatskb(struct sk_buff *skb,
> + unsigned int len, gfp_t gfp)
> +{
I think this should return the new skb, and I don't think it should set
the msg_compat flag in the NETLINK_CB.
> +static struct sk_buff *xfrm_get_compatskb(struct sk_buff *skb)
> +{
making this no longer needed.
> +/*
> + * avoids #ifdefs all over the place. Use of these must be
> conditional via
> + * xfrm_add_compatskb/xfrm_get_compatskb so compiler can remove
> branches.
> + */
> +#define compat_xfrm_user_expire xfrm_user_expire
> +#define compat_xfrm_user_acquire xfrm_user_acquire
> +#define compat_xfrm_user_polexpire xfrm_user_polexpire
> +#define compat_xfrm_userpolicy_info xfrm_userpolicy_info
> +#define compat_xfrm_usersa_info xfrm_usersa_info
> +#define compat_xfrm_userspi_info xfrm_userspi_info
Good thing I didn't have to deal with compat _input_ in wireless since I
had killed the netlink input code :))
> +#endif
> +
> +/*
> + * userspace size of some structures is smaller by this amount
> + * due to different u64 alignment on x86 platform.
> + *
> + * Some of the structures need to use the compat_* structure
> definition
> + * when accessing certain members.
> + */
> +static const u8 xfrm_msg_min_compat_pad[XFRM_NR_MSGTYPES] = {
> + [XFRM_MSG_NEWSA - XFRM_MSG_BASE] = 4, /* padding at end */
> + [XFRM_MSG_NEWPOLICY - XFRM_MSG_BASE] = 4, /* padding at end */
> +
> + [XFRM_MSG_ALLOCSPI - XFRM_MSG_BASE] = 4, /* access might need
> compat_ */
> + [XFRM_MSG_ACQUIRE - XFRM_MSG_BASE] = 4, /* access might need
> compat_ */
> + [XFRM_MSG_EXPIRE - XFRM_MSG_BASE] = 4, /* access might need
> compat_ */
> + [XFRM_MSG_UPDPOLICY - XFRM_MSG_BASE] = 4, /* padding at end */
> + [XFRM_MSG_UPDSA - XFRM_MSG_BASE] = 4, /* padding at end */
> + [XFRM_MSG_POLEXPIRE - XFRM_MSG_BASE] = 4, /* access might need
> compat_ */
> +};
Shouldn't that be some sizeof() trickery instead of hardcoding 4?
> +static int get_user_expire_hard(const struct sk_buff *skb,
> + const struct xfrm_user_expire *ue)
> +{
> + if (xfrm_msg_compat(skb)) {
> + const struct compat_xfrm_user_expire *cmpt;
> + cmpt = (const struct compat_xfrm_user_expire*) ue;
> + return cmpt->hard;
> + }
> + return ue->hard;
> +}
That looks good to me, good use of the compat helper.
> @@ -700,6 +856,9 @@ static int copy_one_state(struct sk_buff *skb,
> struct xfrm_state *x, struct xfrm
> size_t len = sizeof(*p);
> int err;
>
> + if (xfrm_msg_compat(skb))
> + len = sizeof(struct compat_xfrm_usersa_info);
> +
I really dislike this use though.
> @@ -724,6 +883,13 @@ static int dump_one_state(struct xfrm_state *x,
> int count, void *ptr)
> struct xfrm_dump_info *sp = ptr;
> struct sk_buff *skb = sp->out_skb;
> int ret = copy_one_state(skb, x, sp);
> +#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
> + if (ret == 0) {
> + skb = xfrm_get_compatskb(skb);
> + if (skb)
> + copy_one_state(skb, x, sp);
> + }
> +#endif
> return ret;
> }
And that's really convoluted.
> @@ -753,6 +919,8 @@ static int xfrm_dump_sa(struct sk_buff *skb,
> struct netlink_callback *cb)
> xfrm_state_walk_init(walk, 0);
> }
>
> + xfrm_add_compatskb(skb, NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +
> (void) xfrm_state_walk(net, walk, dump_one_state, &info);
>
> return skb->len;
And this just seems wrong -- doesn't that fill _only_ the compat skb
here then??
(same thing happens again in this file)
> @@ -2200,6 +2455,9 @@ static int xfrm_exp_state_notify(struct
> xfrm_state *x, struct km_event *c)
> if (build_expire(skb, x, c) < 0)
> BUG();
>
> + if (xfrm_add_compatskb(skb, compat_xfrm_expire_msgsize(), GFP_ATOMIC))
> + compat_build_expire(skb_shinfo(skb)->frag_list, x, c);
This is a good model, although I'd change it to be like this:
if (cskb = xfrm_add_compatskb(skb, ...))
compat_build_expire(cskb, x, c);
And I think the code would be a lot clearer if the other user were like
build_foo(skb, ..., false /* not compat */);
if ((cskb = xfrm_add_compatskb(skb, ...)))
build_foo(cskb, ..., true /* compat */);
instead of all the trickery with setting msg_compat on the output path
and then checking it again in the fill functions.
So for example:
> @@ -2302,6 +2567,11 @@ static int copy_to_user_xfrm_notify_sa(struct
> sk_buff *skb,
> int sizeof_xfrm_usersa_info = sizeof(*p);
> int headlen = xfrm_notify_sa_headlen(c);
>
> + if (xfrm_msg_compat(skb)) {
> + headlen = compat_xfrm_notify_sa_headlen(c);
> + sizeof_xfrm_usersa_info = sizeof(struct compat_xfrm_usersa_info);
> + }
That'd get a compat parameter, and use it here instead of the
xfrm_msg_compat(skb) check.
> nlh = nlmsg_put(skb, c->pid, c->seq, c->event, headlen, 0);
> if (nlh == NULL)
> goto nla_put_failure;
> @@ -2347,6 +2617,9 @@ static int xfrm_notify_sa(struct xfrm_state *x,
> struct km_event *c)
> if (copy_to_user_xfrm_notify_sa(skb, x, c))
> goto nla_put_failure;
>
> + if (xfrm_add_compatskb(skb, len, GFP_ATOMIC))
> + copy_to_user_xfrm_notify_sa(skb_shinfo(skb)->frag_list, x, c);
and instead of touching frag_list here, you'd pass the result of
xfrm_add_compatskb().
That way, the use of xfrm_msg_compat() is restricted to the input path,
which I think is much clearer.
johannes
^ permalink raw reply
* Re: [PATCH 2/5] netlink: store MSG_CMSG_COMPAT flag in netlink_skb_parms
From: Johannes Berg @ 2010-02-18 7:37 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
In-Reply-To: <1266252393-20911-3-git-send-email-fw@strlen.de>
[-- Attachment #1: Type: text/plain, Size: 496 bytes --]
On Mon, 2010-02-15 at 17:46 +0100, Florian Westphal wrote:
> This allows the netlink processing context to determine if the data
> needs any 32 bit fixups.
> --- a/include/linux/netlink.h
> +++ b/include/linux/netlink.h
> @@ -164,6 +164,7 @@ struct netlink_skb_parms {
> __u32 loginuid; /* Login (audit) uid */
> __u32 sessionid; /* Session id (audit) */
> __u32 sid; /* SELinux security id */
> + __u8 msg_compat; /* Message needs 32bit fixups */
bool?
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [net-next-2.6 PATCH 1/3] vlan: adds vlan_dev_select_queue
From: Eric Dumazet @ 2010-02-18 7:37 UTC (permalink / raw)
To: Vasu Dev; +Cc: David Miller, jeffrey.t.kirsher, netdev, gospo, vasu.dev
In-Reply-To: <1266446441.1891.96.camel@vi2.jf.intel.com>
Le mercredi 17 février 2010 à 14:40 -0800, Vasu Dev a écrit :
> > > ------------------
> > >
> > > For performance reason, we could avoid calling vlan_dev_select_queue()
> > > for real devices without ndo_select_queue() handler, this would need to
> > > mirror vlan_netdev_ops & vlan_netdev_accel_ops.
> > >
>
> This means vlan_netdev_ops & vlan_netdev_accel_ops not to be const
> anymore so that vlan_dev_select_queue() could be initialized to vlan ops
> based on real dev ndo_select_queue() present or not, should be okay to
> be not const anymore. I'll update patch as suggested.
No they should stay const
I said you should mirror (copy if you prefer) the structure, one with a
NULL ndo_select_queue pointer, one with a ndo_select_queue =
vlan_dev_select_queue.
Both structures are const.
You must not dynamiucally change the structure, because the same machine
might have a vlan over a device with no ndo_select_queue() method, and
another vlan over a device with a ndo_select_queue()
Thanks
^ permalink raw reply
* [PATCH V2 net-next 09/15] drivers/net/sis190.c: Use (pr|netdev|netif)_<level> macro helpers
From: Joe Perches @ 2010-02-18 6:00 UTC (permalink / raw)
To: netdev; +Cc: Francois Romieu, linux-kernel
In-Reply-To: <a9449fa4b0219cd2e0699f3afe4f8ca811b72843.1266454576.git.joe@perches.com>
Add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Remove #define PFX
Use pr_<level>
Use netdev_<level>
Use netif_<level> and netif_msg_<test>
Remove local #define net_<test> macros
Remove periods from formats
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/sis190.c | 152 +++++++++++++++++++++++++-------------------------
1 files changed, 75 insertions(+), 77 deletions(-)
diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
index bf2ffbb..267409b 100644
--- a/drivers/net/sis190.c
+++ b/drivers/net/sis190.c
@@ -17,7 +17,9 @@
See the file COPYING in this distribution for more information.
- */
+*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/module.h>
#include <linux/moduleparam.h>
@@ -32,17 +34,6 @@
#include <linux/dma-mapping.h>
#include <asm/irq.h>
-#define net_drv(p, arg...) if (netif_msg_drv(p)) \
- printk(arg)
-#define net_probe(p, arg...) if (netif_msg_probe(p)) \
- printk(arg)
-#define net_link(p, arg...) if (netif_msg_link(p)) \
- printk(arg)
-#define net_intr(p, arg...) if (netif_msg_intr(p)) \
- printk(arg)
-#define net_tx_err(p, arg...) if (netif_msg_tx_err(p)) \
- printk(arg)
-
#define PHY_MAX_ADDR 32
#define PHY_ID_ANY 0x1f
#define MII_REG_ANY 0x1f
@@ -50,7 +41,6 @@
#define DRV_VERSION "1.4"
#define DRV_NAME "sis190"
#define SIS190_DRIVER_NAME DRV_NAME " Gigabit Ethernet driver " DRV_VERSION
-#define PFX DRV_NAME ": "
#define sis190_rx_skb netif_rx
#define sis190_rx_quota(count, quota) count
@@ -382,7 +372,7 @@ static void __mdio_cmd(void __iomem *ioaddr, u32 ctl)
}
if (i > 99)
- printk(KERN_ERR PFX "PHY command failed !\n");
+ pr_err("PHY command failed !\n");
}
static void mdio_write(void __iomem *ioaddr, int phy_id, int reg, int val)
@@ -590,8 +580,7 @@ static int sis190_rx_interrupt(struct net_device *dev,
status = le32_to_cpu(desc->PSize);
- // net_intr(tp, KERN_INFO "%s: Rx PSize = %08x.\n", dev->name,
- // status);
+ //netif_info(tp, intr, dev, "Rx PSize = %08x\n", status);
if (sis190_rx_pkt_err(status, stats) < 0)
sis190_give_to_asic(desc, tp->rx_buf_sz);
@@ -602,9 +591,8 @@ static int sis190_rx_interrupt(struct net_device *dev,
struct pci_dev *pdev = tp->pci_dev;
if (unlikely(pkt_size > tp->rx_buf_sz)) {
- net_intr(tp, KERN_INFO
- "%s: (frag) status = %08x.\n",
- dev->name, status);
+ netif_info(tp, intr, dev,
+ "(frag) status = %08x\n", status);
stats->rx_dropped++;
stats->rx_length_errors++;
sis190_give_to_asic(desc, tp->rx_buf_sz);
@@ -638,12 +626,12 @@ static int sis190_rx_interrupt(struct net_device *dev,
tp->cur_rx = cur_rx;
delta = sis190_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx);
- if (!delta && count && netif_msg_intr(tp))
- printk(KERN_INFO "%s: no Rx buffer allocated.\n", dev->name);
+ if (!delta && count)
+ netif_info(tp, intr, dev, "no Rx buffer allocated\n");
tp->dirty_rx += delta;
- if (((tp->dirty_rx + NUM_RX_DESC) == tp->cur_rx) && netif_msg_intr(tp))
- printk(KERN_EMERG "%s: Rx buffers exhausted.\n", dev->name);
+ if ((tp->dirty_rx + NUM_RX_DESC) == tp->cur_rx)
+ netif_emerg(tp, intr, dev, "Rx buffers exhausted\n");
return count;
}
@@ -752,10 +740,10 @@ static irqreturn_t sis190_interrupt(int irq, void *__dev)
SIS_W32(IntrStatus, status);
- // net_intr(tp, KERN_INFO "%s: status = %08x.\n", dev->name, status);
+// netif_info(tp, intr, dev, "status = %08x\n", status);
if (status & LinkChange) {
- net_intr(tp, KERN_INFO "%s: link change.\n", dev->name);
+ netif_info(tp, intr, dev, "link change\n");
schedule_work(&tp->phy_task);
}
@@ -933,8 +921,7 @@ static void sis190_phy_task(struct work_struct *work)
} else if (!(mdio_read_latched(ioaddr, phy_id, MII_BMSR) &
BMSR_ANEGCOMPLETE)) {
netif_carrier_off(dev);
- net_link(tp, KERN_WARNING "%s: auto-negotiating...\n",
- dev->name);
+ netif_warn(tp, link, dev, "auto-negotiating...\n");
mod_timer(&tp->timer, jiffies + SIS190_PHY_TIMEOUT);
} else {
/* Rejoice ! */
@@ -960,13 +947,13 @@ static void sis190_phy_task(struct work_struct *work)
u16 adv, autoexp, gigadv, gigrec;
val = mdio_read(ioaddr, phy_id, 0x1f);
- net_link(tp, KERN_INFO "%s: mii ext = %04x.\n", dev->name, val);
+ netif_info(tp, link, dev, "mii ext = %04x\n", val);
val = mdio_read(ioaddr, phy_id, MII_LPA);
adv = mdio_read(ioaddr, phy_id, MII_ADVERTISE);
autoexp = mdio_read(ioaddr, phy_id, MII_EXPANSION);
- net_link(tp, KERN_INFO "%s: mii lpa=%04x adv=%04x exp=%04x.\n",
- dev->name, val, adv, autoexp);
+ netif_info(tp, link, dev, "mii lpa=%04x adv=%04x exp=%04x\n",
+ val, adv, autoexp);
if (val & LPA_NPAGE && autoexp & EXPANSION_NWAY) {
/* check for gigabit speed */
@@ -1007,8 +994,7 @@ static void sis190_phy_task(struct work_struct *work)
tp->negotiated_lpa = p->val;
- net_link(tp, KERN_INFO "%s: link on %s mode.\n", dev->name,
- p->msg);
+ netif_info(tp, link, dev, "link on %s mode\n", p->msg);
netif_carrier_on(dev);
}
@@ -1194,9 +1180,8 @@ static netdev_tx_t sis190_start_xmit(struct sk_buff *skb,
if (unlikely(le32_to_cpu(desc->status) & OWNbit)) {
netif_stop_queue(dev);
- net_tx_err(tp, KERN_ERR PFX
- "%s: BUG! Tx Ring full when queue awake!\n",
- dev->name);
+ netif_err(tp, tx_err, dev,
+ "BUG! Tx Ring full when queue awake!\n");
return NETDEV_TX_BUSY;
}
@@ -1296,9 +1281,9 @@ static u16 sis190_default_phy(struct net_device *dev)
if (mii_if->phy_id != phy_default->phy_id) {
mii_if->phy_id = phy_default->phy_id;
- net_probe(tp, KERN_INFO
- "%s: Using transceiver at address %d as default.\n",
- pci_name(tp->pci_dev), mii_if->phy_id);
+ if (netif_msg_probe(tp))
+ pr_info("%s: Using transceiver at address %d as default\n",
+ pci_name(tp->pci_dev), mii_if->phy_id);
}
status = mdio_read(ioaddr, mii_if->phy_id, MII_BMCR);
@@ -1336,14 +1321,15 @@ static void sis190_init_phy(struct net_device *dev, struct sis190_private *tp,
((mii_status & (BMSR_100FULL | BMSR_100HALF)) ?
LAN : HOME) : p->type;
tp->features |= p->feature;
- net_probe(tp, KERN_INFO "%s: %s transceiver at address %d.\n",
- pci_name(tp->pci_dev), p->name, phy_id);
+ if (netif_msg_probe(tp))
+ pr_info("%s: %s transceiver at address %d\n",
+ pci_name(tp->pci_dev), p->name, phy_id);
} else {
phy->type = UNKNOWN;
- net_probe(tp, KERN_INFO
- "%s: unknown PHY 0x%x:0x%x transceiver at address %d\n",
- pci_name(tp->pci_dev),
- phy->id[0], (phy->id[1] & 0xfff0), phy_id);
+ if (netif_msg_probe(tp))
+ pr_info("%s: unknown PHY 0x%x:0x%x transceiver at address %d\n",
+ pci_name(tp->pci_dev),
+ phy->id[0], (phy->id[1] & 0xfff0), phy_id);
}
}
@@ -1407,8 +1393,9 @@ static int __devinit sis190_mii_probe(struct net_device *dev)
}
if (list_empty(&tp->first_phy)) {
- net_probe(tp, KERN_INFO "%s: No MII transceivers found!\n",
- pci_name(tp->pci_dev));
+ if (netif_msg_probe(tp))
+ pr_info("%s: No MII transceivers found!\n",
+ pci_name(tp->pci_dev));
rc = -EIO;
goto out;
}
@@ -1454,7 +1441,8 @@ static struct net_device * __devinit sis190_init_board(struct pci_dev *pdev)
dev = alloc_etherdev(sizeof(*tp));
if (!dev) {
- net_drv(&debug, KERN_ERR PFX "unable to alloc new ethernet\n");
+ if (netif_msg_drv(&debug))
+ pr_err("unable to alloc new ethernet\n");
rc = -ENOMEM;
goto err_out_0;
}
@@ -1467,34 +1455,39 @@ static struct net_device * __devinit sis190_init_board(struct pci_dev *pdev)
rc = pci_enable_device(pdev);
if (rc < 0) {
- net_probe(tp, KERN_ERR "%s: enable failure\n", pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_err("%s: enable failure\n", pci_name(pdev));
goto err_free_dev_1;
}
rc = -ENODEV;
if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM)) {
- net_probe(tp, KERN_ERR "%s: region #0 is no MMIO resource.\n",
- pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_err("%s: region #0 is no MMIO resource\n",
+ pci_name(pdev));
goto err_pci_disable_2;
}
if (pci_resource_len(pdev, 0) < SIS190_REGS_SIZE) {
- net_probe(tp, KERN_ERR "%s: invalid PCI region size(s).\n",
- pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_err("%s: invalid PCI region size(s)\n",
+ pci_name(pdev));
goto err_pci_disable_2;
}
rc = pci_request_regions(pdev, DRV_NAME);
if (rc < 0) {
- net_probe(tp, KERN_ERR PFX "%s: could not request regions.\n",
- pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_err("%s: could not request regions\n",
+ pci_name(pdev));
goto err_pci_disable_2;
}
rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
if (rc < 0) {
- net_probe(tp, KERN_ERR "%s: DMA configuration failed.\n",
- pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_err("%s: DMA configuration failed\n",
+ pci_name(pdev));
goto err_free_res_3;
}
@@ -1502,8 +1495,9 @@ static struct net_device * __devinit sis190_init_board(struct pci_dev *pdev)
ioaddr = ioremap(pci_resource_start(pdev, 0), SIS190_REGS_SIZE);
if (!ioaddr) {
- net_probe(tp, KERN_ERR "%s: cannot remap MMIO, aborting\n",
- pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_err("%s: cannot remap MMIO, aborting\n",
+ pci_name(pdev));
rc = -EIO;
goto err_free_res_3;
}
@@ -1539,9 +1533,8 @@ static void sis190_tx_timeout(struct net_device *dev)
if (tmp8 & CmdTxEnb)
SIS_W8(TxControl, tmp8 & ~CmdTxEnb);
-
- net_tx_err(tp, KERN_INFO "%s: Transmit timeout, status %08x %08x.\n",
- dev->name, SIS_R32(TxControl), SIS_R32(TxSts));
+ netif_info(tp, tx_err, dev, "Transmit timeout, status %08x %08x\n",
+ SIS_R32(TxControl), SIS_R32(TxSts));
/* Disable interrupts by clearing the interrupt mask. */
SIS_W32(IntrMask, 0x0000);
@@ -1570,15 +1563,16 @@ static int __devinit sis190_get_mac_addr_from_eeprom(struct pci_dev *pdev,
u16 sig;
int i;
- net_probe(tp, KERN_INFO "%s: Read MAC address from EEPROM\n",
- pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_info("%s: Read MAC address from EEPROM\n", pci_name(pdev));
/* Check to see if there is a sane EEPROM */
sig = (u16) sis190_read_eeprom(ioaddr, EEPROMSignature);
if ((sig == 0xffff) || (sig == 0x0000)) {
- net_probe(tp, KERN_INFO "%s: Error EEPROM read %x.\n",
- pci_name(pdev), sig);
+ if (netif_msg_probe(tp))
+ pr_info("%s: Error EEPROM read %x\n",
+ pci_name(pdev), sig);
return -EIO;
}
@@ -1612,8 +1606,8 @@ static int __devinit sis190_get_mac_addr_from_apc(struct pci_dev *pdev,
u8 reg, tmp8;
unsigned int i;
- net_probe(tp, KERN_INFO "%s: Read MAC address from APC.\n",
- pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_info("%s: Read MAC address from APC\n", pci_name(pdev));
for (i = 0; i < ARRAY_SIZE(ids); i++) {
isa_bridge = pci_get_device(PCI_VENDOR_ID_SI, ids[i], NULL);
@@ -1622,8 +1616,9 @@ static int __devinit sis190_get_mac_addr_from_apc(struct pci_dev *pdev,
}
if (!isa_bridge) {
- net_probe(tp, KERN_INFO "%s: Can not find ISA bridge.\n",
- pci_name(pdev));
+ if (netif_msg_probe(tp))
+ pr_info("%s: Can not find ISA bridge\n",
+ pci_name(pdev));
return -EIO;
}
@@ -1704,7 +1699,7 @@ static void sis190_set_speed_auto(struct net_device *dev)
int phy_id = tp->mii_if.phy_id;
int val;
- net_link(tp, KERN_INFO "%s: Enabling Auto-negotiation.\n", dev->name);
+ netif_info(tp, link, dev, "Enabling Auto-negotiation\n");
val = mdio_read(ioaddr, phy_id, MII_ADVERTISE);
@@ -1831,7 +1826,8 @@ static int __devinit sis190_init_one(struct pci_dev *pdev,
int rc;
if (!printed_version) {
- net_drv(&debug, KERN_INFO SIS190_DRIVER_NAME " loaded.\n");
+ if (netif_msg_drv(&debug))
+ pr_info(SIS190_DRIVER_NAME " loaded\n");
printed_version = 1;
}
@@ -1871,12 +1867,14 @@ static int __devinit sis190_init_one(struct pci_dev *pdev,
if (rc < 0)
goto err_remove_mii;
- net_probe(tp, KERN_INFO "%s: %s at %p (IRQ: %d), %pM\n",
- pci_name(pdev), sis_chip_info[ent->driver_data].name,
- ioaddr, dev->irq, dev->dev_addr);
-
- net_probe(tp, KERN_INFO "%s: %s mode.\n", dev->name,
- (tp->features & F_HAS_RGMII) ? "RGMII" : "GMII");
+ if (netif_msg_probe(tp)) {
+ netdev_info(dev, "%s: %s at %p (IRQ: %d), %pM\n",
+ pci_name(pdev),
+ sis_chip_info[ent->driver_data].name,
+ ioaddr, dev->irq, dev->dev_addr);
+ netdev_info(dev, "%s mode.\n",
+ (tp->features & F_HAS_RGMII) ? "RGMII" : "GMII");
+ }
netif_carrier_off(dev);
^ permalink raw reply related
* [PATCH V2 net-next 13/15] drivers/net/tg3.c: Use (pr|netdev)_<level> macro helpers
From: Joe Perches @ 2010-02-18 5:44 UTC (permalink / raw)
To: netdev; +Cc: Matt Carlson, Michael Chan, linux-kernel
In-Reply-To: <89936828c4a4c914b849cf8c96985d62fb612e55.1266454576.git.joe@perches.com>
Add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Remove #define PFX
Use pr_<level>
Use netdev_<level>
Remove periods from most formats
Coalesce long formats
Use printk_once
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/tg3.c | 270 +++++++++++++++++++++-------------------------------
1 files changed, 109 insertions(+), 161 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index ddf6615..f204f73 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -67,7 +67,6 @@
#include "tg3.h"
#define DRV_MODULE_NAME "tg3"
-#define PFX DRV_MODULE_NAME ": "
#define DRV_MODULE_VERSION "3.108"
#define DRV_MODULE_RELDATE "February 17, 2010"
@@ -158,7 +157,7 @@
#define FIRMWARE_TG3TSO5 "tigon/tg3_tso5.bin"
static char version[] __devinitdata =
- DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
+ DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")";
MODULE_AUTHOR("David S. Miller (davem@redhat.com) and Jeff Garzik (jgarzik@pobox.com)");
MODULE_DESCRIPTION("Broadcom Tigon3 ethernet driver");
@@ -1099,8 +1098,7 @@ static int tg3_mdio_init(struct tg3 *tp)
i = mdiobus_register(tp->mdio_bus);
if (i) {
- printk(KERN_WARNING "%s: mdiobus_reg failed (0x%x)\n",
- tp->dev->name, i);
+ netdev_warn(tp->dev, "mdiobus_reg failed (0x%x)\n", i);
mdiobus_free(tp->mdio_bus);
return i;
}
@@ -1108,7 +1106,7 @@ static int tg3_mdio_init(struct tg3 *tp)
phydev = tp->mdio_bus->phy_map[TG3_PHY_MII_ADDR];
if (!phydev || !phydev->drv) {
- printk(KERN_WARNING "%s: No PHY devices\n", tp->dev->name);
+ netdev_warn(tp->dev, "No PHY devices\n");
mdiobus_unregister(tp->mdio_bus);
mdiobus_free(tp->mdio_bus);
return -ENODEV;
@@ -1252,27 +1250,22 @@ static void tg3_ump_link_report(struct tg3 *tp)
static void tg3_link_report(struct tg3 *tp)
{
if (!netif_carrier_ok(tp->dev)) {
- if (netif_msg_link(tp))
- printk(KERN_INFO PFX "%s: Link is down.\n",
- tp->dev->name);
+ netif_info(tp, link, tp->dev, "Link is down\n");
tg3_ump_link_report(tp);
} else if (netif_msg_link(tp)) {
- printk(KERN_INFO PFX "%s: Link is up at %d Mbps, %s duplex.\n",
- tp->dev->name,
- (tp->link_config.active_speed == SPEED_1000 ?
- 1000 :
- (tp->link_config.active_speed == SPEED_100 ?
- 100 : 10)),
- (tp->link_config.active_duplex == DUPLEX_FULL ?
- "full" : "half"));
-
- printk(KERN_INFO PFX
- "%s: Flow control is %s for TX and %s for RX.\n",
- tp->dev->name,
- (tp->link_config.active_flowctrl & FLOW_CTRL_TX) ?
- "on" : "off",
- (tp->link_config.active_flowctrl & FLOW_CTRL_RX) ?
- "on" : "off");
+ netdev_info(tp->dev, "Link is up at %d Mbps, %s duplex\n",
+ (tp->link_config.active_speed == SPEED_1000 ?
+ 1000 :
+ (tp->link_config.active_speed == SPEED_100 ?
+ 100 : 10)),
+ (tp->link_config.active_duplex == DUPLEX_FULL ?
+ "full" : "half"));
+
+ netdev_info(tp->dev, "Flow control is %s for TX and %s for RX\n",
+ (tp->link_config.active_flowctrl & FLOW_CTRL_TX) ?
+ "on" : "off",
+ (tp->link_config.active_flowctrl & FLOW_CTRL_RX) ?
+ "on" : "off");
tg3_ump_link_report(tp);
}
}
@@ -1471,7 +1464,7 @@ static int tg3_phy_init(struct tg3 *tp)
phydev = phy_connect(tp->dev, dev_name(&phydev->dev), tg3_adjust_link,
phydev->dev_flags, phydev->interface);
if (IS_ERR(phydev)) {
- printk(KERN_ERR "%s: Could not attach to PHY\n", tp->dev->name);
+ netdev_err(tp->dev, "Could not attach to PHY\n");
return PTR_ERR(phydev);
}
@@ -2500,8 +2493,8 @@ static int tg3_set_power_state(struct tg3 *tp, pci_power_t state)
break;
default:
- printk(KERN_ERR PFX "%s: Invalid power state (D%d) requested\n",
- tp->dev->name, state);
+ netdev_err(tp->dev, "Invalid power state (D%d) requested\n",
+ state);
return -EINVAL;
}
@@ -4342,10 +4335,8 @@ static void tg3_tx_recover(struct tg3 *tp)
BUG_ON((tp->tg3_flags & TG3_FLAG_MBOX_WRITE_REORDER) ||
tp->write32_tx_mbox == tg3_write_indirect_mbox);
- printk(KERN_WARNING PFX "%s: The system may be re-ordering memory-"
- "mapped I/O cycles to the network device, attempting to "
- "recover. Please report the problem to the driver maintainer "
- "and include system chipset information.\n", tp->dev->name);
+ netdev_warn(tp->dev, "The system may be re-ordering memory-mapped I/O cycles to the network device, attempting to recover\n"
+ "Please report the problem to the driver maintainer and include system chipset information.\n");
spin_lock(&tp->lock);
tp->tg3_flags |= TG3_FLAG_TX_RECOVERY_PENDING;
@@ -5269,8 +5260,7 @@ static int tg3_restart_hw(struct tg3 *tp, int reset_phy)
err = tg3_init_hw(tp, reset_phy);
if (err) {
- printk(KERN_ERR PFX "%s: Failed to re-initialize device, "
- "aborting.\n", tp->dev->name);
+ netdev_err(tp->dev, "Failed to re-initialize device, aborting\n");
tg3_halt(tp, RESET_KIND_SHUTDOWN, 1);
tg3_full_unlock(tp);
del_timer_sync(&tp->timer);
@@ -5343,10 +5333,10 @@ out:
static void tg3_dump_short_state(struct tg3 *tp)
{
- printk(KERN_ERR PFX "DEBUG: MAC_TX_STATUS[%08x] MAC_RX_STATUS[%08x]\n",
- tr32(MAC_TX_STATUS), tr32(MAC_RX_STATUS));
- printk(KERN_ERR PFX "DEBUG: RDMAC_STATUS[%08x] WDMAC_STATUS[%08x]\n",
- tr32(RDMAC_STATUS), tr32(WDMAC_STATUS));
+ netdev_err(tp->dev, "DEBUG: MAC_TX_STATUS[%08x] MAC_RX_STATUS[%08x]\n",
+ tr32(MAC_TX_STATUS), tr32(MAC_RX_STATUS));
+ netdev_err(tp->dev, "DEBUG: RDMAC_STATUS[%08x] WDMAC_STATUS[%08x]\n",
+ tr32(RDMAC_STATUS), tr32(WDMAC_STATUS));
}
static void tg3_tx_timeout(struct net_device *dev)
@@ -5354,8 +5344,7 @@ static void tg3_tx_timeout(struct net_device *dev)
struct tg3 *tp = netdev_priv(dev);
if (netif_msg_tx_err(tp)) {
- printk(KERN_ERR PFX "%s: transmit timed out, resetting\n",
- dev->name);
+ netdev_err(dev, "transmit timed out, resetting\n");
tg3_dump_short_state(tp);
}
@@ -5519,8 +5508,7 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
netif_tx_stop_queue(txq);
/* This is a hard error, log it. */
- printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
- "queue awake!\n", dev->name);
+ netdev_err(dev, "BUG! Tx Ring full when queue awake!\n");
}
return NETDEV_TX_BUSY;
}
@@ -5723,8 +5711,7 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
netif_tx_stop_queue(txq);
/* This is a hard error, log it. */
- printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
- "queue awake!\n", dev->name);
+ netdev_err(dev, "BUG! Tx Ring full when queue awake!\n");
}
return NETDEV_TX_BUSY;
}
@@ -6071,11 +6058,8 @@ static int tg3_rx_prodring_alloc(struct tg3 *tp,
/* Now allocate fresh SKBs for each rx ring. */
for (i = 0; i < tp->rx_pending; i++) {
if (tg3_alloc_rx_skb(tp, tpr, RXD_OPAQUE_RING_STD, i) < 0) {
- printk(KERN_WARNING PFX
- "%s: Using a smaller RX standard ring, "
- "only %d out of %d buffers were allocated "
- "successfully.\n",
- tp->dev->name, i, tp->rx_pending);
+ netdev_warn(tp->dev, "Using a smaller RX standard ring, only %d out of %d buffers were allocated successfully\n",
+ i, tp->rx_pending);
if (i == 0)
goto initfail;
tp->rx_pending = i;
@@ -6104,11 +6088,8 @@ static int tg3_rx_prodring_alloc(struct tg3 *tp,
for (i = 0; i < tp->rx_jumbo_pending; i++) {
if (tg3_alloc_rx_skb(tp, tpr, RXD_OPAQUE_RING_JUMBO, i) < 0) {
- printk(KERN_WARNING PFX
- "%s: Using a smaller RX jumbo ring, "
- "only %d out of %d buffers were "
- "allocated successfully.\n",
- tp->dev->name, i, tp->rx_jumbo_pending);
+ netdev_warn(tp->dev, "Using a smaller RX jumbo ring, only %d out of %d buffers were allocated successfully\n",
+ i, tp->rx_jumbo_pending);
if (i == 0)
goto initfail;
tp->rx_jumbo_pending = i;
@@ -6452,8 +6433,7 @@ static int tg3_stop_block(struct tg3 *tp, unsigned long ofs, u32 enable_bit, int
}
if (i == MAX_WAIT_CNT && !silent) {
- printk(KERN_ERR PFX "tg3_stop_block timed out, "
- "ofs=%lx enable_bit=%x\n",
+ pr_err("tg3_stop_block timed out, ofs=%lx enable_bit=%x\n",
ofs, enable_bit);
return -ENODEV;
}
@@ -6500,9 +6480,8 @@ static int tg3_abort_hw(struct tg3 *tp, int silent)
break;
}
if (i >= MAX_WAIT_CNT) {
- printk(KERN_ERR PFX "tg3_abort_hw timed out for %s, "
- "TX_MODE_ENABLE will not clear MAC_TX_MODE=%08x\n",
- tp->dev->name, tr32(MAC_TX_MODE));
+ netdev_err(tp->dev, "%s timed out, TX_MODE_ENABLE will not clear MAC_TX_MODE=%08x\n",
+ __func__, tr32(MAC_TX_MODE));
err |= -ENODEV;
}
@@ -6723,8 +6702,7 @@ static int tg3_poll_fw(struct tg3 *tp)
!(tp->tg3_flags2 & TG3_FLG2_NO_FWARE_REPORTED)) {
tp->tg3_flags2 |= TG3_FLG2_NO_FWARE_REPORTED;
- printk(KERN_INFO PFX "%s: No firmware running.\n",
- tp->dev->name);
+ netdev_info(tp->dev, "No firmware running\n");
}
if (tp->pci_chip_rev_id == CHIPREV_ID_57765_A0) {
@@ -7152,10 +7130,8 @@ static int tg3_halt_cpu(struct tg3 *tp, u32 offset)
}
if (i >= 10000) {
- printk(KERN_ERR PFX "tg3_reset_cpu timed out for %s, "
- "and %s CPU\n",
- tp->dev->name,
- (offset == RX_CPU_BASE ? "RX" : "TX"));
+ netdev_err(tp->dev, "%s timed out, %s CPU\n",
+ __func__, offset == RX_CPU_BASE ? "RX" : "TX");
return -ENODEV;
}
@@ -7180,9 +7156,8 @@ static int tg3_load_firmware_cpu(struct tg3 *tp, u32 cpu_base, u32 cpu_scratch_b
if (cpu_base == TX_CPU_BASE &&
(tp->tg3_flags2 & TG3_FLG2_5705_PLUS)) {
- printk(KERN_ERR PFX "tg3_load_firmware_cpu: Trying to load "
- "TX cpu firmware on %s which is 5705.\n",
- tp->dev->name);
+ netdev_err(tp->dev, "%s: Trying to load TX cpu firmware which is 5705\n",
+ __func__);
return -EINVAL;
}
@@ -7261,10 +7236,8 @@ static int tg3_load_5701_a0_firmware_fix(struct tg3 *tp)
udelay(1000);
}
if (i >= 5) {
- printk(KERN_ERR PFX "tg3_load_firmware fails for %s "
- "to set RX CPU PC, is %08x should be %08x\n",
- tp->dev->name, tr32(RX_CPU_BASE + CPU_PC),
- info.fw_base);
+ netdev_err(tp->dev, "tg3_load_firmware fails to set RX CPU PC, is %08x should be %08x\n",
+ tr32(RX_CPU_BASE + CPU_PC), info.fw_base);
return -ENODEV;
}
tw32(RX_CPU_BASE + CPU_STATE, 0xffffffff);
@@ -7327,10 +7300,8 @@ static int tg3_load_tso_firmware(struct tg3 *tp)
udelay(1000);
}
if (i >= 5) {
- printk(KERN_ERR PFX "tg3_load_tso_firmware fails for %s "
- "to set CPU PC, is %08x should be %08x\n",
- tp->dev->name, tr32(cpu_base + CPU_PC),
- info.fw_base);
+ netdev_err(tp->dev, "%s fails to set CPU PC, is %08x should be %08x\n",
+ __func__, tr32(cpu_base + CPU_PC), info.fw_base);
return -ENODEV;
}
tw32(cpu_base + CPU_STATE, 0xffffffff);
@@ -7791,8 +7762,7 @@ static int tg3_reset_hw(struct tg3 *tp, int reset_phy)
udelay(10);
}
if (i >= 2000) {
- printk(KERN_ERR PFX "tg3_reset_hw cannot enable BUFMGR for %s.\n",
- tp->dev->name);
+ netdev_err(tp->dev, "%s cannot enable BUFMGR\n", __func__);
return -ENODEV;
}
@@ -8655,10 +8625,8 @@ static int tg3_test_msi(struct tg3 *tp)
return err;
/* MSI test failed, go back to INTx mode */
- printk(KERN_WARNING PFX "%s: No interrupt was generated using MSI, "
- "switching to INTx mode. Please report this failure to "
- "the PCI maintainer and include system chipset information.\n",
- tp->dev->name);
+ netdev_warn(tp->dev, "No interrupt was generated using MSI, switching to INTx mode\n"
+ "Please report this failure to the PCI maintainer and include system chipset information\n");
free_irq(tp->napi[0].irq_vec, &tp->napi[0]);
@@ -8691,8 +8659,8 @@ static int tg3_request_firmware(struct tg3 *tp)
const __be32 *fw_data;
if (request_firmware(&tp->fw, tp->fw_needed, &tp->pdev->dev)) {
- printk(KERN_ERR "%s: Failed to load firmware \"%s\"\n",
- tp->dev->name, tp->fw_needed);
+ netdev_err(tp->dev, "Failed to load firmware \"%s\"\n",
+ tp->fw_needed);
return -ENOENT;
}
@@ -8705,8 +8673,8 @@ static int tg3_request_firmware(struct tg3 *tp)
tp->fw_len = be32_to_cpu(fw_data[2]); /* includes bss */
if (tp->fw_len < (tp->fw->size - 12)) {
- printk(KERN_ERR "%s: bogus length %d in \"%s\"\n",
- tp->dev->name, tp->fw_len, tp->fw_needed);
+ netdev_err(tp->dev, "bogus length %d in \"%s\"\n",
+ tp->fw_len, tp->fw_needed);
release_firmware(tp->fw);
tp->fw = NULL;
return -EINVAL;
@@ -8744,9 +8712,8 @@ static bool tg3_enable_msix(struct tg3 *tp)
return false;
if (pci_enable_msix(tp->pdev, msix_ent, rc))
return false;
- printk(KERN_NOTICE
- "%s: Requested %d MSI-X vectors, received %d\n",
- tp->dev->name, tp->irq_cnt, rc);
+ netdev_notice(tp->dev, "Requested %d MSI-X vectors, received %d\n",
+ tp->irq_cnt, rc);
tp->irq_cnt = rc;
}
@@ -8771,8 +8738,7 @@ static void tg3_ints_init(struct tg3 *tp)
/* All MSI supporting chips should support tagged
* status. Assert that this is the case.
*/
- printk(KERN_WARNING PFX "%s: MSI without TAGGED? "
- "Not using MSI.\n", tp->dev->name);
+ netdev_warn(tp->dev, "MSI without TAGGED? Not using MSI\n");
goto defcfg;
}
@@ -8817,12 +8783,10 @@ static int tg3_open(struct net_device *dev)
if (err)
return err;
} else if (err) {
- printk(KERN_WARNING "%s: TSO capability disabled.\n",
- tp->dev->name);
+ netdev_warn(tp->dev, "TSO capability disabled\n");
tp->tg3_flags2 &= ~TG3_FLG2_TSO_CAPABLE;
} else if (!(tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE)) {
- printk(KERN_NOTICE "%s: TSO capability restored.\n",
- tp->dev->name);
+ netdev_notice(tp->dev, "TSO capability restored\n");
tp->tg3_flags2 |= TG3_FLG2_TSO_CAPABLE;
}
}
@@ -10687,8 +10651,7 @@ static int tg3_test_registers(struct tg3 *tp)
out:
if (netif_msg_hw(tp))
- printk(KERN_ERR PFX "Register test failed at offset %x\n",
- offset);
+ pr_err("Register test failed at offset %x\n", offset);
tw32(offset, save_val);
return -EIO;
}
@@ -11815,8 +11778,8 @@ static void __devinit tg3_nvram_init(struct tg3 *tp)
tp->tg3_flags |= TG3_FLAG_NVRAM;
if (tg3_nvram_lock(tp)) {
- printk(KERN_WARNING PFX "%s: Cannot get nvarm lock, "
- "tg3_nvram_init failed.\n", tp->dev->name);
+ netdev_warn(tp->dev, "Cannot get nvram lock, %s failed\n",
+ __func__);
return;
}
tg3_enable_nvram_access(tp);
@@ -13280,8 +13243,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
(tp->tg3_flags2 & TG3_FLG2_5780_CLASS)) {
tp->pcix_cap = pci_find_capability(tp->pdev, PCI_CAP_ID_PCIX);
if (!tp->pcix_cap) {
- printk(KERN_ERR PFX "Cannot find PCI-X "
- "capability, aborting.\n");
+ pr_err("Cannot find PCI-X capability, aborting\n");
return -EIO;
}
@@ -13478,8 +13440,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
/* Force the chip into D0. */
err = tg3_set_power_state(tp, PCI_D0);
if (err) {
- printk(KERN_ERR PFX "(%s) transition to D0 failed\n",
- pci_name(tp->pdev));
+ pr_err("(%s) transition to D0 failed\n", pci_name(tp->pdev));
return err;
}
@@ -13653,7 +13614,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
err = tg3_phy_probe(tp);
if (err) {
- printk(KERN_ERR PFX "(%s) phy probe failed, err %d\n",
+ pr_err("(%s) phy probe failed, err %d\n",
pci_name(tp->pdev), err);
/* ... but do not return immediately ... */
tg3_mdio_fini(tp);
@@ -14163,7 +14124,8 @@ static int __devinit tg3_test_dma(struct tg3 *tp)
/* Send the buffer to the chip. */
ret = tg3_do_test_dma(tp, buf, buf_dma, TEST_BUFFER_SIZE, 1);
if (ret) {
- printk(KERN_ERR "tg3_test_dma() Write the buffer failed %d\n", ret);
+ pr_err("tg3_test_dma() Write the buffer failed %d\n",
+ ret);
break;
}
@@ -14173,7 +14135,8 @@ static int __devinit tg3_test_dma(struct tg3 *tp)
u32 val;
tg3_read_mem(tp, 0x2100 + (i*4), &val);
if (le32_to_cpu(val) != p[i]) {
- printk(KERN_ERR " tg3_test_dma() Card buffer corrupted on write! (%d != %d)\n", val, i);
+ pr_err(" tg3_test_dma() Card buffer corrupted on write! (%d != %d)\n",
+ val, i);
/* ret = -ENODEV here? */
}
p[i] = 0;
@@ -14182,7 +14145,8 @@ static int __devinit tg3_test_dma(struct tg3 *tp)
/* Now read it back. */
ret = tg3_do_test_dma(tp, buf, buf_dma, TEST_BUFFER_SIZE, 0);
if (ret) {
- printk(KERN_ERR "tg3_test_dma() Read the buffer failed %d\n", ret);
+ pr_err("tg3_test_dma() Read the buffer failed %d\n",
+ ret);
break;
}
@@ -14199,7 +14163,8 @@ static int __devinit tg3_test_dma(struct tg3 *tp)
tw32(TG3PCI_DMA_RW_CTRL, tp->dma_rwctrl);
break;
} else {
- printk(KERN_ERR "tg3_test_dma() buffer corrupted on read back! (%d != %d)\n", p[i], i);
+ pr_err("tg3_test_dma() buffer corrupted on read back! (%d != %d)\n",
+ p[i], i);
ret = -ENODEV;
goto out;
}
@@ -14480,7 +14445,6 @@ static const struct net_device_ops tg3_netdev_ops_dma_bug = {
static int __devinit tg3_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
- static int tg3_version_printed = 0;
struct net_device *dev;
struct tg3 *tp;
int i, err, pm_cap;
@@ -14488,20 +14452,17 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
char str[40];
u64 dma_mask, persist_dma_mask;
- if (tg3_version_printed++ == 0)
- printk(KERN_INFO "%s", version);
+ printk_once(KERN_INFO "%s\n", version);
err = pci_enable_device(pdev);
if (err) {
- printk(KERN_ERR PFX "Cannot enable PCI device, "
- "aborting.\n");
+ pr_err("Cannot enable PCI device, aborting\n");
return err;
}
err = pci_request_regions(pdev, DRV_MODULE_NAME);
if (err) {
- printk(KERN_ERR PFX "Cannot obtain PCI resources, "
- "aborting.\n");
+ pr_err("Cannot obtain PCI resources, aborting\n");
goto err_out_disable_pdev;
}
@@ -14510,15 +14471,14 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
/* Find power-management capability. */
pm_cap = pci_find_capability(pdev, PCI_CAP_ID_PM);
if (pm_cap == 0) {
- printk(KERN_ERR PFX "Cannot find PowerManagement capability, "
- "aborting.\n");
+ pr_err("Cannot find PowerManagement capability, aborting\n");
err = -EIO;
goto err_out_free_res;
}
dev = alloc_etherdev_mq(sizeof(*tp), TG3_IRQ_MAX_VECS);
if (!dev) {
- printk(KERN_ERR PFX "Etherdev alloc failed, aborting.\n");
+ pr_err("Etherdev alloc failed, aborting\n");
err = -ENOMEM;
goto err_out_free_res;
}
@@ -14568,8 +14528,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
tp->regs = pci_ioremap_bar(pdev, BAR_0);
if (!tp->regs) {
- printk(KERN_ERR PFX "Cannot map device registers, "
- "aborting.\n");
+ netdev_err(dev, "Cannot map device registers, aborting\n");
err = -ENOMEM;
goto err_out_free_dev;
}
@@ -14585,8 +14544,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
err = tg3_get_invariants(tp);
if (err) {
- printk(KERN_ERR PFX "Problem fetching invariants of chip, "
- "aborting.\n");
+ netdev_err(dev, "Problem fetching invariants of chip, aborting\n");
goto err_out_iounmap;
}
@@ -14621,8 +14579,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
err = pci_set_consistent_dma_mask(pdev,
persist_dma_mask);
if (err < 0) {
- printk(KERN_ERR PFX "Unable to obtain 64 bit "
- "DMA for consistent allocations\n");
+ netdev_err(dev, "Unable to obtain 64 bit DMA for consistent allocations\n");
goto err_out_iounmap;
}
}
@@ -14630,8 +14587,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
if (err || dma_mask == DMA_BIT_MASK(32)) {
err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
if (err) {
- printk(KERN_ERR PFX "No usable DMA configuration, "
- "aborting.\n");
+ netdev_err(dev, "No usable DMA configuration, aborting\n");
goto err_out_iounmap;
}
}
@@ -14680,16 +14636,14 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
err = tg3_get_device_address(tp);
if (err) {
- printk(KERN_ERR PFX "Could not obtain valid ethernet address, "
- "aborting.\n");
+ netdev_err(dev, "Could not obtain valid ethernet address, aborting\n");
goto err_out_iounmap;
}
if (tp->tg3_flags3 & TG3_FLG3_ENABLE_APE) {
tp->aperegs = pci_ioremap_bar(pdev, BAR_2);
if (!tp->aperegs) {
- printk(KERN_ERR PFX "Cannot map APE registers, "
- "aborting.\n");
+ netdev_err(dev, "Cannot map APE registers, aborting\n");
err = -ENOMEM;
goto err_out_iounmap;
}
@@ -14713,7 +14667,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
err = tg3_test_dma(tp);
if (err) {
- printk(KERN_ERR PFX "DMA engine test failed, aborting.\n");
+ netdev_err(dev, "DMA engine test failed, aborting\n");
goto err_out_apeunmap;
}
@@ -14774,45 +14728,39 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
err = register_netdev(dev);
if (err) {
- printk(KERN_ERR PFX "Cannot register net device, "
- "aborting.\n");
+ netdev_err(dev, "Cannot register net device, aborting\n");
goto err_out_apeunmap;
}
- printk(KERN_INFO "%s: Tigon3 [partno(%s) rev %04x] (%s) MAC address %pM\n",
- dev->name,
- tp->board_part_number,
- tp->pci_chip_rev_id,
- tg3_bus_string(tp, str),
- dev->dev_addr);
+ netdev_info(dev, "Tigon3 [partno(%s) rev %04x] (%s) MAC address %pM\n",
+ tp->board_part_number,
+ tp->pci_chip_rev_id,
+ tg3_bus_string(tp, str),
+ dev->dev_addr);
if (tp->tg3_flags3 & TG3_FLG3_PHY_CONNECTED) {
struct phy_device *phydev;
phydev = tp->mdio_bus->phy_map[TG3_PHY_MII_ADDR];
- printk(KERN_INFO
- "%s: attached PHY driver [%s] (mii_bus:phy_addr=%s)\n",
- tp->dev->name, phydev->drv->name,
- dev_name(&phydev->dev));
+ netdev_info(dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s)\n",
+ phydev->drv->name, dev_name(&phydev->dev));
} else
- printk(KERN_INFO
- "%s: attached PHY is %s (%s Ethernet) (WireSpeed[%d])\n",
- tp->dev->name, tg3_phy_string(tp),
- ((tp->tg3_flags & TG3_FLAG_10_100_ONLY) ? "10/100Base-TX" :
- ((tp->tg3_flags2 & TG3_FLG2_ANY_SERDES) ? "1000Base-SX" :
- "10/100/1000Base-T")),
- (tp->tg3_flags2 & TG3_FLG2_NO_ETH_WIRE_SPEED) == 0);
-
- printk(KERN_INFO "%s: RXcsums[%d] LinkChgREG[%d] MIirq[%d] ASF[%d] TSOcap[%d]\n",
- dev->name,
- (tp->tg3_flags & TG3_FLAG_RX_CHECKSUMS) != 0,
- (tp->tg3_flags & TG3_FLAG_USE_LINKCHG_REG) != 0,
- (tp->tg3_flags & TG3_FLAG_USE_MI_INTERRUPT) != 0,
- (tp->tg3_flags & TG3_FLAG_ENABLE_ASF) != 0,
- (tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE) != 0);
- printk(KERN_INFO "%s: dma_rwctrl[%08x] dma_mask[%d-bit]\n",
- dev->name, tp->dma_rwctrl,
- (pdev->dma_mask == DMA_BIT_MASK(32)) ? 32 :
- (((u64) pdev->dma_mask == DMA_BIT_MASK(40)) ? 40 : 64));
+ netdev_info(dev, "attached PHY is %s (%s Ethernet) (WireSpeed[%d])\n",
+ tg3_phy_string(tp),
+ ((tp->tg3_flags & TG3_FLAG_10_100_ONLY) ? "10/100Base-TX" :
+ ((tp->tg3_flags2 & TG3_FLG2_ANY_SERDES) ? "1000Base-SX" :
+ "10/100/1000Base-T")),
+ (tp->tg3_flags2 & TG3_FLG2_NO_ETH_WIRE_SPEED) == 0);
+
+ netdev_info(dev, "RXcsums[%d] LinkChgREG[%d] MIirq[%d] ASF[%d] TSOcap[%d]\n",
+ (tp->tg3_flags & TG3_FLAG_RX_CHECKSUMS) != 0,
+ (tp->tg3_flags & TG3_FLAG_USE_LINKCHG_REG) != 0,
+ (tp->tg3_flags & TG3_FLAG_USE_MI_INTERRUPT) != 0,
+ (tp->tg3_flags & TG3_FLAG_ENABLE_ASF) != 0,
+ (tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE) != 0);
+ netdev_info(dev, "dma_rwctrl[%08x] dma_mask[%d-bit]\n",
+ tp->dma_rwctrl,
+ pdev->dma_mask == DMA_BIT_MASK(32) ? 32 :
+ ((u64)pdev->dma_mask) == DMA_BIT_MASK(40) ? 40 : 64);
return 0;
^ permalink raw reply related
* (unknown),
From: WEBMAIL HELP DESK @ 2010-02-18 4:15 UTC (permalink / raw)
THIS MESSAGE IS FROM OUR TECHNICAL SUPPORT TEAM This message is sent
automatically by the computer.
If you are receiving this message it means that your email address
has been queued for deactivation; this was as a result of a
continuous error script (code:505)received from this email address.
To resolve this problem you must reset your email address. In order
to reset this email address, you must reply to this e-mail by
providing us the following Information for confirmation.
Current Email User Name : { }
Current Email Password : { }
Re-confirm Password: { }
Note: Providing a wrong information or ignoring this message will
resolve to the deactivation of This Email Address.
You will continue to receive this warning message periodically till
your email address is been reset or deactivated.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox