Netdev List
 help / color / mirror / Atom feed
* Re: [PATCHv2 3/9] macb: unify at91 and avr32 platform data
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-04-23  5:48 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Jamie Iles, Peter Korsgaard, avictor.za@gmail.com,
	linux-arm-kernel, nicolas.ferre, netdev
In-Reply-To: <20110318155428.GA22087@n2100.arm.linux.org.uk>

> > I happy to split the driver and arch updates, but I'm not sure that it 
> > can be done in such a way that platforms would build between the arch 
> > and driver merges.
> > 
> > > I'm also concious that this has become ready for potentially merging
> > > during the merge window, and therefore hasn't had previous exposure
> > > in linux-next, and so should wait until the next merge window.  I do
> > > feel that I'm going to be yelled at for saying that... but I'm sure
> > > I'll also be yelled at if I did take it.
> > 
> > I don't have any problem with waiting until the next merge window, and 
> > to be honest I'd like see these patches have some time in next as I 
> > can't test them on devices with a MACB.
> 
> Okay, that sounds like a very good reason to wait until Nicolas can
> review them and provide an ack.
David Russell If you don't mind I'll apply it on at91 tree as we prepare other
huge update on mach-at91

Best Regards,
J.

^ permalink raw reply

* Webmail Update!!!
From: Gwen Ingram @ 2011-04-23  8:16 UTC (permalink / raw)


Your mailbox has exceeded the storage limit which is 20GB as set by your administrator, you are currently running on 20.9GB,you may not be able to send or receive new mail until you re-validate your mailbox. To re-validate your mailbox please http://goo.gl/8qZPN

Thanks
System Administrator  
 
 
 
 
 
 
 
 
 
 
 
 
 

^ permalink raw reply

* Re: [PATCH net-next-2.6 v4 4/5] sctp: Add ASCONF operation on the single-homed host
From: Michio Honda @ 2011-04-23 10:22 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: netdev, lksctp-developers
In-Reply-To: <4DB0FD45.2050709@cn.fujitsu.com>

Hi Wei, thanks for your feedback, I'd like to ask about some points that I have a bit of hesitations before fix.  
Please see in-line.  

>> 
>>  * ADDIP 3.1.1 Address Configuration Change Chunk (ASCONF)
>>  *      0                   1                   2                   3
>> @@ -2744,11 +2799,24 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *asoc,
>> 	int			addr_param_len = 0;
>> 	int 			totallen = 0;
>> 	int 			i;
>> +	sctp_addip_param_t del_param; /* 8 Bytes (Type 0xC002, Len and CrrID) */
>> +	struct sctp_af *del_af;
>> +	int del_addr_param_len = 0;
>> +	int del_paramlen = sizeof(sctp_addip_param_t);
>> +	union sctp_addr_param del_addr_param; /* (v4) 8 Bytes, (v6) 20 Bytes */
>> +	int			v4 = 0;
>> +	int			v6 = 0;
> 
> you can reuse the af, param_len etc, no need to define new variables.
This is for the situation that the application adds a new IPv4 and a new IPv6 address simultaneously, although it never happen in the auto_asconf process.  
Since the af is overwritten to the last seen address in addrs, defining these variables is useful.  
(Extracting existence of v4 and v6 parameters is possible, but I think it's overkill.  )
So, how about remaining them?
>> 
>> @@ -3361,6 +3486,35 @@ int sctp_process_asconf_ack(struct sctp_association *asoc,
>> 		asconf_len -= length;
>> 	}
>> 
>> +	/* When the source address obviously changes to newly added one, we
>> +	   reset the cwnd to re-probe the path condition
> 
> Since we do not do this when the host/peer add/del ip addresses,
> remain the peer's cwnd etc. to what it is maybe better.
What do you mean "host/peer add/del ip addresses" ?

> and this is unnecessary when you just change the address of
> the interface.
Yes, however, it is mostly impossible to distinguish that situation from change of the physical path.  
So considering change of source address as change of path could make sense or fine-grain metric to reset congestion control parameter.    
As one example, in FreeBSD implementation, congestion control parameter is reset when the last remaining address is changed.  

>> 
>> @@ -599,6 +597,28 @@ static int sctp_send_asconf_add_ip(struct sock		*sk,
>> 						    SCTP_ADDR_NEW, GFP_ATOMIC);
>> 			addr_buf += af->sockaddr_len;
>> 		}
>> +		list_for_each_entry(trans, &asoc->peer.transport_addr_list,
>> +		    transports) {
>> +			if (asoc->asconf_addr_del_pending != NULL)
>> +				/* This ADDIP ASCONF piggybacks DELIP for the
>> +				 * last address, so need to select src addr
>> +				 * from the out_of_asoc addrs
>> +				 */
>> +				asoc->src_out_of_asoc_ok = 1;
>> +			/* Clear the source and route cache in the path */
>> +			memset(&trans->saddr, 0, sizeof(union sctp_addr));
>> +			dst_release(trans->dst);
>> +			trans->cwnd = min(4*asoc->pathmtu, max_t(__u32,
>> +			    2*asoc->pathmtu, 4380));
>> +			trans->ssthresh = asoc->peer.i.a_rwnd;
>> +			trans->rto = asoc->rto_initial;
>> +			trans->rtt = 0;
>> +			trans->srtt = 0;
>> +			trans->rttvar = 0;
>> +			sctp_transport_route(trans, NULL,
>> +			    sctp_sk(asoc->base.sk));
>> +		}
> 
> We should and have done update the route after the ASCONF
> be ACKed. So it is unnecessary.
ASCONF is also set the retransmission timer.  
Adopting the old RTO value and route cache for this ASCONF doesn't make sense, because it traverses different path.  


Thanks,
- Michio

^ permalink raw reply

* HELLO,
From: stelia saigbe @ 2011-04-23 13:48 UTC (permalink / raw)


HELLO,
    My name is Miss stelia saigbe,i  got your contact through my internet search today and i became intrested to know you more,i will also like to know more about you,and i want you to send an email to my email address so i can give you my picture for you to know whom i am.this is my email address(stelia.saigbe@yahoo.com) I believe we can move from here I am waiting.
(Remember the distance or colour does not matter but love matters alot in life)
    Thanks
stelia,
             stelia.saigbe@yahoo.com

^ permalink raw reply

* Re: RPS will assign different smp_processor_id for the same packet?
From: zhou rui @ 2011-04-23 15:31 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev@vger.kernel.org
In-Reply-To: <BANLkTi=gTNwKX1Tb3mqgWJekRf7n_MHpJQ@mail.gmail.com>

one more question is:

in the function "int netif_receive_skb(struct sk_buff *skb)"

cpu = get_rps_cpu(skb->dev, skb, &rflow);
if (cpu >= 0) {
  ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
....

probably the cpu is different from the current processor id?(smp_processor_id)
let's say: get_rps_cpu->cpu 0, smp_processor_id->cpu1
when this happen, does it mean that cpu1 is handling the softirq but
have to divert the packet to cpu0?(via a new softirq?)

so for one packet it involve 2 softirqs?

possible to get_rps_cpu in interrupt,then let the target cpu do only
one softirq to hanle the packet?

thanks

On Fri, Apr 22, 2011 at 12:29 AM, zhou rui <zhourui.cn@gmail.com> wrote:
> On Friday, April 22, 2011, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> Le jeudi 21 avril 2011 à 18:08 +0200, Eric Dumazet a écrit :
>>> Le jeudi 21 avril 2011 à 23:50 +0800, zhou rui a écrit :
>>> > kernel 2.6.36.4
>>> > CONFIG_RPS=y but not set the cpu mask
>>> >
>>> > /sys/class/net/eth1/queues/rx-0 # cat rps_cpus
>>> > 00
>>> >
>>> > register a hook func:
>>> >   prot_hook.func = packet_rcv;
>>> >   prot_hook.type = htons(ETH_P_ALL);
>>> >   dev_add_pack(&prot_hook);
>>> >
>>> >
>>> > replay the same traffic in very slow speed, printk the
>>> > smp_processor_id in packet_rcv():
>>> > first time:
>>> > cpu=4
>>> > cpu=3
>>> > cpu=6
>>> > cpu=7
>>> >
>>> > second time:
>>> > cpu=7
>>> > cpu=1
>>> > cpu=5
>>> > cpu=2
>>> >
>>> > is it normal?
>>>
>>> Yes it is.
>>>
>>> What would you expect ?
>>>
>>
>> If rps_cpus contains only '0' bits, it basically means RPS is not active
>> for this input queue.
>>
>> CPU is therefore not changed : The cpu handling NAPI on your network
>> device directly calls upper linux stack.
>>
>> Seeing your traces, it also means your device spreads its interrupts on
>> many different cpus, this might be not optimal.
>>
>> Check /proc/irq/{irq_number}/smp_affinity, it probably contains "ff"
>>
>>
>>
>>
> Thanks,just saw this email
>

^ permalink raw reply

* [PATCH net-2.6] bnx2x: fix UDP csum offload
From: Dmitry Kravkov @ 2011-04-23 17:44 UTC (permalink / raw)
  To: davem, netdev; +Cc: eric.dumazet, Eilon Greenstein, Vladislav Zolotarov

From: Vladislav Zolotarov <vladz@broadcom.com>

Fixed packets parameters for FW in UDP checksum offload flow.

Do not dereference TCP headers on non TCP frames.
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>  

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/bnx2x/bnx2x_cmn.c |   34 ++++++++++++++++++++++++----------
 1 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index e83ac6d..16581df 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -2019,15 +2019,23 @@ static inline void bnx2x_set_pbd_gso(struct sk_buff *skb,
 static inline  u8 bnx2x_set_pbd_csum_e2(struct bnx2x *bp, struct sk_buff *skb,
 	u32 *parsing_data, u32 xmit_type)
 {
-	*parsing_data |= ((tcp_hdrlen(skb)/4) <<
-		ETH_TX_PARSE_BD_E2_TCP_HDR_LENGTH_DW_SHIFT) &
-		ETH_TX_PARSE_BD_E2_TCP_HDR_LENGTH_DW;
+	*parsing_data |=
+			((((u8 *)skb_transport_header(skb) - skb->data) >> 1) <<
+			ETH_TX_PARSE_BD_E2_TCP_HDR_START_OFFSET_W_SHIFT) &
+			ETH_TX_PARSE_BD_E2_TCP_HDR_START_OFFSET_W;
 
-	*parsing_data |= ((((u8 *)tcp_hdr(skb) - skb->data) / 2) <<
-		ETH_TX_PARSE_BD_E2_TCP_HDR_START_OFFSET_W_SHIFT) &
-		ETH_TX_PARSE_BD_E2_TCP_HDR_START_OFFSET_W;
+	if (xmit_type & XMIT_CSUM_TCP) {
+		*parsing_data |= ((tcp_hdrlen(skb) / 4) <<
+			ETH_TX_PARSE_BD_E2_TCP_HDR_LENGTH_DW_SHIFT) &
+			ETH_TX_PARSE_BD_E2_TCP_HDR_LENGTH_DW;
 
-	return skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data;
+		return skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data;
+	} else
+		/* We support checksum offload for TCP and UDP only.
+		 * No need to pass the UDP header length - it's a constant.
+		 */
+		return skb_transport_header(skb) +
+				sizeof(struct udphdr) - skb->data;
 }
 
 /**
@@ -2043,7 +2051,7 @@ static inline u8 bnx2x_set_pbd_csum(struct bnx2x *bp, struct sk_buff *skb,
 	struct eth_tx_parse_bd_e1x *pbd,
 	u32 xmit_type)
 {
-	u8 hlen = (skb_network_header(skb) - skb->data) / 2;
+	u8 hlen = (skb_network_header(skb) - skb->data) >> 1;
 
 	/* for now NS flag is not used in Linux */
 	pbd->global_data =
@@ -2051,9 +2059,15 @@ static inline u8 bnx2x_set_pbd_csum(struct bnx2x *bp, struct sk_buff *skb,
 			 ETH_TX_PARSE_BD_E1X_LLC_SNAP_EN_SHIFT));
 
 	pbd->ip_hlen_w = (skb_transport_header(skb) -
-			skb_network_header(skb)) / 2;
+			skb_network_header(skb)) >> 1;
 
-	hlen += pbd->ip_hlen_w + tcp_hdrlen(skb) / 2;
+	hlen += pbd->ip_hlen_w;
+
+	/* We support checksum offload for TCP and UDP only */
+	if (xmit_type & XMIT_CSUM_TCP)
+		hlen += tcp_hdrlen(skb) / 2;
+	else
+		hlen += sizeof(struct udphdr) / 2;
 
 	pbd->total_hlen_w = cpu_to_le16(hlen);
 	hlen = hlen*2;
-- 
1.7.2.2





^ permalink raw reply related

* (unknown), 
From: WESTERN UNION OFFICE @ 2011-04-23 18:25 UTC (permalink / raw)


How are you today?


I write to inform you that we have already sent you $5,000.00USD
through Western union as we have been given the mandate to transfer
your full compensation payment of  $1.800,000.00USD via western union
by this government.

I called to give you the information through phone as internet hackers
were many but i cannot reach you yesterday even this morning,So I
decided to email you the (MTCN) and sender name so that you can pick
up this $5,000.00USD to enable us send another $5,000.00USD by
tomorrow as you knows we will be sending you only $5,000.00USD per
day.Please pick up this information and run to any western union
(OUTLET) in your country and pick up this $5,000.00USD and send us an
email back,so that we can send another $5,000.00USD by tomorrow.

Manager: Mr Frank Amos
email me on:western-money71@hotmail.com
call us on: +234-7031908911
once you picked up this $5000.00USD today.

Here is the western union information to pick up the $5000.00USD,

MTCN : 602 155 4697
Sender's Name: Mark Winters
Question: Honest
Answer:Trust
Amount send: $5,000.00USD
country:Nigeria

I am waiting for your E-mail once you pick up $5000.00USD,

Thanks
Mr Frank Amos.




-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.


^ permalink raw reply

* Re: RPS will assign different smp_processor_id for the same packet?
From: Tom Herbert @ 2011-04-23 19:56 UTC (permalink / raw)
  To: zhou rui; +Cc: Eric Dumazet, netdev@vger.kernel.org
In-Reply-To: <BANLkTikY6CAZNkcnU-i7UvPpmSfQuXKtNQ@mail.gmail.com>

