Netdev List
 help / color / mirror / Atom feed
* Re: Bypass flow control problems
From: Pasi Kärkkäinen @ 2010-12-29  8:47 UTC (permalink / raw)
  To: Kirsher, Jeffrey T; +Cc: Alkis Georgopoulos, netdev@vger.kernel.org
In-Reply-To: <5BBF316A-A162-4683-B6CA-6662F221F500@intel.com>

On Tue, Dec 28, 2010 at 09:05:42PM -0800, Kirsher, Jeffrey T wrote:
> On Dec 27, 2010, at 8:43, Pasi Kärkkäinen <pasik@iki.fi> wrote:
> 
> > On Wed, Dec 22, 2010 at 07:51:31PM +0200, Alkis Georgopoulos wrote:
> >> Hi,
> >> 
> >> I'm an IT teacher/LTSP developer. In LTSP, thin clients are netbooted
> >> from a server and receive a lot of X and remote disk traffic from it.
> >> 
> >> Many installations have a gigabit NIC on the server, an unmanaged
> >> gigabit switch, and 100 Mbps NICs on the clients.
> >> 
> >> With flow control on, the server is limited to sending 100 Mbps to all
> >> the clients. So with 10 thin clients the server can concurrently send
> >> only 10 Mbps to each one of them.
> >> 
> >> On NICs that support it, we turn flow control off and the server can
> >> properly send 100 Mbps to each client, i.e. 1 Gbps to 10 clients.
> >> 
> >> * Is there any way to bypass that problem on NICs that do not support
> >>   turning off flow control, like e.g. realteks?
> >>   I.e. when a client sends a pause signal to the server, instead of the
> >>   server pausing, to continue sending data to another client?
> >>   Or even to limit the amound of data the server sends to each client,
> >>   so that the clients never have to send pause signals?
> >> 
> > 
> > You could set up QoS rules on the server to limit the network speed per client..
> > 
> >> * I really don't understand why flow control is enabled by default on
> >>   NICs and switches. In which case does it help? As far as I
> >>   understand, all it does is ruin gigabit => 100 Mbps connections...
> >> 
> >> * As a side note, since rtl8169 is a very common chipset, is there a
> >>   way to disable flow control for that specific NIC?
> >> 
> >> This problem affects thousands of LTSP installations, we'd much
> >> appreciate your knowledge and feedback on it.
> >> 
> > 
> > Did you try disabling flow control from the switch? 
> 
> He stated the switch was a un-managed switch, so he would be unable to disable flow control on the switch.
> 

Oh, I missed that :) Never mind then.

-- Pasi


^ permalink raw reply

* Re: Bypass flow control problems
From: Alkis Georgopoulos @ 2010-12-29  8:47 UTC (permalink / raw)
  To: Pasi Kärkkäinen; +Cc: netdev
In-Reply-To: <20101227134346.GD2754@reaktio.net>

Στις 27-12-2010, ημέρα Δευ, και ώρα 15:43 +0200, ο/η Pasi Kärkkäinen
έγραψε:
> You could set up QoS rules on the server to limit the network speed per client..

That sounds promising. I'll try to limit the rate of packages that the
server sends to each client to 90 Mbps and see if that works around the
flow control problem. Any tips for a good method to do that?

Thank you.


^ permalink raw reply

* [PATCH v3 net-next-2.6] sch_sfq: allow big packets and be fair
From: Eric Dumazet @ 2010-12-29  7:53 UTC (permalink / raw)
  To: David Miller; +Cc: jarkao2, kaber, netdev
In-Reply-To: <20101228.134655.112609314.davem@davemloft.net>

Le mardi 28 décembre 2010 à 13:46 -0800, David Miller a écrit :
> Eric this doesn't apply cleanly, since the code in sfq_dump_class_stats()
> is a bit different in net-next-2.6
> 
> Please respin this and resubmit.

Sure, here is the version for latest net-next

Thanks

[PATCH v3 net-next-2.6] sch_sfq: allow big packets and be fair

SFQ is currently 'limited' to small packets, because it uses a 15bit
allotment number per flow. Introduce a scale by 8, so that we can handle
full size TSO/GRO packets.

Use appropriate handling to make sure allot is positive before a new
packet is dequeued, so that fairness is respected.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Jarek Poplawski <jarkao2@gmail.com>
Cc: Patrick McHardy <kaber@trash.net>
---
v3: respin after commit ee09b3c1cf (fix sfq class stats handling)
v2: Use a scale of 8 as Jarek suggested, instead of 18bit fields

 net/sched/sch_sfq.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 0ce421d..79c8967 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -67,7 +67,7 @@
 
 	IMPLEMENTATION:
 	This implementation limits maximal queue length to 128;
-	maximal mtu to 2^15-1; max 128 flows, number of hash buckets to 1024.
+	max mtu to 2^18-1; max 128 flows, number of hash buckets to 1024.
 	The only goal of this restrictions was that all data
 	fit into one 4K page on 32bit arches.
 
@@ -77,6 +77,11 @@
 #define SFQ_SLOTS		128 /* max number of flows */
 #define SFQ_EMPTY_SLOT		255
 #define SFQ_HASH_DIVISOR	1024
+/* We use 16 bits to store allot, and want to handle packets up to 64K
+ * Scale allot by 8 (1<<3) so that no overflow occurs.
+ */
+#define SFQ_ALLOT_SHIFT		3
+#define SFQ_ALLOT_SIZE(X)	DIV_ROUND_UP(X, 1 << SFQ_ALLOT_SHIFT)
 
 /* This type should contain at least SFQ_DEPTH + SFQ_SLOTS values */
 typedef unsigned char sfq_index;
@@ -115,7 +120,7 @@ struct sfq_sched_data
 	struct timer_list perturb_timer;
 	u32		perturbation;
 	sfq_index	cur_depth;	/* depth of longest slot */
-
+	unsigned short  scaled_quantum; /* SFQ_ALLOT_SIZE(quantum) */
 	struct sfq_slot *tail;		/* current slot in round */
 	sfq_index	ht[SFQ_HASH_DIVISOR];	/* Hash table */
 	struct sfq_slot	slots[SFQ_SLOTS];
@@ -395,7 +400,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 			q->tail->next = x;
 		}
 		q->tail = slot;
-		slot->allot = q->quantum;
+		slot->allot = q->scaled_quantum;
 	}
 	if (++sch->q.qlen <= q->limit) {
 		sch->bstats.bytes += qdisc_pkt_len(skb);
@@ -431,8 +436,14 @@ sfq_dequeue(struct Qdisc *sch)
 	if (q->tail == NULL)
 		return NULL;
 
+next_slot:
 	a = q->tail->next;
 	slot = &q->slots[a];
+	if (slot->allot <= 0) {
+		q->tail = slot;
+		slot->allot += q->scaled_quantum;
+		goto next_slot;
+	}
 	skb = slot_dequeue_head(slot);
 	sfq_dec(q, a);
 	sch->q.qlen--;
@@ -447,9 +458,8 @@ sfq_dequeue(struct Qdisc *sch)
 			return skb;
 		}
 		q->tail->next = next_a;
