Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next-2.6] net: Xmit Packet Steering (XPS)
From: Joe Perches @ 2009-11-20 21:43 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Jarek Poplawski, David S. Miller, Tom Herbert, Linux Netdev List
In-Reply-To: <4B070BAC.1090406@gmail.com>

On Fri, 2009-11-20 at 22:35 +0100, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > There is one more problem: this will break things like act_mirred + ifb,
> > and other cases using skb->iif e.g. for filtering on virtual devices at
> > the xmit path.
> Following patch might help us to locate real uses of this obscure field :)
> [PATCH net-next-2.6] net: rename skb->iif to skb->skb_iif

Prefixing member names generally doesn't end well.
Prefixing selected member names? ick.


^ permalink raw reply

* Re: A generic kernel compatibilty code
From: Luis R. Rodriguez @ 2009-11-20 21:38 UTC (permalink / raw)
  To: John W. Linville
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-wireless,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20091120211837.GA22815-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

On Fri, Nov 20, 2009 at 1:18 PM, John W. Linville
<linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:
> On Fri, Nov 20, 2009 at 12:53:51PM -0800, Luis R. Rodriguez wrote:
>> On Fri, Nov 20, 2009 at 12:51 PM, Luis R. Rodriguez <mcgrof-Re5JQEeQqe8@public.gmane.orgm> wrote:
>> > On Fri, Nov 20, 2009 at 12:45 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>> >> Everyone and their mother reinvents the wheel when it comes to
>> >> backporting kernel modules. It a painful job and it seems to me an
>> >> alternative is possible. If we can write generic compatibilty code for
>> >> a new routine introduced on the next kernel how about just merging it
>> >> to the kernel under some generic compat module. This would be
>> >> completey ignored by everyone using the stable kernel but can be
>> >> copied by anyone doing backport work.
>> >>
>> >> So I'm thinking something as simple as a generic compat/comat.ko with
>> >> compat-2.6.32.[ch] files.
>> >
>> > FWIW, I meant a compat-2.6.32.[ch] and compat-2.6.31.[ch] and so on.
>> > All these would link to the compat.ko
>>
>> I supose this could juse be a separate tree with some generic
>> compat.ko module. That might work better.
>
> This is what I would suggest for pursuing this idea.  Perhaps you
> could split-off from compat-wireless, then make that tree depend on
> the new tree (compat-core?)...

OK thanks, will try that for the next kernel.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 net-next-2.6] net: Xmit Packet Steering (XPS)
From: Eric Dumazet @ 2009-11-20 21:35 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David S. Miller, Tom Herbert, Linux Netdev List
In-Reply-To: <4B0701E2.1070806@gmail.com>

Jarek Poplawski a écrit :
> 
> There is one more problem: this will break things like act_mirred + ifb,
> and other cases using skb->iif e.g. for filtering on virtual devices at
> the xmit path.
> 

Following patch might help us to locate real uses of this obscure field :)

[PATCH net-next-2.6] net: rename skb->iif to skb->skb_iif

To help grep games, rename iif to skb_iif

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/ifb.c                 |    6 +++---
 include/linux/skbuff.h            |    4 ++--
 include/net/pkt_cls.h             |    4 ++--
 net/core/dev.c                    |    6 +++---
 net/core/skbuff.c                 |    2 +-
 net/netlabel/netlabel_unlabeled.c |    2 +-
 net/sched/act_mirred.c            |    2 +-
 net/sched/cls_flow.c              |    2 +-
 security/selinux/hooks.c          |    6 +++---
 security/smack/smack_lsm.c        |    4 ++--
 10 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 69c2566..f4081c0 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -99,7 +99,7 @@ static void ri_tasklet(unsigned long dev)
 		stats->tx_bytes +=skb->len;
 
 		rcu_read_lock();
-		skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
+		skb->dev = dev_get_by_index_rcu(&init_net, skb->skb_iif);
 		if (!skb->dev) {
 			rcu_read_unlock();
 			dev_kfree_skb(skb);
@@ -107,7 +107,7 @@ static void ri_tasklet(unsigned long dev)
 			break;
 		}
 		rcu_read_unlock();