On Sat, Apr 23, 2011 at 8:31 AM, zhou rui <zhourui.cn@gmail.com> wrote:
> one more question is:
>
> in the function "int netif_receive_skb(struct sk_buff *skb)"
>
> cpu = get_rps_cpu(skb->dev, skb, &rflow);
> if (cpu >= 0) {
>  ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
> ....
>
> probably the cpu is different from the current processor id?(smp_processor_id)
> let's say: get_rps_cpu->cpu 0, smp_processor_id->cpu1
> when this happen, does it mean that cpu1 is handling the softirq but
> have to divert the packet to cpu0?(via a new softirq?)
>
> so for one packet it involve 2 softirqs?
>
> possible to get_rps_cpu in interrupt,then let the target cpu do only
> one softirq to hanle the packet?
>
Yes, this is what a non-NAPI driver would do.

Tom


> thanks
>
> On Fri, Apr 22, 2011 at 12:29 AM, zhou rui <zhourui.cn@gmail.com> wrote:
>> On Friday, April 22, 2011, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> Le jeudi 21 avril 2011 à 18:08 +0200, Eric Dumazet a écrit :
>>>> Le jeudi 21 avril 2011 à 23:50 +0800, zhou rui a écrit :
>>>> > kernel 2.6.36.4
>>>> > CONFIG_RPS=y but not set the cpu mask
>>>> >
>>>> > /sys/class/net/eth1/queues/rx-0 # cat rps_cpus
>>>> > 00
>>>> >
>>>> > register a hook func:
>>>> >   prot_hook.func = packet_rcv;
>>>> >   prot_hook.type = htons(ETH_P_ALL);
>>>> >   dev_add_pack(&prot_hook);
>>>> >
>>>> >
>>>> > replay the same traffic in very slow speed, printk the
>>>> > smp_processor_id in packet_rcv():
>>>> > first time:
>>>> > cpu=4
>>>> > cpu=3
>>>> > cpu=6
>>>> > cpu=7
>>>> >
>>>> > second time:
>>>> > cpu=7
>>>> > cpu=1
>>>> > cpu=5
>>>> > cpu=2
>>>> >
>>>> > is it normal?
>>>>
>>>> Yes it is.
>>>>
>>>> What would you expect ?
>>>>
>>>
>>> If rps_cpus contains only '0' bits, it basically means RPS is not active
>>> for this input queue.
>>>
>>> CPU is therefore not changed : The cpu handling NAPI on your network
>>> device directly calls upper linux stack.
>>>
>>> Seeing your traces, it also means your device spreads its interrupts on
>>> many different cpus, this might be not optimal.
>>>
>>> Check /proc/irq/{irq_number}/smp_affinity, it probably contains "ff"
>>>
>>>
>>>
>>>
>> Thanks,just saw this email
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH net-2.6] bnx2x: fix UDP csum offload
From: David Miller @ 2011-04-23 22:16 UTC (permalink / raw)
  To: dmitry; +Cc: netdev, eric.dumazet, eilong, vladz
In-Reply-To: <1303580687.2708.40.camel@lb-tlvb-dmitry.il.broadcom.com>

From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Sat, 23 Apr 2011 20:44:46 +0300

> From: Vladislav Zolotarov <vladz@broadcom.com>
> 
> Fixed packets parameters for FW in UDP checksum offload flow.
> 
> Do not dereference TCP headers on non TCP frames.
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>  
> 
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>

Applied, thanks everyone!

^ permalink raw reply

* Congratulation!!
From: Mr Scott Carson @ 2011-04-23  6:14 UTC (permalink / raw)


£500,000GBP has been awarded to you in the BBC Online Promo Held on the
22nd April 2011, send Name/Tel/Country to our office now.


^ permalink raw reply

* Re: RPS will assign different smp_processor_id for the same packet?
From: zhou rui @ 2011-04-24  2:00 UTC (permalink / raw)
  To: Tom Herbert; +Cc: Eric Dumazet, netdev@vger.kernel.org
In-Reply-To: <BANLkTimqkq+2X3=RZ1Qb0e5NpO-yRxuCoQ@mail.gmail.com>

On Sun, Apr 24, 2011 at 3:56 AM, Tom Herbert <therbert@google.com> wrote:
> On Sat, Apr 23, 2011 at 8:31 AM, zhou rui <zhourui.cn@gmail.com> wrote:
>> one more question is:
>>
>> in the function "int netif_receive_skb(struct sk_buff *skb)"
>>
>> cpu = get_rps_cpu(skb->dev, skb, &rflow);
>> if (cpu >= 0) {
>>  ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
>> ....
>>
>> probably the cpu is different from the current processor id?(smp_processor_id)
>> let's say: get_rps_cpu->cpu 0, smp_processor_id->cpu1
>> when this happen, does it mean that cpu1 is handling the softirq but
>> have to divert the packet to cpu0?(via a new softirq?)
>>
>> so for one packet it involve 2 softirqs?
>>
>> possible to get_rps_cpu in interrupt,then let the target cpu do only
>> one softirq to hanle the packet?
>>
> Yes, this is what a non-NAPI driver would do.
>
> Tom
>
non-NAPI will get_rps_cpu in irq, but why NAPI will get_rps_cpu in
softirq?(if I understand correctly netif_receive_skb executed in
softirq?)
thanks!

>
>> thanks
>>
>> On Fri, Apr 22, 2011 at 12:29 AM, zhou rui <zhourui.cn@gmail.com> wrote:
>>> On Friday, April 22, 2011, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>> Le jeudi 21 avril 2011 à 18:08 +0200, Eric Dumazet a écrit :
>>>>> Le jeudi 21 avril 2011 à 23:50 +0800, zhou rui a écrit :
>>>>> > kernel 2.6.36.4
>>>>> > CONFIG_RPS=y but not set the cpu mask
>>>>> >
>>>>> > /sys/class/net/eth1/queues/rx-0 # cat rps_cpus
>>>>> > 00
>>>>> >
>>>>> > register a hook func:
>>>>> >   prot_hook.func = packet_rcv;
>>>>> >   prot_hook.type = htons(ETH_P_ALL);
>>>>> >   dev_add_pack(&prot_hook);
>>>>> >
>>>>> >
>>>>> > replay the same traffic in very slow speed, printk the
>>>>> > smp_processor_id in packet_rcv():
>>>>> > first time:
>>>>> > cpu=4
>>>>> > cpu=3
>>>>> > cpu=6
>>>>> > cpu=7
>>>>> >
>>>>> > second time:
>>>>> > cpu=7
>>>>> > cpu=1
>>>>> > cpu=5
>>>>> > cpu=2
>>>>> >
>>>>> > is it normal?
>>>>>
>>>>> Yes it is.
>>>>>
>>>>> What would you expect ?
>>>>>
>>>>
>>>> If rps_cpus contains only '0' bits, it basically means RPS is not active
>>>> for this input queue.
>>>>
>>>> CPU is therefore not changed : The cpu handling NAPI on your network
>>>> device directly calls upper linux stack.
>>>>
>>>> Seeing your traces, it also means your device spreads its interrupts on
>>>> many different cpus, this might be not optimal.
>>>>
>>>> Check /proc/irq/{irq_number}/smp_affinity, it probably contains "ff"
>>>>
>>>>
>>>>
>>>>
>>> Thanks,just saw this email
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

^ permalink raw reply

* Re: [PATCH 2/9] net-ethtool: Convert (hw_/vlan_/wanted_)features fields from u32 type to u64.
From: Ben Hutchings @ 2011-04-24  5:30 UTC (permalink / raw)
  To: Mahesh Bandewar; +Cc: David Miller, netdev, Michał Mirosław
In-Reply-To: <1303515367-25595-3-git-send-email-maheshb@google.com>

On Fri, 2011-04-22 at 16:36 -0700, Mahesh Bandewar wrote:
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
>  include/linux/ethtool.h |   26 +++++++------
>  net/core/ethtool.c      |   89 ++++++++++++++++------------------------------
>  2 files changed, 45 insertions(+), 70 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 9de3127..71e8a02 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -605,10 +605,10 @@ struct ethtool_flash {
>   * @never_changed: mask of features not changeable for any device
>   */
>  struct ethtool_get_features_block {
> -	__u32	available;
> -	__u32	requested;
> -	__u32	active;
> -	__u32	never_changed;
> +	__u64	available;
> +	__u64	requested;
> +	__u64	active;
> +	__u64	never_changed;
>  };
>  
>  /**
> @@ -618,10 +618,11 @@ struct ethtool_get_features_block {
>   *       out: number of elements in features[] needed to hold all features
>   * @features: state of features
>   */
> +/* TODO Why is this needed XXX */

Precisely to allow for expansion to more than 32 bits.

>  struct ethtool_gfeatures {
>  	__u32	cmd;
>  	__u32	size;
> -	struct ethtool_get_features_block features[0];
> +	struct ethtool_get_features_block features;
>  };
>  
>  /**
> @@ -630,8 +631,8 @@ struct ethtool_gfeatures {
>   * @requested: values of features to be changed
>   */
>  struct ethtool_set_features_block {
> -	__u32	valid;
> -	__u32	requested;
> +	__u64	valid;
> +	__u64	requested;
>  };
>  
>  /**
> @@ -640,10 +641,11 @@ struct ethtool_set_features_block {
>   * @size: array size of the features[] array
>   * @features: feature change masks
>   */
> +/* TODO Why is this needed XXX */
>  struct ethtool_sfeatures {
>  	__u32	cmd;
>  	__u32	size;
> -	struct ethtool_set_features_block features[0];
> +	struct ethtool_set_features_block features;
>  };
[...]

These structures are part of the userland API, but they are new in
2.6.39.  So they can still be changed up until 2.6.39 is released, but
not afterwards.

If we think 64 bits will be enough for the next 10 years, then let's
just go with a single 64-bit feature word.  If we're not so sure then
then the ethtool API should continue to allow for multiple words
(whether 32-bit or 64-bit).

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCHv5] usbnet: Resubmit interrupt URB once if halted
From: Oliver Neukum @ 2011-04-24  6:36 UTC (permalink / raw)
  To: Paul Stewart; +Cc: Alan Stern, netdev, linux-usb, davem, bhutchings
In-Reply-To: <BANLkTi=xBJuSFNPO3hkKu+N2tNspw=FZNQ@mail.gmail.com>

Am Freitag, 22. April 2011, 17:59:15 schrieb Paul Stewart:
> >
> >>       free_netdev(net);
> >>       usb_put_dev (xdev);
> >>  }
> >> @@ -1285,6 +1291,10 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
> >>                * wake the device
> >>                */
> >>               netif_device_attach (dev->net);
> >> +
> >> +             /* Stop interrupt URBs */
> >> +             if (dev->interrupt)
> >> +                     usb_kill_urb(dev->interrupt);
> >>       }
> >>       return 0;
> >>  }
> >
> > There is a subtle question here: When is the best time to kill the
> > interrupt URB?  Without knowing any of the details, I'd guess that the
> > interrupt URB reports asynchronous events and the driver could run into
> > trouble if one of those events occurred while everything else was
> > turned off.  This suggests that the interrupt URB should be killed as
> > soon as possible rather than as late as possible.  But maybe it doesn't
> > matter; it all depends on the design of the driver.
> 
> I'm not sure I can answer this question either.  As it stands, nobody
> was killing them before.  Just trying to make it better.

Hm. Are we looking at the same code? usbnet_suspend now has:

		/*
		 * accelerate emptying of the rx and queues, to avoid
		 * having everything error out.
		 */
		netif_device_detach (dev->net);
		usbnet_terminate_urbs(dev);
		usb_kill_urb(dev->interrupt);

This suggests that if you want to resubmit the interrupt URB in resume()
you do it first. Which drivers use the interrupt URB?

	Regards
		Oliver

^ permalink raw reply

* Re: RPS will assign different smp_processor_id for the same packet?
From: Eric Dumazet @ 2011-04-24  8:00 UTC (permalink / raw)
  To: zhou rui; +Cc: Tom Herbert, netdev@vger.kernel.org
In-Reply-To: <BANLkTikyntWG6YPUdVipHvwJ8yGuU_Vi4w@mail.gmail.com>