-	} else if ((slot->allot -= qdisc_pkt_len(skb)) <= 0) {
-		q->tail = slot;
-		slot->allot += q->quantum;
+	} else {
+		slot->allot -= SFQ_ALLOT_SIZE(qdisc_pkt_len(skb));
 	}
 	return skb;
 }
@@ -485,6 +495,7 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
 
 	sch_tree_lock(sch);
 	q->quantum = ctl->quantum ? : psched_mtu(qdisc_dev(sch));
+	q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
 	q->perturb_period = ctl->perturb_period * HZ;
 	if (ctl->limit)
 		q->limit = min_t(u32, ctl->limit, SFQ_DEPTH - 1);
@@ -525,6 +536,7 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
 	q->tail = NULL;
 	if (opt == NULL) {
 		q->quantum = psched_mtu(qdisc_dev(sch));
+		q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
 		q->perturb_period = 0;
 		q->perturbation = net_random();
 	} else {
@@ -617,7 +629,7 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
 	if (idx != SFQ_EMPTY_SLOT) {
 		const struct sfq_slot *slot = &q->slots[idx];
 
-		xstats.allot = slot->allot;
+		xstats.allot = slot->allot << SFQ_ALLOT_SHIFT;
 		qs.qlen = slot->qlen;
 		slot_queue_walk(slot, skb)
 			qs.backlog += qdisc_pkt_len(skb);



^ permalink raw reply related

* Re: dm9000 patch
From: Baruch Siach @ 2010-12-29  6:06 UTC (permalink / raw)
  To: Angelo Dureghello; +Cc: netdev, linux-kernel
In-Reply-To: <4D1A5C2A.5050606@gmail.com>

Hi Angelo,

On Tue, Dec 28, 2010 at 10:52:42PM +0100, Angelo Dureghello wrote:
> sorry to contact you directly but i couldn't get any help from the
> kernel.org mailing list, since i am not a developer my mails are
> generally skipped.

The best way to get the contact info for a piece of kernel code, is using the 
get_maintainer.pl script. Running 'scripts/get_maintainer.pl -f 
drivers/net/dm9000.c' gives the following output:

netdev@vger.kernel.org
linux-kernel@vger.kernel.org

I added both to Cc.

> I am very near to have a custom board working with MCF5307 cpu and dm9000.
> I am using kernel 2.6.36-rc3 with your last patch about
> spinlock-recursion already included.

You should try to update to the latest .36 kernel, which is currently 
2.6.36.2. The problem that you experience might be unrelated to the dm9000 
driver (or to networking at all), so it might have been fixed in this version.

> I have "ping" and "telnet" to the embedded board fully working.
> If i try to get a sample web page with some images from the board
> httpd with a browser, in 80% of cases i get a trap/oops:

Try to enable KALLSYMS in your kernel .config to make your stack trace more 
meaningful. This is under 'General setup -> Configure standard kernel features 
(for small systems) -> Load all symbols for debugging/ksymoops'.

I hope this helps.

baruch

> [    4.590000] eth0: link up, 100Mbps, full-duplex, lpa 0x45E1
> [   67.630000] BUG: spinlock recursion on CPU#0, httpd/29
> [   67.630000]  lock: 00c42c06, .magic: dead4ead, .owner: httpd/29,
> .owner_cpu: 0
> [   67.630000] Stack from 00d7b914:
> [   67.630000]         00d7b940 000a8cf0 0015f693 00c42c06 dead4ead
> 00dec1d4 0000001d 00000000
> [   67.630000]         00c42c06 00006188 00c42800 00d7b974 000a8ec2
> 00c42c06 0015f6f9 00002704
> [   67.630000]         00000000 0000001f 00146fa4 00152f0c 00c42b60
> 00006188 00c42800 0002b312
> [   67.630000]         00d7b984 0014701e 00c42c06 00000000 00d7b9c4
> 000df21c 00c42c06 00000000
> [   67.630000]         00000000 0000001f 00146fa4 00152f0c 000005ea
> 00cfc640 00006188 000096e8
> [   67.630000]         0002b312 00146fa4 00c42b60 00002704 00d7b9ec
> 00029d3a 0000001f 00c42800
> [   67.630000] Call Trace:
> [   67.630000]  [000a8cf0]  [000a8ec2]  [0014701e]  [000df21c]  [00029d3a]
> [   67.630000]  [00029e84]  [00000bb6]  [0000336e]  [000df162]  [000effd6]
> [   67.630000]  [00100482]  [000f312e]  [000f9ebc]  [0010dd2a]  [0010e4a0]
> [   67.630000]  [0010dfb2]  [0010ef80]  [0011fed6]  [00121170]  [0012188e]
> [   67.630000]  [0011ecc6]  [001249fe]  [000e4084]  [0011621c]  [00131a44]
> [   67.630000]  [000e11ee]  [00041944]  [00041a1c]  [00041e46]  [00003218]
> [   67.630000] BUG: spinlock lockup on CPU#0, httpd/29, 00c42c06
> [   67.630000] Stack from 00d7b934:
> [   67.630000]         00d7b974 000a8f66 0015f703 00000000 00dec1d4
> 0000001d 00c42c06 00002704
> [   67.630000]         00000000 0000001f 00146fa4 00152f0c 00c42b60
> 00006188 00c42800 0002b312
> [   67.630000]         00d7b984 0014701e 00c42c06 00000000 00d7b9c4
> 000df21c 00c42c06 00000000
> [   67.630000]         00000000 0000001f 00146fa4 00152f0c 000005ea
> 00cfc640 00006188 000096e8
> [   67.630000]         0002b312 00146fa4 00c42b60 00002704 00d7b9ec
> 00029d3a 0000001f 00c42800
> [   67.630000]         0016c1b4 00cfc640 0000001f 0016c178 00029d10
> 00146fb8 00d7ba20 00029e84
> [   67.630000] Call Trace:
> [   67.630000]  [000a8f66]  [0014701e]  [000df21c]  [00029d3a]  [00029e84]
> [   67.630000]  [00000bb6]  [0000336e]  [000df162]  [000effd6]  [00100482]
> [   67.630000]  [000f312e]  [000f9ebc]  [0010dd2a]  [0010e4a0]  [0010dfb2]
> [   67.630000]  [0010ef80]  [0011fed6]  [00121170]  [0012188e]  [0011ecc6]
> [   67.630000]  [001249fe]  [000e4084]  [0011621c]  [00131a44]  [000e11ee]
> [   67.630000]  [00041944]  [00041a1c]  [00041e46]  [00003218]
> 
> As i said, i was hoping in your patch but i sadly discovered it is
> already included in this kernel version.
> Hope you can give me some help or can forward me to an appropriate
> mailing list.

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply

* Re: Bypass flow control problems
From: Kirsher, Jeffrey T @ 2010-12-29  5:05 UTC (permalink / raw)
  To: Pasi Kärkkäinen; +Cc: Alkis Georgopoulos, netdev@vger.kernel.org
In-Reply-To: <20101227134346.GD2754@reaktio.net>

On Dec 27, 2010, at 8:43, Pasi Kärkkäinen <pasik@iki.fi> wrote:

> On Wed, Dec 22, 2010 at 07:51:31PM +0200, Alkis Georgopoulos wrote:
>> Hi,
>> 
>> I'm an IT teacher/LTSP developer. In LTSP, thin clients are netbooted
>> from a server and receive a lot of X and remote disk traffic from it.
>> 
>> Many installations have a gigabit NIC on the server, an unmanaged
>> gigabit switch, and 100 Mbps NICs on the clients.
>> 
>> With flow control on, the server is limited to sending 100 Mbps to all
>> the clients. So with 10 thin clients the server can concurrently send
>> only 10 Mbps to each one of them.
>> 
>> On NICs that support it, we turn flow control off and the server can
>> properly send 100 Mbps to each client, i.e. 1 Gbps to 10 clients.
>> 
>> * Is there any way to bypass that problem on NICs that do not support
>>   turning off flow control, like e.g. realteks?
>>   I.e. when a client sends a pause signal to the server, instead of the
>>   server pausing, to continue sending data to another client?
>>   Or even to limit the amound of data the server sends to each client,
>>   so that the clients never have to send pause signals?
>> 
> 
> You could set up QoS rules on the server to limit the network speed per client..
> 
>> * I really don't understand why flow control is enabled by default on
>>   NICs and switches. In which case does it help? As far as I
>>   understand, all it does is ruin gigabit => 100 Mbps connections...
>> 
>> * As a side note, since rtl8169 is a very common chipset, is there a
>>   way to disable flow control for that specific NIC?
>> 
>> This problem affects thousands of LTSP installations, we'd much
>> appreciate your knowledge and feedback on it.
>> 
> 
> Did you try disabling flow control from the switch? 

He stated the switch was a un-managed switch, so he would be unable to disable flow control on the switch.

> 

^ permalink raw reply

* Re: [PATCH net-next 2/3] dcbnl: adding DCBX feature flags get-set
From: John Fastabend @ 2010-12-29  0:15 UTC (permalink / raw)
  To: Shmulik Ravid, davem@davemloft.net
  Cc: eilong@broadcom.com, Liu, Lucy, netdev@vger.kernel.org
In-Reply-To: <1292959973.7142.131.camel@lb-tlvb-shmulik.il.broadcom.com>

On 12/21/2010 11:32 AM, Shmulik Ravid wrote:
> Adding a pair of set-get functions to dcbnl for setting the negotiation
> flags of the various DCB features. The user sets these flags (enable,
> advertise, willing) for each feature to be used by the device DCBX
> engine. The 'get' routine returns which of the features is enabled
> after the negotiation.
> 
> Signed-off-by: Shmulik Ravid <shmulikr@broadcom.com>

Acked-by: John Fastabend <john.r.fastabend@intel.com>

Dave, these are going to conflict with my patches

[net-next-2.6,1/3] dcbnl: add support for ieee8021Qaz attributes
[net-next-2.6,2/3] dcbnl: add appliction tlv handlers

If needed either Shmulik or myself can work out the conflicts if you want a single series that applies cleanly.

Thanks,
John.






^ permalink raw reply

* Re: [PATCH net-next 1/3] dcbnl: adding DCBX engine capability
From: John Fastabend @ 2010-12-29  0:05 UTC (permalink / raw)
  To: Shmulik Ravid
  Cc: davem@davemloft.net, eilong@broadcom.com, Liu, Lucy,
	netdev@vger.kernel.org
In-Reply-To: <1292959963.7142.130.camel@lb-tlvb-shmulik.il.broadcom.com>

On 12/21/2010 11:32 AM, Shmulik Ravid wrote:
> Adding an optional DCBX capability and a pair for get-set routines for
> setting the device DCBX mode. The DCBX capability is a bit field of
> supported attributes. The user is expected to set the DCBX mode with a
> subset of the advertised attributes.
> 
> 
> Signed-off-by: Shmulik Ravid <shmulikr@broadcom.com>
> ---
>  include/linux/dcbnl.h |   17 +++++++++++++++++
>  include/net/dcbnl.h   |    2 ++
>  net/dcb/dcbnl.c       |   42 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dcbnl.h b/include/linux/dcbnl.h
> index 8723491..974fd1e 100644
> --- a/include/linux/dcbnl.h
> +++ b/include/linux/dcbnl.h
> @@ -50,6 +50,8 @@ struct dcbmsg {
>   * @DCB_CMD_SBCN: get backward congestion notification configration.
>   * @DCB_CMD_GAPP: get application protocol configuration
>   * @DCB_CMD_SAPP: set application protocol configuration
> + * @DCB_CMD_GDCBX: get DCBX engine configuration
> + * @DCB_CMD_SDCBX: set DCBX engine configuration
>   */
>  enum dcbnl_commands {
>  	DCB_CMD_UNDEFINED,
> @@ -83,6 +85,9 @@ enum dcbnl_commands {
>  	DCB_CMD_GAPP,
>  	DCB_CMD_SAPP,
>  
> +	DCB_CMD_GDCBX,
> +	DCB_CMD_SDCBX,
> +
>  	__DCB_CMD_ENUM_MAX,
>  	DCB_CMD_MAX = __DCB_CMD_ENUM_MAX - 1,
>  };
> @@ -102,6 +107,7 @@ enum dcbnl_commands {
>   * @DCB_ATTR_CAP: DCB capabilities of the device (NLA_NESTED)
>   * @DCB_ATTR_NUMTCS: number of traffic classes supported (NLA_NESTED)
>   * @DCB_ATTR_BCN: backward congestion notification configuration (NLA_NESTED)
> + * @DCB_ATTR_DCBX: DCBX engine configuration in the device (NLA_U8)
>   */
>  enum dcbnl_attrs {
>  	DCB_ATTR_UNDEFINED,
> @@ -118,6 +124,7 @@ enum dcbnl_attrs {
>  	DCB_ATTR_NUMTCS,
>  	DCB_ATTR_BCN,
>  	DCB_ATTR_APP,
> +	DCB_ATTR_DCBX,
>  
>  	__DCB_ATTR_ENUM_MAX,
>  	DCB_ATTR_MAX = __DCB_ATTR_ENUM_MAX - 1,
> @@ -262,6 +269,8 @@ enum dcbnl_tc_attrs {
>   * @DCB_CAP_ATTR_GSP: (NLA_U8) device supports group strict priority
>   * @DCB_CAP_ATTR_BCN: (NLA_U8) device supports Backwards Congestion
>   *                             Notification
> + * @DCB_CAP_ATTR_DCBX: (NLA_U8) device supports DCBX engine
> + *
>   */
>  enum dcbnl_cap_attrs {
>  	DCB_CAP_ATTR_UNDEFINED,
> @@ -273,11 +282,19 @@ enum dcbnl_cap_attrs {
>  	DCB_CAP_ATTR_PFC_TCS,
>  	DCB_CAP_ATTR_GSP,
>  	DCB_CAP_ATTR_BCN,
> +	DCB_CAP_ATTR_DCBX,
>  
>  	__DCB_CAP_ATTR_ENUM_MAX,
>  	DCB_CAP_ATTR_MAX = __DCB_CAP_ATTR_ENUM_MAX - 1,
>  };
>  
> +/* DCBX capabilities */
> +#define DCB_CAP_DCBX_HOST	0x01 /* host based DCBX engine support */
> +#define DCB_CAP_DCBX_HW		0x02 /* HW DCBX engine support */
> +#define DCB_CAP_DCBX_VER_CEE	0x04 /* HW DCBX supports CEE protocol */
> +#define DCB_CAP_DCBX_VER_IEEE	0x08 /* HW DCBX supports IEEE protocol */
> +#define DCB_CAP_DCBX_STATIC	0x10 /* HW DCBX supports static config */
> +

I would like to use these bits for guests using a VF as well. The problem is multiple lldp agents advertising dcbx tlvs on the same link is not spec compliant. In the VF case there may or may not be a hardware lldp engine all the VF driver (ie ixgbevf) should need to know is that some other entity is managing the DCB attributes.

To reflect this I would propose changing DCB_CAP_DCBX_HW and the comments by removing "HW". The two ideas I had were DCB_CAP_DCBX_READONLY or DCB_CAP_DCBX_LLD_MANAGED. Admittedly a bit of a nitpick but its a bit confusing to set the DCBX_HW bit when there really is no HW engine in the 82599 adapter case.

Otherwise this all looks good to me. I was hoping someone would get around to this. Thanks a lot!

-- John.

^ permalink raw reply

* Re: [PATCH net-next-2.6] ifb: add performance flags to dev->features
From: Jarek Poplawski @ 2010-12-28 23:07 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, xiaosuo, pstaszewski, netdev
In-Reply-To: <1293575781.20573.391.camel@edumazet-laptop>

On Tue, Dec 28, 2010 at 11:36:21PM +0100, Eric Dumazet wrote:
> Le mardi 28 décembre 2010 ?? 13:50 -0800, David Miller a écrit :
> 
> > Changli has suggested that these flags can also be set in
> > ->vlan_features, please address that if you expect me to
> > apply the change.
> 
> Hmm, I am just scratching my head to actually setup vlans on top of ifb,
> to test such a change ?

Ingress is before vlans handler so these features and the
NETIF_F_HW_VLAN_TX flag seem useful for ifb considering
dev_hard_start_xmit() checks.

Jarek P.

^ permalink raw reply

* Re: [PATCH net-next-2.6] ifb: add performance flags to dev->features
From: Eric Dumazet @ 2010-12-28 22:36 UTC (permalink / raw)
  To: David Miller; +Cc: xiaosuo, jarkao2, pstaszewski, netdev
In-Reply-To: <20101228.135013.39173075.davem@davemloft.net>

Le mardi 28 décembre 2010 à 13:50 -0800, David Miller a écrit :

> Changli has suggested that these flags can also be set in
> ->vlan_features, please address that if you expect me to
> apply the change.

Hmm, I am just scratching my head to actually setup vlans on top of ifb,
to test such a change ?






^ permalink raw reply

* Re: [patch] net/unix: do not forget to autobind in standard case
From: David Miller @ 2010-12-28 21:58 UTC (permalink / raw)
  To: eric.dumazet; +Cc: jengelh, netdev
In-Reply-To: <1293554655.20573.5.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 28 Dec 2010 17:44:15 +0100

> Le samedi 25 décembre 2010 à 01:08 +0100, Jan Engelhardt a écrit :
>> parent 67514fc40bbec857018cbc689d440282b75551db (v2.6.37-rc1-229-g67514fc)
>> commit 973bdc63c6aba703dd7b62a6ec7ae4bab7847731
>> Author: Jan Engelhardt <jengelh@medozas.de>
>> Date:   Sat Dec 25 00:50:59 2010 +0100
>> 
>> net/unix: do not forget to autobind in standard case
>> 
>> A program using recvmsg on an AF_LOCAL datagram socket does not
>> receive the peer's address when the remote has not issued a bind. This
>> makes it impossible to send messages back. (But the same _does_ work
>> in IP.) The cause for this is because autobinding was only done when
>> credential passing was requested.
>> 
> 
> It would be good you did some research and tell us why current code
> tests SOCK_PASSCRED
> 
> There must be a reason this restrictive code is here, dont you think ?

I definitely think that test is intentional.  I'm not applying this
without at least some basic research.

^ permalink raw reply

* Re: [patch] skfp: testing the wrong variable in skfp_driver_init()
From: David Miller @ 2010-12-28 21:55 UTC (permalink / raw)
  To: jpirko; +Cc: error27, netdev, eric.dumazet, hsweeten, kernel-janitors
In-Reply-To: <20101224084538.GA2791@psychotron.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Fri, 24 Dec 2010 09:45:39 +0100

> Fri, Dec 24, 2010 at 06:17:34AM CET, error27@gmail.com wrote:
>>The intent here was to test if the allocation failed but we tested 
>>"SharedMemSize" instead of "SharedMemAddr" by mistake.
>>
>>Signed-off-by: Dan Carpenter <error27@gmail.com>
>>
>>diff --git a/drivers/net/skfp/skfddi.c b/drivers/net/skfp/skfddi.c
>>index 0a66fed..16c6265 100644
>>--- a/drivers/net/skfp/skfddi.c
>>+++ b/drivers/net/skfp/skfddi.c
>>@@ -412,7 +412,7 @@ static  int skfp_driver_init(struct net_device *dev)
>> 		bp->SharedMemAddr = pci_alloc_consistent(&bp->pdev,
>> 							 bp->SharedMemSize,
>> 							 &bp->SharedMemDMA);
>>-		if (!bp->SharedMemSize) {
>>+		if (!bp->SharedMemAddr) {
>> 			printk("could not allocate mem for ");
>> 			printk("hardware module: %ld byte\n",
>> 			       bp->SharedMemSize);
> 
> Looks obvious.
> 
> Reviewed-by: Jiri Pirko <jpirko@redhat.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] ppp: allow disabling multilink protocol ID compression
From: David Miller @ 2010-12-28 21:53 UTC (permalink / raw)
  To: paulus; +Cc: shemminger, linux-ppp, netdev
In-Reply-To: <20101221224400.GA17383@brick.ozlabs.ibm.com>

From: Paul Mackerras <paulus@samba.org>
Date: Wed, 22 Dec 2010 09:44:00 +1100

> On Mon, Dec 20, 2010 at 07:58:33PM -0800, Stephen Hemminger wrote:
> 
>> Linux would not connect to other router running old version Cisco IOS (12.0).
>> This is most likely a bug in that version of IOS, since it is fixed
>> in later versions. As a workaround this patch allows a module parameter
>> to be set to disable compressing the protocol ID.
>> 
>> See: https://bugzilla.vyatta.com/show_bug.cgi?id=3979
>> 
>> RFC 1990 allows an implementation to formulate MP fragments as if protocol
>> compression had been negotiated.  This allows us to always send compressed
>> protocol IDs.  But some implementations don't accept MP fragments with
>> compressed protocol IDs.  This parameter allows us to interoperate with
>> them.  The default value of the configurable parameter is the same as the
>> current behavior:  protocol compression is enabled.  If protocol compression
>> is disabled we will not send compressed protocol IDs.
>> 
>> This is based on an earlier patch by Bob Gilligan (using a sysctl).
>> Module parameter is writable to allow for enabling even if ppp
>> is already loaded for other uses.
>> 
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> Acked-by: Paul Mackerras <paulus@samba.org>

Applied, thanks.

^ permalink raw reply

* RE: [patch] vxge: remove duplicated part of check {nodisc}
From: Ramkrishna Vepa @ 2010-12-28 21:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Sivakumar Subramani, Sreenivasa Honnur, Jon Mason,
	netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <20101224061539.GP1936@bicker>

> This is just a cleanup to make the static checkers happy.  We don't need
> to check "own" twice.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/drivers/net/vxge/vxge-traffic.c b/drivers/net/vxge/vxge-
> traffic.c
> index 42cc298..4c10d6c 100644
> --- a/drivers/net/vxge/vxge-traffic.c
> +++ b/drivers/net/vxge/vxge-traffic.c
> @@ -1240,7 +1240,7 @@ enum vxge_hw_status vxge_hw_ring_rxd_next_completed(
>  	*t_code	= (u8)VXGE_HW_RING_RXD_T_CODE_GET(control_0);
> 
>  	/* check whether it is not the end */
> -	if (!own || ((*t_code == VXGE_HW_RING_T_CODE_FRM_DROP) && own)) {
> +	if (!own || *t_code == VXGE_HW_RING_T_CODE_FRM_DROP) {
> 
>  		vxge_assert(((struct vxge_hw_ring_rxd_1 *)rxdp)->host_control
> !=
>  				0);
Looks good, thanks.

Acked-by: Ram Vepa <ram.vepa@exar.com>

^ permalink raw reply

* Re: [PATCH] ehea: Avoid changing vlan flags
From: David Miller @ 2010-12-28 21:51 UTC (permalink / raw)
  To: leitao; +Cc: shemminger, netdev, jesse
In-Reply-To: <1292871757-24918-1-git-send-email-leitao@linux.vnet.ibm.com>

From: leitao@linux.vnet.ibm.com
Date: Mon, 20 Dec 2010 17:02:37 -0200

> This patch avoids disabling the vlan flags using ethtool.
> 
> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>

Applied.

Although in the long term, having each and every driver handle this
seems rediculious.

^ permalink raw reply

* Re: [PATCH net-next-2.6] ifb: add performance flags to dev->features
From: David Miller @ 2010-12-28 21:50 UTC (permalink / raw)
  To: eric.dumazet; +Cc: xiaosuo, jarkao2, pstaszewski, netdev
In-Reply-To: <1292855120.2800.45.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 20 Dec 2010 15:25:20 +0100

> Le lundi 20 décembre 2010 à 22:05 +0800, Changli Gao a écrit :
>> 2010/12/20 Jarek Poplawski <jarkao2@gmail.com>:
>> >
>> > IMHO it should, (probably even more, like loopback) but we should
>> > consider mirred can xmit to other than ifb too.
>> >
>> 
>> I also think so. And when making ifb a multiqueue NIC, I tried to add
>> these dev features to ifb. :)
>> 
> 
> This has litle to do with your multiqueue work,
> its more an effect of GRO being more and more deployed.
> 
> I dont see dev->features being changed in one of your previous patches.
> I did dummy case in commit 6d81f41c58c6
> 
> [PATCH net-next-2.6] ifb: add performance flags to dev->features
> 
> IFB can use the full set of features flags (NETIF_F_SG |
> NETIF_F_FRAGLIST | NETIF_F_TSO | NETIF_F_NO_CSUM | NETIF_F_HIGHDMA) to
> avoid unecessary split of some packets (GRO for example)
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Changli has suggested that these flags can also be set in
->vlan_features, please address that if you expect me to
apply the change.

^ permalink raw reply

* Re: [PATCH] ip: reuse ip_summed of first fragment for all subsequent fragments
From: David Miller @ 2010-12-28 21:49 UTC (permalink / raw)
  To: timo.lindfors; +Cc: netdev
In-Reply-To: <1292841525-15572-1-git-send-email-timo.lindfors@iki.fi>

From: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Date: Mon, 20 Dec 2010 12:38:45 +0200

> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 439d2a3..c0743ed 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1167,7 +1167,7 @@ ssize_t	ip_append_page(struct sock *sk, struct page *page,
>  			/*
>  			 *	Fill in the control structures
>  			 */
> -			skb->ip_summed = CHECKSUM_NONE;
> +			skb->ip_summed = skb_prev->ip_summed;
>  			skb->csum = 0;
>  			skb_reserve(skb, hh_len);

You can't just assign skb_prev->ip_summed here, if it's CHECKSUM_PARTIAL
then things will go completely wrong.  This is especially true since
we're about to zero out skb->csum.

^ permalink raw reply

* Re: [PATCH v2 net-next-2.6] sch_sfq: allow big packets and be fair
From: David Miller @ 2010-12-28 21:46 UTC (permalink / raw)
  To: eric.dumazet; +Cc: jarkao2, kaber, netdev
In-Reply-To: <1292936699.2720.23.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 21 Dec 2010 14:04:59 +0100

> Le mardi 21 décembre 2010 à 12:17 +0000, Jarek Poplawski a écrit :
> 
>> Oops! You're right yet ;-) This skipping shouldn't happen with quantum
>> bigger than max packet size, so this patch is OK.
> 
> Thanks Jarek, here is a v2 with the scale you suggested.
> 
> [PATCH v2 net-next-2.6] sch_sfq: allow big packets and be fair
> 
> SFQ is currently 'limited' to small packets, because it uses a 15bit
> allotment number per flow. Introduce a scale by 8, so that we can handle
> full size TSO/GRO packets.
> 
> Use appropriate handling to make sure allot is positive before a new
> packet is dequeued, so that fairness is respected.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Jarek Poplawski <jarkao2@gmail.com>
> Cc: Patrick McHardy <kaber@trash.net>
> ---
> v2: Use a scale of 8 as Jarek suggested, instead of 18bit fields

Eric this doesn't apply cleanly, since the code in sfq_dump_class_stats()
is a bit different in net-next-2.6

Please respin this and resubmit.

^ permalink raw reply

* Re: [PATCH] ueagle-atm: fix PHY signal initialization race
From: David Miller @ 2010-12-28 21:42 UTC (permalink / raw)
  To: dcbw-H+wXaHxf7aLQT0dZR+AlfA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, duncan.sands-GANU6spQydw,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	hicham.haouari-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1292782671.5282.20.camel-wKZy7rqYPVb5EHUCmHmTqw@public.gmane.org>

From: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Sun, 19 Dec 2010 12:17:50 -0600

> A race exists when initializing ueagle-atm devices where the generic atm
> device may not yet be created before the driver attempts to initialize
> it's PHY signal state, which checks whether the atm device has been
> created or not.  This often causes the sysfs 'carrier' attribute to be
> '1' even though no signal has actually been found.
> 
> uea_probe
>    usbatm_usb_probe
>       driver->bind (uea_bind)
>          uea_boot
>             kthread_run(uea_kthread)     uea_kthread
>       usbatm_atm_init                       uea_start_reset
>          atm_dev_register                      UPDATE_ATM_SIGNAL
 ...
> Signed-off-by: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Applied, thanks Dan.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ISDN, Gigaset: Fix memory leak in do_disconnect_req()
From: Tilman Schmidt @ 2010-12-28 17:42 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: gigaset307x-common, Hansjoerg Lipp, Karsten Keil, netdev,
	linux-kernel
In-Reply-To: <alpine.LNX.2.00.1012262053130.20797@swampdragon.chaosbits.net>

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

Quite correct. Thanks for finding and fixing this.

Am 26.12.2010 20:59 schrieb Jesper Juhl:
> Hi,
> 
> In drivers/isdn/gigaset/capi.c::do_disconnect_req() we will leak the 
> memory allocated (with kmalloc) to 'b3cmsg' if the call to alloc_skb() 
> fails.
> 
> ...
> 		b3cmsg = kmalloc(sizeof(*b3cmsg), GFP_KERNEL);
> 	allocation here ------^
> 		if (!b3cmsg) {
> 			dev_err(cs->dev, "%s: out of memory\n", __func__);
> 			send_conf(iif, ap, skb, CAPI_MSGOSRESOURCEERR);
> 			return;
> 		}
> 		capi_cmsg_header(b3cmsg, ap->id, CAPI_DISCONNECT_B3, CAPI_IND,
> 				 ap->nextMessageNumber++,
> 				 cmsg->adr.adrPLCI | (1 << 16));
> 		b3cmsg->Reason_B3 = CapiProtocolErrorLayer1;
> 		b3skb = alloc_skb(CAPI_DISCONNECT_B3_IND_BASELEN, GFP_KERNEL);
> 		if (b3skb == NULL) {
> 			dev_err(cs->dev, "%s: out of memory\n", __func__);
> 			send_conf(iif, ap, skb, CAPI_MSGOSRESOURCEERR);
> 			return;
> 	leak here ------^
> ...
> 
> This leak is easily fixed by just kfree()'ing the memory allocated to 
> 'b3cmsg' right before we return. The following patch does that.
> 
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Acked-by: Tilman Schmidt <tilman@imap.cc>

> ---
>  capi.c |    1 +
>  1 file changed, 1 insertion(+)
> 
>   compile tested only since I have no way to actually test this.
> 
> diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
> index bcc174e..658e75f 100644
> --- a/drivers/isdn/gigaset/capi.c
> +++ b/drivers/isdn/gigaset/capi.c
> @@ -1900,6 +1900,7 @@ static void do_disconnect_req(struct gigaset_capi_ctr *iif,
>  		if (b3skb == NULL) {
>  			dev_err(cs->dev, "%s: out of memory\n", __func__);
>  			send_conf(iif, ap, skb, CAPI_MSGOSRESOURCEERR);
> +			kfree(b3cmsg);
>  			return;
>  		}
>  		capi_cmsg2message(b3cmsg,
> 
> 

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


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

^ permalink raw reply

* Re: [patch] net/unix: do not forget to autobind in standard case
From: Eric Dumazet @ 2010-12-28 16:44 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Linux Networking Developer Mailing List, David S. Miller
In-Reply-To: <alpine.LNX.2.01.1012250107330.22521@obet.zrqbmnf.qr>

Le samedi 25 décembre 2010 à 01:08 +0100, Jan Engelhardt a écrit :
> parent 67514fc40bbec857018cbc689d440282b75551db (v2.6.37-rc1-229-g67514fc)
> commit 973bdc63c6aba703dd7b62a6ec7ae4bab7847731
> Author: Jan Engelhardt <jengelh@medozas.de>
> Date:   Sat Dec 25 00:50:59 2010 +0100
> 
> net/unix: do not forget to autobind in standard case
> 
> A program using recvmsg on an AF_LOCAL datagram socket does not
> receive the peer's address when the remote has not issued a bind. This
> makes it impossible to send messages back. (But the same _does_ work
> in IP.) The cause for this is because autobinding was only done when
> credential passing was requested.
> 

It would be good you did some research and tell us why current code
tests SOCK_PASSCRED

There must be a reason this restrictive code is here, dont you think ?

> --->8---
> 
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
>  net/unix/af_unix.c |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 7ff31c6..945797c 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -964,8 +964,8 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr *addr,
>  			goto out;
>  		alen = err;
>  
> -		if (test_bit(SOCK_PASSCRED, &sock->flags) &&
> -		    !unix_sk(sk)->addr && (err = unix_autobind(sock)) != 0)
> +		if (unix_sk(sk)->addr == NULL &&
> +		    (err = unix_autobind(sock)) != 0)
>  			goto out;
>  
>  restart:
> @@ -1063,8 +1063,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
>  		goto out;
>  	addr_len = err;
>  
> -	if (test_bit(SOCK_PASSCRED, &sock->flags) && !u->addr &&
> -	    (err = unix_autobind(sock)) != 0)
> +	if (u->addr == NULL && (err = unix_autobind(sock)) != 0)
>  		goto out;
>  
>  	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
> @@ -1419,8 +1418,7 @@ static int unix_dgram_sendmsg(struct kiocb *kiocb, struct socket *sock,
>  			goto out;
>  	}
>  
> -	if (test_bit(SOCK_PASSCRED, &sock->flags) && !u->addr
> -	    && (err = unix_autobind(sock)) != 0)
> +	if (u->addr == NULL && (err = unix_autobind(sock)) != 0)
>  		goto out;
>  
>  	err = -EMSGSIZE;



^ permalink raw reply

* Re: [RFC PATCH v2] Gemini: Gigabit ethernet driver
From: Michał Mirosław @ 2010-12-28 11:45 UTC (permalink / raw)
  To: Hans Ulli Kroll; +Cc: gemini-board-dev, netdev, Christoph Biedl
In-Reply-To: <alpine.LNX.2.00.1012271956260.8215@localhost>

On Mon, Dec 27, 2010 at 08:21:44PM +0100, Hans Ulli Kroll wrote:
> On Mon, 27 Dec 2010, Michał Mirosław wrote:
> > +		.dis_tx = 1,
> > +		.dis_rx = 1,
> > +		.max_len = 2,	/* magic; 512 << max_len? */
> > +		.ipv4_rx_chksum = 1,
> > +		.ipv6_rx_chksum = 1,
> 
> This is what I found in some netbsd code for maxlen
> 
> #define	CONFIG0_MAXLEN_MASK			__BITS(8,10)
> #define	CONFIG0_MAXLEN_GET(x)			(((x) >> 8) & 
> CONFIG0_MAXLEN_MASK)
> #define	CONFIG0_MAXLEN(x)			(((x) & 
> CONFIG0_MAXLEN_MASK) << 8)
> #define	CONFIG0_MAXLEN_1536			0
> #define	CONFIG0_MAXLEN_1518			1
> #define	CONFIG0_MAXLEN_1522			2
> #define	CONFIG0_MAXLEN_1548			3
> #define	CONFIG0_MAXLEN_JUMBO			4
> 
> spec. for the SL3616 says jumbo is 9k frames, but i didn't proved this.

This is what I got after quick testing:

max_len rx_max
0       1536
1       1518
2       1522
3       1542
4       9212
5       10236
6       1518
7       1518

rx_max is total packet's length (MAC header + (VLAN) + payload).
 
Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH net-2.6] starfire: Replace broken preprocessor test for dma_addr_t size
From: FUJITA Tomonori @ 2010-12-28  5:39 UTC (permalink / raw)
  To: akinobu.mita; +Cc: ben, davem, ionut, netdev, fujita.tomonori, akpm, ralf
In-Reply-To: <AANLkTinx-n045io5bPT-u_7T5cEiu6+74p30=94vkhPU@mail.gmail.com>

On Tue, 28 Dec 2010 14:27:44 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> 2010/12/28 Ben Hutchings <ben@decadent.org.uk>:
> > Commit 56543af "starfire: use BUILD_BUG_ON for netdrv_addr_t" revealed
> > that the preprocessor condition used to find the size of dma_addr_t
> > yielded the wrong result for some architectures and configurations.
> > This was kluged for 64-bit PowerPC in commit 3e502e6 by adding yet
> > another case to the condition.  However, some MIPS configurations are
> > still handled incorrectly.
> >
> > Replace the preprocessor test with expressions using ?: or
> > __builtin_choose_expr() as necessary.
> 
> I remember ARCH_DMA_ADDR_T_64BIT was introduced to handle this kind of
> test easily.
> 
> So please use CONFIG_ARCH_DMA_ADDR_T_64BIT for this test.
> You may also need to set ARCH_DMA_ADDR_T_64BIT for the MIPS
> architecture correctly.

I already sent CONFIG_ARCH_DMA_ADDR_T_64BIT patch for MIPS. It has
been in -mm:

http://userweb.kernel.org/~akpm/mmotm/broken-out/mips-enable-arch_dma_addr_t_64bit-with-highmem-64bit_phys_addr-64bit.patch

^ permalink raw reply

* Re: [PATCH net-2.6] starfire: Replace broken preprocessor test for dma_addr_t size
From: Akinobu Mita @ 2010-12-28  5:27 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, Ion Badulescu, netdev, fujita.tomonori
In-Reply-To: <1293509989.2928.99.camel@localhost>

2010/12/28 Ben Hutchings <ben@decadent.org.uk>:
> Commit 56543af "starfire: use BUILD_BUG_ON for netdrv_addr_t" revealed
> that the preprocessor condition used to find the size of dma_addr_t
> yielded the wrong result for some architectures and configurations.
> This was kluged for 64-bit PowerPC in commit 3e502e6 by adding yet
> another case to the condition.  However, some MIPS configurations are
> still handled incorrectly.
>
> Replace the preprocessor test with expressions using ?: or
> __builtin_choose_expr() as necessary.

I remember ARCH_DMA_ADDR_T_64BIT was introduced to handle this kind of
test easily.

So please use CONFIG_ARCH_DMA_ADDR_T_64BIT for this test.
You may also need to set ARCH_DMA_ADDR_T_64BIT for the MIPS
architecture correctly.

> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> This is compile-tested only.
>
> The build failure on MIPS can be seen at
> <https://buildd.debian.org/fetch.cgi?pkg=linux-2.6;ver=2.6.37~rc7-1~experimental.1;arch=mips;stamp=1293414847>.
>
> Ben.
>
>  drivers/net/starfire.c |   59 +++++++++++++++++++----------------------------
>  1 files changed, 24 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/net/starfire.c b/drivers/net/starfire.c
> index 4adf124..ea789d6 100644
> --- a/drivers/net/starfire.c
> +++ b/drivers/net/starfire.c
> @@ -144,31 +144,22 @@ static int full_duplex[MAX_UNITS] = {0, };
>  /* Time in jiffies before concluding the transmitter is hung. */
>  #define TX_TIMEOUT     (2 * HZ)
>
> -/*
> - * This SUCKS.
> - * We need a much better method to determine if dma_addr_t is 64-bit.
> - */
> -#if (defined(__i386__) && defined(CONFIG_HIGHMEM64G)) || defined(__x86_64__) || defined (__ia64__) || defined(__alpha__) || defined(__mips64__) || (defined(__mips__) && defined(CONFIG_HIGHMEM) && defined(CONFIG_64BIT_PHYS_ADDR)) || (defined(__powerpc64__) || defined(CONFIG_PHYS_64BIT))
> -/* 64-bit dma_addr_t */
> -#define ADDR_64BITS    /* This chip uses 64 bit addresses. */
> -#define netdrv_addr_t __le64
> -#define cpu_to_dma(x) cpu_to_le64(x)
> -#define dma_to_cpu(x) le64_to_cpu(x)
> -#define RX_DESC_Q_ADDR_SIZE RxDescQAddr64bit
> -#define TX_DESC_Q_ADDR_SIZE TxDescQAddr64bit
> -#define RX_COMPL_Q_ADDR_SIZE RxComplQAddr64bit
> -#define TX_COMPL_Q_ADDR_SIZE TxComplQAddr64bit
> -#define RX_DESC_ADDR_SIZE RxDescAddr64bit
> -#else  /* 32-bit dma_addr_t */
> -#define netdrv_addr_t __le32
> -#define cpu_to_dma(x) cpu_to_le32(x)
> -#define dma_to_cpu(x) le32_to_cpu(x)
> -#define RX_DESC_Q_ADDR_SIZE RxDescQAddr32bit
> -#define TX_DESC_Q_ADDR_SIZE TxDescQAddr32bit
> -#define RX_COMPL_Q_ADDR_SIZE RxComplQAddr32bit
> -#define TX_COMPL_Q_ADDR_SIZE TxComplQAddr32bit
> -#define RX_DESC_ADDR_SIZE RxDescAddr32bit
> -#endif
> +#define IS_ADDR_64BIT (sizeof(dma_addr_t) == 8)
> +#define cpu_to_dma(x)                                                  \
> +       __builtin_choose_expr(IS_ADDR_64BIT, cpu_to_le64(x), cpu_to_le32(x))
> +#define dma_to_cpu(x)                                                  \
> +       __builtin_choose_expr(IS_ADDR_64BIT, le64_to_cpu(x), le32_to_cpu(x))
> +typedef typeof(cpu_to_dma(0)) netdrv_addr_t;
> +#define RX_DESC_Q_ADDR_SIZE                                            \
> +       (IS_ADDR_64BIT ? RxDescQAddr64bit : RxDescQAddr32bit)
> +#define TX_DESC_Q_ADDR_SIZE                                            \
> +       (IS_ADDR_64BIT ? TxDescQAddr64bit : TxDescQAddr32bit)
> +#define RX_COMPL_Q_ADDR_SIZE                                           \
> +       (IS_ADDR_64BIT ? RxComplQAddr64bit : RxComplQAddr32bit)
> +#define TX_COMPL_Q_ADDR_SIZE                                           \
> +       (IS_ADDR_64BIT ? TxComplQAddr64bit : TxComplQAddr32bit)
> +#define RX_DESC_ADDR_SIZE                                              \
> +       (IS_ADDR_64BIT ? RxDescAddr64bit : RxDescAddr32bit)
>
>  #define skb_first_frag_len(skb)        skb_headlen(skb)
>  #define skb_num_frags(skb) (skb_shinfo(skb)->nr_frags + 1)
> @@ -512,13 +503,12 @@ struct starfire_tx_desc_2 {
>        __le64 addr;
>  };
>
> -#ifdef ADDR_64BITS
> -typedef struct starfire_tx_desc_2 starfire_tx_desc;
> -#define TX_DESC_TYPE TxDescType2
> -#else  /* not ADDR_64BITS */
> -typedef struct starfire_tx_desc_1 starfire_tx_desc;
> -#define TX_DESC_TYPE TxDescType1
> -#endif /* not ADDR_64BITS */
> +extern struct starfire_tx_desc_1 starfire_tx_desc_1_dummy(void);
> +extern struct starfire_tx_desc_2 starfire_tx_desc_2_dummy(void);
> +typedef typeof(__builtin_choose_expr(IS_ADDR_64BIT, starfire_tx_desc_2_dummy(),
> +                                    starfire_tx_desc_1_dummy()))
> +starfire_tx_desc;
> +#define TX_DESC_TYPE (IS_ADDR_64BIT ? TxDescType2 : TxDescType1)
>  #define TX_DESC_SPACING TxDescSpaceUnlim
>
>  enum tx_desc_bits {
> @@ -731,9 +721,8 @@ static int __devinit starfire_init_one(struct pci_dev *pdev,
>  #ifdef VLAN_SUPPORT
>        dev->features |= NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_FILTER;
>  #endif /* VLAN_RX_KILL_VID */
> -#ifdef ADDR_64BITS
> -       dev->features |= NETIF_F_HIGHDMA;
> -#endif /* ADDR_64BITS */
> +       if (IS_ADDR_64BIT)
> +               dev->features |= NETIF_F_HIGHDMA;
>
>        /* Serial EEPROM reads are hidden by the hardware. */
>        for (i = 0; i < 6; i++)
> --
> 1.7.2.3
>
>
>

^ permalink raw reply

* Re: [RFC PATCH v2] Gemini: Gigabit ethernet driver
From: Stephen Hemminger @ 2010-12-28  5:12 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Michał Mirosław, gemini-board-dev, netdev,
	Christoph Biedl, Hans Ulli Kroll
In-Reply-To: <1293511020.2928.105.camel@localhost>



----- Original Message -----
> On Mon, 2010-12-27 at 20:21 +0100, Hans Ulli Kroll wrote:
> [...]
> > BTW:
> >
> > Why u64_stats ?
> > I see only a few driver are using u64_stats.
> > vlan, br_device and some intel driver
> > no gigabit driver for marvell devices uses u64_stats
> 
> All new net drivers should implement 64-bit stats. net_device_stats is
> kept for backward compatibility because we couldn't change all the
> existing drivers at once (it's not a simple change for all of them).
> 
> Ben.

BS. drivers with old stats are fine. 64 bit only really matters
at higher speed. Anyway, it is the kind of thing that can easily
be fixed later after driver is merged.




^ permalink raw reply

* Re: [RFC PATCH v2] Gemini: Gigabit ethernet driver
From: Ben Hutchings @ 2010-12-28  4:37 UTC (permalink / raw)
  To: Hans Ulli Kroll
  Cc: Michał Mirosław, gemini-board-dev, netdev,
	Christoph Biedl
In-Reply-To: <alpine.LNX.2.00.1012271956260.8215@localhost>

On Mon, 2010-12-27 at 20:21 +0100, Hans Ulli Kroll wrote:
[...]
> BTW:
> 
> Why u64_stats ?
> I see only a few driver are using u64_stats.
> vlan, br_device and some intel driver 
> no gigabit driver for marvell devices uses u64_stats

All new net drivers should implement 64-bit stats.  net_device_stats is
kept for backward compatibility because we couldn't change all the
existing drivers at once (it's not a simple change for all of them).

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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


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