-		skb->iif = _dev->ifindex;
+		skb->skb_iif = _dev->ifindex;
 
 		if (from & AT_EGRESS) {
 			dp->st_rx_frm_egr++;
@@ -172,7 +172,7 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
 	stats->rx_packets++;
 	stats->rx_bytes+=skb->len;
 
-	if (!(from & (AT_INGRESS|AT_EGRESS)) || !skb->iif) {
+	if (!(from & (AT_INGRESS|AT_EGRESS)) || !skb->skb_iif) {
 		dev_kfree_skb(skb);
 		stats->rx_dropped++;
 		return NETDEV_TX_OK;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 63f4742..89eed8c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -299,7 +299,7 @@ typedef unsigned char *sk_buff_data_t;
  *	@nfctinfo: Relationship of this skb to the connection
  *	@nfct_reasm: netfilter conntrack re-assembly pointer
  *	@nf_bridge: Saved data about a bridged frame - see br_netfilter.c
- *	@iif: ifindex of device we arrived on
+ *	@skb_iif: ifindex of device we arrived on
  *	@queue_mapping: Queue mapping for multiqueue devices
  *	@tc_index: Traffic control index
  *	@tc_verd: traffic control verdict
@@ -366,7 +366,7 @@ struct sk_buff {
 	struct nf_bridge_info	*nf_bridge;
 #endif
 
-	int			iif;
+	int			skb_iif;
 #ifdef CONFIG_NET_SCHED
 	__u16			tc_index;	/* traffic control index */
 #ifdef CONFIG_NET_CLS_ACT
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 3dd210d..dd3031a 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -343,9 +343,9 @@ tcf_match_indev(struct sk_buff *skb, char *indev)
 	struct net_device *dev;
 
 	if (indev[0]) {
-		if  (!skb->iif)
+		if  (!skb->skb_iif)
 			return 0;
-		dev = __dev_get_by_index(dev_net(skb->dev), skb->iif);
+		dev = __dev_get_by_index(dev_net(skb->dev), skb->skb_iif);
 		if (!dev || strcmp(indev, dev->name))
 			return 0;
 	}
diff --git a/net/core/dev.c b/net/core/dev.c
index 9977288..09f3d6b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2287,7 +2287,7 @@ static int ing_filter(struct sk_buff *skb)
 	if (MAX_RED_LOOP < ttl++) {
 		printk(KERN_WARNING
 		       "Redir loop detected Dropping packet (%d->%d)\n",
-		       skb->iif, dev->ifindex);
+		       skb->skb_iif, dev->ifindex);
 		return TC_ACT_SHOT;
 	}
 
@@ -2395,8 +2395,8 @@ int netif_receive_skb(struct sk_buff *skb)
 	if (netpoll_receive_skb(skb))
 		return NET_RX_DROP;
 
-	if (!skb->iif)
-		skb->iif = skb->dev->ifindex;
+	if (!skb->skb_iif)
+		skb->skb_iif = skb->dev->ifindex;
 
 	null_or_orig = NULL;
 	orig_dev = skb->dev;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 739b8f4..bfa3e78 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -549,7 +549,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 #endif
 	new->protocol		= old->protocol;
 	new->mark		= old->mark;
-	new->iif		= old->iif;
+	new->skb_iif		= old->skb_iif;
 	__nf_copy(new, old);
 #if defined(CONFIG_NETFILTER_XT_TARGET_TRACE) || \
     defined(CONFIG_NETFILTER_XT_TARGET_TRACE_MODULE)
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 3dfe2ba..98ed22e 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1550,7 +1550,7 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
 	struct netlbl_unlhsh_iface *iface;
 
 	rcu_read_lock();
-	iface = netlbl_unlhsh_search_iface_def(skb->iif);
+	iface = netlbl_unlhsh_search_iface_def(skb->skb_iif);
 	if (iface == NULL)
 		goto unlabel_getattr_nolabel;
 	switch (family) {
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 7974793..d329170 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -185,7 +185,7 @@ static int tcf_mirred(struct sk_buff *skb, struct tc_action *a,
 		skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
 
 	skb2->dev = dev;
-	skb2->iif = skb->dev->ifindex;
+	skb2->skb_iif = skb->dev->ifindex;
 	dev_queue_xmit(skb2);
 	err = 0;
 
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 9402a7f..e054c62 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -171,7 +171,7 @@ static u32 flow_get_proto_dst(const struct sk_buff *skb)
 
 static u32 flow_get_iif(const struct sk_buff *skb)
 {
-	return skb->iif;
+	return skb->skb_iif;
 }
 
 static u32 flow_get_priority(const struct sk_buff *skb)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index bb230d5..83a4aad 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4085,7 +4085,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
 	char *addrp;
 
 	COMMON_AUDIT_DATA_INIT(&ad, NET);
-	ad.u.net.netif = skb->iif;
+	ad.u.net.netif = skb->skb_iif;
 	ad.u.net.family = family;
 	err = selinux_parse_skb(skb, &ad, &addrp, 1, NULL);
 	if (err)
@@ -4147,7 +4147,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		return 0;
 
 	COMMON_AUDIT_DATA_INIT(&ad, NET);
-	ad.u.net.netif = skb->iif;
+	ad.u.net.netif = skb->skb_iif;
 	ad.u.net.family = family;
 	err = selinux_parse_skb(skb, &ad, &addrp, 1, NULL);
 	if (err)
@@ -4159,7 +4159,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		err = selinux_skb_peerlbl_sid(skb, family, &peer_sid);
 		if (err)
 			return err;
-		err = selinux_inet_sys_rcv_skb(skb->iif, addrp, family,
+		err = selinux_inet_sys_rcv_skb(skb->skb_iif, addrp, family,
 					       peer_sid, &ad);
 		if (err) {
 			selinux_netlbl_err(skb, err, 0);
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index c33b6bb..529c9ca 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2602,7 +2602,7 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 #ifdef CONFIG_AUDIT
 	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NET);
 	ad.a.u.net.family = sk->sk_family;
-	ad.a.u.net.netif = skb->iif;
+	ad.a.u.net.netif = skb->skb_iif;
 	ipv4_skb_to_auditdata(skb, &ad.a, NULL);
 #endif
 	/*
@@ -2757,7 +2757,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 #ifdef CONFIG_AUDIT
 	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NET);
 	ad.a.u.net.family = family;
-	ad.a.u.net.netif = skb->iif;
+	ad.a.u.net.netif = skb->skb_iif;
 	ipv4_skb_to_auditdata(skb, &ad.a, NULL);
 #endif
 	/*

^ permalink raw reply related

* Re: A generic kernel compatibilty code
From: John W. Linville @ 2009-11-20 21:18 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-kernel, linux-wireless, netdev
In-Reply-To: <43e72e890911201253obc466c8k2102e04457c48c92@mail.gmail.com>

On Fri, Nov 20, 2009 at 12:53:51PM -0800, Luis R. Rodriguez wrote:
> On Fri, Nov 20, 2009 at 12:51 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> > On Fri, Nov 20, 2009 at 12:45 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> >> Everyone and their mother reinvents the wheel when it comes to
> >> backporting kernel modules. It a painful job and it seems to me an
> >> alternative is possible. If we can write generic compatibilty code for
> >> a new routine introduced on the next kernel how about just merging it
> >> to the kernel under some generic compat module. This would be
> >> completey ignored by everyone using the stable kernel but can be
> >> copied by anyone doing backport work.
> >>
> >> So I'm thinking something as simple as a generic compat/comat.ko with
> >> compat-2.6.32.[ch] files.
> >
> > FWIW, I meant a compat-2.6.32.[ch] and compat-2.6.31.[ch] and so on.
> > All these would link to the compat.ko
> 
> I supose this could juse be a separate tree with some generic
> compat.ko module. That might work better.

This is what I would suggest for pursuing this idea.  Perhaps you
could split-off from compat-wireless, then make that tree depend on
the new tree (compat-core?)...

Hth...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: A generic kernel compatibilty code
From: Mauro Carvalho Chehab @ 2009-11-20 21:16 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-kernel, linux-wireless, netdev
In-Reply-To: <43e72e890911201245r4de5b039hb2dd5011dabf2399@mail.gmail.com>

Hi Luis,

Em Fri, 20 Nov 2009 12:45:51 -0800
"Luis R. Rodriguez" <mcgrof@gmail.com> escreveu:

> Everyone and their mother reinvents the wheel when it comes to
> backporting kernel modules. It a painful job and it seems to me an
> alternative is possible. If we can write generic compatibilty code for
> a new routine introduced on the next kernel how about just merging it
> to the kernel under some generic compat module. This would be
> completey ignored by everyone using the stable kernel but can be
> copied by anyone doing backport work.
> 
> So I'm thinking something as simple as a generic compat/comat.ko with
> compat-2.6.32.[ch] files.
> 
> We've already backported everything needed for wireless drivers under
> compat-wireless under this format down to even 2.6.25. I volunteer to
> be the sucker for this if this is reasonable and given the shot to try
> it.
> 

We have several backport stuff at our development tree, that works for
V4L/DVB up to kernel 2.6.16 (so, up to RHEL5).

It is based on a compat.h file, plus some extra tests inside the drivers.
They are at:
	http://linuxtv.org/hg/v4l-dvb

I also started working on a different approach of dynamically patching the
drivers based on some semantic rules, parsed in perl, but I hadn't enough
time to finish.

If you want to take a look, it is at:

	http://linuxtv.org/hg/~mchehab/backport/


Cheers,
Mauro

^ permalink raw reply

* Re: A generic kernel compatibilty code
From: Luis R. Rodriguez @ 2009-11-20 21:07 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: linux-kernel, linux-wireless, netdev
In-Reply-To: <1258750858.2877.58.camel@achroite.uk.solarflarecom.com>

On Fri, Nov 20, 2009 at 1:00 PM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Fri, 2009-11-20 at 12:45 -0800, Luis R. Rodriguez wrote:
>> Everyone and their mother reinvents the wheel when it comes to
>> backporting kernel modules. It a painful job and it seems to me an
>> alternative is possible. If we can write generic compatibilty code for
>> a new routine introduced on the next kernel how about just merging it
>> to the kernel under some generic compat module. This would be
>> completey ignored by everyone using the stable kernel but can be
>> copied by anyone doing backport work.
>>
>> So I'm thinking something as simple as a generic compat/comat.ko with
>> compat-2.6.32.[ch] files.
>>
>> We've already backported everything needed for wireless drivers under
>> compat-wireless under this format down to even 2.6.25.
> [...]
>
> If you think 2.6.25 is old then I don't think you understand the scale
> of the problem.
>
> OEMs still expect us to support RHEL 4 (2.6.9) and SLES 9 (2.6.5) though
> the latter will probably be dropped soon.  Some other vendors apparently
> still need to support even 2.4 kernels!

Heh understood. Well shouldn't this help with that then? Sure I'd love
to see the Enteprise Linux releases on 2.6.31 but that's not going to
happen right? Shouldn't this help then?

  Luis

^ permalink raw reply

* Re: A generic kernel compatibilty code
From: Ben Hutchings @ 2009-11-20 21:00 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-kernel, linux-wireless, netdev
In-Reply-To: <43e72e890911201245r4de5b039hb2dd5011dabf2399@mail.gmail.com>

On Fri, 2009-11-20 at 12:45 -0800, Luis R. Rodriguez wrote:
> Everyone and their mother reinvents the wheel when it comes to
> backporting kernel modules. It a painful job and it seems to me an
> alternative is possible. If we can write generic compatibilty code for
> a new routine introduced on the next kernel how about just merging it
> to the kernel under some generic compat module. This would be
> completey ignored by everyone using the stable kernel but can be
> copied by anyone doing backport work.
> 
> So I'm thinking something as simple as a generic compat/comat.ko with
> compat-2.6.32.[ch] files.
> 
> We've already backported everything needed for wireless drivers under
> compat-wireless under this format down to even 2.6.25.
[...]

If you think 2.6.25 is old then I don't think you understand the scale
of the problem.

OEMs still expect us to support RHEL 4 (2.6.9) and SLES 9 (2.6.5) though
the latter will probably be dropped soon.  Some other vendors apparently
still need to support even 2.4 kernels!

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

* Re: [net-next-2.6 PATCH v7 5/7 RFC] TCPCT part 1e: implement socket option TCP_COOKIE_TRANSACTIONS
From: Joe Perches @ 2009-11-20 20:54 UTC (permalink / raw)
  To: David Miller; +Cc: william.allen.simpson, netdev
In-Reply-To: <20091120.092651.254794724.davem@davemloft.net>

On Fri, 2009-11-20 at 09:26 -0800, David Miller wrote:
> From: William Allen Simpson <william.allen.simpson@gmail.com>
> > +		ctd.tcpct_flags = (tp->rx_opt.cookie_in_always
> > +				   ? TCP_COOKIE_IN_ALWAYS : 0)
> > +				| (tp->rx_opt.cookie_out_never
> > +				   ? TCP_COOKIE_OUT_NEVER : 0);
> 
> "?" should be at end of previous line not at beginning of
> next one, please fix this up.

Perhaps the ? position is taster's choice.

(false positives exist)

$ grep -rP --include=*.[ch] "^\s*\?" * | wc -l
884

$ grep -rP --include=*.[ch] -h "\?\s*$" * | \
  grep -vP "^\s*\*" | grep -vP "(//|/\*)" | wc -l
2968



^ permalink raw reply

* Re: [PATCH net-next-2.6] net: Xmit Packet Steering (XPS)
From: Jarek Poplawski @ 2009-11-20 20:53 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, Linux Netdev List
In-Reply-To: <4B05D8DC.7020907@gmail.com>

Eric Dumazet wrote, On 11/20/2009 12:46 AM:

> Here is first version of XPS.
> 
> Goal of XPS is to free TX completed skbs by the cpu that submitted the transmit.
> 
> Because I chose to union skb->iif with skb->sending_cpu, I chose
> to introduce a new xps_consume_skb(skb), and not generalize consume_skb() itself.

...

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 63f4742..e8e4795 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -366,7 +366,10 @@ struct sk_buff {
>  	struct nf_bridge_info	*nf_bridge;
>  #endif
>  
> -	int			iif;
> +	union {
> +		int		iif;
> +		int		sending_cpu;
> +	};

...

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 9977288..9e134f6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1965,6 +1965,7 @@ int dev_queue_xmit(struct sk_buff *skb)
>  	struct netdev_queue *txq;
>  	struct Qdisc *q;
>  	int rc = -ENOMEM;
> +	int cpu;
>  
>  	/* GSO will handle the following emulations directly. */
>  	if (netif_needs_gso(dev, skb))
> @@ -2000,6 +2001,7 @@ gso:
>  	 */
>  	rcu_read_lock_bh();
>  
> +	skb->sending_cpu = cpu = smp_processor_id();

There is one more problem: this will break things like act_mirred + ifb,
and other cases using skb->iif e.g. for filtering on virtual devices at
the xmit path.

Jarek P.

^ permalink raw reply

* Re: A generic kernel compatibilty code
From: Luis R. Rodriguez @ 2009-11-20 20:53 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-wireless, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <43e72e890911201251t6210ee19n177eaf003a4fffc-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Nov 20, 2009 at 12:51 PM, Luis R. Rodriguez <mcgrof-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Nov 20, 2009 at 12:45 PM, Luis R. Rodriguez <mcgrof-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Everyone and their mother reinvents the wheel when it comes to
>> backporting kernel modules. It a painful job and it seems to me an
>> alternative is possible. If we can write generic compatibilty code for
>> a new routine introduced on the next kernel how about just merging it
>> to the kernel under some generic compat module. This would be
>> completey ignored by everyone using the stable kernel but can be
>> copied by anyone doing backport work.
>>
>> So I'm thinking something as simple as a generic compat/comat.ko with
>> compat-2.6.32.[ch] files.
>
> FWIW, I meant a compat-2.6.32.[ch] and compat-2.6.31.[ch] and so on.
> All these would link to the compat.ko

I supose this could juse be a separate tree with some generic
compat.ko module. That might work better.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 net-next-2.6] net: Xmit Packet Steering (XPS)
From: David Miller @ 2009-11-20 20:53 UTC (permalink / raw)
  To: andi; +Cc: eric.dumazet, therbert, netdev
In-Reply-To: <87aayg539l.fsf@basil.nowhere.org>

From: Andi Kleen <andi@firstfloor.org>
Date: Fri, 20 Nov 2009 21:51:02 +0100

> -Andi (who is a bit sceptical and would rather see generic work for
> -this in slab)

SLAB does this to an extent, but it's the other things about
SKB freeing that are expensive and hits lots of cache misses
in these cases.

^ permalink raw reply

* Re: A generic kernel compatibilty code
From: Luis R. Rodriguez @ 2009-11-20 20:51 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-wireless, netdev
In-Reply-To: <43e72e890911201245r4de5b039hb2dd5011dabf2399@mail.gmail.com>

On Fri, Nov 20, 2009 at 12:45 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> Everyone and their mother reinvents the wheel when it comes to
> backporting kernel modules. It a painful job and it seems to me an
> alternative is possible. If we can write generic compatibilty code for
> a new routine introduced on the next kernel how about just merging it
> to the kernel under some generic compat module. This would be
> completey ignored by everyone using the stable kernel but can be
> copied by anyone doing backport work.
>
> So I'm thinking something as simple as a generic compat/comat.ko with
> compat-2.6.32.[ch] files.

FWIW, I meant a compat-2.6.32.[ch] and compat-2.6.31.[ch] and so on.
All these would link to the compat.ko

  Luis

^ permalink raw reply

* Re: [net-next-2.6 PATCH v7 2/7 RFC] TCPCT part 1b: generate Responder Cookie secret
From: David Miller @ 2009-11-20 20:51 UTC (permalink / raw)
  To: andi; +Cc: william.allen.simpson, netdev
In-Reply-To: <87eins53fu.fsf@basil.nowhere.org>

From: Andi Kleen <andi@firstfloor.org>
Date: Fri, 20 Nov 2009 21:47:17 +0100

> David Miller <davem@davemloft.net> writes:
> 
>> From: William Allen Simpson <william.allen.simpson@gmail.com>
>> Date: Fri, 20 Nov 2009 09:23:21 -0500
>>
>>> +static DEFINE_SPINLOCK(tcp_secret_locker);
>>
>> So connection creation scalability will be limited now because
>> we'll always have to go through this centralized spinlock even
>> for independent listening sockets, right?
> 
> I was about to complain about the same thing in a earlier version
> of this patch kit, but then I noticed the spin lock aquiring 
> is guarded by
> 
> if (unlikely(time_after_eq(jiffy, tcp_secret_generating->expires))) {
> 
> which presumably makes it rare enough?

Works for me.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: Xmit Packet Steering (XPS)
From: Andi Kleen @ 2009-11-20 20:51 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, Linux Netdev List
In-Reply-To: <4B05D8DC.7020907@gmail.com>

Eric Dumazet <eric.dumazet@gmail.com> writes:

> Here is first version of XPS.
>
> Goal of XPS is to free TX completed skbs by the cpu that submitted the transmit.
>
> Because I chose to union skb->iif with skb->sending_cpu, I chose
> to introduce a new xps_consume_skb(skb), and not generalize consume_skb() itself.
>
> This means that selected drivers must use new function to benefit from XPS
>
> Preliminary tests are quite good, especially on NUMA machines.
>
> Only NAPI drivers can use this new infrastructure (xps_consume_skb() cannot
> be called from hardirq context, only from softirq)
>
> I converted tg3 and pktgen for my tests

Do you have numbers on this? It seems like a lot of effort to avoid transfering
a few cache lines.

-Andi (who is a bit sceptical and would rather see generic work for this in slab)

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply

* Re: [net-next-2.6 PATCH v7 2/7 RFC] TCPCT part 1b: generate Responder Cookie secret
From: Andi Kleen @ 2009-11-20 20:47 UTC (permalink / raw)
  To: David Miller; +Cc: william.allen.simpson, netdev
In-Reply-To: <20091120.092228.216274668.davem@davemloft.net>

David Miller <davem@davemloft.net> writes:

> From: William Allen Simpson <william.allen.simpson@gmail.com>
> Date: Fri, 20 Nov 2009 09:23:21 -0500
>
>> +static DEFINE_SPINLOCK(tcp_secret_locker);
>
> So connection creation scalability will be limited now because
> we'll always have to go through this centralized spinlock even
> for independent listening sockets, right?

I was about to complain about the same thing in a earlier version
of this patch kit, but then I noticed the spin lock aquiring 
is guarded by

if (unlikely(time_after_eq(jiffy, tcp_secret_generating->expires))) {

which presumably makes it rare enough?

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply

* A generic kernel compatibilty code
From: Luis R. Rodriguez @ 2009-11-20 20:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-wireless, netdev

Everyone and their mother reinvents the wheel when it comes to
backporting kernel modules. It a painful job and it seems to me an
alternative is possible. If we can write generic compatibilty code for
a new routine introduced on the next kernel how about just merging it
to the kernel under some generic compat module. This would be
completey ignored by everyone using the stable kernel but can be
copied by anyone doing backport work.

So I'm thinking something as simple as a generic compat/comat.ko with
compat-2.6.32.[ch] files.

We've already backported everything needed for wireless drivers under
compat-wireless under this format down to even 2.6.25. I volunteer to
be the sucker for this if this is reasonable and given the shot to try
it.

If you would like a better idea of what I mean please check out this
git tree and check out the files under compat/*.[ch]

git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/compat-wireless-2.6.git

Things which *cannot* be backported through new defines or exported
symbols are handled manually through patches (check compat/patches/)
but no need for something like that upstream it seems.

Please let me know what you think.

 Luis

^ permalink raw reply

* Re: [RFC] p54: software hot-swap eeprom image
From: Kalle Valo @ 2009-11-20 20:32 UTC (permalink / raw)
  To: Dan Williams; +Cc: Christian Lamparter, linux-wireless, netdev
In-Reply-To: <1258746786.26965.83.camel@localhost.localdomain>

Dan Williams <dcbw@redhat.com> writes:

> On Fri, 2009-11-20 at 20:27 +0100, Christian Lamparter wrote:
>> This (aggregated) patch adds a new sysfs file "eeprom_overwrite"
>> which can be used to upload an customized eeprom image to the
>> driver during normal operation.

[...]

> Honestly I wonder if this would be better done with ethtool.  There's
> already ethtool --eeprom-dump, would it be so hard to do an ethtool
> --eeprom-load ?
>
> I can see a number of devices doing this, so in my mind it makes sense
> to have a generic facility for this sort of thing.  But then again it's
> not so common an operation and its fraught with danger :)

I think that devices without eeprom at all (eg. stlc4550, most wl1251
and all wl1271 chips) request_firmware() and a udev script is the best
way to go, as they will need the eeprom data from user space during
every boot. This was discussed few months back, but unfortunately I
have yet to seen an implementation of this :) Hopefully I find some
time to try this soon.

But for devices with eeprom, having support in ethtool sounds good to
me.

> Libertas could potentially use this, but libertas has *two* EEPROM
> images that it might need to update. So if ethtool did grow
> --eeprom-load we may want to account for more than one eeprom image.

Makes sense to mee.

-- 
Kalle Valo

^ permalink raw reply

* Re: [RFC] p54: software hot-swap eeprom image
From: Luis R. Rodriguez @ 2009-11-20 20:32 UTC (permalink / raw)
  To: Dan Williams
  Cc: Christian Lamparter, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1258746786.26965.83.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

On Fri, Nov 20, 2009 at 11:53 AM, Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 2009-11-20 at 20:27 +0100, Christian Lamparter wrote:
>> This (aggregated) patch adds a new sysfs file "eeprom_overwrite"
>> which can be used to upload an customized eeprom image to the
>> driver during normal operation.
>>
>> The old wlanX device will quickly vanish. And if the new EEPROM
>> image passes all sanity checks the device will reappear.
>>
>> Hopefully, this proves to be an adequate solution for everyone
>> with a bad/missing EEPROM data/chip.
>
> Honestly I wonder if this would be better done with ethtool.  There's
> already ethtool --eeprom-dump, would it be so hard to do an ethtool
> --eeprom-load ?
>
> I can see a number of devices doing this, so in my mind it makes sense
> to have a generic facility for this sort of thing.  But then again it's
> not so common an operation and its fraught with danger :)  Libertas
> could potentially use this, but libertas has *two* EEPROM images that it
> might need to update.  So if ethtool did grow --eeprom-load we may want
> to account for more than one eeprom image.
>
> What do people think?

Can't request_firmware() be generalized for this sort of stuff too?

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 net-next-2.6] net: Xmit Packet Steering (XPS)
From: Jarek Poplawski @ 2009-11-20 20:04 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, Linux Netdev List
In-Reply-To: <4B06AB96.8040805@gmail.com>

On Fri, Nov 20, 2009 at 03:45:42PM +0100, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > On 20-11-2009 00:46, Eric Dumazet wrote:
> >> Here is first version of XPS.
> >>
> >> Goal of XPS is to free TX completed skbs by the cpu that submitted the transmit.
> > 
> > But why?... OK, you write in another message about sock_wfree(). Then
> > how about users, who don't sock_wfree (routers)? Will there be any way
> > to disable it?
> 
> 
> This is open for discussion, but I saw no problem with routing workloads.

IMHO, it should depend on testing: if you can  prove there is a
distinct gain in "common" use case (which isn't probably a numa box
used e.g. for google.com or even kernel.org yet), and no visible
slowdown for such a router, then we could probably forget about
disabling, and look more at optimizations of the fast path.

> 
> sock_wfree() is not that expensive for tcp anyway.
> You also have a cost of kfreeing() two blocks of memory per skb, if allocation was done by another cpu.
> 
> If this happens to be a problem, we can immediately free packet if it 
> has no destructors :
> 
> At xmit time, initialize skb->sending_cpu like that
> 
> skb->sending_cpu = (skb->destructor) ? smp_processor_id() : 0xFFFF;
> 
> to make sure we dont touch too many cache lines at tx completion time.
> 
> 
> >> +/*
> >> + * XPS : Xmit Packet Steering
> >> + *
> >> + * TX completion packet freeing is performed on cpu that sent packet.
> >> + */
> >> +#if defined(CONFIG_SMP)
> > 
> > Shouldn't it be in the Makefile?
> 
> It is in Makefile too, I let it in prelim code to make it clear this was CONFIG_SMP only.

Aha! Now it's clear why I made this mistake. ;-)
> 
> > 
> > ...
> >> +/*
> >> + * called at end of net_rx_action()
> >> + * preemption (and cpu migration/offline/online) disabled
> >> + */
> >> +void xps_flush(void)
> >> +{
> >> +	int cpu, prevlen;
> >> +	struct sk_buff_head *head = per_cpu_ptr(xps_array, smp_processor_id());
> >> +	struct xps_pcpu_queue *q;
> >> +	struct sk_buff *skb;
> >> +
> >> +	for_each_cpu_mask_nr(cpu, __get_cpu_var(xps_cpus)) {
> >> +		q = &per_cpu(xps_pcpu_queue, cpu);
> >> +		if (cpu_online(cpu)) {
> >> +			spin_lock(&q->list.lock);
> > 
> > This lock probably needs irq disabling: let's say 2 cpus run this at
> > the same time and both are interrupted with these (previously
> > scheduled) IPIs?
> 
> Repeat after me :
> 
> lockdep is my friend, lockdep is my friend, lockdep is my friend... :)

Hmm... Actually, why did I have to do lockdep's job...
> 
> Seriously, I must think again on this locking schem.
> 
> >> +static void remote_free_skb_list(void *arg)
> >> +{
> >> +	struct sk_buff *last;
> >> +	struct softnet_data *sd;
> >> +	struct xps_pcpu_queue *q = arg; /* &__get_cpu_var(xps_pcpu_queue); */
> >> +
> >> +	spin_lock(&q->list.lock);
> >> +
> >> +	last = q->list.prev;
> > 
> > Is q->list handled in case this cpu goes down before this IPI is
> > triggered?
> 
> 
> [block migration] (how ? this is the question)
> 
> if (cpu_online(cpu)) { 
> 	give_work_to_cpu(cpu);
> 	trigger IPI
> } else {
> 	handle_work_ourself()
> }
> 
> [unblock migration]
> 
> General problem is : what guards cpu going off line between the if (cpu_online(cpu))
> and the IPI.
> I dont know yet, but it seems that disabling preemption is enough to get this
> guarantee. This seems strange.
> 
> We can add a notifier (or better call a function from existing one : dev_cpu_callback()) to 
> flush this queue when necessary.

Using dev_cpu_callback() looks quite obvious to me.

Jarek P.

^ permalink raw reply

* RE: Compile Error for TI Davinci
From: Govindarajan, Sriramakrishnan @ 2009-11-20 19:56 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org,
	davinci-linux-open-source@linux.davincidsp.com,
	Subrahmanya, Chaithrika
In-Reply-To: <20091120.095000.265125164.davem@davemloft.net>



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, November 20, 2009 11:20 PM
> To: Govindarajan, Sriramakrishnan
> Cc: netdev@vger.kernel.org; davinci-linux-open-
> source@linux.davincidsp.com; Subrahmanya, Chaithrika
> Subject: Re: Compile Error for TI Davinci
> 
> From: "Govindarajan, Sriramakrishnan" <srk@ti.com>
> Date: Fri, 20 Nov 2009 17:36:38 +0530
> 
> > Patch for "TI DaVinci EMAC: Clear statistics register properly"(commit-
> 0fe7463a35aadfaf22d1ca58325ab3851b8d757c on net-next) has dependency on
> > Patch "TI DaVinci EMAC: Minor macro related updates "(commit -
> 69ef9694099802f7feeb23182dfb869e7c5f76f0 on net-next).
> >
> > The first patch has been pushed upstream while the later still remains
> on
> > Net-next. Due to the dependency between these two, we are seeing
> compilation
> > Failure when we build for Davinci.
> >
> > Can you please push "TI DaVinci EMAC: Minor macro related updates" -
> 69ef9694099802f7feeb23182dfb869e7c5f76f0 up-stream .
> 
> This should not be happening, because yesterday I merged Linus's
> tree into net-2.6 and afterwards I merged net-2.6 into net-next-2.6
> 
> Please verify that the problem still occurs.
[Sriram]  David the commit " TI DaVinci EMAC: Minor macro related updates" exists only on the net-next-2.6 git. As mentioned in my earlier email, subsequent patch " DaVinci EMAC: Clear statistics register properly" is dependent on this  and is reflected in the net-2.6 and Linus's GIT. Hence compilation for Davinci would fail on both net-2.6 and Linus's GIT

I just verified the failure by building against the tip of Linus's GIT
Regards
Sriram


^ permalink raw reply

* Re: [RFC] p54: software hot-swap eeprom image
From: Dan Williams @ 2009-11-20 19:53 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: linux-wireless, netdev
In-Reply-To: <200911202027.13284.chunkeey@googlemail.com>

On Fri, 2009-11-20 at 20:27 +0100, Christian Lamparter wrote:
> This (aggregated) patch adds a new sysfs file "eeprom_overwrite"
> which can be used to upload an customized eeprom image to the
> driver during normal operation.
> 
> The old wlanX device will quickly vanish. And if the new EEPROM
> image passes all sanity checks the device will reappear.
> 
> Hopefully, this proves to be an adequate solution for everyone
> with a bad/missing EEPROM data/chip.

Honestly I wonder if this would be better done with ethtool.  There's
already ethtool --eeprom-dump, would it be so hard to do an ethtool
--eeprom-load ?

I can see a number of devices doing this, so in my mind it makes sense
to have a generic facility for this sort of thing.  But then again it's
not so common an operation and its fraught with danger :)  Libertas
could potentially use this, but libertas has *two* EEPROM images that it
might need to update.  So if ethtool did grow --eeprom-load we may want
to account for more than one eeprom image.

What do people think?

Dan

> ---
> there are plenty of bugs left!
> 
> would be nice to hear from p54spi & p54pci folks!
> 
> Regards,
> 	Chr
> ---
> diff --git a/drivers/net/wireless/p54/eeprom.c b/drivers/net/wireless/p54/eeprom.c
> index 8e3818f..e5c94d9 100644
> --- a/drivers/net/wireless/p54/eeprom.c
> +++ b/drivers/net/wireless/p54/eeprom.c
> @@ -121,25 +121,20 @@ static int p54_generate_band(struct ieee80211_hw *dev,
>  			     enum ieee80211_band band)
>  {
>  	struct p54_common *priv = dev->priv;
> -	struct ieee80211_supported_band *tmp, *old;
> +	struct ieee80211_supported_band *old_band, *new_band;
> +	struct ieee80211_channel *tmp, *old_chan, *new_chan;
>  	unsigned int i, j;
> -	int ret = -ENOMEM;
> +	int old_chan_num, ret = 0;
>  
>  	if ((!list->entries) || (!list->band_channel_num[band]))
>  		return -EINVAL;
>  
> -	tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
> -	if (!tmp)
> -		goto err_out;
> -
> -	tmp->channels = kzalloc(sizeof(struct ieee80211_channel) *
> -				list->band_channel_num[band], GFP_KERNEL);
> -	if (!tmp->channels)
> -		goto err_out;
> -
> -	ret = p54_fill_band_bitrates(dev, tmp, band);
> -	if (ret)
> -		goto err_out;
> +	tmp = kzalloc(sizeof(struct ieee80211_channel) *
> +		list->band_channel_num[band], GFP_KERNEL);
> +	if (!tmp) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
>  
>  	for (i = 0, j = 0; (j < list->band_channel_num[band]) &&
>  			   (i < list->entries); i++) {
> @@ -161,8 +156,8 @@ static int p54_generate_band(struct ieee80211_hw *dev,
>  			continue;
>  		}
>  
> -		tmp->channels[j].band = list->channels[i].band;
> -		tmp->channels[j].center_freq = list->channels[i].freq;
> +		tmp[j].band = list->channels[i].band;
> +		tmp[j].center_freq = list->channels[i].freq;
>  		j++;
>  	}
>  
> @@ -172,25 +167,60 @@ static int p54_generate_band(struct ieee80211_hw *dev,
>  		       "2 GHz" : "5 GHz");
>  
>  		ret = -ENODATA;
> -		goto err_out;
> +		goto out;
>  	}
>  
> -	tmp->n_channels = j;
> -	old = priv->band_table[band];
> -	priv->band_table[band] = tmp;
> -	if (old) {
> -		kfree(old->channels);
> -		kfree(old);
> +	old_band = priv->band_table[band];
> +	if (old_band) {
> +		old_chan_num = old_band->n_channels;
> +		old_chan = old_band->channels;
> +		old_band->n_channels = 0;
> +		old_band->n_bitrates = 0;
> +	} else {
> +		old_chan_num = 0;
> +		old_chan = NULL;
>  	}
>  
> -	return 0;
> +	if (j < old_chan_num) {
> +		printk(KERN_ERR "%s: Channel list can only be extended.\n",
> +		       wiphy_name(dev->wiphy));
> +		goto out;
> +	}
> +
> +	new_band = krealloc(old_band, sizeof(*new_band), GFP_KERNEL);
> +	if (!new_band) {
> +		printk(KERN_ERR "%s: Unable to modify band table.\n",
> +		       wiphy_name(dev->wiphy));
> +
> +		goto out;
> +	}
> +
> +	new_band->band = band;
> +	new_band->ht_cap.ht_supported = false;
>  
> -err_out:
> -	if (tmp) {
> -		kfree(tmp->channels);
> -		kfree(tmp);
> +	new_chan = krealloc(old_chan, j * sizeof(struct ieee80211_channel),
> +			    GFP_KERNEL);
> +
> +	if (!new_chan) {
> +		printk(KERN_ERR "%s: Unable to modify band channel table.\n",
> +		       wiphy_name(dev->wiphy));
> +
> +		goto out;
>  	}
>  
> +	memcpy(new_chan, tmp, j * sizeof(struct ieee80211_channel));
> +
> +	new_band->channels = new_chan;
> +	new_band->n_channels = j;
> +
> +	ret = p54_fill_band_bitrates(dev, new_band, band);
> +	if (ret)
> +		goto out;
> +
> +	priv->band_table[band] = new_band;
> +
> +out:
> +	kfree(tmp);
>  	return ret;
>  }
>  
> @@ -533,7 +563,19 @@ static struct p54_cal_database *p54_convert_db(struct pda_custom_wrapper *src,
>  	return dst;
>  }
>  
> -int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
> +static void p54_reset_phydata(struct p54_common *priv)
> +{
> +	memset(&priv->rssical_db, 0, sizeof(priv->rssical_db));
> +	priv->iq_autocal_len = 0;
> +	kfree(priv->output_limit);
> +	kfree(priv->curve_data);
> +	kfree(priv->iq_autocal);
> +	priv->output_limit = NULL;
> +	priv->curve_data = NULL;
> +	priv->iq_autocal = NULL;
> +}
> +
> +int p54_parse_eeprom(struct ieee80211_hw *dev, const void *eeprom, int len)
>  {
>  	struct p54_common *priv = dev->priv;
>  	struct eeprom_pda_wrap *wrap;
> @@ -543,10 +585,14 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
>  	int err;
>  	u8 *end = (u8 *)eeprom + len;
>  	u16 synth = 0;
> +	bool has_rssical = false, has_mac = false, has_country = false;
>  
>  	wrap = (struct eeprom_pda_wrap *) eeprom;
>  	entry = (void *)wrap->data + le16_to_cpu(wrap->len);
>  
> +	mutex_lock(&priv->conf_mutex);
> +	p54_reset_phydata(priv);
> +
>  	/* verify that at least the entry length/code fits */
>  	while ((u8 *)entry <= end - sizeof(*entry)) {
>  		entry_len = le16_to_cpu(entry->len);
> @@ -558,21 +604,34 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
>  
>  		switch (le16_to_cpu(entry->code)) {
>  		case PDR_MAC_ADDRESS:
> +			if (has_mac)
> +				break;
> +
> +			has_mac = true;
> +
>  			if (data_len != ETH_ALEN)
>  				break;
> +
>  			SET_IEEE80211_PERM_ADDR(dev, entry->data);
>  			break;
> +
>  		case PDR_PRISM_PA_CAL_OUTPUT_POWER_LIMITS:
>  			if (priv->output_limit)
>  				break;
> +
>  			err = p54_convert_output_limits(dev, entry->data,
>  							data_len);
>  			if (err)
>  				goto err;
>  			break;
> +
>  		case PDR_PRISM_PA_CAL_CURVE_DATA: {
>  			struct pda_pa_curve_data *curve_data =
>  				(struct pda_pa_curve_data *)entry->data;
> +
> +			if (priv->curve_data)
> +				break;
> +
>  			if (data_len < sizeof(*curve_data)) {
>  				err = -EINVAL;
>  				goto err;
> @@ -597,7 +656,11 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
>  				goto err;
>  			}
>  			break;
> +
>  		case PDR_PRISM_ZIF_TX_IQ_CALIBRATION:
> +			if (priv->iq_autocal)
> +				break;
> +
>  			priv->iq_autocal = kmalloc(data_len, GFP_KERNEL);
>  			if (!priv->iq_autocal) {
>  				err = -ENOMEM;
> @@ -607,11 +670,22 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
>  			memcpy(priv->iq_autocal, entry->data, data_len);
>  			priv->iq_autocal_len = data_len / sizeof(struct pda_iq_autocal_entry);
>  			break;
> +
>  		case PDR_DEFAULT_COUNTRY:
> +			if (has_country)
> +				break;
> +
> +			has_country = true;
> +
>  			p54_parse_default_country(dev, entry->data, data_len);
>  			break;
> +
>  		case PDR_INTERFACE_LIST:
>  			tmp = entry->data;
> +
> +			if (synth)
> +				break;
> +
>  			while ((u8 *)tmp < entry->data + data_len) {
>  				struct exp_if *exp_if = tmp;
>  				if (exp_if->if_id == cpu_to_le16(IF_ID_ISL39000))
> @@ -619,22 +693,38 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
>  				tmp += sizeof(*exp_if);
>  			}
>  			break;
> +
>  		case PDR_HARDWARE_PLATFORM_COMPONENT_ID:
> +			if (priv->version)
> +				break;
> +
>  			if (data_len < 2)
>  				break;
>  			priv->version = *(u8 *)(entry->data + 1);
>  			break;
> +
>  		case PDR_RSSI_LINEAR_APPROXIMATION:
>  		case PDR_RSSI_LINEAR_APPROXIMATION_DUAL_BAND:
>  		case PDR_RSSI_LINEAR_APPROXIMATION_EXTENDED:
> +			if (has_rssical)
> +				break;
> +
> +			has_rssical = true;
> +
>  			p54_parse_rssical(dev, entry->data, data_len,
>  					  le16_to_cpu(entry->code));
>  			break;
> +
>  		case PDR_RSSI_LINEAR_APPROXIMATION_CUSTOM: {
>  			__le16 *src = (void *) entry->data;
>  			s16 *dst = (void *) &priv->rssical_db;
>  			int i;
>  
> +			if (has_rssical)
> +				break;
> +
> +			has_rssical = true;
> +
>  			if (data_len != sizeof(priv->rssical_db)) {
>  				err = -EINVAL;
>  				goto err;
> @@ -644,24 +734,30 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
>  				*(dst++) = (s16) le16_to_cpu(*(src++));
>  			}
>  			break;
> +
>  		case PDR_PRISM_PA_CAL_OUTPUT_POWER_LIMITS_CUSTOM: {
>  			struct pda_custom_wrapper *pda = (void *) entry->data;
>  			if (priv->output_limit || data_len < sizeof(*pda))
>  				break;
> +
>  			priv->output_limit = p54_convert_db(pda, data_len);
>  			}
>  			break;
> +
>  		case PDR_PRISM_PA_CAL_CURVE_DATA_CUSTOM: {
>  			struct pda_custom_wrapper *pda = (void *) entry->data;
>  			if (priv->curve_data || data_len < sizeof(*pda))
>  				break;
> +
>  			priv->curve_data = p54_convert_db(pda, data_len);
>  			}
>  			break;
> +
>  		case PDR_END:
>  			/* make it overrun */
>  			entry_len = len;
>  			break;
> +
>  		default:
>  			break;
>  		}
> @@ -670,7 +766,11 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
>  	}
>  
>  	if (!synth || !priv->iq_autocal || !priv->output_limit ||
> -	    !priv->curve_data) {
> +	    !priv->curve_data || !has_rssical) {
> +		printk(KERN_INFO "%d %p %p %p %d\n",
> +			synth, priv->iq_autocal, priv->output_limit,
> +			priv->curve_data, has_rssical);
> +
>  		printk(KERN_ERR "%s: not all required entries found in eeprom!\n",
>  			wiphy_name(dev->wiphy));
>  		err = -EINVAL;
> @@ -708,18 +808,18 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
>  		wiphy_name(dev->wiphy),	dev->wiphy->perm_addr, priv->version,
>  		p54_rf_chips[priv->rxhw]);
>  
> +	mutex_unlock(&priv->conf_mutex);
> +
>  	return 0;
>  
>  err:
> -	kfree(priv->iq_autocal);
> -	kfree(priv->output_limit);
> -	kfree(priv->curve_data);
> -	priv->iq_autocal = NULL;
> -	priv->output_limit = NULL;
> -	priv->curve_data = NULL;
> +	p54_reset_phydata(priv);
>  
>  	printk(KERN_ERR "%s: eeprom parse failed!\n",
>  		wiphy_name(dev->wiphy));
> +
> +	mutex_unlock(&priv->conf_mutex);
> +
>  	return err;
>  }
>  EXPORT_SYMBOL_GPL(p54_parse_eeprom);
> @@ -753,8 +853,39 @@ int p54_read_eeprom(struct ieee80211_hw *dev)
>  	}
>  
>  	ret = p54_parse_eeprom(dev, eeprom, offset);
> +
>  free:
>  	kfree(eeprom);
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(p54_read_eeprom);
> +
> +ssize_t p54_overwrite_eeprom(struct ieee80211_hw *dev,
> +			     const char *buf, size_t len)
> +{
> +	struct p54_common *priv = dev->priv;
> +	int err;
> +
> +	if (len < 64 || len > 0x2020)
> +		return -EINVAL;
> +
> +	mutex_lock(&priv->sysfs_mutex);
> +	p54_unregister_common(dev);
> +
> +	err = p54_parse_eeprom(dev, buf, len);
> +	if (err)
> +		goto out;
> +
> +	err = p54_register_common(dev, priv->pdev);
> +	if (err)
> +		goto out;
> +
> +out:
> +
> +	if (err)
> +		dev_err(priv->pdev, "failed to (re)-register device.\n");
> +
> +	mutex_unlock(&priv->sysfs_mutex);
> +	return (err) ? err : len;
> +}
> +EXPORT_SYMBOL_GPL(p54_overwrite_eeprom);
> diff --git a/drivers/net/wireless/p54/main.c b/drivers/net/wireless/p54/main.c
> index 18012db..e1395d4 100644
> --- a/drivers/net/wireless/p54/main.c
> +++ b/drivers/net/wireless/p54/main.c
> @@ -582,6 +582,7 @@ struct ieee80211_hw *p54_init_common(size_t priv_data_len)
>  	dev->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
>  
>  	mutex_init(&priv->conf_mutex);
> +	mutex_init(&priv->sysfs_mutex);
>  	mutex_init(&priv->eeprom_mutex);
>  	init_completion(&priv->eeprom_comp);
>  	init_completion(&priv->beacon_comp);
> @@ -596,6 +597,9 @@ int p54_register_common(struct ieee80211_hw *dev, struct device *pdev)
>  	struct p54_common *priv = dev->priv;
>  	int err;
>  
> +	if (priv->registered)
> +		return -EBUSY;
> +
>  	err = ieee80211_register_hw(dev);
>  	if (err) {
>  		dev_err(pdev, "Cannot register device (%d).\n", err);
> @@ -607,8 +611,9 @@ int p54_register_common(struct ieee80211_hw *dev, struct device *pdev)
>  	if (err)
>  		return err;
>  #endif /* CONFIG_P54_LEDS */
> -
> +	priv->pdev = pdev;
>  	dev_info(pdev, "is registered as '%s'\n", wiphy_name(dev->wiphy));
> +	priv->registered = true;
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(p54_register_common);
> @@ -618,6 +623,11 @@ void p54_free_common(struct ieee80211_hw *dev)
>  	struct p54_common *priv = dev->priv;
>  	unsigned int i;
>  
> +	BUG_ON(priv->registered);
> +
> +	mutex_destroy(&priv->conf_mutex);
> +	mutex_destroy(&priv->eeprom_mutex);
> +
>  	for (i = 0; i < IEEE80211_NUM_BANDS; i++)
>  		kfree(priv->band_table[i]);
>  
> @@ -637,12 +647,14 @@ void p54_unregister_common(struct ieee80211_hw *dev)
>  {
>  	struct p54_common *priv = dev->priv;
>  
> +	if (!priv->registered)
> +		return ;
> +
>  #ifdef CONFIG_P54_LEDS
>  	p54_unregister_leds(priv);
>  #endif /* CONFIG_P54_LEDS */
>  
> -	ieee80211_unregister_hw(dev);
> -	mutex_destroy(&priv->conf_mutex);
> -	mutex_destroy(&priv->eeprom_mutex);
> +	ieee80211_unregister_hw(priv->hw);
> +	priv->registered = false;
>  }
>  EXPORT_SYMBOL_GPL(p54_unregister_common);
> diff --git a/drivers/net/wireless/p54/p54.h b/drivers/net/wireless/p54/p54.h
> index 1afc394..3e097e0 100644
> --- a/drivers/net/wireless/p54/p54.h
> +++ b/drivers/net/wireless/p54/p54.h
> @@ -159,6 +159,7 @@ struct p54_led_dev {
>  
>  struct p54_common {
>  	struct ieee80211_hw *hw;
> +	struct device *pdev;
>  	struct ieee80211_vif *vif;
>  	void (*tx)(struct ieee80211_hw *dev, struct sk_buff *skb);
>  	int (*open)(struct ieee80211_hw *dev);
> @@ -166,6 +167,7 @@ struct p54_common {
>  	struct sk_buff_head tx_pending;
>  	struct sk_buff_head tx_queue;
>  	struct mutex conf_mutex;
> +	bool registered;
>  
>  	/* memory management (as seen by the firmware) */
>  	u32 rx_start;
> @@ -233,14 +235,17 @@ struct p54_common {
>  	void *eeprom;
>  	struct completion eeprom_comp;
>  	struct mutex eeprom_mutex;
> +	struct mutex sysfs_mutex;
>  };
>  
>  /* interfaces for the drivers */
>  int p54_rx(struct ieee80211_hw *dev, struct sk_buff *skb);
>  void p54_free_skb(struct ieee80211_hw *dev, struct sk_buff *skb);
>  int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw);
> -int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len);
> +int p54_parse_eeprom(struct ieee80211_hw *dev, const void *eeprom, int len);
>  int p54_read_eeprom(struct ieee80211_hw *dev);
> +ssize_t p54_overwrite_eeprom(struct ieee80211_hw *dev,
> +			    const char *eeprom, size_t len);
>  
>  struct ieee80211_hw *p54_init_common(size_t priv_data_len);
>  int p54_register_common(struct ieee80211_hw *dev, struct device *pdev);
> diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
> index e3a4c90..b8040e4 100644
> --- a/drivers/net/wireless/p54/p54pci.c
> +++ b/drivers/net/wireless/p54/p54pci.c
> @@ -466,6 +466,18 @@ static int p54p_open(struct ieee80211_hw *dev)
>  	return 0;
>  }
>  
> +static ssize_t p54p_sysfs_overwrite_eeprom(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)
> +{
> +	struct p54p_priv *priv = dev_get_drvdata(dev);
> +
> +	return p54_overwrite_eeprom(priv->common.hw, buf, count);
> +}
> +
> +static DEVICE_ATTR(eeprom_overwrite, S_IWUSR,
> +		   NULL, p54p_sysfs_overwrite_eeprom);
> +
>  static int __devinit p54p_probe(struct pci_dev *pdev,
>  				const struct pci_device_id *id)
>  {
> @@ -561,6 +573,13 @@ static int __devinit p54p_probe(struct pci_dev *pdev,
>  	if (err)
>  		goto err_free_common;
>  
> +	err = device_create_file(&pdev->dev, &dev_attr_eeprom_overwrite);
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "failed to create sysfs file "
> +				    "eeprom_overwrite.\n");
> +		goto err_free_common;
> +	}
> +
>  	return 0;
>  
>   err_free_common:
> @@ -591,6 +610,8 @@ static void __devexit p54p_remove(struct pci_dev *pdev)
>  		return;
>  
>  	p54_unregister_common(dev);
> +	device_remove_file(&pdev->dev, &dev_attr_eeprom_overwrite);
> +
>  	priv = dev->priv;
>  	release_firmware(priv->firmware);
>  	pci_free_consistent(pdev, sizeof(*priv->ring_control),
> diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
> index afd26bf..6fb0863 100644
> --- a/drivers/net/wireless/p54/p54spi.c
> +++ b/drivers/net/wireless/p54/p54spi.c
> @@ -592,6 +592,18 @@ static void p54spi_op_stop(struct ieee80211_hw *dev)
>  	mutex_unlock(&priv->mutex);
>  }
>  
> +static ssize_t p54s_sysfs_overwrite_eeprom(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)
> +{
> +	struct p54s_priv *priv = dev_get_drvdata(dev);
> +
> +	return p54_overwrite_eeprom(priv->common.hw, buf, count);
> +}
> +
> +static DEVICE_ATTR(eeprom_overwrite, S_IWUSR,
> +		   NULL, p54s_sysfs_overwrite_eeprom);
> +
>  static int __devinit p54spi_probe(struct spi_device *spi)
>  {
>  	struct p54s_priv *priv = NULL;
> @@ -667,7 +679,9 @@ static int __devinit p54spi_probe(struct spi_device *spi)
>  	if (ret)
>  		goto err_free_common;
>  
> -	return 0;
> +        ret = device_create_file(&spi->dev, &dev_attr_eeprom_overwrite);
> +
> +	return ret;
>  
>  err_free_common:
>  	p54_free_common(priv->hw);
> @@ -680,6 +694,9 @@ static int __devexit p54spi_remove(struct spi_device *spi)
>  
>  	p54_unregister_common(priv->hw);
>  
> +	device_remove_file(&spi->dev, &dev_attr_eeprom_overwrite);
> +	dev_set_drvdata(&spi->dev, NULL);
> +
>  	free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
>  
>  	gpio_free(p54spi_gpio_power);
> diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
> index 92af9b9..be699d6 100644
> --- a/drivers/net/wireless/p54/p54usb.c
> +++ b/drivers/net/wireless/p54/p54usb.c
> @@ -874,6 +874,19 @@ static void p54u_stop(struct ieee80211_hw *dev)
>  	return;
>  }
>  
> +static ssize_t p54u_sysfs_overwrite_eeprom(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)
> +{
> +        struct ieee80211_hw *hw = (struct ieee80211_hw *)
> +                usb_get_intfdata(to_usb_interface(dev));
> +
> +	return p54_overwrite_eeprom(hw, buf, count);
> +}
> +
> +static DEVICE_ATTR(eeprom_overwrite, S_IWUSR,
> +                   NULL, p54u_sysfs_overwrite_eeprom);
> +
>  static int __devinit p54u_probe(struct usb_interface *intf,
>  				const struct usb_device_id *id)
>  {
> @@ -959,7 +972,9 @@ static int __devinit p54u_probe(struct usb_interface *intf,
>  	if (err)
>  		goto err_free_fw;
>  
> -	return 0;
> +	err = device_create_file(&intf->dev, &dev_attr_eeprom_overwrite);
> +
> +	return err;
>  
>  err_free_fw:
>  	release_firmware(priv->fw);
> @@ -982,6 +997,9 @@ static void __devexit p54u_disconnect(struct usb_interface *intf)
>  	p54_unregister_common(dev);
>  
>  	priv = dev->priv;
> +	device_remove_file(&intf->dev, &dev_attr_eeprom_overwrite); 
> +
> +	usb_set_intfdata(intf, NULL);
>  	usb_put_dev(interface_to_usbdev(intf));
>  	release_firmware(priv->fw);
>  	p54_free_common(dev);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* [PATCH] net: ETHOC should depend on HAS_DMA (was: Re: [PATCH] ETHOC: fix build breakage on s390)
From: Geert Uytterhoeven @ 2009-11-20 19:19 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: David S. Miller, Thierry Reding, Martin Schwidefsky, netdev,
	linux-kernel, linux-s390

On Mon, Mar 30, 2009 at 10:30, Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>
> Let driver depend on HAS_IOMEM to avoid build breakage on s390:
>
>  CC      drivers/net/ethoc.o
> drivers/net/ethoc.c: In function 'ethoc_read':
> drivers/net/ethoc.c:221: error: implicit declaration of function 'ioread32'
> drivers/net/ethoc.c: In function 'ethoc_write':
> drivers/net/ethoc.c:226: error: implicit declaration of function 'iowrite32'
> drivers/net/ethoc.c: In function 'ethoc_rx':
> drivers/net/ethoc.c:405: error: implicit declaration of function 'memcpy_fromio'
> drivers/net/ethoc.c: In function 'ethoc_start_xmit':
> drivers/net/ethoc.c:828: error: implicit declaration of function 'memcpy_toio'
>
> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>  drivers/net/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6/drivers/net/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/net/Kconfig
> +++ linux-2.6/drivers/net/Kconfig
> @@ -974,7 +974,7 @@ config ENC28J60_WRITEVERIFY
>
>  config ETHOC
>        tristate "OpenCores 10/100 Mbps Ethernet MAC support"
> -       depends on NET_ETHERNET
> +       depends on NET_ETHERNET && HAS_IOMEM

This is not sufficient, it has to depend on HAS_DMA, too:

From cdb6fc81662e9afd6019221acc7f56d2d7dcd161 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Fri, 20 Nov 2009 20:07:09 +0100
Subject: [PATCH] net: ETHOC should depend on HAS_DMA

When building for Sun 3:

drivers/net/ethoc.c:1091: undefined reference to `dma_free_coherent'
drivers/built-in.o: In function `ethoc_probe':
drivers/net/ethoc.c:965: undefined reference to `dma_alloc_coherent'
drivers/net/ethoc.c:1063: undefined reference to `dma_free_coherent'

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 8015ecc..9e7c76e 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -993,7 +993,7 @@ config ENC28J60_WRITEVERIFY

 config ETHOC
 	tristate "OpenCores 10/100 Mbps Ethernet MAC support"
-	depends on NET_ETHERNET && HAS_IOMEM
+	depends on NET_ETHERNET && HAS_IOMEM && HAS_DMA
 	select MII
 	select PHYLIB
 	select CRC32
-- 
1.6.0.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply related

* Re: [PATCH] ixgbe: move tc variable to CONFIG_IXGBE_DCB
From: Jeff Kirsher @ 2009-11-20 19:16 UTC (permalink / raw)
  To: David Miller
  Cc: jaswinder, yi.zou, peter.p.waskiewicz.jr, netdev, linux-kernel
In-Reply-To: <20091120.111355.141716974.davem@davemloft.net>

On Fri, Nov 20, 2009 at 11:13, David Miller <davem@davemloft.net> wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Fri, 20 Nov 2009 06:24:21 -0800
>
>> On Fri, Nov 20, 2009 at 06:02, Jaswinder Singh Rajput
>> <jaswinder@kernel.org> wrote:
>>>
>>> tc is required by CONFIG_IXGBE_DCB.
>>> This also fixes compilation warning:
>>>
>>>  drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_tx_is_paused’:
>>>  drivers/net/ixgbe/ixgbe_main.c:245: warning: unused variable ‘tc’
>>>
>>> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
>  ...
>> I just submitted a patch to fix this.  So NAK.
>
> But his is much cleaner that your's, less ifdefs.  I think we
> should therefore use Jasdinder's version.
>
>

:) That is fine.  The end result is the same.

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [PATCH] ixgbe: move tc variable to CONFIG_IXGBE_DCB
From: David Miller @ 2009-11-20 19:13 UTC (permalink / raw)
  To: jeffrey.t.kirsher
  Cc: jaswinder, yi.zou, peter.p.waskiewicz.jr, netdev, linux-kernel
In-Reply-To: <9929d2390911200624mf352c28u55a8b2aee8b52fa3@mail.gmail.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 20 Nov 2009 06:24:21 -0800

> On Fri, Nov 20, 2009 at 06:02, Jaswinder Singh Rajput
> <jaswinder@kernel.org> wrote:
>>
>> tc is required by CONFIG_IXGBE_DCB.
>> This also fixes compilation warning:
>>
>>  drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_tx_is_paused’:
>>  drivers/net/ixgbe/ixgbe_main.c:245: warning: unused variable ‘tc’
>>
>> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
 ...
> I just submitted a patch to fix this.  So NAK.

But his is much cleaner that your's, less ifdefs.  I think we
should therefore use Jasdinder's version.


^ permalink raw reply

* Re: [PATCH 1/2] rps: core implementation
From: Jarek Poplawski @ 2009-11-20 19:00 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, netdev
In-Reply-To: <65634d660911200908l5acbf3d0v5fc9a2e1adb7c3a5@mail.gmail.com>

On Fri, Nov 20, 2009 at 09:08:10AM -0800, Tom Herbert wrote:
> > The description reads: "This solution queues packets early on in the
> > receive path on the backlog queues of other CPUs.", so I'm not sure
> > it's intended.
> 
> That is precisely the intent.  Getting packets quickly distributed to
> the target cpus maximizes parallelism and reduces latency.

Then precisely the intent is "the backlog queues of target CPUs".
"The backlog queues of other CPUs" may suggest that e.g. one cpu is
only doing distribution etc.

> 
> Did you test it like this (and it was visibly worse)?:
> >
> >        if (cpu < 0 || cpu == smp_processor_id())
> >
> >> +             return __netif_receive_skb(skb);
> >> +     else
> >> +             return enqueue_to_backlog(skb, cpu);
> >> +}
> >> +
> This increases overall latency due to head of line blocking which will
> outweigh the benefits of optimizing for this one case.

The way I asked should suggest I "suspected" it's on purpose, and was
curious about "digits", but thanks for confirming this.

Jarek P.

^ 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