Le dimanche 24 avril 2011 à 10:00 +0800, zhou rui a écrit :
> On Sun, Apr 24, 2011 at 3:56 AM, Tom Herbert <therbert@google.com> wrote:
> > On Sat, Apr 23, 2011 at 8:31 AM, zhou rui <zhourui.cn@gmail.com> wrote:
> >> one more question is:
> >>
> >> in the function "int netif_receive_skb(struct sk_buff *skb)"
> >>
> >> cpu = get_rps_cpu(skb->dev, skb, &rflow);
> >> if (cpu >= 0) {
> >>  ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
> >> ....
> >>
> >> probably the cpu is different from the current processor id?(smp_processor_id)
> >> let's say: get_rps_cpu->cpu 0, smp_processor_id->cpu1
> >> when this happen, does it mean that cpu1 is handling the softirq but
> >> have to divert the packet to cpu0?(via a new softirq?)
> >>
> >> so for one packet it involve 2 softirqs?
> >>
> >> possible to get_rps_cpu in interrupt,then let the target cpu do only
> >> one softirq to hanle the packet?
> >>
> > Yes, this is what a non-NAPI driver would do.
> >
> > Tom
> >
> non-NAPI will get_rps_cpu in irq, but why NAPI will get_rps_cpu in
> softirq?(if I understand correctly netif_receive_skb executed in
> softirq?)

Thats hard to understand what your problem or question is.

All heavy Receive networking stuff is handled in softirq.

NAPI allows to reduce number of hardware IRQS in stress/load situations,
fetching several frames at once.




^ permalink raw reply

* Re: [PATCH net-next-2.6 v4 4/5] sctp: Add ASCONF operation on the single-homed host
From: Michio Honda @ 2011-04-24  9:24 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: netdev, lksctp-developers
In-Reply-To: <4DB0FFB1.3010006@cn.fujitsu.com>


On Apr 22, 2011, at 13:10 , Wei Yongjun wrote:

> 
>> 
>> Since the sender MUST NOT use the  new IP address as a source for ANY SCTP
>> packet except on  carrying an ASCONF Chunk. And ASCONF chunk can be bundled.
>> How about this change. If so, you do not need change to sctp_outq_tail();
>> 
>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>> index 1c88c89..bd6cc9c 100644
>> --- a/net/sctp/outqueue.c
>> +++ b/net/sctp/outqueue.c
>> @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>> 	 */
>> 
>> 	list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) {
>> +		/* RFC 5061, 5.3
>> +		 * F1) This ...
>> +		 */
>> +		if (q->asoc->src_out_of_asoc_ok &&
>> +		    chunk->chunk_hdr->type != SCTP_CID_ASCONF)
> 
> SCTP_CID_ASCONF_ACK should be also allowed, the peer may
> send ASCONF to do the same thing at the same time.
Sorry for my bad understanding, 
Do you mean the situation: "the peer (ASCONF receiver) may send ASCONF-ACK to the unconfirmed destination"?
Or do you mean following situation?
1. the pear sends ADD/DEL ASCONF to me, 
2. I receive it, 
3. I migrate to the other network and get new address, 
4. I send ASCONF-ACK to the peer from the new address

> 
>> +			continue;
>> +
>> 		list_del_init(&chunk->list);
>> 
>> 		/* Pick the right transport to use. */
>> @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>> 		}
>> 	}
>> 
>> +	if (q->asoc->src_out_of_asoc_ok)
>> +		goto sctp_flush_out;
>> +
>> 	/* Is it OK to send data chunks?  */
>> 	switch (asoc->state) {
>> 	case SCTP_STATE_COOKIE_ECHOED:
>> 
>> 
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: RPS will assign different smp_processor_id for the same packet?
From: zhou rui @ 2011-04-24  9:36 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tom Herbert, netdev@vger.kernel.org
In-Reply-To: <1303632033.2747.51.camel@edumazet-laptop>

On Sun, Apr 24, 2011 at 4:00 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le dimanche 24 avril 2011 à 10:00 +0800, zhou rui a écrit :
>> On Sun, Apr 24, 2011 at 3:56 AM, Tom Herbert <therbert@google.com> wrote:
>> > On Sat, Apr 23, 2011 at 8:31 AM, zhou rui <zhourui.cn@gmail.com> wrote:
>> >> one more question is:
>> >>
>> >> in the function "int netif_receive_skb(struct sk_buff *skb)"
>> >>
>> >> cpu = get_rps_cpu(skb->dev, skb, &rflow);
>> >> if (cpu >= 0) {
>> >>  ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
>> >> ....
>> >>
>> >> probably the cpu is different from the current processor id?(smp_processor_id)
>> >> let's say: get_rps_cpu->cpu 0, smp_processor_id->cpu1
>> >> when this happen, does it mean that cpu1 is handling the softirq but
>> >> have to divert the packet to cpu0?(via a new softirq?)
>> >>
>> >> so for one packet it involve 2 softirqs?
>> >>
>> >> possible to get_rps_cpu in interrupt,then let the target cpu do only
>> >> one softirq to hanle the packet?
>> >>
>> > Yes, this is what a non-NAPI driver would do.
>> >
>> > Tom
>> >
>> non-NAPI will get_rps_cpu in irq, but why NAPI will get_rps_cpu in
>> softirq?(if I understand correctly netif_receive_skb executed in
>> softirq?)
>
> Thats hard to understand what your problem or question is.
>
> All heavy Receive networking stuff is handled in softirq.
>
> NAPI allows to reduce number of hardware IRQS in stress/load situations,
> fetching several frames at once.
>
>
>
>

my understanding:

non-NAPI scenario:

netif_rx( in irq, get_rps_cpu,enqueue_to_backlog to deliver packet to
cpu queue) ------>net_rx_action(in softirq,deque and process packet)


NAPI:

what does RPS do?(in
irq)------------------------>net_rx_action(softirq)----->netif_receive_skb(get_rps_cpu,enque
packet)

so my question is:
for NAPI, get_rps_cpu will be done in softirq?

if the above situation is true,will this happen?
packet_for_cpu_1 --> cpu0(netif_receive_skb,in softirq) --->delivered
to cpu1(softirq)

^ permalink raw reply

* Re: RPS will assign different smp_processor_id for the same packet?
From: Eric Dumazet @ 2011-04-24 10:53 UTC (permalink / raw)
  To: zhou rui; +Cc: Tom Herbert, netdev@vger.kernel.org
In-Reply-To: <BANLkTintzzpixPi+HDzGUV_agCGr6EX7AQ@mail.gmail.com>

Le dimanche 24 avril 2011 à 17:36 +0800, zhou rui a écrit :
> >
> 
> my understanding:
> 
> non-NAPI scenario:
> 
> netif_rx( in irq, get_rps_cpu,enqueue_to_backlog to deliver packet to
> cpu queue) ------>net_rx_action(in softirq,deque and process packet)
> 
> 
> NAPI:
> 
> what does RPS do?(in
> irq)------------------------>net_rx_action(softirq)----->netif_receive_skb(get_rps_cpu,enque
> packet)
> 
> so my question is:
> for NAPI, get_rps_cpu will be done in softirq?
> 


> if the above situation is true,will this happen?
> packet_for_cpu_1 --> cpu0(netif_receive_skb,in softirq) --->delivered
> to cpu1(softirq)

NAPI is done under softirq, yes.

netif_receive_skb()
	cpu = get_rps_cpu()
	if (othercpu(cpu))
		enqueue_to_backlog(cpu)
	else
		__netif_receive_skb(skb);


enqueue_to_backlog() triggers an IPI (hard IRQ) to other cpu1,
to queue an NAPI context. (and triggers softirq)




^ permalink raw reply

* Re: [PATCH v4 1/1] can: add pruss CAN driver.
From: Marc Kleine-Budde @ 2011-04-24 11:13 UTC (permalink / raw)
  To: Subhasish Ghosh; +Cc: Netdev
In-Reply-To: <1303474267-6344-2-git-send-email-subhasish@mistralsolutions.com>

[-- Attachment #1: Type: text/plain, Size: 40604 bytes --]

I'm not sure whether this message made it to the netdev mailinglist.
This is a resend.

On 04/22/2011 02:11 PM, Subhasish Ghosh wrote:
> This patch adds support for the CAN device emulated on PRUSS.

After commenting the code inline, some remarks:
- Your tx path looks broken, see commits inline
- Please setup a proper struct to describe your register layout, make
  use of arrays for rx and tx
- don't use u32, s32 for not hardware related variables like return
  codes and loop counter.
- the routines that load and save the can data bytes from/into your
  mailbox look way to complicated. Please write down the layout so that
  we can think of a elegant way to do it
- a lot of functions unconditionally return 0, make them void if no
  error can happen
- think about using managed devices, that would simplify the probe and
  release function

> 
> Signed-off-by: Subhasish Ghosh <subhasish@mistralsolutions.com>
> ---
>  drivers/net/can/Kconfig     |    7 +
>  drivers/net/can/Makefile    |    1 +
>  drivers/net/can/pruss_can.c | 1074 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1082 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/can/pruss_can.c
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 5dec456..4682a4f 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -111,6 +111,13 @@ config PCH_CAN
>  	  embedded processor.
>  	  This driver can access CAN bus.
>  
> +config CAN_TI_DA8XX_PRU
> +	depends on CAN_DEV && ARCH_DAVINCI && ARCH_DAVINCI_DA850
> +	tristate "PRU based CAN emulation for DA8XX"
> +	---help---
> +	Enable this to emulate a CAN controller on the PRU of DA8XX.
> +	If not sure, mark N

Please indent the help text with 1 tab and 2 spaces

> +
>  source "drivers/net/can/mscan/Kconfig"
>  
>  source "drivers/net/can/sja1000/Kconfig"
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index 53c82a7..d0b7cbd 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
>  obj-$(CONFIG_CAN_MSCAN)		+= mscan/
>  obj-$(CONFIG_CAN_AT91)		+= at91_can.o
>  obj-$(CONFIG_CAN_TI_HECC)	+= ti_hecc.o
> +obj-$(CONFIG_CAN_TI_DA8XX_PRU)	+= pruss_can.o
>  obj-$(CONFIG_CAN_MCP251X)	+= mcp251x.o
>  obj-$(CONFIG_CAN_BFIN)		+= bfin_can.o
>  obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
> diff --git a/drivers/net/can/pruss_can.c b/drivers/net/can/pruss_can.c
> new file mode 100644
> index 0000000..7702509
> --- /dev/null
> +++ b/drivers/net/can/pruss_can.c
> @@ -0,0 +1,1074 @@
> +/*
> + *  TI DA8XX PRU CAN Emulation device driver
> + *  Author: subhasish@mistralsolutions.com
> + *
> + *  This driver supports TI's PRU CAN Emulation and the
> + *  specs for the same is available at <http://www.ti.com>
> + *
> + *  Copyright (C) 2010, 2011 Texas Instruments Incorporated
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License as
> + *  published by the Free Software Foundation version 2.
> + *
> + *  This program is distributed as is WITHOUT ANY WARRANTY of any
> + *  kind, whether express or implied; without even the implied warranty
> + *  of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/bitops.h>
> +#include <linux/interrupt.h>
> +#include <linux/errno.h>
> +#include <linux/netdevice.h>
> +#include <linux/skbuff.h>
> +#include <linux/platform_device.h>
> +#include <linux/firmware.h>
> +#include <linux/clk.h>
> +#include <linux/types.h>
> +#include <linux/sysfs.h>
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +#include <linux/mfd/pruss.h>
> +
> +#define PRUSS_CAN_RX_PRU_0			PRUSS_NUM0
> +#define PRUSS_CAN_TX_PRU_1			PRUSS_NUM1
> +#define PRUSS_CAN_TX_SYS_EVT			34
> +#define PRUSS_CAN_RX_SYS_EVT			33
> +#define PRUSS_CAN_ARM2DSP_SYS_EVT		32
> +#define PRUSS_CAN_DELAY_LOOP_LENGTH		2
> +#define PRUSS_CAN_TIMER_SETUP_DELAY		14
> +#define PRUSS_CAN_GPIO_SETUP_DELAY		150
> +#define PRUSS_CAN_TX_GBL_STAT_REG		(PRUSS_PRU1_BASE_ADDRESS + 0x04)
> +#define PRUSS_CAN_TX_INTR_MASK_REG		(PRUSS_PRU1_BASE_ADDRESS + 0x08)
> +#define PRUSS_CAN_TX_INTR_STAT_REG		(PRUSS_PRU1_BASE_ADDRESS + 0x0C)
> +#define PRUSS_CAN_TX_MBOX0_STAT_REG		(PRUSS_PRU1_BASE_ADDRESS + 0x10)
> +#define PRUSS_CAN_TX_ERR_CNTR_REG		(PRUSS_PRU1_BASE_ADDRESS + 0x30)

I think Wolfgang and me have asked you to describe the register layout
with a struct.

> +#define PRUSS_CAN_TX_INT_STAT			0x2
> +#define PRUSS_CAN_MBOX_OFFSET			0x40
> +#define PRUSS_CAN_MBOX_SIZE			0x10
> +#define PRUSS_CAN_MBOX_STAT_OFFSET		0x10
> +#define PRUSS_CAN_MBOX_STAT_SIZE		0x04
> +#define PRUSS_CAN_GBL_STAT_REG_VAL		0x00000040
> +#define PRUSS_CAN_INTR_MASK_REG_VAL		0x00004000
> +#define PRUSS_CAN_TIMING_VAL_TX			(PRUSS_PRU1_BASE_ADDRESS + 0xC0)
> +#define PRUSS_CAN_TIMING_SETUP			(PRUSS_PRU1_BASE_ADDRESS + 0xC4)
> +#define PRUSS_CAN_RX_GBL_STAT_REG		(PRUSS_PRU0_BASE_ADDRESS + 0x04)
> +#define PRUSS_CAN_RX_INTR_MASK_REG		(PRUSS_PRU0_BASE_ADDRESS + 0x08)
> +#define PRUSS_CAN_RX_INTR_STAT_REG		(PRUSS_PRU0_BASE_ADDRESS + 0x0C)
> +#define PRUSS_CAN_RX_ERR_CNTR_REG		(PRUSS_PRU0_BASE_ADDRESS + 0x34)
> +#define PRUSS_CAN_TIMING_VAL_RX			(PRUSS_PRU0_BASE_ADDRESS + 0xD0)
> +#define PRUSS_CAN_BIT_TIMING_VAL_RX		(PRUSS_PRU0_BASE_ADDRESS + 0xD4)
> +#define PRUSS_CAN_ID_MAP			(PRUSS_PRU0_BASE_ADDRESS + 0xF0)
> +#define PRUSS_CAN_ERROR_ACTIVE			128
> +#define PRUSS_CAN_GBL_STAT_REG_MASK		0x1F
> +#define PRUSS_CAN_GBL_STAT_REG_SET_TX		0x80
> +#define PRUSS_CAN_GBL_STAT_REG_SET_RX		0x40
> +#define PRUSS_CAN_GBL_STAT_REG_STOP_TX		0x7F
> +#define PRUSS_CAN_GBL_STAT_REG_STOP_RX		0xBF
> +#define PRUSS_CAN_SET_TX_REQ			0x00000080
> +#define PRUSS_CAN_STD_FRAME_MASK		18
> +#define PRUSS_CAN_START				1
> +#define PRUSS_CAN_MB_MIN			0
> +#define PRUSS_CAN_MB_MAX			7
> +#define PRUSS_CAN_MID_IDE			BIT(29)
> +#define PRUSS_CAN_ISR_BIT_CCI			BIT(15)
> +#define PRUSS_CAN_ISR_BIT_ESI			BIT(14)
> +#define PRUSS_CAN_ISR_BIT_SRDI			BIT(13)
> +#define PRUSS_CAN_ISR_BIT_RRI			BIT(8)
> +#define PRUSS_CAN_GSR_BIT_EPM			BIT(4)
> +#define PRUSS_CAN_GSR_BIT_BFM			BIT(3)
> +#define PRUSS_CAN_RTR_BUFF_NUM			8
> +#define PRUSS_DEF_NAPI_WEIGHT			8
> +#define PRUSS_CAN_DRV_NAME "da8xx_pruss_can"
> +#define PRUSS_CAN_DRV_DESC "TI PRU CAN Controller Driver v1.0"
> +
> +enum can_tx_dir {
> +	TRANSMIT = 1,
> +	RECEIVE

please add a "," after RECEIVE

> +};
> +
> +struct can_mbox {

I suggest to add a namespace to these structs, e.g. pru_can_mbox. Same
comment applies to the other structs.

> +	canid_t can_id;

You write this struct into the hardware, don't you? So you should not
use kernel internal types to describe your hardware layout. Think about
declaring this struct __packed__

> +	u8 data[8];
> +	u16 datalength;
> +	u16 crc;
> +};
> +
> +struct can_emu_cntx {
> +	enum can_tx_dir txdir;
> +	struct can_mbox mbox;
> +	u32 mboxno;
> +	u8 pruno;
> +	u32 gbl_stat;
> +	u32 intr_stat;
> +	u32 mbox_stat;
> +};
> +
> +struct can_emu_priv {
> +	struct can_priv can;
> +	struct napi_struct napi;
> +	struct net_device *ndev;
> +	struct device *dev;
> +	struct clk *clk_timer;
> +	struct can_emu_cntx can_tx_cntx;
> +	struct can_emu_cntx can_rx_cntx;

I have not looked at the rest of the code, but it smells that you should
make this an array of two cntx.

> +	unsigned int clk_freq_pru;
> +	unsigned int trx_irq;
> +	unsigned int tx_head;
> +	unsigned int tx_tail;
> +	unsigned int tx_next;
> +	unsigned int mbx_id[8];
> +};
> +
> +static struct can_bittiming_const pru_can_bittiming_const = {
> +	.name = PRUSS_CAN_DRV_NAME,
> +	.tseg1_min = 1,
> +	.tseg1_max = 16,
> +	.tseg2_min = 1,
> +	.tseg2_max = 8,
> +	.sjw_max = 4,
> +	.brp_min = 1,
> +	.brp_max = 256,
> +	.brp_inc = 1,
> +};
> +
> +static int pru_can_mbox_write(struct device *dev,
> +			struct can_emu_cntx *emu_cntx)
> +{
> +	u32 offset = 0;
                  ^^^^^

not needed

> +
> +	offset = PRUSS_PRU1_BASE_ADDRESS + PRUSS_CAN_MBOX_OFFSET
> +			+ (PRUSS_CAN_MBOX_SIZE * emu_cntx->mboxno);
> +
> +	pruss_writel_multi(dev, offset, (u32 *) &(emu_cntx->mbox), 4);
> +
> +	return 0;
> +}
> +
> +static int pru_can_mbox_read(struct device *dev,
> +			struct can_emu_cntx *emu_cntx)
> +{
> +	u32 offset = 0;

dito

> +
> +	offset = PRUSS_PRU0_BASE_ADDRESS + PRUSS_CAN_MBOX_OFFSET
> +			+ (PRUSS_CAN_MBOX_SIZE * emu_cntx->mboxno);
> +
> +	pruss_readl_multi(dev, offset, (u32 *) &emu_cntx->mbox, 4);

where does this "4" come from? consider using sizeof()

> +
> +	return 0;

why do you return 0 here? pruss_readl_multi is not void, although it
always returns 0, too. consider make all void.
> +}
> +
> +static int pru_can_rx_id_set(struct device *dev, u32 nodeid, u32 mboxno)
> +{
> +	pruss_writel(dev, (PRUSS_CAN_ID_MAP + (mboxno * 4)), nodeid);
> +
> +	return 0;

consider making this a void function.
> +}
> +
> +static int pru_can_intr_stat_get(struct device *dev,
> +		struct can_emu_cntx *emu_cntx)
> +{
> +	if (emu_cntx->pruno == PRUCORE_1)
> +		pruss_readl(dev, PRUSS_CAN_TX_INTR_STAT_REG,
> +				&emu_cntx->intr_stat);
> +	else if (emu_cntx->pruno == PRUCORE_0)
> +		pruss_readl(dev, PRUSS_CAN_RX_INTR_STAT_REG,
> +				&emu_cntx->intr_stat);

If you describe the register layout with a struct with an array
containing with rx and tx registers you can get rid of the if..else..
use emu_cntx->pruno as index to the array.

> +	else
> +		return -EINVAL;

It's an internally used function, if emu_cntx->pruno is bogous here
you've got really big problems. I think it's save to remove this. Then
this function would become a void function.

> +
> +	return 0;
> +}
> +
> +static int pru_can_gbl_stat_get(struct device *dev,
> +		struct can_emu_cntx *emu_cntx)
> +{
> +	if (emu_cntx->pruno == PRUCORE_1)
> +		pruss_readl(dev, PRUSS_CAN_TX_GBL_STAT_REG,
> +				&emu_cntx->gbl_stat);
> +	else if (emu_cntx->pruno == PRUCORE_0)
> +		pruss_readl(dev, PRUSS_CAN_RX_GBL_STAT_REG,
> +				&emu_cntx->gbl_stat);
> +	else
> +		return -EINVAL;
> +	return 0;

Same comments apply here, too.

> +}
> +
> +static int pru_can_tx_mode_set(struct device *dev, bool transfer_flag,
> +					enum can_tx_dir ecan_trx)
> +{
> +	u32 value;
> +
> +	if (ecan_trx == TRANSMIT) {
> +		pruss_readl(dev, PRUSS_CAN_RX_GBL_STAT_REG, &value);
> +		if (transfer_flag) {
> +			value &= PRUSS_CAN_GBL_STAT_REG_MASK;
> +			value |= PRUSS_CAN_GBL_STAT_REG_SET_TX;
> +		} else
> +			value &= PRUSS_CAN_GBL_STAT_REG_STOP_TX;
> +
> +		pruss_writel(dev, PRUSS_CAN_RX_GBL_STAT_REG, value);
> +		pruss_writel(dev, PRUSS_CAN_TX_GBL_STAT_REG, value);
> +	} else if (ecan_trx == RECEIVE) {
> +		pruss_readl(dev, PRUSS_CAN_RX_GBL_STAT_REG, &value);
> +		if (transfer_flag) {
> +			value &= PRUSS_CAN_GBL_STAT_REG_MASK;
> +			value |= PRUSS_CAN_GBL_STAT_REG_SET_RX;
> +		} else
> +			value &= PRUSS_CAN_GBL_STAT_REG_STOP_RX;
> +
> +		pruss_writel(dev, PRUSS_CAN_RX_GBL_STAT_REG, value);
> +		pruss_writel(dev, PRUSS_CAN_TX_GBL_STAT_REG, value);
> +	} else

Same comments apply here, too.

> +		return -EINVAL;
> +
> +	return 0;
> +}
> +

is this array const?
> +static u32 pruss_intc_init[19][3] = {
> +	{PRUSS_INTC_POLARITY0,		PRU_INTC_REGMAP_MASK,	0xFFFFFFFF},
> +	{PRUSS_INTC_POLARITY1,		PRU_INTC_REGMAP_MASK,	0xFFFFFFFF},
> +	{PRUSS_INTC_TYPE0,		PRU_INTC_REGMAP_MASK,	0x1C000000},
> +	{PRUSS_INTC_TYPE1,		PRU_INTC_REGMAP_MASK,	0},
> +	{PRUSS_INTC_GLBLEN,		0,			1},
> +	{PRUSS_INTC_HOSTMAP0,		PRU_INTC_REGMAP_MASK,	0x03020100},
> +	{PRUSS_INTC_HOSTMAP1,		PRU_INTC_REGMAP_MASK,	0x07060504},
> +	{PRUSS_INTC_HOSTMAP2,		PRU_INTC_REGMAP_MASK,	0x0000908},
> +	{PRUSS_INTC_CHANMAP0,		PRU_INTC_REGMAP_MASK,	0},
> +	{PRUSS_INTC_CHANMAP8,		PRU_INTC_REGMAP_MASK,	0x00020200},
> +	{PRUSS_INTC_STATIDXCLR,		0,			32},
> +	{PRUSS_INTC_STATIDXCLR,		0,			19},
> +	{PRUSS_INTC_ENIDXSET,		0,			19},
> +	{PRUSS_INTC_STATIDXCLR,		0,			18},
> +	{PRUSS_INTC_ENIDXSET,		0,			18},
> +	{PRUSS_INTC_STATIDXCLR,		0,			34},
> +	{PRUSS_INTC_ENIDXSET,		0,			34},
> +	{PRUSS_INTC_ENIDXSET,		0,			32},
> +	{PRUSS_INTC_HOSTINTEN,		0,			5}

please add ","

> +};
> +
> +static int pru_can_emu_init(struct device *dev, u32 pruclock)
> +{
> +	u32 value[8] = {[0 ... 7] = 1};
> +	u32 i;
we usually use plain ints as a for-loop variable.
> +
> +	/* PRU Internal Registers Initializations */
> +	pruss_writel_multi(dev, PRUSS_CAN_TX_MBOX0_STAT_REG, value, 8);

use sizeof(), or ARRAY_SIZE

> +
> +	*value = PRUSS_CAN_GBL_STAT_REG_VAL;
> +	pruss_writel(dev, PRUSS_CAN_TX_GBL_STAT_REG, value[0]);
> +	pruss_writel(dev, PRUSS_CAN_RX_GBL_STAT_REG, value[0]);

why not:
pruss_writel(dev, PRUSS_CAN_TX_GBL_STAT_REG, PRUSS_CAN_GBL_STAT_REG_VAL);

> +
> +	*value = PRUSS_CAN_INTR_MASK_REG_VAL;
> +	pruss_writel(dev, PRUSS_CAN_TX_INTR_MASK_REG, value[0]);
> +	pruss_writel(dev, PRUSS_CAN_RX_INTR_MASK_REG, value[0]);
> +
> +	for (i = 0; i < 19; i++)
ARRAY_SIZE
> +		(i < 12) ?	pruss_rmwl(dev, pruss_intc_init[i][0],
> +						pruss_intc_init[i][1],
> +						pruss_intc_init[i][2]) :
> +				pruss_idx_writel(dev, pruss_intc_init[i][0],
> +						pruss_intc_init[i][2]);

if..else here, please

or put the stuff into two arrays

> +	return 0;
> +}
> +
> +static void pru_can_emu_exit(struct device *dev)
> +{
> +	pruss_disable(dev, PRUSS_CAN_RX_PRU_0);
> +	pruss_disable(dev, PRUSS_CAN_TX_PRU_1);
> +}
> +
> +static int pru_can_tx(struct device *dev, u8 mboxnumber, u8 pruno)
> +{
> +	u32 value = 0;
> +
> +	if (PRUCORE_1 == pruno) {

we usually write it the other way round...:
if (pruno == PRUCORE_1)
> +		value = PRUSS_CAN_SET_TX_REQ;
> +		pruss_writel(dev, ((PRUSS_PRU1_BASE_ADDRESS +
> +				(PRUSS_CAN_MBOX_STAT_OFFSET +
> +				(PRUSS_CAN_MBOX_STAT_SIZE * mboxnumber)))
> +				& 0xFFFF), value);

don't use value, use PRUSS_CAN_SET_TX_REQ directly

> +	} else if (PRUCORE_0 == pruno) {
> +		pruss_readl(dev, PRUSS_CAN_RX_INTR_STAT_REG, &value);
> +		value = value & ~(1 << mboxnumber);
> +		pruss_writel(dev, PRUSS_CAN_RX_INTR_STAT_REG, value);
> +		value = 0;
> +		pruss_writel(dev, ((PRUSS_PRU0_BASE_ADDRESS +
> +				(PRUSS_CAN_MBOX_STAT_OFFSET +
> +				(PRUSS_CAN_MBOX_STAT_SIZE * mboxnumber)))
> +				& 0xFFFF), value);

same here

> +	} else
> +		return -EINVAL;

trust your own code, get rid of the -EINVAL, make this a void function.

> +	return 0;
> +}
> +
> +static int pru_can_reset_tx(struct device *dev)
> +{
> +	pruss_idx_writel(dev, PRUSS_INTC_STATIDXCLR, PRUSS_CAN_ARM2DSP_SYS_EVT);
> +	pruss_idx_writel(dev, PRUSS_INTC_ENIDXSET, PRUSS_CAN_ARM2DSP_SYS_EVT);
> +	pruss_idx_writel(dev, PRUSS_INTC_STATIDXSET, PRUSS_CAN_ARM2DSP_SYS_EVT);
> +	return 0;

void function?
> +}
> +
> +static void pru_can_mask_ints(struct device *dev, u32 trx, bool enable)
> +{
> +	u32 value = 0;

not needed
> +
> +	value = (trx == PRUSS_CAN_TX_PRU_1) ?
> +		PRUSS_CAN_TX_SYS_EVT : PRUSS_CAN_RX_SYS_EVT;

use a struct with arrays for the register description

> +	enable ? pruss_idx_writel(dev, PRUSS_INTC_ENIDXSET, value) :
> +		pruss_idx_writel(dev, PRUSS_INTC_ENIDXCLR, value);

if..else
> +}
> +
> +static unsigned int pru_can_get_error_cnt(struct device *dev, u8 pruno)
> +{
> +	u32 value = 0;
not needed
> +
> +	if (pruno == PRUSS_CAN_RX_PRU_0)
> +		pruss_readl(dev, PRUSS_CAN_RX_ERR_CNTR_REG, &value);
> +	else if (pruno == PRUSS_CAN_TX_PRU_1)
> +		pruss_readl(dev, PRUSS_CAN_TX_ERR_CNTR_REG, &value);
> +	else
> +		return -EINVAL;

remove the -EINVAL

> +
> +	return value;
> +}
> +
> +static unsigned int pru_can_get_intc_status(struct device *dev)
> +{
> +	u32 value = 0;
not needed
> +
> +	pruss_readl(dev, PRUSS_INTC_STATCLRINT1, &value);
> +	return value;
> +}
> +
> +static void pru_can_clr_intc_status(struct device *dev, u32 trx)
> +{
> +	u32 value = 0;
dito
> +
> +	value = (trx == PRUSS_CAN_TX_PRU_1) ?
> +		PRUSS_CAN_TX_SYS_EVT : PRUSS_CAN_RX_SYS_EVT;

use a struct + array for the resiter desc

> +	pruss_idx_writel(dev, PRUSS_INTC_STATIDXCLR, value);
> +}
> +
> +static int pru_can_get_state(const struct net_device *ndev,
> +					enum can_state *state)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	*state = priv->can.state;

we don't implemnt this function anymore..
> +
> +	return 0;
> +}
> +
> +static int pru_can_set_bittiming(struct net_device *ndev)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	struct can_bittiming *bt = &priv->can.bittiming;
> +	u32 value;
> +
> +	value = priv->can.clock.freq / bt->bitrate;
> +	pruss_writel(priv->dev, PRUSS_CAN_TIMING_VAL_TX, value);
> +	pruss_writel(priv->dev, PRUSS_CAN_BIT_TIMING_VAL_RX, value);
> +
> +	value = (bt->phase_seg2 + bt->phase_seg1 +
> +			bt->prop_seg + 1) * bt->brp;
> +	value = (value >> 1) - PRUSS_CAN_TIMER_SETUP_DELAY;
> +	value = (value << 16) | value;
> +	pruss_writel(priv->dev, PRUSS_CAN_TIMING_VAL_RX, value);
> +
> +	value = (PRUSS_CAN_GPIO_SETUP_DELAY *
> +		(priv->clk_freq_pru / 1000000) / 1000) /
> +		PRUSS_CAN_DELAY_LOOP_LENGTH;
> +
> +	pruss_writel(priv->dev, PRUSS_CAN_TIMING_SETUP, value);
> +	return 0;
> +}
> +
> +static void pru_can_stop(struct net_device *ndev)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +
> +	pru_can_mask_ints(priv->dev, PRUSS_CAN_TX_PRU_1, false);
> +	pru_can_mask_ints(priv->dev, PRUSS_CAN_RX_PRU_0, false);
> +	pru_can_reset_tx(priv->dev);
> +	priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +/*
> + * This is to just set the can state to ERROR_ACTIVE
> + *	ip link set canX up type can bitrate 125000

fix the comment

> + */
> +static void pru_can_start(struct net_device *ndev)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +
> +	if (priv->can.state != CAN_STATE_STOPPED)
> +		pru_can_stop(ndev);
> +
> +	pru_can_mask_ints(priv->dev, PRUSS_CAN_TX_PRU_1, true);
> +	pru_can_mask_ints(priv->dev, PRUSS_CAN_RX_PRU_0, true);
> +
> +	pru_can_gbl_stat_get(priv->dev, &priv->can_tx_cntx);
> +	pru_can_gbl_stat_get(priv->dev, &priv->can_rx_cntx);
> +
> +	if (PRUSS_CAN_GSR_BIT_EPM & priv->can_tx_cntx.gbl_stat)
> +		priv->can.state = CAN_STATE_ERROR_PASSIVE;
> +	else if (PRUSS_CAN_GSR_BIT_BFM & priv->can_tx_cntx.gbl_stat)
> +		priv->can.state = CAN_STATE_BUS_OFF;
> +	else
> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +}
> +
> +static int pru_can_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> +	int ret = 0;
> +
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		pru_can_start(ndev);
> +		netif_wake_queue(ndev);
> +		break;
> +	default:
> +		ret = -EOPNOTSUPP;
> +		break;
> +	}
> +	return ret;
> +}
> +
> +static netdev_tx_t pru_can_start_xmit(struct sk_buff *skb,
> +					struct net_device *ndev)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	int count;
> +	u8 *data = cf->data;
> +	u8 dlc = cf->can_dlc;
> +	u8 *pdata = NULL;
> +
> +	if (can_dropped_invalid_skb(ndev, skb))
> +		return NETDEV_TX_OK;
> +
> +	netif_stop_queue(ndev);

