* [PATCH] deal with if frags[0].size is pulled to 0 in dev_gro_receive()
From: xiaohui.xin @ 2010-08-03 3:17 UTC (permalink / raw)
To: netdev, herbert, davem; +Cc: Xin Xiaohui
From: Xin Xiaohui <xiaohui.xin@intel.com>
Now in dev_gro_receive(), if frags[0].size is pulled to 0, memmove is called and
the null page is released. But it's not enough, we should reset size of each frags
left as well. Compared to this, we can have another way to do this, it's not do do
anything at all.
Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
---
net/core/dev.c | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 264137f..28cdbbf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2730,13 +2730,6 @@ pull:
skb_shinfo(skb)->frags[0].page_offset += grow;
skb_shinfo(skb)->frags[0].size -= grow;
-
- if (unlikely(!skb_shinfo(skb)->frags[0].size)) {
- put_page(skb_shinfo(skb)->frags[0].page);
- memmove(skb_shinfo(skb)->frags,
- skb_shinfo(skb)->frags + 1,
- --skb_shinfo(skb)->nr_frags);
- }
}
ok:
--
1.5.4.4
^ permalink raw reply related
* [PATCH v3 1/2] core: Factor out flow calculation from get_rps_cpu
From: Krishna Kumar @ 2010-08-03 3:02 UTC (permalink / raw)
To: davem, arnd; +Cc: bhutchings, netdev, therbert, Krishna Kumar, mst
From: Krishna Kumar <krkumar2@in.ibm.com>
Factor out flow calculation code from get_rps_cpu, since macvtap
driver can use the same code.
Revisions:
v2 - Ben: Separate flow calcuation out and use in select queue
v3 - Arnd: Don't re-implement MIN
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
include/linux/netdevice.h | 1
net/core/dev.c | 94 ++++++++++++++++++++++--------------
2 files changed, 59 insertions(+), 36 deletions(-)
diff -ruNp org/include/linux/netdevice.h new/include/linux/netdevice.h
--- org/include/linux/netdevice.h 2010-08-03 08:19:57.000000000 +0530
+++ new/include/linux/netdevice.h 2010-08-03 08:19:57.000000000 +0530
@@ -2253,6 +2253,7 @@ static inline const char *netdev_name(co
return dev->name;
}
+extern int skb_calculate_flow(struct net_device *dev, struct sk_buff *skb);
extern int netdev_printk(const char *level, const struct net_device *dev,
const char *format, ...)
__attribute__ ((format (printf, 3, 4)));
diff -ruNp org/net/core/dev.c new/net/core/dev.c
--- org/net/core/dev.c 2010-08-03 08:19:57.000000000 +0530
+++ new/net/core/dev.c 2010-08-03 08:19:57.000000000 +0530
@@ -2263,51 +2263,24 @@ static inline void ____napi_schedule(str
__raise_softirq_irqoff(NET_RX_SOFTIRQ);
}
-#ifdef CONFIG_RPS
-
-/* One global table that all flow-based protocols share. */
-struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
-EXPORT_SYMBOL(rps_sock_flow_table);
-
/*
- * 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.
- * rcu_read_lock must be held on entry.
+ * skb_calculate_flow: calculate a flow hash based on src/dst addresses
+ * and src/dst port numbers. On success, returns a hash number (> 0),
+ * otherwise -1.
*/
-static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
- struct rps_dev_flow **rflowp)
+int skb_calculate_flow(struct net_device *dev, struct sk_buff *skb)
{
+ int hash = skb->rxhash;
struct ipv6hdr *ip6;
struct iphdr *ip;
- struct netdev_rx_queue *rxqueue;
- struct rps_map *map;
- struct rps_dev_flow_table *flow_table;
- struct rps_sock_flow_table *sock_flow_table;
- int cpu = -1;
u8 ip_proto;
- u16 tcpu;
u32 addr1, addr2, ihl;
union {
u32 v32;
u16 v16[2];
} ports;
- if (skb_rx_queue_recorded(skb)) {
- u16 index = skb_get_rx_queue(skb);
- if (unlikely(index >= dev->num_rx_queues)) {
- WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
- "on queue %u, but number of RX queues is %u\n",
- dev->name, index, dev->num_rx_queues);
- goto done;
- }
- rxqueue = dev->_rx + index;
- } else
- rxqueue = dev->_rx;
-
- if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
- goto done;
-
- if (skb->rxhash)
+ if (hash)
goto got_hash; /* Skip hash computation on packet header */
switch (skb->protocol) {
@@ -2334,6 +2307,7 @@ static int get_rps_cpu(struct net_device
default:
goto done;
}
+
switch (ip_proto) {
case IPPROTO_TCP:
case IPPROTO_UDP:
@@ -2356,11 +2330,59 @@ static int get_rps_cpu(struct net_device
/* get a consistent hash (same value on both flow directions) */
if (addr2 < addr1)
swap(addr1, addr2);
- skb->rxhash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
- if (!skb->rxhash)
- skb->rxhash = 1;
+
+ hash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
+ if (!hash)
+ hash = 1;
got_hash:
+ return hash;
+
+done:
+ return -1;
+}
+EXPORT_SYMBOL(skb_calculate_flow);
+
+#ifdef CONFIG_RPS
+
+/* One global table that all flow-based protocols share. */
+struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
+EXPORT_SYMBOL(rps_sock_flow_table);
+
+/*
+ * 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.
+ * rcu_read_lock must be held on entry.
+ */
+static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
+ struct rps_dev_flow **rflowp)
+{
+ struct netdev_rx_queue *rxqueue;
+ struct rps_map *map;
+ struct rps_dev_flow_table *flow_table;
+ struct rps_sock_flow_table *sock_flow_table;
+ int cpu = -1;
+ u16 tcpu;
+
+ if (skb_rx_queue_recorded(skb)) {
+ u16 index = skb_get_rx_queue(skb);
+ if (unlikely(index >= dev->num_rx_queues)) {
+ WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
+ "on queue %u, but number of RX queues is %u\n",
+ dev->name, index, dev->num_rx_queues);
+ goto done;
+ }
+ rxqueue = dev->_rx + index;
+ } else
+ rxqueue = dev->_rx;
+
+ if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
+ goto done;
+
+ skb->rxhash = skb_calculate_flow(dev, skb);
+ if (skb->rxhash < 0)
+ goto done;
+
flow_table = rcu_dereference(rxqueue->rps_flow_table);
sock_flow_table = rcu_dereference(rps_sock_flow_table);
if (flow_table && sock_flow_table) {
^ permalink raw reply
* [PATCH v3 2/2] macvtap: Implement multiqueue macvtap driver
From: Krishna Kumar @ 2010-08-03 3:03 UTC (permalink / raw)
To: davem, arnd; +Cc: bhutchings, netdev, mst, Krishna Kumar, therbert
In-Reply-To: <20100803030256.8486.82622.sendpatchset@krkumar2.in.ibm.com>
From: Krishna Kumar <krkumar2@in.ibm.com>
Implement multiqueue facility for macvtap driver. The idea is that
a macvtap device can be opened multiple times and the fd's can be
used to register eg, as backend for vhost.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
drivers/net/macvtap.c | 89 ++++++++++++++++++++++++++++-------
include/linux/if_macvlan.h | 9 +++
2 files changed, 80 insertions(+), 18 deletions(-)
diff -ruNp org/include/linux/if_macvlan.h new/include/linux/if_macvlan.h
--- org/include/linux/if_macvlan.h 2010-08-03 08:19:57.000000000 +0530
+++ new/include/linux/if_macvlan.h 2010-08-03 08:20:39.000000000 +0530
@@ -40,6 +40,12 @@ struct macvlan_rx_stats {
unsigned long rx_errors;
};
+/*
+ * Maximum times a macvtap device can be opened. This can be used to
+ * configure the number of receive queue, e.g. for multiqueue virtio.
+ */
+#define MAX_MACVTAP_QUEUES (NR_CPUS < 16 ? NR_CPUS : 16)
+
struct macvlan_dev {
struct net_device *dev;
struct list_head list;
@@ -50,7 +56,8 @@ struct macvlan_dev {
enum macvlan_mode mode;
int (*receive)(struct sk_buff *skb);
int (*forward)(struct net_device *dev, struct sk_buff *skb);
- struct macvtap_queue *tap;
+ struct macvtap_queue *taps[MAX_MACVTAP_QUEUES];
+ int numvtaps;
};
static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
diff -ruNp org/drivers/net/macvtap.c new/drivers/net/macvtap.c
--- org/drivers/net/macvtap.c 2010-08-03 08:19:57.000000000 +0530
+++ new/drivers/net/macvtap.c 2010-08-03 08:19:57.000000000 +0530
@@ -84,26 +84,45 @@ static const struct proto_ops macvtap_so
static DEFINE_SPINLOCK(macvtap_lock);
/*
- * Choose the next free queue, for now there is only one
+ * get_slot: return a [unused/occupied] slot in vlan->taps[]:
+ * - if 'q' is NULL, return the first empty slot;
+ * - otherwise, return the slot this pointer occupies.
*/
+static int get_slot(struct macvlan_dev *vlan, struct macvtap_queue *q)
+{
+ int i;
+
+ for (i = 0; i < MAX_MACVTAP_QUEUES; i++) {
+ if (rcu_dereference(vlan->taps[i]) == q)
+ return i;
+ }
+
+ /* Should never happen */
+ BUG_ON(1);
+}
+
static int macvtap_set_queue(struct net_device *dev, struct file *file,
struct macvtap_queue *q)
{
struct macvlan_dev *vlan = netdev_priv(dev);
+ int index;
int err = -EBUSY;
spin_lock(&macvtap_lock);
- if (rcu_dereference(vlan->tap))
+ if (vlan->numvtaps == MAX_MACVTAP_QUEUES)
goto out;
err = 0;
+ index = get_slot(vlan, NULL);
rcu_assign_pointer(q->vlan, vlan);
- rcu_assign_pointer(vlan->tap, q);
+ rcu_assign_pointer(vlan->taps[index], q);
sock_hold(&q->sk);
q->file = file;
file->private_data = q;
+ vlan->numvtaps++;
+
out:
spin_unlock(&macvtap_lock);
return err;
@@ -124,9 +143,12 @@ static void macvtap_put_queue(struct mac
spin_lock(&macvtap_lock);
vlan = rcu_dereference(q->vlan);
if (vlan) {
- rcu_assign_pointer(vlan->tap, NULL);
+ int index = get_slot(vlan, q);
+
+ rcu_assign_pointer(vlan->taps[index], NULL);
rcu_assign_pointer(q->vlan, NULL);
sock_put(&q->sk);
+ --vlan->numvtaps;
}
spin_unlock(&macvtap_lock);
@@ -136,39 +158,72 @@ static void macvtap_put_queue(struct mac
}
/*
- * Since we only support one queue, just dereference the pointer.
+ * Select a queue based on the rxq of the device on which this packet
+ * arrived. If the incoming device is not mq, calculate a flow hash to
+ * select a queue. vlan->numvtaps is cached in case it reduces during
+ * the execution of this function.
*/
static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
struct sk_buff *skb)
{
struct macvlan_dev *vlan = netdev_priv(dev);
+ struct macvtap_queue *tap = NULL;
+ int numvtaps = vlan->numvtaps;
+ u16 rxq;
+
+ if (!numvtaps)
+ goto out;
+
+ if (likely(skb_rx_queue_recorded(skb))) {
+ rxq = skb_get_rx_queue(skb);
+
+ while (unlikely(rxq >= numvtaps))
+ rxq -= numvtaps;
- return rcu_dereference(vlan->tap);
+ tap = rcu_dereference(vlan->taps[rxq]);
+ if (tap)
+ goto out;
+ }
+
+ rxq = skb_calculate_flow(dev, skb);
+ if (rxq < 0)
+ rxq = smp_processor_id();
+
+ tap = rcu_dereference(vlan->taps[rxq & (numvtaps - 1)]);
+
+out:
+ return tap;
}
/*
* The net_device is going away, give up the reference
- * that it holds on the queue (all the queues one day)
- * and safely set the pointer from the queues to NULL.
+ * that it holds on all queues and safely set the pointer
+ * from the queues to NULL.
*/
static void macvtap_del_queues(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
- struct macvtap_queue *q;
+ struct macvtap_queue *q, *qlist[MAX_MACVTAP_QUEUES];
+ int i, j = 0;
+ /* macvtap_put_queue can free some slots, so go through all slots */
spin_lock(&macvtap_lock);
- q = rcu_dereference(vlan->tap);
- if (!q) {
- spin_unlock(&macvtap_lock);
- return;
+ for (i = 0; i < MAX_MACVTAP_QUEUES && vlan->numvtaps; i++) {
+ q = rcu_dereference(vlan->taps[i]);
+ if (q) {
+ qlist[j++] = q;
+ rcu_assign_pointer(vlan->taps[i], NULL);
+ rcu_assign_pointer(q->vlan, NULL);
+ vlan->numvtaps--;
+ }
}
-
- rcu_assign_pointer(vlan->tap, NULL);
- rcu_assign_pointer(q->vlan, NULL);
+ BUG_ON(vlan->numvtaps != 0);
spin_unlock(&macvtap_lock);
synchronize_rcu();
- sock_put(&q->sk);
+
+ for (--j; j >= 0; j--)
+ sock_put(&qlist[j]->sk);
}
/*
^ permalink raw reply
* [PATCH] Fixes a typo from "dev" to "ndev"
From: Henrique Camargo @ 2010-08-03 3:10 UTC (permalink / raw)
To: David Miller
Cc: chaithrika, srk, khilman, jpirko, netdev, linux-kernel, segooon
In-Reply-To: <20100730.222034.183058824.davem@davemloft.net>
The typo was causing compilation errors since "dev" was not defined.
Signed-off-by: Henrique Camargo <henrique.camargo@ensitec.com.br>
---
drivers/net/davinci_emac.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index 25e14d2..b89b7bf 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -1182,8 +1182,8 @@ static int emac_net_tx_complete(struct emac_priv *priv,
struct net_device *ndev = priv->ndev;
u32 cnt;
- if (unlikely(num_tokens && netif_queue_stopped(dev)))
- netif_start_queue(dev);
+ if (unlikely(num_tokens && netif_queue_stopped(ndev)))
+ netif_start_queue(ndev);
for (cnt = 0; cnt < num_tokens; cnt++) {
struct sk_buff *skb = (struct sk_buff *)net_data_tokens[cnt];
if (skb == NULL)
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v3 1/2] core: Factor out flow calculation from get_rps_cpu
From: Changli Gao @ 2010-08-03 4:05 UTC (permalink / raw)
To: Krishna Kumar; +Cc: davem, arnd, bhutchings, netdev, therbert, mst
In-Reply-To: <20100803030256.8486.82622.sendpatchset@krkumar2.in.ibm.com>
On Tue, Aug 3, 2010 at 11:02 AM, Krishna Kumar <krkumar2@in.ibm.com> wrote:
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> Factor out flow calculation code from get_rps_cpu, since macvtap
> driver can use the same code.
>
> Revisions:
>
> v2 - Ben: Separate flow calcuation out and use in select queue
> v3 - Arnd: Don't re-implement MIN
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
> include/linux/netdevice.h | 1
> net/core/dev.c | 94 ++++++++++++++++++++++--------------
> 2 files changed, 59 insertions(+), 36 deletions(-)
>
> diff -ruNp org/include/linux/netdevice.h new/include/linux/netdevice.h
> --- org/include/linux/netdevice.h 2010-08-03 08:19:57.000000000 +0530
> +++ new/include/linux/netdevice.h 2010-08-03 08:19:57.000000000 +0530
> @@ -2253,6 +2253,7 @@ static inline const char *netdev_name(co
> return dev->name;
> }
>
> +extern int skb_calculate_flow(struct net_device *dev, struct sk_buff *skb);
> extern int netdev_printk(const char *level, const struct net_device *dev,
> const char *format, ...)
> __attribute__ ((format (printf, 3, 4)));
> diff -ruNp org/net/core/dev.c new/net/core/dev.c
> --- org/net/core/dev.c 2010-08-03 08:19:57.000000000 +0530
> +++ new/net/core/dev.c 2010-08-03 08:19:57.000000000 +0530
> @@ -2263,51 +2263,24 @@ static inline void ____napi_schedule(str
> __raise_softirq_irqoff(NET_RX_SOFTIRQ);
> }
>
> -#ifdef CONFIG_RPS
> -
> -/* One global table that all flow-based protocols share. */
> -struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
> -EXPORT_SYMBOL(rps_sock_flow_table);
> -
> /*
> - * 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.
> - * rcu_read_lock must be held on entry.
> + * skb_calculate_flow: calculate a flow hash based on src/dst addresses
> + * and src/dst port numbers. On success, returns a hash number (> 0),
> + * otherwise -1.
> */
> -static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
> - struct rps_dev_flow **rflowp)
> +int skb_calculate_flow(struct net_device *dev, struct sk_buff *skb)
> {
> + int hash = skb->rxhash;
> struct ipv6hdr *ip6;
> struct iphdr *ip;
> - struct netdev_rx_queue *rxqueue;
> - struct rps_map *map;
> - struct rps_dev_flow_table *flow_table;
> - struct rps_sock_flow_table *sock_flow_table;
> - int cpu = -1;
> u8 ip_proto;
> - u16 tcpu;
> u32 addr1, addr2, ihl;
> union {
> u32 v32;
> u16 v16[2];
> } ports;
>
> - if (skb_rx_queue_recorded(skb)) {
> - u16 index = skb_get_rx_queue(skb);
> - if (unlikely(index >= dev->num_rx_queues)) {
> - WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
> - "on queue %u, but number of RX queues is %u\n",
> - dev->name, index, dev->num_rx_queues);
> - goto done;
> - }
> - rxqueue = dev->_rx + index;
> - } else
> - rxqueue = dev->_rx;
> -
> - if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
> - goto done;
> -
> - if (skb->rxhash)
> + if (hash)
> goto got_hash; /* Skip hash computation on packet header */
>
> switch (skb->protocol) {
> @@ -2334,6 +2307,7 @@ static int get_rps_cpu(struct net_device
> default:
> goto done;
> }
> +
> switch (ip_proto) {
> case IPPROTO_TCP:
> case IPPROTO_UDP:
> @@ -2356,11 +2330,59 @@ static int get_rps_cpu(struct net_device
> /* get a consistent hash (same value on both flow directions) */
> if (addr2 < addr1)
> swap(addr1, addr2);
> - skb->rxhash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
> - if (!skb->rxhash)
> - skb->rxhash = 1;
> +
> + hash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
> + if (!hash)
> + hash = 1;
>
> got_hash:
> + return hash;
> +
> +done:
> + return -1;
> +}
> +EXPORT_SYMBOL(skb_calculate_flow);
I have noticed that you use skb_calculate_flow() in
macvtap_get_queue() where skb->data doesn't point to the network
header but the ethernet header. However, skb_calculate_flow() assume
skb->data points to the network header. There are two choices:
* update skb_calculate_flow to support called in ethernet layer.
* pull skb before skb_calculate_flow, and push skb after
skb_calculate_flow() in macvtap_get_queue().
I prefer the former way.
BTW: the function name skb_calculate_flow isn't good. How about
skb_get_rxhash(). Maybe we can implement two versions: fast path and
slow path. And implement the fast path version as a inline function in
skbuff.h.
static inline u32 skb_get_rxhash(struct sk_buff *skb)
{
u32 rxhash;
rxhash = skb->rxhash;
if (!rxhash)
return __skb_get_rxhash(skb);
return rxhash;
}
> +
> +#ifdef CONFIG_RPS
> +
> +/* One global table that all flow-based protocols share. */
> +struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
> +EXPORT_SYMBOL(rps_sock_flow_table);
> +
> +/*
> + * 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.
> + * rcu_read_lock must be held on entry.
> + */
> +static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
> + struct rps_dev_flow **rflowp)
> +{
> + struct netdev_rx_queue *rxqueue;
> + struct rps_map *map;
> + struct rps_dev_flow_table *flow_table;
> + struct rps_sock_flow_table *sock_flow_table;
> + int cpu = -1;
> + u16 tcpu;
> +
> + if (skb_rx_queue_recorded(skb)) {
> + u16 index = skb_get_rx_queue(skb);
> + if (unlikely(index >= dev->num_rx_queues)) {
> + WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
> + "on queue %u, but number of RX queues is %u\n",
> + dev->name, index, dev->num_rx_queues);
> + goto done;
> + }
> + rxqueue = dev->_rx + index;
> + } else
> + rxqueue = dev->_rx;
> +
> + if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
> + goto done;
> +
> + skb->rxhash = skb_calculate_flow(dev, skb);
> + if (skb->rxhash < 0)
> + goto done;
> +
> flow_table = rcu_dereference(rxqueue->rps_flow_table);
> sock_flow_table = rcu_dereference(rps_sock_flow_table);
> if (flow_table && sock_flow_table) {
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [net-2.6 PATCH] e1000e: 82577/82578 PHY register access issues
From: David Miller @ 2010-08-03 4:10 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, stable, bruce.w.allan
In-Reply-To: <AANLkTi=P_+3wpmAA5+eqaWoGE39OkZPFhRXEuXGypAco@mail.gmail.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 2 Aug 2010 18:04:43 -0700
> Have you sync'd up your net-next-2.6 tree with net-2.6 tree? Because
> I do not see this change in the net-next-2.6 tree.
No, I plan to do that tonight or tomorrow.
^ permalink raw reply
* Re: [net-next-2.6 PATCH 1/3] e1000e: register pm_qos request on hardware activation
From: David Miller @ 2010-08-03 4:21 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, florian
In-Reply-To: <20100803002622.4179.31850.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 02 Aug 2010 17:27:00 -0700
> From: Florian Mickler <florian@mickler.org>
>
> The pm_qos_add_request call has to register the pm_qos request with the pm_qos
> susbsystem before first use of the pm_qos request via
> pm_qos_update_request.
>
> As pm_qos changed to use plists there is no benefit in registering and
> unregistering the pm_qos request on ifup/ifdown and thus we move the
> registering into e1000_open and the unregistering in e1000_close.
>
> This fixes the following warning:
...
> Signed-off-by: Florian Mickler <florian@mickler.org>
> Tested-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net-next-2.6 PATCH 2/3] e1000e: Fix irq_synchronize in MSI-X case
From: David Miller @ 2010-08-03 4:21 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: netdev, gospo, bphilips, jdelvare, jesse.brandeburg,
bruce.w.allan
In-Reply-To: <20100803002721.4179.75916.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 02 Aug 2010 17:27:23 -0700
> Based on original patch/work from Jean Delvare <jdelvare@suse.de>
> Synchronize all IRQs when in MSI-X IRQ mode.
>
> Jean's original patch hard coded the sync with the 3 possible vectors,
> this patch incorporates more flexibility for the future and aligns
> with how igb stores the number of vectors into the adapter structure.
>
> CC: Jean Delvare <jdelvare@suse.de>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Acked-by: Bruce Allan <bruce.w.allan@intel.com>
Applied.
^ permalink raw reply
* Re: [net-next-2.6 PATCH 3/3] e1000e: update to workaround for jumbo frames on 82577
From: David Miller @ 2010-08-03 4:21 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, bruce.w.allan
In-Reply-To: <AANLkTi=TvRmXxJu1+y9B8x7G55Ur9wNhYzDZ8Q=+HJRJ@mail.gmail.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 2 Aug 2010 17:36:56 -0700
> On Mon, Aug 2, 2010 at 17:27, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
>> From: Bruce Allan <bruce.w.allan@intel.com>
>>
>> For OEM systems with this part that also has Spread Spectrum Clocking (SSC)
>> enabled in the BIOS, there is an Rx performance issue with 4K jumbo frames.
>> Leaving the defaults in PHY page 770 register 26 resolves the issue, and
>> does not negatively impact jumbo frames on systems with SSC disabled.
>>
>> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>>
>> drivers/net/e1000e/netdev.c | 5 -----
>> 1 files changed, 0 insertions(+), 5 deletions(-)
>>
>
> Please disregard this patch, it was sent out accidentally (my bad).
> During testing issues were found and changes need to be made to this
> patch.
Ok.
^ permalink raw reply
* Re: [net-next-2.6 PATCH] igb: Use irq_synchronize per vector when using MSI-X
From: David Miller @ 2010-08-03 4:21 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, jdelvare, emil.s.tantilov
In-Reply-To: <20100803004044.4441.34335.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 02 Aug 2010 17:40:52 -0700
> From: Emil Tantilov <emil.s.tantilov@intel.com>
>
> Synchronize all IRQs when using MSI-X. Similar to ixgbe.
> Issue was reported on e1000e, but the patch is also valid for igb.
>
> CC: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ixgbevf: fix null pointer dereference due to filter being set for VLAN 0
From: David Miller @ 2010-08-03 4:21 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, alexander.h.duyck
In-Reply-To: <20100803005849.4678.10583.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 02 Aug 2010 17:59:04 -0700
> From: Alexander Duyck <alexander.h.duyck@intel.com>
>
> This change corrects an issue that resulted in a null pointer dereference
> for the addition of VLAN 0 without any VLANs being registered. Also this
> code removes some unnecessary checks for defines and the unnecessary setting
> of VLAN flags since that is now handled within the kernel via the
> vlan_features.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Tested-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH] deal with if frags[0].size is pulled to 0 in dev_gro_receive()
From: Herbert Xu @ 2010-08-03 4:56 UTC (permalink / raw)
To: xiaohui.xin; +Cc: netdev, davem
In-Reply-To: <1280805439-18988-1-git-send-email-xiaohui.xin@intel.com>
On Tue, Aug 03, 2010 at 11:17:19AM +0800, xiaohui.xin@intel.com wrote:
> From: Xin Xiaohui <xiaohui.xin@intel.com>
>
> Now in dev_gro_receive(), if frags[0].size is pulled to 0, memmove is called and
> the null page is released. But it's not enough, we should reset size of each frags
> left as well. Compared to this, we can have another way to do this, it's not do do
> anything at all.
>
> Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
This patch can only work if you audit everything that uses skb
frags to ensure that they can tolerate a zero-sided frag.
I think it's much easier to just fix the memmove.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] Fixes a typo from "dev" to "ndev"
From: David Miller @ 2010-08-03 5:03 UTC (permalink / raw)
To: henrique.camargo
Cc: chaithrika, srk, khilman, jpirko, netdev, linux-kernel, segooon
In-Reply-To: <1280805042.2089.6.camel@lemming>
From: Henrique Camargo <henrique.camargo@ensitec.com.br>
Date: Tue, 03 Aug 2010 00:10:42 -0300
> The typo was causing compilation errors since "dev" was not defined.
>
> Signed-off-by: Henrique Camargo <henrique.camargo@ensitec.com.br>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] deal with if frags[0].size is pulled to 0 in dev_gro_receive()
From: David Miller @ 2010-08-03 5:03 UTC (permalink / raw)
To: herbert; +Cc: xiaohui.xin, netdev
In-Reply-To: <20100803045637.GA14173@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 3 Aug 2010 12:56:38 +0800
> On Tue, Aug 03, 2010 at 11:17:19AM +0800, xiaohui.xin@intel.com wrote:
>> From: Xin Xiaohui <xiaohui.xin@intel.com>
>>
>> Now in dev_gro_receive(), if frags[0].size is pulled to 0, memmove is called and
>> the null page is released. But it's not enough, we should reset size of each frags
>> left as well. Compared to this, we can have another way to do this, it's not do do
>> anything at all.
>>
>> Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
>
> This patch can only work if you audit everything that uses skb
> frags to ensure that they can tolerate a zero-sided frag.
>
> I think it's much easier to just fix the memmove.
Agreed.
^ permalink raw reply
* Re: [PATCH] u32: negative offset fix
From: David Miller @ 2010-08-03 5:08 UTC (permalink / raw)
To: shemminger; +Cc: xiaosuo, netdev
In-Reply-To: <20100802164413.6f327ce6@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 2 Aug 2010 16:44:13 -0700
> It was possible to use a negative offset in a u32 match to reference
> the ethernet header or other parts of the link layer header.
> This fixes the regression caused by:
>
> commit fbc2e7d9cf49e0bf89b9e91fd60a06851a855c5d
> Author: Changli Gao <xiaosuo@gmail.com>
> Date: Wed Jun 2 07:32:42 2010 -0700
>
> cls_u32: use skb_header_pointer() to dereference data safely
>
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 0/2] Minor extensions to marvell phy driver
From: David Miller @ 2010-08-03 5:09 UTC (permalink / raw)
To: cyril; +Cc: netdev
In-Reply-To: <1280778294-2993-1-git-send-email-cyril@ti.com>
From: Cyril Chemparathy <cyril@ti.com>
Date: Mon, 2 Aug 2010 15:44:52 -0400
> phy/marvell: add 88e1121 interface mode support
Applied.
> phy/marvell: add 88ec048 support
You have to update marvel_id_tbl otherwise you break PHY
module autoloading.
Please fix this and resubmit this second patch.
Thanks.
^ permalink raw reply
* Re: RFC: New BGF 'LOOP' instruction
From: David Miller @ 2010-08-03 5:13 UTC (permalink / raw)
To: leonerd; +Cc: netdev
In-Reply-To: <20100802110334.GK11110@cel.leo>
From: Paul LeoNerd Evans <leonerd@leonerd.org.uk>
Date: Mon, 2 Aug 2010 12:03:34 +0100
> Any comments on this, while I proceed? Barring any major complaints,
> I'll have a hack at some code and present a patch in due course...
We're not adding loop instructions, it's just asking for trouble
since any user can attach BPF filters to a socket and it's just
way too easy to make a loop endless.
There's a reason no loop primitives were added to the original
BPF specification, perhaps you should take a look at what their
reasoning was.
It still applies now.
^ permalink raw reply
* Re: RFC: New BGF 'LOOP' instruction
From: David Miller @ 2010-08-03 5:18 UTC (permalink / raw)
To: hagen; +Cc: leonerd, netdev
In-Reply-To: <20100802201629.GA5973@nuttenaction>
From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Mon, 2 Aug 2010 22:16:29 +0200
> PS: the LOOP opcode must be secure against any ressource attack -> the loop
> must be break after n iterations.
Oh yeah, what is an iteration in your definition? See this is why I
totally refuse to add a looping construct to BPF.
If you just check for a single loop hitting, the user will just use
a chaining of two looping constructs. And then three levels of
indirection, then four, etc. He can run up to just before exhasting
the "iteration limit" of one loop, and branch to the next one, and
so on and so forth.
There are probably a million ways to exploit this, and once you come
up with a validation or limiting scheme one of two things will happen:
1) The limiting scheme will make legitimate scripts USELESS
2) Someone will just figure out another hole to punch through and
exploit
There's a reason no back branching construct was added to BPF to begin
with. It violates the core design principles of BPF.
^ permalink raw reply
* [PATCH] net: cleanup inclusion
From: Changli Gao @ 2010-08-02 22:11 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Changli Gao
Commit ab95bfe01f9872459c8678572ccadbf646badad0 replaces bridge and macvlan
hooks in __netif_receive_skb(), so dev.c doesn't need to include their headers.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 5d1282d..8c663db 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -101,8 +101,6 @@
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
#include <linux/stat.h>
-#include <linux/if_bridge.h>
-#include <linux/if_macvlan.h>
#include <net/dst.h>
#include <net/pkt_sched.h>
#include <net/checksum.h>
^ permalink raw reply related
* Re: [PATCH] net: cleanup inclusion
From: David Miller @ 2010-08-03 5:45 UTC (permalink / raw)
To: xiaosuo; +Cc: netdev
In-Reply-To: <1280787101-5589-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Tue, 3 Aug 2010 06:11:41 +0800
> Commit ab95bfe01f9872459c8678572ccadbf646badad0 replaces bridge and macvlan
> hooks in __netif_receive_skb(), so dev.c doesn't need to include their headers.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Applied, thanks.
^ permalink raw reply
* reminder: bug fixes only
From: David Miller @ 2010-08-03 5:48 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA; +Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA
2.6.35 is out, we're about to enter the merge window, so
if it isn't already in my tree I don't want to see it unless
it's a bug fix.
Thanks.
--
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 v3 1/2] core: Factor out flow calculation from get_rps_cpu
From: Krishna Kumar2 @ 2010-08-03 5:57 UTC (permalink / raw)
To: Changli Gao; +Cc: arnd, bhutchings, davem, mst, netdev, therbert
In-Reply-To: <AANLkTikNOstJr3qqMUqtUoOm1xHOjw5uM7NpQUNJz4PR@mail.gmail.com>
Hi Changli,
Good catch.
Instead of adding support for ethernet header or pull/push,
I could defer the skb_push(ETH_HLEN), something like:
static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
{
struct macvtap_queue *q = macvtap_get_queue(dev, skb);
if (!q)
goto drop;
if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
goto drop;
+ skb_push(skb, ETH_HLEN);
...
}
and remove the same in macvtap_receive. Will this be better?
Your other suggestions also looks good.
Thanks,
- KK
Changli Gao <xiaosuo@gmail.com> wrote on 08/03/2010 09:35:34 AM:
> Changli Gao <xiaosuo@gmail.com>
> 08/03/2010 09:35 AM
>
> To
>
> Krishna Kumar2/India/IBM@IBMIN
>
> cc
>
> davem@davemloft.net, arnd@arndb.de, bhutchings@solarflare.com,
> netdev@vger.kernel.org, therbert@google.com, mst@redhat.com
>
> Subject
>
> Re: [PATCH v3 1/2] core: Factor out flow calculation from get_rps_cpu
>
> On Tue, Aug 3, 2010 at 11:02 AM, Krishna Kumar <krkumar2@in.ibm.com>
wrote:
> > From: Krishna Kumar <krkumar2@in.ibm.com>
> >
> > Factor out flow calculation code from get_rps_cpu, since macvtap
> > driver can use the same code.
> >
> > Revisions:
> >
> > v2 - Ben: Separate flow calcuation out and use in select queue
> > v3 - Arnd: Don't re-implement MIN
> >
> > Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> > ---
> > include/linux/netdevice.h | 1
> > net/core/dev.c | 94 ++++++++++++++++++++++--------------
> > 2 files changed, 59 insertions(+), 36 deletions(-)
> >
> > diff -ruNp org/include/linux/netdevice.h new/include/linux/netdevice.h
> > --- org/include/linux/netdevice.h 2010-08-03 08:19:57.000000000
+0530
> > +++ new/include/linux/netdevice.h 2010-08-03 08:19:57.000000000
+0530
> > @@ -2253,6 +2253,7 @@ static inline const char *netdev_name(co
> > return dev->name;
> > }
> >
> > +extern int skb_calculate_flow(struct net_device *dev, struct sk_buff
*skb);
> > extern int netdev_printk(const char *level, const struct net_device
*dev,
> > const char *format, ...)
> > __attribute__ ((format (printf, 3, 4)));
> > diff -ruNp org/net/core/dev.c new/net/core/dev.c
> > --- org/net/core/dev.c 2010-08-03 08:19:57.000000000 +0530
> > +++ new/net/core/dev.c 2010-08-03 08:19:57.000000000 +0530
> > @@ -2263,51 +2263,24 @@ static inline void ____napi_schedule(str
> > __raise_softirq_irqoff(NET_RX_SOFTIRQ);
> > }
> >
> > -#ifdef CONFIG_RPS
> > -
> > -/* One global table that all flow-based protocols share. */
> > -struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
> > -EXPORT_SYMBOL(rps_sock_flow_table);
> > -
> > /*
> > - * 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.
> > - * rcu_read_lock must be held on entry.
> > + * skb_calculate_flow: calculate a flow hash based on src/dst
addresses
> > + * and src/dst port numbers. On success, returns a hash number (> 0),
> > + * otherwise -1.
> > */
> > -static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
> > - struct rps_dev_flow **rflowp)
> > +int skb_calculate_flow(struct net_device *dev, struct sk_buff *skb)
> > {
> > + int hash = skb->rxhash;
> > struct ipv6hdr *ip6;
> > struct iphdr *ip;
> > - struct netdev_rx_queue *rxqueue;
> > - struct rps_map *map;
> > - struct rps_dev_flow_table *flow_table;
> > - struct rps_sock_flow_table *sock_flow_table;
> > - int cpu = -1;
> > u8 ip_proto;
> > - u16 tcpu;
> > u32 addr1, addr2, ihl;
> > union {
> > u32 v32;
> > u16 v16[2];
> > } ports;
> >
> > - if (skb_rx_queue_recorded(skb)) {
> > - u16 index = skb_get_rx_queue(skb);
> > - if (unlikely(index >= dev->num_rx_queues)) {
> > - WARN_ONCE(dev->num_rx_queues > 1, "%s
> received packet "
> > - "on queue %u, but number of RX
> queues is %u\n",
> > - dev->name, index, dev->num_rx_queues);
> > - goto done;
> > - }
> > - rxqueue = dev->_rx + index;
> > - } else
> > - rxqueue = dev->_rx;
> > -
> > - if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
> > - goto done;
> > -
> > - if (skb->rxhash)
> > + if (hash)
> > goto got_hash; /* Skip hash computation on packet header
*/
> >
> > switch (skb->protocol) {
> > @@ -2334,6 +2307,7 @@ static int get_rps_cpu(struct net_device
> > default:
> > goto done;
> > }
> > +
> > switch (ip_proto) {
> > case IPPROTO_TCP:
> > case IPPROTO_UDP:
> > @@ -2356,11 +2330,59 @@ static int get_rps_cpu(struct net_device
> > /* get a consistent hash (same value on both flow directions) */
> > if (addr2 < addr1)
> > swap(addr1, addr2);
> > - skb->rxhash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
> > - if (!skb->rxhash)
> > - skb->rxhash = 1;
> > +
> > + hash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
> > + if (!hash)
> > + hash = 1;
> >
> > got_hash:
> > + return hash;
> > +
> > +done:
> > + return -1;
> > +}
> > +EXPORT_SYMBOL(skb_calculate_flow);
>
> I have noticed that you use skb_calculate_flow() in
> macvtap_get_queue() where skb->data doesn't point to the network
> header but the ethernet header. However, skb_calculate_flow() assume
> skb->data points to the network header. There are two choices:
> * update skb_calculate_flow to support called in ethernet layer.
> * pull skb before skb_calculate_flow, and push skb after
> skb_calculate_flow() in macvtap_get_queue().
>
> I prefer the former way.
>
> BTW: the function name skb_calculate_flow isn't good. How about
> skb_get_rxhash(). Maybe we can implement two versions: fast path and
> slow path. And implement the fast path version as a inline function in
> skbuff.h.
>
> static inline u32 skb_get_rxhash(struct sk_buff *skb)
> {
> u32 rxhash;
>
> rxhash = skb->rxhash;
> if (!rxhash)
> return __skb_get_rxhash(skb);
> return rxhash;
> }
>
>
> > +
> > +#ifdef CONFIG_RPS
> > +
> > +/* One global table that all flow-based protocols share. */
> > +struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
> > +EXPORT_SYMBOL(rps_sock_flow_table);
> > +
> > +/*
> > + * 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.
> > + * rcu_read_lock must be held on entry.
> > + */
> > +static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
> > + struct rps_dev_flow **rflowp)
> > +{
> > + struct netdev_rx_queue *rxqueue;
> > + struct rps_map *map;
> > + struct rps_dev_flow_table *flow_table;
> > + struct rps_sock_flow_table *sock_flow_table;
> > + int cpu = -1;
> > + u16 tcpu;
> > +
> > + if (skb_rx_queue_recorded(skb)) {
> > + u16 index = skb_get_rx_queue(skb);
> > + if (unlikely(index >= dev->num_rx_queues)) {
> > + WARN_ONCE(dev->num_rx_queues > 1, "%s
> received packet "
> > + "on queue %u, but number of RX
> queues is %u\n",
> > + dev->name, index, dev->num_rx_queues);
> > + goto done;
> > + }
> > + rxqueue = dev->_rx + index;
> > + } else
> > + rxqueue = dev->_rx;
> > +
> > + if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
> > + goto done;
> > +
> > + skb->rxhash = skb_calculate_flow(dev, skb);
> > + if (skb->rxhash < 0)
> > + goto done;
> > +
> > flow_table = rcu_dereference(rxqueue->rps_flow_table);
> > sock_flow_table = rcu_dereference(rps_sock_flow_table);
> > if (flow_table && sock_flow_table) {
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
>
>
> --
> Regards,
> Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH v3 1/2] core: Factor out flow calculation from get_rps_cpu
From: Changli Gao @ 2010-08-03 6:11 UTC (permalink / raw)
To: Krishna Kumar2; +Cc: arnd, bhutchings, davem, mst, netdev, therbert
In-Reply-To: <OF46AA822C.08F8EAED-ON65257774.001F5AE3-65257774.00209BBF@in.ibm.com>
On Tue, Aug 3, 2010 at 1:57 PM, Krishna Kumar2 <krkumar2@in.ibm.com> wrote:
> Hi Changli,
>
> Good catch.
>
> Instead of adding support for ethernet header or pull/push,
> I could defer the skb_push(ETH_HLEN), something like:
>
> static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
> {
> struct macvtap_queue *q = macvtap_get_queue(dev, skb);
> if (!q)
> goto drop;
>
> if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
> goto drop;
>
> + skb_push(skb, ETH_HLEN);
> ...
> }
>
> and remove the same in macvtap_receive. Will this be better?
>
I am confused by the call sites of macvlan_dev.receive and
macvlan_dev.forward. They both are possible to be called in both
RX(skb->data points to network header) and TX(skb->data points to
ethernet) paths. The current code in macvtap shows that
macvlan_dev.receive should be called in network layer, and
macvlan_dev.forward should be called in dev layer. Am I correct?
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: Is it a possible bug in dev_gro_receive()?
From: Jarek Poplawski @ 2010-08-03 6:45 UTC (permalink / raw)
To: Xin, Xiaohui
Cc: netdev@vger.kernel.org, herbert@gondor.apana.org.au,
davem@davemloft.net
In-Reply-To: <F2E9EB7348B8264F86B6AB8151CE2D791BAB3078E0@shsmsx502.ccr.corp.intel.com>
On Tue, Aug 03, 2010 at 10:33:24AM +0800, Xin, Xiaohui wrote:
> >-----Original Message-----
> >From: Jarek Poplawski [mailto:jarkao2@gmail.com]
> >Sent: Monday, August 02, 2010 6:29 PM
> >To: Xin, Xiaohui
> >Cc: netdev@vger.kernel.org; herbert@gondor.apana.org.au; davem@davemloft.net
> >Subject: Re: Is it a possible bug in dev_gro_receive()?
> >
> >Xin Xiaohui wrote:
> >> I looked into the code dev_gro_receive(), found the code here:
> >> if the frags[0] is pulled to 0, then the page will be released,
> >> and memmove() frags left.
> >> Is that right? I'm not sure if memmove do right or not, but
> >> frags[0].size is never set after memove at least. what I think
> >> a simple way is not to do anything if we found frags[0].size == 0.
> >> The patch is as followed.
> >>
> >> Or am I missing something here?
> >
> >I think, you're right, but fixing memmove looks nicer to me:
> >
> > - --skb_shinfo(skb)->nr_frags);
> > + --skb_shinfo(skb)->nr_frags * sizeof(skb_frag_t));
> >
> >Jarek P.
>
> Is there a little hurt of performance to do memmove() if skb_shinfo(skb)->nr_frags is large?
> We're now working on the zero-copy patches based on napi_gro_frags() interface, and in
> this case, we have found a lot of skbs which frags[0] is pulled to 0. And after the memmove is
> fixed, each frags[x].size is needed to modify too.
> So I think don't do anything is better. Or is there any side effect with a null page in the stack?
Even if it's better, generally you should separate fixes from
optimizations. On the other hand, it was expected to be "unlikely" by
design, so you should probably explain more why it has to be changed
here too.
Thanks,
Jarek P.
>
> Thanks
> Xiaohui
> >
> >>
> >> ---
> >> net/core/dev.c | 7 -------
> >> 1 files changed, 0 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/net/core/dev.c b/net/core/dev.c
> >> index 264137f..28cdbbf 100644
> >> --- a/net/core/dev.c
> >> +++ b/net/core/dev.c
> >> @@ -2730,13 +2730,6 @@ pull:
> >>
> >> skb_shinfo(skb)->frags[0].page_offset += grow;
> >> skb_shinfo(skb)->frags[0].size -= grow;
> >> -
> >> - if (unlikely(!skb_shinfo(skb)->frags[0].size)) {
> >> - put_page(skb_shinfo(skb)->frags[0].page);
> >> - memmove(skb_shinfo(skb)->frags,
> >> - skb_shinfo(skb)->frags + 1,
> >> - --skb_shinfo(skb)->nr_frags);
> >> - }
> >> }
> >>
> >> ok:
> >
> >
>
^ permalink raw reply
* Re: RFC: New BGF 'LOOP' instruction
From: Paul LeoNerd Evans @ 2010-08-03 7:04 UTC (permalink / raw)
To: David Miller, netdev
In-Reply-To: <20100802.221341.137851732.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]
On Mon, Aug 02, 2010 at 10:13:41PM -0700, David Miller wrote:
> > Any comments on this, while I proceed? Barring any major complaints,
> > I'll have a hack at some code and present a patch in due course...
>
> We're not adding loop instructions, it's just asking for trouble
> since any user can attach BPF filters to a socket and it's just
> way too easy to make a loop endless.
>
> There's a reason no loop primitives were added to the original
> BPF specification, perhaps you should take a look at what their
> reasoning was.
Yes. I am very aware of that.
Please read carefully my suggestion. These loops cannot be made endless
- they will be bounded by, at most, the number of bytes in the packet
buffer. The loop is required to increment X at least 1 at every
iteration, and will not allow it to continue past the end of the packet.
This puts a strict bound on the runtime of the loop.
--
Paul "LeoNerd" Evans
leonerd@leonerd.org.uk
ICQ# 4135350 | Registered Linux# 179460
http://www.leonerd.org.uk/
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox