Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] RPS: support 802.1q and pppoe session
From: Eric Dumazet @ 2010-03-25  5:13 UTC (permalink / raw)
  To: xiaosuo, David Miller; +Cc: Tom Herbert, netdev
In-Reply-To: <4BAAE6E9.6030103@gmail.com>

Le jeudi 25 mars 2010 à 12:30 +0800, Changli Gao a écrit :
> +#ifdef CONFIG_SMP
> +#include <linux/if_pppox.h>
> +#endif

BTW, when I see this kind of illogical thing, I do think we should have
a CONFIG_RPS setting...

David, may I submit again my former patch adding CONFIG_RPS, but not a
user selectable boolean ?

[PATCH net-next-2.6] rps: add CONFIG_RPS

RPS currently depends on SMP and SYSFS

Adding a CONFIG_RPS makes sense in case this requirement changes in the
future. This patch saves about 1500 bytes of kernel text in case SMP is
on but SYSFS is off.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/netdevice.h |    4 ++++
 net/Kconfig               |    5 +++++
 net/core/dev.c            |   29 +++++++++++++++++++----------
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c96c41e..53c272f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -531,6 +531,7 @@ struct netdev_queue {
 	unsigned long		tx_dropped;
 } ____cacheline_aligned_in_smp;
 
+#ifdef CONFIG_RPS
 /*
  * This structure holds an RPS map which can be of variable length.  The
  * map is an array of CPUs.
@@ -549,6 +550,7 @@ struct netdev_rx_queue {
 	struct netdev_rx_queue *first;
 	atomic_t count;
 } ____cacheline_aligned_in_smp;
+#endif
 
 /*
  * This structure defines the management hooks for network devices.
@@ -897,12 +899,14 @@ struct net_device {
 
 	unsigned char		broadcast[MAX_ADDR_LEN];	/* hw bcast add	*/
 
+#ifdef CONFIG_RPS
 	struct kset		*queues_kset;
 
 	struct netdev_rx_queue	*_rx;
 
 	/* Number of RX queues allocated at alloc_netdev_mq() time  */
 	unsigned int		num_rx_queues;
+#endif
 
 	struct netdev_queue	rx_queue;
 
diff --git a/net/Kconfig b/net/Kconfig
index 041c35e..6851464 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -203,6 +203,11 @@ source "net/ieee802154/Kconfig"
 source "net/sched/Kconfig"
 source "net/dcb/Kconfig"
 
+config RPS
+	boolean
+	depends on SMP && SYSFS
+	default y
+
 menu "Network testing"
 
 config NET_PKTGEN
diff --git a/net/core/dev.c b/net/core/dev.c
index 5e3dc28..bcb3ed2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2177,7 +2177,7 @@ int weight_p __read_mostly = 64;            /* old backlog weight */
 
 DEFINE_PER_CPU(struct netif_rx_stats, netdev_rx_stat) = { 0, };
 