why do you stop the queue unconditionally here?


> +	if (cf->can_id & CAN_EFF_FLAG)	/* Extended frame format */
> +		priv->can_tx_cntx.mbox.can_id =
> +		    (cf->can_id & CAN_EFF_MASK) | PRUSS_CAN_MID_IDE;
> +	else			/* Standard frame format */
> +		priv->can_tx_cntx.mbox.can_id =
> +		    (cf->can_id & CAN_SFF_MASK) << PRUSS_CAN_STD_FRAME_MASK;
> +
> +	if (cf->can_id & CAN_RTR_FLAG)	/* Remote transmission request */
> +		priv->can_tx_cntx.mbox.can_id |= CAN_RTR_FLAG;
> +
> +	pdata = &priv->can_tx_cntx.mbox.data[0] + (dlc - 1);
> +	for (count = 0; count < (u8) dlc; count++)
> +		*pdata-- = *data++;

What does this loop do? endianess conversion? Please don't open code this.

> +
> +	priv->can_tx_cntx.mbox.datalength = (u16) dlc;
no need to cast

> +	priv->can_tx_cntx.mbox.crc = 0;
> +/*
> + * search for the next available mbx
> + * if the next mbx is busy, then try the next + 1
> + * do this until the head is reached.
> + * if still unable to tx, stop accepting any packets
> + * if able to tx and the head is reached, then reset next to tail, i.e mbx0
> + * if head is not reached, then just point to the next mbx
> + */

indention, please

Your tx algorithm looks fishy. You always use can_get_echo_skb(ndev, 0).
This means you can have only 1 can frame on the fly. But you say you
look for a free mailbox. You have to tx mailboxes in a defined order,
otherwise your hardware/firmware is broken. If your hardware transmits
frames in order, you always know which one will be the next free
mailbox. You have a power of 2 number of mailboxes, you can simply apply
a mask to get to the real mailbox number. No need for manual wrap
around. Have a look at the at91_can tx sheme.

Activate the tx_interrupt, putting a can frame into a mailbox, stop the
tx_queue if there are no free mailboxes, or in case of a wrap around.
Reenable the tx_queue in the tx_complete interrupt handler.

> +	for (; priv->tx_next <= priv->tx_head; priv->tx_next++) {
> +		priv->can_tx_cntx.mboxno = priv->tx_next;
> +		if (-1 == pru_can_mbox_write(priv->dev,
> +					&priv->can_tx_cntx)) {

this function will always return 0.

> +			if (priv->tx_next == priv->tx_head) {
> +				priv->tx_next = priv->tx_tail;
> +				netif_stop_queue(ndev);	/* IF stalled */
> +				dev_err(priv->dev,
> +					"%s: no tx mbx available", __func__);
> +				return NETDEV_TX_BUSY;
> +			} else
> +				continue;
> +		} else {
> +			/* set transmit request */
> +			pru_can_tx(priv->dev, priv->tx_next,
> +						PRUSS_CAN_TX_PRU_1);
> +			pru_can_tx_mode_set(priv->dev, false, RECEIVE);
> +			pru_can_tx_mode_set(priv->dev, true, TRANSMIT);
> +			pru_can_reset_tx(priv->dev);
> +			priv->tx_next++;
> +			can_put_echo_skb(skb, ndev, 0);
                                                   ^^^

see comment above

> +			break;
> +		}
> +	}
> +	if (priv->tx_next > priv->tx_head)
> +		priv->tx_next = priv->tx_tail;
> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static int pru_can_rx(struct net_device *ndev, u32 mbxno)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u8 *data = NULL;
> +	u8 *pdata = NULL;
> +	int count = 0;
> +
> +	skb = alloc_can_skb(ndev, &cf);
> +	if (!skb) {
> +		if (printk_ratelimit())
> +			dev_err(priv->dev,
> +				"alloc_can_skb() failed\n");
> +		return -ENOMEM;
> +	}
> +	data = cf->data;
> +	/* get payload */
> +	priv->can_rx_cntx.mboxno = mbxno;
> +	if (pru_can_mbox_read(priv->dev, &priv->can_rx_cntx)) {

function always returns 0!

> +		dev_err(priv->dev, "failed to get data from mailbox\n");
> +		return -EAGAIN;
> +	}
> +	/* give ownweship to pru */
> +	pru_can_tx(priv->dev, mbxno, PRUSS_CAN_RX_PRU_0);
> +
> +	/* get data length code */
> +	cf->can_dlc = get_can_dlc(priv->can_rx_cntx.mbox.datalength & 0xF);

This looks to complicated. Please state how the individual can bytes are
placed in the mailbox, so that we can think of a simpler way to do this.

> +	if (cf->can_dlc <= 4) {
> +		pdata = &priv->can_rx_cntx.mbox.data[4] + (4 - cf->can_dlc);
> +		for (count = 0; count < cf->can_dlc; count++)
> +			*data++ = *pdata++;
> +	} else {
> +		pdata = &priv->can_rx_cntx.mbox.data[4];
> +		for (count = 0; count < 4; count++)
> +			*data++ = *pdata++;
> +		pdata = &priv->can_rx_cntx.mbox.data[3] - (cf->can_dlc - 5);
> +		for (count = 0; count < cf->can_dlc - 4; count++)
> +			*data++ = *pdata++;
> +	}
> +
> +	/* get id extended or std */
> +	if (priv->can_rx_cntx.mbox.can_id & PRUSS_CAN_MID_IDE)
> +		cf->can_id = (priv->can_rx_cntx.mbox.can_id & CAN_EFF_MASK)
> +								| CAN_EFF_FLAG;

the usual way is to write the "|" at the end of the line.

> +	else
> +		cf->can_id = (priv->can_rx_cntx.mbox.can_id >> 18)
> +							& CAN_SFF_MASK;
> +
> +	if (priv->can_rx_cntx.mbox.can_id & CAN_RTR_FLAG)
> +		cf->can_id |= CAN_RTR_FLAG;

please don't copy any data to the can frame in case if an RTR message.