-#ifdef CONFIG_SMP
+#ifdef CONFIG_RPS
 /*
  * get_rps_cpu is called from netif_receive_skb and returns the target
  * CPU from the RPS map of the receiving queue for a given skb.
@@ -2325,7 +2325,7 @@ enqueue:
 
 		/* Schedule NAPI for backlog device */
 		if (napi_schedule_prep(&queue->backlog)) {
-#ifdef CONFIG_SMP
+#ifdef CONFIG_RPS
 			if (cpu != smp_processor_id()) {
 				struct rps_remote_softirq_cpus *rcpus =
 				    &__get_cpu_var(rps_remote_softirq_cpus);
@@ -2376,7 +2376,7 @@ int netif_rx(struct sk_buff *skb)
 	if (!skb->tstamp.tv64)
 		net_timestamp(skb);
 
-#ifdef CONFIG_SMP
+#ifdef CONFIG_RPS
 	cpu = get_rps_cpu(skb->dev, skb);
 	if (cpu < 0)
 		cpu = smp_processor_id();
@@ -2750,7 +2750,7 @@ out:
  */
 int netif_receive_skb(struct sk_buff *skb)
 {
-#ifdef CONFIG_SMP
+#ifdef CONFIG_RPS
 	int cpu;
 
 	cpu = get_rps_cpu(skb->dev, skb);
@@ -3189,7 +3189,7 @@ void netif_napi_del(struct napi_struct *napi)
 }
 EXPORT_SYMBOL(netif_napi_del);
 
-#ifdef CONFIG_SMP
+#ifdef CONFIG_RPS
 /*
  * net_rps_action sends any pending IPI's for rps.  This is only called from
  * softirq and interrupts must be enabled.
@@ -3214,7 +3214,7 @@ static void net_rx_action(struct softirq_action *h)
 	unsigned long time_limit = jiffies + 2;
 	int budget = netdev_budget;
 	void *have;
-#ifdef CONFIG_SMP
+#ifdef CONFIG_RPS
 	int select;
 	struct rps_remote_softirq_cpus *rcpus;
 #endif
@@ -3280,7 +3280,7 @@ static void net_rx_action(struct softirq_action *h)
 		netpoll_poll_unlock(have);
 	}
 out:
-#ifdef CONFIG_SMP
+#ifdef CONFIG_RPS
 	rcpus = &__get_cpu_var(rps_remote_softirq_cpus);
 	select = rcpus->select;
 	rcpus->select ^= 1;
@@ -5277,6 +5277,7 @@ int register_netdevice(struct net_device *dev)
 
 	dev->iflink = -1;
 
+#ifdef CONFIG_RPS
 	if (!dev->num_rx_queues) {
 		/*
 		 * Allocate a single RX queue if driver never called
@@ -5293,7 +5294,7 @@ int register_netdevice(struct net_device *dev)
 		atomic_set(&dev->_rx->count, 1);
 		dev->num_rx_queues = 1;
 	}
-
+#endif
 	/* Init, if this function is available */
 	if (dev->netdev_ops->ndo_init) {
 		ret = dev->netdev_ops->ndo_init(dev);
@@ -5653,11 +5654,13 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 		void (*setup)(struct net_device *), unsigned int queue_count)
 {
 	struct netdev_queue *tx;
-	struct netdev_rx_queue *rx;
 	struct net_device *dev;
 	size_t alloc_size;
 	struct net_device *p;
+#ifdef CONFIG_RPS
+	struct netdev_rx_queue *rx;
 	int i;
+#endif
 
 	BUG_ON(strlen(name) >= sizeof(dev->name));
 
@@ -5683,6 +5686,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 		goto free_p;
 	}
 
+#ifdef CONFIG_RPS
 	rx = kcalloc(queue_count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
 	if (!rx) {
 		printk(KERN_ERR "alloc_netdev: Unable to allocate "
@@ -5698,6 +5702,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	 */
 	for (i = 0; i < queue_count; i++)
 		rx[i].first = rx;
+#endif
 
 	dev = PTR_ALIGN(p, NETDEV_ALIGN);
 	dev->padded = (char *)dev - (char *)p;
@@ -5713,8 +5718,10 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	dev->num_tx_queues = queue_count;
 	dev->real_num_tx_queues = queue_count;
 
+#ifdef CONFIG_RPS
 	dev->_rx = rx;
 	dev->num_rx_queues = queue_count;
+#endif
 
 	dev->gso_max_size = GSO_MAX_SIZE;
 
@@ -5731,8 +5738,10 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	return dev;
 
 free_rx:
+#ifdef CONFIG_RPS
 	kfree(rx);
 free_tx:
+#endif
 	kfree(tx);
 free_p:
 	kfree(p);
@@ -6236,7 +6245,7 @@ static int __init net_dev_init(void)
 		queue->completion_queue = NULL;
 		INIT_LIST_HEAD(&queue->poll_list);
 
-#ifdef CONFIG_SMP
+#ifdef CONFIG_RPS
 		queue->csd.func = trigger_softirq;
 		queue->csd.info = queue;
 		queue->csd.flags = 0;



^ permalink raw reply related

* Re: [PATCH] RPS: support 802.1q and pppoe session
From: Changli Gao @ 2010-03-25  5:12 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, netdev
In-Reply-To: <1269493402.15280.29.camel@edumazet-laptop>

On Thu, Mar 25, 2010 at 1:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> While this might sounds a good idea, you really should split this in two
> parts.
>
> By the way, why not handling IPIP too ?

I'm not sure if it is a good idea to support VLAN and PPPOE, and
actually David don't like it. :(

>
> Because I believe 802.1q part has no added value for instance, since
> packet handled by CPUX will be decoded and passed to VLAN device, having
> a chance to be fully taken by RPS, since we go back to netif_rx().
>
> Probably same thing for IPIP / PPPOE can be discussed.

It is useful when Linux is run as a bridge.

>
> I agree we might need a flag or something to reset rxhash to 0 somewhere
> (probably in non accelerated vlan rx handling) to force second
> get_rps_cpu() invocation to recompute it. This small correction has no
> cost if put outside of get_rps_cpus().
>
> If get_rps_cpus() is too complex, it might become too slow for typical
> use. We should find smart ways to solve your performance problem if they
> ever exist.
>

It means that more than one IPI will be sent for just a single
packets, I don't think the cost is acceptable.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] RPS: support 802.1q and pppoe session
From: Eric Dumazet @ 2010-03-25  5:03 UTC (permalink / raw)
  To: xiaosuo; +Cc: David S. Miller, Tom Herbert, netdev
In-Reply-To: <4BAAE6E9.6030103@gmail.com>

Le jeudi 25 mars 2010 à 12:30 +0800, Changli Gao a écrit :
> support 802.1q and pppoe session
> 
> Support 802.1q and pppoe session, and these two protocols can get the
> benefit from RPS.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> net/core/dev.c | 38 ++++++++++++++++++++++++++++++--------
> 1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a03aab4..647ecc4 100644

While this might sounds a good idea, you really should split this in two
parts.

By the way, why not handling IPIP too ?

Because I believe 802.1q part has no added value for instance, since
packet handled by CPUX will be decoded and passed to VLAN device, having
a chance to be fully taken by RPS, since we go back to netif_rx().

Probably same thing for IPIP / PPPOE can be discussed.

I agree we might need a flag or something to reset rxhash to 0 somewhere
(probably in non accelerated vlan rx handling) to force second
get_rps_cpu() invocation to recompute it. This small correction has no
cost if put outside of get_rps_cpus().

If get_rps_cpus() is too complex, it might become too slow for typical
use. We should find smart ways to solve your performance problem if they
ever exist.




^ permalink raw reply

* Re: [PATCH] RPS: support 802.1q and pppoe session
From: David Miller @ 2010-03-25  4:50 UTC (permalink / raw)
  To: xiaosuo; +Cc: therbert, netdev
In-Reply-To: <4BAAE6E9.6030103@gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Thu, 25 Mar 2010 12:30:33 +0800

> support 802.1q and pppoe session
> 
> Support 802.1q and pppoe session, and these two protocols can get the
> benefit from RPS.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

This is getting rediculious.

The TX hasher doesn't support this, neither should RPS.

Most of the patches you've been posting to RPS are very specialized
hacks and frankly not very welcome.

^ permalink raw reply

* Re: bug 15606: register_netdevice: Kobject changed too ealier
From: Eric Dumazet @ 2010-03-25  4:35 UTC (permalink / raw)
  To: Richard Zhao; +Cc: netdev
In-Reply-To: <4e090d471003241944v6e1b6712h740f79f522514ca4@mail.gmail.com>

Le jeudi 25 mars 2010 à 10:44 +0800, Richard Zhao a écrit :
> Hi,
> 
> Could someone take bug 15606?
> https://bugzilla.kernel.org/show_bug.cgi?id=15606
> 

But... 

What's the point of opening a bugzilla report if you already have a
patch ?

Just send here a formal patch and we can discuss it directly, Ack it or
reject it...

Thanks



^ permalink raw reply

* [PATCH] RPS: support 802.1q and pppoe session
From: Changli Gao @ 2010-03-25  4:30 UTC (permalink / raw)
  To: David S. Miller; +Cc: Tom Herbert, xiaosuo, netdev

support 802.1q and pppoe session

Support 802.1q and pppoe session, and these two protocols can get the
benefit from RPS.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 38 ++++++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index a03aab4..647ecc4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -130,6 +130,10 @@
 #include <linux/random.h>
 #include <trace/events/napi.h>
 
+#ifdef CONFIG_SMP
+#include <linux/if_pppox.h>
+#endif
+
 #include "net-sysfs.h"
 
 /* Instead of increasing this, you should create a hash table. */
@@ -2190,7 +2194,8 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb)
 	struct rps_map *map;
 	int cpu = -1;
 	u8 ip_proto;
-	u32 addr1, addr2, ports, ihl;
+	__be16 protocol;
+	u32 addr1, addr2, ports, offset;
 
 	rcu_read_lock();
 
@@ -2214,26 +2219,43 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb)
 	if (skb->rxhash)
 		goto got_hash; /* Skip hash computation on packet header */
 
-	switch (skb->protocol) {
+	offset = 0;
+	protocol = skb->protocol;
+nest:
+	switch (protocol) {
+	case __constant_htons(ETH_P_8021Q):
+		if (!pskb_may_pull(skb, offset + VLAN_HLEN))
+			goto done;
+		protocol = ((struct vlan_hdr*)(skb->data +
+				offset))->h_vlan_encapsulated_proto;
+		offset += VLAN_HLEN;
+		goto nest;
+	case __constant_htons(ETH_P_PPP_SES):
+		if (!pskb_may_pull(skb, offset + PPPOE_SES_HLEN))
+			goto done;
+		protocol = *((__be16 *)(skb->data + offset +
+				sizeof(struct pppoe_hdr)));
+		offset += PPPOE_SES_HLEN;
+		goto nest;
 	case __constant_htons(ETH_P_IP):
-		if (!pskb_may_pull(skb, sizeof(*ip)))
+		if (!pskb_may_pull(skb, offset + sizeof(*ip)))
 			goto done;
 
 		ip = (struct iphdr *) skb->data;
 		ip_proto = ip->protocol;
 		addr1 = ip->saddr;
 		addr2 = ip->daddr;
-		ihl = ip->ihl;
+		offset += ip->ihl << 2;
 		break;
 	case __constant_htons(ETH_P_IPV6):
-		if (!pskb_may_pull(skb, sizeof(*ip6)))
+		if (!pskb_may_pull(skb, offset + sizeof(*ip6)))
 			goto done;
 
 		ip6 = (struct ipv6hdr *) skb->data;
 		ip_proto = ip6->nexthdr;
 		addr1 = ip6->saddr.s6_addr32[3];
 		addr2 = ip6->daddr.s6_addr32[3];
-		ihl = (40 >> 2);
+		offset += 40;
 		break;
 	default:
 		goto done;
@@ -2247,8 +2269,8 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb)
 	case IPPROTO_AH:
 	case IPPROTO_SCTP:
 	case IPPROTO_UDPLITE:
-		if (pskb_may_pull(skb, (ihl * 4) + 4))
-			ports = *((u32 *) (skb->data + (ihl * 4)));
+		if (pskb_may_pull(skb, offset + 4))
+			ports = *((u32 *) (skb->data + offset));
 		break;
 
 	default:



^ permalink raw reply related

* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: YOSHIFUJI Hideaki @ 2010-03-25  4:20 UTC (permalink / raw)
  To: Jozsef Kadlecsik
  Cc: Patrick McHardy, Shan Wei, YOSHIFUJI Hideaki, David Miller,
	Alexey Dobriyan, Yasuyuki KOZAKAI, netdev@vger.kernel.org,
	netfilter-devel, YOSHIFUJI Hideaki
In-Reply-To: <alpine.DEB.2.00.1003232050540.18241@blackhole.kfki.hu>

(2010/03/24 5:10), Jozsef Kadlecsik wrote:

> On Wed, 24 Mar 2010, YOSHIFUJI Hideaki wrote:
>
>>> In this case without conntrack, IPv6 would send an ICMPv6 message,
>>> so in my opinion the transparent thing to do would be to still send
>>> them. Of course only if reassembly is done on an end host.
>>
>> Well, no.  conntrack should just forward even uncompleted fragments
>> to next process (e.g. core ipv6 code), and then the core would send
>> ICMP error back.  ICMP should be sent by the core ipv6 code according
>> to decision of itself, not according to netfilter.
>
> But what state could be associated by conntrack to the uncompleted
> fragments but the INVALID state? In consequence, in any sane setup, the
> uncompleted fragments will be dropped silently by a filter table rule
> and no ICMP error message will be sent back.
>
> Therefore I think iff the destination of the fragments is the host
> itself, then conntrack should generate an ICMP error message. But that
> error message must be processed by conntrack to set its state and then
> the fate of the generated packet can be decided by a rule.

Well.... no.

First of all. in "sane" setup, people should configure according
to their own requirements.  They may or may not want send back
icmp packet.  And, even if the core is to send icmp back, the
state would be correctly assigned.

We cannot (and should not) do something "cleaver" (excluding
packet drops) in conntrack in PRE_ROUTING chain.

One reason is that in PRE_ROUTING context, we can NOT determine
if the address we see in the IP header is really the final
destination.  The overall situation is the same even if the
routing entry corresponding the "current" destination points
the node itself, or even if the node is configured as host.

It might seems that we could do something in the "filter"
table in LOCAL_IN, FORWARD or LOCAL_OUT (after routing header
process).

But well, we unfortunately cannot do this (at least
automatically) because even in LOCAL_IN, the final
destination has not been decided until we process all
of extension headers.

Sending ICMP in netfilter (especially in conntrack) is too
patchy, and is not right.  If we do the right thing (and
I believe we should do so),  I'd propose to have hooks
around handlers inside ip6_input_finish().

...I remember that I was thinking about this before.

For my conclusion, first option is just to drop
uncompleted fragments as we do today.  Second option
would be  to forward them to the next process so that
core code could send ICMPv6 etc. or, we could have
new code to send ICMPV6_TIME_EXCEED in REJECT target.
In longer term, I think it is better to introduce
per-exthdr hooks.

Regards,

--yoshfuji

^ permalink raw reply

* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: YOSHIFUJI Hideaki @ 2010-03-25  4:19 UTC (permalink / raw)
  To: Shan Wei
  Cc: Patrick McHardy, YOSHIFUJI Hideaki, David Miller, Alexey Dobriyan,
	Yasuyuki KOZAKAI, netdev@vger.kernel.org, netfilter-devel,
	YOSHIFUJI Hideaki
In-Reply-To: <4BAACA44.6010306@cn.fujitsu.com>

(2010/03/25 11:28), Shan Wei wrote:
> Patrick McHardy wrote, at 03/23/2010 11:05 PM:
>> Shan Wei wrote:
>>>> On the other hand, I'd even say we should NOT send
>>>> icmp here (at least by default) because standard routers
>>>> never send such packet.
>>>>
>>> Yes,for routers, the patch-set does not send icmp message to
>>> source host. It only does on destination host with IPv6 connection
>>> track enable.
>>>
>>
>> The nf-next tree is open again, now would be a good time to resubmit
>> these patches.
>> Thanks!
>
> If no body opposes them, i will resubmit these patches with v3.
>

I still disagree 4-7.

--yoshfuji
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Seeing new kernel unaligned access messages in linux-next on ia64
From: David Miller @ 2010-03-25  3:32 UTC (permalink / raw)
  To: jengelh; +Cc: schwab, tony.luck, netdev
In-Reply-To: <alpine.LSU.2.01.1003250014400.16715@obet.zrqbmnf.qr>

From: Jan Engelhardt <jengelh@medozas.de>
Date: Thu, 25 Mar 2010 00:17:01 +0100 (CET)

> No more unaligned messages - but is this an acceptable solution?

We already rely on this elsewhere, particularly in the
xfrm_user code.

^ permalink raw reply

* Re: Seeing new kernel unaligned access messages in linux-next on ia64
From: David Miller @ 2010-03-25  3:32 UTC (permalink / raw)
  To: tony.luck; +Cc: jengelh, schwab, netdev
In-Reply-To: <987664A83D2D224EAE907B061CE93D53B30F00F1@orsmsx505.amr.corp.intel.com>

From: "Luck, Tony" <tony.luck@intel.com>
Date: Wed, 24 Mar 2010 16:19:38 -0700

>> No more unaligned messages - but is this an acceptable solution?
> 
> There are a bunch of macros in include/linux/unaligned/*.h to
> handle this sort of thing.

It's totally unnecessary here and it would be overkill to
use those interfaces one at a time on every struct member
when a proper memcpy() fro ma type-pruned void pointer
suffices.

^ permalink raw reply

* why does the tcp_time_wait() call tcp_done(sk)?
From: 杨硕 @ 2010-03-25  3:24 UTC (permalink / raw)
  To: netdev

that means the connection has been closed?
Does kernel think like that???
we can use "STATE_TRACE" to see FIN_WAIT_2--->CLOSE rather than
FIN_WAIT_2-->TIME_WAIT-->CLOSE.

TIA.

void tcp_time_wait(struct sock *sk, int state, int timeo)
{
	struct inet_timewait_sock *tw = NULL;
	const struct inet_connection_sock *icsk = inet_csk(sk);
	const struct tcp_sock *tp = tcp_sk(sk);
	int recycle_ok = 0;

	if (tcp_death_row.sysctl_tw_recycle && tp->rx_opt.ts_recent_stamp)
		recycle_ok = icsk->icsk_af_ops->remember_stamp(sk);

	if (tcp_death_row.tw_count < tcp_death_row.sysctl_max_tw_buckets)
		tw = inet_twsk_alloc(sk, state);

	if (tw != NULL) {
		struct tcp_timewait_sock *tcptw = tcp_twsk((struct sock *)tw);
		const int rto = (icsk->icsk_rto << 2) - (icsk->icsk_rto >> 1);

		tw->tw_rcv_wscale	= tp->rx_opt.rcv_wscale;
		tcptw->tw_rcv_nxt	= tp->rcv_nxt;
		tcptw->tw_snd_nxt	= tp->snd_nxt;
		tcptw->tw_rcv_wnd	= tcp_receive_window(tp);
		tcptw->tw_ts_recent	= tp->rx_opt.ts_recent;
		tcptw->tw_ts_recent_stamp = tp->rx_opt.ts_recent_stamp;

#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
		if (tw->tw_family == PF_INET6) {
			struct ipv6_pinfo *np = inet6_sk(sk);
			struct inet6_timewait_sock *tw6;

			tw->tw_ipv6_offset = inet6_tw_offset(sk->sk_prot);
			tw6 = inet6_twsk((struct sock *)tw);
			ipv6_addr_copy(&tw6->tw_v6_daddr, &np->daddr);
			ipv6_addr_copy(&tw6->tw_v6_rcv_saddr, &np->rcv_saddr);
			tw->tw_ipv6only = np->ipv6only;
		}
#endif

#ifdef CONFIG_TCP_MD5SIG
		/*
		 * The timewait bucket does not have the key DB from the
		 * sock structure. We just make a quick copy of the
		 * md5 key being used (if indeed we are using one)
		 * so the timewait ack generating code has the key.
		 */
		do {
			struct tcp_md5sig_key *key;
			memset(tcptw->tw_md5_key, 0, sizeof(tcptw->tw_md5_key));
			tcptw->tw_md5_keylen = 0;
			key = tp->af_specific->md5_lookup(sk, sk);
			if (key != NULL) {
				memcpy(&tcptw->tw_md5_key, key->key, key->keylen);
				tcptw->tw_md5_keylen = key->keylen;
				if (tcp_alloc_md5sig_pool() == NULL)
					BUG();
			}
		} while (0);
#endif

		/* Linkage updates. */
		__inet_twsk_hashdance(tw, sk, &tcp_hashinfo);

		/* Get the TIME_WAIT timeout firing. */
		if (timeo < rto)
			timeo = rto;

		if (recycle_ok) {
			tw->tw_timeout = rto;
		} else {
			tw->tw_timeout = TCP_TIMEWAIT_LEN;
			if (state == TCP_TIME_WAIT)
				timeo = TCP_TIMEWAIT_LEN;
		}

		inet_twsk_schedule(tw, &tcp_death_row, timeo,
				   TCP_TIMEWAIT_LEN);
		inet_twsk_put(tw);
	} else {
		/* Sorry, if we're out of memory, just CLOSE this
		 * socket up.  We've got bigger problems than
		 * non-graceful socket closings.
		 */
		LIMIT_NETDEBUG(KERN_INFO "TCP: time wait bucket table overflow\n");
	}

	tcp_update_metrics(sk);
	tcp_done(sk);
}

^ permalink raw reply

* Re: [iproute2]  tc pedit   modify  ethhdr ?
From: jamal @ 2010-03-25  3:21 UTC (permalink / raw)
  To: Xiaofei Wu; +Cc: linux netdev
In-Reply-To: <326421.66567.qm@web111606.mail.gq1.yahoo.com>


I put some effort in responding to you. Please put some
effort in testing it.

cheers,
jamal

On Wed, 2010-03-24 at 19:59 -0700, Xiaofei Wu wrote:
> 
> 
> I made a test like your example.
> The terminal prints:
> parse_cmd argc 20 set offset -14 length 2
> pack_key16: Final val 90000000 mask 0000ffff
> parse_cmd done argc 18 munge offset -16 length 2
> 
> what's the meaning of the above 3 lines;  It modify the value of  -16,-15,-14,-13 ,  or just modify -14,-13 ?
> If it modified -16,-15 ,   would it cause any problems?
> 
> Thanks.
> 
> 
> --
> Wu
> 
> 
> 
> ----- Original Message ----
> > but the size of ethhdr is 14 bytes, 14 is not multiple of 4 .
> > How to use 'tc ... pedit ...'  modify  a packet's   h_dest and h_source 
> > of ethhdr ?
> > Use ' ... pedit munge offset -14 u16 set 0x0090 munge offset -12 u32 set 0x9600030a ... '   or 
> > use ' ... pedit munge offset -16 u32 ... munge offset -12  ... '  ?
> > 
> 
> 0 is at ip header.
> dst MAC starts at -14
> src MAC at -8
> ethertype at -2
> 
> Example:
> tc filter add dev eth0 parent ffff: protocol ip prio 10 u32 \
> match ip src 192.168.1.10/32 flowid 1:2 \
> action pedit munge offset -14 u16 set 0x0000 \
> munge offset -12 u32 set 0x00010100 \
> munge offset -8 u32 set 0x0aaf0100 \
> munge offset -4 u32 set 0x0008ec06 pipe \
> action mirred egress redirect dev eth1
> 
> 
>       


^ permalink raw reply

* Re: [iproute2]  tc pedit   modify  ethhdr ?
From: Xiaofei Wu @ 2010-03-25  2:59 UTC (permalink / raw)
  To: hadi; +Cc: linux netdev
In-Reply-To: <1269438652.4089.22.camel@bigi>

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




I made a test like your example.
The terminal prints:
parse_cmd argc 20 set offset -14 length 2
pack_key16: Final val 90000000 mask 0000ffff
parse_cmd done argc 18 munge offset -16 length 2

what's the meaning of the above 3 lines;  It modify the value of  -16,-15,-14,-13 ,  or just modify -14,-13 ?
If it modified -16,-15 ,   would it cause any problems?

Thanks.


--
Wu



----- Original Message ----
> but the size of ethhdr is 14 bytes, 14 is not multiple of 4 .
> How to use 'tc ... pedit ...'  modify  a packet's   h_dest and h_source 
> of ethhdr ?
> Use ' ... pedit munge offset -14 u16 set 0x0090 munge offset -12 u32 set 0x9600030a ... '   or 
> use ' ... pedit munge offset -16 u32 ... munge offset -12  ... '  ?
> 

0 is at ip header.
dst MAC starts at -14
src MAC at -8
ethertype at -2

Example:
tc filter add dev eth0 parent ffff: protocol ip prio 10 u32 \
match ip src 192.168.1.10/32 flowid 1:2 \
action pedit munge offset -14 u16 set 0x0000 \
munge offset -12 u32 set 0x00010100 \
munge offset -8 u32 set 0x0aaf0100 \
munge offset -4 u32 set 0x0008ec06 pipe \
action mirred egress redirect dev eth1


      

[-- Attachment #2: test.png --]
[-- Type: application/octet-stream, Size: 61263 bytes --]

^ permalink raw reply

* bug 15606: register_netdevice: Kobject changed too ealier
From: Richard Zhao @ 2010-03-25  2:44 UTC (permalink / raw)
  To: netdev

Hi,

Could someone take bug 15606?
https://bugzilla.kernel.org/show_bug.cgi?id=15606

Thanks
Richard

^ permalink raw reply

* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: Shan Wei @ 2010-03-25  2:28 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: YOSHIFUJI Hideaki, David Miller, Alexey Dobriyan,
	Yasuyuki KOZAKAI, netdev@vger.kernel.org, netfilter-devel,
	yoshfuji@linux-ipv6.org >> YOSHIFUJI Hideaki
In-Reply-To: <4BA8D8AF.1040307@trash.net>

Patrick McHardy wrote, at 03/23/2010 11:05 PM:
> Shan Wei wrote:
>>> On the other hand, I'd even say we should NOT send
>>> icmp here (at least by default) because standard routers
>>> never send such packet.
>>>     
>> Yes,for routers, the patch-set does not send icmp message to
>> source host. It only does on destination host with IPv6 connection 
>> track enable.
>>   
> 
> The nf-next tree is open again, now would be a good time to resubmit
> these patches.
> Thanks!

If no body opposes them, i will resubmit these patches with v3. 

-- 
Best Regards
-----
Shan Wei
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: Shan Wei @ 2010-03-25  2:22 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki
  Cc: Patrick McHardy, YOSHIFUJI Hideaki, David Miller, Alexey Dobriyan,
	Yasuyuki KOZAKAI, netdev@vger.kernel.org, netfilter-devel
In-Reply-To: <4BA90F72.6010404@linux-ipv6.org>

yoshifuji-san:

YOSHIFUJI Hideaki wrote, at 03/24/2010 02:58 AM:
>> In this case without conntrack, IPv6 would send an ICMPv6 message,
>> so in my opinion the transparent thing to do would be to still send
>> them. Of course only if reassembly is done on an end host.
> 
> Well, no.  conntrack should just forward even uncompleted fragments
> to next process (e.g. core ipv6 code), and then the core would send
> ICMP error back.  ICMP should be sent by the core ipv6 code according
> to decision of itself, not according to netfilter.

It's bad to forward uncompleted fragments to IPv4/IPv6 stack.

One the one hand, helper modules of conntrack analyze application data
in packets. They need to parse overall segment or datagram. If packets are 
fragmented, conntrack needs to reassemble them.

On the other hand, if uncompleted fragments are forwarded to IPv4/IPv6 stack,
they will be reassemble twice, and the result is also failure.
So, conntrack will drop uncompleted fragments after reassemble timeout.
 
> Would you please explain more about what you mean by consistency
> between IPv4 and IPv6 conntrack?
> 
> I do think it is rather different, anyway (because original packets
> is to be preserved in IPv6, but not in IPv4).

Yes, the defragment implement of IPv6 conntrack is absolutely different from that of IPv4 conntrack.
But, the handle after reassemble timeout should be consistent.
For IPv4 conntrack, an end host with conntrack enabled must send ICMP fragment reassembly timeout
message to source host. Details see commit e9017b
(Title: IP: Send an ICMP "Fragment Reassembly Timeout" message when enabling connection track).

-- 
Best Regards
-----
Shan Wei

> 
> --yoshfuji
> 
 


^ permalink raw reply

* mmotm 2010-03-23 - IPv6 warnings...
From: Valdis.Kletnieks @ 2010-03-25  1:36 UTC (permalink / raw)
  To: akpm; +Cc: netdev, linux-kernel
In-Reply-To: <201003232301.o2NN1bms031050@imap1.linux-foundation.org>

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

On Tue, 23 Mar 2010 15:34:59 PDT, akpm@linux-foundation.org said:
> The mm-of-the-moment snapshot 2010-03-23-15-34 has been uploaded to
> 
>    http://userweb.kernel.org/~akpm/mmotm/

Seen in my dmesg.  It may be relevant that I'm at home, and my IPv6
prefix arrives via a PPP VPN connection. This happened about 20-25 seconds
after I launched pppd.

[  175.269411] ------------[ cut here ]------------
[  175.269433] WARNING: at net/ipv6/addrconf.c:544 inet6_ifa_finish_destroy+0x40/0xbb()
[  175.269441] Hardware name: Latitude E6500                  
[  175.269447] Modules linked in: ppp_async sunrpc usb_storage [last unloaded: scsi_wait_scan]
[  175.269473] Pid: 7, comm: ksoftirqd/1 Tainted: G        W   2.6.34-rc2-mmotm0323 #1
[  175.269480] Call Trace:
[  175.269488]  <IRQ>  [<ffffffff8103728a>] warn_slowpath_common+0x80/0x98
[  175.269517]  [<ffffffff810372b7>] warn_slowpath_null+0x15/0x17
[  175.269530]  [<ffffffff814f2093>] inet6_ifa_finish_destroy+0x40/0xbb
[  175.269542]  [<ffffffff814f2122>] in6_ifa_put+0x14/0x16
[  175.269555]  [<ffffffff814f4893>] addrconf_prefix_rcv+0x583/0x786
[  175.269568]  [<ffffffff814f70b3>] ? rt6_get_dflt_router+0xab/0xbb
[  175.269581]  [<ffffffff814fd98c>] ndisc_rcv+0x90f/0xdb4
[  175.269595]  [<ffffffff810852a7>] ? __rcu_read_unlock+0x30c/0x341
[  175.269609]  [<ffffffff81502f6d>] ? debug_lockdep_rcu_enabled+0x33/0x39
[  175.269622]  [<ffffffff815040ae>] icmpv6_rcv+0x59b/0x659
[  175.269634]  [<ffffffff81504c9e>] ? ipv6_chk_mcast_addr+0xaf/0x13d
[  175.269648]  [<ffffffff81063e21>] ? trace_hardirqs_on_caller+0x16/0x13c
[  175.269661]  [<ffffffff8103d5b3>] ? _local_bh_enable_ip+0x15c/0x16e
[  175.269674]  [<ffffffff8103d5ce>] ? local_bh_enable_ip+0x9/0xb
[  175.269689]  [<ffffffff81598c44>] ? _raw_read_unlock_bh+0x39/0x3e
[  175.269702]  [<ffffffff81504d13>] ? ipv6_chk_mcast_addr+0x124/0x13d
[  175.269714]  [<ffffffff814eee7d>] ? ip6_input_finish+0x0/0x343
[  175.269726]  [<ffffffff814ef066>] ip6_input_finish+0x1e9/0x343
[  175.269737]  [<ffffffff814eee7d>] ? ip6_input_finish+0x0/0x343
[  175.269748]  [<ffffffff814ef20c>] NF_HOOK.clone.1+0x4c/0x53
[  175.269759]  [<ffffffff814ef6d6>] ip6_input+0x1c/0x1e
[  175.269769]  [<ffffffff814ef7df>] ip6_mc_input+0x107/0x11c
[  175.269780]  [<ffffffff814eee17>] ? ip6_rcv_finish+0x0/0x26
[  175.269791]  [<ffffffff814eee39>] ip6_rcv_finish+0x22/0x26
[  175.269802]  [<ffffffff814ef20c>] NF_HOOK.clone.1+0x4c/0x53
[  175.269812]  [<ffffffff814ef666>] ipv6_rcv+0x453/0x4a7
[  175.269827]  [<ffffffff8146527e>] __netif_receive_skb+0x2da/0x307
[  175.269839]  [<ffffffff81465348>] process_backlog+0x9d/0xd6
[  175.269851]  [<ffffffff81465631>] net_rx_action+0xf2/0x356
[  175.269863]  [<ffffffff8103dc41>] __do_softirq+0x14a/0x282
[  175.269876]  [<ffffffff8100360c>] call_softirq+0x1c/0x34
[  175.269882]  <EOI>  [<ffffffff81004b58>] ? do_softirq+0x44/0xf0
[  175.269901]  [<ffffffff8103d7dd>] run_ksoftirqd+0xbc/0x1fb
[  175.269912]  [<ffffffff8103d721>] ? run_ksoftirqd+0x0/0x1fb
[  175.269925]  [<ffffffff81052c6b>] kthread+0x7a/0x82
[  175.269937]  [<ffffffff81003514>] kernel_thread_helper+0x4/0x10
[  175.269949]  [<ffffffff81599900>] ? restore_args+0x0/0x30
[  175.269961]  [<ffffffff81052bf1>] ? kthread+0x0/0x82
[  175.269971]  [<ffffffff81003510>] ? kernel_thread_helper+0x0/0x10
[  175.269979] ---[ end trace a7919e7f17c0a727 ]---
[  175.269986] Freeing alive inet6 address ffff880107e1a400
[  176.472172] ------------[ cut here ]------------
[  176.472192] WARNING: at net/ipv6/addrconf.c:544 inet6_ifa_finish_destroy+0x40/0xbb()
[  176.472200] Hardware name: Latitude E6500                  
[  176.472206] Modules linked in: ppp_async sunrpc usb_storage [last unloaded: scsi_wait_scan]
[  176.472232] Pid: 4008, comm: fetchmail Tainted: G        W   2.6.34-rc2-mmotm0323 #1
[  176.472239] Call Trace:
[  176.472256]  [<ffffffff8103728a>] warn_slowpath_common+0x80/0x98
[  176.472269]  [<ffffffff810372b7>] warn_slowpath_null+0x15/0x17
[  176.472280]  [<ffffffff814f2093>] inet6_ifa_finish_destroy+0x40/0xbb
[  176.472291]  [<ffffffff814f2122>] in6_ifa_put+0x14/0x16
[  176.472303]  [<ffffffff814f2a7b>] ipv6_dev_get_saddr+0x2b3/0x2c7
[  176.472316]  [<ffffffff814f8f32>] ? fib6_rule_lookup+0x18/0x1a
[  176.472328]  [<ffffffff814ebb04>] ip6_dst_lookup_tail+0x90/0xc5
[  176.472339]  [<ffffffff814ebc37>] ip6_dst_lookup+0x10/0x12
[  176.472352]  [<ffffffff8150f712>] ip6_datagram_connect+0x334/0x4f6
[  176.472365]  [<ffffffff8103d5b3>] ? _local_bh_enable_ip+0x15c/0x16e
[  176.472378]  [<ffffffff81457d82>] ? release_sock+0xff/0x108
[  176.472390]  [<ffffffff814c0042>] inet_dgram_connect+0x56/0x5d
[  176.472402]  [<ffffffff81455907>] sys_connect+0x75/0x98
[  176.472415]  [<ffffffff810e24b4>] ? path_put+0x1d/0x22
[  176.472429]  [<ffffffff81063e21>] ? trace_hardirqs_on_caller+0x16/0x13c
[  176.472442]  [<ffffffff8107d442>] ? audit_syscall_entry+0xcb/0x19c
[  176.472455]  [<ffffffff810026eb>] system_call_fastpath+0x16/0x1b
[  176.472464] ---[ end trace a7919e7f17c0a728 ]---
[  176.472472] Freeing alive inet6 address ffff880107e1a400


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply

* Re: why not use the tcp_set_state() in inet_csk_listen_start()?
From: 杨硕 @ 2010-03-25  0:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1269421323.3213.16.camel@edumazet-laptop>

But if we use "STATE_TRACE", maybe the state will be incomplete.

> Because its not necessary ?
>
> tcp_set_state() takes care of particular state transitions.
>
> In case of inet_csk_listen_start(), old state is TCP_CLOSE, so there is
> nothing special to do in tcp_set_state().
>

^ permalink raw reply

* RE: [PATCH] e1000e: Use pr_<level> and netdev_<level>
From: Joe Perches @ 2010-03-25  0:04 UTC (permalink / raw)
  To: Allan, Bruce W
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com
In-Reply-To: <8DD2590731AB5D4C9DBF71A877482A9061B3334E@orsmsx509.amr.corp.intel.com>

On Wed, 2010-03-24 at 16:54 -0700, Allan, Bruce W wrote:
> On Wednesday, March 24, 2010 4:43 PM, Joe Perches wrote:
> > On Wed, 2010-03-24 at 15:55 -0700, Jeff Kirsher wrote:
> >> From: Bruce Allan <bruce.w.allan@intel.com>
> >> As an alternative to a quite large patch previously submitted by Joe
> >> Perches to make use of kernel logging API, this patch is much less
> >> intrusive.
> > []
> >> diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
> >> index 118bdf4..23ab67c 100644 --- a/drivers/net/e1000e/e1000.h
> >> +++ b/drivers/net/e1000e/e1000.h
> >> @@ -42,25 +42,16 @@
> >> 
> >>  struct e1000_info;
> >> 
> >> -#define e_printk(level, adapter, format, arg...) \
> >> -	printk(level "%s: %s: " format, pci_name(adapter->pdev), \
> >> -	       adapter->netdev->name, ## arg)
> >> -
> >> -#ifdef DEBUG
> >>  #define e_dbg(format, arg...) \
> >> -	e_printk(KERN_DEBUG , hw->adapter, format, ## arg)
> >> -#else
> >> -#define e_dbg(format, arg...) do { (void)(hw); } while (0)
> >> -#endif
> >> -
> >> +	netdev_dbg(hw->adapter->netdev, format, ## arg)
> >>  #define e_err(format, arg...) \
> >> -	e_printk(KERN_ERR, adapter, format, ## arg)
> >> +	netdev_err(adapter->netdev, format, ## arg)
> >>  #define e_info(format, arg...) \
> >> -	e_printk(KERN_INFO, adapter, format, ## arg)
> >> +	netdev_info(adapter->netdev, format, ## arg)
> >>  #define e_warn(format, arg...) \
> >> -	e_printk(KERN_WARNING, adapter, format, ## arg)
> >> +	netdev_warn(adapter->netdev, format, ## arg)
> >>  #define e_notice(format, arg...) \
> >> -	e_printk(KERN_NOTICE, adapter, format, ## arg)
> >> +	netdev_notice(adapter->netdev, format, ## arg)
> > 
> > Macros that use global variables are undesirable.
> > I think there's value in using standard mechanisms.
> > 
> No global variable is used - both adapter and hw are either a local
> variable or a passed in parameter for all functions that use these macros.

hw and adapter are locally scoped in each function here,
but are not local to the macro.  That can make conversions
of macros to functions difficult.

It's your code, not mine.
Proving the conversion correct is pretty trivial.
Do what's best.

cheers, Joe


^ permalink raw reply

* RE: [PATCH] e1000e: Use pr_<level> and netdev_<level>
From: Allan, Bruce W @ 2010-03-24 23:54 UTC (permalink / raw)
  To: Joe Perches, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <1269474180.1541.11.camel@Joe-Laptop.home>

On Wednesday, March 24, 2010 4:43 PM, Joe Perches wrote:
> On Wed, 2010-03-24 at 15:55 -0700, Jeff Kirsher wrote:
>> From: Bruce Allan <bruce.w.allan@intel.com>
>> As an alternative to a quite large patch previously submitted by Joe
>> Perches to make use of kernel logging API, this patch is much less
>> intrusive.
> []
>> diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
>> index 118bdf4..23ab67c 100644 --- a/drivers/net/e1000e/e1000.h
>> +++ b/drivers/net/e1000e/e1000.h
>> @@ -42,25 +42,16 @@
>> 
>>  struct e1000_info;
>> 
>> -#define e_printk(level, adapter, format, arg...) \
>> -	printk(level "%s: %s: " format, pci_name(adapter->pdev), \
>> -	       adapter->netdev->name, ## arg)
>> -
>> -#ifdef DEBUG
>>  #define e_dbg(format, arg...) \
>> -	e_printk(KERN_DEBUG , hw->adapter, format, ## arg)
>> -#else
>> -#define e_dbg(format, arg...) do { (void)(hw); } while (0)
>> -#endif
>> -
>> +	netdev_dbg(hw->adapter->netdev, format, ## arg)
>>  #define e_err(format, arg...) \
>> -	e_printk(KERN_ERR, adapter, format, ## arg)
>> +	netdev_err(adapter->netdev, format, ## arg)
>>  #define e_info(format, arg...) \
>> -	e_printk(KERN_INFO, adapter, format, ## arg)
>> +	netdev_info(adapter->netdev, format, ## arg)
>>  #define e_warn(format, arg...) \
>> -	e_printk(KERN_WARNING, adapter, format, ## arg)
>> +	netdev_warn(adapter->netdev, format, ## arg)
>>  #define e_notice(format, arg...) \
>> -	e_printk(KERN_NOTICE, adapter, format, ## arg)
>> +	netdev_notice(adapter->netdev, format, ## arg)
> 
> Macros that use global variables are undesirable.
> 
> I think there's value in using standard mechanisms.
> 
> cheers, Joe

No global variable is used - both adapter and hw are either a local variable or a passed in parameter for all functions that use these macros.

^ permalink raw reply

* Re: [PATCH] e1000e: Use pr_<level> and netdev_<level>
From: Joe Perches @ 2010-03-24 23:43 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, Bruce Allan
In-Reply-To: <20100324225529.7104.17189.stgit@localhost.localdomain>

On Wed, 2010-03-24 at 15:55 -0700, Jeff Kirsher wrote:
> From: Bruce Allan <bruce.w.allan@intel.com>
> As an alternative to a quite large patch previously submitted by Joe
> Perches to make use of kernel logging API, this patch is much less
> intrusive.
[]
> diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
> index 118bdf4..23ab67c 100644
> --- a/drivers/net/e1000e/e1000.h
> +++ b/drivers/net/e1000e/e1000.h
> @@ -42,25 +42,16 @@
>  
>  struct e1000_info;
>  
> -#define e_printk(level, adapter, format, arg...) \
> -	printk(level "%s: %s: " format, pci_name(adapter->pdev), \
> -	       adapter->netdev->name, ## arg)
> -
> -#ifdef DEBUG
>  #define e_dbg(format, arg...) \
> -	e_printk(KERN_DEBUG , hw->adapter, format, ## arg)
> -#else
> -#define e_dbg(format, arg...) do { (void)(hw); } while (0)
> -#endif
> -
> +	netdev_dbg(hw->adapter->netdev, format, ## arg)
>  #define e_err(format, arg...) \
> -	e_printk(KERN_ERR, adapter, format, ## arg)
> +	netdev_err(adapter->netdev, format, ## arg)
>  #define e_info(format, arg...) \
> -	e_printk(KERN_INFO, adapter, format, ## arg)
> +	netdev_info(adapter->netdev, format, ## arg)
>  #define e_warn(format, arg...) \
> -	e_printk(KERN_WARNING, adapter, format, ## arg)
> +	netdev_warn(adapter->netdev, format, ## arg)
>  #define e_notice(format, arg...) \
> -	e_printk(KERN_NOTICE, adapter, format, ## arg)
> +	netdev_notice(adapter->netdev, format, ## arg)

Macros that use global variables are undesirable.

I think there's value in using standard mechanisms.

cheers, Joe


^ permalink raw reply

* Re: behavior of recvmmsg() on blocking sockets
From: Brandon Black @ 2010-03-24 23:35 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel
In-Reply-To: <g76h4xrt5j1pw2kk3tUYAxe124vaj_firegpg@mail.gmail.com>

On Wed, Mar 24, 2010 at 1:34 PM,  <drepper@gmail.com> wrote:
> On Wed, Mar 24, 2010 at 11:28, Brandon Black <blblack@gmail.com> wrote:
>>
>> I still think having a "block until at least one packet arrives" mode
>> for recvmmsg() makes sense though.
>
> I agree.  This is the mode I've seen people asking for.  They want the call
> to return as quickly as possible if there is data and then with as many
> messages as possible.  A MSG_WAITFORONE flag would do the trick nicely.

This patch might be woefully inadequate, as I'm not intimately
familiar with the rest of the Linux socket code, and I'm not sure what
the impact is of (a) adding that new flag, which is the first beyond
the 16-bit space, and (b) having that extra undefined flag present
during the underlying recvmsg() calls, but this patch Works For Me for
my isolated case.  Thoughts? (hoping gmail doesn't mangle this)

[blblack@xpc kernels]$ diff -u linux-2.6.33-orig/net/socket.c
linux-2.6.33/net/socket.c
--- linux-2.6.33-orig/net/socket.c	2010-02-24 12:52:17.000000000 -0600
+++ linux-2.6.33/net/socket.c	2010-03-24 18:10:37.156234986 -0500
@@ -2133,7 +2133,10 @@

 		if (err)
 			break;
-		++datagrams;
+
+                /* MSG_WAITFORONE turns on MSG_DONTWAIT after one packet */
+		if (!datagrams++ && flags & MSG_WAITFORONE)
+			flags |= MSG_DONTWAIT;

 		if (timeout) {
 			ktime_get_ts(timeout);
[blblack@xpc kernels]$ diff -u
linux-2.6.33-orig/include/linux/socket.h
linux-2.6.33/include/linux/socket.h
--- linux-2.6.33-orig/include/linux/socket.h	2010-02-24 12:52:17.000000000 -0600
+++ linux-2.6.33/include/linux/socket.h	2010-03-24 17:35:14.009266280 -0500
@@ -255,6 +255,7 @@
 #define MSG_ERRQUEUE	0x2000	/* Fetch message from error queue */
 #define MSG_NOSIGNAL	0x4000	/* Do not generate SIGPIPE */
 #define MSG_MORE	0x8000	/* Sender will send more */
+#define MSG_WAITFORONE  0x10000 /* recvmmsg(): block until 1+ packets avail */

 #define MSG_EOF         MSG_FIN

^ permalink raw reply

* RE: Seeing new kernel unaligned access messages in linux-next on ia64
From: Luck, Tony @ 2010-03-24 23:19 UTC (permalink / raw)
  To: Jan Engelhardt, Andreas Schwab; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <alpine.LSU.2.01.1003250014400.16715@obet.zrqbmnf.qr>

> No more unaligned messages - but is this an acceptable solution?

There are a bunch of macros in include/linux/unaligned/*.h to
handle this sort of thing.

-Tony

^ permalink raw reply

* Re: Seeing new kernel unaligned access messages in linux-next on ia64
From: Jan Engelhardt @ 2010-03-24 23:17 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: David Miller, tony.luck, netdev
In-Reply-To: <m2vdcl5ocj.fsf@igel.home>


On Wednesday 2010-03-24 23:47, Andreas Schwab wrote:
>Jan Engelhardt writes:
>
>> Using a temporary and memcpying it off would normally fix this,
>> as in the patch below. During testing however, I still get
>> unaligned messages even with the patch - and I would not know
>> what causes this.
>
>The memcpy will not fix the alignment issue because the copy operation
>is fully equivalent to a direct assignment, and the compiler can still
>take advantage of the known alignment of the types.  You have to
>explicitly tell the compiler about the reduced alignment guarantee.

You're right, I remember seeing that sort of optimization before. So I 
have changed the function's signature to read

-static void copy_rtnl_link_stats64(struct rtnl_link_stats64 *v,
-                                  const struct net_device_stats *b)
+static void copy_rtnl_link_stats64(void *v, const struct net_device_stats *b)
 {
 ...
 	memcpy(v, &a, sizeof(a));
 }

No more unaligned messages - but is this an acceptable solution?

thanks,
Jan

^ permalink raw reply

* [PATCH] e1000e: Use pr_<level> and netdev_<level>
From: Jeff Kirsher @ 2010-03-24 22:55 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Joe Perches, Bruce Allan, Jeff Kirsher

From: Bruce Allan <bruce.w.allan@intel.com>

As an alternative to a quite large patch previously submitted by Joe
Perches to make use of kernel logging API, this patch is much less
intrusive.

Convert e_<level> to netdev_<level>
Use #define pr_fmt
Convert a few printks to pr_<level>

Cc: Joe Perches <joe@perches.com>
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000e/e1000.h  |   19 +++++--------------
 drivers/net/e1000e/netdev.c |    9 +++++----
 2 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index 118bdf4..23ab67c 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -42,25 +42,16 @@
 
 struct e1000_info;
 
-#define e_printk(level, adapter, format, arg...) \
-	printk(level "%s: %s: " format, pci_name(adapter->pdev), \
-	       adapter->netdev->name, ## arg)
-
-#ifdef DEBUG
 #define e_dbg(format, arg...) \
-	e_printk(KERN_DEBUG , hw->adapter, format, ## arg)
-#else
-#define e_dbg(format, arg...) do { (void)(hw); } while (0)
-#endif
-
+	netdev_dbg(hw->adapter->netdev, format, ## arg)
 #define e_err(format, arg...) \
-	e_printk(KERN_ERR, adapter, format, ## arg)
+	netdev_err(adapter->netdev, format, ## arg)
 #define e_info(format, arg...) \
-	e_printk(KERN_INFO, adapter, format, ## arg)
+	netdev_info(adapter->netdev, format, ## arg)
 #define e_warn(format, arg...) \
-	e_printk(KERN_WARNING, adapter, format, ## arg)
+	netdev_warn(adapter->netdev, format, ## arg)
 #define e_notice(format, arg...) \
-	e_printk(KERN_NOTICE, adapter, format, ## arg)
+	netdev_notice(adapter->netdev, format, ## arg)
 
 
 /* Interrupt modes, as used by the IntMode parameter */
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index e1cceb6..ad4546e 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -26,6 +26,8 @@
 
 *******************************************************************************/
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/init.h>
@@ -5394,10 +5396,9 @@ static struct pci_driver e1000_driver = {
 static int __init e1000_init_module(void)
 {
 	int ret;
-	printk(KERN_INFO "%s: Intel(R) PRO/1000 Network Driver - %s\n",
-	       e1000e_driver_name, e1000e_driver_version);
-	printk(KERN_INFO "%s: Copyright (c) 1999 - 2009 Intel Corporation.\n",
-	       e1000e_driver_name);
+	pr_info("Intel(R) PRO/1000 Network Driver - %s\n",
+		e1000e_driver_version);
+	pr_info("Copyright (c) 1999 - 2009 Intel Corporation.\n");
 	ret = pci_register_driver(&e1000_driver);
 
 	return ret;


^ permalink raw reply related


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