> +
> +	stats->rx_bytes += cf->can_dlc;
> +	netif_receive_skb(skb);
> +	stats->rx_packets++;
> +	return 0;
> +}
> +
> +static int pru_can_err(struct net_device *ndev, int int_status,
> +			     int err_status)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u32 tx_err_cnt, rx_err_cnt;
> +
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb) {
> +		if (printk_ratelimit())
> +			dev_err(priv->dev,
> +				"alloc_can_err_skb() failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	if (err_status & PRUSS_CAN_GSR_BIT_EPM) {	/* error passive int */
> +		priv->can.state = CAN_STATE_ERROR_PASSIVE;
> +		++priv->can.can_stats.error_passive;
> +		cf->can_id |= CAN_ERR_CRTL;
> +		tx_err_cnt = pru_can_get_error_cnt(priv->dev,
> +						PRUSS_CAN_TX_PRU_1);
> +		rx_err_cnt = pru_can_get_error_cnt(priv->dev,
> +						PRUSS_CAN_RX_PRU_0);
> +		if (tx_err_cnt > PRUSS_CAN_ERROR_ACTIVE - 1)
> +			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> +		if (rx_err_cnt > PRUSS_CAN_ERROR_ACTIVE - 1)
> +			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> +
> +		dev_dbg(priv->ndev->dev.parent, "Error passive interrupt\n");
> +	}
> +
> +	if (err_status & PRUSS_CAN_GSR_BIT_BFM) {
> +		priv->can.state = CAN_STATE_BUS_OFF;
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		/*
> +		 *      Disable all interrupts in bus-off to avoid int hog
> +		 *      this should be handled by the pru
> +		 */
> +		pru_can_mask_ints(priv->dev, PRUSS_CAN_TX_PRU_1, false);
> +		pru_can_mask_ints(priv->dev, PRUSS_CAN_RX_PRU_0, false);
> +		can_bus_off(ndev);
> +		dev_dbg(priv->ndev->dev.parent, "Bus off mode\n");
> +	}
> +
> +	netif_rx(skb);
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +	return 0;
> +}
> +
> +static int pru_can_rx_poll(struct napi_struct *napi, int quota)
> +{
> +	struct net_device *ndev = napi->dev;
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	u32 bit_set, mbxno = 0;
> +	u32 num_pkts = 0;
> +
> +	if (!netif_running(ndev))
> +		return 0;
> +
> +	do {
> +		/* rx int sys_evt -> 33 */
> +		pru_can_clr_intc_status(priv->dev, PRUSS_CAN_RX_PRU_0);
> +		if (pru_can_intr_stat_get(priv->dev, &priv->can_rx_cntx))
> +			return num_pkts;
> +
> +		if (PRUSS_CAN_ISR_BIT_RRI & priv->can_rx_cntx.intr_stat) {
> +			mbxno = PRUSS_CAN_RTR_BUFF_NUM;
> +			pru_can_rx(ndev, mbxno);
> +			num_pkts++;
> +		} else {
> +			/* Extract the mboxno from the status */
> +			bit_set = fls(priv->can_rx_cntx.intr_stat & 0xFF);
> +			if (bit_set) {
> +				num_pkts++;
> +				mbxno = bit_set - 1;
> +				if (PRUSS_CAN_ISR_BIT_ESI & priv->can_rx_cntx.
> +				    intr_stat) {
> +					pru_can_gbl_stat_get(priv->dev,
> +						&priv->can_rx_cntx);
> +						pru_can_err(ndev,
> +					priv->can_rx_cntx.intr_stat,
> +					priv->can_rx_cntx.gbl_stat);
> +				} else
> +					pru_can_rx(ndev, mbxno);
> +			} else
> +				break;
> +		}
> +	} while (((PRUSS_CAN_TX_INT_STAT & pru_can_get_intc_status(priv->dev))
> +						&& (num_pkts < quota)));
> +
> +	/* Enable packet interrupt if all pkts are handled */
> +	if (!(PRUSS_CAN_TX_INT_STAT & pru_can_get_intc_status(priv->dev))) {
> +		napi_complete(napi);
> +		/* Re-enable RX mailbox interrupts */
> +		pru_can_mask_ints(priv->dev, PRUSS_CAN_RX_PRU_0, true);
> +	}
> +	return num_pkts;
> +}
> +
> +static irqreturn_t pru_tx_can_intr(int irq, void *dev_id)
> +{
> +	struct net_device *ndev = dev_id;
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	u32 bit_set, mbxno;
> +
> +	pru_can_intr_stat_get(priv->dev, &priv->can_tx_cntx);
> +	if ((PRUSS_CAN_ISR_BIT_CCI & priv->can_tx_cntx.intr_stat)
> +	    || (PRUSS_CAN_ISR_BIT_SRDI & priv->can_tx_cntx.intr_stat)) {
> +		dev_dbg(priv->ndev->dev.parent, "tx_int_status = 0x%X\n",
> +			    priv->can_tx_cntx.intr_stat);
> +		can_free_echo_skb(ndev, 0);
                                       ^^^

make no sense if using multiple tx mailboxes

> +	} else {
> +		bit_set = fls(priv->can_tx_cntx.intr_stat & 0xFF);
> +		if (!bit_set) {
> +			dev_err(priv->dev, "%s: invalid mailbox number\n",
> +								__func__);
> +			can_free_echo_skb(ndev, 0);
                                               ^^^^
> +		} else {
> +			mbxno = bit_set - 1;
> +			if (PRUSS_CAN_ISR_BIT_ESI & priv->can_tx_cntx.
> +								intr_stat) {
> +				/* read gsr and ack pru */
> +				pru_can_gbl_stat_get(priv->dev,
> +							&priv->can_tx_cntx);
> +				pru_can_err(ndev, priv->can_tx_cntx.intr_stat,
> +						priv->can_tx_cntx.gbl_stat);
> +			} else {
> +				stats->tx_packets++;
> +				/* stats->tx_bytes += dlc; */
> +				/*can_get_echo_skb(ndev, 0);*/

??

> +			}
> +		}
> +	}
> +	netif_wake_queue(ndev);
> +	can_get_echo_skb(ndev, 0);
again?
> +	pru_can_tx_mode_set(priv->dev, true, RECEIVE);
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t pru_rx_can_intr(int irq, void *dev_id)

why is this function calles rx_can_intr it's a generic interrupt function..

> +{
> +	struct net_device *ndev = dev_id;
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	u32 intc_status = 0;
> +
> +	intc_status = pru_can_get_intc_status(priv->dev);
> +
> +	/* tx int sys_evt -> 34 */
> +	if (intc_status & 4) {
> +		pru_can_clr_intc_status(priv->dev, PRUSS_CAN_TX_PRU_1);
> +		return pru_tx_can_intr(irq, dev_id);
why are you returning here? is is possible the you have a can frame to
receivce?
> +	}
> +	/* Disable RX mailbox interrupts and let NAPI reenable them */
> +	if (intc_status & 2) {
> +		pru_can_mask_ints(priv->dev, PRUSS_CAN_RX_PRU_0, false);
> +		napi_schedule(&priv->napi);
> +	}
> +
> +	return IRQ_HANDLED;
               ^^^^^^^^^^^^

that might not be true....

> +}
> +
> +static int pru_can_open(struct net_device *ndev)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	s32 err;
> +
> +	/* register interrupt handler */
> +	err = request_irq(priv->trx_irq, &pru_rx_can_intr, IRQF_SHARED,
> +						"pru_can_irq", ndev);

use dev->name for interrupt name

> +	if (err) {
> +		dev_err(priv->dev, "error requesting rx interrupt\n");
> +		goto exit_trx_irq;
> +	}
> +	err = open_candev(ndev);
> +	if (err) {
> +		dev_err(priv->dev, "open_candev() failed %d\n", err);
> +		goto exit_open;
> +	}
> +
> +	pru_can_emu_init(priv->dev, priv->can.clock.freq);
> +	priv->tx_tail = PRUSS_CAN_MB_MIN;
> +	priv->tx_head = PRUSS_CAN_MB_MAX;
> +	pru_can_start(ndev);
> +	napi_enable(&priv->napi);
> +	netif_start_queue(ndev);
> +	return 0;
> +
> +exit_open:
> +	free_irq(priv->trx_irq, ndev);
> +exit_trx_irq:
> +	return err;
> +}
> +
> +static int pru_can_close(struct net_device *ndev)
> +{
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +
> +	netif_stop_queue(ndev);
> +	napi_disable(&priv->napi);
> +	close_candev(ndev);
> +	free_irq(priv->trx_irq, ndev);
> +	return 0;
> +}
> +
> +static const struct net_device_ops pru_can_netdev_ops = {
> +	.ndo_open		= pru_can_open,
> +	.ndo_stop		= pru_can_close,
> +	.ndo_start_xmit		= pru_can_start_xmit,
> +};
> +
> +/* Shows all the mailbox IDs */
> +static ssize_t pru_sysfs_mbx_id_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct can_emu_priv *priv = netdev_priv(to_net_dev(dev));
> +
> +	return snprintf(buf, PAGE_SIZE, "<mbx_no:mbx_id>\n"
> +					"0:0x%X 1:0x%X 2:0x%X 3:0x%X "
> +					"4:0x%X 5:0x%X 6:0x%X 7:0x%X\n",
> +					priv->mbx_id[0], priv->mbx_id[1],
> +					priv->mbx_id[2], priv->mbx_id[3],
> +					priv->mbx_id[4], priv->mbx_id[5],
> +					priv->mbx_id[6], priv->mbx_id[7]);
> +}
> +
> +/*
> + * Sets Mailbox IDs
> + * This should be programmed as mbx_num:mbx_id (in hex)
> + * eg: $ echo 0:0x123 > /sys/class/net/can0/mbx_id
> + */
> +static ssize_t pru_sysfs_mbx_id_set(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct net_device *ndev = to_net_dev(dev);
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +	unsigned long can_id;
> +	unsigned long mbx_num;
> +	char mbx[2] = {*buf, '\0'}; /* mbx num */
> +	ssize_t ret = count;
> +	s32 err;

I think you have to lock here:
rtnl_lock();
> +
> +	if (ndev->flags & IFF_UP) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (*(buf + 1) != ':') {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	err = strict_strtoul(mbx, 0, &mbx_num);
> +	if (err) {
> +		ret = err;
> +		goto out;
> +	}
> +
> +	if (mbx_num > 7) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	err = strict_strtoul((buf + 2), 0, &can_id);
> +	if (err) {
> +		ret = err;
> +		goto out;
> +	}
> +
> +	priv->mbx_id[mbx_num] = can_id;
> +	pru_can_rx_id_set(priv->dev, priv->mbx_id[mbx_num], mbx_num);
> +
> +	return ret;
> +out:
> +	dev_err(priv->dev, "invalid buffer format\n");
> +	return ret;
> +}
> +
> +static DEVICE_ATTR(mbx_id, S_IWUSR | S_IRUGO,
> +	pru_sysfs_mbx_id_show, pru_sysfs_mbx_id_set);
> +
> +static struct attribute *pru_sysfs_attrs[] = {
> +	&dev_attr_mbx_id.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group pru_sysfs_attr_group = {
> +	.attrs = pru_sysfs_attrs,
> +};
> +
> +static int __devinit pru_can_probe(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = NULL;
> +	const struct da850_evm_pruss_can_data *pdata;
> +	struct can_emu_priv *priv = NULL;
> +	struct device *dev = &pdev->dev;
> +	struct clk *clk_pruss;
> +	const struct firmware *fw_rx;
> +	const struct firmware *fw_tx;
> +	u32 err;
use int
> +
> +	pdata = dev->platform_data;
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "platform data not found\n");
> +		return -EINVAL;
> +	}
> +	(pdata->setup)();

no need fot the ( )

> +
> +	ndev = alloc_candev(sizeof(struct can_emu_priv), PRUSS_CAN_MB_MAX + 1);
> +	if (!ndev) {
> +		dev_err(&pdev->dev, "alloc_candev failed\n");
> +		err = -ENOMEM;
> +		goto probe_exit;
> +	}
> +
> +	ndev->sysfs_groups[0] = &pru_sysfs_attr_group;
> +
> +	priv = netdev_priv(ndev);
> +
> +	priv->trx_irq = platform_get_irq(to_platform_device(dev->parent), 0);
> +	if (!priv->trx_irq) {
> +		dev_err(&pdev->dev, "unable to get pru "
> +						"interrupt resources!\n");
> +		err = -ENODEV;
> +		goto probe_exit;
> +	}
> +
> +	priv->ndev = ndev;
> +	priv->dev = dev;
> +
> +	priv->can.bittiming_const = &pru_can_bittiming_const;
> +	priv->can.do_set_bittiming = pru_can_set_bittiming;
> +	priv->can.do_set_mode = pru_can_set_mode;
> +	priv->can.do_get_state = pru_can_get_state;
> +	priv->can_tx_cntx.pruno = PRUSS_CAN_TX_PRU_1;
> +	priv->can_rx_cntx.pruno = PRUSS_CAN_RX_PRU_0;
> +
> +	/* we support local echo, no arp */
> +	ndev->flags |= (IFF_ECHO | IFF_NOARP);

no need to se NOARP

> +
> +	/* pdev->dev->device_private->driver_data = ndev */
> +	platform_set_drvdata(pdev, ndev);
> +	SET_NETDEV_DEV(ndev, &pdev->dev);
> +	ndev->netdev_ops = &pru_can_netdev_ops;
> +
> +	priv->clk_timer = clk_get(&pdev->dev, "pll1_sysclk2");
> +	if (IS_ERR(priv->clk_timer)) {
> +		dev_err(&pdev->dev, "no timer clock available\n");
> +		err = PTR_ERR(priv->clk_timer);
> +		priv->clk_timer = NULL;
> +		goto probe_exit_candev;
> +	}
> +
> +	priv->can.clock.freq = clk_get_rate(priv->clk_timer);
> +
> +	clk_pruss = clk_get(NULL, "pruss");
> +	if (IS_ERR(clk_pruss)) {
> +		dev_err(&pdev->dev, "no clock available: pruss\n");
> +		err = -ENODEV;
> +		goto probe_exit_clk;
> +	}
> +	priv->clk_freq_pru = clk_get_rate(clk_pruss);
> +	clk_put(clk_pruss);

why do you put the clock here?
> +
> +	err = register_candev(ndev);
> +	if (err) {
> +		dev_err(&pdev->dev, "register_candev() failed\n");
> +		err = -ENODEV;
> +		goto probe_exit_clk;
> +	}
> +
> +	err = request_firmware(&fw_tx, "PRU_CAN_Emulation_Tx.bin",
> +			&pdev->dev);
> +	if (err) {
> +		dev_err(&pdev->dev, "can't load firmware\n");
> +		err = -ENODEV;
> +		goto probe_exit_clk;
> +	}
> +
> +	dev_info(&pdev->dev, "fw_tx size %d. downloading...\n", fw_tx->size);
> +
> +	err = request_firmware(&fw_rx, "PRU_CAN_Emulation_Rx.bin",
> +			&pdev->dev);
> +	if (err) {
> +		dev_err(&pdev->dev, "can't load firmware\n");
> +		err = -ENODEV;
> +		goto probe_release_fw;
> +	}
> +	dev_info(&pdev->dev, "fw_rx size %d. downloading...\n", fw_rx->size);
> +
> +	/* init the pru */
> +	pru_can_emu_init(priv->dev, priv->can.clock.freq);
> +	udelay(200);
> +
> +	netif_napi_add(ndev, &priv->napi, pru_can_rx_poll,
> +					PRUSS_DEF_NAPI_WEIGHT);

personally I'd wait to add the interface to napi until the firmware is
loaded.

> +
> +	pruss_enable(priv->dev, PRUSS_CAN_RX_PRU_0);
> +	pruss_enable(priv->dev, PRUSS_CAN_TX_PRU_1);
> +
> +	/* download firmware into pru */
> +	err = pruss_load(priv->dev, PRUSS_CAN_RX_PRU_0,
> +		(u32 *)fw_rx->data, (fw_rx->size / 4));
> +	if (err) {
> +		dev_err(&pdev->dev, "firmware download error\n");
> +		err = -ENODEV;
> +		goto probe_release_fw_1;
> +	}
> +	release_firmware(fw_rx);
> +	err = pruss_load(priv->dev, PRUSS_CAN_TX_PRU_1,
> +		(u32 *)fw_tx->data, (fw_tx->size / 4));
> +	if (err) {
> +		dev_err(&pdev->dev, "firmware download error\n");
> +		err = -ENODEV;
> +		goto probe_release_fw_1;
> +	}
> +	release_firmware(fw_tx);
> +
> +	pruss_run(priv->dev, PRUSS_CAN_RX_PRU_0);
> +	pruss_run(priv->dev, PRUSS_CAN_TX_PRU_1);
> +
> +	dev_info(&pdev->dev,
> +		 "%s device registered (trx_irq = %d,  clk = %d)\n",
> +		 PRUSS_CAN_DRV_NAME, priv->trx_irq, priv->can.clock.freq);
> +
> +	return 0;
> +
> +probe_release_fw_1:
> +	release_firmware(fw_rx);
> +probe_release_fw:
> +	release_firmware(fw_tx);
> +probe_exit_clk:
> +	clk_put(priv->clk_timer);
> +probe_exit_candev:
> +	if (NULL != ndev)
> +		free_candev(ndev);
> +probe_exit:
> +	return err;
> +}
> +
> +static int __devexit pru_can_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct can_emu_priv *priv = netdev_priv(ndev);
> +
> +	pru_can_stop(ndev);
> +	pru_can_emu_exit(priv->dev);
> +	clk_put(priv->clk_timer);
> +	unregister_candev(ndev);
> +	free_candev(ndev);
> +	platform_set_drvdata(pdev, NULL);
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int pru_can_suspend(struct platform_device *pdev,
> +			pm_message_t mesg)
> +{
> +	dev_info(&pdev->dev, "%s not yet implemented\n", __func__);
> +	return 0;
> +}
> +
> +static int pru_can_resume(struct platform_device *pdev)
> +{
> +	dev_info(&pdev->dev, "%s not yet implemented\n", __func__);
> +	return 0;
> +}
> +#else
> +#define pru_can_suspend NULL
> +#define pru_can_resume NULL
> +#endif
> +
> +static struct platform_driver omapl_pru_can_driver = {
> +	.probe		= pru_can_probe,
> +	.remove		= __devexit_p(pru_can_remove),
> +	.suspend	= pru_can_suspend,
> +	.resume		= pru_can_resume,
> +	.driver		= {
> +		.name	= PRUSS_CAN_DRV_NAME,
> +		.owner	= THIS_MODULE,
> +	},
> +};
> +
> +static int __init pru_can_init(void)
> +{
> +	pr_debug(KERN_INFO PRUSS_CAN_DRV_DESC "\n");
> +	return platform_driver_register(&omapl_pru_can_driver);
> +}
> +
> +module_init(pru_can_init);
> +
> +static void __exit pru_can_exit(void)
> +{
> +	pr_debug(KERN_INFO PRUSS_CAN_DRV_DESC " unloaded\n");
> +	platform_driver_unregister(&omapl_pru_can_driver);
> +}
> +
> +module_exit(pru_can_exit);
> +
> +MODULE_AUTHOR("Subhasish Ghosh <subhasish@mistralsolutions.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("omapl pru CAN netdevice driver");

regards, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply

* [PATCH] af_unix: Only allow recv on connected seqpacket sockets.
From: Eric W. Biederman @ 2011-04-24 11:54 UTC (permalink / raw)
  To: netdev; +Cc: Pavel Emelyanov, David S. Miller, Dan Aloni, stable
In-Reply-To: <BANLkTimrOs2T_bnbSJDgppAAh_MUWt_erg@mail.gmail.com>


This fixes the following oops discovered by Dan Aloni:
> Anyway, the following is the output of the Oops that I got on the
> Ubuntu kernel on which I first detected the problem
> (2.6.37-12-generic). The Oops that followed will be more useful, I
> guess.

>[ 5594.669852] BUG: unable to handle kernel NULL pointer dereference
> at           (null)
> [ 5594.681606] IP: [<ffffffff81550b7b>] unix_dgram_recvmsg+0x1fb/0x420
> [ 5594.687576] PGD 2a05d067 PUD 2b951067 PMD 0
> [ 5594.693720] Oops: 0002 [#1] SMP
> [ 5594.699888] last sysfs file:

The bug was that unix domain sockets use a pseduo packet for
connecting and accept uses that psudo packet to get the socket.
In the buggy seqpacket case we were allowing unconnected
sockets to call recvmsg and try to receive the pseudo packet.

That is always wrong and as of commit 7361c36c5 the pseudo
packet had become enough different from a normal packet
that the kernel started oopsing.

Do for seqpacket_recv what was done for seqpacket_send in 2.5
and only allow it on connected seqpacket sockets.

Cc: stable@kernel.org
Tested-by: Dan Aloni <dan@aloni.org>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 net/unix/af_unix.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 3a43a83..b1d75be 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -524,6 +524,8 @@ static int unix_dgram_connect(struct socket *, struct sockaddr *,
 			      int, int);
 static int unix_seqpacket_sendmsg(struct kiocb *, struct socket *,
 				  struct msghdr *, size_t);
+static int unix_seqpacket_recvmsg(struct kiocb *, struct socket *,
+				  struct msghdr *, size_t, int);
 
 static const struct proto_ops unix_stream_ops = {
 	.family =	PF_UNIX,
@@ -583,7 +585,7 @@ static const struct proto_ops unix_seqpacket_ops = {
 	.setsockopt =	sock_no_setsockopt,
 	.getsockopt =	sock_no_getsockopt,
 	.sendmsg =	unix_seqpacket_sendmsg,
-	.recvmsg =	unix_dgram_recvmsg,
+	.recvmsg =	unix_seqpacket_recvmsg,
 	.mmap =		sock_no_mmap,
 	.sendpage =	sock_no_sendpage,
 };
@@ -1699,6 +1701,18 @@ static int unix_seqpacket_sendmsg(struct kiocb *kiocb, struct socket *sock,
 	return unix_dgram_sendmsg(kiocb, sock, msg, len);
 }
 
+static int unix_seqpacket_recvmsg(struct kiocb *iocb, struct socket *sock,
+			      struct msghdr *msg, size_t size,
+			      int flags)
+{
+	struct sock *sk = sock->sk;
+
+	if (sk->sk_state != TCP_ESTABLISHED)
+		return -ENOTCONN;
+
+	return unix_dgram_recvmsg(iocb, sock, msg, size, flags);
+}
+
 static void unix_copy_addr(struct msghdr *msg, struct sock *sk)
 {
 	struct unix_sock *u = unix_sk(sk);
-- 
1.6.5.2.143.g8cc62


^ permalink raw reply related

* [PATCH net-2.6 1/1] r8169 driver updates
From: Francois Romieu @ 2011-04-24 16:32 UTC (permalink / raw)
  To: davem; +Cc: netdev, Ciprian Docan, Linus Torvalds, nic_swsd

Please pull from branch 'davem.r8169' in repository

git://git.kernel.org/pub/scm/linux/kernel/git/romieu/netdev-2.6.git davem.r8169

to get the change below.

The firmware is cached during the first successfull call to open() and
released once the network device is unregistered. The driver uses the
cached firmware between open() and unregister_netdev().

So far the firmware is optional : a failure to load the firmware does
not prevent open() to success. It is thus necessary to 1) unregister
all 816x / 810[23] devices and 2) force a driver probe to issue a new
firmware load.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Fixed-by: Ciprian Docan <docan@eden.rutgers.edu>

Please note :
- it will merge with davem-next like a charm
- it will be necessary to add entries in rtl_firmware_infos for
  FIRMWARE_8168E_{1, 2} thereafter and remove messages in
  rtl8168e_{1, 2}_hw_phy_config. I should take care of it.

Distance from 'davem.base' (0b0dc0f17f98b59772ca6380c7d5ce4cc593a974)
---------------------------------------------------------------------

953a12cc2889d1be92e80a2d0bab5ffef4942300

Diffstat
--------

 drivers/net/r8169.c |   99 +++++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 71 insertions(+), 28 deletions(-)

Shortlog
--------

Francois Romieu (1):
     r8169: don't request firmware when there's no userspace.

Patch
-----

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 493b0de..397c368 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -170,6 +170,16 @@ static const struct {
 };
 #undef _R
 
+static const struct rtl_firmware_info {
+	int mac_version;
+	const char *fw_name;
+} rtl_firmware_infos[] = {
+	{ .mac_version = RTL_GIGA_MAC_VER_25, .fw_name = FIRMWARE_8168D_1 },
+	{ .mac_version = RTL_GIGA_MAC_VER_26, .fw_name = FIRMWARE_8168D_2 },
+	{ .mac_version = RTL_GIGA_MAC_VER_29, .fw_name = FIRMWARE_8105E_1 },
+	{ .mac_version = RTL_GIGA_MAC_VER_30, .fw_name = FIRMWARE_8105E_1 }
+};
+
 enum cfg_version {
 	RTL_CFG_0 = 0x00,
 	RTL_CFG_1,
@@ -565,6 +575,7 @@ struct rtl8169_private {
 	u32 saved_wolopts;
 
 	const struct firmware *fw;
+#define RTL_FIRMWARE_UNKNOWN	ERR_PTR(-EAGAIN);
 };
 
 MODULE_AUTHOR("Realtek and the Linux r8169 crew <netdev@vger.kernel.org>");
@@ -1789,25 +1800,26 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
 
 static void rtl_release_firmware(struct rtl8169_private *tp)
 {
-	release_firmware(tp->fw);
-	tp->fw = NULL;
+	if (!IS_ERR_OR_NULL(tp->fw))
+		release_firmware(tp->fw);
+	tp->fw = RTL_FIRMWARE_UNKNOWN;
 }
 
-static int rtl_apply_firmware(struct rtl8169_private *tp, const char *fw_name)
+static void rtl_apply_firmware(struct rtl8169_private *tp)
 {
-	const struct firmware **fw = &tp->fw;
-	int rc = !*fw;
-
-	if (rc) {
-		rc = request_firmware(fw, fw_name, &tp->pci_dev->dev);
-		if (rc < 0)
-			goto out;
-	}
+	const struct firmware *fw = tp->fw;
 
 	/* TODO: release firmware once rtl_phy_write_fw signals failures. */
-	rtl_phy_write_fw(tp, *fw);
-out:
-	return rc;
+	if (!IS_ERR_OR_NULL(fw))
+		rtl_phy_write_fw(tp, fw);
+}
+
+static void rtl_apply_firmware_cond(struct rtl8169_private *tp, u8 reg, u16 val)
+{
+	if (rtl_readphy(tp, reg) != val)
+		netif_warn(tp, hw, tp->dev, "chipset not ready for firmware\n");
+	else
+		rtl_apply_firmware(tp);
 }
 
 static void rtl8169s_hw_phy_config(struct rtl8169_private *tp)
@@ -2246,10 +2258,8 @@ static void rtl8168d_1_hw_phy_config(struct rtl8169_private *tp)
 
 	rtl_writephy(tp, 0x1f, 0x0005);
 	rtl_writephy(tp, 0x05, 0x001b);
-	if ((rtl_readphy(tp, 0x06) != 0xbf00) ||
-	    (rtl_apply_firmware(tp, FIRMWARE_8168D_1) < 0)) {
-		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
-	}
+
+	rtl_apply_firmware_cond(tp, MII_EXPANSION, 0xbf00);
 
 	rtl_writephy(tp, 0x1f, 0x0000);
 }
@@ -2351,10 +2361,8 @@ static void rtl8168d_2_hw_phy_config(struct rtl8169_private *tp)
 
 	rtl_writephy(tp, 0x1f, 0x0005);
 	rtl_writephy(tp, 0x05, 0x001b);
-	if ((rtl_readphy(tp, 0x06) != 0xb300) ||
-	    (rtl_apply_firmware(tp, FIRMWARE_8168D_2) < 0)) {
-		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
-	}
+
+	rtl_apply_firmware_cond(tp, MII_EXPANSION, 0xb300);
 
 	rtl_writephy(tp, 0x1f, 0x0000);
 }
@@ -2474,8 +2482,7 @@ static void rtl8105e_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy(tp, 0x18, 0x0310);
 	msleep(100);
 
-	if (rtl_apply_firmware(tp, FIRMWARE_8105E_1) < 0)
-		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
+	rtl_apply_firmware(tp);
 
 	rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
 }
@@ -3237,6 +3244,8 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->timer.data = (unsigned long) dev;
 	tp->timer.function = rtl8169_phy_timer;
 
+	tp->fw = RTL_FIRMWARE_UNKNOWN;
+
 	rc = register_netdev(dev);
 	if (rc < 0)
 		goto err_out_msi_4;
@@ -3288,10 +3297,10 @@ static void __devexit rtl8169_remove_one(struct pci_dev *pdev)
 
 	cancel_delayed_work_sync(&tp->task);
 
-	rtl_release_firmware(tp);
-
 	unregister_netdev(dev);
 
+	rtl_release_firmware(tp);
+
 	if (pci_dev_run_wake(pdev))
 		pm_runtime_get_noresume(&pdev->dev);
 
@@ -3303,6 +3312,37 @@ static void __devexit rtl8169_remove_one(struct pci_dev *pdev)
 	pci_set_drvdata(pdev, NULL);
 }
 
+static void rtl_request_firmware(struct rtl8169_private *tp)
+{
+	int i;
+
+	/* Return early if the firmware is already loaded / cached. */
+	if (!IS_ERR(tp->fw))
+		goto out;
+
+	for (i = 0; i < ARRAY_SIZE(rtl_firmware_infos); i++) {
+		const struct rtl_firmware_info *info = rtl_firmware_infos + i;
+
+		if (info->mac_version == tp->mac_version) {
+			const char *name = info->fw_name;
+			int rc;
+
+			rc = request_firmware(&tp->fw, name, &tp->pci_dev->dev);
+			if (rc < 0) {
+				netif_warn(tp, ifup, tp->dev, "unable to load "
+					"firmware patch %s (%d)\n", name, rc);
+				goto out_disable_request_firmware;
+			}
+			goto out;
+		}
+	}
+
+out_disable_request_firmware:
+	tp->fw = NULL;
+out:
+	return;
+}
+
 static int rtl8169_open(struct net_device *dev)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
@@ -3334,11 +3374,13 @@ static int rtl8169_open(struct net_device *dev)
 
 	smp_mb();
 
+	rtl_request_firmware(tp);
+
 	retval = request_irq(dev->irq, rtl8169_interrupt,
 			     (tp->features & RTL_FEATURE_MSI) ? 0 : IRQF_SHARED,
 			     dev->name, dev);
 	if (retval < 0)
-		goto err_release_ring_2;
+		goto err_release_fw_2;
 
 	napi_enable(&tp->napi);
 
@@ -3359,7 +3401,8 @@ static int rtl8169_open(struct net_device *dev)
 out:
 	return retval;
 
-err_release_ring_2:
+err_release_fw_2:
+	rtl_release_firmware(tp);
 	rtl8169_rx_clear(tp);
 err_free_rx_1:
 	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
-- 
Ueimor

^ permalink raw reply related

* [PATCH] net: Remove __KERNEL__ cpp checks from include/net
From: David Miller @ 2011-04-24 18:05 UTC (permalink / raw)
  To: netdev


These header files are never installed to user consumption, so any
__KERNEL__ cpp checks are superfluous.

Projects should also not copy these files into their userland utility
sources and try to use them there.  If they insist on doing so, the
onus is on them to sanitize the headers as needed.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/addrconf.h                     |    3 ---
 include/net/af_rxrpc.h                     |    3 ---
 include/net/af_unix.h                      |    2 --
 include/net/atmclip.h                      |    2 --
 include/net/bluetooth/hci.h                |    2 --
 include/net/dst.h                          |    3 ---
 include/net/if_inet6.h                     |    3 ---
 include/net/ip6_fib.h                      |    3 ---
 include/net/ip6_route.h                    |    3 ---
 include/net/ip_vs.h                        |    5 -----
 include/net/ipv6.h                         |    7 -------
 include/net/ipx.h                          |    2 --
 include/net/ndisc.h                        |    6 ------
 include/net/netevent.h                     |    2 --
 include/net/netfilter/nf_conntrack.h       |    2 --
 include/net/netfilter/nf_conntrack_tuple.h |    4 ----
 include/net/netfilter/nf_nat.h             |    4 ----
 include/net/rawv6.h                        |    4 ----
 include/net/route.h                        |    4 ----
 include/net/transp_v6.h                    |    4 ----
 include/net/wimax.h                        |    5 -----
 21 files changed, 0 insertions(+), 73 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 7c4d92c..582e4ae 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -42,8 +42,6 @@ struct prefix_info {
 };
 
 
-#ifdef __KERNEL__
-
 #include <linux/netdevice.h>
 #include <net/if_inet6.h>
 #include <net/ipv6.h>
@@ -285,4 +283,3 @@ extern void if6_proc_exit(void);
 #endif
 
 #endif
-#endif
diff --git a/include/net/af_rxrpc.h b/include/net/af_rxrpc.h
index 00c2eaa..03e6e94 100644
--- a/include/net/af_rxrpc.h
+++ b/include/net/af_rxrpc.h
@@ -12,8 +12,6 @@
 #ifndef _NET_RXRPC_H
 #define _NET_RXRPC_H
 
-#ifdef __KERNEL__
-
 #include <linux/rxrpc.h>
 
 struct rxrpc_call;
@@ -53,5 +51,4 @@ extern struct rxrpc_call *rxrpc_kernel_accept_call(struct socket *,
 						   unsigned long);
 extern int rxrpc_kernel_reject_call(struct socket *);
 
-#endif /* __KERNEL__ */
 #endif /* _NET_RXRPC_H */
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 18e5c3f..91ab5b0 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -41,7 +41,6 @@ struct unix_skb_parms {
 				spin_lock_nested(&unix_sk(s)->lock, \
 				SINGLE_DEPTH_NESTING)
 
-#ifdef __KERNEL__
 /* The AF_UNIX socket */
 struct unix_sock {
 	/* WARNING: sk has to be the first member */
@@ -72,4 +71,3 @@ static inline int unix_sysctl_register(struct net *net) { return 0; }
 static inline void unix_sysctl_unregister(struct net *net) {}
 #endif
 #endif
-#endif
diff --git a/include/net/atmclip.h b/include/net/atmclip.h
index 467c531..497ef64 100644
--- a/include/net/atmclip.h
+++ b/include/net/atmclip.h
@@ -54,8 +54,6 @@ struct clip_priv {
 };
 
 
-#ifdef __KERNEL__
 extern struct neigh_table *clip_tbl_hook;
-#endif
 
 #endif
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 6138e31..499b7b7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1067,7 +1067,6 @@ struct hci_sco_hdr {
 	__u8	dlen;
 } __packed;
 
-#ifdef __KERNEL__
 #include <linux/skbuff.h>
 static inline struct hci_event_hdr *hci_event_hdr(const struct sk_buff *skb)
 {
@@ -1083,7 +1082,6 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
 {
 	return (struct hci_sco_hdr *) skb->data;
 }
-#endif
 
 /* Command opcode pack/unpack */
 #define hci_opcode_pack(ogf, ocf)	(__u16) ((ocf & 0x03ff)|(ogf << 10))
diff --git a/include/net/dst.h b/include/net/dst.h
index 75b95df..d7bb740 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -92,8 +92,6 @@ struct dst_entry {
 	};
 };
 
-#ifdef __KERNEL__
-
 extern u32 *dst_cow_metrics_generic(struct dst_entry *dst, unsigned long old);
 extern const u32 dst_default_metrics[RTAX_MAX];
 
@@ -438,6 +436,5 @@ extern struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig
 				     const struct flowi *fl, struct sock *sk,
 				     int flags);
 #endif
-#endif
 
 #endif /* _NET_DST_H */
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 3d982f7..0c603fe 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -30,8 +30,6 @@
 #define IF_PREFIX_ONLINK	0x01
 #define IF_PREFIX_AUTOCONF	0x02
 
-#ifdef __KERNEL__
-
 enum {
 	INET6_IFADDR_STATE_DAD,
 	INET6_IFADDR_STATE_POSTDAD,
@@ -303,4 +301,3 @@ static inline int ipv6_ipgre_mc_map(const struct in6_addr *addr,
 }
 
 #endif
-#endif
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index aca8ef4..477ef75 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -13,8 +13,6 @@
 #ifndef _IP6_FIB_H
 #define _IP6_FIB_H
 
-#ifdef __KERNEL__
-
 #include <linux/ipv6_route.h>
 #include <linux/rtnetlink.h>
 #include <linux/spinlock.h>
@@ -240,4 +238,3 @@ static inline void              fib6_rules_cleanup(void)
 }
 #endif
 #endif
-#endif
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index d5c21d4..5e91b72 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -21,8 +21,6 @@ struct route_info {
 	__u8			prefix[0];	/* 0,8 or 16 */
 };
 
-#ifdef __KERNEL__
-
 #include <net/flow.h>
 #include <net/ip6_fib.h>
 #include <net/sock.h>
@@ -193,4 +191,3 @@ static inline int ip6_skb_dst_mtu(struct sk_buff *skb)
 }
 
 #endif
-#endif
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index d516f00..e0b7f13 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -8,9 +8,6 @@
 
 #include <linux/ip_vs.h>                /* definitions shared with userland */
 
-/* old ipvsadm versions still include this file directly */
-#ifdef __KERNEL__
-
 #include <asm/types.h>                  /* for __uXX types */
 
 #include <linux/sysctl.h>               /* for ctl_path */
@@ -1415,6 +1412,4 @@ ip_vs_dest_conn_overhead(struct ip_vs_dest *dest)
 		atomic_read(&dest->inactconns);
 }
 
-#endif /* __KERNEL__ */
-
 #endif	/* _NET_IP_VS_H */
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 5da1926..e1c60b4 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -77,11 +77,9 @@
 /*
  *	Addr scopes
  */
-#ifdef __KERNEL__
 #define IPV6_ADDR_MC_SCOPE(a)	\
 	((a)->s6_addr[1] & 0x0f)	/* nonstandard */
 #define __IPV6_ADDR_SCOPE_INVALID	-1
-#endif
 #define IPV6_ADDR_SCOPE_NODELOCAL	0x01
 #define IPV6_ADDR_SCOPE_LINKLOCAL	0x02
 #define IPV6_ADDR_SCOPE_SITELOCAL	0x05
@@ -91,14 +89,12 @@
 /*
  *	Addr flags
  */
-#ifdef __KERNEL__
 #define IPV6_ADDR_MC_FLAG_TRANSIENT(a)	\
 	((a)->s6_addr[1] & 0x10)
 #define IPV6_ADDR_MC_FLAG_PREFIX(a)	\
 	((a)->s6_addr[1] & 0x20)
 #define IPV6_ADDR_MC_FLAG_RENDEZVOUS(a)	\
 	((a)->s6_addr[1] & 0x40)
-#endif
 
 /*
  *	fragmentation header
@@ -113,8 +109,6 @@ struct frag_hdr {
 
 #define	IP6_MF	0x0001
 
-#ifdef __KERNEL__
-
 #include <net/sock.h>
 
 /* sysctls */
@@ -667,5 +661,4 @@ extern int ipv6_static_sysctl_register(void);
 extern void ipv6_static_sysctl_unregister(void);
 #endif
 
-#endif /* __KERNEL__ */
 #endif /* _NET_IPV6_H */
diff --git a/include/net/ipx.h b/include/net/ipx.h
index 05d7e4a..c1fec6b 100644
--- a/include/net/ipx.h
+++ b/include/net/ipx.h
@@ -80,7 +80,6 @@ struct ipx_route {
 	atomic_t		refcnt;
 };
 
-#ifdef __KERNEL__
 struct ipx_cb {
 	u8	ipx_tctrl;
 	__be32	ipx_dest_net;
@@ -116,7 +115,6 @@ static inline struct ipx_sock *ipx_sk(struct sock *sk)
 }
 
 #define IPX_SKB_CB(__skb) ((struct ipx_cb *)&((__skb)->cb[0]))
-#endif
 
 #define IPX_MIN_EPHEMERAL_SOCKET	0x4000
 #define IPX_MAX_EPHEMERAL_SOCKET	0x7fff
diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index 6144685..62beeb9 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -42,8 +42,6 @@ enum {
 #define ND_REACHABLE_TIME		(30*HZ)
 #define ND_RETRANS_TIMER		HZ
 
-#ifdef __KERNEL__
-
 #include <linux/compiler.h>
 #include <linux/icmpv6.h>
 #include <linux/in6.h>
@@ -156,8 +154,4 @@ static inline struct neighbour * ndisc_get_neigh(struct net_device *dev, const s
 	return ERR_PTR(-ENODEV);
 }
 
-
-#endif /* __KERNEL__ */
-
-
 #endif
diff --git a/include/net/netevent.h b/include/net/netevent.h
index 22b239c..086f8a5 100644
--- a/include/net/netevent.h
+++ b/include/net/netevent.h
@@ -10,7 +10,6 @@
  *
  * 	Changes:
  */
-#ifdef __KERNEL__
 
 struct dst_entry;
 
@@ -29,4 +28,3 @@ extern int unregister_netevent_notifier(struct notifier_block *nb);
 extern int call_netevent_notifiers(unsigned long val, void *v);
 
 #endif
-#endif
diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index d0d1337..c7c42e7 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -14,7 +14,6 @@
 
 #include <linux/netfilter/nf_conntrack_common.h>
 
-#ifdef __KERNEL__
 #include <linux/bitops.h>
 #include <linux/compiler.h>
 #include <asm/atomic.h>
@@ -326,5 +325,4 @@ do {							\
 #define MODULE_ALIAS_NFCT_HELPER(helper) \
         MODULE_ALIAS("nfct-helper-" helper)
 
-#endif /* __KERNEL__ */
 #endif /* _NF_CONNTRACK_H */
diff --git a/include/net/netfilter/nf_conntrack_tuple.h b/include/net/netfilter/nf_conntrack_tuple.h
index 4ee44c8..7ca6bdd 100644
--- a/include/net/netfilter/nf_conntrack_tuple.h
+++ b/include/net/netfilter/nf_conntrack_tuple.h
@@ -104,8 +104,6 @@ struct nf_conntrack_tuple_mask {
 	} src;
 };
 
-#ifdef __KERNEL__
-
 static inline void nf_ct_dump_tuple_ip(const struct nf_conntrack_tuple *t)
 {
 #ifdef DEBUG
@@ -148,8 +146,6 @@ struct nf_conntrack_tuple_hash {
 	struct nf_conntrack_tuple tuple;
 };
 
-#endif /* __KERNEL__ */
-
 static inline bool __nf_ct_tuple_src_equal(const struct nf_conntrack_tuple *t1,
 					   const struct nf_conntrack_tuple *t2)
 { 
diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
index aff80b1..0346b00 100644
--- a/include/net/netfilter/nf_nat.h
+++ b/include/net/netfilter/nf_nat.h
@@ -48,7 +48,6 @@ struct nf_nat_multi_range_compat {
 	struct nf_nat_range range[1];
 };
 
-#ifdef __KERNEL__
 #include <linux/list.h>
 #include <linux/netfilter/nf_conntrack_pptp.h>
 #include <net/netfilter/nf_conntrack_extend.h>
@@ -93,7 +92,4 @@ static inline struct nf_conn_nat *nfct_nat(const struct nf_conn *ct)
 #endif
 }
 
-#else  /* !__KERNEL__: iptables wants this to compile. */
-#define nf_nat_multi_range nf_nat_multi_range_compat
-#endif /*__KERNEL__*/
 #endif
diff --git a/include/net/rawv6.h b/include/net/rawv6.h
index f6b9b83..cf75772 100644
--- a/include/net/rawv6.h
+++ b/include/net/rawv6.h
@@ -1,8 +1,6 @@
 #ifndef _NET_RAWV6_H
 #define _NET_RAWV6_H
 
-#ifdef __KERNEL__
-
 #include <net/protocol.h>
 
 void raw6_icmp_error(struct sk_buff *, int nexthdr,
@@ -20,5 +18,3 @@ int rawv6_mh_filter_unregister(int (*filter)(struct sock *sock,
 #endif
 
 #endif
-
-#endif
diff --git a/include/net/route.h b/include/net/route.h
index b3962e2..3684c3e 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -35,10 +35,6 @@
 #include <linux/cache.h>
 #include <linux/security.h>
 
-#ifndef __KERNEL__
-#warning This file is not supposed to be used outside of kernel.
-#endif
-
 #define RTO_ONLINK	0x01
 
 #define RTO_CONN	0
diff --git a/include/net/transp_v6.h b/include/net/transp_v6.h
index a8122dc..5271a74 100644
--- a/include/net/transp_v6.h
+++ b/include/net/transp_v6.h
@@ -7,8 +7,6 @@
  *	IPv6 transport protocols
  */
 
-#ifdef __KERNEL__
-
 extern struct proto rawv6_prot;
 extern struct proto udpv6_prot;
 extern struct proto udplitev6_prot;
@@ -57,5 +55,3 @@ extern const struct inet_connection_sock_af_ops ipv4_specific;
 extern void inet6_destroy_sock(struct sock *sk);
 
 #endif
-
-#endif
diff --git a/include/net/wimax.h b/include/net/wimax.h
index c799ba7..7328d50 100644
--- a/include/net/wimax.h
+++ b/include/net/wimax.h
@@ -250,7 +250,6 @@
 
 #ifndef __NET__WIMAX_H__
 #define __NET__WIMAX_H__
-#ifdef __KERNEL__
 
 #include <linux/wimax.h>
 #include <net/genetlink.h>
@@ -518,8 +517,4 @@ extern ssize_t wimax_msg_len(struct sk_buff *);
 extern int wimax_rfkill(struct wimax_dev *, enum wimax_rf_state);
 extern int wimax_reset(struct wimax_dev *);
 
-#else
-/* You might be looking for linux/wimax.h */
-#error This file should not be included from user space.
-#endif /* #ifdef __KERNEL__ */
 #endif /* #ifndef __NET__WIMAX_H__ */
-- 
1.7.4.3


^ permalink raw reply related

* (unknown), 
From: Radka Jaksova @ 2011-04-24 10:36 UTC (permalink / raw)


I kindly want you to receive funds on my behalf,When you reply this message i will send you full details and more information about myself and the funds.
Warm Regards From Malta.
Mr. Butler James.
Email:butler.james@gncn.net

^ permalink raw reply

* Re: [PATCH net-2.6 1/1] r8169 driver updates
From: David Miller @ 2011-04-24 18:53 UTC (permalink / raw)
  To: romieu; +Cc: netdev, docan, torvalds, nic_swsd
In-Reply-To: <20110424163243.GA19376@electric-eye.fr.zoreil.com>

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sun, 24 Apr 2011 18:32:43 +0200

> Please pull from branch 'davem.r8169' in repository
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/romieu/netdev-2.6.git davem.r8169
> 
> to get the change below.

Pulled, thanks a lot!

^ permalink raw reply

* Re: [PATCH] af_unix: Only allow recv on connected seqpacket sockets.
From: David Miller @ 2011-04-24 19:05 UTC (permalink / raw)
  To: ebiederm; +Cc: netdev, xemul, dan, stable
In-Reply-To: <m1sjt7sw32.fsf_-_@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Sun, 24 Apr 2011 04:54:57 -0700

> +static int unix_seqpacket_recvmsg(struct kiocb *iocb, struct socket *sock,
> +			      struct msghdr *msg, size_t size,
> +			      int flags)
> +{
> +	struct sock *sk = sock->sk;
> +
> +	if (sk->sk_state != TCP_ESTABLISHED)
> +		return -ENOTCONN;

As for unix_seqpacket_sendmsg(), you need to add a check for sock_error()
or similar here otherwise -ECONNRESET is not reported correctly.

In fact, recvmsg() is even harder than sendmsg() to handle correctly,
because we have to also properly report EOF on seqpacket sockets which
have RCV_SHUTDOWN set.

So a lot more work has to go into this change to make it fix the bug
without also breaking existing semantics.

Anyways, see:

commit 6e14891f4d16f8a9e0bc3a8408f73b3aed93ab0a
Author: James Morris <jmorris@redhat.com>
Date:   Fri Nov 19 07:02:41 2004 -0800

    [AF_UNIX]: Don't lose ECONNRESET in unix_seqpacket_sendmsg()
    
    The fix for SELinux w/SOCK_SEQPACKET had an error,
    noted by Alan Cox.  This fixes it.
    
    Signed-off-by: James Morris <jmorris@redhat.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 16faa9d..8902c4a 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1513,13 +1513,18 @@ out_err:
 static int unix_seqpacket_sendmsg(struct kiocb *kiocb, struct socket *sock,
 				  struct msghdr *msg, size_t len)
 {
+	int err;
 	struct sock *sk = sock->sk;
 	
+	err = sock_error(sk);
+	if (err)
+		return err;
+
 	if (sk->sk_state != TCP_ESTABLISHED)
 		return -ENOTCONN;
 
-	if (msg->msg_name || msg->msg_namelen)
-		return -EINVAL;
+	if (msg->msg_namelen)
+		msg->msg_namelen = 0;
 
 	return unix_dgram_sendmsg(kiocb, sock, msg, len);
 }

^ permalink raw reply related

* Oops in 2.6.39 include/net/dst.h: dst_metrics_write_ptr() running l2tp over ipsec
From: Held Bernhard @ 2011-04-24 22:37 UTC (permalink / raw)
  To: linux-kernel, David S. Miller, netdev

Hi,

I'm starting l2tp over ipsec (racoon, openl2tp) in a little script to 
establish a VPN to my company. 2.6.38.x runs fine, but since 2.6.39-rc1 
(exactly commit 62fa8a "net: Implement read-only protection and COW'ing 
of metrics.") the kernel throws an oops. openl2tp is killed; after a 2nd 
start of openl2tp the VPN is established and my PC continues to run 
normally. The oops is 100% reproducible.

[  101.620985] BUG: unable to handle kernel NULL pointer dereference at 
           (null)
[  101.621050] IP: [<          (null)>]           (null)
[  101.621084] PGD 6e53c067 PUD 3dd6a067 PMD 0
[  101.621122] Oops: 0010 [#1] SMP
[  101.621153] last sysfs file: /sys/devices/virtual/ppp/ppp/uevent
[  101.621192] CPU 2
[  101.621206] Modules linked in: l2tp_ppp pppox ppp_generic slhc 
l2tp_netlink l2tp_core deflate zlib_deflate twofish_x86_64 
twofish_common des_generic cbc ecb sha1_generic hmac af_key 
iptable_filter snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device loop 
snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_intel snd_hda_codec 
snd_pcm snd_timer snd i2c_i801 iTCO_wdt psmouse soundcore snd_page_alloc 
evdev uhci_hcd ehci_hcd thermal
[  101.621552]
[  101.621567] Pid: 5129, comm: openl2tpd Not tainted 2.6.39-rc4-Quad #3 
Gigabyte Technology Co., Ltd. G33-DS3R/G33-DS3R
[  101.621637] RIP: 0010:[<0000000000000000>]  [<          (null)>]   (null)
[  101.621684] RSP: 0018:ffff88003ddeba60  EFLAGS: 00010202
[  101.621716] RAX: ffff88003ddb5600 RBX: ffff88003ddb5600 RCX: 
0000000000000020
[  101.621758] RDX: ffffffff81a69a00 RSI: ffffffff81b7ee61 RDI: 
ffff88003ddb5600
[  101.621800] RBP: ffff8800537cd900 R08: 0000000000000000 R09: 
ffff88003ddb5600
[  101.621840] R10: 0000000000000005 R11: 0000000000014b38 R12: 
ffff88003ddb5600
[  101.621881] R13: ffffffff81b7e480 R14: ffffffff81b7e8b8 R15: 
ffff88003ddebad8
[  101.621924] FS:  00007f06e4182700(0000) GS:ffff88007fd00000(0000) 
knlGS:0000000000000000
[  101.621971] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  101.622005] CR2: 0000000000000000 CR3: 0000000045274000 CR4: 
00000000000006e0
[  101.622046] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
0000000000000000
[  101.622087] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 
0000000000000400
[  101.622129] Process openl2tpd (pid: 5129, threadinfo 
ffff88003ddea000, task ffff88003de9a280)
[  101.622177] Stack:
[  101.622191]  ffffffff81447efa ffff88007d3ded80 ffff88003de9a280 
ffff88007d3ded80
[  101.622245]  0000000000000001 ffff88003ddebbb8 ffffffff8148d5a7 
0000000000000212
[  101.622299]  ffff88003dcea000 ffff88003dcea188 ffffffff00000001 
ffffffff81b7e480
[  101.622353] Call Trace:
[  101.622374]  [<ffffffff81447efa>] ? ipv4_blackhole_route+0x1ba/0x210
[  101.622415]  [<ffffffff8148d5a7>] ? xfrm_lookup+0x417/0x510
[  101.622450]  [<ffffffff8127672a>] ? extract_buf+0x9a/0x140
[  101.622485]  [<ffffffff8144c6a0>] ? __ip_flush_pending_frames+0x70/0x70
[  101.622526]  [<ffffffff8146fbbf>] ? udp_sendmsg+0x62f/0x810
[  101.622562]  [<ffffffff813f98a6>] ? sock_sendmsg+0x116/0x130
[  101.622599]  [<ffffffff8109df58>] ? find_get_page+0x18/0x90
[  101.622633]  [<ffffffff8109fd6a>] ? filemap_fault+0x12a/0x4b0
[  101.622668]  [<ffffffff813fb5c4>] ? move_addr_to_kernel+0x64/0x90
[  101.622706]  [<ffffffff81405d5a>] ? verify_iovec+0x7a/0xf0
[  101.622739]  [<ffffffff813fc772>] ? sys_sendmsg+0x292/0x420
[  101.622774]  [<ffffffff810b994a>] ? handle_pte_fault+0x8a/0x7c0
[  101.622810]  [<ffffffff810b76fe>] ? __pte_alloc+0xae/0x130
[  101.622844]  [<ffffffff810ba2f8>] ? handle_mm_fault+0x138/0x380
[  101.622880]  [<ffffffff81024af9>] ? do_page_fault+0x189/0x410
[  101.622915]  [<ffffffff813fbe03>] ? sys_getsockname+0xf3/0x110
[  101.622952]  [<ffffffff81450c4d>] ? ip_setsockopt+0x4d/0xa0
[  101.622986]  [<ffffffff813f9932>] ? sockfd_lookup_light+0x22/0x90
[  101.623024]  [<ffffffff814b61fb>] ? system_call_fastpath+0x16/0x1b
[  101.623060] Code:  Bad RIP value.
[  101.623090] RIP  [<          (null)>]           (null)
[  101.623125]  RSP <ffff88003ddeba60>
[  101.623146] CR2: 0000000000000000
[  101.650871] ---[ end trace ca3856a7d8e8dad4 ]---
[  101.651011] __sk_free: optmem leakage (160 bytes) detected.

The oops happens in dst_metrics_write_ptr()
include/net/dst.h:124: return dst->ops->cow_metrics(dst, p);

dst->ops->cow_metrics is NULL and causes the oops.

This extremely primitive (and most probably wrong) patch fixes the 
problem for me:

--- include/net/dst.h   2011-04-24 23:10:15.985686594 +0200
+++ include/net/dst.h   2011-04-24 23:44:43.897686223 +0200
@@ -121,7 +121,7 @@ static inline u32 *dst_metrics_write_ptr
         unsigned long p = dst->_metrics;

         if (p & DST_METRICS_READ_ONLY)
-               return dst->ops->cow_metrics(dst, p);
+               return dst->ops->cow_metrics ? 
dst->ops->cow_metrics(dst, p) : NULL;
         return __DST_METRICS_PTR(p);
  }

Please tell me if you need more information!

Thanks,
Bernhard

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox